{"thread":{"id":"22251","subject":"[PATCH] builtin-apply.c: Skip filenames without enough components","startedAt":"2010-01-17T02:05:10Z","lastAt":"2010-01-18T19:57:40Z","messageCount":5,"participants":["Andreas Gruenbacher","Junio C Hamano","Nanako Shiraishi"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"131916","messageId":"201001170305.10793.agruen@suse.de","threadId":"22251","inReplyTo":null,"subject":"[PATCH] builtin-apply.c: Skip filenames without enough components","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-17T02:05:10Z","receivedAt":"2010-01-17T02:05:10Z","isPatch":true,"sender":{"key":"agruen@suse.de","avatar":null},"body":"find_name() wrongly returned the whole filename for filenames without\nenough leading pathname components (e.g., when applying a patch to a\ntop-level file with -p2).\n\nInclude the -p value used in the error message when no filenames can be\nfound.\n\nSigned-off-by: Andreas Gruenbacher \n---\n builtin-apply.c | 11 +++++++++--\n 1 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 541493e..b99db0b 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -404,6 +404,9 @@ static char *squash_slash(char *name)\n {\n \tint i = 0, j = 0;\n \n+\tif (!name)\n+\t\treturn NULL;\n+\n \twhile (name[i]) {\n \t\tif ((name[j++] = name[i++]) == '/')\n \t\t\twhile (name[i] == '/')\n@@ -416,7 +419,10 @@ static char *squash_slash(char *name)\n static char *find_name(const char *line, char *def, int p_value, int terminate)\n {\n \tint len;\n-\tconst char *start = line;\n+\tconst char *start = NULL;\n+\n+\tif (p_value == 0)\n+\t\tstart = line;\n \n \tif (*line == '\"') {\n \t\tstruct strbuf name = STRBUF_INIT;\n@@ -1199,7 +1205,8 @@ static int find_header(char *line, unsigned long size, int *hdrsize, \nstruct patc\n \t\t\t\tcontinue;\n \t\t\tif (!patch->old_name && !patch->new_name) {\n \t\t\t\tif (!patch->def_name)\n-\t\t\t\t\tdie(\"git diff header lacks filename information (line %d)\", linenr);\n+\t\t\t\t\tdie(\"git diff header lacks filename information when removing \"\n+\t\t\t\t\t \"%d leading pathname components (line %d)\" , p_value, linenr);\n \t\t\t\tpatch->old_name = patch->new_name = patch->def_name;\n \t\t\t}\n \t\t\tpatch->is_toplevel_relative = 1;\n-- \n1.6.6.197.g9c4a28\n"},{"id":"131917","messageId":"7vr5ppa2st.fsf@alter.siamese.dyndns.org","threadId":"22251","inReplyTo":"201001170305.10793.agruen@suse.de","subject":"Re: [PATCH] builtin-apply.c: Skip filenames without enough components","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-17T02:22:10Z","receivedAt":"2010-01-17T02:22:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tests?\n"},{"id":"131918","messageId":"201001170344.34525.agruen@suse.de","threadId":"22251","inReplyTo":"7vr5ppa2st.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] builtin-apply.c: Skip filenames without enough components","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-17T02:44:34Z","receivedAt":"2010-01-17T02:44:34Z","isPatch":true,"sender":{"key":"agruen@suse.de","avatar":null},"body":"On Sunday 17 January 2010 03:22:10 am Junio C Hamano wrote:\n> Tests?\n\nSure if you think it's worth a regression test ... \"git apply -p2\" of the \nfollowing patch fails with \"fatal: git diff header lacks filename information \nwhen removing 2 leading pathname components (line 6)\" with the fix, and \ncreates b/f without:\n\n\tdiff --git a/f b/f\n\tnew file mode 100644\n\tindex 0000000..6a69f92\n\t--- /dev/null\n\t+++ b/f\n\t@@ -0,0 +1 @@\n\t+f\n\n(Some earlier versions of git failed with \"fatal: git apply: bad git-diff - \ninconsistent new filename on line 5\" in this case.)\n\nAndreas\n"},{"id":"132027","messageId":"20100118192235.6117@nanako3.lavabit.com","threadId":"22251","inReplyTo":"201001170344.34525.agruen@suse.de","subject":"Re: [PATCH] builtin-apply.c: Skip filenames without enough components","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2010-01-18T10:22:35Z","receivedAt":"2010-01-18T10:22:35Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Andreas Gruenbacher \n\n> On Sunday 17 January 2010 03:22:10 am Junio C Hamano wrote:\n>> Tests?\n>\n> Sure if you think it's worth a regression test ...\n\nOf 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.\n\n> Sure if you think it's worth a regression test ... \"git apply -p2\" of the \n> following patch fails...\n\nYou 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.\n\nJunio, in case you don't want to wait for Andreas, you can squash this test in.\n\ndiff --git a/t/t4120-apply-popt.sh b/t/t4120-apply-popt.sh\nindex 83d4ba6..b463b4f 100755\n--- a/t/t4120-apply-popt.sh\n+++ b/t/t4120-apply-popt.sh\n@@ -22,4 +22,9 @@ test_expect_success 'apply git diff with -p2' '\n \tgit apply -p2 patch.file\n '\n \n+test_expect_success 'apply with too large -p' '\n+\ttest_must_fail git apply --stat -p3 patch.file 2>err &&\n+\tgrep \"removing 3 leading\" err\n+'\n+\n test_done\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"132044","messageId":"201001182057.41032.agruen@suse.de","threadId":"22251","inReplyTo":"20100118192235.6117@nanako3.lavabit.com","subject":"Re: [PATCH] builtin-apply.c: Skip filenames without enough components","fromName":"Andreas Gruenbacher","fromEmail":"agruen@suse.de","sentAt":"2010-01-18T19:57:40Z","receivedAt":"2010-01-18T19:57:40Z","isPatch":true,"sender":{"key":"agruen@suse.de","avatar":null},"body":"On Monday 18 January 2010 11:22:35 am Nanako Shiraishi wrote:\n> Junio, in case you don't want to wait for Andreas, you can squash this test\n> in.\n\nThanks, looks good!\n\nAndreas\n"}]}