{"thread":{"id":"48028","subject":"Opinions on changing add/add conflict resolution?","startedAt":"2018-03-12T18:32:15Z","lastAt":"2018-03-13T23:30:41Z","messageCount":22,"participants":["Elijah Newren","Jonathan Nieder","Ævar Arnfjörð Bjarmason","Hilco Wijbenga","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"341486","messageId":"CABPp-BEXcvgxcn4-OrDhjUpuOsGCJwE_XT+0eYOK4Ad_O5maOg@mail.gmail.com","threadId":"48028","inReplyTo":null,"subject":"Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-12T18:32:09Z","receivedAt":"2018-03-12T18:32:15Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi everyone,\n\nI'd like to change add/add conflict resolution.  Currently when such a\nconflict occurs (say at ${path}), git unconditionally does a two-way\nmerge of the two files and sticks the result in the working tree at\n${path}.\n\nI would like to make it instead first check whether the two files are\nsimilar; if they are, then do the two-way merge, but if they're not,\nthen instead write the two files out to separate paths (${path}~HEAD\nand ${path}~$MERGE, while making sure that ${path} is removed from the\nworking copy).\n\nThoughts?\n\nI have a patch series[1] with more details and other changes, but\nwanted to especially get feedback on this issue even from folks that\ndidn't have enough time to read the patches or even the cover letter.\n\nThanks,\nElijah\n\n\n[1] https://public-inbox.org/git/20180305171125.22331-1-newren@gmail.com/\n"},{"id":"341488","messageId":"20180312184734.GA58506@aiede.svl.corp.google.com","threadId":"48028","inReplyTo":"CABPp-BEXcvgxcn4-OrDhjUpuOsGCJwE_XT+0eYOK4Ad_O5maOg@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-12T18:47:34Z","receivedAt":"2018-03-12T18:47:44Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nElijah Newren wrote:\n\n> Hi everyone,\n>\n> I'd like to change add/add conflict resolution.  Currently when such a\n> conflict occurs (say at ${path}), git unconditionally does a two-way\n> merge of the two files and sticks the result in the working tree at\n> ${path}.\n>\n> I would like to make it instead first check whether the two files are\n> similar; if they are, then do the two-way merge, but if they're not,\n> then instead write the two files out to separate paths (${path}~HEAD\n> and ${path}~$MERGE, while making sure that ${path} is removed from the\n> working copy).\n>\n> Thoughts?\n\nMy immediate reaction is that it seems inconsistent with the rest of\nmerge behavior.  Why would add/add behave this way but edit/edit not\nbehave this way?\n\nWould this behavior be configurable or unconditional?  I suspect I\nwould want it turned off in my own use.\n\nOn the other hand, in the case of wild difference between the two\nfiles, skipping the two-way merge and just writing one of the versions\nto the worktree (like we do for binary files) sounds like something I\nwould like in my own use.\n\nCan you add something more about the motivation to the commit message?\nE.g. is this about performance, interaction with some tools, to\nsupport some particular workflow, etc?\n\nThanks and hope that helps,\nJonathan\n\n> I have a patch series[1] with more details and other changes, but\n> wanted to especially get feedback on this issue even from folks that\n> didn't have enough time to read the patches or even the cover letter.\n>\n> Thanks,\n> Elijah\n>\n> [1] https://public-inbox.org/git/20180305171125.22331-1-newren@gmail.com/\n"},{"id":"341492","messageId":"CABPp-BEdh+UOCpFn5Y1_RydR==dDHWTeNtBub+pPjH_06Ub28w@mail.gmail.com","threadId":"48028","inReplyTo":"20180312184734.GA58506@aiede.svl.corp.google.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-12T21:26:02Z","receivedAt":"2018-03-12T21:26:09Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nCool, thanks for taking a look!\n\nOn Mon, Mar 12, 2018 at 11:47 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>\n> My immediate reaction is that it seems inconsistent with the rest of\n> merge behavior.  Why would add/add behave this way but edit/edit not\n> behave this way?\n\nFair enough.  I have two separate reasons for believing that edit/edit\nIS different than add/add; further, the current behavior for add/add\nis inconsistent with the rest of merge behavior on a different axis.\nI think it's helpful to get the motivation for my changes before\ntrying to explain why those are different and before delving into the\nother inconsistency, so I'll add the explanation of this claim at the\nend of this email.\n\n> Would this behavior be configurable or unconditional?  I suspect I\n> would want it turned off in my own use.\n>\n> On the other hand, in the case of wild difference between the two\n> files, skipping the two-way merge and just writing one of the versions\n> to the worktree (like we do for binary files) sounds like something I\n> would like in my own use.\n\nI think you just said the exact opposite thing in these last two\nparagraphs; that you wouldn't want my proposed behavior and that you'd\nwant it.  I suspect that may mean that I misunderstood something you\nsaid here.  Could you clarify?\n\n> Can you add something more about the motivation to the commit message?\n> E.g. is this about performance, interaction with some tools, to\n> support some particular workflow, etc?\n\nTo be honest, I'm a little unsure how without even more excessive and\nrepetitive wording across commits.  Let me attempt here, and maybe you\ncan suggest how to change my commit messages?\n\n  * When files are wildly dissimilar -- as you mentioned -- it'd be\neasier for users to resolve conflicts if we wrote files out to\nseparate paths instead of two-way merging them.\n  * There is a weird inconsistency between handling of add/add,\nrename/add, and rename/rename(2to1).  I want this inconsistency fixed.\n  * There is a significant performance optimization possible for\nrename detection in git merge if we remove the above inconsistency and\ntreat these conflict types the same.  (Actually, we only need them to\nbe the same in the special case where a renamed file is unmodified on\nthe un-renamed side of history, but I don't want to special case that\nsituation because it sounds like a recipe for inconsistent results).\n  * If we insist on these conflict types being handled differently\nbecause there really is some important distinction between them, then\nI'm almost certain that build_fake_ancestor() and it's usage in\nam/rebase is broken/wrong.  This is because the usage of\nbuild_fake_ancestor(), as currently implemented, will cause\nmis-detection of one conflict type as another.\n\n> Thanks and hope that helps,\n> Jonathan\n\nAs promised above, here are my reasons for believing that edit/edit IS\nfundamentally different than add/add for the behavior considered here,\nas well as my explanation of the weird inconsistency add/add currently\nhas with the rest of merge behavior:\n\n==Reason 1==\nedit/edit means that ${path} existed in the merge base as well as both\nsides.  It is more likely than not that ${path} on each side is\nsimilar to the merge base and thus similar (though less so) to each\nother.  On the other hand, add/add means ${path} did NOT exist in the\nmerge base.  Thus, we cannot use the same reason to believe they are\nsimilar.  The only reason we have to assume similarity is the\nfilename, which, while a useful indicator, gives us a weird\ninconsistency within git for handling pathname collisions -- see\n\"Weird inconsistency\" below.\n\n==Reason 2==\nIn merges, git does rename detection, but not copy or break detection.\nIn particular, break detection is what would allow us to find out that\nthe edited files are not similar to the original.  add/add conflicts\nautomatically come to us \"pre-broken\" (is there a better term for\nknowing that two files are not paired?), though it is possible that\nthey just happen to be similar and should be paired/joined.\n\n==Follow on to reason 2==\nNot doing break detection means we get e.g. rename/add-source cases\nwrong, so there are valid arguments for saying we should add break\ndetection.  However, it would be computationally expensive and would\nrequire a fair amount of work re-thinking all the cases in\nmerge-recursive to make sure we get rename-breaks right (there are\nFIXMEs and comments and testcases I added years ago to help someone\nalong the path if they want to try).  Since rename/add-source just\nisn't a conflict type that occurs much (if it all) in real world\nrepositories, the motivation to implement it is somewhat low.\n\n==Weird inconsistency git currently exhibits==\nThere are three types of conflicts representing two (possibly\nunrelated) files colliding at the same path: add/add, rename/add, and\nrename/rename(2to1).  add/add does the two-way merge of the colliding\nfiles, and the other two conflict types write the two colliding files\nout to separate paths.  I think the motivation behind the current\nbehavior was that add/add was written assuming filename-match implies\nsimilarity, and the other two were written assuming filename-match\ndidn't imply anything and the files should be assumed dissimilar.\nThat seems weird to me.  I think all three should behave the same\n(especially when the renamed file was unmodified on the un-renamed\nside of history, and likely whenever there is no content conflicts for\nthe renamed file).\n"},{"id":"341493","messageId":"20180312213521.GB58506@aiede.svl.corp.google.com","threadId":"48028","inReplyTo":"CABPp-BEdh+UOCpFn5Y1_RydR==dDHWTeNtBub+pPjH_06Ub28w@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-12T21:35:21Z","receivedAt":"2018-03-12T21:35:31Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi again,\n\nElijah Newren wrote:\n> On Mon, Mar 12, 2018 at 11:47 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>> Would this behavior be configurable or unconditional?  I suspect I\n>> would want it turned off in my own use.\n>>\n>> On the other hand, in the case of wild difference between the two\n>> files, skipping the two-way merge and just writing one of the versions\n>> to the worktree (like we do for binary files) sounds like something I\n>> would like in my own use.\n>\n> I think you just said the exact opposite thing in these last two\n> paragraphs; that you wouldn't want my proposed behavior and that you'd\n> want it.  I suspect that may mean that I misunderstood something you\n> said here.  Could you clarify?\n\nSorry for the lack of clarity.  My understanding was that the proposed\nbehavior was to write two files:\n\n\t${path}~HEAD\n\t${path}~MERGE\n\nMy proposal is instead to write one file:\n\n\t${path}\n\nwith the content that would have gone to ${path}~HEAD.  This is what\nalready happens when trying to merge binary files.\n\n[...]\n>> Can you add something more about the motivation to the commit message?\n>> E.g. is this about performance, interaction with some tools, to\n>> support some particular workflow, etc?\n>\n> To be honest, I'm a little unsure how without even more excessive and\n> repetitive wording across commits.\n\nSimplest way IMHO is to just put the rationale in patch 5/5. :)  In\nother words, explain the rationale for the end-user facing change in the\nsame patch that changes the end-user facing behavior.\n\n>                                     Let me attempt here, and maybe you\n> can suggest how to change my commit messages?\n>\n>   * When files are wildly dissimilar -- as you mentioned -- it'd be\n> easier for users to resolve conflicts if we wrote files out to\n> separate paths instead of two-way merging them.\n\nToday what we do (in both the wildly-dissimilar case and the\nless-dissimilar case) is write one proposed resolution to the worktree\nand put the competing versions in the index.  Tools like \"git\nmergetool\" are then able to pull the competing versions out of the\nindex to allow showing them at the same time.\n\nMy bias is that I've used VCSes before that wrote multiple competing\nfiles to the worktree and I have been happier with my experience\nresolving conflicts in git.  E.g. at any step I can run a build to try\nout the current proposed resolution, and there's less of a chance of\naccidentally commiting a ~HEAD file.\n\n[...]\n> There are three types of conflicts representing two (possibly\n> unrelated) files colliding at the same path: add/add, rename/add, and\n> rename/rename(2to1).  add/add does the two-way merge of the colliding\n> files, and the other two conflict types write the two colliding files\n> out to separate paths.\n\nInteresting.  I would be tempted to resolve this inconsistency the\nother way: by doing a half-hearted two-way merge (e.g. by picking one\nof the two versions of the colliding file) and marking the path as\nconflicted in the index.  That way it's more similar to edit/edit,\ntoo.\n\nThanks,\nJonathan\n"},{"id":"341499","messageId":"876061q6m6.fsf@evledraar.gmail.com","threadId":"48028","inReplyTo":"CABPp-BEXcvgxcn4-OrDhjUpuOsGCJwE_XT+0eYOK4Ad_O5maOg@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-03-12T22:19:45Z","receivedAt":"2018-03-12T22:19:54Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Mar 12 2018, Elijah Newren jotted:\n\n> Hi everyone,\n>\n> I'd like to change add/add conflict resolution.  Currently when such a\n> conflict occurs (say at ${path}), git unconditionally does a two-way\n> merge of the two files and sticks the result in the working tree at\n> ${path}.\n>\n> I would like to make it instead first check whether the two files are\n> similar; if they are, then do the two-way merge, but if they're not,\n> then instead write the two files out to separate paths (${path}~HEAD\n> and ${path}~$MERGE, while making sure that ${path} is removed from the\n> working copy).\n>\n> Thoughts?\n>\n> I have a patch series[1] with more details and other changes, but\n> wanted to especially get feedback on this issue even from folks that\n> didn't have enough time to read the patches or even the cover letter.\n\nDoes this mean that e.g. in this case of merging two files, one\ncontaining \"foo\" and one containing \"bar\":\n\n    (\n        rm -rf /tmp/test.git &&\n        git init /tmp/test.git &&\n        cd /tmp/test.git &&\n        echo foo >README &&\n        git add README &&\n        git commit -mfoo &&\n        git checkout --orphan trunk &&\n        git reset --hard &&\n        echo bar >README &&\n        git add README &&\n        git commit -mbar &&\n        git merge --allow-unrelated-histories master;\n        cat README\n    )\n\nThat instead of getting:\n\n    <<<<<<< HEAD\n    bar\n    =======\n    foo\n    >>>>>>> master\n\nI'd now get these split into different files?\n\nI'm assuming by similarity you're talking about the same heuristic we\napply for git diff -M, i.e. if \"moving\" a file would consider it\nremoved/added instead of moved you'd want two files instead of the\ntwo-way merge.\n\nI don't mind this being a configurable option if you want it, but I\ndon't think it should be on by default, reasons:\n\n 1) There's lots of cases where we totally screw up the \"is this\n    similar?\" check, in particular with small files.\n\n    E.g. let's say you have a config file like 'fs-path \"/tmp/git\"' and\n    in two branches you change that to 'fs-path \"/opt/git\"' and 'fs-path\n    \"/var/git\"'. The rename detection will think this these have nothing\n    to do with each other since they share no common lines, but to a\n    human reader they're really similar, and would make sense in the\n    context of resolving a bigger merge where /{opt,var}/git changes are\n    conflicting.\n\n    This is not some theoretical concern, there's lots of things that\n    e.g. use small 5-10 line config files to configure some app that\n    because of some combo of indentation changes and changing a couple\n    of lines will make git's rename detection totally give up, but to a\n    human reader they're 95% the same.\n\n 2) This will play havoc with already established merge tools on top of\n    git which a lot of users use instead of manually resolving these in\n    vi or whatever.\n\n    If we made this the default they'd need to to deal with this new\n    state, and even if it's not the default we'll have some confused\n    users wondering why Emacs Ediff or whatever isn't showing the right\n    thing because it isn't supporting this yet.\n\nSo actually, given that last point in #2 I'm slightly negative on the\nwhole thing, but maybe splitting it into some new format existing tools\ndon't understand is compelling enough to justify the downstream breakage.\n\nI don't think we've ever documented the format we leave the tree in\nafter a failed merge as equivalent to plumbing, but for the purposes of\ntools that build on top of git it really is.\n"},{"id":"341505","messageId":"CAE1pOi3jbLyZ7zgCO5J_BrQurrxZvm5RH+ZjXTTBf4MMmVOduA@mail.gmail.com","threadId":"48028","inReplyTo":"20180312213521.GB58506@aiede.svl.corp.google.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Hilco Wijbenga","fromEmail":"hilco.wijbenga@gmail.com","sentAt":"2018-03-12T23:08:29Z","receivedAt":"2018-03-12T23:08:56Z","isPatch":false,"sender":{"key":"hilco.wijbenga@gmail.com","avatar":null},"body":"On Mon, Mar 12, 2018 at 2:35 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Interesting.  I would be tempted to resolve this inconsistency the\n> other way: by doing a half-hearted two-way merge (e.g. by picking one\n> of the two versions of the colliding file) and marking the path as\n> conflicted in the index.  That way it's more similar to edit/edit,\n> too.\n\nIf work is going to be done in this area, would it be possible to\ninclude making auto-merging (in general) optional? Preferably,\nconfigurable by file (or glob) but I'd already be happy with a global\nsetting to opt out.\n\nI keep running into bugs caused by Git's automatic merging. (And I\ndon't see how this could be improved without teaching Git the\nspecifics of various file types.) It's especially hard when rebasing\nlarge numbers of commits. The bug is introduced early on but I will\nnot notice anything is wrong until late in the rebase.\n\n(Since I seem to keep asking for things that turn out to already have\nbeen implemented ... if this is already possible please just point me\nto the right setting and consider me a happy camper. :-) )\n"},{"id":"341506","messageId":"20180312231419.GA235366@aiede.svl.corp.google.com","threadId":"48028","inReplyTo":"CAE1pOi3jbLyZ7zgCO5J_BrQurrxZvm5RH+ZjXTTBf4MMmVOduA@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-12T23:14:19Z","receivedAt":"2018-03-12T23:14:32Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nHilco Wijbenga wrote:\n> On Mon, Mar 12, 2018 at 2:35 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>> Interesting.  I would be tempted to resolve this inconsistency the\n>> other way: by doing a half-hearted two-way merge (e.g. by picking one\n>> of the two versions of the colliding file) and marking the path as\n>> conflicted in the index.  That way it's more similar to edit/edit,\n>> too.\n>\n> If work is going to be done in this area, would it be possible to\n> include making auto-merging (in general) optional? Preferably,\n> configurable by file (or glob) but I'd already be happy with a global\n> setting to opt out.\n\nHave you experimented with the 'merge' attribute (see \"git help\nattributes\")?  E.g. you can put\n\n * -merge\n\nin .gitattributes or .git/info/attributes.\n\nIf that helps, then a patch adding a pointer to the most helpful place\n(maybe git-merge.txt?) would be very welcome.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"341519","messageId":"CABPp-BE653uMpwN4zfCCP8teRGmZ6v5NEyASCR1PTvHhoMKE1w@mail.gmail.com","threadId":"48028","inReplyTo":"20180312213521.GB58506@aiede.svl.corp.google.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-13T00:38:58Z","receivedAt":"2018-03-13T00:39:05Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"I'm worried that my attempt to extract add/add from the rest of the\ndiscussion with rename/add and rename/rename resulted in a false sense\nof simplification.  Trying to handle all the edge and corner cases and\nremain consistent sometimes gets a little hairy.  Anyway...\n\nOn Mon, Mar 12, 2018 at 2:35 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Elijah Newren wrote:\n>> On Mon, Mar 12, 2018 at 11:47 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>\n\n> Sorry for the lack of clarity.  My understanding was that the proposed\n> behavior was to write two files:\n>\n>         ${path}~HEAD\n>         ${path}~MERGE\n\n(Just to be clear: The proposed behavior was to do that only when the\ncolliding files were dissimilar, and otherwise two-way merging.)\n\n> My proposal is instead to write one file:\n>\n>         ${path}\n>\n> with the content that would have gone to ${path}~HEAD.  This is what\n> already happens when trying to merge binary files.\n\nThanks for clarifying.\n\nI feel the analogy to merging binary files breaks down here in more\nthan one way:\n\n1)\n\nMerging binary files is more complex than you suggest here.  In\nparticular, when creating a virtual merge base, the resolution chosen\nis not the version of the file from HEAD, but the version of the file\nfrom the merge base.  Nasty problems would result otherwise for the\nrecursive case.\n\nIf we tried to match how merging binary files behaved, we run into the\nproblem that the colliding file conflict case has no common version of\nthe file from a merge base.  So the same strategy just doesn't apply.\nThe closest would be outright deleting both versions of the file for\nthe virtual merge base and punting to the outer merge to deal with it.\nThat might be okay, but seems really odd to me.  I feel like it'd\nunnecessarily increase the number of conflicts users will see, though\nmaybe it wouldn't be horrible if this was only done when the files\nwere considered dissimilar anyway.\n\n2)\n\nmerging binary files only has 3 versions of the file to store at a\nsingle $path in the index, which fit naturally in stages 1-3;\nrename/add and rename/rename(2to1) have 4 and 6, respectively.  Having\nthree versions of a file from a 3-way merge makes fairly intuitive\nsense.  Have 4 or 6 versions of a file at a path from a 3-way merge is\ninherently more complex.  I think that just using the version from\nHEAD risks users resolving things incorrectly much more likely than in\nthe case of a binary merge.\n\n\n> [...]\n>>> Can you add something more about the motivation to the commit message?\n>>> E.g. is this about performance, interaction with some tools, to\n>>> support some particular workflow, etc?\n>>\n>> To be honest, I'm a little unsure how without even more excessive and\n>> repetitive wording across commits.\n>\n> Simplest way IMHO is to just put the rationale in patch 5/5. :)  In\n> other words, explain the rationale for the end-user facing change in the\n> same patch that changes the end-user facing behavior.\n\nPatches 3, 4, and 5 all change end-user facing behavior -- one patch\nper conflict type to make the three types behave the same (the first\ntwo patches add more testcases, and write a common function all three\ntypes can use).  The rationale for the sets of changes is largely the\nsame, and is somewhat lengthy.  Should I repeat the rationale in full\nin all three places?\n\n>>                                     Let me attempt here, and maybe you\n>> can suggest how to change my commit messages?\n>>\n>>   * When files are wildly dissimilar -- as you mentioned -- it'd be\n>> easier for users to resolve conflicts if we wrote files out to\n>> separate paths instead of two-way merging them.\n>\n> Today what we do (in both the wildly-dissimilar case and the\n> less-dissimilar case) is write one proposed resolution to the worktree\n> and put the competing versions in the index.  Tools like \"git\n> mergetool\" are then able to pull the competing versions out of the\n> index to allow showing them at the same time.\n>\n> My bias is that I've used VCSes before that wrote multiple competing\n> files to the worktree and I have been happier with my experience\n> resolving conflicts in git.  E.g. at any step I can run a build to try\n> out the current proposed resolution, and there's less of a chance of\n> accidentally commiting a ~HEAD file.\n>\n> [...]\n>> There are three types of conflicts representing two (possibly\n>> unrelated) files colliding at the same path: add/add, rename/add, and\n>> rename/rename(2to1).  add/add does the two-way merge of the colliding\n>> files, and the other two conflict types write the two colliding files\n>> out to separate paths.\n>\n> Interesting.  I would be tempted to resolve this inconsistency the\n> other way: by doing a half-hearted two-way merge (e.g. by picking one\n> of the two versions of the colliding file) and marking the path as\n> conflicted in the index.  That way it's more similar to edit/edit,\n> too.\n\nYour proposal is underspecified; there are more complicated cases\nwhere your wording could have different meanings.\n\nI also had a backup plan was to propose making rename/add and\nrename/rename(2to1) behave more like add/add (instead of making the\nbehavior for all three a blend of their current behaviors), but the\nproblem is I have two backup proposals to choose between.  And it\nsounds like you're adding a third, which follows your ~HEAD strategy\nfrom above.  Let me see if I can explain why your proposal is\nunderspecified:\n\nFor rename/rename(2to1) we have up to 3 sets of conflicts.  Let's say\nwe renamed A->C in HEAD and B->C on the other side.  We first need to\ndo a three-way content merge on the first C (with the other two\nversions of A), then we need to do a three-way content merge on the\nsecond C (with the other two versions of B).  This gives us two\ndifferent C's, both with possible conflict markers, that want to be\nstored at the same pathname.  Let me call these two versions of C, C1\nand C2.  C1 and C2 may or may not be similar.  Resolving the fact that\nboth want to be stored at C is what gives us potentially a third set\nof conflicts.\n\nWhat's your resolution strategy here, for each combination of cases\n(has conflict markers or not; is similar to the other C or not;\nworking on a virtual merge base or not)?\n\nFor reference there are two possible backup strategies I could propose here:\n\nBackup Proposal #1: act stricly like add/add:\nDo a two-way merge of C1 and C2.  If we end up with nested conflict\nmarkers, oh well.  Good luck, user.\n\nBackup Proposal #2: act like add/add, but only if C1 and C2 don't have\nconflict markers.\nIf C1 or C2 have conflict markers, behave as currently -- record C1\nand C2 to different paths.\nIf C1 and C2 do not have conflict markers, two-way merge them and\nrecord in the worktree at C.\n\nMy question for your counter-proposal:\nDo you record C1 or C2 as C?  Or do you record the version of C from\nHEAD as C?  And what do you pick when creating a virtual merge base?\n\nProblems I see regardless of the choice in your counter-proposal:\n  * If you choose C from HEAD, you are ignoring 3 other versions of\n\"C\" (as opposed to just one other version when merging a binary file);\nwill the user recognize that and know how to look at all four\nversions?\n  * If you choose C1 or C2, the user will see conflict markers; will\nthe user recognize that this is only a _subset_ of the conflicts, and\nthat there's a lot of content that was completely ignored?\n  * There is no \"merge base of C1 and C2\" to use for the virtual merge\nbase.  What will you use?\n"},{"id":"341521","messageId":"CABPp-BFsiWBiDYYvz=cOofECUHUMJs8x8RuMXqeCF1qP5HzGoQ@mail.gmail.com","threadId":"48028","inReplyTo":"CABPp-BHDOimDoLxWxS=BDOBkm6CUTrXTzD16=TSkWGN-HOiU2g@mail.gmail.com","subject":"Fwd: Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-13T02:53:10Z","receivedAt":"2018-03-13T02:53:17Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"[Re-sending because this computer happened to have plain-text mode\nturned off for some weird reason, and thus the email bounced]\n\nHi,\n\nOn Mon, Mar 12, 2018 at 3:19 PM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n>\n> Does this mean that e.g. in this case of merging two files, one\n> containing \"foo\" and one containing \"bar\":\n>\n>     (\n>         rm -rf /tmp/test.git &&\n>         git init /tmp/test.git &&\n>         cd /tmp/test.git &&\n>         echo foo >README &&\n>         git add README &&\n>         git commit -mfoo &&\n>         git checkout --orphan trunk &&\n>         git reset --hard &&\n>         echo bar >README &&\n>         git add README &&\n>         git commit -mbar &&\n>         git merge --allow-unrelated-histories master;\n>         cat README\n>     )\n>\n> That instead of getting:\n>\n>     <<<<<<< HEAD\n>     bar\n>     =======\n>     foo\n>     >>>>>>> master\n>\n> I'd now get these split into different files?\n\nAs currently implemented, yes.  However, I was more concerned the idea\nof handling files differently based on whether or not they were\nsimilar, rather than on what the precise definition of \"similar\" is\nfor this context.\n\nAs far as the definition of similarity goes, estimate_similarity() is\ncurrently used by rename detection to compare files recorded at\ndifferent pathnames.  By contrast, in this context, we are comparing\ntwo files which were recorded with the same pathname.  That suggests\nthe heuristic could be a bit different and use more than just\nestimate_similarity().  (e.g. \"We consider these files similar IF more\nthan 50% of the lines match OR both files are less than 2K.\")\n\n\n> I don't mind this being a configurable option if you want it, but I\n> don't think it should be on by default, reasons:\n\nI don't think a configurable option makes sense, at least not for my\npurposes.  Having rename/rename conflicts be \"safely\" mis-detected as\nrename/add or add/add, and having rename/add conflicts be \"safely\"\nmis-detected as add/add is my overriding concern.  Thus, making these\nthree conflict types behave consistently is what I need.  Options\nwould make that more difficult for me, and would thus feel like a step\nbackwards.\n\ngit am/rebase has been doing such mis-detections for years (almost\nsince the \"dawn\" of git time), but it feels really broken to me\nbecause the conflict types aren't handled consistently.  (The facts\nthat (a) I'm the only one that has added rename/add testcases to\ngit.git, (b) that I've added all but one of the rename/rename(2to1)\ntestcases to the git.git testsuite, and (c) that rename/add has had\nmultiple bugs for many years, all combine to suggest to me that folks\njust don't hit those conflict types in practice and thus that they\njust aren't noticing this breakage -- yet.)\n\nI also want to allow such mis-detections for cherry-picks and merges\nbecause of the significant (nearly order-of-magnitude in some cases)\nperformance improvements I can get in rename detection if it's\nallowed.\n\n\n>  1) There's lots of cases where we totally screw up the \"is this\n>     similar?\" check, in particular with small files.\n>\n>     E.g. let's say you have a config file like 'fs-path \"/tmp/git\"' and\n>     in two branches you change that to 'fs-path \"/opt/git\"' and 'fs-path\n>     \"/var/git\"'. The rename detection will think this these have nothing\n>     to do with each other since they share no common lines, but to a\n>     human reader they're really similar, and would make sense in the\n>     context of resolving a bigger merge where /{opt,var}/git changes are\n>     conflicting.\n>\n>     This is not some theoretical concern, there's lots of things that\n>     e.g. use small 5-10 line config files to configure some app that\n>     because of some combo of indentation changes and changing a couple\n>     of lines will make git's rename detection totally give up, but to a\n>     human reader they're 95% the same.\n\nFair enough.  The small files case could potentially be handled by\njust changing the similarity metric for these conflict types, as noted\nabove.  If it's a small file, that might be the easiest way for a user\nto deal with it too.\n\nI'm not sure I see the problem with the bigger files, though.  If you\nhave bigger files with less than 50% of the lines matching, then\nyou'll essentially end up with a great big conflict block with one\nfile on one side and the other file on the other side, which doesn't\nseem that different to me than having them be in two separate files.\nIn fact, separate files seems easier to deal with because then the\nuser can run e.g. 'git diff --no-index --color-words FILE1 FILE2',\nsomething that they can't do when it's in one file.  That has bothered\nme more than once, and made me wish they were just in separate files.\n\n\n>  2) This will play havoc with already established merge tools on top of\n>     git which a lot of users use instead of manually resolving these in\n>     vi or whatever.\n>\n>     If we made this the default they'd need to to deal with this new\n>     state, and even if it's not the default we'll have some confused\n>     users wondering why Emacs Ediff or whatever isn't showing the right\n>     thing because it isn't supporting this yet.\n>\n> So actually, given that last point in #2 I'm slightly negative on the\n> whole thing, but maybe splitting it into some new format existing tools\n> don't understand is compelling enough to justify the downstream breakage.\n\nTo me, this is a bigger concern.  We have changed conflict resolutions\nin various ways at various times over the years, so I don't think the\noutput should be considered fixed in stone, but I am very sympathetic\nto arguments that this particular change is too painful.\n\nThere is a possible way to allow for a transition period, though.\nJunio has already requested that some of the changes I'm working on be\ndone as a new merge strategy that is a rewrite of merge-recursive\n(https://public-inbox.org/git/xmqqk1ydkbx0.fsf@gitster.mtv.corp.google.com/).\nIt would be a little unfortunate to not be able to use the new merge\nstrategy as an exact drop-in replacement of the current recursive\nmerge strategy, but it would provide a way for people to get some time\nto migrate other tools.\n\nHowever, I'm far more concerned with the three collision conflict\ntypes having consistent behavior than I am with changing add/add\nconflict handling.  And if your two concerns or Jonathan's concern\nwould prevent you from wanting to use the new merge strategy (and\npossibly prevent it from becoming the default in the future), I'd much\nrather just modify rename/add and rename/rename to behave like\nadd/add.  Would that be more to your liking?\n\n\nElijah\n"},{"id":"341523","messageId":"CAPc5daVu8vv9RdGON8JiXEO3ycDVqQ38ySzZc-cpo+AQcAKXjA@mail.gmail.com","threadId":"48028","inReplyTo":"CABPp-BEdh+UOCpFn5Y1_RydR==dDHWTeNtBub+pPjH_06Ub28w@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-13T05:30:49Z","receivedAt":"2018-03-13T05:31:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"While I do not think it is a bad idea to add an optional way to write the\ncontents of conflicted stages out to separate files, I do not think it is a\ngood idea to change what happens to add/add conflict by default.\n\nTwo integrators picking up a same patch that adds a file separately\nand allowing them to diverge before they are merged should not be\nall that surprising, just like two integrators picking up a same patch\nthat changes an existing path and letting them evolve independently,\nso from that point of view, I do not think there is fundamental difference\nbetween edit/edit vs add/add and rename/rename conflict. The latter\ncertainly would be much rarer, but that is because edit happens a lot\nmore often than add.\n\nThere certainly are cases where conflicts is easier to resolve when\nthe merged tip versions are unrelated or diverged too vastly, with or\nwithout a common merge base. As I already agreed to, it would be\nuseful in such a case to have an option to write the conflicted stages\nto separate files to let an external merge tool to examine them and\nhelp you resolve them. But that is not limited to resolving vast\ndifferece between contents involved in an add/add conflict, is it?\nThe same tool (which can be driven as a mergetool backend) would\nbe useful in reconciling the difference between contents involved in an\nedit/edit conflict, no?\n\nIf anything, if rename/rename behaves differently by always writing\nthe result out to separate files, it is that codepath whose behaviour\nshould be fixed to match how add/add conflicts are dealt with.\n"},{"id":"341528","messageId":"871sgoqos3.fsf@evledraar.gmail.com","threadId":"48028","inReplyTo":"CABPp-BHDOimDoLxWxS=BDOBkm6CUTrXTzD16=TSkWGN-HOiU2g@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-03-13T09:59:40Z","receivedAt":"2018-03-13T09:59:48Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 13 2018, Elijah Newren jotted:\n\n> Hi,\n>\n> On Mon, Mar 12, 2018 at 3:19 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> wrote:\n>\n>>\n>> Does this mean that e.g. in this case of merging two files, one\n>> containing \"foo\" and one containing \"bar\":\n>>\n>>     (\n>>         rm -rf /tmp/test.git &&\n>>         git init /tmp/test.git &&\n>>         cd /tmp/test.git &&\n>>         echo foo >README &&\n>>         git add README &&\n>>         git commit -mfoo &&\n>>         git checkout --orphan trunk &&\n>>         git reset --hard &&\n>>         echo bar >README &&\n>>         git add README &&\n>>         git commit -mbar &&\n>>         git merge --allow-unrelated-histories master;\n>>         cat README\n>>     )\n>>\n>> That instead of getting:\n>>\n>>     <<<<<<< HEAD\n>>     bar\n>>     =======\n>>     foo\n>>     >>>>>>> master\n>>\n>> I'd now get these split into different files?\n>>\n>\n> As currently implemented, yes.  However, I was more concerned the idea of\n> handling files differently based on whether or not they were similar,\n> rather than on what the precise definition of \"similar\" is for this context.\n>\n> As far as the definition of similarity goes, estimate_similarity() is\n> currently used by rename detection to compare files recorded at different\n> pathnames.  By contrast, in this context, we are comparing two files which\n> were recorded with the same pathname.  That suggests the heuristic could be\n> a bit different and use more than just estimate_similarity().  (e.g. \"We\n> consider these files similar IF >50% of the lines match OR both files are\n> less than 2K.\")\n\nYeah that's fair enough, we could definitely tweak a dedicated heuristic\nfor just this purpose to do the right thing most of the time, although\nreally having completely unrelated 10-line README files use the old\ntwo-way merge would, I guess, also be confusing to users who want this\nfeature.\n\n>> I don't mind this being a configurable option if you want it, but I\n>> don't think it should be on by default, reasons:\n>>\n>\n> I don't think a configurable option makes sense, at least not for my\n> purposes.  Having rename/rename conflicts be \"safely\" mis-detected as\n> rename/add or add/add, and having rename/add conflicts be \"safely\"\n> mis-detected as add/add is my overriding concern.  Thus, making these three\n> conflict types behave consistently is what I need.  Options would make that\n> more difficult for me, and would thus feel like a step backwards.\n\nI think there's three types of users who interact with the current\nformat:\n\n 1) The user using no special tool (e.g. editing with nano) and runs\n    into a conflict, maybe this helps these users the most.\n\n 2) The user who's also using no special tool (e.g. editing with nano)\n    but just prefers to get in-file conflict markers.\n\n 3) The user using a tool to solve the conflict, who doesn't care if\n    we'd spew out our current format, this new formar, or some XML file\n    with both versions or whatevery, they're using some other UI.\n\nI don't think we should make some change that breaks existing tools for\nusers of #3 by default.\n\n>  1) There's lots of cases where we totally screw up the \"is this\n>>     similar?\" check, in particular with small files.\n>>\n>>     E.g. let's say you have a config file like 'fs-path \"/tmp/git\"' and\n>>     in two branches you change that to 'fs-path \"/opt/git\"' and 'fs-path\n>>     \"/var/git\"'. The rename detection will think this these have nothing\n>>     to do with each other since they share no common lines, but to a\n>>     human reader they're really similar, and would make sense in the\n>>     context of resolving a bigger merge where /{opt,var}/git changes are\n>>     conflicting.\n>>\n>>     This is not some theoretical concern, there's lots of things that\n>>     e.g. use small 5-10 line config files to configure some app that\n>>     because of some combo of indentation changes and changing a couple\n>>     of lines will make git's rename detection totally give up, but to a\n>>     human reader they're 95% the same.\n>>\n>\n> Fair enough.  The small files case could potentially be handled by just\n> changing the similarity metric for these conflict types, as noted above.\n> If it's a small file, that might be the easiest way for a user to deal with\n> it too.\n\nOr it might be easier for the user to deal with even big conflicts with\nin-file conflict markers, see use-case #2 & #3 above.\n\n> I'm not sure I see the problem with the bigger files, though.  If you have\n> bigger files with less than 50% of the lines matching, then you'll\n> essentially end up with a great big conflict block with one file on one\n> side and the other file on the other side, which doesn't seem that\n> different to me than having them be in two separate files.  In fact,\n> separate files seems easier to deal with because then the user can run e.g.\n> 'git diff --no-index --color-words FILE1 FILE2', something that they can't\n> do when it's in one file.  That has bothered me more than once, and made me\n> wish they were just in separate files.\n\nRight, but this assumes they user isn't using a merge-tool as noted in\n#3.\n\n>>  2) This will play havoc with already established merge tools on top of\n>>     git which a lot of users use instead of manually resolving these in\n>>     vi or whatever.\n>>\n>>     If we made this the default they'd need to to deal with this new\n>>     state, and even if it's not the default we'll have some confused\n>>     users wondering why Emacs Ediff or whatever isn't showing the right\n>>     thing because it isn't supporting this yet.\n>>\n>> So actually, given that last point in #2 I'm slightly negative on the\n>> whole thing, but maybe splitting it into some new format existing tools\n>> don't understand is compelling enough to justify the downstream breakage.\n>>\n>\n> To me, this is a bigger concern.  We have changed conflict resolutions in\n> various ways at various times over the years, so I don't think the output\n> should be considered fixed in stone, but I am very sympathetic to arguments\n> that this particular change is too painful.\n\nI think we've changed around a lot of the details, and certainly how the\nconflict is resolved. By \"format\" I mean that we've been emitting a file\nwith conflict markers, which third-party tools know how to parse\n(e.g. ediff). That's a relatively stable format that we (with minor\nchanges) also share with cvs, svn, or plain patch(1) etc.\n\n> [...]\n> However, I'm far more concerned with the three collision conflict types\n> having consistent behavior than I am with changing add/add conflict\n> handling.  And if your two concerns or Jonathan's concern would prevent you\n> from wanting to use the new merge strategy (and possibly prevent it from\n> becoming the default in the future), I'd much rather just modify rename/add\n> and rename/rename to behave like add/add.  Would that be more to your\n> liking?\n\nI don't really object to these changes, I don't know enough about this\narea, I skimmed your patches and 90% of it is over my head (although I\ncould keep digging...).\n\nI'm just chiming in because it seems to me from upthread that you're\npurely focusing on the use-case of the user of git who's using git at\nthe abstraction of using a dumb editor that doesn't do anything to help\nthem with conflict markers to resolve conflicts.\n\nI think that in those cases if you have two unrelated 3k line files that\ncollide splitting them out is certainly more helpful.\n\nBut there's also the use-case of existing tools (e.g. ediff) essentially\nusing this as a plumbing format, and we have to keep those tools in\nmind, because to them it really doesn't matter what the format is, but\nit *does* matter that they can understand it, but if we took these\npatches as-is e.g. ediff wouldn't even show these conflicting files,\ncorrect?\n\nAlso another caveat: Since these are new side-files what happens when\nyou're in a conflict and run `git clean -dxf`, are these new files wiped\naway, or are they somehow known to the index?\n"},{"id":"341554","messageId":"CABPp-BEOi6GAoHUZyfcJdd-eLwPLoyRMOiS-J1dvkjqm7VGj9Q@mail.gmail.com","threadId":"48028","inReplyTo":"871sgoqos3.fsf@evledraar.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-13T17:09:01Z","receivedAt":"2018-03-13T17:09:09Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 13, 2018 at 2:59 AM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> On Tue, Mar 13 2018, Elijah Newren jotted:\n\n>> However, I'm far more concerned with the three collision conflict types\n>> having consistent behavior than I am with changing add/add conflict\n>> handling.  And if your two concerns or Jonathan's concern would prevent you\n>> from wanting to use the new merge strategy (and possibly prevent it from\n>> becoming the default in the future), I'd much rather just modify rename/add\n>> and rename/rename to behave like add/add.  Would that be more to your\n>> liking?\n>\n> I don't really object to these changes, I don't know enough about this\n> area, I skimmed your patches and 90% of it is over my head (although I\n> could keep digging...).\n>\n> I'm just chiming in because it seems to me from upthread that you're\n> purely focusing on the use-case of the user of git who's using git at\n> the abstraction of using a dumb editor that doesn't do anything to help\n> them with conflict markers to resolve conflicts.\n\nYeah, that's totally why I started this separate thread; I was worried\nthe original would push away folks who weren't familiar enough with\nmerge-recursive or were just overwhelmed by all the different changes\nand rationale, but I really wanted to get feedback on at least the\npiece that people were likely to hit in practice and would understand.\nThanks for doing so; your, and Jonathan's, and Junio's comments have\nprovided some good context.\n\n> Also another caveat: Since these are new side-files what happens when\n> you're in a conflict and run `git clean -dxf`, are these new files wiped\n> away, or are they somehow known to the index?\n\nA git clean would wipe them out...and if that scares you (and I can\ntotally see how it might), then rest assured that the situation is a\nwhole lot worse: this isn't limited to rename/add and\nrename/rename(2to1) conflicts.  There are several paths in\nmerge-recursive.c that call unique_path() to get a temporary filename\nthat is in no way tracked in the index but which is used for storing\ncontent relevant to the merge.  These include directory/file\nconflicts, untracked files being in the way of putting a renamed file\nwhere it belongs, untracked files being in the way of a modify/delete,\nuntracked files being in the way of simple adds on the other side of\nhistory, rename/rename(1to2)/add/add, and maybe others.  In all cases,\na git clean is going to wipe out the files that were written to\ndifferent temporary locations.\n\nMy rewrite I'm trying to find time to work on would get rid of the\ncode paths involving untracked files being in the way of other stuff,\nbut would do nothing for the other cases.  I would love to get rid of\nall of them, but don't have a clue how to do so.\n"},{"id":"341555","messageId":"CABPp-BHDVqq4atmCw=WX4KkV1C-kGh0MubowevJBw6n=qF56nA@mail.gmail.com","threadId":"48028","inReplyTo":"CABPp-BE653uMpwN4zfCCP8teRGmZ6v5NEyASCR1PTvHhoMKE1w@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-13T17:22:42Z","receivedAt":"2018-03-13T17:23:01Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Mar 12, 2018 at 5:38 PM, Elijah Newren <newren@gmail.com> wrote:\n> I feel the analogy to merging binary files breaks down here in more\n> than one way:\n\nActually, after mulling this over, I'm going to retract the \"more\nthan\" piece of this sentence.  I'm trying to figure out how to retract\nmore, but have only figured out one piece.  In particular...\n\n> 1)\n>\n> Merging binary files is more complex than you suggest here.  In\n> particular, when creating a virtual merge base, the resolution chosen\n> is not the version of the file from HEAD, but the version of the file\n> from the merge base.  Nasty problems would result otherwise for the\n> recursive case.\n>\n> If we tried to match how merging binary files behaved, we run into the\n> problem that the colliding file conflict case has no common version of\n> the file from a merge base.  So the same strategy just doesn't apply.\n> The closest would be outright deleting both versions of the file for\n> the virtual merge base and punting to the outer merge to deal with it.\n> That might be okay, but seems really odd to me.  I feel like it'd\n> unnecessarily increase the number of conflicts users will see, though\n> maybe it wouldn't be horrible if this was only done when the files\n> were considered dissimilar anyway.\n\nThinking about this more, it really isn't that weird at all.  The code\nis already set up to use null_oid as the \"original\" version when\ncreating a virtual merge base, going back to the content from a\nrecursive merge base is a strategy used in multiple places in\nrecursive cases, and null is precisely the version from the recursive\nmerge base.  If two added files differ wildly, I don't think using\nnull would increase the number of conflicts appreciably, if at all.\nSo, the analogy to merging binary files holds just fine from this\nangle.\n\nSo, if we could figure out how to handle the higher numbers of paths\nfor e.g. the rename/rename cases, then perhaps this is a strategy that\ncould work.\n\n>> Interesting.  I would be tempted to resolve this inconsistency the\n>> other way: by doing a half-hearted two-way merge (e.g. by picking one\n>> of the two versions of the colliding file) and marking the path as\n>> conflicted in the index.  That way it's more similar to edit/edit,\n>> too.\n>\n> Your proposal is underspecified; there are more complicated cases\n> where your wording could have different meanings.\n>\n<snip>\n> My question for your counter-proposal:\n> Do you record C1 or C2 as C?  Or do you record the version of C from\n> HEAD as C?  And what do you pick when creating a virtual merge base?\n>\n> Problems I see regardless of the choice in your counter-proposal:\n>   * If you choose C from HEAD, you are ignoring 3 other versions of\n> \"C\" (as opposed to just one other version when merging a binary file);\n> will the user recognize that and know how to look at all four\n> versions?\n>   * If you choose C1 or C2, the user will see conflict markers; will\n> the user recognize that this is only a _subset_ of the conflicts, and\n> that there's a lot of content that was completely ignored?\n\n>   * There is no \"merge base of C1 and C2\" to use for the virtual merge\n> base.  What will you use?\n\nFor this last question, the answer is \"null\", and it works just fine.\nI don't yet have good answers to the other questions, though.  If\nsomeone else does, I'd love to hear it.\n\n\nOh, and by the way Jonathan, thanks very much for your feedback and\nalternative ideas for consideration.  You gave me more angles to try\nto think about this problem from.\n"},{"id":"341561","messageId":"CABPp-BF-LnGPx6vKzCNaFNi_fkxK-CpenatOVa2fvJRZHd0hZw@mail.gmail.com","threadId":"48028","inReplyTo":"CAPc5daVu8vv9RdGON8JiXEO3ycDVqQ38ySzZc-cpo+AQcAKXjA@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-13T18:21:37Z","receivedAt":"2018-03-13T18:21:48Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Mar 12, 2018 at 10:30 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> While I do not think it is a bad idea to add an optional way to write the\n> contents of conflicted stages out to separate files, I do not think it is a\n> good idea to change what happens to add/add conflict by default.\n\n<snip some other good arguments>\n\n> If anything, if rename/rename behaves differently by always writing\n> the result out to separate files, it is that codepath whose behaviour\n> should be fixed to match how add/add conflicts are dealt with.\n\nCool, thanks for the context.  I'm happy to go down this path, but\nthere is one question I'd like your opinion on: what if the\nintermediate content merges have conflicts themselves?  If that\nquestion isn't clear, let me be more precise...\n\nLet's say A is renamed to C in HEAD, and both sides modified.  We thus\nneed to do a 3-way content merge of C from HEAD with the two A's.\nLet's call the result C1.  Further, let's say that B is renamed to C\nin $MERGE_BRANCH, and both sides modified.  Again we need a 3-way\ncontent merge for that other C; let's call the result C2.  If neither\nC1 nor C2 have content conflicts, then making rename/rename(2to1) act\nlike add/add is easy: just do a two-way merge of C1 and C2.  But what\nif C1 or C2 have conflicts?  Doing a two-way merge in that case could\nresult in nested conflict markers.  Is that okay, or would we want to\ncontinue the old behavior of writing C1 and C2 out to separate files\nfor that special case only?\n\nI'm inclined to special case content conflicts in the intermediate\nmerges and handle them by writing out to separate files, but I was the\none who suggested sometimes using separate files for add/add\nconflicts; I'm curious if perhaps my strong aversion to nested\nconflicts is also off base.\n"},{"id":"341597","messageId":"xmqqk1uf1v7l.fsf@gitster-ct.c.googlers.com","threadId":"48028","inReplyTo":"CABPp-BFsiWBiDYYvz=cOofECUHUMJs8x8RuMXqeCF1qP5HzGoQ@mail.gmail.com","subject":"Re: Fwd: Opinions on changing add/add conflict resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-13T22:12:14Z","receivedAt":"2018-03-13T22:12:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> As currently implemented, yes.  However, I was more concerned the idea\n> of handling files differently based on whether or not they were\n> similar, rather than on what the precise definition of \"similar\" is\n> for this context.\n>\n> As far as the definition of similarity goes, estimate_similarity() is\n> currently used by rename detection to compare files recorded at\n> different pathnames.  By contrast, in this context, we are comparing\n> two files which were recorded with the same pathname.  That suggests\n> the heuristic could be a bit different and use more than just\n> estimate_similarity().  (e.g. \"We consider these files similar IF more\n> than 50% of the lines match OR both files are less than 2K.\")\n\nYeah, I think there is similar difference between similarity score\nthat is used by diffcore-rename and dissimilarity score that is used\nby diffcore-break exactly for that reason.  If you start from a\n100-line original file and grow it to 100-line one by adding 900\nlines, as long as you kept the original 100 lines, it is easier to\nview the change within the same path as a continued development,\ninstead of saying they are so dissimilar.\n\nIn any case, I think the way the stage #2 and stage #3 (i.e. ours\nand theirs) contents are externalized during a conflicted mergy\noperation should be consistent across edit/edit and add/add\nconflict, so if we are adding a new way to write out extra temporary\nfiles out of these higher stage index entries, it should be made\napplicable not only to add/add conflict (i.e. there shounld't be a\ncode that says \"oh, let's see, this lacks stage #1, so do this\ndifferent thing\").\n\nPersonally, I think it is best to leave it all outside of the core\nand make \"git mergetool\" to be responsible for the job of\nexternalizing higher stage index entries to temporary working tree\nfiles.  They already need to do so in order to work with external\ntools that do not read directly from our index file anyway, no?\n\n"},{"id":"341599","messageId":"xmqqbmfr1uj3.fsf@gitster-ct.c.googlers.com","threadId":"48028","inReplyTo":"CABPp-BF-LnGPx6vKzCNaFNi_fkxK-CpenatOVa2fvJRZHd0hZw@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-13T22:26:56Z","receivedAt":"2018-03-13T22:27:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> Cool, thanks for the context.  I'm happy to go down this path, but\n> there is one question I'd like your opinion on: what if the\n> intermediate content merges have conflicts themselves?  If that\n> question isn't clear, let me be more precise...\n\nI think you answered this yourself after (re)discovering the virtual\nancestor merge in the recursive strategy, and no longer need my\ninput here ;-)\n\n"},{"id":"341600","messageId":"CABPp-BGXdBJSi5EUyeom0PcgRXvwNy4EBA71MP3FZK9xsw-jdA@mail.gmail.com","threadId":"48028","inReplyTo":"xmqqbmfr1uj3.fsf@gitster-ct.c.googlers.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-13T22:42:54Z","receivedAt":"2018-03-13T22:43:02Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 13, 2018 at 3:26 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Elijah Newren <newren@gmail.com> writes:\n>\n>> Cool, thanks for the context.  I'm happy to go down this path, but\n>> there is one question I'd like your opinion on: what if the\n>> intermediate content merges have conflicts themselves?  If that\n>> question isn't clear, let me be more precise...\n>\n> I think you answered this yourself after (re)discovering the virtual\n> ancestor merge in the recursive strategy, and no longer need my\n> input here ;-)\n\nThe question about what to put into the index was another issue, and\nit's good to hear that you seem to approve of my logic on that one.\nThanks.  :-)\n\nHowever, my question here about what to write to the working tree for\na rename/rename(2to1) conflict in one particular corner case still\nremains.  Should a two-way merge be performed even if it may result in\nnested sets of conflict markers, or is that a sufficiently bad outcome\nfor the user that it's the one case we do want to write colliding\nfiles out to different temporary paths?\n"},{"id":"341601","messageId":"xmqq7eqf1tc9.fsf@gitster-ct.c.googlers.com","threadId":"48028","inReplyTo":"CABPp-BGXdBJSi5EUyeom0PcgRXvwNy4EBA71MP3FZK9xsw-jdA@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-13T22:52:38Z","receivedAt":"2018-03-13T22:52:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> However, my question here about what to write to the working tree for\n> a rename/rename(2to1) conflict in one particular corner case still\n> remains.\n\nHmph, is it a bad idea to model this after what recursive merge\nstrategy does?  I think what is written out from that codepath to\nthe working tree has the nested conflict markers (with a bit of\ntweak to the marker length, IIRC) in it.\n\n\n"},{"id":"341602","messageId":"20180313225639.GC147135@aiede.svl.corp.google.com","threadId":"48028","inReplyTo":"CABPp-BGXdBJSi5EUyeom0PcgRXvwNy4EBA71MP3FZK9xsw-jdA@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-03-13T22:56:39Z","receivedAt":"2018-03-13T22:56:46Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nElijah Newren wrote:\n\n> However, my question here about what to write to the working tree for\n> a rename/rename(2to1) conflict in one particular corner case still\n> remains.  Should a two-way merge be performed even if it may result in\n> nested sets of conflict markers, or is that a sufficiently bad outcome\n> for the user that it's the one case we do want to write colliding\n> files out to different temporary paths?\n\nNested conflict markers only happen in the conflictstyle=diff3 case, I\nwould think.\n\nmerge-recursive writes them already.  I've often wished that it would\nuse a union merge strategy when building the common ancestor to avoid\nthe nested conflicts that rerere doesn't understand.  But anyway,\nthat's an orthogonal issue: in the rename/rename context, it should be\nfine to write nested conflict markers since that's consistent with\nwhat merge-recursive already does.\n\nThanks,\nJonathan\n"},{"id":"341603","messageId":"CABPp-BEBvywrCAeLAe-LQ9XOJ=sKG0-vYJRrXYoHZebUM-kd7A@mail.gmail.com","threadId":"48028","inReplyTo":"xmqq7eqf1tc9.fsf@gitster-ct.c.googlers.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-13T23:04:09Z","receivedAt":"2018-03-13T23:04:15Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 13, 2018 at 3:52 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Elijah Newren <newren@gmail.com> writes:\n>\n>> However, my question here about what to write to the working tree for\n>> a rename/rename(2to1) conflict in one particular corner case still\n>> remains.\n>\n> Hmph, is it a bad idea to model this after what recursive merge\n> strategy does?  I think what is written out from that codepath to\n> the working tree has the nested conflict markers (with a bit of\n> tweak to the marker length, IIRC) in it.\n\nOh, that's cool; I didn't know that.  It looks like that was\nintroduced in commit d694a17986 (\"ll-merge: use a longer conflict\nmarker for internal merge\", 2016-04-14).  That seems like a good idea;\nI'll go with that.  Thanks.\n"},{"id":"341605","messageId":"CABPp-BE=D_TgjLZkhRh6-7k3UpnDDMfZB6vE=FMdwFT+mmp_tw@mail.gmail.com","threadId":"48028","inReplyTo":"20180313225639.GC147135@aiede.svl.corp.google.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-13T23:14:03Z","receivedAt":"2018-03-13T23:14:09Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 13, 2018 at 3:56 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Elijah Newren wrote:\n>\n>> However, my question here about what to write to the working tree for\n>> a rename/rename(2to1) conflict in one particular corner case still\n>> remains.  Should a two-way merge be performed even if it may result in\n>> nested sets of conflict markers, or is that a sufficiently bad outcome\n>> for the user that it's the one case we do want to write colliding\n>> files out to different temporary paths?\n>\n> Nested conflict markers only happen in the conflictstyle=diff3 case, I\n> would think.\n\nCurrently, yes.  To be clear, though, this change would make it\npossible even when there is no recursive merge being done and when\nconflictstyle=merge.\n\n> merge-recursive writes them already.  I've often wished that it would\n> use a union merge strategy when building the common ancestor to avoid\n> the nested conflicts that rerere doesn't understand.  But anyway,\n> that's an orthogonal issue: in the rename/rename context, it should be\n> fine to write nested conflict markers since that's consistent with\n> what merge-recursive already does.\n\nCool, sounds like we're now all on the same page.\n\nSomeone in the future might hate us if they use conflictstyle=diff3,\nand have a recursive merge, and have a rename/rename(2to1) conflict in\nthe virtual merge base with nested conflicts, and that resulting file\nis also involved in a separate rename/rename(2to1) conflict in the\nouter merge that has its own nested conflicts; we'd have up to four\nlevels of nested conflicts.  But for now, I'm going to write that off\nas a crazy thought (or dismiss it as nigh impossible to reason about\nregardless of what we did to help them) and just proceed ahead.  :-)\n"},{"id":"341606","messageId":"CAPc5daUC+6cHpexXTO24p4mG_5eL1JmxrYm8h3UfdTh_FMka=w@mail.gmail.com","threadId":"48028","inReplyTo":"CABPp-BE=D_TgjLZkhRh6-7k3UpnDDMfZB6vE=FMdwFT+mmp_tw@mail.gmail.com","subject":"Re: Opinions on changing add/add conflict resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-13T23:30:15Z","receivedAt":"2018-03-13T23:30:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Tue, Mar 13, 2018 at 4:14 PM, Elijah Newren <newren@gmail.com> wrote:\n>\n> Someone in the future might hate us if they use conflictstyle=diff3,\n> and have a recursive merge,\n\nFWIW I already always use diff3 style and see the nested markers,\nand I already hate it, so you are no worse off ;-)\n"}]}