{"thread":{"id":"14387","subject":"[PATCH] apply: fix copy/rename breakage","startedAt":"2008-07-10T03:10:58Z","lastAt":"2008-07-10T15:43:48Z","messageCount":6,"participants":["Junio C Hamano","Stephan Beyer","Don Zickus","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"82772","messageId":"7vy74aqvr1.fsf@gitster.siamese.dyndns.org","threadId":"14387","inReplyTo":null,"subject":"[PATCH] apply: fix copy/rename breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-10T03:10:58Z","receivedAt":"2008-07-10T03:10:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Recently, 7ebd52a (Merge branch 'dz/apply-again', 2008-07-01) taught\n\"git-apply\" to grok a (non-git) patch that is a concatenation of separate\npatches that touch the same file number of files, by recording the\npostimage of patch application of previous round and using it as the\npreimage for later rounds.\n\nHowever, this \"incremental\" mode of patch application contradicts with the\nway git rename/copy patches are fundamentally designed. When a git patch\ntalks about a file A getting modified, and a new file B created out of B,\nlike this:\n\n\tdiff --git a/A b/A\n\t--- a/A\n\t+++ b/A\n\t... change text here ...\n\tdiff --git a/A b/B\n\tcopy from A\n\tcopy to B\n\t--- a/A\n\t+++ b/B\n\t... change text here ...\n\nthe second change to produce B does not depend on what is done to A with\nthe first change (this is explicitly done so for reviewability of\nindividual patches).\n\nWith this patch, we disable the postimage record 'fn_table' when applying\na patch to produce new files out of existing file by copying to fix this\nissue.\n\nSigned-off-by: Junio C Hamano \n---\n\n * Applies to 'master'. I am CC'ing Linus not because he is in any way\n responsible for this breakage, but because this breakage can affect\n heavy users of \"git apply\".\n\n builtin-apply.c | 10 +++++++---\n 1 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex b3fc290..d13313f 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -2296,7 +2296,8 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \n \tstrbuf_init(&buf, 0);\n \n-\tif ((tpatch = in_fn_table(patch->old_name)) != NULL) {\n+\tif (!(patch->is_copy || patch->is_rename) &&\n+\t ((tpatch = in_fn_table(patch->old_name)) != NULL)) {\n \t\tif (tpatch == (struct patch *) -1) {\n \t\t\treturn error(\"patch %s has been renamed/deleted\",\n \t\t\t\tpatch->old_name);\n@@ -2375,7 +2376,7 @@ static int verify_index_match(struct cache_entry *ce, struct stat *st)\n static int check_preimage(struct patch *patch, struct cache_entry **ce, struct stat *st)\n {\n \tconst char *old_name = patch->old_name;\n-\tstruct patch *tpatch;\n+\tstruct patch *tpatch = NULL;\n \tint stat_ret = 0;\n \tunsigned st_mode = 0;\n \n@@ -2389,7 +2390,9 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s\n \t\treturn 0;\n \n \tassert(patch->is_new <= 0);\n-\tif ((tpatch = in_fn_table(old_name)) != NULL) {\n+\n+\tif (!(patch->is_copy || patch->is_rename) &&\n+\t (tpatch = in_fn_table(old_name)) != NULL) {\n \t\tif (tpatch == (struct patch *) -1) {\n \t\t\treturn error(\"%s: has been deleted/renamed\", old_name);\n \t\t}\n@@ -2399,6 +2402,7 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s\n \t\tif (stat_ret && errno != ENOENT)\n \t\t\treturn error(\"%s: %s\", old_name, strerror(errno));\n \t}\n+\n \tif (check_index && !tpatch) {\n \t\tint pos = cache_name_pos(old_name, strlen(old_name));\n \t\tif (pos < 0) {\n-- \n1.5.6.2.291.g7eef3\n"},{"id":"82773","messageId":"7vtzeyqvnj.fsf@gitster.siamese.dyndns.org","threadId":"14387","inReplyTo":"7vy74aqvr1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] apply: fix copy/rename breakage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-10T03:13:04Z","receivedAt":"2008-07-10T03:13:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Recently, 7ebd52a (Merge branch 'dz/apply-again', 2008-07-01) taught\n> \"git-apply\" to grok a (non-git) patch that is a concatenation of separate\n> patches that touch the same file number of files, by recording the\n\nEh, s/files/times/;\n\n> postimage of patch application of previous round and using it as the\n> preimage for later rounds.\n>\n> However, this \"incremental\" mode of patch application contradicts with the\n> way git rename/copy patches are fundamentally designed....\n> a patch to produce new files out of existing file by copying to fix this\n> issue.\n>\n> Signed-off-by: Junio C Hamano \n> ---\n>\n> * Applies to 'master'. I am CC'ing Linus not because he is in any way\n> responsible for this breakage, but because this breakage can affect\n> heavy users of \"git apply\".\n"},{"id":"82776","messageId":"20080710042144.GF18030@leksak.fem-net","threadId":"14387","inReplyTo":"7vy74aqvr1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] apply: fix copy/rename breakage","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-10T04:21:44Z","receivedAt":"2008-07-10T04:21:44Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nOn Wed, Jul 09, 2008 at 08:10:58PM -0700, Junio C Hamano wrote:\n> \n> \tdiff --git a/A b/A\n> \t--- a/A\n> \t+++ b/A\n> \t... change text here ...\n> \tdiff --git a/A b/B\n> \tcopy from A\n> \tcopy to B\n> \t--- a/A\n> \t+++ b/B\n> \t... change text here ...\n\nBig thanks! Now my patch applies cleanly again and many others, too. So:\n\nTested-by: Stephan Beyer \n\n;)\n\nRegards,\n Stephan\n\n-- \nStephan Beyer , PGP 0x6EDDD207FCC5040F\n"},{"id":"82811","messageId":"20080710140154.GN26957@redhat.com","threadId":"14387","inReplyTo":"7vy74aqvr1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] apply: fix copy/rename breakage","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2008-07-10T14:01:54Z","receivedAt":"2008-07-10T14:01:54Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"On Wed, Jul 09, 2008 at 08:10:58PM -0700, Junio C Hamano wrote:\n> Recently, 7ebd52a (Merge branch 'dz/apply-again', 2008-07-01) taught\n> \"git-apply\" to grok a (non-git) patch that is a concatenation of separate\n> patches that touch the same file number of files, by recording the\n> postimage of patch application of previous round and using it as the\n> preimage for later rounds.\n> \n> However, this \"incremental\" mode of patch application contradicts with the\n> way git rename/copy patches are fundamentally designed. When a git patch\n> talks about a file A getting modified, and a new file B created out of B,\n> like this:\n> \n> \tdiff --git a/A b/A\n> \t--- a/A\n> \t+++ b/A\n> \t... change text here ...\n> \tdiff --git a/A b/B\n> \tcopy from A\n> \tcopy to B\n> \t--- a/A\n> \t+++ b/B\n> \t... change text here ...\n> \n> the second change to produce B does not depend on what is done to A with\n> the first change (this is explicitly done so for reviewability of\n> individual patches).\n> \n> With this patch, we disable the postimage record 'fn_table' when applying\n> a patch to produce new files out of existing file by copying to fix this\n> issue.\n\nOdd. I guess the way I read this workflow is\n\napply change X to A, copy A' to B, apply change Y to B => B' now has changes X+Y\n\nBut instead you are saying B' only has change Y because A is copied to B\nnot A'.\n\nRegardless, it doesn't affect my workflow.\n\nACK.\n\nCheers,\nDon\n"},{"id":"82818","messageId":"48762919.6070902@viscovery.net","threadId":"14387","inReplyTo":"20080710140154.GN26957@redhat.com","subject":"Re: [PATCH] apply: fix copy/rename breakage","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-07-10T15:22:01Z","receivedAt":"2008-07-10T15:22:01Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Don Zickus schrieb:\n> On Wed, Jul 09, 2008 at 08:10:58PM -0700, Junio C Hamano wrote:\n>> However, this \"incremental\" mode of patch application contradicts with the\n>> way git rename/copy patches are fundamentally designed. When a git patch\n>> talks about a file A getting modified, and a new file B created out of B,\n>> like this:\n>>\n>> \tdiff --git a/A b/A\n>> \t--- a/A\n>> \t+++ b/A\n>> \t... change text X here ...\n>> \tdiff --git a/A b/B\n>> \tcopy from A\n>> \tcopy to B\n>> \t--- a/A\n>> \t+++ b/B\n>> \t... change text Y here ...\n>>\n>> the second change to produce B does not depend on what is done to A with\n>> the first change (this is explicitly done so for reviewability of\n>> individual patches).\n>>\n>> With this patch, we disable the postimage record 'fn_table' when applying\n>> a patch to produce new files out of existing file by copying to fix this\n>> issue.\n> \n> Odd. I guess the way I read this workflow is\n> \n> apply change X to A, copy A' to B, apply change Y to B => B' now has changes X+Y\n> \n> But instead you are saying B' only has change Y because A is copied to B\n> not A'.\n> \n> Regardless, it doesn't affect my workflow.\n\nOh, it does. It's a normal git diff where a copy was detected!\n\nDon't let you distract by the word \"incremental\" and by the names A and B.\nIn the example above, the change X comes first because 'A' is sorted\nbefore 'B'. If the roles of A and B were swapped, then you have this patch:\n\n \tdiff --git a/A b/A\n \tcopy from B\n \tcopy to A\n \t--- a/A\n \t+++ b/A\n \t... change text Y here ...\n \tdiff --git a/A b/B\n \t--- a/A\n \t+++ b/B\n \t... change text X here ...\n\nSee?\n\n-- Hannes\n"},{"id":"82826","messageId":"20080710154348.GC22201@redhat.com","threadId":"14387","inReplyTo":"48762919.6070902@viscovery.net","subject":"Re: [PATCH] apply: fix copy/rename breakage","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2008-07-10T15:43:48Z","receivedAt":"2008-07-10T15:43:48Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"On Thu, Jul 10, 2008 at 05:22:01PM +0200, Johannes Sixt wrote:\n> >> With this patch, we disable the postimage record 'fn_table' when applying\n> >> a patch to produce new files out of existing file by copying to fix this\n> >> issue.\n> > \n> > Odd. I guess the way I read this workflow is\n> > \n> > apply change X to A, copy A' to B, apply change Y to B => B' now has changes X+Y\n> > \n> > But instead you are saying B' only has change Y because A is copied to B\n> > not A'.\n> > \n> > Regardless, it doesn't affect my workflow.\n> \n> Oh, it does. It's a normal git diff where a copy was detected!\n> \n> Don't let you distract by the word \"incremental\" and by the names A and B.\n> In the example above, the change X comes first because 'A' is sorted\n> before 'B'. If the roles of A and B were swapped, then you have this patch:\n> \n> \tdiff --git a/A b/A\n> \tcopy from B\n> \tcopy to A\n> \t--- a/A\n> \t+++ b/A\n> \t... change text Y here ...\n> \tdiff --git a/A b/B\n> \t--- a/A\n> \t+++ b/B\n> \t... change text X here ...\n> \n> See?\n\nYes, thank you!\n\nCheers,\nDon\n"}]}