Re: git-apply fails on creating a new file, with both -p and --directory specified

"Steven J. Murdoch" [off-list ref] writes:

This appears to be because I was both using -p to strip some path
components, and --directory to add different ones in. Only creating
new files was affected.

A very nicely done report.

In addition to your test case, I suspect that a patch that only changes
mode would have acted funny with -p<n> option.

-- >8 --
[PATCH] builtin-apply.c: pay attention to -p<n> when determining the name

The patch structure has def_name component that is used to validate the
sanity of a "diff --git" patch by checking pathnames that appear on the
patch header lines for consistency.  The git_header_name() function is
used to compute this out of "diff --git a/... b/..." line, but the code
always stripped one level of prefix (i.e. "a/" and "b/"), without paying
attention to -p<n> option.  Code in find_name() function that parses other
lines in the patch header (e.g. "--- a/..." and "+++ b/..." lines) however
did strip the correct number of leading paths prefixes, and the sanity
check between these computed values failed.

Teach git_header_name() to honor -p<n> option like find_name() function
does.

Signed-off-by: Junio C Hamano <redacted>
---
 builtin-apply.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index f667368..36e2f9d 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c

@@ -823,12 +823,13 @@ static int gitdiff_unrecognized(const char *line, struct patch *patch)
 
 static const char *stop_at_slash(const char *line, int llen)
 {
+	int nslash = p_value;
 	int i;
 
 	for (i = 0; i < llen; i++) {
 		int ch = line[i];
-		if (ch == '/')
-			return line + i;
+		if (ch == '/' && --nslash <= 0)
+			return &line[i];
 	}
 	return NULL;
 }

Junio C Hamano <gitster <at> pobox.com> writes:

"Steven J. Murdoch" <git+Steven.Murdoch <at> cl.cam.ac.uk> writes:

quoted

This appears to be because I was both using -p to strip some path
components, and --directory to add different ones in. Only creating
new files was affected.

A very nicely done report.

In addition to your test case, I suspect that a patch that only changes
mode would have acted funny with -p<n> option.

It looks like this may have introduced a bug when staging a file
removal.  Here's an example git session showing the issue:

$ git init test
Initialized empty Git repository in /local_disk/tmp/test/.git/
$ cd test
$ echo "foo" > foo
$ git add foo
$ git commit -m 'Add foo'
[master (root-commit) 3643b5d] Add foo
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 foo
$ mv foo bar
$ git add -p

diff --git a/foo b/foo
index 257cc56..0000000
--- a/foo
+++ /dev/null

@@ -1 +0,0 @@
-foo

Stage this hunk [y,n,q,a,d,/,e,?]? y

$ git status
# On branch master
# Changes to be committed:
#   (use "git reset HEAD ..." to unstage)
#
#       new file:   dev/null
#       deleted:    foo
#
# Changed but not updated:
#   (use "git add/rm ..." to update what will be committed)
#   (use "git checkout -- ..." to discard changes in working directory)
#
#       deleted:    dev/null
#
# Untracked files:
#   (use "git add ..." to include in what will be committed)
#
#       bar

Replacing the 'git add -p' with

  git diff | sed '/^deleted file/d' | git apply --cached

also exhibits the problem.