Hi Sergey, On 14/03/2018 08:21, Sergey Organov wrote: > > There are still 2 issues about the implementation that need to be > discussed though: > > 1. Still inverted order of the second merge compared to RFC. > > It'd be simple to "fix" again, except I'm not sure it'd be better, and > as there is no existing experiences with this step to follow, it > probably should be left as in the original, where it means "merge the > changes made in B' (w.r.t B) into our intermediate version of the > resulting merge". > > The original Phillip's version seems to better fit the asymmetry between > mainline and side-branch handling. > > The actual difference will be only in the order of ours vs theirs in > conflicts though, and thus it's not that critical. Shouldn`t this be easy to solve just by changing the order of <head> and <remote>, on passing to `git merge-recursive`, if needed? (or that`s what you meant by "simple to fix"?) > 2. The U1' == U2' consistency check in RFC that I still think is worth > to be implemented. At the moment, I think we`d appreciate test cases where it actually proves useful, as the general consensus seems to be leaning towards it possibly being annoying (over-paranoid). > In application to the method being discussed, we only need the check if > the final merge went without conflicts, so the user was not already > involved, and the check itself is then pretty simple: > > "proceed without stop only if $tree = $tree_U1'" > > Its equivalence to the U1' == U2' test in the RFC follows from the fact > that if M' is non-conflicting merge of U1' and U2', then M' == U1' if > and only if U2' == U1'. Nicely spot! I`m glad there`s still (kind of) former U1' == U2' check in this approach, too, in case it proves useful :) > Finally, here is a sketch of the implementation that I'd suggest to > use: > > git-rebase-first-parent --onto A' M > tree_U1'=$(git write-tree) > git merge-recursive B -- $tree_U1' B' > tree=$(git write-tree) > M'=$(git log --pretty=%B -1 M | git commit-tree -pA' -pB') > [ $conflicted_last_merge = "yes" ] || > trees-match $tree_U1' $tree || > stop-for-user-amendment Yes, in case where we would want the "no-op merge" check (equivalent to U1' == U2' with original approach), this aligns with something I would expect. Note that all the "rebase merge commit" steps leading to the check will/should probably be observed as a single one from user`s perspective (in worst case ending with nested conflicts we discussed), thus `$conflicted_last_merge` is not related to `merge-recursive` step(s) only, but `rebase-first-parent`, too (just in case this isn`t implied). Might be easier to reason about simply as `[ $conflicts = "yes" ] || ` > where 'git-rebase-first-parent' denotes whatever machinery is currently > being used to rebase simple non-merge commit. Handy approximation of > which for stand-alone scripting is: > > git checkout --detach A' && git cherry-pick -m 1 M > > [As an interesting note, observe how, after all, that original Johannes > Sixt's idea of rebasing of merge commit by cherry-picking its first > parent is back there.] Heh ;) It`s always a bit enlightening when people start from different positions, opposing ones, even (or so it may seem, at least), but eventually end up in the same place, through means of open (minded) discussion. Regards, Buga
