Thread (32 messages) flat view 32 messages, 4 authors, 2016-06-15

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help