# [PATCH] builtin-apply.c: Skip filenames without enough components 5 messages from 2010-01-17 to 2010-01-18. Participants: Andreas Gruenbacher, Junio C Hamano, Nanako Shiraishi. Thread: https://gitlist.dev/t/22251 ## Andreas Gruenbacher, 2010-01-17 02:05 Subject: [PATCH] builtin-apply.c: Skip filenames without enough components Message-ID: <201001170305.10793.agruen@suse.de> URL: https://gitlist.dev/e/201001170305.10793.agruen%40suse.de ``` 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 --- 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 ``` ## Junio C Hamano, 2010-01-17 02:22 Subject: Re: [PATCH] builtin-apply.c: Skip filenames without enough components Message-ID: <7vr5ppa2st.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr5ppa2st.fsf%40alter.siamese.dyndns.org In-Reply-To: <201001170305.10793.agruen@suse.de> ``` Tests? ``` ## Andreas Gruenbacher, 2010-01-17 02:44 Subject: Re: [PATCH] builtin-apply.c: Skip filenames without enough components Message-ID: <201001170344.34525.agruen@suse.de> URL: https://gitlist.dev/e/201001170344.34525.agruen%40suse.de In-Reply-To: <7vr5ppa2st.fsf@alter.siamese.dyndns.org> ``` 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 ``` ## Nanako Shiraishi, 2010-01-18 10:22 Subject: Re: [PATCH] builtin-apply.c: Skip filenames without enough components Message-ID: <20100118192235.6117@nanako3.lavabit.com> URL: https://gitlist.dev/e/20100118192235.6117%40nanako3.lavabit.com In-Reply-To: <201001170344.34525.agruen@suse.de> ``` Quoting 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 ... 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/ ``` ## Andreas Gruenbacher, 2010-01-18 19:57 Subject: Re: [PATCH] builtin-apply.c: Skip filenames without enough components Message-ID: <201001182057.41032.agruen@suse.de> URL: https://gitlist.dev/e/201001182057.41032.agruen%40suse.de In-Reply-To: <20100118192235.6117@nanako3.lavabit.com> ``` 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 ```