git log fails to show all changes for a file

I wonder why this command fails to show all commits that modify a given function:

  linux.git $ git log -p -M --stat -- drivers/hv/channel_mgmt.c

With commit 1f656ff3fdddc2f59649cc84b633b799908f1f7b init_vp_index() has "const uuid_le *type_guid" already. And somewhere between commit d3ba720dd58cdf6630fee4b89482c465d5ad0d0f and the one above the "const" was added. But git log does not show this commit.

Why is that so, and whats the command to really show all and every change?

Olaf

On Tue, Jul 14, 2015 at 09:30:35AM +0200, Olaf Hering wrote:

It was added in an evil merge (f9da455b93f6ba076935b4ef4589f61e529ae046), try:

	git log -p -M --stat --cc -- drivers/hv/channel_mgmt.c

On Tue, Jul 14, John Keeping wrote:

Thanks. Thats rather useless output...

@@@ -404,7 -365,7 +404,7 @@@ static u32  next_vp
   * performance critical channels (IDE, SCSI and Network) will be uniformly
   * distributed across all available CPUs.
   */
- static void init_vp_index(struct vmbus_channel *channel, uuid_le *type_guid)
 -static u32 get_vp_index(const uuid_le *type_guid)
++static void init_vp_index(struct vmbus_channel *channel, const uuid_le *type_guid)
  {
        u32 cur_cpu;
        int i;

Olaf

Olaf Hering <olaf@aepfle.de> writes:

Why do you think this is useless?

One branch renamed get_vp_index to init_vp_index, the other branch added the const attribute. This hunk combines both changes.

Andreas.

On Tue, Jul 14, 2015 at 12:59 AM, Olaf Hering <olaf@aepfle.de> wrote:

That's not an evil merge. That's just a regular merge. One side had changed the argument to "const":

 - 1b9d48f2a579 ("hyper-v: make uuid_le const")

while the other side had renamed the function and added an argument, and changed the return type:

 - d3ba720dd58c ("Drivers: hv: Eliminate the channel spinlock in the
callback path")

an "evil merge" is something that makes changes that came from neither side and aren't actually resolving a conflict.

That said, I do wonder if we should just make "-p" imply "--cc". Right now we have the kind of odd situation that "git log -p" doesn't show merge patches, but "git show <cmit>" does show them. And you kind of have to know a lot about git to know the "--cc" option.

In fact, that "git show" behavior really is very subtle, but very useful. It comes from show_rev_tweak_rev(), which is a magic git-show thing.

               Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

I would think "git log -p" that implies "--cc" would be a good change, as long as there is an easy escape hatch to let us do what we have to do with a rather lengthy "git log -p --first-parent -m" (i.e. show the change relative to its first parent while traversing the first parent chain) today. Perhaps only when there is no explicit "-m" but "-p" is given, automatically enable "--cc", or something like that.

On Wed, Jul 15, 2015 at 9:29 AM, Junio C Hamano <gitster@pobox.com> wrote:

That's very close to what "git show" does through that magic show_rev_tweak_rev() logic, with the crucial difference being that "git show" is designed to always show the diff, so the "-p" is implied.

That said, having thought about it more, I'm not entirely sure we can do it. The big conceptual difference between "git log" and "git show" is obviously that "git show" doesn't walk the revision list, and you always explicitly say "show _this_ commit".

And that means that with "git show", you kind of _know_ that the commit is relevant and interesting, in a way that "git log" does not. So got "git show", it's very natural to say "show all the relevant information", while for "git log" we did make the choice that maybe commit diffs aren't relevant by default.

And the whole issue ends up boiling down to "maybe we picked the wrong choice default". We default to that "ignore_merges = 1" behavior.

Now, we defaulted to ignoring merge diffs because long long ago, in a galaxy far away, we didn't have a great way to show the diffs. The whole "--cc" option goes back to January -06 and commit d8f4790e6fe7 ("diff-tree --cc: denser combined diff output for a merge commit") . And before that option - so for about 8 months - we had no good way to show the diffs of merges in a good dense way.

So the whole "don't show diffs for merges by default" actually made a lot of sense originally, because our merge diffs were not very useful.

But that does mean that if we now enable "--cc" by default when you ask for diffs, we have no good way to _disable_ it. We picked "disable by default", and "-m" means "enable merge diffs". And that made sense back in 2005 because we really wanted to disable the whole "show diffs for merges" thing.

Of course, you can use "--no-merges" to basically not show merges at all, so maybe that's ok. But I get the feeling that if we make "-p" imply "--cc", we should probably add a "--no-merge-diff" option too to replace the (broken) "-m" flag properly. And I'm a tiny bit worried that it might break some script (although I'm really not seeing how/why you'd script "git log -p" output and not want to get a --cc patch for a merge).

And "-m"? We should probably get rid of it. The diffs we get for merges when "-c" or "--cc" isn't given are _not_ useful. The original non-combined diff format was really just useful for showing that "yeah, we have multiple parents, and they are different in all these ways". So there is no actual valid use for "-m" that I can imagine. It's simply just an odd historical artifact from a time when we didn't know how to show merges.

                 Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

I agree with most of your analysis of the history, but not with "-m" (rather, a part of what "-m" does).

When combined with "--first-parent" it is a way to view what the merge brought in from a side branch.

    $ git log --first-parent -m -p ko/master..master

is something I do every time I merge a batch of topics to the 'master' branch (where ko/master keeps track of what was pushed to k.org the last time).

The fact that "-m" shows diffs with each and every parent is an odd historical behaviour, and the only justification for it is that people might want to view differences with second and later parents, which is weak, and one way to allow them to view differences with later parents is to show all (and force people to skip them in normal use case), which is even weaker.

So to be constructive and to pu a different proposal on the table, how about aiming for this end-game state?

 * When '-p' is given, we show only diff with first-parent by
   default, regardless of the traversal (i.e. --first-parent option
   currently controls both traversal and patch display, but in the
   new world order, it reverts back to purely a traversal option).

 * We could allow you to say '-p -m2' to get second-parent diff for
   merges, but I do not think we should bother, for at least two
   reasons.  It opens a can of worms, like "what should '-p -m4' to
   for a two-parent merge?"  Also "log" unlike "show" is about many
   commits, and I do not think of a reason why you would want a
   series of diff between the side-branch tip and the result.  '-m2'
   would only be useful for commits that was made by mistake,
   merging the trunk into a side branch, swapping the
   first-parenthood, which ought to be rare.

 * If somebody wants to omit patch for merges, while still showing
   the merge log, use '-p --no-show-merge-diff'.

 * When people want '--cc' or '-c', they can with 'log -p --cc',
   just like today.  We might want to make '--cc' imply '-p', but I
   vaguely recall some discussion against this [*2*].

Transition plan is a separate matter.

[Footnotes]

*1* With modern Git if you were pushing only to a single
    destination, you could say master@{push} instead, but I do not
    have remote.origin in my repository and push to multiple places,
    so master@{push} does not resolve ;-)

*2* http://thread.gmane.org/gmane.comp.version-control.git/215341/focus=215470

On Wed, Jul 15, 2015 at 10:58 AM, Junio C Hamano <gitster@pobox.com> wrote:

So this is a suggested change to "-p -m" behavior?

Yes, that sounds sane. The current "-p -m" behavior is not useful at all.

So if I understand rightly, we'd have:

 "-p" would be what is currently  "-p --cc"
 "-p -m" would be what is currently "-p --first-parent"
 "-p --no-show-merge-diffs" would be what is currently "-p"

and the rationale would be that

 (a) the current "-p" is hiding things, and while you can add "--cc",
that requires that you really understand what is being hidden, which
is a bad default (the complaint that started this discussion)

 (b) the current "-p -m" is useless crazy stuff, and you'd rather use
it for something that you actually find very common and useful

If so, I agree entirely.

              Linus

Linus Torvalds <torvalds@linux-foundation.org> writes:

Not really. This is a suggested behaviour for "git log -p"; I wasn't very enthused by the idea to turn --cc when user said -p without telling them what we are doing. In other words, if the users want combined, they should say --cc (and they will get a single-parent patch for non-merges with --cc) so there is no reason not to do this, as long as we fix --cc so that "git log --cc" implies "git log --cc -p".

Not really.

Not really. I was dropping "-m" entirely with "we could do -p -m2 but I do not think we should bother".

Yes.

On Wed, Jul 15, 2015 at 11:17:50AM -0700, Junio C Hamano wrote:

Like you, I frequently use "--first-parent -m". If I understand your proposal, a regular "git log" would have the first-parent-diff behavior of those options, but still traverse other parents.

One oddity of that proposal is that the user ends up seeing any given change on a side-branch _twice_. Once in the original commit that introduced it, and once in the merge of the branch. And commit-selection tools like "git log -Ssome_code" will select both, too, and they'll see the merge commit along with the original. I can't decide if that's a good thing or not.

-Peff

Jeff King <peff@peff.net> writes:

Hmm, you are right. That may be a problem, and Linus's makes tons of practical sense (especially for us experienced Git users).

It just is that '-p', that clearly stands for 'patch' but does more than 'patch' to produce something that cannot be fed to 'apply' by defaulting to '--cc', makes me hesitate. By making it a lot more convenient for experienced people who understand these issues, I have this suspicion that it would make the options less orthgonal and harder to explain to new people.

On Wed, Jul 15, 2015 at 11:17 AM, Junio C Hamano <gitster@pobox.com> wrote:

Oh, in that case I say NAK NAK NAK.

That would be totally horrible, and completely unacceptable.

I do "git log -p" all the time, and merge-diffs with --first-parent are regularly hundreds of thousands of lines for the kernel.

So no. That is COMPLETELY unacceptable. Not even worth discussing.

                   Linus