[PATCH] --count feature for git shortlog

[PATCH] --count feature for git shortlog

[Lawrence Siebert]

This is a new feature for short log, which lets you count commits on a per
file or repository basis easily.

Currently if you want a total count of commits with shortlog, you would
need to add up each authors commits after using --summary. This adds a 
-N / --count option to shortlog for this purpose.  I also created
a test. It's not a great test, but it works.

I'm sure I've make a beginner mistake in some part of the process;
I've done my best to follow documentation, so please be explicit
when you tell me I've done something wrong.

Thanks,
Lawrence Siebert


Lawrence Siebert (2):
  shortlog: -N/--count option
  shortlog: added --count test

 builtin/shortlog.c  | 14 +++++++++-----
 shortlog.h          |  1 +
 t/t4201-shortlog.sh |  5 +++++
 3 files changed, 15 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH] --count feature for git shortlog

[Lawrence Siebert]

--summary is per author --count counts all

Signed-off-by: Lawrence Siebert <lawrencesiebert@gmail.com>
---
 builtin/shortlog.c | 14 +++++++++-----
 shortlog.h         |  1 +
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index c0bab6a..4b79dc8 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -226,8 +226,10 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 	int nongit = !startup_info->have_repository;
 
 	static const struct option options[] = {
+		 OPT_BOOL('N', "count", &log.show_count,
+			N_("display the total number of commits for all authors")),
 		OPT_BOOL('n', "numbered", &log.sort_by_number,
-			 N_("sort output according to the number of commits per author")),
+			 N_("Sort output according to the number of commits per author")),
 		OPT_BOOL('s', "summary", &log.summary,
 			 N_("Suppress commit descriptions, only provides commit count")),
 		OPT_BOOL('e', "email", &log.email,
@@ -290,8 +292,8 @@ static void add_wrapped_shortlog_msg(struct strbuf *sb, const char *s,
 void shortlog_output(struct shortlog *log)
 {
 	int i, j;
+  unsigned int count = 0;
 	struct strbuf sb = STRBUF_INIT;
-
 	if (log->sort_by_number)
 		qsort(log->list.items, log->list.nr, sizeof(struct string_list_item),
 			compare_by_number);
@@ -300,7 +302,9 @@ void shortlog_output(struct shortlog *log)
 
 		if (log->summary) {
 			printf("%6d\t%s\n", onelines->nr, log->list.items[i].string);
-		} else {
+		} else if (log->show_count) {
+        count += onelines->nr;
+    } else {
 			printf("%s (%d):\n", log->list.items[i].string, onelines->nr);
 			for (j = onelines->nr - 1; j >= 0; j--) {
 				const char *msg = onelines->items[j].string;
@@ -315,13 +319,13 @@ void shortlog_output(struct shortlog *log)
 			}
 			putchar('\n');
 		}
-
 		onelines->strdup_strings = 1;
 		string_list_clear(onelines, 0);
 		free(onelines);
 		log->list.items[i].util = NULL;
 	}
-
+  if (log->show_count) 
+    printf("%d\n", count);
 	strbuf_release(&sb);
 	log->list.strdup_strings = 1;
 	string_list_clear(&log->list, 1);
diff --git a/shortlog.h b/shortlog.h
index de4f86f..57797b7 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -8,6 +8,7 @@ struct shortlog {
 	int summary;
 	int wrap_lines;
 	int sort_by_number;
+	int show_count;
 	int wrap;
 	int in1;
 	int in2;
-- 
1.9.1

[PATCH] --count feature for git shortlog

[Lawrence Siebert]

Signed-off-by: Lawrence Siebert <lawrencesiebert@gmail.com>
---
 t/t4201-shortlog.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 7600a3e..33ecb4a 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -194,4 +194,9 @@ test_expect_success 'shortlog with revision pseudo options' '
 	git shortlog --exclude=refs/heads/m* --all
 '
 
+test_expect_success 'shortlog --count' '
+	git shortlog --count HEAD > actual &&
+	echo  "4" >expect &&
+	test_cmp expect actual'
+
 test_done
-- 
1.9.1

Re: [PATCH] --count feature for git shortlog

[Junio C Hamano]

Lawrence Siebert <lawrencesiebert@gmail.com> writes:

> This is a new feature for short log, which lets you count commits on a per
> file or repository basis easily.
>
> Currently if you want a total count of commits with shortlog, you would
> need to add up each authors commits after using --summary. This adds a 
> -N / --count option to shortlog for this purpose.
  
The standard way to do that is

    git log --oneline ... whatever other args ... | wc -l

or more kosher from a script

    git rev-list ... whatever other args ... | wc -l

Adding an option to ignore per-author breakdown smells backwards.
The whole point of shortlog is to give you the commit stats broken
out for each author.  I guess it is not so far-fetched to add an
option to "git log" to only show the number of commits that would be
output, if you really wanted to avoid "| wc -l", but this option
does not belong to shortlog.

And it certainly does not deserve a short-and-sweet single letter
option "-N", I think.

Re: [PATCH] --count feature for git shortlog

[Lawrence Siebert]

Junio,

I appreciate your help. Okay, That all makes sense.

I would note that something like:
 git shortlog -s "$FILENAME:  | cut -f 1 | paste -sd+ - | bc

seems like it run much faster then:

 git log --oneline "$FILENAME" | wc -l

Which was why I was looking at shortlog.  I was using it to sort files
by commit count when provided a list of files, which git rev-list
doesn't really work for.

Anyway I can try and put it in log proper, if you think that's the best place.

Thank you,
Lawrence Siebert

Re: [PATCH] --count feature for git shortlog

[Junio C Hamano]

Lawrence Siebert <lawrencesiebert@gmail.com> writes:

> I was using it to sort files
> by commit count when provided a list of files, which git rev-list
> doesn't really work for.

What makes you say rev-list does not work (perhaps 'really' is the
key word there?)

    git rev-list --no-merges maint..master -- Makefile
    git shortlog --no-merges maint..master -- Makefile
    git log --oneline --no-merges maint..master -- Makefile

all gives me results that are consistent among them.

Re: [PATCH] --count feature for git shortlog

[Lawrence Siebert]

My apologies, I misunderstood and thought rev-list didn't take filenames.

Lawrence Siebert

On Mon, Jun 29, 2015 at 10:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Lawrence Siebert <lawrencesiebert@gmail.com> writes:
>
>> I was using it to sort files
>> by commit count when provided a list of files, which git rev-list
>> doesn't really work for.
>
> What makes you say rev-list does not work (perhaps 'really' is the
> key word there?)
>
>     git rev-list --no-merges maint..master -- Makefile
>     git shortlog --no-merges maint..master -- Makefile
>     git log --oneline --no-merges maint..master -- Makefile
>
> all gives me results that are consistent among them.
>



-- 
About Me: http://about.me/lawrencesiebert
Constantly Coding: http://constantcoding.blogspot.com

Re: [PATCH] --count feature for git shortlog

[Johannes Schindelin]

Hi Lawrence,

On 2015-06-29 18:46, Lawrence Siebert wrote:

> I appreciate your help. Okay, That all makes sense.
> 
> I would note that something like:
>  git shortlog -s "$FILENAME:  | cut -f 1 | paste -sd+ - | bc
> 
> seems like it run much faster then:
> 
>  git log --oneline "$FILENAME" | wc -l

How does it compare to `git rev-list -- "$FILENAME" | wc -l`?

Ciao,
Johannes

Re: [PATCH] --count feature for git shortlog

[John Keeping]

On Tue, Jun 30, 2015 at 02:10:49PM +0200, Johannes Schindelin wrote:
> On 2015-06-29 18:46, Lawrence Siebert wrote:
> 
> > I appreciate your help. Okay, That all makes sense.
> > 
> > I would note that something like:
> >  git shortlog -s "$FILENAME:  | cut -f 1 | paste -sd+ - | bc
> > 
> > seems like it run much faster then:
> > 
> >  git log --oneline "$FILENAME" | wc -l
> 
> How does it compare to `git rev-list -- "$FILENAME" | wc -l`?

Or even `git rev-list --count HEAD -- "$FILENAME"`.

Re: [PATCH] --count feature for git shortlog

[Lawrence Siebert]

Vincent, I'm ccing you because of --use-bitmap-index

John, Johannes,

I really appreciate both your thoughts.

`git rev-list --count HEAD -- "$FILENAME"` runs noticeably faster then
my patch code for `git shortlog --count`, git shortlog -s "$FILENAME"
| cut -f 1 | paste -sd+ -|bc, and faster than any use of piping to wc
-l mentioned so far.  Anyway I think Junio is quite right that my code
doesn't belong in shortlog, at least as it currently is.

I have some ideas for future work for myself, both code and
documentation changes.  I can detail it and get comments first, or
just submit patches and get comments after, whichever is the preferred
practice. One in particular is worth mentioning.

The following doesn't currently run:  `git rev-list --count
--use-bitmap-index HEAD`

This is an optional parameter for rev-list from commit
aa32939fea9c8934b41efce56015732fa12b8247 which can't currently be used
because of changes in parameter parsing, but which modifies `--count`
and which may be faster. I've gotten it working again, both by
changing the current repo code to make it work, and also by building
from that commit, and when I tested it on the whole repo, it seems
like it's less variable in speed then `git rev-list --count HEAD`. but
not necessarily consistently faster like tests suggested it was when
it was committed. Obviously I'm not testing on the same system as the
original committer, or with the same compiler, or even using the same
version of the linux kernel repo, so those may be a factor.  It may
also work better in a circumstance that I haven't accounted for, like
an older repo, on a per file basis when getting per file commit counts
for all files, or something like that.

I'm thinking I could submit a patch that makes it work again, and
leave it to the user to decide whether to use it or not.   There is
also a --test-bitmap option which compares the regular count with the
bitmap count. I'm not sure if the implication there was regression
testing or that --use-bitmap-index might give the wrong results in
certain circumstances.  Vincent, could you clarify?

Thanks,
Lawrence Siebert
http://www.github.com/gryftir

On Tue, Jun 30, 2015 at 5:23 AM, John Keeping <john@keeping.me.uk> wrote:
>
> On Tue, Jun 30, 2015 at 02:10:49PM +0200, Johannes Schindelin wrote:
> > On 2015-06-29 18:46, Lawrence Siebert wrote:
> >
> > > I appreciate your help. Okay, That all makes sense.
> > >
> > > I would note that something like:
> > >  git shortlog -s "$FILENAME:  | cut -f 1 | paste -sd+ - | bc
> > >
> > > seems like it run much faster then:
> > >
> > >  git log --oneline "$FILENAME" | wc -l
> >
> > How does it compare to `git rev-list -- "$FILENAME" | wc -l`?
>
> Or even `git rev-list --count HEAD -- "$FILENAME"`.

Re: [PATCH] --count feature for git shortlog

[Jeff King]

On Tue, Jun 30, 2015 at 08:00:53PM -0700, Lawrence Siebert wrote:

> The following doesn't currently run:  `git rev-list --count
> --use-bitmap-index HEAD`
> 
> This is an optional parameter for rev-list from commit
> aa32939fea9c8934b41efce56015732fa12b8247 which can't currently be used
> because of changes in parameter parsing, but which modifies `--count`
> and which may be faster. I've gotten it working again, both by
> changing the current repo code to make it work, and also by building
> from that commit, and when I tested it on the whole repo, it seems
> like it's less variable in speed then `git rev-list --count HEAD`. but
> not necessarily consistently faster like tests suggested it was when
> it was committed. Obviously I'm not testing on the same system as the
> original committer, or with the same compiler, or even using the same
> version of the linux kernel repo, so those may be a factor.  It may
> also work better in a circumstance that I haven't accounted for, like
> an older repo, on a per file basis when getting per file commit counts
> for all files, or something like that.

Can you give more details?

In a copy of linux.git with bitmaps:

  $ git log -1 --oneline
  64fb1d0 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc

  $ ls -l .git/objects/pack/
  total 892792
  -r--r--r-- 1 peff peff  24498374 May 21 13:39 pack-182149ca37c3f2d8fa190e4add772ae08af0e9d2.bitmap
  -r--r--r-- 1 peff peff 115283036 May 21 13:39 pack-182149ca37c3f2d8fa190e4add772ae08af0e9d2.idx
  -r--r--r-- 1 peff peff 774420808 May 21 13:39 pack-182149ca37c3f2d8fa190e4add772ae08af0e9d2.pack

The packfiles were created with "git repack -adb". It shows big
speedups for this exact operation:

  $ git version
  git version 2.5.0.rc0

  $ time git rev-list --count HEAD
  515406

  real    0m9.500s
  user    0m9.424s
  sys     0m0.092s

  $ time git rev-list --use-bitmap-index --count HEAD
  515406

  real    0m0.392s
  user    0m0.328s
  sys     0m0.064s

Note that this would not work with, say:

  git rev-list --use-bitmap-index --count HEAD -- Makefile

as the bitmap index does not have enough information to do path limiting
(we should probably disallow this or fall back to the non-bitmap code
path, but right now we just ignore the path limiter).

> I'm thinking I could submit a patch that makes it work again, and
> leave it to the user to decide whether to use it or not.   There is
> also a --test-bitmap option which compares the regular count with the
> bitmap count. I'm not sure if the implication there was regression
> testing or that --use-bitmap-index might give the wrong results in
> certain circumstances.  Vincent, could you clarify?

Yes, `--test-bitmap` is just for internal testing; you should always get
the same results.

-Peff

Re: [PATCH] --count feature for git shortlog

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Note that this would not work with, say:
>
>   git rev-list --use-bitmap-index --count HEAD -- Makefile
>
> as the bitmap index does not have enough information to do path limiting
> (we should probably disallow this or fall back to the non-bitmap code
> path, but right now we just ignore the path limiter).

Yeah, at least the former but preferrably the latter.

Thanks.

Re: [PATCH] --count feature for git shortlog

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> On Tue, Jun 30, 2015 at 02:10:49PM +0200, Johannes Schindelin wrote:
>> On 2015-06-29 18:46, Lawrence Siebert wrote:
>> 
>> > I appreciate your help. Okay, That all makes sense.
>> > 
>> > I would note that something like:
>> >  git shortlog -s "$FILENAME:  | cut -f 1 | paste -sd+ - | bc
>> > 
>> > seems like it run much faster then:
>> > 
>> >  git log --oneline "$FILENAME" | wc -l
>> 
>> How does it compare to `git rev-list -- "$FILENAME" | wc -l`?
>
> Or even `git rev-list --count HEAD -- "$FILENAME"`.

Ahh, OK.  I didn't know we already had "rev-list --count".

Then please disregard the suggestion to add the option to "log"; it
still holds true that the option does not belong to "shortlog", but
I do think "how many changes were made to this path" statistics
driven by a script should use "rev-list" plumbing, and if it already
has "--count" option, that is perfect ;-)

Thanks.

Re: [PATCH] --count feature for git shortlog

[Lawrence Siebert]

On Fri, Jul 3, 2015 at 10:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
> John Keeping <john@keeping.me.uk> writes:
>
>> On Tue, Jun 30, 2015 at 02:10:49PM +0200, Johannes Schindelin wrote:
>>> On 2015-06-29 18:46, Lawrence Siebert wrote:
>>>
>>> > I appreciate your help. Okay, That all makes sense.
>>> >
>>> > I would note that something like:
>>> >  git shortlog -s "$FILENAME:  | cut -f 1 | paste -sd+ - | bc
>>> >
>>> > seems like it run much faster then:
>>> >
>>> >  git log --oneline "$FILENAME" | wc -l
>>>
>>> How does it compare to `git rev-list -- "$FILENAME" | wc -l`?
>>
>> Or even `git rev-list --count HEAD -- "$FILENAME"`.
>
> Ahh, OK.  I didn't know we already had "rev-list --count".
>
> Then please disregard the suggestion to add the option to "log"; it
> still holds true that the option does not belong to "shortlog", but
> I do think "how many changes were made to this path" statistics
> driven by a script should use "rev-list" plumbing, and if it already
> has "--count" option, that is perfect ;-)
>
> Thanks.
>
>
>
Junio,

I think, respectfully, there is still a benefit to adding it as a
feature to "log", in that more Git users know of and use "log" than
"rev-list". I hadn't heard of "rev-list" before joining this mailing
list.

That means "log --count" will get used more. That also means that more
eyeballs will hit --count with bug reports and better tests; I've
already seen 2-3 suggestions for "log --count" tests that "rev-list
--count" also doesn't have tests for.

I would like to keep working on implementing "log --count", sharing
code with rev-list where possible so they both are improved, unless
you are saying you won't merge.

Thanks,
Lawrence



-- 
About Me: http://about.me/lawrencesiebert
Constantly Coding: http://constantcoding.blogspot.com

Re: [PATCH] --count feature for git shortlog

[Jakub Narębski]

Lawrence Siebert wrote:
> On Fri, Jul 3, 2015 at 10:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> John Keeping <john@keeping.me.uk> writes:

>>> Or even `git rev-list --count HEAD -- "$FILENAME"`.
>>
>> Ahh, OK.  I didn't know we already had "rev-list --count".
>>
>> Then please disregard the suggestion to add the option to "log"; it
>> still holds true that the option does not belong to "shortlog", but
>> I do think "how many changes were made to this path" statistics
>> driven by a script should use "rev-list" plumbing, and if it already
>> has "--count" option, that is perfect ;-)
>>
> Junio,
> 
> I think, respectfully, there is still a benefit to adding it as a
> feature to "log", in that more Git users know of and use "log" than
> "rev-list". I hadn't heard of "rev-list" before joining this mailing
> list.
> 
> That means "log --count" will get used more. That also means that more
> eyeballs will hit --count with bug reports and better tests; I've
> already seen 2-3 suggestions for "log --count" tests that "rev-list
> --count" also doesn't have tests for.
> 
> I would like to keep working on implementing "log --count", sharing
> code with rev-list where possible so they both are improved, unless
> you are saying you won't merge.

Lawrence,

As git-rev-list is (mainly) plumbing for git-log porcelain, I think
what you would need to do to add "--count" support to "git log" is
just parse option, exclude nonsense combinations, and pass down to
the revision parsing machinery.

HTH
-- 
Jakub Narębski