What should I do to display the diff of even a simple merge commit?

What should I do to display the diff of even a simple merge commit?

[Christian MICHON]

Hi list,

I'm performing many merges between developpers branches these days,
most of them not yielding into conflicts. (understand: simple merges)

All is good, but sometimes, I would like to really like what has been changed.

As I do not systematically do this "git merge --no-commit --stat
<list_to_merge>" and then fire "git gui" to inspect the diffs before
the real commit, I'm wondering: how could I do this using some
plumbing ?

Right now, I've tried the obvious git log -c -p, git show -u --cc, but
since the merge are simple merges, I cannot get any diff output. I
believe this is by construction.

Any hints ?

Thanks in advance.

-- 
Christian
--
http://detaolb.sourceforge.net/, a linux distribution for Qemu with Git inside !

Re: What should I do to display the diff of even a simple merge commit?

[Christian MICHON]

On Wed, Feb 10, 2010 at 12:45 AM, Christian MICHON
<christian.michon@gmail.com> wrote:
> Hi list,
>
> I'm performing many merges between developpers branches these days,
> most of them not yielding into conflicts. (understand: simple merges)
>
> All is good, but sometimes, I would like to really like what has been changed.

I meant << I would really like to see what has been changed. >>

>
> As I do not systematically do this "git merge --no-commit --stat
> <list_to_merge>" and then fire "git gui" to inspect the diffs before
> the real commit, I'm wondering: how could I do this using some
> plumbing ?
>
> Right now, I've tried the obvious git log -c -p, git show -u --cc, but
> since the merge are simple merges, I cannot get any diff output. I
> believe this is by construction.
>
> Any hints ?
>
> Thanks in advance.
>

-- 
Christian
--
http://detaolb.sourceforge.net/, a linux distribution for Qemu with Git inside !

Re: What should I do to display the diff of even a simple merge commit?

[Petr Baudis]

  Hi!

On Wed, Feb 10, 2010 at 12:45:44AM +0100, Christian MICHON wrote:
> I'm performing many merges between developpers branches these days,
> most of them not yielding into conflicts. (understand: simple merges)
> 
> All is good, but sometimes, I would like to really like what has been changed.
> 
> As I do not systematically do this "git merge --no-commit --stat
> <list_to_merge>" and then fire "git gui" to inspect the diffs before
> the real commit, I'm wondering: how could I do this using some
> plumbing ?
> 
> Right now, I've tried the obvious git log -c -p, git show -u --cc, but
> since the merge are simple merges, I cannot get any diff output. I
> believe this is by construction.
> 
> Any hints ?

  I'm not sure if there is any clever switch for this, but I usually
just use one of

	git diff mergecommit^1 mergecommit
	git diff mergecommit^2 mergecommit

depending on which parent I want the diff against. If you always do your
merges as "on mainline, merging in a topic" without fast-forwarding,
diff against the first parent will be probably the right one and you can
simply use:

	git diff mergecommit^ mergecommit

  Kind regards,

				Petr "Pasky" Baudis

Re: What should I do to display the diff of even a simple merge commit?

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>   I'm not sure if there is any clever switch for this, but I usually
> just use one of
>
> 	git diff mergecommit^1 mergecommit
> 	git diff mergecommit^2 mergecommit
>
> depending on which parent I want the diff against. If you always do your
> merges as "on mainline, merging in a topic" without fast-forwarding,
> diff against the first parent will be probably the right one and you can
> simply use:
>
> 	git diff mergecommit^ mergecommit

Frankly, we should make "git log --first-parent -p" DTRT, I think.

The attitude towards merges we maintain officially is "all parents are
equal", but in practice, there often are cases where --first-parent
traversal makes a lot more sense when browsing the history (especially
"the official" one).  The use of that option should be a clear enough sign
that diff between the first parent and the merge result is asked for.

[PATCH] git log -p -m: Document, honor --first-parent

[Petr Baudis]

On Tue, Feb 09, 2010 at 04:05:50PM -0800, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> >   I'm not sure if there is any clever switch for this, but I usually
> > just use one of
> >
> > 	git diff mergecommit^1 mergecommit
> > 	git diff mergecommit^2 mergecommit
> >
> > depending on which parent I want the diff against. If you always do your
> > merges as "on mainline, merging in a topic" without fast-forwarding,
> > diff against the first parent will be probably the right one and you can
> > simply use:
> >
> > 	git diff mergecommit^ mergecommit
> 
> Frankly, we should make "git log --first-parent -p" DTRT, I think.
> 
> The attitude towards merges we maintain officially is "all parents are
> equal", but in practice, there often are cases where --first-parent
> traversal makes a lot more sense when browsing the history (especially
> "the official" one).  The use of that option should be a clear enough sign
> that diff between the first parent and the merge result is asked for.

I have also discovered -m while digging into this, which seems to have
the effect of showing the merge TWICE, each time against a different
parent; this is sort-of-almost what Christian also wanted.

This seems to be undocumented and does have this effect only in log,
not in show - I have absolutely no idea why from cursory code
examination.

I think making just --first-parent alone imply this behavior is wrong,
IMHO first-parent alone does not warrant avoiding combined-diff behavior.
I'm not really sure though, so feel free to add another

	if (revs->diff && revs->follow_first_parent)
		revs->ignore_merges = 0;

test at the right place (whatever that is).  At any rate, -m explicitly
states the intent and the current behavior of not honoring
--first-parent is IMHO a bug.

I think --first-parent documentation is still accurate with the new
behavior, so I adjusted just -m documentation - also making the flag
actually visible for non-diff-tree-stdin cases. Sorry that it's both
conflated in a single patch, I'd rather avoid creating a patch queue
out of this supposedly-5-minute hack.

--8<--

git log -p -m has a special magic behavior of showin one merge entry per
parent, with an appropriate diff; this can be frequently useful when
examining histories where full set of changes introduced by a merged
branch is interesting, not only the conflicts.

This patch properly documents the -m switch, which has so far been
mentioned only as a fairly special diff-tree flag. It also makes the
code show full patch entry only for the first parent in case
--first-parent is used. Thus,

	git log -p -m --first-parent

will show the history from the "main branch perspective", while also
including full diff of changes introduced by other merged in branches.

Signed-off-by: Petr Baudis <pasky@suse.cz>

---

diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt
index 0f25ba7..8f9a241 100644
--- a/Documentation/diff-generate-patch.txt
+++ b/Documentation/diff-generate-patch.txt
@@ -56,7 +56,8 @@ combined diff format
 
 "git-diff-tree", "git-diff-files" and "git-diff" can take '-c' or
 '--cc' option to produce 'combined diff'.  For showing a merge commit
-with "git log -p", this is the default format.
+with "git log -p", this is the default format; you can force showing
+full diff with the '-m' option.
 A 'combined diff' format looks like this:
 
 ------------
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 0e39bb6..a2a2d04 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -118,6 +118,15 @@ git log master --not --remotes=*/master::
 	Shows all commits that are in local master but not in any remote
 	repository master branches.
 
+git log -p -m --first-parent::
+
+	Shows the history including change diffs, but only from the
+	"main branch" perspective, skipping commits that come only from
+	merges, and showing full diffs of changes introduced by the merges.
+	This makes sense only when following a strict policy of merging all
+	topic branches when staying on a single integration branch and
+	making sure the merges are not fast-forwards.
+
 Discussion
 ----------
 
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 6e9baf8..d7d0dee 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -108,8 +108,8 @@ options may be given. See linkgit:git-diff-files[1] for more options.
 
 -c::
 
-	This flag changes the way a merge commit is displayed.  It shows
-	the differences from each of the parents to the merge result
+	This flag forces the default way a merge commit is displayed.  It
+	shows the differences from each of the parents to the merge result
 	simultaneously instead of showing pairwise diff between a parent
 	and the result one at a time. Furthermore, it lists only files
 	which were modified from all parents.
@@ -121,6 +121,15 @@ options may be given. See linkgit:git-diff-files[1] for more options.
 	the parents have only two variants and the merge result picks
 	one of them without modification.
 
+-m::
+
+	This flag makes the merge commits show the full diff like
+	regular commits; for each merge parent, a separate log entry
+	and diff is generated. (An exception is made if '--first-parent'
+	option has been also passed; in that case, only diff against
+	the first parent is shown, representing the changes the merge
+	brought _into_ the then-current branch.)
+
 -r::
 
 	Show recursive diffs.
diff --git a/log-tree.c b/log-tree.c
index 27afcf6..fb990a1 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -514,6 +514,14 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 			return 0;
 		else if (opt->combine_merges)
 			return do_diff_combined(opt, commit);
+		else if (opt->first_parent_only) {
+			/* Generate merge log entry only for the first
+			 * parent, showing summary diff of the others
+			 * we merged _in_. */
+			diff_tree_sha1(parents->item->object.sha1, sha1, "", &opt->diffopt);
+			log_tree_diff_flush(opt);
+			return !opt->loginfo;
+		}
 
 		/* If we show individual diffs, show the parent info */
 		log->parent = parents->item;

Re: [PATCH] git log -p -m: Document, honor --first-parent

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 0e39bb6..a2a2d04 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -118,6 +118,15 @@ git log master --not --remotes=*/master::
>  	Shows all commits that are in local master but not in any remote
>  	repository master branches.
>  
> +git log -p -m --first-parent::
> +
> +	Shows the history including change diffs, but only from the
> +	"main branch" perspective, skipping commits that come only from
> +	merges, and showing full diffs of changes introduced by the merges.
> +	This makes sense only when following a strict policy of merging all
> +	topic branches when staying on a single integration branch and
> +	making sure the merges are not fast-forwards.

I think the tone of the last three lines is too strong.

Why is it necessary to make a merge with a single commit side branch when
fast-forward would do?  And if the side branch is actually two or more
commits, it will show the broken-down changes in more detail, but the fact
that it was made on the "primary" history would also have some
significance (e.g. trivial and obvious fixes made directly on 'master',
other branches merged from topic after cooking).

It is Ok to elaborate on the "policy" issues in the Discussion section,
but otherwise, I would rather see you spend the same number of lines to
clarify "showing full diffs of changes introduced by the merges" a bit
better (e.g. it is unclear if you are showing diff from each parents or
just from the first parent).  Perhaps "s/introduced /& to the first-parent
ancestry /" may suffice.

> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 6e9baf8..d7d0dee 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -108,8 +108,8 @@ options may be given. See linkgit:git-diff-files[1] for more options.
>  
>  -c::
>  
> -	This flag changes the way a merge commit is displayed.  It shows
> -	the differences from each of the parents to the merge result
> +	This flag forces the default way a merge commit is displayed.  It
> +	shows the differences from each of the parents to the merge result
>  	simultaneously instead of showing pairwise diff between a parent

Sorry, I don't understand this change; "forces the default?"  Any option
"forces" the command to behave differently.  At least the original is
understandable "Ah, without it it shows one way but with this it shows in
a different way", even though that does not carry much useful information
(i.e. what are the two ways?  ah, I need to read further down).

> diff --git a/log-tree.c b/log-tree.c
> index 27afcf6..fb990a1 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -514,6 +514,14 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
>  			return 0;
>  		else if (opt->combine_merges)
>  			return do_diff_combined(opt, commit);
> +		else if (opt->first_parent_only) {
> +			/* Generate merge log entry only for the first
> +			 * parent, showing summary diff of the others
> +			 * we merged _in_. */

Style?

Don't we use --cc as default for "show" (and possibly "log"---I don't
remember the details)?

> +			diff_tree_sha1(parents->item->object.sha1, sha1, "", &opt->diffopt);
> +			log_tree_diff_flush(opt);
> +			return !opt->loginfo;
> +		}

This needs some tests but I think it is a good first step in the right
direction.

Thanks.

Re: [PATCH] git log -p -m: Document, honor --first-parent

[Petr Baudis]

On Tue, Feb 09, 2010 at 05:30:29PM -0800, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> > diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> > index 0e39bb6..a2a2d04 100644
> > --- a/Documentation/git-log.txt
> > +++ b/Documentation/git-log.txt
> > @@ -118,6 +118,15 @@ git log master --not --remotes=*/master::
> >  	Shows all commits that are in local master but not in any remote
> >  	repository master branches.
> >  
> > +git log -p -m --first-parent::
> > +
> > +	Shows the history including change diffs, but only from the
> > +	"main branch" perspective, skipping commits that come only from
> > +	merges, and showing full diffs of changes introduced by the merges.
> > +	This makes sense only when following a strict policy of merging all
> > +	topic branches when staying on a single integration branch and
> > +	making sure the merges are not fast-forwards.
> 
> I think the tone of the last three lines is too strong.
> 
> Why is it necessary to make a merge with a single commit side branch when
> fast-forward would do?  And if the side branch is actually two or more
> commits, it will show the broken-down changes in more detail, but the fact
> that it was made on the "primary" history would also have some
> significance (e.g. trivial and obvious fixes made directly on 'master',
> other branches merged from topic after cooking).

Ok, so what about "...the merges are not fast-forwards if the branch
histories are non-trivial"? Since there are two cases:

* The branch was created on top of HEAD, the commits were made and now
the branch is merged back, that's an "ok fastforward".

* The branch was created long ago, but has merged latest changes of the
top of HEAD in, and now the branch is merged back, that's a "bad
fastforward" since that flips the perspective of main-vs-topic branch.

I feel that it's important to point out this caveat.

> It is Ok to elaborate on the "policy" issues in the Discussion section,
> but otherwise, I would rather see you spend the same number of lines to
> clarify "showing full diffs of changes introduced by the merges" a bit
> better (e.g. it is unclear if you are showing diff from each parents or
> just from the first parent).  Perhaps "s/introduced /& to the first-parent
> ancestry /" may suffice.

I really dislike the "first-parent ancestry" wording, I think it muds
down the whole issue.  It would seem to me that the basic idea is clear
from the description (which might even now be excessively verbose) and
if anyone is still confused, they can quickly peek at -m description or
actually try the command out.

> > diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> > index 6e9baf8..d7d0dee 100644
> > --- a/Documentation/rev-list-options.txt
> > +++ b/Documentation/rev-list-options.txt
> > @@ -108,8 +108,8 @@ options may be given. See linkgit:git-diff-files[1] for more options.
> >  
> >  -c::
> >  
> > -	This flag changes the way a merge commit is displayed.  It shows
> > -	the differences from each of the parents to the merge result
> > +	This flag forces the default way a merge commit is displayed.  It
> > +	shows the differences from each of the parents to the merge result
> >  	simultaneously instead of showing pairwise diff between a parent
> 
> Sorry, I don't understand this change; "forces the default?"  Any option
> "forces" the command to behave differently.  At least the original is
> understandable "Ah, without it it shows one way but with this it shows in
> a different way", even though that does not carry much useful information
> (i.e. what are the two ways?  ah, I need to read further down).

At some point when making this change, I was in the state of believing
that combined diffs are always the default. :-) That is not true, so
I will drop this change again.

> > diff --git a/log-tree.c b/log-tree.c
> > index 27afcf6..fb990a1 100644
> > --- a/log-tree.c
> > +++ b/log-tree.c
> > @@ -514,6 +514,14 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
> >  			return 0;
> >  		else if (opt->combine_merges)
> >  			return do_diff_combined(opt, commit);
> > +		else if (opt->first_parent_only) {
> > +			/* Generate merge log entry only for the first
> > +			 * parent, showing summary diff of the others
> > +			 * we merged _in_. */
> 
> Style?

What's wrong? There should be an empty line at the comment beginning?
I have a faint memory of getting some-such undocumented comment ugliness
requirement wrong before. ;-)

> Don't we use --cc as default for "show" (and possibly "log"---I don't
> remember the details)?

Ah, that must be it! Turning that off in -m code makes it work for show
as well.

> > +			diff_tree_sha1(parents->item->object.sha1, sha1, "", &opt->diffopt);
> > +			log_tree_diff_flush(opt);
> > +			return !opt->loginfo;
> > +		}
> 
> This needs some tests but I think it is a good first step in the right
> direction.

Thanks. Hrmh, testsuites... ;-)

-- 
				Petr "Pasky" Baudis
If you can't see the value in jet powered ants you should turn in
your nerd card. -- Dunbal (464142)

Re: [PATCH] git log -p -m: Document, honor --first-parent

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> * The branch was created long ago, but has merged latest changes of the
> top of HEAD in, and now the branch is merged back, that's a "bad
> fastforward" since that flips the perspective of main-vs-topic branch.
>
> I feel that it's important to point out this caveat.

Ah, I thought you were contrasting between ff and non-ff, but instead you
were giving caveat about trusting "fast-parent", which I didn't realize.

Yeah, but in general, unless it is the final merge to consolidate the work
on the topic to mainline that was delegated by the mainline maintainer to
the topic person, merging _from_ mainline _to_ topic should rarely happen.
And when it happens, relying on the first-parent ancestry obviously breaks
down.

> I really dislike the "first-parent ancestry" wording, I think it muds
> down the whole issue.

I am not particularly fond of the wording, either, but any other word you
would use, you would need to explain the background information, i.e. how
and why the concept embodied by that other word you choose to use relates
to the "--first-parent" option.

You can for example say "the changes introduced to the mainline by each
commit" (and by "commit" we mean both single parent ones directly made
while the mainline was the current branch, and merges made into that
branch); you need to define what you mean by "the mainline", and what your
assumptions are about the workflow employed (e.g. "rarely if ever merge
goes the wrong direction").

>> > +		else if (opt->first_parent_only) {
>> > +			/* Generate merge log entry only for the first
>> > +			 * parent, showing summary diff of the others
>> > +			 * we merged _in_. */
>> 
>> Style?
>
> What's wrong?

	/*
         * We prefer to write multi-line comments
         * like this.
         */

Re: What should I do to display the diff of even a simple merge commit?

[Christian MICHON]

On Wed, Feb 10, 2010 at 12:57 AM, Petr Baudis <pasky@suse.cz> wrote:
>  Hi!
>
> On Wed, Feb 10, 2010 at 12:45:44AM +0100, Christian MICHON wrote:
>> I'm performing many merges between developpers branches these days,
>> most of them not yielding into conflicts. (understand: simple merges)
>>
>> All is good, but sometimes, I would like to really like what has been changed.
>>
>> As I do not systematically do this "git merge --no-commit --stat
>> <list_to_merge>" and then fire "git gui" to inspect the diffs before
>> the real commit, I'm wondering: how could I do this using some
>> plumbing ?
>>
>> Right now, I've tried the obvious git log -c -p, git show -u --cc, but
>> since the merge are simple merges, I cannot get any diff output. I
>> believe this is by construction.
>>
>> Any hints ?
>
>  I'm not sure if there is any clever switch for this, but I usually
> just use one of
>
>        git diff mergecommit^1 mergecommit
>        git diff mergecommit^2 mergecommit
>
> depending on which parent I want the diff against. If you always do your
> merges as "on mainline, merging in a topic" without fast-forwarding,
> diff against the first parent will be probably the right one and you can
> simply use:
>
>        git diff mergecommit^ mergecommit
>

Hi Petr,

unfortunately it does not ouput anything :-( the diff is empty, again...

I'm fiddling now with "git checkout -f <that-commit>", and I'm faking
an "amend last commit" using "git gui". With this dirty trick, I get
the same diff I would have gotten from git gui with a merge
--no-commit.

Thanks for suggesting !

-- 
Christian
--
http://detaolb.sourceforge.net/, a linux distribution for Qemu with Git inside !