On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:
> 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.
Well, fnmatch is what I think git uses for <pattern> e.g. for
git-for-each-ref.
> Colon-separated, fnmatched components is probably the easiest thing to
> implement to have multiple refs. I'll go with whatever is chosen for
> core.
I think that having actual list of patterns in $feature{'notes'}{'default'}
might be more clear; you would still need colon separated (or space
separated) list of patterns in per-repo override in gitweb.notes config
variable.
So it would be
$feature{'notes'}{'default'} = ['commits', '*svn*'];
$feature{'notes'}{'override'} = 1;
but
[gitweb]
notes = commits:*svn*
Note that refs names cannot contain either colon ':' or space ' '
(see git-check-ref-format).
>>>> 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.
On the third hand ;-P you propose below a trick to deal with fan-out
schemes, assuming that they use 2-character component breaking.
Also, perhaps "git notes show" should acquire --batch / --batch-check
options, similar to git-cat-file's options of the same name?
>
>>>> 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.
Right. This method would be contrary to the goals of fan-out schemes...
well, we could use 'git ls-tree' without '-r' option, or simply
'git cat-file --batch' to read trees (note that we would get raw,
unformatted tree, which is parseable with Perl, but it is not that easy),
and go down level-by-level.
>
> 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.
Nice trick! It seems like quite a good idea... but it would absolutely
require using 'git cat-file --batch' rather than one git-show per try.
>> 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
Errr, I not made myself clear.
I have added a note to a commit, using "git notes edit d6bbe7f". Now if
you take a look at gitweb output for this commit (e.g. 'commit' view for
this commit) using gitweb without your changes, you would see that it
flattened notes at the bottom of the commit message (which I think is
intended result by notes implementation).
If you run the command that parse_commit runs, namely
$ git rev-list --parents --header -z --max-count=1 \
d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3
you would get (up to invisible NUL characters) the output shown above.
>From this output I would like to separate commit message from notes
in parse_commit_text subroutine.
I have set neither GIT_NOTES_REF nor core.notesRef.
--
Jakub Narebski
Poland
--
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