* git log fails to show all changes for a file @ 2015-07-14 7:30 Olaf Hering 2015-07-14 7:45 ` John Keeping 0 siblings, 1 reply; 13+ messages in thread From: Olaf Hering @ 2015-07-14 7:30 UTC (permalink / raw) To: git 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-14 7:30 git log fails to show all changes for a file Olaf Hering @ 2015-07-14 7:45 ` John Keeping 2015-07-14 7:59 ` Olaf Hering 0 siblings, 1 reply; 13+ messages in thread From: John Keeping @ 2015-07-14 7:45 UTC (permalink / raw) To: Olaf Hering; +Cc: git On Tue, Jul 14, 2015 at 09:30:35AM +0200, Olaf Hering wrote: > > 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? It was added in an evil merge (f9da455b93f6ba076935b4ef4589f61e529ae046), try: git log -p -M --stat --cc -- drivers/hv/channel_mgmt.c ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-14 7:45 ` John Keeping @ 2015-07-14 7:59 ` Olaf Hering 2015-07-14 17:54 ` Andreas Schwab 2015-07-14 19:16 ` Linus Torvalds 0 siblings, 2 replies; 13+ messages in thread From: Olaf Hering @ 2015-07-14 7:59 UTC (permalink / raw) To: John Keeping; +Cc: git On Tue, Jul 14, John Keeping wrote: > It was added in an evil merge (f9da455b93f6ba076935b4ef4589f61e529ae046), > try: > > git log -p -M --stat --cc -- drivers/hv/channel_mgmt.c 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-14 7:59 ` Olaf Hering @ 2015-07-14 17:54 ` Andreas Schwab 2015-07-14 19:16 ` Linus Torvalds 1 sibling, 0 replies; 13+ messages in thread From: Andreas Schwab @ 2015-07-14 17:54 UTC (permalink / raw) To: Olaf Hering; +Cc: John Keeping, git Olaf Hering <olaf@aepfle.de> writes: > On Tue, Jul 14, John Keeping wrote: > >> It was added in an evil merge (f9da455b93f6ba076935b4ef4589f61e529ae046), >> try: >> >> git log -p -M --stat --cc -- drivers/hv/channel_mgmt.c > > Thanks. Thats rather useless output... Why do you think this is useless? > @@@ -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) One branch renamed get_vp_index to init_vp_index, the other branch added the const attribute. This hunk combines both changes. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-14 7:59 ` Olaf Hering 2015-07-14 17:54 ` Andreas Schwab @ 2015-07-14 19:16 ` Linus Torvalds 2015-07-15 16:29 ` Junio C Hamano 1 sibling, 1 reply; 13+ messages in thread From: Linus Torvalds @ 2015-07-14 19:16 UTC (permalink / raw) To: Olaf Hering; +Cc: John Keeping, Git Mailing List On Tue, Jul 14, 2015 at 12:59 AM, Olaf Hering <olaf@aepfle.de> wrote: > On Tue, Jul 14, John Keeping wrote: > >> It was added in an evil merge (f9da455b93f6ba076935b4ef4589f61e529ae046), 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-14 19:16 ` Linus Torvalds @ 2015-07-15 16:29 ` Junio C Hamano 2015-07-15 17:13 ` Linus Torvalds 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2015-07-15 16:29 UTC (permalink / raw) To: Linus Torvalds; +Cc: Olaf Hering, John Keeping, Git Mailing List Linus Torvalds <torvalds@linux-foundation.org> writes: > 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. 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-15 16:29 ` Junio C Hamano @ 2015-07-15 17:13 ` Linus Torvalds 2015-07-15 17:58 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: Linus Torvalds @ 2015-07-15 17:13 UTC (permalink / raw) To: Junio C Hamano; +Cc: Olaf Hering, John Keeping, Git Mailing List On Wed, Jul 15, 2015 at 9:29 AM, Junio C Hamano <gitster@pobox.com> wrote: > > 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. 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-15 17:13 ` Linus Torvalds @ 2015-07-15 17:58 ` Junio C Hamano 2015-07-15 18:11 ` Linus Torvalds 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2015-07-15 17:58 UTC (permalink / raw) To: Linus Torvalds; +Cc: Olaf Hering, John Keeping, Git Mailing List Linus Torvalds <torvalds@linux-foundation.org> writes: > ... > 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. I agree with most of your analysis of the history, but not with "-m" (rather, a part of what "-m" does). > 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. 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-15 17:58 ` Junio C Hamano @ 2015-07-15 18:11 ` Linus Torvalds 2015-07-15 18:17 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: Linus Torvalds @ 2015-07-15 18:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: Olaf Hering, John Keeping, Git Mailing List On Wed, Jul 15, 2015 at 10:58 AM, Junio C Hamano <gitster@pobox.com> wrote: > > * 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). 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-15 18:11 ` Linus Torvalds @ 2015-07-15 18:17 ` Junio C Hamano 2015-07-15 18:57 ` Jeff King 2015-07-15 19:17 ` Linus Torvalds 0 siblings, 2 replies; 13+ messages in thread From: Junio C Hamano @ 2015-07-15 18:17 UTC (permalink / raw) To: Linus Torvalds; +Cc: Olaf Hering, John Keeping, Git Mailing List Linus Torvalds <torvalds@linux-foundation.org> writes: > On Wed, Jul 15, 2015 at 10:58 AM, Junio C Hamano <gitster@pobox.com> wrote: >> >> * 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). > > So this is a suggested change to "-p -m" behavior? 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". > 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" Not really. > "-p -m" would be what is currently "-p --first-parent" Not really. I was dropping "-m" entirely with "we could do -p -m2 but I do not think we should bother". > "-p --no-show-merge-diffs" would be what is currently "-p" Yes. > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-15 18:17 ` Junio C Hamano @ 2015-07-15 18:57 ` Jeff King 2015-07-15 19:22 ` Junio C Hamano 2015-07-15 19:17 ` Linus Torvalds 1 sibling, 1 reply; 13+ messages in thread From: Jeff King @ 2015-07-15 18:57 UTC (permalink / raw) To: Junio C Hamano Cc: Linus Torvalds, Olaf Hering, John Keeping, Git Mailing List On Wed, Jul 15, 2015 at 11:17:50AM -0700, Junio C Hamano wrote: > > So this is a suggested change to "-p -m" behavior? > > 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". 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-15 18:57 ` Jeff King @ 2015-07-15 19:22 ` Junio C Hamano 0 siblings, 0 replies; 13+ messages in thread From: Junio C Hamano @ 2015-07-15 19:22 UTC (permalink / raw) To: Jeff King; +Cc: Linus Torvalds, Olaf Hering, John Keeping, Git Mailing List Jeff King <peff@peff.net> writes: > On Wed, Jul 15, 2015 at 11:17:50AM -0700, Junio C Hamano wrote: > >> > So this is a suggested change to "-p -m" behavior? >> >> 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". > > 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. 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: git log fails to show all changes for a file 2015-07-15 18:17 ` Junio C Hamano 2015-07-15 18:57 ` Jeff King @ 2015-07-15 19:17 ` Linus Torvalds 1 sibling, 0 replies; 13+ messages in thread From: Linus Torvalds @ 2015-07-15 19:17 UTC (permalink / raw) To: Junio C Hamano; +Cc: Olaf Hering, John Keeping, Git Mailing List On Wed, Jul 15, 2015 at 11:17 AM, Junio C Hamano <gitster@pobox.com> wrote: >> >> So this is a suggested change to "-p -m" behavior? > > Not really. This is a suggested behaviour for "git log -p" 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-07-15 19:22 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-07-14 7:30 git log fails to show all changes for a file Olaf Hering 2015-07-14 7:45 ` John Keeping 2015-07-14 7:59 ` Olaf Hering 2015-07-14 17:54 ` Andreas Schwab 2015-07-14 19:16 ` Linus Torvalds 2015-07-15 16:29 ` Junio C Hamano 2015-07-15 17:13 ` Linus Torvalds 2015-07-15 17:58 ` Junio C Hamano 2015-07-15 18:11 ` Linus Torvalds 2015-07-15 18:17 ` Junio C Hamano 2015-07-15 18:57 ` Jeff King 2015-07-15 19:22 ` Junio C Hamano 2015-07-15 19:17 ` Linus Torvalds
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).