On Sat, Feb 6, 2010 at 11:14 PM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:
>> 2010/2/6 Jakub Narebski <jnareb@xxxxxxxxx>:
>>> 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?
Sort of. fnmatch doesn't do brace expansion, which is a pity IMO, but
that's just my personal preference. Colon-separated, fnmatched
components is probably the easiest thing to implement to have multiple
refs. I'll go with whatever is chosen for core.
>>> 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...
On the other hand, as mentioned by Junio, this approach is not
future-proof enough for any kind of fan-out schemes.
>>> 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).
What worries me is that you're going to get fan-outs when there are
LOTS of notes, and that's precisely the kind of situation where you
_don't_ want to go through all the notes to pick the ones you're only
interested in.
If we have a guarantee that the fan-outs follow a 2/[2/...] scheme,
the open2 approach might still be the best way to go, by just trying
not only namespace:xxxxx...xxx but also namespace:xx/xxxxx etc.
Horrible, but could still be coalesced in a single call. It mgiht also
be optimized to stop at the first successfull hit in a namespace.
> 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@xxxxxxxxx> 1265309578 +0100
> committer Jakub Narebski <jnareb@xxxxxxxxx> 1265309578 +0100
>
> This is a commit message
>
> Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx>
>
> 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'}).
I'm not getting these in the repo I'm testing this. And I think this
is indeed the behavior of current git next
--
Giuseppe "Oblomov" Bilotta
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html