{"thread":{"id":"8584","subject":"unexpected git-cherry-pick conflict","startedAt":"2007-06-13T09:16:24Z","lastAt":"2007-12-22T08:20:38Z","messageCount":23,"participants":["Gerrit Pape","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"44907","messageId":"20070613091624.26463.qmail@353090644b4917.315fe32.mid.smarden.org","threadId":"8584","inReplyTo":"6b8a91420706070252y3fd581a3w427d91e5b982d29d@mail.gmail.com","subject":"unexpected git-cherry-pick conflict","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-06-13T09:16:24Z","receivedAt":"2007-06-13T09:16:24Z","isPatch":false,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Thu, Jun 07, 2007 at 11:52:19AM +0200, Remi Vanicat wrote:\n> how to reproduce :\n> create a repos with a link in it, then in a branch, remove the link,\n> and add a directory in place of the link (same name).\n> \n> Then try to cherry pick a commit from a branch where there is the\n> directory into a branch where there is the link : it failed even if\n> the modification ave nothing to do with said link/directory.\n\nHi, please see http://bugs.debian.org/417885\n\nThis is how I can reproduce the conflict, and I too didn't expect that.\nThe link/dir that conflicts is not changed in the commit that's\ncherry-pick'ed:\n\n $ mkdir repo && cd repo\n $ git init\n Initialized empty Git repository in .git/\n $ echo foo >file\n $ ln -s dangling link\n $ git add .\n $ git commit -mfoo\n Created initial commit c6a9189: foo\n  2 files changed, 2 insertions(+), 0 deletions(-)\n  create mode 100644 file\n  create mode 120000 link\n $ git checkout -b branch\n Switched to a new branch \"branch\"\n $ git rm link\n rm 'link'\n $ git commit -mremovelink\n Created commit 2c60f15: removelink\n  1 files changed, 0 insertions(+), 1 deletions(-)\n  delete mode 120000 link\n $ mkdir link\n $ echo bar >link/file\n $ git add link\n $ git commit -m adddir\n Created commit d3b30b5: adddir\n  1 files changed, 1 insertions(+), 0 deletions(-)\n  create mode 100644 link/file\n $ echo bar >>file\n $ git commit -mfile file\n Created commit 8ddc4d5: file\n  1 files changed, 1 insertions(+), 0 deletions(-)\n $ git checkout master\n Switched to branch \"master\"\n $ git cherry-pick 8ddc4d5\n CONFLICT (file/directory): There is a directory with name link in\n 8ddc4d5... file. Added link as link~HEAD\n Automatic cherry-pick failed.  After resolving the conflicts,\n mark the corrected paths with 'git-add <paths>'\n and commit the result.\n When commiting, use the option '-c 8ddc4d5' to retain authorship and\n message.\n $ \n\nThanks, Gerrit.\n"},{"id":"44928","messageId":"Pine.LNX.4.64.0706131354250.4059@racer.site","threadId":"8584","inReplyTo":"20070613091624.26463.qmail@353090644b4917.315fe32.mid.smarden.org","subject":"Re: unexpected git-cherry-pick conflict","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-13T12:58:51Z","receivedAt":"2007-06-13T12:58:51Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 13 Jun 2007, Gerrit Pape wrote:\n\n>  $ mkdir repo && cd repo\n>  $ git init\n>  Initialized empty Git repository in .git/\n>  $ echo foo >file\n>  $ ln -s dangling link\n>  $ git add .\n>  $ git commit -mfoo\n>  Created initial commit c6a9189: foo\n>   2 files changed, 2 insertions(+), 0 deletions(-)\n>   create mode 100644 file\n>   create mode 120000 link\n\nSo, basically your master has a file and a symbolic link.\n\n>  $ git checkout -b branch\n>  Switched to a new branch \"branch\"\n>  $ git rm link\n>  rm 'link'\n>  $ git commit -mremovelink\n>  Created commit 2c60f15: removelink\n>   1 files changed, 0 insertions(+), 1 deletions(-)\n>   delete mode 120000 link\n\nHere, you remove the link from the branch.\n\n>  $ mkdir link\n>  $ echo bar >link/file\n>  $ git add link\n>  $ git commit -m adddir\n>  Created commit d3b30b5: adddir\n>   1 files changed, 1 insertions(+), 0 deletions(-)\n>   create mode 100644 link/file\n\nHere you added a directory of the same name as the symbolic link has in \nmaster.\n\n>  $ git checkout master\n>  Switched to branch \"master\"\n>  $ git cherry-pick 8ddc4d5\n>  CONFLICT (file/directory): There is a directory with name link in\n>  8ddc4d5... file. Added link as link~HEAD\n\nHere you _still_ have the file in master. So that conflict is really \nexpected, since a cherry-pick will only do a three-way merge.\n\nI guess you want to use git-rebase instead.\n\nCiao,\nDscho\n"},{"id":"44930","messageId":"20070613134336.13661.qmail@c61f4fed932273.315fe32.mid.smarden.org","threadId":"8584","inReplyTo":"Pine.LNX.4.64.0706131354250.4059@racer.site","subject":"Re: unexpected git-cherry-pick conflict","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-06-13T13:43:35Z","receivedAt":"2007-06-13T13:43:35Z","isPatch":false,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Wed, Jun 13, 2007 at 01:58:51PM +0100, Johannes Schindelin wrote:\n> On Wed, 13 Jun 2007, Gerrit Pape wrote:\n> >  $ git checkout master\n> >  Switched to branch \"master\"\n> >  $ git cherry-pick 8ddc4d5\n> >  CONFLICT (file/directory): There is a directory with name link in\n> >  8ddc4d5... file. Added link as link~HEAD\n> \n> Here you _still_ have the file in master. So that conflict is really \n> expected, since a cherry-pick will only do a three-way merge.\n\ngit-cherry-pick(1) states\n Given one existing commit, apply the change the patch introduces, and\n record a new commit that records it. This requires your working tree to\n be clean (no modifications from the HEAD commit).\n\nThe patch introduced by the commit that's cherry-pick'ed has nothing to\ndo with the link or new directory, it just changes 'file'\n\n $ git show 8ddc4d5\n commit 8ddc4d59444a362261e10a3b22324818f5dd2fa7\n Author: Gerrit Pape <pape@smarden.org>\n Date:   Wed Jun 13 09:10:30 2007 +0000\n \n     file\n \n diff --git a/file b/file\n index 257cc56..3bd1f0e 100644\n --- a/file\n +++ b/file\n @@ -1 +1,2 @@\n  foo\n +bar\n $ \n\nThe patch applies to master just fine.  Where's my thinking wrong?\n\nThanks, Gerrit.\n"},{"id":"44937","messageId":"Pine.LNX.4.64.0706131543140.4059@racer.site","threadId":"8584","inReplyTo":"20070613134336.13661.qmail@c61f4fed932273.315fe32.mid.smarden.org","subject":"Re: unexpected git-cherry-pick conflict","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-13T14:43:48Z","receivedAt":"2007-06-13T14:43:48Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 13 Jun 2007, Gerrit Pape wrote:\n\n> On Wed, Jun 13, 2007 at 01:58:51PM +0100, Johannes Schindelin wrote:\n> > On Wed, 13 Jun 2007, Gerrit Pape wrote:\n> > >  $ git checkout master\n> > >  Switched to branch \"master\"\n> > >  $ git cherry-pick 8ddc4d5\n> > >  CONFLICT (file/directory): There is a directory with name link in\n> > >  8ddc4d5... file. Added link as link~HEAD\n> > \n> > Here you _still_ have the file in master. So that conflict is really \n> > expected, since a cherry-pick will only do a three-way merge.\n> \n> git-cherry-pick(1) states\n>  Given one existing commit, apply the change the patch introduces, and\n>  record a new commit that records it. This requires your working tree to\n>  be clean (no modifications from the HEAD commit).\n> \n> The patch introduced by the commit that's cherry-pick'ed has nothing to\n> do with the link or new directory, it just changes 'file'\n> \n>  $ git show 8ddc4d5\n>  commit 8ddc4d59444a362261e10a3b22324818f5dd2fa7\n>  Author: Gerrit Pape <pape@smarden.org>\n>  Date:   Wed Jun 13 09:10:30 2007 +0000\n>  \n>      file\n>  \n>  diff --git a/file b/file\n>  index 257cc56..3bd1f0e 100644\n>  --- a/file\n>  +++ b/file\n>  @@ -1 +1,2 @@\n>   foo\n>  +bar\n>  $ \n> \n> The patch applies to master just fine.  Where's my thinking wrong?\n\nHmm. Indeed. Thanks for clearing that up. Will work on it later.\n\nCiao,\nDscho\n"},{"id":"45743","messageId":"20070625071819.8091.qmail@5e4088a43a10fd.315fe32.mid.smarden.org","threadId":"8584","inReplyTo":"Pine.LNX.4.64.0706131543140.4059@racer.site","subject":"Re: unexpected git-cherry-pick conflict","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-06-25T07:18:19Z","receivedAt":"2007-06-25T07:18:19Z","isPatch":false,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Wed, Jun 13, 2007 at 03:43:48PM +0100, Johannes Schindelin wrote:\n> On Wed, 13 Jun 2007, Gerrit Pape wrote:\n> > On Wed, Jun 13, 2007 at 01:58:51PM +0100, Johannes Schindelin wrote:\n> > > On Wed, 13 Jun 2007, Gerrit Pape wrote:\n> > > >  $ git checkout master\n> > > >  Switched to branch \"master\"\n> > > >  $ git cherry-pick 8ddc4d5\n> > > >  CONFLICT (file/directory): There is a directory with name link in\n> > > >  8ddc4d5... file. Added link as link~HEAD\n> > > \n> > > Here you _still_ have the file in master. So that conflict is really \n> > > expected, since a cherry-pick will only do a three-way merge.\n> > \n> > git-cherry-pick(1) states\n> >  Given one existing commit, apply the change the patch introduces, and\n> >  record a new commit that records it. This requires your working tree to\n> >  be clean (no modifications from the HEAD commit).\n> > \n> > The patch introduced by the commit that's cherry-pick'ed has nothing to\n> > do with the link or new directory, it just changes 'file'\n> > \n> >  $ git show 8ddc4d5\n> >  commit 8ddc4d59444a362261e10a3b22324818f5dd2fa7\n> >  Author: Gerrit Pape <pape@smarden.org>\n> >  Date:   Wed Jun 13 09:10:30 2007 +0000\n> >  \n> >      file\n> >  \n> >  diff --git a/file b/file\n> >  index 257cc56..3bd1f0e 100644\n> >  --- a/file\n> >  +++ b/file\n> >  @@ -1 +1,2 @@\n> >   foo\n> >  +bar\n> >  $ \n> > \n> > The patch applies to master just fine.  Where's my thinking wrong?\n> \n> Hmm. Indeed. Thanks for clearing that up. Will work on it later.\n\nHi, did you get to this yet?, not to stress you, just to make sure we\ndon't forget about it.\n\nThanks, Gerrit.\n"},{"id":"45752","messageId":"Pine.LNX.4.64.0706250854120.4059@racer.site","threadId":"8584","inReplyTo":"20070625071819.8091.qmail@5e4088a43a10fd.315fe32.mid.smarden.org","subject":"Re: unexpected git-cherry-pick conflict","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-25T07:55:21Z","receivedAt":"2007-06-25T07:55:21Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 Jun 2007, Gerrit Pape wrote:\n\n> On Wed, Jun 13, 2007 at 03:43:48PM +0100, Johannes Schindelin wrote:\n> > On Wed, 13 Jun 2007, Gerrit Pape wrote:\n> > > On Wed, Jun 13, 2007 at 01:58:51PM +0100, Johannes Schindelin wrote:\n> > > > On Wed, 13 Jun 2007, Gerrit Pape wrote:\n> > > > >  $ git checkout master\n> > > > >  Switched to branch \"master\"\n> > > > >  $ git cherry-pick 8ddc4d5\n> > > > >  CONFLICT (file/directory): There is a directory with name link in\n> > > > >  8ddc4d5... file. Added link as link~HEAD\n> > > > \n> > > > Here you _still_ have the file in master. So that conflict is really \n> > > > expected, since a cherry-pick will only do a three-way merge.\n> > > \n> > > git-cherry-pick(1) states\n> > >  Given one existing commit, apply the change the patch introduces, and\n> > >  record a new commit that records it. This requires your working tree to\n> > >  be clean (no modifications from the HEAD commit).\n> > > \n> > > The patch introduced by the commit that's cherry-pick'ed has nothing to\n> > > do with the link or new directory, it just changes 'file'\n> > > \n> > >  $ git show 8ddc4d5\n> > >  commit 8ddc4d59444a362261e10a3b22324818f5dd2fa7\n> > >  Author: Gerrit Pape <pape@smarden.org>\n> > >  Date:   Wed Jun 13 09:10:30 2007 +0000\n> > >  \n> > >      file\n> > >  \n> > >  diff --git a/file b/file\n> > >  index 257cc56..3bd1f0e 100644\n> > >  --- a/file\n> > >  +++ b/file\n> > >  @@ -1 +1,2 @@\n> > >   foo\n> > >  +bar\n> > >  $ \n> > > \n> > > The patch applies to master just fine.  Where's my thinking wrong?\n> > \n> > Hmm. Indeed. Thanks for clearing that up. Will work on it later.\n> \n> Hi, did you get to this yet?, not to stress you, just to make sure we\n> don't forget about it.\n\nI did not have time yet. Thanks for the reminder.\n\nJust for the record, if you send a reply to my message to the list, but \nwithout Cc: to me, I am very likely to miss it. Just by chance I did not, \nthis time.\n\nCiao,\nDscho\n"},{"id":"46718","messageId":"Pine.LNX.4.64.0707071949440.4093@racer.site","threadId":"8584","inReplyTo":"20070625071819.8091.qmail@5e4088a43a10fd.315fe32.mid.smarden.org","subject":"Re: unexpected git-cherry-pick conflict","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-07T20:58:08Z","receivedAt":"2007-07-07T20:58:08Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 Jun 2007, Gerrit Pape wrote:\n\n> Hi, did you get to this yet?, not to stress you, just to make sure we \n> don't forget about it.\n\nOkay. Since now both you and Junio asked for it, and I made today a Git \nday for me, I looked into this.\n\nI'm not yet done, but preliminary results are: \n\n- cherry-pick calls merge-recursive, which in turn calls unpack_trees(), \n  which in turn gives the ball to threeway_merge. But for d/f conflicts, \n  it goes wrong.\n\n- Documentation/technical/trivial-merge.txt is maybe about a trivial \n  merge. But the document is not trivial in and of itself.\n\nI'll keep going,\nDscho\n"},{"id":"46722","messageId":"Pine.LNX.4.64.0707080148370.4093@racer.site","threadId":"8584","inReplyTo":"20070625071819.8091.qmail@5e4088a43a10fd.315fe32.mid.smarden.org","subject":"[PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T00:52:06Z","receivedAt":"2007-07-08T00:52:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen a merge has a d/f conflict on a path which was not touched\nbetween the merge base(s) and the remote HEAD, and the index and\nHEAD contain the same version for that path (even if empty), it\nis not really a conflict.\n\nNoticed by Rémi Vanicat, reported to the Git list by Gerrit Pape.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tThe only peculiar result is that you can have an entry at\n\tstage 0 for one path, and stages 1 and 3 for another path\n\twhere the first path is a prefix.\n\n\tThis can happen now, when you have deleted a directory\n\tsince the branching point, and put a file in the same place, but\n\tthe other side has changed files in that directory.\n\n\tThis change is reflected in the change to t3030.\n\n\tI have no idea if the change is the best there is, but after\n\tspending some hours trying to get my head around what is written\n\tin Documentation/technical/trivial-merge.txt, and what is in\n\tthe function threeway_merge(), and still not understanding most of \n\tit, there is not much more that I can do.\n\n\tFunny note at the side: when I finally got to Gerrit's email again\n\ttoday, gmane said _almost_ that it was 3 weeks, 3 days, 3 hours\n\tand 3 minutes ago...\n\n t/t3030-merge-recursive.sh |    2 +-\n t/t3502-cherry-pick.sh     |   31 +++++++++++++++++++++++++++++++\n unpack-trees.c             |   14 ++++++++++++++\n 3 files changed, 46 insertions(+), 1 deletions(-)\n create mode 100755 t/t3502-cherry-pick.sh\n\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex 607f57f..d413af1 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -450,7 +450,7 @@ test_expect_success 'merge-recursive d/f conflict result' '\n \t\techo \"100644 $o1 0\ta\"\n \t\techo \"100644 $o0 0\tb\"\n \t\techo \"100644 $o0 0\tc\"\n-\t\techo \"100644 $o6 2\td\"\n+\t\techo \"100644 $o6 0\td\"\n \t\techo \"100644 $o0 1\td/e\"\n \t\techo \"100644 $o1 3\td/e\"\n \t) >expected &&\ndiff --git a/t/t3502-cherry-pick.sh b/t/t3502-cherry-pick.sh\nnew file mode 100755\nindex 0000000..da3d26e\n--- /dev/null\n+++ b/t/t3502-cherry-pick.sh\n@@ -0,0 +1,31 @@\n+#!/bin/sh\n+\n+test_description='test cherry-pick'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\techo foo > file &&\n+\tln -s dangling link &&\n+\tgit add file link &&\n+\ttest_tick &&\n+\tgit commit -m foo &&\n+\tgit checkout -b branch &&\n+\tgit rm link &&\n+\ttest_tick &&\n+\tgit commit -m \"remove link\" &&\n+\tmkdir link &&\n+\techo bar > link/file &&\n+\tgit add link/file &&\n+\ttest_tick &&\n+\tgit commit -m \"add dir\" &&\n+\techo bar > file &&\n+\tgit commit -m \"change file\" file &&\n+\tgit checkout master\n+'\n+\n+test_expect_success 'cherry-pick ignores unrelated dir/symlink conflict' '\n+\tgit cherry-pick branch\n+'\n+\n+test_done\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex cac2411..080b016 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -643,6 +643,20 @@ int threeway_merge(struct cache_entry **stages,\n \tindex = stages[0];\n \thead = stages[o->head_idx];\n \n+\t/*\n+\t * Special case: do not care about a dir/file conflict, when\n+\t * the entries have not been touched.\n+\t * IOW if the ancestors are identical to the remote, and the\n+\t * index is the same as head, just take head.\n+\t */\n+\tif (head && same(index, head)) {\n+\t\tfor (i = 1; i < o->head_idx; i++)\n+\t\t\tif (!same(stages[i], remote))\n+\t\t\t\tbreak;\n+\t\tif (i == o->head_idx)\n+\t\t\treturn merged_entry(head, index, o);\n+\t}\n+\n \tif (head == o->df_conflict_entry) {\n \t\tdf_conflict_head = 1;\n \t\thead = NULL;\n-- \n1.5.3.rc0.2712.g125b7f\n\n"},{"id":"46724","messageId":"7vabu765r0.fsf@assigned-by-dhcp.cox.net","threadId":"8584","inReplyTo":"Pine.LNX.4.64.0707080148370.4093@racer.site","subject":"Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-08T01:31:15Z","receivedAt":"2007-07-08T01:31:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> @@ -643,6 +643,20 @@ int threeway_merge(struct cache_entry **stages,\n>  \tindex = stages[0];\n>  \thead = stages[o->head_idx];\n>  \n> +\t/*\n> +\t * Special case: do not care about a dir/file conflict, when\n> +\t * the entries have not been touched.\n> +\t * IOW if the ancestors are identical to the remote, and the\n> +\t * index is the same as head, just take head.\n> +\t */\n\nSuppose paths \"A\" and \"A/B\" are involved, and you resolved with\nthis logic to have \"A\" as a blob (so your HEAD does not have\n\"A/B\").  If the remote adds \"A/B\", what prevents the resulting\nindex to have both \"A\" and \"A/B\" resolved at stage #0?\n\nA logic to do \"if it is unchanged on one and changed in another,\ntake changed one\" already exists in later part of the code; your\npatch just circumvents D/F checks built into threeway_merge for\nthis one case, and only because this one case happens to have\nreported.  It doesn't feel right.\n\nIOW, don't make unpack-trees to make policy decisions on final\nresolution, unless it is operating under aggressive rule (where\nthe caller explicitly allows it to make more than the \"trivial\"\ndecisions).  The caller (in this case, merge-recursive) should\nsee A at stage #2 with A/B at stages #1 and #3 and decide what\nto do.\n"},{"id":"46726","messageId":"Pine.LNX.4.64.0707080248320.4093@racer.site","threadId":"8584","inReplyTo":"7vabu765r0.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T02:00:08Z","receivedAt":"2007-07-08T02:00:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 7 Jul 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > @@ -643,6 +643,20 @@ int threeway_merge(struct cache_entry **stages,\n> >  \tindex = stages[0];\n> >  \thead = stages[o->head_idx];\n> >  \n> > +\t/*\n> > +\t * Special case: do not care about a dir/file conflict, when\n> > +\t * the entries have not been touched.\n> > +\t * IOW if the ancestors are identical to the remote, and the\n> > +\t * index is the same as head, just take head.\n> > +\t */\n> \n> Suppose paths \"A\" and \"A/B\" are involved, and you resolved with\n> this logic to have \"A\" as a blob (so your HEAD does not have\n> \"A/B\").  If the remote adds \"A/B\", what prevents the resulting\n> index to have both \"A\" and \"A/B\" resolved at stage #0?\n\nHmm.\n\n> A logic to do \"if it is unchanged on one and changed in another,\n> take changed one\" already exists in later part of the code; your\n> patch just circumvents D/F checks built into threeway_merge for\n> this one case, and only because this one case happens to have\n> reported.  It doesn't feel right.\n\nWell, for me the code in threeway_merge does not feel right. There is a \ntable in technical/trivial-merge.txt (which I not fully understand, since \nnowhere in the table, there is a mention of the index, and nowhere is \nexplained what \"ALT\" is supposed to mean), but threeway_merge only \nreferences the numbers. The code is not obvious to me.\n\nI spent some hours staring on, and trying to make sense of it. Alas, I am \nan idiot or something, since my brain feels like a mashed potato, and I \nstill do not understand the code. For example, it is not apparent to me \nwhy the variable \"head\" should be set to NULL, when it was the \ndf_conflict_entry.\n\nSo yeah, this patch was marked as \"PATCH\", but the subject line is not \nlong enough for the proper prefix, which would have started like \n\"[This PATCH works for me, and I do not know how to make it better, since \nI do not understand the code in threeway_merge(), and that does not make \nme happy, but that is the way things are, and maybe someone more \nintelligent than me recognizes what is meant by my little patch, and can \nfix it up, ...]\".\n\n> IOW, don't make unpack-trees to make policy decisions on final \n> resolution, unless it is operating under aggressive rule (where the \n> caller explicitly allows it to make more than the \"trivial\" decisions).  \n> The caller (in this case, merge-recursive) should see A at stage #2 with \n> A/B at stages #1 and #3 and decide what to do.\n\nOkay, so you're saying that merge-recursive should use the aggressive \nstrategy?\n\nCiao,\nDscho\n"},{"id":"46730","messageId":"Pine.LNX.4.64.0707080311280.4093@racer.site","threadId":"8584","inReplyTo":"Pine.LNX.4.64.0707080248320.4093@racer.site","subject":"Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T02:18:26Z","receivedAt":"2007-07-08T02:18:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 8 Jul 2007, Johannes Schindelin wrote:\n\n> On Sat, 7 Jul 2007, Junio C Hamano wrote:\n> \n> > IOW, don't make unpack-trees to make policy decisions on final \n> > resolution, unless it is operating under aggressive rule (where the \n> > caller explicitly allows it to make more than the \"trivial\" \n> > decisions).  The caller (in this case, merge-recursive) should see A \n> > at stage #2 with A/B at stages #1 and #3 and decide what to do.\n> \n> Okay, so you're saying that merge-recursive should use the aggressive \n> strategy?\n\nTo refine on that: from my (limited, I admit) understanding of the code, \nby the time it hits that \"if (o->aggressive)\", in case of a df conflict, \nthe chance has long whizzed by to decide anything useful, since either \nhead or remote were set to NULL. So they are no longer what they would \nhave to be in order to make any sense.\n\nWell, I try to cobble up a patch for merge-recursive like you suggested, \nand stay away from threeway_merge() as far as I can, for the rest of my \nlife.\n\nHowever, it feels somehow wrong that I have to check all the files in the \nunmerged index, when unpack_trees could have easily seen that the tree did \nnot change between all (in that case, just one) ancestors and the remote, \nbut that head has changed that path to a file, and head agrees with index \non that, and remote can stay where it is with its darned directory.\n\nCiao,\nDscho\n"},{"id":"46732","messageId":"Pine.LNX.4.64.0707080526170.4093@racer.site","threadId":"8584","inReplyTo":"Pine.LNX.4.64.0707080311280.4093@racer.site","subject":"Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T04:35:58Z","receivedAt":"2007-07-08T04:35:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 8 Jul 2007, Johannes Schindelin wrote:\n\n> Well, I try to cobble up a patch for merge-recursive like you suggested, \n> and stay away from threeway_merge() as far as I can, for the rest of my \n> life.\n\nHere is a WIP. Note: it only does half of the job. For performance \nreasons, we do not write out the working tree changes for intermediate \nmerges. However, in the last step, we do. And this patch does not reflect \nthat, but only updates the index.\n\nCiao,\nDscho\n\n merge-recursive.c |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 88 insertions(+), 0 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex dbd06aa..99da2bb 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -243,6 +243,92 @@ static int git_merge_trees(int index_only,\n \treturn rc;\n }\n \n+/*\n+ * If there were dir/file conflicts, which are not really dir/file\n+ * conflicts, because only one side changed anything, take that\n+ * side instead of outputting silly conflicts.\n+ *\n+ * NOTE! unpack_trees() would have a much better chance at resolving\n+ * these conflicts, since it gets the _tree_ objects we have to\n+ * reconstruct tediously, and it could get away with one simple\n+ * comparison between sha1's, while that is not possible here.\n+ */\n+static inline int is_df(struct cache_entry *in_dir, struct cache_entry *file)\n+{\n+\tint len = ce_namelen(file);\n+\tconst char *name = in_dir->name;\n+\treturn !memcmp(name, file->name, len) && name[len] == '/';\n+}\n+\n+static int try_to_fix_up_df_conflicts_which_are_none(const unsigned char *base,\n+\t\tconst unsigned char *head, const unsigned char *merge)\n+{\n+\tint i;\n+\tfor (i = 0; i + 2 < active_nr; i++) {\n+\t\tint last;\n+\t\tunsigned char sha1[20], sha2[20];\n+\t\tunsigned dummy;\n+\t\tchar *name;\n+\t\tstruct cache_entry *ce = active_cache[i], *ce1, *ce2;\n+\n+\t\tif (ce_stage(ce) == 0)\n+\t\t\tcontinue;\n+\t\tce1 = active_cache[i + 1];\n+\t\tce2 = active_cache[i + 2];\n+\t\tif (ce_same_name(ce, ce2) || !is_df(ce2, ce)) {\n+\t\t\t/* not a d/f conflict */\n+\t\t\ti += 2;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tfor (last = i + 3; last < active_nr &&\n+\t\t\t\tis_df(active_cache[last], ce); last++)\n+\t\t\t; /* do nothing */\n+\t\t/* check if HEAD as an unchanged file, remote a dir */\n+\t\tif (ce_same_name(ce, ce1)) {\n+\t\t\tif (ce_stage(ce) != 1 ||\n+\t\t\t\t\thashcmp(ce->sha1, ce1->sha1)) {\n+\t\t\t\t/* file was changed */\n+\t\t\t\ti = last - 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\t/* other side removed file, added dir */\n+\t\t\tif (!remove_file_from_cache(ce->name))\n+\t\t\t\treturn error(\"index error\");\n+\t\t\tfor (i -= 2, last -= 2; i < last; i++)\n+\t\t\t\tactive_cache[i]->ce_flags &=\n+\t\t\t\t\t~ntohs(CE_STAGEMASK);\n+\t\t\ti--;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (i + 1 == last)\n+\t\t\tcontinue;\n+\t\t/* check if HEAD changed dir to a file */\n+\t\tif (ce_stage(ce) == 1) {\n+\t\t\ti = last - 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tname = xstrndup(ce->name, ce_namelen(ce));\n+\t\tif (get_tree_entry(base, name, sha1, &dummy) ||\n+\t\t\t\tget_tree_entry(ce_stage(ce) == 2 ?\n+\t\t\t\t\tmerge : head,\n+\t\t\t\t\tname, sha2, &dummy)) {\n+\t\t\tfree(name);\n+\t\t\ti = last - 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tfree(name);\n+\t\tif (!hashcmp(sha1, sha2)) {\n+\t\t\t/* remove tree */\n+\t\t\tmemmove(active_cache + i + 1, active_cache + last,\n+\t\t\t\t(active_nr - last) *\n+\t\t\t\tsizeof(struct cache_entry *));\n+\t\t\tactive_nr -= last - i - 1;\n+\t\t\tce->ce_flags &= ~ntohs(CE_STAGEMASK);\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n static int unmerged_index(void)\n {\n \tint i;\n@@ -1520,6 +1606,8 @@ static int merge_trees(struct tree *head,\n \t\t    sha1_to_hex(head->object.sha1),\n \t\t    sha1_to_hex(merge->object.sha1));\n \n+\ttry_to_fix_up_df_conflicts_which_are_none(common->object.sha1,\n+\t\t\thead->object.sha1, merge->object.sha1);\n \tif (unmerged_index()) {\n \t\tstruct path_list *entries, *re_head, *re_merge;\n \t\tint i;\n"},{"id":"46735","messageId":"7v644v5tr3.fsf@assigned-by-dhcp.cox.net","threadId":"8584","inReplyTo":"Pine.LNX.4.64.0707080248320.4093@racer.site","subject":"Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-08T05:50:24Z","receivedAt":"2007-07-08T05:50:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> IOW, don't make unpack-trees to make policy decisions on final \n>> resolution, unless it is operating under aggressive rule (where the \n>> caller explicitly allows it to make more than the \"trivial\" decisions).  \n>> The caller (in this case, merge-recursive) should see A at stage #2 with \n>> A/B at stages #1 and #3 and decide what to do.\n>\n> Okay, so you're saying that merge-recursive should use the aggressive \n> strategy?\n\nI do not think so.  Isn't the whole \"see if there are renames\" thing\ndepend on threeway_merge() not resolving \"one side removes other\nside leaves intact\" case itself?  Aggressive resolves it saying\n\"Ok that is a remove\", which risks it to miss the case in which\nthat the side that apparently \"removed\" the path in fact moved\nit somewhere else.\n\nThe last time I looked at merge-recursive's D/F check, I found\nthat it was not quite doing things right.  I may be able to dig\nup what I posted to the list...\n"},{"id":"46737","messageId":"7vwsxb4e2q.fsf@assigned-by-dhcp.cox.net","threadId":"8584","inReplyTo":"7v644v5tr3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-08T06:14:21Z","receivedAt":"2007-07-08T06:14:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> ...\n>> Okay, so you're saying that merge-recursive should use the aggressive \n>> strategy?\n>\n> I do not think so.  Isn't the whole \"see if there are renames\" thing\n> depend on threeway_merge() not resolving \"one side removes other\n> side leaves intact\" case itself?  Aggressive resolves it saying\n> \"Ok that is a remove\", which risks it to miss the case in which\n> that the side that apparently \"removed\" the path in fact moved\n> it somewhere else.\n>\n> The last time I looked at merge-recursive's D/F check, I found\n> that it was not quite doing things right.  I may be able to dig\n> up what I posted to the list...\n\nIt was from around April 7th-10th this year.\n\n    http://thread.gmane.org/gmane.comp.version-control.git/43970/focus=44158\n    http://thread.gmane.org/gmane.comp.version-control.git/43971/focus=43997\n\nI think the case described in the latter message is almost the\nopposite case of what your patch tries to deal with.\n\nIn the web interface of\n\n    http://news.gmane.org/gmane.comp.version-control.git\n\nthe patch series that led to my complaints are at around page 76\nfor me.\n"},{"id":"46752","messageId":"Pine.LNX.4.64.0707081351330.4248@racer.site","threadId":"8584","inReplyTo":"7v644v5tr3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T12:53:03Z","receivedAt":"2007-07-08T12:53:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 7 Jul 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >> IOW, don't make unpack-trees to make policy decisions on final \n> >> resolution, unless it is operating under aggressive rule (where the \n> >> caller explicitly allows it to make more than the \"trivial\" decisions).  \n> >> The caller (in this case, merge-recursive) should see A at stage #2 with \n> >> A/B at stages #1 and #3 and decide what to do.\n> >\n> > Okay, so you're saying that merge-recursive should use the aggressive \n> > strategy?\n> \n> I do not think so.\n\nYes, I realized that by running the tests with it.  A rename A->B in one, \nand A->C in the other branch will go undetected.\n\nI should have written this into my mail posting the WIP patch, but \nfrankly, I was too tired.\n\nCiao,\nDscho\n"},{"id":"46753","messageId":"Pine.LNX.4.64.0707081353560.4248@racer.site","threadId":"8584","inReplyTo":"7vwsxb4e2q.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T13:16:38Z","receivedAt":"2007-07-08T13:16:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 7 Jul 2007, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > The last time I looked at merge-recursive's D/F check, I found that it \n> > was not quite doing things right.  I may be able to dig up what I \n> > posted to the list...\n> \n> It was from around April 7th-10th this year.\n\nUnfortunately, this is way over my time budget.  As well as over my \nintelligence budget, since I did not even succeed in understanding the \ncode in threeway_merge _at all_.\n\nBesides, IMHO there is a deeper issue. Since merge-recursive started out \nas a Python script, and grew there until it was usable, and grew the \nrename detection therein, too, until it was finally converted to C, it \naccumulated a lot of features that would have been nice to have \nindependently.\n\nAlmost the same goes for unpack-trees, which (its name to the contrary) \ndoes quite a few things to merge entries, too.  And it tries to detect d/f \nconflicts, too.\n\nSo there we are, with two really big and unwieldy chunks of code, each \ndeserving an own GSoC project to clean them up.  Or maybe not even a GSoC \nproject, but a longer project.\n\nWhat I would _like_ to see is something as clean as merge-tree.  Which is \nclearly separated (code and file wise, too) into these stages:\n\n- reading the trees\n\n- determining renames\n\n- determining true d/f conflicts\n\n- threeway merge\n\n- writing the tree object\n\n- writing the work tree\n\n- recursive\n\nIdeally, merge-recursive would really have been as simple as\n\n\tcase \"$1\" in\n\t--index_only)\n\t\tindex_only=$1\n\t\tshift\n\tesac\n\ta=\"$1\"\n\tb=\"$2\"\n\tset $(git merge-base --all $a $b)\n\ttemp=$1\n\tshift\n\twhile case $# in 0) break;; esac\n\tdo\n\t\ttemp=$(git merge-recursive --index-only $temp $1)\n\t\tshift\n\tdone\n\tgit merge-non-recursive $index_only $temp -- \"$a\" \"$b\"\n\nbecause _read-tree -m_ should have learnt about renames, _not_ \nmerge-recursive.\n\nAs it is, both unpack_trees() and merge-recursive have a certain degree of \nnot-quite duplicated yet wants-to-do-largely-the-same functionality.  \nWhich of course leads to much finger pointing: \"it's unpack_trees() fault. \nno. it's merge-recursive's fault. no, vice versa.\"\n\nMaybe the proper way out is really to start from merge-tree.c and do \nsomething which is easy to understand, and concise, and thus has a much \nlesser chance of being buggy.\n\nCiao,\nDscho\n"},{"id":"46784","messageId":"7v8x9q1x5t.fsf@assigned-by-dhcp.cox.net","threadId":"8584","inReplyTo":"Pine.LNX.4.64.0707081353560.4248@racer.site","subject":"Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-08T20:02:38Z","receivedAt":"2007-07-08T20:02:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Besides, IMHO there is a deeper issue. Since merge-recursive started out \n> as a Python script, and grew there until it was usable, and grew the \n> rename detection therein, too, until it was finally converted to C, it \n> accumulated a lot of features that would have been nice to have \n> independently.\n> ...\n> So there we are, with two really big and unwieldy chunks of code, each \n> deserving an own GSoC project to clean them up.  Or maybe not even a GSoC \n> project, but a longer project.\n\nI would not disagree that tree merging part is a large piece of\ncode with nontrivial development history.  While I do agree that\nmerge-tree approach is attractive in the longer run, I do not\nthink the current \"use read-tree for policy-neutral merge and\nthen higher level to decide what to do with conflicts\" is beyond\nrepair.\n\nI tried a variant of cherry-pick that does _not_ use\nmerge-recursive on the reproduction recipe from Gerrit & Rémi.\nEssentially, a cherry-pick with a backend is:\n\n\tgit-merge-$backend $commit^ -- HEAD $commit\n\nIt was made cumbersome to try different merge backend when\ncherry-pick has become built-in, but the above essentially was\nwhat we had in the shell script version.  \n\nInterestingly, git-merge-resolve fails, but for an entirely\ndifferent reason; there is a bug in git-merge-one-file.  I am\nkind of surprised that this has been left undetected for a long\ntime (this shows how everybody uses git-merge-recursive these\ndays and not git-merge-resolve).  commit ed93b449 adds tests for\nonly the \"read-tree\" part, even though it makes \"matching\"\nchange to merge-one-file, which was the breakage.  The bug is\nthat merge-one-file did not \"resolve\" an unmerged index entry\nwhen it decided what the result should be --- it decided only in\nits comments but not with what it does!  Embarrassing (a fix is\nattached).\n\nWith that fix, the above \"cherry-pick\" lookalike with $backend\nset to \"resolve\" and $commit set to \"branch\" in Gerrit's example\nreproduction recipe seems to do the right thing.\n\n> As it is, both unpack_trees() and merge-recursive have a certain degree of \n> not-quite duplicated yet wants-to-do-largely-the-same functionality.  \n> Which of course leads to much finger pointing: \"it's unpack_trees() fault. \n> no. it's merge-recursive's fault. no, vice versa.\"\n\nI do not think there is any pointing-fingers involved in this\ncase.  The division of labor between \"read-tree -m\" and its\ncaller has been very clear from the beginning, and has not\nchanged.  The former does \"uncontroversial parts of merge\", and\nthe latter uses its own policy decision to resolve conflicts.\n\nThe \"uncontroversial parts of merge\" explicitly exclude \"one\nside removes (or adds), other side does not do anything\" case.\nThis is cumbersome for rename-unaware merge-resolve, because its\npolicy is \"we do not worry about the case that the apparent\nremoval is in fact it moves it to somewhere else -- if one side\nremoves, the result will not have it\", and for that policy if\n\"read-tree -m\" did so it would have less work to do.  But we\ndon't, exactly because other policy like merge-recursive may\nwant to look at the apparently removed path and figure out if\nthere is a rename involved.\n\nThe last time I looked at merge-recursive.c, I think the problem\nI saw was the way it maintains and uses two lists that keeps\ntrack of the set of directories and files; get_files_dirs() is\ncalled for both head and merge at the beginning, and I did not\nsee a code that says \"Oh, we recorded the path 'link' as being\npotentially a directory at the beginning, but we have decided to\nresolve 'link/file' by removing it, and 'link' does not have to\nexist as a directory anymore. resolving 'link' as a symbolic\nlink is perfectly fine\" --- and the reason Gerrit's test case\nfails was something like that.\n\n-- >8 --\n[PATCH] Fix merge-one-file for our-side-added/our-side-removed cases\n\nWhen commit ed93b449 changed the script so that it does not\ntouch untracked working tree file, we forgot that we still\nneeded to resolve the index entry (otherwise they are left\nunmerged).\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\ndiff --git a/git-merge-one-file.sh b/git-merge-one-file.sh\nindex ebbb575..1e7727d 100755\n--- a/git-merge-one-file.sh\n+++ b/git-merge-one-file.sh\n@@ -27,8 +27,9 @@ case \"${1:-.}${2:-.}${3:-.}\" in\n \t\t# read-tree checked that index matches HEAD already,\n \t\t# so we know we do not have this path tracked.\n \t\t# there may be an unrelated working tree file here,\n-\t\t# which we should just leave unmolested.\n-\t\texit 0\n+\t\t# which we should just leave unmolested.  Make sure\n+\t\t# we do not have it in the index, though.\n+\t\texec git update-index --remove -- \"$4\"\n \tfi\n \tif test -f \"$4\"; then\n \t\trm -f -- \"$4\" &&\n@@ -42,7 +43,8 @@ case \"${1:-.}${2:-.}${3:-.}\" in\n #\n \".$2.\")\n \t# the other side did not add and we added so there is nothing\n-\t# to be done.\n+\t# to be done, except making the path merged.\n+\texec git update-index --add --cacheinfo \"$6\" \"$2\" \"$4\"\n \t;;\n \"..$3\")\n \techo \"Adding $4\"\n@@ -50,7 +52,7 @@ case \"${1:-.}${2:-.}${3:-.}\" in\n \t\techo \"ERROR: untracked $4 is overwritten by the merge.\"\n \t\texit 1\n \t}\n-\tgit update-index --add --cacheinfo \"$6$7\" \"$2$3\" \"$4\" &&\n+\tgit update-index --add --cacheinfo \"$7\" \"$3\" \"$4\" &&\n \t\texec git checkout-index -u -f -- \"$4\"\n \t;;\n \n"},{"id":"46872","messageId":"Pine.LNX.4.64.0707091604030.5546@racer.site","threadId":"8584","inReplyTo":"7v8x9q1x5t.fsf@assigned-by-dhcp.cox.net","subject":"merge-one-file, was Re: [PATCH] merge-tree: sometimes, d/f conflict is not an issue","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-09T15:06:44Z","receivedAt":"2007-07-09T15:06:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 8 Jul 2007, Junio C Hamano wrote:\n\n> [PATCH] Fix merge-one-file for our-side-added/our-side-removed cases\n\nFWIW I have a patch series in my local repo, which makes merge-one-file a \nbuiltin, and even avoids fork()+exec()ing the program when called from \nunpack_trees().  However, I never came around to understand all the corner \ncases, and therefore did not write tests for it.  Therefore, I am not \nreally sure if it works as intended.  So I'd be very grateful if you could \nwrite tests while you are touching it anyway...\n\nCiao,\nDscho\n"},{"id":"47652","messageId":"Pine.LNX.4.64.0707171812490.14781@racer.site","threadId":"8584","inReplyTo":"7v8x9q1x5t.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 1/2] merge-recursive: sometimes, d/f conflict is not an issue","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-17T17:13:12Z","receivedAt":"2007-07-17T17:13:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen a merge has a d/f conflict on a path which was not touched\nbetween the merge base(s) and the remote HEAD, and the index and\nHEAD contain the same version for that path (even if empty), it\nis not really a conflict.\n\nNoticed by Rémi Vanicat, reported to the Git list by Gerrit Pape.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tTurns out I was wrong with my \"this is only half of it\" remark.\n\tAFAICT \"merge-tree -u -m\" does not touch the files which have\n\tunmerged entries, but merge-recursive does.  And since\n\tfix_up_df_conflicts() is called before any of that, it just works.\n\n merge-recursive.c |   87 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 87 insertions(+), 0 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex c8539ec..fb487ba 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -243,6 +243,91 @@ static int git_merge_trees(int index_only,\n \treturn rc;\n }\n \n+/*\n+ * If there were dir/file conflicts, which are not really dir/file\n+ * conflicts, because only one side changed anything, take that\n+ * side instead of outputting silly conflicts.\n+ *\n+ * We keep it nice and easy, in that we only fix the conflicts if one\n+ * side was renamed without changing the contents _at all_.  We check that\n+ * by comparing the hash.\n+ */\n+static inline int is_df(struct cache_entry *in_dir, struct cache_entry *file)\n+{\n+\tint len = ce_namelen(file);\n+\tconst char *name = in_dir->name;\n+\treturn !memcmp(name, file->name, len) && name[len] == '/';\n+}\n+\n+static int fix_up_df_conflicts(const unsigned char *base,\n+\t\tconst unsigned char *head, const unsigned char *merge)\n+{\n+\tint i;\n+\tfor (i = 0; i + 2 < active_nr; i++) {\n+\t\tint last;\n+\t\tunsigned char sha1[20], sha2[20];\n+\t\tunsigned dummy;\n+\t\tchar *name;\n+\t\tstruct cache_entry *ce = active_cache[i], *ce1, *ce2;\n+\n+\t\tif (ce_stage(ce) == 0)\n+\t\t\tcontinue;\n+\t\tce1 = active_cache[i + 1];\n+\t\tce2 = active_cache[i + 2];\n+\t\tif (ce_same_name(ce, ce2) || !is_df(ce2, ce)) {\n+\t\t\t/* not a d/f conflict */\n+\t\t\ti += 2;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tfor (last = i + 3; last < active_nr &&\n+\t\t\t\tis_df(active_cache[last], ce); last++)\n+\t\t\t; /* do nothing */\n+\t\t/* check if unchanged in HEAD */\n+\t\tif (ce_same_name(ce, ce1)) {\n+\t\t\tif (ce_stage(ce) != 1 ||\n+\t\t\t\t\thashcmp(ce->sha1, ce1->sha1)) {\n+\t\t\t\t/* file was changed */\n+\t\t\t\ti = last - 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\t/* other side removed file, added dir */\n+\t\t\tif (!remove_file_from_cache(ce->name))\n+\t\t\t\treturn error(\"index error\");\n+\t\t\tfor (i -= 2, last -= 2; i < last; i++)\n+\t\t\t\tactive_cache[i]->ce_flags &=\n+\t\t\t\t\t~ntohs(CE_STAGEMASK);\n+\t\t\ti--;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (i + 1 == last)\n+\t\t\tcontinue;\n+\t\t/* check if HEAD changed type (e.g. dir->file) */\n+\t\tif (ce_stage(ce) == 1) {\n+\t\t\ti = last - 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tname = xstrndup(ce->name, ce_namelen(ce));\n+\t\tif (get_tree_entry(base, name, sha1, &dummy) ||\n+\t\t\t\tget_tree_entry(ce_stage(ce) == 2 ?\n+\t\t\t\t\tmerge : head,\n+\t\t\t\t\tname, sha2, &dummy)) {\n+\t\t\tfree(name);\n+\t\t\ti = last - 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tfree(name);\n+\t\tif (!hashcmp(sha1, sha2)) {\n+\t\t\t/* remove tree */\n+\t\t\tmemmove(active_cache + i + 1, active_cache + last,\n+\t\t\t\t(active_nr - last) *\n+\t\t\t\tsizeof(struct cache_entry *));\n+\t\t\tactive_nr -= last - i - 1;\n+\t\t\tce->ce_flags &= ~ntohs(CE_STAGEMASK);\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n static int unmerged_index(void)\n {\n \tint i;\n@@ -1542,6 +1627,8 @@ static int merge_trees(struct tree *head,\n \t\t    sha1_to_hex(head->object.sha1),\n \t\t    sha1_to_hex(merge->object.sha1));\n \n+\tfix_up_df_conflicts(common->object.sha1,\n+\t\t\thead->object.sha1, merge->object.sha1);\n \tif (unmerged_index()) {\n \t\tstruct path_list *entries, *re_head, *re_merge;\n \t\tint i;\n-- \n1.5.3.rc1.16.g9d6f-dirty\n\n"},{"id":"47653","messageId":"Pine.LNX.4.64.0707171814330.14781@racer.site","threadId":"8584","inReplyTo":"7v8x9q1x5t.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 2/2] Add tests for cherry-pick d/f conflict which should be none","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-17T17:14:48Z","receivedAt":"2007-07-17T17:14:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis tests that we fixed the problem when a directory was renamed,\nand a symlink was added in its place (or for that matter, if\nthe type changed in one branch, and the name in another).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tThe last test checks that also a file/symlink conflict does\n\tnot break cherry-pick, if one side had a clean rename.\n\n t/t3502-cherry-pick.sh |   43 +++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 43 insertions(+), 0 deletions(-)\n create mode 100755 t/t3502-cherry-pick.sh\n\ndiff --git a/t/t3502-cherry-pick.sh b/t/t3502-cherry-pick.sh\nnew file mode 100755\nindex 0000000..9921de5\n--- /dev/null\n+++ b/t/t3502-cherry-pick.sh\n@@ -0,0 +1,43 @@\n+#!/bin/sh\n+\n+test_description='test cherry-pick'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\techo foo > file &&\n+\tln -s dangling link &&\n+\tgit add file link &&\n+\ttest_tick &&\n+\tgit commit -m foo &&\n+\tgit checkout -b branch &&\n+\tgit rm link &&\n+\ttest_tick &&\n+\tgit commit -m \"remove link\" &&\n+\tmkdir link &&\n+\techo bar > link/file &&\n+\tgit add link/file &&\n+\ttest_tick &&\n+\tgit commit -m \"add dir\" &&\n+\techo bar > file &&\n+\tgit commit -m \"change file\" file &&\n+\tgit checkout master\n+'\n+\n+test_expect_success 'cherry-pick ignores unrelated dir/symlink conflict' '\n+\tgit cherry-pick branch\n+'\n+\n+test_expect_success 'cherry-pick ignores unrelated file/symlink conflict' '\n+\tgit checkout -b branch2 master^ &&\n+\tgit rm link &&\n+\ttest_tick &&\n+\tgit commit -m \"remove link\" &&\n+\t: > link &&\n+\tgit add link &&\n+\ttest_tick &&\n+\tgit commit -m \"add file\" &&\n+\tgit cherry-pick master\n+'\n+\n+test_done\n-- \n1.5.3.rc1.16.g9d6f-dirty\n"},{"id":"50192","messageId":"20070808143943.12061.qmail@de98ec6b1268cc.315fe32.mid.smarden.org","threadId":"8584","inReplyTo":"Pine.LNX.4.64.0707171812490.14781@racer.site","subject":"Re: [PATCH 1/2] merge-recursive: sometimes, d/f conflict is not an issue","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-08-08T14:39:43Z","receivedAt":"2007-08-08T14:39:43Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Tue, Jul 17, 2007 at 06:13:12PM +0100, Johannes Schindelin wrote:\n> When a merge has a d/f conflict on a path which was not touched\n> between the merge base(s) and the remote HEAD, and the index and\n> HEAD contain the same version for that path (even if empty), it\n> is not really a conflict.\n\nHi, this patch solves the reported problem fine, and it didn't break\nanything for me yet.  Can this still be considered for 1.5.3?\n\nThanks, Gerrit.\n"},{"id":"63912","messageId":"20071221103743.2210.qmail@80f7ec9d6d380d.315fe32.mid.smarden.org","threadId":"8584","inReplyTo":"Pine.LNX.4.64.0707071949440.4093@racer.site","subject":"Re: unexpected git-cherry-pick conflict","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-12-21T10:37:43Z","receivedAt":"2007-12-21T10:37:43Z","isPatch":false,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Sat, Jul 07, 2007 at 09:58:08PM +0100, Johannes Schindelin wrote:\n> On Mon, 25 Jun 2007, Gerrit Pape wrote:\n> > Hi, did you get to this yet?, not to stress you, just to make sure we \n> > don't forget about it.\n> \n> Okay. Since now both you and Junio asked for it, and I made today a Git \n> day for me, I looked into this.\n\nHi, the discussion on this topic unfortunately didn't result in a patch.\nThe problem is still true with current master, here's again how to\nreproduce it\n\n $ mkdir repo && cd repo\n $ git init\n Initialized empty Git repository in .git/\n $ echo foo >file\n $ ln -s dangling link\n $ git add .\n $ git commit -mfoo\n Created initial commit c6a9189: foo\n  2 files changed, 2 insertions(+), 0 deletions(-)\n  create mode 100644 file\n  create mode 120000 link\n $ git checkout -b branch\n Switched to a new branch \"branch\"\n $ git rm link\n rm 'link'\n $ git commit -mremovelink\n Created commit 2c60f15: removelink\n  1 files changed, 0 insertions(+), 1 deletions(-)\n  delete mode 120000 link\n $ mkdir link\n $ echo bar >link/file\n $ git add link\n $ git commit -m adddir\n Created commit d3b30b5: adddir\n  1 files changed, 1 insertions(+), 0 deletions(-)\n  create mode 100644 link/file\n $ echo bar >>file\n $ git commit -mfile file\n Created commit 8ddc4d5: file\n  1 files changed, 1 insertions(+), 0 deletions(-)\n $ git checkout master\n Switched to branch \"master\"\n $ git cherry-pick 8ddc4d5\n CONFLICT (file/directory): There is a directory with name link in\n 8ddc4d5... file. Added link as link~HEAD\n Automatic cherry-pick failed.  After resolving the conflicts,\n mark the corrected paths with 'git-add <paths>'\n and commit the result.\n When commiting, use the option '-c 8ddc4d5' to retain authorship and\n message.\n $ \n\nThanks, Gerrit.\n"},{"id":"63985","messageId":"7vsl1vywbt.fsf@gitster.siamese.dyndns.org","threadId":"8584","inReplyTo":"20071221103743.2210.qmail@80f7ec9d6d380d.315fe32.mid.smarden.org","subject":"Re: unexpected git-cherry-pick conflict","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-22T08:20:38Z","receivedAt":"2007-12-22T08:20:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gerrit Pape <pape@smarden.org> writes:\n\n> On Sat, Jul 07, 2007 at 09:58:08PM +0100, Johannes Schindelin wrote:\n>> On Mon, 25 Jun 2007, Gerrit Pape wrote:\n>> > Hi, did you get to this yet?, not to stress you, just to make sure we \n>> > don't forget about it.\n>> \n>> Okay. Since now both you and Junio asked for it, and I made today a Git \n>> day for me, I looked into this.\n>\n> Hi, the discussion on this topic unfortunately didn't result in a patch.\n\nI know.  Unfortunately the code structure in merge-recursive\naround directory-file conflict handling needs major surgery to\nfix this, and it won't be until somebody really sits down and\nrethink the issue. I've been quietly working on some ideas and a\nsmall nascent part of it is in 'pu' (or 'offcuts', perhaps, I do\nnot remember offhand), but it will take time.\n"}]}