Re: how do you review auto-resolved files

4 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: how do you review auto-resolved files

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:07

"Neal Kreitzinger" [off-list ref] writes:
When git does a merges (merge/rebase/cherry-pick) it auto-resolves same-file 
changes that do not conflict on the same line(s).

Technical Question:  What are the recommended commands for reviewing the 
files that auto-resolved after a "merge"?
Imagine that you are the maintainer of the mainline and are reviewing the
work made on a side branch that you just merged, but pretend that the
contribution came as a patch instead.  How would you assess the damage to
your mainline?

You would use "git show --first-parent $commit" for that.

And then look at what the sideline wanted to do to the old baseline:

	git log -p $commit^..$commit

which would, unless the person who worked on the side branch did a shoddy
job describing his work, explain what the side branch wanted to achieve
and also _how_ it wanted to achieve it.

And then re-read the first "git show" output with that knowledge, together
with the knowledge you have on your mainline codebase, and decide if the
solution used by the side branch is still valid.  If it makes sense, you
are done.  If the advance in your mainline since the side branch forked
invalidated some assumption the side branch made (e.g. a helper function
the side branch used has changed its meaning, a helper function the side
branch changed its meaning gained more callsite on the mainline, etc.),
you have a semantic conflict that you would need to address.

It is unclear what exactly you consider "auto-resolve" in your message, so
I'd refrain from commenting on the "Philosophical" part, at least for now.

Re: how do you review auto-resolved files

From: Neal Kreitzinger <hidden>
Date: 2016-06-15 22:53:07

On 2/21/2012 3:19 PM, Junio C Hamano wrote:
"Neal Kreitzinger"[off-list ref]  writes:
quoted
When git does a merges (merge/rebase/cherry-pick) it auto-resolves same-file
changes that do not conflict on the same line(s).

Technical Question:  What are the recommended commands for reviewing the
files that auto-resolved after a "merge"?
Imagine that you are the maintainer of the mainline and are reviewing the
work made on a side branch that you just merged, but pretend that the
contribution came as a patch instead.  How would you assess the damage to
your mainline?

You would use "git show --first-parent $commit" for that.

And then look at what the sideline wanted to do to the old baseline:

	git log -p $commit^..$commit

which would, unless the person who worked on the side branch did a shoddy
job describing his work, explain what the side branch wanted to achieve
and also _how_ it wanted to achieve it.

And then re-read the first "git show" output with that knowledge, together
with the knowledge you have on your mainline codebase, and decide if the
solution used by the side branch is still valid.  If it makes sense, you
are done.  If the advance in your mainline since the side branch forked
invalidated some assumption the side branch made (e.g. a helper function
the side branch used has changed its meaning, a helper function the side
branch changed its meaning gained more callsite on the mainline, etc.),
you have a semantic conflict that you would need to address.

It is unclear what exactly you consider "auto-resolve" in your message, so
I'd refrain from commenting on the "Philosophical" part, at least for now.
Context: (git-merge manpage definition of merge-conflict) "During a 
merge, the working tree files are updated to reflect the result of the 
merge... When both sides made changes to the same area, however, git 
cannot randomly pick one side over the other, and asks you to resolve it 
by leaving what both sides did to that area."

My definition for "auto-resolve": "During a merge, the working tree 
files are updated to reflect the result of the merge... When both sides 
made changes to different areas of the same file, git picks both sides 
automatically, and leaves its up to you to make sure you review those 
merge results for correctness after git has made the merge commit."

IOW, an "auto-resolve" specifically means that both sides (ours and 
theirs) made changes to file(a) since the common-ancestor version of 
fila(a), and git picked both sides without raising a merge-conflict. 
(The reason I came up with the term "auto-resolve" is because in the 
git-merge output the term "Auto-merging" can also indicate that only one 
side (theirs) changed file(a) since the common-ancestor and that git is 
just "fast-forwarding" theirs file(a) on top of common-ancestor file(a).)

v/r,
neal

Re: how do you review auto-resolved files

From: Jeff King <hidden>
Date: 2016-06-15 22:53:07

On Tue, Feb 21, 2012 at 05:22:09PM -0600, Neal Kreitzinger wrote:
My definition for "auto-resolve": "During a merge, the working tree
files are updated to reflect the result of the merge... When both
sides made changes to different areas of the same file, git picks
both sides automatically, and leaves its up to you to make sure you
review those merge results for correctness after git has made the
merge commit."
Once the merge commit is made, you can review these with:

  $ git show --raw

which will give you the list of paths that were touched on both sides,
and then you can examine them manually.

You can also use:

  $ git show -c

to get the combined diff, showing hunks that were changed on both sides
(but only in files that would have been listed above). Annoyingly, I
don't think there is a way to get the same multi-way diff information
before the commit is created (i.e., when you still have some conflicts
in the index and working tree left to resolve).

But even both of those are not sufficient to find merge errors. Even
though there is no textual conflict, there may be semantic conflicts
that cross file boundaries (e.g., function foo() changes in foo.c, but a
caller in bar.c is introduced on a side branch). There is no replacement
for actually looking at the full result (though for the lazy, compiling
and running the test suite can often catch the low-hanging fruit).

-Peff

Re: how do you review auto-resolved files

From: Zbigniew Jędrzejewski-Szmek <hidden>
Date: 2016-06-15 22:53:07

On 02/21/2012 10:19 PM, Junio C Hamano wrote:
Imagine that you are the maintainer of the mainline and are reviewing the
work made on a side branch that you just merged, but pretend that the
contribution came as a patch instead.  How would you assess the damage to
your mainline?

You would use "git show --first-parent $commit" for that.
Hi,
it seems that git show --first-parent is not documented in the man page.
This option is only documented for rev-list and log. I think that
- this example should land in Examples in git-show.txt
- --first-parent should be documented in git-show.txt because it (at 
least to me, but I guess that for other people also) it isn't 
immediately obvious that it means to _diff_ with the first parent.

--
Zbyszek
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help