[PATCH] new test fails "add -p" for adds on the top line

Subsystems: the rest

DORMANTno replies

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH] new test fails "add -p" for adds on the top line

From: Matt Graham <hidden>
Date: 2016-06-15 22:46:47

add -p doesn't work for some diffs.  diffs adding a new line at the top of
the file with other adds later in the file are one way to trigger the problem.

during add -p, split the diff and then answer y for all segments.  the file
won't have been added to the index.

Signed-off-by: Matthew Graham <redacted>
---
 t/t3701-add-interactive.sh |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index dfc6560..45da6c8 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -163,6 +163,38 @@ test_expect_success FILEMODE 'stage mode but not hunk' '
 	git diff          file | grep "+content"
 '

+# Write the patch file with a new line at the top and bottom
+cat >patch <<EOF
+index 180b47c..b6f2c08 100644
+--- a/file
++++ b/file
+@@ -1,2 +1,4 @@
++firstline
+ baseline
+ content
++lastline
+EOF
+# Expected output, similar to the patch but w/ diff at the top
+cat >expected <<EOF
+diff --git a/file b/file
+index b6f2c08..61b9053 100755
+--- a/file
++++ b/file
+@@ -1,2 +1,4 @@
++firstline
+ baseline
+ content
++lastline
+EOF
+# Test splitting the first patch, then adding both
+test_expect_failure 'add first line works' '
+	git commit -am "clear local changes" &&
+	git apply patch &&
+	(echo s; echo y; echo y) | git add -p file &&
+	git diff --cached > diff &&
+	test_cmp expected diff
+'
+
 # end of tests disabled when filemode is not usable

 test_done
-- 
1.6.3.9.g6345

Re: [PATCH] new test fails "add -p" for adds on the top line

From: Nanako Shiraishi <hidden>
Date: 2016-06-15 22:46:47

Quoting Matt Graham [off-list ref]:
add -p doesn't work for some diffs.  diffs adding a new line at the top of
the file with other adds later in the file are one way to trigger the problem.

during add -p, split the diff and then answer y for all segments.  the file
won't have been added to the index.

Signed-off-by: Matthew Graham <redacted>
I tried "git-add -p" from different versions and I found out that versions before the commit 0beee4c6dec15292415e3d56075c16a76a22af54 doesn't have this problem.

commit 0beee4c6dec15292415e3d56075c16a76a22af54
Author: Thomas Rast [off-list ref]
Date:   Wed Jul 2 23:59:44 2008 +0200

    git-add--interactive: remove hunk coalescing
    
    Current git-apply has no trouble at all applying chunks that have
    overlapping context, as produced by the splitting feature. So we can
    drop the manual coalescing.
    
    Signed-off-by: Thomas Rast [off-list ref]
    Signed-off-by: Junio C Hamano [off-list ref]

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [PATCH] new test fails "add -p" for adds on the top line

From: Thomas Rast <hidden>
Date: 2016-06-15 22:46:47

Nanako Shiraishi wrote:
Quoting Matt Graham [off-list ref]:
quoted
add -p doesn't work for some diffs.  diffs adding a new line at the top of
the file with other adds later in the file are one way to trigger the problem.

during add -p, split the diff and then answer y for all segments.  the file
won't have been added to the index.

Signed-off-by: Matthew Graham <redacted>
I tried "git-add -p" from different versions and I found out that versions before the commit 0beee4c6dec15292415e3d56075c16a76a22af54 doesn't have this problem.

commit 0beee4c6dec15292415e3d56075c16a76a22af54
Author: Thomas Rast [off-list ref]
Date:   Wed Jul 2 23:59:44 2008 +0200

    git-add--interactive: remove hunk coalescing
    
    Current git-apply has no trouble at all applying chunks that have
    overlapping context, as produced by the splitting feature. So we can
    drop the manual coalescing.
    
    Signed-off-by: Thomas Rast [off-list ref]
    Signed-off-by: Junio C Hamano [off-list ref]
The above commit still reverts cleanly, but AFAICS merge_hunk blindly
trusts the hunk headers, an assumption that is no longer valid due to
the 'edit' feature.  So either we need to recount the hunk headers
prior to merging (which was rejected back in the 'edit' feature
discussion due to code complexity) or find some other solution.

Passing either --unidiff-zero or -C1 with the failing patch fixes the
problem, but oddly (to me at least) -C2 does not.  The generated error
looks like

  $ git apply --check -v -C2 < patch
  Checking patch file...
  error: while searching for:
  baseline
  content

  error: patch failed: file:1
  error: file: patch does not apply

The corresponding call (builtin-apply.c:2093) is

			error("while searching for:\n%.*s",
			      (int)(old - oldlines), oldlines);

so it does not seem to insert the extra newline.  Is it actually
looking for a blank line in the context?  If so, wouldn't that be a
git-apply bug?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help