Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

[Alireza]

Hi,

Even though the merge commit's message includes conflicted files by
default, the *resolution* itself is lost, that is, it's hard or
impossible to review how the author *resolved* said conflicts.

The proposal is that an option like `-X clean` would commit a clean
merge and leave out any conflicting hunks in the tree for a follow-up
commit to resolve conflicts.

That would be extremely helpful for a code reviewer to see how a
possibly external contributor has dealt with upstream changes e.g. in
a long-standing branch.

Any comment would be appreciated.

Thanks,
Alireza

Re: Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

[Michal Suchánek]

On Thu, Jul 16, 2020 at 09:45:24PM +0430, Alireza wrote:
> Hi,
> 
> Even though the merge commit's message includes conflicted files by
> default, the *resolution* itself is lost, that is, it's hard or
> impossible to review how the author *resolved* said conflicts.
No, the merge commit includes the resolution. You can compare with all
parents, and even perform the same merge locally using your strategy and
tooling of choice and compare the result with what the merge author
committed as resolution.

> 
> The proposal is that an option like `-X clean` would commit a clean
> merge and leave out any conflicting hunks in the tree for a follow-up
> commit to resolve conflicts.
I don't see any value above performing the same merge locally and
comparing with the committed resolution. Can you please elaborate on the
expected format of the merge that does add some value?

Thanks

Michal

Re: Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

[Junio C Hamano]

Alireza <rezaxm@gmail.com> writes:

> The proposal is that an option like `-X clean` would commit a clean
> merge and leave out any conflicting hunks in the tree for a follow-up
> commit to resolve conflicts.

You need to clarify what "leave out" means to you.  If you and a
side branch started from the same place and you did one thing while
the side branch did something else, you would get a conflict.

What would the "clean" (by your definition) result have in that
block of contents that actually has a conflict?  Do you mean to say
"Pick our version and ignore theirs in the blocks where the changes
conflict"?  If so perhaps -Xours merge strategy option that the
recursive backend offers is what you are looking for?

Re: Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

[Alireza]

Say I'm merging from upstream with 100 changed files, but I only get
two conflicts.
If I manually resolve those, the changes I made in the process is
actually lost in a large merge diff.
What I'm trying to do is to separate those manual changes from
anything else that could merge without conflict.


> What would the "clean" (by your definition) result have in that
> block of contents that actually has a conflict?  Do you mean to say
> "Pick our version and ignore theirs in the blocks where the changes
> conflict"?  If so perhaps -Xours merge strategy option that the
> recursive backend offers is what you are looking for?

That's actually what I first tried. But when I use -Xours, I can't run
`git merge <previous>` again
to reproduce those conflicting hunks - because the resulting commit is
deemed to be in sync with both parents.
As a result, all the upstream changes are now overridden in this side branch.

Re: Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

[Elijah Newren]

On Thu, Jul 16, 2020 at 10:17 AM Alireza <rezaxm@gmail.com> wrote:
>
> Hi,
>
> Even though the merge commit's message includes conflicted files by
> default, the *resolution* itself is lost, that is, it's hard or
> impossible to review how the author *resolved* said conflicts.
>
> The proposal is that an option like `-X clean` would commit a clean
> merge and leave out any conflicting hunks in the tree for a follow-up
> commit to resolve conflicts.
>
> That would be extremely helpful for a code reviewer to see how a
> possibly external contributor has dealt with upstream changes e.g. in
> a long-standing branch.
>
> Any comment would be appreciated.

I disagree that they are "lost".  Rather, git doesn't make them very
easy to access: git log -p won't show you any output for a merge by
default, and the only options that exist (-c, -cc) don't do what you
need to see how conflicts were resolved.  Thus, the only way to get
them would be to check out the first parent of the merge, do a merge
with the second parent, then do a "git diff -R $merge_commit".  That's
doable, it's just annoying.

If there were an option to allow git log for a merge to show the
difference between what an automatic merge would do (complete with
conflicts) and the end-state that was committed, then the resolution
would become very accessible and the rest of your request would be
moot.  See https://bugs.chromium.org/p/git/issues/detail?id=12.  I'm
getting closer to having such a thing.

Elijah

Re: Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

[Alireza]

> If there were an option to allow git log for a merge to show the
> difference between what an automatic merge would do (complete with
> conflicts) and the end-state that was committed, then the resolution
> would become very accessible and the rest of your request would be
> moot.

Happy to see there's already some interest to make this easier.
However, my main use case is not just to see those changes, ideally it
should be a separate commit to be easily reviewed in a PR, for
example, otherwise the reviewer must pull all changes locally, which
doesn't sound like an improvement, IMO.

Re: Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

[Elijah Newren]

On Tue, Jul 21, 2020 at 10:17 AM Alireza <rezaxm@gmail.com> wrote:
>
> > If there were an option to allow git log for a merge to show the
> > difference between what an automatic merge would do (complete with
> > conflicts) and the end-state that was committed, then the resolution
> > would become very accessible and the rest of your request would be
> > moot.
>
> Happy to see there's already some interest to make this easier.
> However, my main use case is not just to see those changes, ideally it
> should be a separate commit to be easily reviewed in a PR, for
> example, otherwise the reviewer must pull all changes locally, which
> doesn't sound like an improvement, IMO.

Well, perhaps here is where we diverge.  Providing the ability in core
git from the command line to make it easy to review manual user
changes within a merge commit would make it so that various PR-related
tools could add that capability as well.  In fact, gerrit already has
such a capability and has for years, so it's not even necessary for
git to have any changes to get this kind of ability.

In contrast, recording the automatic changes and the manual changes as
separate commits is a short term workaround for a lack of a feature,
but one that has some long term ramifications.  In my opinion this
workaround wrecks history and makes it ugly for the rest of forever.
Yes, I have a strong opinion here, but it's actually a bit worse than
that: if people like broken history, they can already record the merge
as two commits -- so why do we need to add a separate option to
facilitate breaking things?  I don't like the idea of breaking things
just because external tools don't yet have an ability we would like.
I think the external tools should be fixed, and perhaps we could
provide a base capability those tools can use to more easily implement
such a feature.

Just my (strongly worded) $0.02, of course.

Elijah

Re: Request for adding a "clean" merge strategy for a double-commit merge to deal with conflicts separately

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> I disagree that they are "lost".  Rather, git doesn't make them very
> easy to access: git log -p won't show you any output for a merge by
> default, and the only options that exist (-c, -cc) don't do what you
> need to see how conflicts were resolved.

With "sometimes" sprinkled into appropriate places, you are 110%
right.