{"thread":{"id":"32204","subject":"Interesting git-format-patch bug","startedAt":"2012-11-26T21:33:01Z","lastAt":"2012-11-27T20:31:50Z","messageCount":5,"participants":["Olsen, Alan R","Junio C Hamano","Perry Hutchison"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"203935","messageId":"4B2793BF110AAB47AB0EE7B9089703854CA7BA61@fmsmsx110.amr.corp.intel.com","threadId":"32204","inReplyTo":null,"subject":"Interesting git-format-patch bug","fromName":"Olsen, Alan R","fromEmail":"alan.r.olsen@intel.com","sentAt":"2012-11-26T21:33:01Z","receivedAt":"2012-11-26T21:33:01Z","isPatch":false,"sender":{"key":"alan.r.olsen@intel.com","avatar":null},"body":"I found an interesting bug in git-format-patch.\n\nSay you have a branch A. You create branch B and add a patch to it. You then merge that patch into branch A. After the merge, some other process (we will call it 'gerrit') uses annotate and changes the comment on the patch that exists on branch B.\n\nNow someone runs git-format-patch for the last n patches on branch A. You should just get the original patch that was merged over to branch A. What you get is the patch that was merged to branch A *and* the patch with the modified commit comment on branch B. (Double the patches, double the clean-up...)\n\nThis is should be one of those rare corner case \"don't do that\" occurrences. Unfortunately it does happen once in a while on our branches and it screws up some of the automated processes we rely on.\n\nIs there a way around that (other than \"don't\") or can this be fixed?\n"},{"id":"203943","messageId":"7vobikotwd.fsf@alter.siamese.dyndns.org","threadId":"32204","inReplyTo":"4B2793BF110AAB47AB0EE7B9089703854CA7BA61@fmsmsx110.amr.corp.intel.com","subject":"Re: Interesting git-format-patch bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T22:56:34Z","receivedAt":"2012-11-26T22:56:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Olsen, Alan R\" writes:\n\n> I found an interesting bug in git-format-patch.\n>\n> Say you have a branch A. You create branch B and add a patch to\n> it. You then merge that patch into branch A. After the merge, some\n> other process (we will call it 'gerrit') uses annotate and changes\n> the comment on the patch that exists on branch B.\n>\n> Now someone runs git-format-patch for the last n patches on branch\n> A. You should just get the original patch that was merged over to\n> branch A. What you get is the patch that was merged to branch A\n> *and* the patch with the modified commit comment on branch\n> B. (Double the patches, double the clean-up...)\n\nAs you literally have patches that do essentially the same or\nsimilar things on two branches that was merged, you cannot expect to\nexport each individual commit into a patch and not have conflicts\namong them. So I do not think there is no answer than \"don't do\nthat\".\n\nI think you could make your \"some other process\" that rewrites\ncommits to cull the duplicates out of the format-patch output,\nthough. Each output file identifies what commit object the patch\ncame from, and your \"some other process\" that rewrote the commits\nought to know which commit updated which other commit did, which is\nthe piece of information needed to remove duplicates that format-patch\ndoes not have.\n"},{"id":"203966","messageId":"50b4304c.EwQy4JquPwsUyMfZ%perryh@pluto.rain.com","threadId":"32204","inReplyTo":"7vobikotwd.fsf@alter.siamese.dyndns.org","subject":"Re: Interesting git-format-patch bug","fromName":"Perry Hutchison","fromEmail":"perryh@pluto.rain.com","sentAt":"2012-11-27T04:15:24Z","receivedAt":"2012-11-27T04:15:24Z","isPatch":false,"sender":{"key":"perryh@pluto.rain.com","avatar":null},"body":"Junio C Hamano wrote:\n> \"Olsen, Alan R\" writes:\n> > I found an interesting bug in git-format-patch.\n> >\n> > Say you have a branch A. You create branch B and add a patch to\n> > it. You then merge that patch into branch A. After the merge,\n> > some other process (we will call it 'gerrit') uses annotate and\n> > changes the comment on the patch that exists on branch B.\n> >\n> > Now someone runs git-format-patch for the last n patches on\n> > branch A. You should just get the original patch that was\n> > merged over to branch A. What you get is the patch that was\n> > merged to branch A *and* the patch with the modified commit\n> > comment on branch B. (Double the patches, double the\n> > clean-up...)\n>\n> As you literally have patches that do essentially the same or\n> similar things on two branches that was merged, you cannot\n> expect to export each individual commit into a patch and not\n> have conflicts among them. So I do not think there is no\n> answer than \"don't do that\".\n\nTo me, this seems to miss Alan's point: only one patch was merged\nto branch A, so git-format-patch applied to branch A should find\nonly one patch. It can be argued either way whether that one-patch\nreport should include the gerrit annotations, but surely the\napplication of gerrit on branch B, _after the merge to branch A\nhas already been performed_, should not cause an additional patch\nto magically appear on branch A.\n"},{"id":"203999","messageId":"7vy5hnlzta.fsf@alter.siamese.dyndns.org","threadId":"32204","inReplyTo":"50b4304c.EwQy4JquPwsUyMfZ%perryh@pluto.rain.com","subject":"Re: Interesting git-format-patch bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-27T17:29:21Z","receivedAt":"2012-11-27T17:29:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"perryh@pluto.rain.com (Perry Hutchison) writes:\n\n> Junio C Hamano wrote:\n>> \"Olsen, Alan R\" writes:\n>> > I found an interesting bug in git-format-patch.\n>> >\n>> > Say you have a branch A. You create branch B and add a patch to\n>> > it. You then merge that patch into branch A. After the merge,\n>> > some other process (we will call it 'gerrit') uses annotate and\n>> > changes the comment on the patch that exists on branch B.\n>> >\n>> > Now someone runs git-format-patch for the last n patches on\n>> > branch A. You should just get the original patch that was\n>> > merged over to branch A. What you get is the patch that was\n>> > merged to branch A *and* the patch with the modified commit\n>> > comment on branch B. (Double the patches, double the\n>> > clean-up...)\n>>\n>> As you literally have patches that do essentially the same or\n>> similar things on two branches that was merged, you cannot\n>> expect to export each individual commit into a patch and not\n>> have conflicts among them. So I do not think there is no\n>> answer than \"don't do that\".\n>\n> To me, this seems to miss Alan's point: only one patch was merged\n> to branch A,...\n\nAre you sure about this part?\n\nI thought Alan's description was that he originally had this\n\n x-----A\n \\ \\\n B-----M (a)\n\nand then \"some other process\" made it like so:\n\n x-----A\n |\\ \\\n | B-----M\n \\ \\\n B'------M' (a)\n\nand then you ask to linealize the last n patches starting from the\nrewritten M'.\n\nIf that \"some other process\" instead created a history like this:\n\n x-----A---\\\n |\\ \\ \\ \n | B-----M \\\n \\ \\\n B'----------M' (a)\n\nthen the redone-merge M' will not see the old B that was fixed later\nto B' in the history, but then format-patch would not show B so we\nwouldn't be having this discussion thread.\n\nIt is possible that \"some other process\" may (ab)use the parent\nfield to record the evolution of B, to create a topology like this:\n\n x-----A---\\\n |\\ \\ \\\n | B-----M \\\n \\ \\ \\\n \\-B'--------M' (a)\n\nin which case M' has parent B' but B' has a (phoney) parent B.\n\nSo again, it all depends on what \"some other process\" does to the\nhistory when it rewrites it, and if somebody wants to fiter cruft in\nthe resulting history when flattening it, the knowledge of what\n\"some other process\" does need to help that process.\n\nWhich is what I already said, I guess ;-)\n\n> so git-format-patch applied to branch A should find\n> only one patch. It can be argued either way whether that one-patch\n> report should include the gerrit annotations, but surely the\n> application of gerrit on branch B, _after the merge to branch A\n> has already been performed_, should not cause an additional patch\n> to magically appear on branch A.\n"},{"id":"204021","messageId":"4B2793BF110AAB47AB0EE7B9089703854CA7C128@fmsmsx110.amr.corp.intel.com","threadId":"32204","inReplyTo":"50b4304c.EwQy4JquPwsUyMfZ%perryh@pluto.rain.com","subject":"RE: Interesting git-format-patch bug","fromName":"Olsen, Alan R","fromEmail":"alan.r.olsen@intel.com","sentAt":"2012-11-27T20:31:50Z","receivedAt":"2012-11-27T20:31:50Z","isPatch":false,"sender":{"key":"alan.r.olsen@intel.com","avatar":null},"body":"[Sorry for the top posting. Outlook is crap.]\n\nYou are correct. I should only get one copy of the patch on branch A. Branch B was modified after the merge and git-format-patch includes the original patch from the merge and a duplicate copy with the changed comments. Note that this patch only has different comments. The body of the patch is exactly the same.\n\nHow gerrit mangles things is out of my control. I would prefer that they cherry-pick instead of merges. I have to live with the bad choices of both gerrit and developers in this case.\n\nI guess I will have to diagram out a better example of what is happening here.\n\n-----Original Message-----\nFrom: Perry Hutchison [mailto:perryh@pluto.rain.com] \nSent: Monday, November 26, 2012 8:15 PM\nTo: gitster@pobox.com\nCc: git@vger.kernel.org; Olsen, Alan R\nSubject: Re: Interesting git-format-patch bug\n\nJunio C Hamano wrote:\n> \"Olsen, Alan R\" writes:\n> > I found an interesting bug in git-format-patch.\n> >\n> > Say you have a branch A. You create branch B and add a patch to it. \n> > You then merge that patch into branch A. After the merge, some other \n> > process (we will call it 'gerrit') uses annotate and changes the \n> > comment on the patch that exists on branch B.\n> >\n> > Now someone runs git-format-patch for the last n patches on branch \n> > A. You should just get the original patch that was merged over to \n> > branch A. What you get is the patch that was merged to branch A \n> > *and* the patch with the modified commit comment on branch B. \n> > (Double the patches, double the\n> > clean-up...)\n>\n> As you literally have patches that do essentially the same or similar \n> things on two branches that was merged, you cannot expect to export \n> each individual commit into a patch and not have conflicts among them. \n> So I do not think there is no answer than \"don't do that\".\n\nTo me, this seems to miss Alan's point: only one patch was merged to branch A, so git-format-patch applied to branch A should find only one patch. It can be argued either way whether that one-patch report should include the gerrit annotations, but surely the application of gerrit on branch B, _after the merge to branch A has already been performed_, should not cause an additional patch to magically appear on branch A.\n\n \n"}]}