Re: [PATCH 1/4] gitweb: notes feature
From: Jakub Narebski <hidden>
Date: 2016-06-15 22:48:11
On Thu, 4 Feb 2010, Giuseppe Bilotta wrote: BTW. shouldn't this series be marked as RFC?
Introduce support for notes by collecting them when creating commit lists. The list of noterefs to look into is configurable, and can be a(n array of) refspec(s), which will be looked for in the refs/notes namespace. The feature is disabled by default because it's presently not very efficient (one extra git call per configured refspec, plus two extra git calls per commit per noteref).
Signoff?
quoted hunk ↗ jump to hunk
--- gitweb/gitweb.perl | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 62 insertions(+), 0 deletions(-)diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index d0c3ff2..9ba5815 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl@@ -411,6 +411,22 @@ our %feature = ( 'override' => 0, 'default' => [16]}, + # 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).
+
+ # To enable system wide have in $GITWEB_CONFIG
+ # $feature{'notes'}{'default'} = ['*'];
+ # To have project specific config enable override in $GITWEB_CONFIG
+ # $feature{'notes'}{'override'} = 1;
+ # and in project config gitweb.notes = namespace;How you can provide list of notes here? Is overriding limited to single name or shell-glob? See feature_snapshot for example implementation.
quoted hunk ↗ jump to hunk
+ 'notes' => { + 'sub' => \&feature_notes, + 'override' => 0, + 'default' => []}, + # Avatar support. When this feature is enabled, views such as # shortlog or commit will display an avatar associated with # the email of the committer(s) and/or author(s).@@ -513,6 +529,16 @@ sub feature_patches { return ($_[0]); } +sub feature_notes { + my @val = (git_get_project_config('notes')); + + if (@val) { + return @val; + } + + return @_; +}
First, this I think limits overriding in repository config to single value. Second, perhaps it is time to refactor all those similar feature_xxx subroutines (just a possible suggestion)?
quoted hunk ↗ jump to hunk
+ sub feature_avatar { my @val = (git_get_project_config('avatar'));@@ -2786,10 +2812,30 @@ sub parse_commit { return %co; } +# return all refs matching refs/notes/<globspecs> where the globspecs +# are taken from the notes feature content. +sub get_note_refs { + my @globs = gitweb_get_feature('notes'); + my @note_refs = (); + foreach my $glob (@globs) { + if (open my $fd, '-|', git_cmd(), 'for-each-ref', + '--format=%(refname)', "refs/notes/$glob") {
open my $fd, '-|', git_cmd(), 'for-each-ref', '--format=%(refname)', "refs/notes/$glob" or return; would reduce indent level a bit.
+ while (<$fd>) {
+ chomp;
+ push @note_refs, $_ if $_;
+ }Why not simply chomp(@note_refs = <$fd>);
quoted hunk ↗ jump to hunk
+ close $fd; + } + } + return @note_refs; +} + sub parse_commits { my ($commit_id, $maxcount, $skip, $filename, @args) = @_; my @cos; + my @note_refs = get_note_refs(); + $maxcount ||= 1; $skip ||= 0;@@ -2807,6 +2853,22 @@ sub parse_commits { or die_error(500, "Open git-rev-list failed"); while (my $line = <$fd>) { my %co = parse_commit_text($line); + 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). 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.
+ $co{'notes'} = \%notes;
push @cos, \%co;
}
close $fd;
--
1.7.0.rc1.193.ge8618
-- Jakub Narebski Poland