Re: [PATCH 1/4] gitweb: notes feature

[Jakub Narebski <jnareb@gmail.com>]

On Thu, 4 Feb 2010, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>>
>>> +		my %notes = () ;
>>> +		foreach my $note_ref (@note_refs) {
>>> +			my $obj = "$note_ref:$co{'id'}";
>>
>> I think this look-up is wrong (meaning: will stop working anytime in the
>> future, and needs to be rewritten).
> 
> IOW, the code should be reading output from:
> 
>     GIT_NOTES_REF=$note_ref git show -s --format=%N $co{'id'}
> 
> as the notes tree may not be storing notes in a flat one-level namespace
> like you are assuming.

First, for some mechanism of deployment (IIRC Apache's mod_perl) changes
to environment variables from CGI script are not passed to invoked
commands (I guess for security reasons).  That is why gitweb uses --git-dir
parameter to git wrapper, and not GIT_DIR environment variable since
25691fb (gitweb: Use --git-dir parameter instead of setting $ENV{'GIT_DIR'},
2006-08-28).  So for proper support we would need --notes-ref (or similar)
option to git wrapper

  git --notes-ref=$note_ref show -s --format=%N $co{'id'}


Second, parse_commit / parse_commits use

  git rev-list -z --parents --header --max-count-X

If this command automatically shows notes (or it can be modified to
automatically show notes) after unindented "Notes:" line (as per
git-notes documentation), then the only thing that needs to be
changed to fill %commit{'notes'} is parse_commit_text subroutine.

There would be no need for extra subroutine (and extra command, or
even up to two extra commands per note) to get notes data.

-- 
Jakub Narebski
Poland