Re: [PATCH] Documentation: Update information about <format> in git-for-each-ref

Subsystems: the rest

9 messages, 3 authors, 2016-08-11 · open the first message on its own page

Re: [PATCH] Documentation: Update information about <format> in git-for-each-ref

From: Junio C Hamano <hidden>
Date: 2016-08-11 20:34:53 

Jakub Narebski [off-list ref] writes:

By the way one thing missing from git-for-each-ref to use it
in gitweb without changing its output is ablility to sort on
"epoch", i.e. taggerdate for tags and committerdate for commits.
This is needed to sort heavyweight and lightweight tags together
on epoch in "summary" and "tags" views in gitweb. (It is not
needed for heads/branches, but it is the tags sorting that
cripples gitweb "summary" view performance.)
I guess something like this, but I haven't tested it heavily.
If you want it in "next" please ack (with necessary fix-up
patches if any).

-- >8 --
[PATCH] for-each-ref: epoch and epochdate

This adds "epoch" (which is parallel to "tagger" or "committer")
and "epochdate" (corresponds to "taggerdate" and
"committerdate").

As other "date" fields, "epochdate" sorts numerically
and displays human readably

Signed-off-by: Junio C Hamano <redacted>
---

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 698618b..3dc68cc 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -59,6 +59,8 @@ static struct {
 	{ "taggername" },
 	{ "taggeremail" },
 	{ "taggerdate", FIELD_TIME },
+	{ "epoch" },
+	{ "epochdate", FIELD_TIME },
 	{ "subject" },
 	{ "body" },
 	{ "contents" },
@@ -401,6 +403,29 @@ static void grab_person(const char *who,
 		else if (!strcmp(name + wholen, "date"))
 			grab_date(wholine, v);
 	}
+
+	/* For a tag or a commit object, if "epoch" or "epochdate" is
+	 * requested, do something special.
+	 */
+	if (strcmp(who, "tagger") && strcmp(who, "committer"))
+		return; /* "author" for commit object is not wanted */
+	if (!wholine)
+		wholine = find_wholine(who, wholen, buf, sz);
+	if (!wholine)
+		return;
+	for (i = 0; i < used_atom_cnt; i++) {
+		const char *name = used_atom[i];
+		struct atom_value *v = &val[i];
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+
+		if (!strcmp(name, "epochdate"))
+			grab_date(wholine, v);
+		else if (!strcmp(name, "epoch"))
+			v->s = copy_line(wholine);
+	}
 }
 
 static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body)

[PATCH 1/2] for-each-ref: "creator" and "creatordate" fields

From: Jakub Narebski <hidden>
Date: 2016-08-11 19:18:50 

From fa1a32c9a7c8a31b122df7d07f4a8885cbe120d0 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <redacted>
Date: Sat, 28 Oct 2006 13:33:46 -0700
Subject: [PATCH 1/2] for-each-ref: "creator" and "creatordate" fields

This adds "creator" (which is parallel to "tagger" or "committer")
and "creatordate" (corresponds to "taggerdate" and
"committerdate").

As other "date" fields, "creatordate" sorts numerically
and displays human readably. This allows for example for
sorting together heavyweigth and lightweight tags.

[jn: originally fields were named "epoch" and "epochdate".]

Signed-off-by: Junio C Hamano <redacted>
Acked-by: Jakub Narebski <redacted>
---
 builtin-for-each-ref.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 93d3d7e..173bf38 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -59,6 +59,8 @@ static struct {
 	{ "taggername" },
 	{ "taggeremail" },
 	{ "taggerdate", FIELD_TIME },
+	{ "creator" },
+	{ "creatordate", FIELD_TIME },
 	{ "subject" },
 	{ "body" },
 	{ "contents" },
@@ -401,6 +403,29 @@ static void grab_person(const char *who,
 		else if (!strcmp(name + wholen, "date"))
 			grab_date(wholine, v);
 	}
+
+	/* For a tag or a commit object, if "creator" or "creatordate" is
+	 * requested, do something special.
+	 */
+	if (strcmp(who, "tagger") && strcmp(who, "committer"))
+		return; /* "author" for commit object is not wanted */
+	if (!wholine)
+		wholine = find_wholine(who, wholen, buf, sz);
+	if (!wholine)
+		return;
+	for (i = 0; i < used_atom_cnt; i++) {
+		const char *name = used_atom[i];
+		struct atom_value *v = &val[i];
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+
+		if (!strcmp(name, "creatordate"))
+			grab_date(wholine, v);
+		else if (!strcmp(name, "creator"))
+			v->s = copy_line(wholine);
+	}
 }
 
 static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body)
-- 
1.4.3.3

Re: [PATCH 2/2] gitweb: Use git-for-each-ref to generate list of heads and/or tags

From: Junio C Hamano <hidden>
Date: 2016-08-11 19:53:36 

Jakub Narebski [off-list ref] writes:

From 4d86b34d49dd1f18da465952be8306348fef5150 Mon Sep 17 00:00:00 2001
From: Jakub Narebski <redacted>
Date: Thu, 2 Nov 2006 20:14:15 +0100
Subject: [PATCH 2/2] gitweb: Use git-for-each-ref to generate list of heads and/or tags
The first line should not be in the body of the message.

CHANGES IN OUTPUT: Before, if ref in refs/tags was tag pointing to
commit we used committer epoch as epoch for ref, and used tagger epoch
as epoch only for tag pointing to object of other type. If ref in
refs/tags was commit, we used committer epoch as epoch for ref (see
parse_ref; we sorted in gitweb by 'epoch' field).
I think that behaviour was inconsistent and just silly.  Using
creatordate consistently is better so if the code generates what
the commit log claims to, I think that is fine (I haven't looked
at it deeply yet).

Thanks.  Will apply.

[PATCH 2/2] gitweb: Use git-for-each-ref to generate list of heads and/or tags

From: Jakub Narebski <hidden>
Date: 2016-08-11 19:55:22 

From 4d86b34d49dd1f18da465952be8306348fef5150 Mon Sep 17 00:00:00 2001
From: Jakub Narebski <redacted>
Date: Thu, 2 Nov 2006 20:14:15 +0100
Subject: [PATCH 2/2] gitweb: Use git-for-each-ref to generate list of heads and/or tags

Add two subroutines: git_get_heads_list and git_get_refs_list, which
fill out needed parts of refs info (heads and tags respectively) info
using single call to git-for-each-ref, instead of using
git-peek-remote to get list of references and using parse_ref for each
ref to get ref info, which in turn uses at least one call of git
command.

Replace call to git_get_refs_list in git_summary by call to
git_get_references, git_get_heads_list and git_get_tags_list
(simplifying this subroutine a bit). Use git_get_heads_list in
git_heads and git_get_tags_list in git_tags. Modify git_tags_body
slightly to accept output from git_get_tags_list.

Remove no longer used, and a bit hackish, git_get_refs_list.
parse_ref is no longer used, but is left for now.

Generating "summary" and "tags" views should be much faster for
projects which have large number of tags.

CHANGES IN OUTPUT: Before, if ref in refs/tags was tag pointing to
commit we used committer epoch as epoch for ref, and used tagger epoch
as epoch only for tag pointing to object of other type. If ref in
refs/tags was commit, we used committer epoch as epoch for ref (see
parse_ref; we sorted in gitweb by 'epoch' field).

Currently we use committer epoch for refs pointing to commit objects,
and tagger epoch for refs pointing to tag object, even if tag points
to commit. Going back to old behavior needs additional changes to
git-for-each-ref sorting (sort by given field and use other if first
is not available, --sort=<key>|<key>).

Signed-off-by: Jakub Narebski <redacted>
---
Pasky, you asked for faster "summary" and "tags" view. Simple ab benchmark
before and after this patch for my git.git repository (git/jnareb-git.git)
with some heads and tags added as compared to git.git repository, shows
around 2.4-3.0 times speedup for "summary" and "tags" views:

 summary   3134 +/- 24.2 ms  -->   1081 +/- 30.2 ms
 tags      2886 +/- 18.9 ms  -->   1196 +/- 15.6 ms

 gitweb/gitweb.perl |  153 +++++++++++++++++++++++++++++++---------------------
 1 files changed, 92 insertions(+), 61 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index bf5f829..7710cc2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1294,47 +1294,88 @@ sub parse_ls_tree_line ($;%) {
 ## ......................................................................
 ## parse to array of hashes functions
 
-sub git_get_refs_list {
-	my $type = shift || "";
-	my %refs;
-	my @reflist;
-
-	my @refs;
-	open my $fd, "-|", $GIT, "peek-remote", "$projectroot/$project/"
+sub git_get_heads_list {
+	my $limit = shift;
+	my @headslist;
+
+	open my $fd, '-|', git_cmd(), 'for-each-ref',
+		($limit ? '--count='.($limit+1) : ()), '--sort=-committerdate',
+		'--format=%(objectname) %(refname) %(subject)%00%(committer)',
+		'refs/heads'
 		or return;
 	while (my $line = <$fd>) {
-		chomp $line;
-		if ($line =~ m/^([0-9a-fA-F]{40})\trefs\/($type\/?([^\^]+))(\^\{\})?$/) {
-			if (defined $refs{$1}) {
-				push @{$refs{$1}}, $2;
-			} else {
-				$refs{$1} = [ $2 ];
-			}
+		my %ref_item;
 
-			if (! $4) { # unpeeled, direct reference
-				push @refs, { hash => $1, name => $3 }; # without type
-			} elsif ($3 eq $refs[-1]{'name'}) {
-				# most likely a tag is followed by its peeled
-				# (deref) one, and when that happens we know the
-				# previous one was of type 'tag'.
-				$refs[-1]{'type'} = "tag";
-			}
+		chomp $line;
+		my ($refinfo, $committerinfo) = split(/\0/, $line);
+		my ($hash, $name, $title) = split(' ', $refinfo, 3);
+		my ($committer, $epoch, $tz) =
+			($committerinfo =~ /^(.*) ([0-9]+) (.*)$/);
+		$name =~ s!^refs/heads/!!;
+
+		$ref_item{'name'}  = $name;
+		$ref_item{'id'}    = $hash;
+		$ref_item{'title'} = $title || '(no commit message)';
+		$ref_item{'epoch'} = $epoch;
+		if ($epoch) {
+			$ref_item{'age'} = age_string(time - $ref_item{'epoch'});
+		} else {
+			$ref_item{'age'} = "unknown";
 		}
+
+		push @headslist, \%ref_item;
 	}
 	close $fd;
 
-	foreach my $ref (@refs) {
-		my $ref_file = $ref->{'name'};
-		my $ref_id   = $ref->{'hash'};
+	return wantarray ? @headslist : \@headslist;
+}
+
+sub git_get_tags_list {
+	my $limit = shift;
+	my @tagslist;
 
-		my $type = $ref->{'type'} || git_get_type($ref_id) || next;
-		my %ref_item = parse_ref($ref_file, $ref_id, $type);
+	open my $fd, '-|', git_cmd(), 'for-each-ref',
+		($limit ? '--count='.($limit+1) : ()), '--sort=-creatordate',
+		'--format=%(objectname) %(objecttype) %(refname) '.
+		'%(*objectname) %(*objecttype) %(subject)%00%(creator)',
+		'refs/tags'
+		or return;
+	while (my $line = <$fd>) {
+		my %ref_item;
 
-		push @reflist, \%ref_item;
+		chomp $line;
+		my ($refinfo, $creatorinfo) = split(/\0/, $line);
+		my ($id, $type, $name, $refid, $reftype, $title) = split(' ', $refinfo, 6);
+		my ($creator, $epoch, $tz) =
+			($creatorinfo =~ /^(.*) ([0-9]+) (.*)$/);
+		$name =~ s!^refs/tags/!!;
+
+		$ref_item{'type'} = $type;
+		$ref_item{'id'} = $id;
+		$ref_item{'name'} = $name;
+		if ($type eq "tag") {
+			$ref_item{'subject'} = $title;
+			$ref_item{'reftype'} = $reftype;
+			$ref_item{'refid'}   = $refid;
+		} else {
+			$ref_item{'reftype'} = $type;
+			$ref_item{'refid'}   = $id;
+		}
+
+		if ($type eq "tag" || $type eq "commit") {
+			$ref_item{'epoch'} = $epoch;
+			if ($epoch) {
+				$ref_item{'age'} = age_string(time - $ref_item{'epoch'});
+			} else {
+				$ref_item{'age'} = "unknown";
+			}
+		}
+
+		push @tagslist, \%ref_item;
 	}
-	# sort refs by age
-	@reflist = sort {$b->{'epoch'} <=> $a->{'epoch'}} @reflist;
-	return (\@reflist, \%refs);
+	close $fd;
+
+	return wantarray ? @tagslist : \@tagslist;
 }
 
 ## ----------------------------------------------------------------------
@@ -2264,8 +2305,7 @@ sub git_tags_body {
 	for (my $i = $from; $i <= $to; $i++) {
 		my $entry = $taglist->[$i];
 		my %tag = %$entry;
-		my $comment_lines = $tag{'comment'};
-		my $comment = shift @$comment_lines;
+		my $comment = $tag{'subject'};
 		my $comment_short;
 		if (defined $comment) {
 			$comment_short = chop_str($comment, 30, 5);
@@ -2298,7 +2338,7 @@ sub git_tags_body {
 		      $cgi->a({-href => href(action=>$tag{'reftype'}, hash=>$tag{'refid'})}, $tag{'reftype'});
 		if ($tag{'reftype'} eq "commit") {
 			print " | " . $cgi->a({-href => href(action=>"shortlog", hash=>$tag{'name'})}, "shortlog") .
-			      " | " . $cgi->a({-href => href(action=>"log", hash=>$tag{'refid'})}, "log");
+			      " | " . $cgi->a({-href => href(action=>"log", hash=>$tag{'name'})}, "log");
 		} elsif ($tag{'reftype'} eq "blob") {
 			print " | " . $cgi->a({-href => href(action=>"blob_plain", hash=>$tag{'refid'})}, "raw");
 		}
@@ -2323,23 +2363,23 @@ sub git_heads_body {
 	my $alternate = 1;
 	for (my $i = $from; $i <= $to; $i++) {
 		my $entry = $headlist->[$i];
-		my %tag = %$entry;
-		my $curr = $tag{'id'} eq $head;
+		my %ref = %$entry;
+		my $curr = $ref{'id'} eq $head;
 		if ($alternate) {
 			print "<tr class=\"dark\">\n";
 		} else {
 			print "<tr class=\"light\">\n";
 		}
 		$alternate ^= 1;
-		print "<td><i>$tag{'age'}</i></td>\n" .
-		      ($tag{'id'} eq $head ? "<td class=\"current_head\">" : "<td>") .
-		      $cgi->a({-href => href(action=>"shortlog", hash=>$tag{'name'}),
-		               -class => "list name"},esc_html($tag{'name'})) .
+		print "<td><i>$ref{'age'}</i></td>\n" .
+		      ($curr ? "<td class=\"current_head\">" : "<td>") .
+		      $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'name'}),
+		               -class => "list name"},esc_html($ref{'name'})) .
 		      "</td>\n" .
 		      "<td class=\"link\">" .
-		      $cgi->a({-href => href(action=>"shortlog", hash=>$tag{'name'})}, "shortlog") . " | " .
-		      $cgi->a({-href => href(action=>"log", hash=>$tag{'name'})}, "log") . " | " .
-		      $cgi->a({-href => href(action=>"tree", hash=>$tag{'name'}, hash_base=>$tag{'name'})}, "tree") .
+		      $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'name'})}, "shortlog") . " | " .
+		      $cgi->a({-href => href(action=>"log", hash=>$ref{'name'})}, "log") . " | " .
+		      $cgi->a({-href => href(action=>"tree", hash=>$ref{'name'}, hash_base=>$ref{'name'})}, "tree") .
 		      "</td>\n" .
 		      "</tr>";
 	}
@@ -2489,18 +2529,9 @@ sub git_summary {
 
 	my $owner = git_get_project_owner($project);
 
-	my ($reflist, $refs) = git_get_refs_list();
-
-	my @taglist;
-	my @headlist;
-	foreach my $ref (@$reflist) {
-		if ($ref->{'name'} =~ s!^heads/!!) {
-			push @headlist, $ref;
-		} else {
-			$ref->{'name'} =~ s!^tags/!!;
-			push @taglist, $ref;
-		}
-	}
+	my $refs = git_get_references();
+	my @taglist  = git_get_tags_list(15);
+	my @headlist = git_get_heads_list(15);
 
 	git_header_html();
 	git_print_page_nav('summary','', $head);
@@ -2792,9 +2823,9 @@ sub git_tags {
 	git_print_page_nav('','', $head,undef,$head);
 	git_print_header_div('summary', $project);
 
-	my ($taglist) = git_get_refs_list("tags");
-	if (@$taglist) {
-		git_tags_body($taglist);
+	my @tagslist = git_get_tags_list();
+	if (@tagslist) {
+		git_tags_body(\@tagslist);
 	}
 	git_footer_html();
 }
@@ -2805,9 +2836,9 @@ sub git_heads {
 	git_print_page_nav('','', $head,undef,$head);
 	git_print_header_div('summary', $project);
 
-	my ($headlist) = git_get_refs_list("heads");
-	if (@$headlist) {
-		git_heads_body($headlist, $head);
+	my @headslist = git_get_heads_list();
+	if (@headslist) {
+		git_heads_body(\@headslist, $head);
 	}
 	git_footer_html();
 }
-- 
1.4.3.3

Re: [PATCH 1/2] for-each-ref: "creator" and "creatordate" fields

From: Jakub Narebski <hidden>
Date: 2016-08-11 20:01:32 

Dnia piątek 3. listopada 2006 03:40, Junio C Hamano napisał:
Jakub Narebski [off-list ref] writes:
quoted
From fa1a32c9a7c8a31b122df7d07f4a8885cbe120d0 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <redacted>
Date: Sat, 28 Oct 2006 13:33:46 -0700
Subject: [PATCH 1/2] for-each-ref: "creator" and "creatordate" fields

This adds "creator" (which is parallel to "tagger" or "committer")
and "creatordate" (corresponds to "taggerdate" and
"committerdate").
The first line should not be in the message.
I'm sorry. I just tried to send patch without changing authorship
(from yours).

-- 
Jakub Narebski

Re: [PATCH 1/2] for-each-ref: "creator" and "creatordate" fields

From: Junio C Hamano <hidden>
Date: 2016-08-11 20:03:54 

Jakub Narebski [off-list ref] writes:

From fa1a32c9a7c8a31b122df7d07f4a8885cbe120d0 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <redacted>
Date: Sat, 28 Oct 2006 13:33:46 -0700
Subject: [PATCH 1/2] for-each-ref: "creator" and "creatordate" fields

This adds "creator" (which is parallel to "tagger" or "committer")
and "creatordate" (corresponds to "taggerdate" and
"committerdate").
The first line should not be in the message.  I agree that it is
sensible not to call this "epoch", and "creator" is fine by me.

Thanks.  Will apply.

Re: [PATCH] Documentation: Update information about <format> in git-for-each-ref

From: Jakub Narebski <hidden>
Date: 2016-08-11 20:21:57 

Junio C Hamano wrote:
[PATCH] for-each-ref: epoch and epochdate

This adds "epoch" (which is parallel to "tagger" or "committer")
and "epochdate" (corresponds to "taggerdate" and
"committerdate").

As other "date" fields, "epochdate" sorts numerically
and displays human readably
I was thinking about having only "epochdate" (corresponding to either 
"taggerdate" or "committerdate"), only named "epoch". There is I think 
no need for field which would be "tagger" or "committer", and 
especially not named "epoch" ;-).

Otherwise looks fine, thanks a lot.


BTW. I had to translate
+       if (strcmp(who, "tagger") && strcmp(who, "committer"))
to
+       if (strcmp(who, "tagger") == 0 || strcmp(who, "committer") == 0)
to understand it. But this is probably my lack of contact with such
C idioms.
-- 
Jakub Narebski

Re: [PATCH] Documentation: Update information about <format> in git-for-each-ref

From: Andreas Ericsson <hidden>
Date: 2016-08-11 20:46:59 

Jakub Narebski wrote:
Junio C Hamano wrote:
quoted
[PATCH] for-each-ref: epoch and epochdate

This adds "epoch" (which is parallel to "tagger" or "committer")
and "epochdate" (corresponds to "taggerdate" and
"committerdate").

As other "date" fields, "epochdate" sorts numerically
and displays human readably
I was thinking about having only "epochdate" (corresponding to either "taggerdate" or "committerdate"), only named "epoch". There is I think no need for field which would be "tagger" or "committer", and especially not named "epoch" ;-).

Otherwise looks fine, thanks a lot.


BTW. I had to translate
+       if (strcmp(who, "tagger") && strcmp(who, "committer"))
to
+       if (strcmp(who, "tagger") == 0 || strcmp(who, "committer") == 0)
to understand it. But this is probably my lack of contact with such
C idioms.

But this does the exact opposite. The condition will now be true if the 'who' variable holds a pointer to a string that is either "tagger" or "committer", whereas it used to be true for strings that were anything *but* any of those.

"Compare" (as in "strcmp") also translates to "are equal to" and isn't only a verb. This is unfortunate for people who aren't natively english and has had me confused on many a long night. I once ended up doing a macro called "string_matches" just to wrap my head around an insanely long conditional with too many strcmp() with about half of them negated by !.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se

Re: [PATCH 2/2] gitweb: Use git-for-each-ref to generate list of heads and/or tags

From: Jakub Narebski <hidden>
Date: 2016-08-11 20:47:00 

Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
CHANGES IN OUTPUT: Before, if ref in refs/tags was tag pointing to
commit we used committer epoch as epoch for ref, and used tagger epoch
as epoch only for tag pointing to object of other type. If ref in
refs/tags was commit, we used committer epoch as epoch for ref (see
parse_ref; we sorted in gitweb by 'epoch' field).
I think that behaviour was inconsistent and just silly.  Using
creatordate consistently is better so if the code generates what
the commit log claims to, I think that is fine (I haven't looked
at it deeply yet).
Well, I _could_ argue on the former behaviour, namely that we use
committerdate when available (when ref points to commit, or to tag
pointing to commit), and if it is not available we use taggerdate
if possible.

The latter is that we use always "creation" date.

-- 
Jakub Narebski
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help