[PATCH] builtin-apply.c: Skip filenames without enough components

[PATCH] builtin-apply.c: Skip filenames without enough components

[Andreas Gruenbacher]

find_name() wrongly returned the whole filename for filenames without
enough leading pathname components (e.g., when applying a patch to a
top-level file with -p2).

Include the -p value used in the error message when no filenames can be
found.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
---
 builtin-apply.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index 541493e..b99db0b 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -404,6 +404,9 @@ static char *squash_slash(char *name)
 {
 	int i = 0, j = 0;
 
+	if (!name)
+		return NULL;
+
 	while (name[i]) {
 		if ((name[j++] = name[i++]) == '/')
 			while (name[i] == '/')
@@ -416,7 +419,10 @@ static char *squash_slash(char *name)
 static char *find_name(const char *line, char *def, int p_value, int terminate)
 {
 	int len;
-	const char *start = line;
+	const char *start = NULL;
+
+	if (p_value == 0)
+		start = line;
 
 	if (*line == '"') {
 		struct strbuf name = STRBUF_INIT;
@@ -1199,7 +1205,8 @@ static int find_header(char *line, unsigned long size, int *hdrsize, 
struct patc
 				continue;
 			if (!patch->old_name && !patch->new_name) {
 				if (!patch->def_name)
-					die("git diff header lacks filename information (line %d)", linenr);
+					die("git diff header lacks filename information when removing "
+					    "%d leading pathname components (line %d)" , p_value, linenr);
 				patch->old_name = patch->new_name = patch->def_name;
 			}
 			patch->is_toplevel_relative = 1;
-- 
1.6.6.197.g9c4a28

Re: [PATCH] builtin-apply.c: Skip filenames without enough components

[Junio C Hamano]

Tests?

Re: [PATCH] builtin-apply.c: Skip filenames without enough components

[Andreas Gruenbacher]

On Sunday 17 January 2010 03:22:10 am Junio C Hamano wrote:
> Tests?

Sure if you think it's worth a regression test ... "git apply -p2" of the 
following patch fails with "fatal: git diff header lacks filename information 
when removing 2 leading pathname components (line 6)" with the fix, and 
creates b/f without:

	diff --git a/f b/f
	new file mode 100644
	index 0000000..6a69f92
	--- /dev/null
	+++ b/f
	@@ -0,0 +1 @@
	+f

(Some earlier versions of git failed with "fatal: git apply: bad git-diff - 
inconsistent new filename on line 5" in this case.)

Andreas

Re: [PATCH] builtin-apply.c: Skip filenames without enough components

[Nanako Shiraishi]

Quoting Andreas Gruenbacher <agruen@suse.de>

> On Sunday 17 January 2010 03:22:10 am Junio C Hamano wrote:
>> Tests?
>
> Sure if you think it's worth a regression test ...

Of course you must have a test when you are fixing things. Tests aren't to prove that your patch is correct. They are to prevent other people from breaking your change long after you leave the project.

> Sure if you think it's worth a regression test ... "git apply -p2" of the 
> following patch fails...

You do so by sending a patch to add your new test in t/; adding to an existing related test is preferred if the test is small.

Junio, in case you don't want to wait for Andreas, you can squash this test in.

diff --git a/t/t4120-apply-popt.sh b/t/t4120-apply-popt.sh
index 83d4ba6..b463b4f 100755
--- a/t/t4120-apply-popt.sh
+++ b/t/t4120-apply-popt.sh
@@ -22,4 +22,9 @@ test_expect_success 'apply git diff with -p2' '
 	git apply -p2 patch.file
 '
 
+test_expect_success 'apply with too large -p' '
+	test_must_fail git apply --stat -p3 patch.file 2>err &&
+	grep "removing 3 leading" err
+'
+
 test_done

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

Re: [PATCH] builtin-apply.c: Skip filenames without enough components

[Andreas Gruenbacher]

On Monday 18 January 2010 11:22:35 am Nanako Shiraishi wrote:
> Junio, in case you don't want to wait for Andreas, you can squash this test
>  in.

Thanks, looks good!

Andreas