Hi Sergey,
On 15/03/2018 08:52, Sergey Organov wrote:
>
> > > 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).
>
> As we now have a simple way to actually check it even in this algorithm,
> I'd suggest command-line option to either relax or enforce the check,
> whatever the default is. For the default, I'd still opt for safety, as
> without it we will gather little experience with this new matter.
>
> Honestly, without this check available, I'd likely vote for at least an
> option for stopping on every rebased merge, on the ground that if
> rebasing a non-merge could be a trouble, rebasing a merge is at least
> double-trouble, and it's not that frequent anyway. So the check we
> discuss is actually a way to make all the process much less paranoid,
> not more.
>
> By the way, nobody yet commented about "rerere" behavior that basically
> stops rebasing every time it fires. Do you consider it over-paranoid?
I wouldn`t really know, my workflows are usually/still rather simple, I
don`t think I`ve ever used it on purpose, and I don`t really remember
I`ve triggered it by accident, not having it stop for amendment, at least.
You did say you find it annoying yourself, though ;) But also accepting
it as something that probably has a good reason, though (thus not
considering it over-paranoid, even if annoying).
> As for test cases, I have none myself, but "-s ours" merge may be an
> example of an actual trouble.
>
> If we don't treat it specially, then changes to side branch will be
> silently propagated over the merge, that's obviously not what is needed,
> provided user keeps his intention to leave the merge "-s ours".
>
> If we do treat it specially, it could be the case that the merge in
> question only looks like "-s ours" by pure accident, and thus changes to
> the side branch should be propagated.
>
> I don't see how we can safely proceed without stop for user assistance.
> Had we already achieved some consensus on this issue?
I don`t know, from what Johannes said in the past, I got an
impression that this is to be expected ("by design"), and not worth
bothering to stop for. And he is one of the heaviest users of (merge)
rebasing I know.
Personally, I still feel it would make sense to stop in case like
this, indeed, but it`s just my humble (and not necessarily much
educated) opinion.
> > > 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" ] || `
>
> No. For this check it's essential to ensure that no tweaking of the
> content has been performed under the hood after the user has resolved
> conflicts, i.e., after he has been involved last time.
>
> If all this is done in one "huge merge" step from user point of view,
> then the check belongs to this merge, as this is the last (and the only)
> one. If it's done in steps (and I vote for it), only the last merge
> status is essential for the check, preceding merges don't matter.
"Huge merge" step (from user point of view) is exactly how I perceived
Johannes` opinion on it, describing it`s already part of Git user
experience (with possible nested conflicts), while otherwise possibly
hard to explain where we are precisely at in the moment of stopping for
(intermediate) conflict resolution.
Thus only `$conflicts`, meaning anything in the "huge merge", as no user
action/tweaking/involvement can happen until the "huge merge" is done.
> As I said, putting myself on the user side, I'd prefer entirely separate
> first step of the algorithm, exactly as written, with its own conflict
> resolution, all running entirely the same way as it does with non-merge
> commits. I'm used to it and don't want to learn something new without
> necessity. I.e., I'd prefer to actually see it in two separate stages,
> like this:
>
> Rebasing mainline of the merge...
> [.. possible conflicts resolution ..]
> Merging in changes to side branch(es)...
> [.. possible conflicts resolution ..]
>
> And if the second stage gives non-trivial conflicts, I'd like to have a
> simple way to just do "merge -s ours <heads>" on top of already rebased
> mainline of the merge and go with it. Note that the latter is
> significantly different than re-merging everything from scratch, that
> would be the only choice with "all-in-one" approach, and it essentially
> gives me back those simple "rebase first parent and just record other
> parents" semantics when needed.
I`m undecided here, and while I do see a point in what you`re saying,
this being new to general public I dont`t think you being accustomed
to it is a very strong argument :)
Yes, having more steps would mean more power/options to the user, but
more complexity to explain to and guide him through as well, not really
sure where the line should be drawn - for the first time, at least.
Also note that, for example, in case side branch(es) dropped some
commits (interactively or otherwise), first step alone would still
reintroduce those dropped changes, thus later possible `merge -s ours
<heads>` would be a pretty bad "evil merge" case and a wrong thing to
do in general.
Regards, Buga
