{"thread":{"id":"50163","subject":"Make \"git log --count\" work like \"git rev-list\"","startedAt":"2019-01-05T22:47:00Z","lastAt":"2019-01-11T15:35:45Z","messageCount":7,"participants":["Linus Torvalds","Stefan Beller","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"366195","messageId":"CAHk-=wg0NUNFjZumgC-9f=kmU3L4T+qOAgXwiDAfPaNtuFfvFg@mail.gmail.com","threadId":"50163","inReplyTo":null,"subject":"Make \"git log --count\" work like \"git rev-list\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2019-01-05T22:46:37Z","receivedAt":"2019-01-05T22:47:00Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"This is a ridiculous patch. And I understand entirely if nobody else\ncares, because I don't think anybody else has ever even noticed.\n\nIt turns out that I still use \"git rev-list\" outside of some hard-core\nscripting for one silly reason: the linux-next statistics are all\nabout non-merge commits, and so to do a rough comparison with\nlinux-next, I do\n\n git rev-list --count --no-merges v4.20..\n\nto get an approximate idea of how much I've merged compared to what is\nin linux-next.\n\n(See also\n\n http://neuling.org/linux-next-size.html\n\nfor the graphical view of it all, even though it looks a bit odd right\nnow because of how linux-next wasn't being updated over the holidats\nand right at the 4.19 release).\n\nAnyway, I've occasionally thought to myself that I should just fix\n\"git log\" to learn that too, so that I wouldn't have to type out \"git\nrev-list\". Because \"git log\" does actually take the \"--count\"\nargument, it just doesn't honor it.\n\nThis is that patch.\n\n Linus\n\n\nFrom e3bc5387404bcefbd86081fbc82a93fc5c9efa99 Mon Sep 17 00:00:00 2001\nFrom: Linus Torvalds \nDate: Sat, 5 Jan 2019 14:39:41 -0800\nSubject: [PATCH] git log: honor the '--count' argument\n\n\"git log\" is really the human-facing useful command that long long ago\nused to scripted around \"git rev-list\".\n\nIn fact, if you want to use the old scripting code, you can still\napproximate \"git log\" with something like\n\n git rev-list --pretty HEAD | less\n\nbut you'd be silly to do that, since \"git log\" is clearly a much nicer\ninterface and is what people should use.\n\nExcept I find myself still occasionally using \"git rev-list\" simply\nbecause \"git log\" doesn't do one odd little quirk: commit counting.\n\nSo if you want to count the number of non-merge commits since the last\nkernel version, you'd have to do something like\n\n git rev-list --count --no-merges v4.20..\n\nbecause while \"git log\" actually silently _accepts_ the \"--count\"\nargument, it doesn't do anything about it.\n\nThis little patch copies the rev-list code for counting to \"git log\".\n\nSigned-off-by: Linus Torvalds \n---\n builtin/log.c | 11 +++++++++++\n log-tree.c | 10 ++++++++++\n 2 files changed, 21 insertions(+)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e8e51068bd..62bef62f8a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -411,6 +411,17 @@ static int cmd_log_walk(struct rev_info *rev)\n \tif (close_file)\n \t\tfclose(rev->diffopt.file);\n \n+\tif (rev->count) {\n+\t\tif (rev->left_right && rev->cherry_mark)\n+\t\t\tprintf(\"%d\\t%d\\t%d\\n\", rev->count_left, rev->count_right, rev->count_same);\n+\t\telse if (rev->left_right)\n+\t\t\tprintf(\"%d\\t%d\\n\", rev->count_left, rev->count_right);\n+\t\telse if (rev->cherry_mark)\n+\t\t\tprintf(\"%d\\t%d\\n\", rev->count_left + rev->count_right, rev->count_same);\n+\t\telse\n+\t\t\tprintf(\"%d\\n\", rev->count_left + rev->count_right);\n+\t}\n+\n \tif (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF &&\n \t rev->diffopt.flags.check_failed) {\n \t\treturn 02;\ndiff --git a/log-tree.c b/log-tree.c\nindex 10680c139e..49ff485320 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -913,6 +913,16 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)\n \tstruct log_info log;\n \tint shown, close_file = opt->diffopt.close_file;\n \n+\tif (opt->count) {\n+\t\tif (commit->object.flags & PATCHSAME)\n+\t\t\topt->count_same++;\n+\t\telse if (commit->object.flags & SYMMETRIC_LEFT)\n+\t\t\topt->count_left++;\n+\t\telse\n+\t\t\topt->count_right++;\n+\t\treturn 1;\n+\t}\n+\n \tlog.commit = commit;\n \tlog.parent = NULL;\n \topt->loginfo = &log;\n-- \n2.20.1.101.g60ba6df0c4\n\n"},{"id":"366429","messageId":"CAGZ79kaOcoVSRgTR+k9ENoDz6FHh27MzpZ9D-MeFArN2vgBK0w@mail.gmail.com","threadId":"50163","inReplyTo":"CAHk-=wg0NUNFjZumgC-9f=kmU3L4T+qOAgXwiDAfPaNtuFfvFg@mail.gmail.com","subject":"Re: Make \"git log --count\" work like \"git rev-list\"","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2019-01-09T17:21:31Z","receivedAt":"2019-01-09T17:21:46Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Jan 5, 2019 at 2:47 PM Linus Torvalds\n wrote:\n>\n> This is a ridiculous patch. And I understand entirely if nobody else\n> cares, because I don't think anybody else has ever even noticed.\n>\n> It turns out that I still use \"git rev-list\" outside of some hard-core\n> scripting for one silly reason: the linux-next statistics are all\n> about non-merge commits, and so to do a rough comparison with\n> linux-next, I do\n>\n> git rev-list --count --no-merges v4.20..\n>\n> to get an approximate idea of how much I've merged compared to what is\n> in linux-next.\n>\n> (See also\n>\n> http://neuling.org/linux-next-size.html\n>\n> for the graphical view of it all, even though it looks a bit odd right\n> now because of how linux-next wasn't being updated over the holidats\n> and right at the 4.19 release).\n>\n> Anyway, I've occasionally thought to myself that I should just fix\n> \"git log\" to learn that too, so that I wouldn't have to type out \"git\n> rev-list\". Because \"git log\" does actually take the \"--count\"\n> argument, it just doesn't honor it.\n>\n> This is that patch.\n\nSounds reasonable to me to have such functionality,\nas I tend to use\n git log --oneline origin/master..origin/next --no-merges |wc -l\nfor such queries, which I always assume to be doing useless\nwork as I'd be interested in *only* the count, and not the\nintermediate oneline output, but that is the best output\nthat wc works on.\n\nSo maybe the --count option would want to suppress\nother output if given (or we'd want to have another option\nfor no output)?\n\nInstead of printing, do we want to redirect to\nrev->diffopt.file, or send it through the diff codes\nbuffer if we ever anticipate needing this output in\nbuffered?\n\nA test would be nice, too.\n\nStefan\n"},{"id":"366431","messageId":"CAHk-=wjip45uqONG=Qzevd0HoT47t7NatSpr+N3ifMZehr2M0Q@mail.gmail.com","threadId":"50163","inReplyTo":"CAGZ79kaOcoVSRgTR+k9ENoDz6FHh27MzpZ9D-MeFArN2vgBK0w@mail.gmail.com","subject":"Re: Make \"git log --count\" work like \"git rev-list\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2019-01-09T17:44:14Z","receivedAt":"2019-01-09T17:44:35Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Wed, Jan 9, 2019 at 9:21 AM Stefan Beller wrote:\n>\n> Sounds reasonable to me to have such functionality,\n> as I tend to use\n> git log --oneline origin/master..origin/next --no-merges |wc -l\n> for such queries, which I always assume to be doing useless\n> work as I'd be interested in *only* the count, and not the\n> intermediate oneline output, but that is the best output\n> that wc works on.\n\nRight. I've been known to do that too, but because I grew up with \"git\nrev-list\", I know about --count and tend to use it.\n\nIn fact, I've occasionally used it with \"git log\" already (before my\npatch), and it would silently accept the parameter (because it's\nparsed by the generic revision parsing), it just wouldn't work.\n\n> So maybe the --count option would want to suppress\n> other output if given (or we'd want to have another option\n> for no output)?\n\nIt already does so in my patch, exactly because it uses the same logic\nas \"git rev-list --count\" does.\n\nWhich is to simply react to the \"--count\" thing early in\nlog_tree_commit(), and do the counting and then say \"I showed this\"\n(without showing anything).\n\nSo you can do silly things like this:\n\n [torvalds@i7 linux]$ git log --count -10\n 10\n [torvalds@i7 linux]$\n\nand it just works. Nonsensical, but logical.\n\n Linus\n"},{"id":"366442","messageId":"20190109195428.GA12645@sigill.intra.peff.net","threadId":"50163","inReplyTo":"CAHk-=wg0NUNFjZumgC-9f=kmU3L4T+qOAgXwiDAfPaNtuFfvFg@mail.gmail.com","subject":"Re: Make \"git log --count\" work like \"git rev-list\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-01-09T19:54:28Z","receivedAt":"2019-01-09T19:54:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 05, 2019 at 02:46:37PM -0800, Linus Torvalds wrote:\n\n> \"git log\" is really the human-facing useful command that long long ago\n> used to scripted around \"git rev-list\".\n> \n> In fact, if you want to use the old scripting code, you can still\n> approximate \"git log\" with something like\n> \n> git rev-list --pretty HEAD | less\n> \n> but you'd be silly to do that, since \"git log\" is clearly a much nicer\n> interface and is what people should use.\n> \n> Except I find myself still occasionally using \"git rev-list\" simply\n> because \"git log\" doesn't do one odd little quirk: commit counting.\n\nI suspect it's more than that one little quirk. E.g., \"git log\" does not\ndo anything useful with \"--objects\" or \"--use-bitmap-index\", and perhaps\nsome others. I'm not at all opposed to bringing the feature-sets more\ninline (where it makes sense -- I'm not sure what \"log --objects\" would\ndo), but I hope that we can generally do so by pushing features into\nrevision.c, and not just re-implementing them.\n\nThe counting part may have to be specific to each command, since it\ndepends on the loop around get_revision(). But I wonder if we can push\nthe logic into a helper function (and ditto for the printing code).\n\n> So if you want to count the number of non-merge commits since the last\n> kernel version, you'd have to do something like\n> \n> git rev-list --count --no-merges v4.20..\n> \n> because while \"git log\" actually silently _accepts_ the \"--count\"\n> argument, it doesn't do anything about it.\n\nYeah, silently ignoring \"--count\" is just awful. I agree we should make\nit do the sensible thing here. For something like \"log --objects\", we\nshould probably flag an error.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index e8e51068bd..62bef62f8a 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -411,6 +411,17 @@ static int cmd_log_walk(struct rev_info *rev)\n> \tif (close_file)\n> \t\tfclose(rev->diffopt.file);\n> \n> +\tif (rev->count) {\n> +\t\tif (rev->left_right && rev->cherry_mark)\n> +\t\t\tprintf(\"%d\\t%d\\t%d\\n\", rev->count_left, rev->count_right, rev->count_same);\n> +\t\telse if (rev->left_right)\n> +\t\t\tprintf(\"%d\\t%d\\n\", rev->count_left, rev->count_right);\n> +\t\telse if (rev->cherry_mark)\n> +\t\t\tprintf(\"%d\\t%d\\n\", rev->count_left + rev->count_right, rev->count_same);\n> +\t\telse\n> +\t\t\tprintf(\"%d\\n\", rev->count_left + rev->count_right);\n> +\t}\n\nOK, this makes sense, and is lifted from rev-list.\n\n> diff --git a/log-tree.c b/log-tree.c\n> index 10680c139e..49ff485320 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -913,6 +913,16 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)\n> \tstruct log_info log;\n> \tint shown, close_file = opt->diffopt.close_file;\n> \n> +\tif (opt->count) {\n> +\t\tif (commit->object.flags & PATCHSAME)\n> +\t\t\topt->count_same++;\n> +\t\telse if (commit->object.flags & SYMMETRIC_LEFT)\n> +\t\t\topt->count_left++;\n> +\t\telse\n> +\t\t\topt->count_right++;\n> +\t\treturn 1;\n> +\t}\n\nAnd the logic here make sense, but I wonder if this is the best place to\nput it.\n\nWe call log_tree_commit() from lots of places besides git-log, but\npresumably none of would set opt->count recreationally, so that's\nprobably not a big deal.\n\nBut does this catch all of the limiting that git-log would do? I notice\nthat it happens before the call to log_tree_diff(), which conditionally\nreturns a \"shown\" flag. So you get weird results with some options. For\nexample:\n\n # works, because pathspec limiting happens early\n git log --count builtin/log.c\n\n # doesn't work, because --follow disables pruning\n git log --follow --count builtin/log.c\n\nI know \"--follow\" is a bit hacky in general, but I think there are other\ncases where log_tree_diff() may decide not to show a commit (maybe\nwithout --root, though I guess that's the default these days).\n\nUnfortunately I'm not sure of an easy fix. We'd have to tell the log\ncode \"figure out if you're going to show the commit, but don't actually\nshow anything\", which means respecting the count flag virtually\neverywhere that would produce output.\n\nI dunno. Certainly respecting \"--count\" even for the simple cases is an\nimprovement over the status quo. Maybe it would be enough to give a\nwarning in the manpage that it may not work with exotic options.\n\n-Peff\n"},{"id":"366445","messageId":"CAGZ79kan3yavtQOPAHUdkjTXeHpCGPb-igtNMPXjS6GCxOCS+Q@mail.gmail.com","threadId":"50163","inReplyTo":"20190109195428.GA12645@sigill.intra.peff.net","subject":"Re: Make \"git log --count\" work like \"git rev-list\"","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2019-01-09T20:14:26Z","receivedAt":"2019-01-09T20:14:40Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> Unfortunately I'm not sure of an easy fix. We'd have to tell the log\n> code \"figure out if you're going to show the commit, but don't actually\n> show anything\", which means respecting the count flag virtually\n> everywhere that would produce output.\n\nThis is (partially) solved for commit parts already,\ne6e045f803 (diff.c: buffer all output if asked to, 2017-06-29)\nbut we'd have to extend that to revision walking?\n"},{"id":"366541","messageId":"xmqq5zuw17g8.fsf@gitster-ct.c.googlers.com","threadId":"50163","inReplyTo":"20190109195428.GA12645@sigill.intra.peff.net","subject":"Re: Make \"git log --count\" work like \"git rev-list\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-10T22:19:51Z","receivedAt":"2019-01-10T22:19:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But does this catch all of the limiting that git-log would do? I notice\n> that it happens before the call to log_tree_diff(), which conditionally\n> returns a \"shown\" flag. So you get weird results with some options. For\n> example:\n>\n> # works, because pathspec limiting happens early\n> git log --count builtin/log.c\n>\n> # doesn't work, because --follow disables pruning\n> git log --follow --count builtin/log.c\n\nWell, that's a bad one.\n\n> I know \"--follow\" is a bit hacky in general, but I think there are other\n> cases where log_tree_diff() may decide not to show a commit (maybe\n> without --root, though I guess that's the default these days).\n>\n> I dunno. Certainly respecting \"--count\" even for the simple cases is an\n> improvement over the status quo. Maybe it would be enough to give a\n> warning in the manpage that it may not work with exotic options.\n\nHmph, perhaps. I wonder if it is easy enough to redirect the entire\ncodeflow to that of rev-list when we see --count in cmd_log().\n\n"},{"id":"366561","messageId":"20190111153541.GC16754@sigill.intra.peff.net","threadId":"50163","inReplyTo":"xmqq5zuw17g8.fsf@gitster-ct.c.googlers.com","subject":"Re: Make \"git log --count\" work like \"git rev-list\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-01-11T15:35:42Z","receivedAt":"2019-01-11T15:35:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 10, 2019 at 02:19:51PM -0800, Junio C Hamano wrote:\n\n> > I know \"--follow\" is a bit hacky in general, but I think there are other\n> > cases where log_tree_diff() may decide not to show a commit (maybe\n> > without --root, though I guess that's the default these days).\n> >\n> > I dunno. Certainly respecting \"--count\" even for the simple cases is an\n> > improvement over the status quo. Maybe it would be enough to give a\n> > warning in the manpage that it may not work with exotic options.\n> \n> Hmph, perhaps. I wonder if it is easy enough to redirect the entire\n> codeflow to that of rev-list when we see --count in cmd_log().\n\nInteresting idea. I think it might end up creating weird inconsistencies\nfor other cases, though (e.g., log respecting config like log.showroot\nthat rev-list does not).\n\nI suppose it depends on whether we want to advertise that \"log --count\"\nis just a convenience wrapper for \"rev-list --count\". Personally, that\nsounds hacky to me. I'd expect \"log --count \" to give the same\nanswer as \"log --oneline | wc -l\".\n\n-Peff\n"}]}