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

[Jakub Narebski <jnareb@gmail.com>]

On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:
> 2010/2/6 Jakub Narebski <jnareb@gmail.com>:
>> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:

>>> +     # Notes support. When this feature is enabled, the presence of notes
>>> +     # for any commit is signaled, and the note content is made available
>>> +     # in a way appropriate for the current view.
>>> +     # Set this to '*' to enable all notes namespace, or to a shell-glob
>>> +     # specification to enable specific namespaces only.
>>
>> It is not obvious from this description that you can provide _list_ of
>> notes namespaces (or list of shell-globs).
> 
> I'm starting to think it might make sense to not have a list here, but
> rather a single value only. First of all, multiple refs can be
> indicated à la shell with {ref1,ref2,ref3}. Or, we can also use the
> intended command-line syntax ref1:ref2:ref3, which would help
> consistency. It also makes things easier for project overrides, as per
> your subsequent comment:

So it is to be single shell-glob / fnmatch (I think) compatible pattern,
isn't it?
 
[...]
>>> +             my %notes = () ;
>>> +             foreach my $note_ref (@note_refs) {
>>> +                     my $obj = "$note_ref:$co{'id'}";
>>> +                     if (open my $fd, '-|', git_cmd(), 'rev-parse',
>>> +                             '--verify', '-q', $obj) {
>>> +                             my $exists = <$fd>;
>>> +                             close $fd;
>>> +                             if (defined $exists) {
>>> +                                     if (open $fd, '-|', git_cmd(), 'show', $obj) {
>>> +                                             $notes{$note_ref} = scalar <$fd>;
>>> +                                             close $fd;
>>> +                                     }
>>> +                             }
>>> +                     }
>>> +             }
>>
>> First, there are '--batch' and '--batch-check' options to git-cat-file.
>> With these I think you can get all notes with just single git command,
>> although using it is a bit complicated (requires open2 from IPC::Open2
>> for bidi communication).
> 
> Hm. The IPC::Open2 doc makes it sound horribly scary, but still doable.

It would look something like the following (fragment of my WIP code):

	use IPC::Open2 qw(open2);
	use IO::Handle;

	# ...

	unless ($object_stdout) {
		# Open bidi pipe the first time get_object is called.
		# open2 raises an exception on error, no need to 'or die'.
		$object_pid =
			open2($object_stdout, $object_stdin,
			      git_cmd(), 'cat-file', '--batch');
	}
	$object_stdin->printflush("$object_id\n") # NB: \n required to avoid deadlocking
		or die "get_object: cannot write to pipe: $!";
	my ($sha1, $type, $size) =
		split ' ', $object_stdout->getline()
		or die "get_object: cannot read from pipe: $!";
	die "'$object_id' not found in repository"
		if $type eq 'missing';
	$object_stdout->read(my $content, $size);
	$object_stdout->getline();  # eat trailing newline
 
The above fragment of code is tested that it works.  You would probably
need to replace dies with something less fatal...

>> Second, if not using 'git cat-file --batch', perhaps it would be easier
>> to read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and
>> parse its output to check for which commits/objects there are notes
>> available, and only then call 'git show' (or reuse connection to
>> 'git cat-file --batch').
>>
>> The second solution, with a bit more work, could work even in presence
>> of fan-out schemes for notes, I think.
> 
> An interesting approach. Without fan-out, git ls-tree -r
> refs/notes/whatever [hash ...] gives us the blobs we're interested in.
> In case of fan-out schemes, the efficiency of this approach probably
> depends on the kind of fan-out we have, and would require some
> heavy-duty grepping. A git ls-notes plumbing with a similar syntax and
> output would be a nice thing to have.

No grepping, just pass '-r' option to 'git-ls-tree', and use
parse_ls_tree_line() to parse lines.  Then if we have fanout scheme
we would get, I guess, something like the following:

  100644 blob 23da787d...	de/adbeef...
  100644 blob bc10f25f...	c5/31d986...
  100644 blob c9656ece...	24/d93129...

Now you only need to s!/!!g on filename to get SHA1 of annotated object
(for which is the note).

The identifier of note itself would be either id from tree (e.g. 23da787d...
for note from first line), or note namespace composed with note "pathname",
(e.g. refs/notes/commits:de/adbeef... for note from first line).

Even if you use one git-show per note, it would need only git commands
to discover object to note mapping.


P.S. We still would want parse_commit_text to parse notes from default
namespace.  parse_commit / parse_commits output contains notes from
default namespace, e.g.:

  d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6
  tree b9ee8876df81b80b13c6b017be993fff8427cfaf
  parent 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6
  author Jakub Narebski <jnareb@gmail.com> 1265309578 +0100
  committer Jakub Narebski <jnareb@gmail.com> 1265309578 +0100

      This is a commit message
      
      Signed-off-by: Jakub Narebski <jnareb@gmail.com>
  
  Notes:
      This is just a note for commit d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3

to get commit message lines in $co{'comment'} (as array reference),
and notes in $co{'note'} (or $co{'notes'}).

-- 
Jakub Narebski
Poland