patchgitweb: Do not parse refs by hand, use git-peek-remote instead

This is response to Linus work on packed refs. Additionally it makes gitweb work with symrefs, too.

Do not parse refs by hand, using File::Find and reading individual heads to get hash of reference, but use git-peek-remote output instead. Assume that the hash for deref (with ^{}) always follows hash for ref, and that we hav derefs only for tags objects; this removes call to git_get_type (and git-cat-file -t invocation) for tags, which speeds "summary" and "tags" views generation, but might slow generation of "heads" view a bit. As of now we do not save and use the deref hash.

Remove git_get_hash_by_ref while at it, as git_get_refs_list was the only place it was used.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
By the way, the previous patch
  "gitweb: Use File::Find::find in git_get_projects_list"
was added during working on this feature.

 gitweb/gitweb.perl |   39 +++++++++++++++++----------------------
 1 files changed, 17 insertions(+), 22 deletions(-)

Jakub Narebski <jnareb@gmail.com> writes:

That's a very good news.

The assumption is good; we never show a ref^{} before showing ref itself. But you probably would want to safeguard yourself from future changes by not depending on "immediately after". At least you can check $refs[-1]{'name'} is the same as the unpeeled ref you are looking at, like this:

	if ($line =~ m/^([0-9a-fA-F]{40})\t$ref_dir\/?([^\^]+)$/) {
		push @refs, { hash => $1, name => $2 };
	} elsif ($line =~ m/^[0-9a-fA-F]{40}\t$ref_dir\/?(.*)\^\{\}$/ &&
                 $1 eq $refs[-1]{'name'}) {
		# most likely a tag is followed by its peeled
		# one, and when that happens we know the
		# previous one was of type 'tag'.
		$refs[$#refs]->{'type'} = "tag";
	} ...

And this is a good incremental change to reduce number of external calls.

This is in response to Linus work on packed refs. Additionally it makes gitweb work with symrefs, too.

Do not parse refs by hand, using File::Find and reading individual heads to get hash of reference, but use git-peek-remote output instead. Assume that the hash for deref (with ^{}) always follows hash for ref, and that we hav derefs only for tags objects; this removes call to git_get_type (and git-cat-file -t invocation) for tags, which speeds "summary" and "tags" views generation, but might slow generation of "heads" view a bit. As of now we do not save and use the deref hash.

Remove git_get_hash_by_ref while at it, as git_get_refs_list was the only place it was used.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   41 +++++++++++++++++++----------------------
 1 files changed, 19 insertions(+), 22 deletions(-)

Jakub Narebski <jnareb@gmail.com> writes:

Looks nicer.  Will apply.

Now, once we start doing this, it may make sense to rethink how this function and git_get_references functions are used. I think

	git grep -n -e '^sub ' \
        	-e git_get_references \
                -e git_get_refs_list gitweb/gitweb.perl

would be instructive how wasteful the current code is.

get_refs_list is called _TWICE_ in git_summary and worse yet very late in the function, after calling git_get_references that could already have done what the function does (by the way, git_get_references already knows how to use peek-remote output but for some reason it uses ls-remote -- I think you can safely rewrite it to use peek-remote). So you end up doing peek-remote three times to draw the summary page.

git_get_references are called from almost everywhere that shows the list of commits, which is understandable because we would want to see those markers in the list.

I very much suspect that you can use git_get_refs_list to return a hash and a sorted list at the same time from the same input and make git_summary to do with just a single call to it, and get rid of git_get_references with a little bit of restructuring of the caller.

Hmm?

Junio C Hamano wrote:

[...]

It can be done. Well, we could also collapse git_get_references and git_get_refs_list into one subroutine, but it would be slightly slower than git_get_references.

But, if we change git_get_refs_list to do also work of git_get_references, we should also change git_get_references to not use info/refs file at all (it can, and usually for unknown reasons is stale) but always use git-peek-remote, for consistency.

Jakub Narebski <jnareb@gmail.com> writes:

Yes that would make sense. A repository served by gitweb does not necessarily has to serve objects over http transport, so it is nicer not to require info/refs to even exist or up to date.

Junio C Hamano wrote:

We do not require info/refs, as currently git_get_references falls back to git-ls-remotes (should be git-peek-remote as it is faster) if info/refs does not exist. But info/refs is usually stale; I guess it is updated on pull/fetch/push, but not on commit.

Jakub Narebski <jnareb@gmail.com> writes:

Yes, we are in agreement and are saying the same thing.

Junio C Hamano wrote:

We can easily collapse two calls for git_get_refs_list in gi_summary, one for tags and one for heads into one call plus some filtering. Changing git_get_refs_list to do also the job of git_get_references means that in git_tags and git_heads we do extra the job of git_get_references. Neither git_tags, nor git_heads use references and refs marker; using the heads references in git_heads and tags references in git_tags is repeating the same information twice, cluttering the output. Unless we want to add yet another subroutine...

But as call to git-peek-remote is what takes most time, we can waste some time processing references which we would not use for the sake of clarity. Well, we can get rid of git_get_references too, if we don't mind slight decrease in performance.

We would then use:
git_summary:
  my ($refs, $reflist) = git_get_refs_list();
  my @taglist = map { s!^tags/!! } grep { m!^tags/! } @$reflist;
  my @headlist = map { s!^heads/!! } grep { m!^heads/! } @$reflist;

git_heads, git_tags:
  my (undef, $taglist)  = git_get_refs_list("tags");
  my (undef, $headlist) = git_get_refs_list("heads");

everywhere else
  my ($refs, undef) = git_get_refs_list($type);