Skip to contentSkip to conversation

gitweb: Bulk load commit objects

PATCH In review Robert Fitzsimons 2 participants 11 messages Jun 15, 2016

lore
Share & tools
lore

Robert Fitzsimons

author
#0 lore raw

cover gitweb: Bulk load commit objects

The following is a set of patchs which should improve the preformance of gitweb by bulk loading commit objects.

Note's:
* With a slight change in command line options and the parsing logic it was possible to bulk load the history action aswell.

* Even without the rev-list --skip option there should be some improvement as it is possible to bulk load the first page of commit object.

[PATCH 1/8] gitweb: Add parse_commits, used to bulk load commit objects. [PATCH 2/8] gitweb: We do longer need the --parents flag in rev-list.

Base support for bulk loading commit objects.

[PATCH 3/8] gitweb: Change summary, shortlog actions to use parse_commits. [PATCH 4/8] gitweb: Change log action to use parse_commits. [PATCH 5/8] gitweb: Change header search action to use parse_commits. [PATCH 6/8] gitweb: Change atom, rss actions to use parse_commits. [PATCH 7/8] gitweb: Change history action to use parse_commits.

Convert all the commit listing actions (except pickaxe search) to use parse_commits, also fixes up a few nav links.

[PATCH 8/8] gitweb: Use rev-list --skip option.

This change requires the rev-list --skip change from next to have been merged.

Robert

author
#1 lore raw

PATCH 1/8 gitweb: Add parse_commits, used to bulk load commit objects.

Add a new method parse_commits which is able to parse multiple commit objects at once. Reworked parse_commit to share the commit object parsing logic.

Signed-off-by Robert Fitzsimons <[email protected]>
---
 gitweb/gitweb.perl |   91 ++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 74 insertions(+), 17 deletions(-)
gitweb/gitweb.perl +74 -17
index d01d689..6bd57a4 100755
@@ -1270,25 +1270,13 @@ sub parse_tag {
 	return %tag
 }
 
-sub parse_commit {
-	my $commit_id = shift;
-	my $commit_text = shift;
-
-	my @commit_lines;
+sub parse_commit_text {
+	my ($commit_text) = @_;
+	my @commit_lines = split '\n', $commit_text;
 	my %co;
 
-	if (defined $commit_text) {
-		@commit_lines = @$commit_text;
-	} else {
-		local $/ = "\0";
-		open my $fd, "-|", git_cmd(), "rev-list",
-			"--header", "--parents", "--max-count=1",
-			$commit_id, "--"
-			or return;
-		@commit_lines = split '\n', <$fd>;
-		close $fd or return;
-		pop @commit_lines;
-	}
+	pop @commit_lines; # Remove '\0'
+
 	my $header = shift @commit_lines;
 	if (!($header =~ m/^[0-9a-fA-F]{40}/)) {
 		return;

@@ -1375,6 +1363,75 @@ sub parse_commit {
 	return %co;
 }
 
+sub parse_commit {
+	my ($commit_id) = @_;
+	my %co;
+
+	local $/ = "\0";
+
+	open my $fd, "-|", git_cmd(), "rev-list",
+		"--header",
+		"--parents",
+		"--max-count=1",
+		$commit_id,
+		"--",
+		or die_error(undef, "Open git-rev-list failed");
+	%co = parse_commit_text(<$fd>);
+	close $fd;
+
+	return %co;
+}
+
+sub parse_commits {
+	my ($commit_id, $maxcount, $skip, $arg, $filename) = @_;
+	my @cos;
+
+	$maxcount ||= 1;
+	$skip ||= 0;
+
+	# Delete once rev-list supports the --skip option
+	if ($skip > 0) {
+		open my $fd, "-|", git_cmd(), "rev-list",
+			($arg ? ($arg) : ()),
+			("--max-count=" . ($maxcount + $skip)),
+			$commit_id,
+			"--",
+			($filename ? ($filename) : ())
+			or die_error(undef, "Open git-rev-list failed");
+		while (my $line = <$fd>) {
+			if ($skip-- <= 0) {
+				chomp $line;
+				my %co = parse_commit($line);
+				push @cos, \%co;
+			}
+		}
+		close $fd;
+
+		return wantarray ? @cos : \@cos;
+	}
+
+	local $/ = "\0";
+
+	open my $fd, "-|", git_cmd(), "rev-list",
+		"--header",
+		"--parents",
+		($arg ? ($arg) : ()),
+		("--max-count=" . $maxcount),
+		# Add once rev-list supports the --skip option
+		# ("--skip=" . $skip),
+		$commit_id,
+		"--",
+		($filename ? ($filename) : ())
+		or die_error(undef, "Open git-rev-list failed");
+	while (my $line = <$fd>) {
+		my %co = parse_commit_text($line);
+		push @cos, \%co;
+	}
+	close $fd;
+
+	return wantarray ? @cos : \@cos;
+}
+
 # parse ref from ref_file, given by ref_id, with given type
 sub parse_ref {
 	my $ref_file = shift;
Signature · 1 line
1.4.4.3.ge655-dirty
author
#2 lore raw

PATCH 2/8 gitweb: We do longer need the --parents flag in rev-list.

We only want to know the direct parents of a given commit object, these parents are available in the --header output of rev-list. If --parents is supplied with --full-history the output includes merge commits that aren't relevant.

Signed-off-by Robert Fitzsimons <[email protected]>
---
 gitweb/gitweb.perl |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)
gitweb/gitweb.perl +6 -5
index 6bd57a4..c645686 100755
@@ -1281,13 +1281,14 @@ sub parse_commit_text {
 	if (!($header =~ m/^[0-9a-fA-F]{40}/)) {
 		return;
 	}
-	($co{'id'}, my @parents) = split ' ', $header;
-	$co{'parents'} = \@parents;
-	$co{'parent'} = $parents[0];
+	$co{'id'} = $header;
+	my @parents;
 	while (my $line = shift @commit_lines) {
 		last if $line eq "\n";
 		if ($line =~ m/^tree ([0-9a-fA-F]{40})$/) {
 			$co{'tree'} = $1;
+		} elsif ($line =~ m/^parent ([0-9a-fA-F]{40})$/) {
+			push @parents, $1;
 		} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {
 			$co{'author'} = $1;
 			$co{'author_epoch'} = $2;

@@ -1314,6 +1315,8 @@ sub parse_commit_text {
 	if (!defined $co{'tree'}) {
 		return;
 	};
+	$co{'parents'} = \@parents;
+	$co{'parent'} = $parents[0];
 
 	foreach my $title (@commit_lines) {
 		$title =~ s/^    //;

@@ -1371,7 +1374,6 @@ sub parse_commit {
 
 	open my $fd, "-|", git_cmd(), "rev-list",
 		"--header",
-		"--parents",
 		"--max-count=1",
 		$commit_id,
 		"--",

@@ -1414,7 +1416,6 @@ sub parse_commits {
 
 	open my $fd, "-|", git_cmd(), "rev-list",
 		"--header",
-		"--parents",
 		($arg ? ($arg) : ()),
 		("--max-count=" . $maxcount),
 		# Add once rev-list supports the --skip option
Signature · 1 line
1.4.4.3.ge655-dirty
author
#3 lore raw

PATCH 3/8 gitweb: Change summary, shortlog actions to use parse_commits.

Also added missing accesskey.

Signed-off-by Robert Fitzsimons <[email protected]>
---
 gitweb/gitweb.perl |   34 ++++++++++++----------------------
 1 files changed, 12 insertions(+), 22 deletions(-)
gitweb/gitweb.perl +12 -22
index c645686..5f1ace9 100755
@@ -2704,20 +2704,19 @@ sub git_project_list_body {
 
 sub git_shortlog_body {
 	# uses global variable $project
-	my ($revlist, $from, $to, $refs, $extra) = @_;
+	my ($commitlist, $from, $to, $refs, $extra) = @_;
 
 	my $have_snapshot = gitweb_have_snapshot();
 
 	$from = 0 unless defined $from;
-	$to = $#{$revlist} if (!defined $to || $#{$revlist} < $to);
+	$to = $#{$commitlist} if (!defined $to || $#{$commitlist} < $to);
 
 	print "<table class=\"shortlog\" cellspacing=\"0\">\n";
 	my $alternate = 1;
 	for (my $i = $from; $i <= $to; $i++) {
-		my $commit = $revlist->[$i];
-		#my $ref = defined $refs ? format_ref_marker($refs, $commit) : '';
+		my %co = %{$commitlist->[$i]};
+		my $commit = $co{'id'};
 		my $ref = format_ref_marker($refs, $commit);
-		my %co = parse_commit($commit);
 		if ($alternate) {
 			print "<tr class=\"dark\">\n";
 		} else {

@@ -3081,14 +3080,10 @@ sub git_summary {
 
 	# we need to request one more than 16 (0..15) to check if
 	# those 16 are all
-	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=17",
-		$head, "--"
-		or die_error(undef, "Open git-rev-list failed");
-	my @revlist = map { chomp; $_ } <$fd>;
-	close $fd;
+	my @commitlist = parse_commits($head, 17);
 	git_print_header_div('shortlog');
-	git_shortlog_body(\@revlist, 0, 15, $refs,
-	                  $#revlist <=  15 ? undef :
+	git_shortlog_body(\@commitlist, 0, 15, $refs,
+	                  $#commitlist <=  15 ? undef :
 	                  $cgi->a({-href => href(action=>"shortlog")}, "..."));
 
 	if (@taglist) {

@@ -4456,26 +4451,21 @@ sub git_shortlog {
 	}
 	my $refs = git_get_references();
 
-	my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
-	open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash, "--"
-		or die_error(undef, "Open git-rev-list failed");
-	my @revlist = map { chomp; $_ } <$fd>;
-	close $fd;
+	my @commitlist = parse_commits($head, 101, (100 * $page));
 
-	my $paging_nav = format_paging_nav('shortlog', $hash, $head, $page, $#revlist);
+	my $paging_nav = format_paging_nav('shortlog', $hash, $head, $page, (100 * ($page+1)));
 	my $next_link = '';
-	if ($#revlist >= (100 * ($page+1)-1)) {
+	if ($#commitlist >= 100) {
 		$next_link =
 			$cgi->a({-href => href(action=>"shortlog", hash=>$hash, page=>$page+1),
-			         -title => "Alt-n"}, "next");
+			         -accesskey => "n", -title => "Alt-n"}, "next");
 	}
 
-
 	git_header_html();
 	git_print_page_nav('shortlog','', $hash,$hash,$hash, $paging_nav);
 	git_print_header_div('summary', $project);
 
-	git_shortlog_body(\@revlist, ($page * 100), $#revlist, $refs, $next_link);
+	git_shortlog_body(\@commitlist, 0, 99, $refs, $next_link);
 
 	git_footer_html();
 }
Signature · 1 line
1.4.4.3.ge655-dirty
author
#4 lore raw

PATCH 4/8 gitweb: Change log action to use parse_commits.

Also add missing next link to bottom of page.

Signed-off-by Robert Fitzsimons <[email protected]>
---
 gitweb/gitweb.perl |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)
gitweb/gitweb.perl +14 -11
index 5f1ace9..42b7449 100755
@@ -3645,28 +3645,25 @@ sub git_log {
 	}
 	my $refs = git_get_references();
 
-	my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
-	open my $fd, "-|", git_cmd(), "rev-list", $limit, $hash, "--"
-		or die_error(undef, "Open git-rev-list failed");
-	my @revlist = map { chomp; $_ } <$fd>;
-	close $fd;
+	my @commitlist = parse_commits($hash, 101, (100 * $page));
 
-	my $paging_nav = format_paging_nav('log', $hash, $head, $page, $#revlist);
+	my $paging_nav = format_paging_nav('log', $hash, $head, $page, (100 * ($page+1)));
 
 	git_header_html();
 	git_print_page_nav('log','', $hash,undef,undef, $paging_nav);
 
-	if (!@revlist) {
+	if (!@commitlist) {
 		my %co = parse_commit($hash);
 
 		git_print_header_div('summary', $project);
 		print "<div class=\"page_body\"> Last change $co{'age_string'}.<br/><br/></div>\n";
 	}
-	for (my $i = ($page * 100); $i <= $#revlist; $i++) {
-		my $commit = $revlist[$i];
-		my $ref = format_ref_marker($refs, $commit);
-		my %co = parse_commit($commit);
+	my $to = ($#commitlist >= 99) ? (99) : ($#commitlist);
+	for (my $i = 0; $i <= $to; $i++) {
+		my %co = %{$commitlist[$i]};
 		next if !%co;
+		my $commit = $co{'id'};
+		my $ref = format_ref_marker($refs, $commit);
 		my %ad = parse_date($co{'author_epoch'});
 		git_print_header_div('commit',
 		               "<span class=\"age\">$co{'age_string'}</span>" .

@@ -3688,6 +3685,12 @@ sub git_log {
 		git_print_log($co{'comment'}, -final_empty_line=> 1);
 		print "</div>\n";
 	}
+	if ($#commitlist >= 100) {
+		print "<div class=\"page_nav\">\n";
+		print $cgi->a({-href => href(action=>"log", hash=>$hash, page=>$page+1),
+			       -accesskey => "n", -title => "Alt-n"}, "next");
+		print "</div>\n";
+	}
 	git_footer_html();
 }
Signature · 1 line
1.4.4.3.ge655-dirty
author
#5 lore raw

PATCH 5/8 gitweb: Change header search action to use parse_commits.

Signed-off-by Robert Fitzsimons <[email protected]>
---
 gitweb/gitweb.perl |   23 +++++++++--------------
 1 files changed, 9 insertions(+), 14 deletions(-)
gitweb/gitweb.perl +9 -14
index 42b7449..53dd225 100755
@@ -2910,18 +2910,18 @@ sub git_heads_body {
 }
 
 sub git_search_grep_body {
-	my ($greplist, $from, $to, $extra) = @_;
+	my ($commitlist, $from, $to, $extra) = @_;
 	$from = 0 unless defined $from;
-	$to = $#{$greplist} if (!defined $to || $#{$greplist} < $to);
+	$to = $#{$commitlist} if (!defined $to || $#{$commitlist} < $to);
 
 	print "<table class=\"grep\" cellspacing=\"0\">\n";
 	my $alternate = 1;
 	for (my $i = $from; $i <= $to; $i++) {
-		my $commit = $greplist->[$i];
-		my %co = parse_commit($commit);
+		my %co = %{$commitlist->[$i]};
 		if (!%co) {
 			next;
 		}
+		my $commit = $co{'id'};
 		if ($alternate) {
 			print "<tr class=\"dark\">\n";
 		} else {

@@ -4307,13 +4307,8 @@ sub git_search {
 		} elsif ($searchtype eq 'committer') {
 			$greptype = "--committer=";
 		}
-		open my $fd, "-|", git_cmd(), "rev-list",
-			("--max-count=" . (100 * ($page+1))),
-			($greptype . $searchtext),
-			$hash, "--"
-			or next;
-		my @revlist = map { chomp; $_ } <$fd>;
-		close $fd;
+		$greptype .= $searchtext;
+		my @commitlist = parse_commits($hash, 101, (100 * $page), $greptype);
 
 		my $paging_nav = '';
 		if ($page > 0) {

@@ -4330,7 +4325,7 @@ sub git_search {
 			$paging_nav .= "first";
 			$paging_nav .= " &sdot; prev";
 		}
-		if ($#revlist >= (100 * ($page+1)-1)) {
+		if ($#commitlist >= 100) {
 			$paging_nav .= " &sdot; " .
 				$cgi->a({-href => href(action=>"search", hash=>$hash,
 						       searchtext=>$searchtext, searchtype=>$searchtype,

@@ -4340,7 +4335,7 @@ sub git_search {
 			$paging_nav .= " &sdot; next";
 		}
 		my $next_link = '';
-		if ($#revlist >= (100 * ($page+1)-1)) {
+		if ($#commitlist >= 100) {
 			$next_link =
 				$cgi->a({-href => href(action=>"search", hash=>$hash,
 						       searchtext=>$searchtext, searchtype=>$searchtype,

@@ -4350,7 +4345,7 @@ sub git_search {
 
 		git_print_page_nav('','', $hash,$co{'tree'},$hash, $paging_nav);
 		git_print_header_div('commit', esc_html($co{'title'}), $hash);
-		git_search_grep_body(\@revlist, ($page * 100), $#revlist, $next_link);
+		git_search_grep_body(\@commitlist, 0, 99, $next_link);
 	}
 
 	if ($searchtype eq 'pickaxe') {
Signature · 1 line
1.4.4.3.ge655-dirty
author
#6 lore raw

PATCH 6/8 gitweb: Change atom, rss actions to use parse_commits.

Signed-off-by Robert Fitzsimons <[email protected]>
---
 gitweb/gitweb.perl |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)
gitweb/gitweb.perl +7 -11
index 53dd225..f752a6f 100755
@@ -4483,11 +4483,7 @@ sub git_feed {
 
 	# log/feed of current (HEAD) branch, log of given branch, history of file/directory
 	my $head = $hash || 'HEAD';
-	open my $fd, "-|", git_cmd(), "rev-list", "--max-count=150",
-		$head, "--", (defined $file_name ? $file_name : ())
-		or die_error(undef, "Open git-rev-list failed");
-	my @revlist = map { chomp; $_ } <$fd>;
-	close $fd or die_error(undef, "Reading git-rev-list failed");
+	my @commitlist = parse_commits($head, 150);
 
 	my %latest_commit;
 	my %latest_date;

@@ -4497,8 +4493,8 @@ sub git_feed {
 		# browser (feed reader) prefers text/xml
 		$content_type = 'text/xml';
 	}
-	if (defined($revlist[0])) {
-		%latest_commit = parse_commit($revlist[0]);
+	if (defined($commitlist[0])) {
+		%latest_commit = %{$commitlist[0]};
 		%latest_date   = parse_date($latest_commit{'author_epoch'});
 		print $cgi->header(
 			-type => $content_type,

@@ -4588,9 +4584,9 @@ XML
 	}
 
 	# contents
-	for (my $i = 0; $i <= $#revlist; $i++) {
-		my $commit = $revlist[$i];
-		my %co = parse_commit($commit);
+	for (my $i = 0; $i <= $#commitlist; $i++) {
+		my %co = %{$commitlist[$i]};
+		my $commit = $co{'id'};
 		# we read 150, we always show 30 and the ones more recent than 48 hours
 		if (($i >= 20) && ((time - $co{'author_epoch'}) > 48*60*60)) {
 			last;

@@ -4598,7 +4594,7 @@ XML
 		my %cd = parse_date($co{'author_epoch'});
 
 		# get list of changed files
-		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
+		open my $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
 			$co{'parent'}, $co{'id'}, "--", (defined $file_name ? $file_name : ())
 			or next;
 		my @difftree = map { chomp; $_ } <$fd>;
Signature · 1 line
1.4.4.3.ge655-dirty
author
#7 lore raw

PATCH 7/8 gitweb: Change history action to use parse_commits.

Also added missing accesskey.

Signed-off-by Robert Fitzsimons <[email protected]>
---
 gitweb/gitweb.perl |   27 +++++++++------------------
 1 files changed, 9 insertions(+), 18 deletions(-)
gitweb/gitweb.perl +9 -18
index f752a6f..f9994d9 100755
@@ -2750,23 +2750,19 @@ sub git_shortlog_body {
 
 sub git_history_body {
 	# Warning: assumes constant type (blob or tree) during history
-	my ($revlist, $from, $to, $refs, $hash_base, $ftype, $extra) = @_;
+	my ($commitlist, $from, $to, $refs, $hash_base, $ftype, $extra) = @_;
 
 	$from = 0 unless defined $from;
-	$to = $#{$revlist} unless (defined $to && $to <= $#{$revlist});
+	$to = $#{$commitlist} unless (defined $to && $to <= $#{$commitlist});
 
 	print "<table class=\"history\" cellspacing=\"0\">\n";
 	my $alternate = 1;
 	for (my $i = $from; $i <= $to; $i++) {
-		if ($revlist->[$i] !~ m/^([0-9a-fA-F]{40})/) {
-			next;
-		}
-
-		my $commit = $1;
-		my %co = parse_commit($commit);
+		my %co = %{$commitlist->[$i]};
 		if (!%co) {
 			next;
 		}
+		my $commit = $co{'id'};
 
 		my $ref = format_ref_marker($refs, $commit);
 

@@ -4219,12 +4215,7 @@ sub git_history {
 		$ftype = git_get_type($hash);
 	}
 
-	open my $fd, "-|",
-		git_cmd(), "rev-list", $limit, "--full-history", $hash_base, "--", $file_name
-			or die_error(undef, "Open git-rev-list-failed");
-	my @revlist = map { chomp; $_ } <$fd>;
-	close $fd
-		or die_error(undef, "Reading git-rev-list failed");
+	my @commitlist = parse_commits($hash_base, 101, (100 * $page), "--full-history", $file_name);
 
 	my $paging_nav = '';
 	if ($page > 0) {

@@ -4240,7 +4231,7 @@ sub git_history {
 		$paging_nav .= "first";
 		$paging_nav .= " &sdot; prev";
 	}
-	if ($#revlist >= (100 * ($page+1)-1)) {
+	if ($#commitlist >= 100) {
 		$paging_nav .= " &sdot; " .
 			$cgi->a({-href => href(action=>"history", hash=>$hash, hash_base=>$hash_base,
 			                       file_name=>$file_name, page=>$page+1),

@@ -4249,11 +4240,11 @@ sub git_history {
 		$paging_nav .= " &sdot; next";
 	}
 	my $next_link = '';
-	if ($#revlist >= (100 * ($page+1)-1)) {
+	if ($#commitlist >= 100) {
 		$next_link =
 			$cgi->a({-href => href(action=>"history", hash=>$hash, hash_base=>$hash_base,
 			                       file_name=>$file_name, page=>$page+1),
-			         -title => "Alt-n"}, "next");
+			         -accesskey => "n", -title => "Alt-n"}, "next");
 	}
 
 	git_header_html();

@@ -4261,7 +4252,7 @@ sub git_history {
 	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
 	git_print_page_path($file_name, $ftype, $hash_base);
 
-	git_history_body(\@revlist, ($page * 100), $#revlist,
+	git_history_body(\@commitlist, 0, 99,
 	                 $refs, $hash_base, $ftype, $next_link);
 
 	git_footer_html();
Signature · 1 line
1.4.4.3.ge655-dirty
author
#8 lore raw

PATCH 8/8 gitweb: Use rev-list --skip option.

Signed-off-by Robert Fitzsimons <[email protected]>
---
 gitweb/gitweb.perl |   24 +-----------------------
 1 files changed, 1 insertions(+), 23 deletions(-)
gitweb/gitweb.perl +1 -23
index f9994d9..65fcdb0 100755
@@ -1391,35 +1391,13 @@ sub parse_commits {
 	$maxcount ||= 1;
 	$skip ||= 0;
 
-	# Delete once rev-list supports the --skip option
-	if ($skip > 0) {
-		open my $fd, "-|", git_cmd(), "rev-list",
-			($arg ? ($arg) : ()),
-			("--max-count=" . ($maxcount + $skip)),
-			$commit_id,
-			"--",
-			($filename ? ($filename) : ())
-			or die_error(undef, "Open git-rev-list failed");
-		while (my $line = <$fd>) {
-			if ($skip-- <= 0) {
-				chomp $line;
-				my %co = parse_commit($line);
-				push @cos, \%co;
-			}
-		}
-		close $fd;
-
-		return wantarray ? @cos : \@cos;
-	}
-
 	local $/ = "\0";
 
 	open my $fd, "-|", git_cmd(), "rev-list",
 		"--header",
 		($arg ? ($arg) : ()),
 		("--max-count=" . $maxcount),
-		# Add once rev-list supports the --skip option
-		# ("--skip=" . $skip),
+		("--skip=" . $skip),
 		$commit_id,
 		"--",
 		($filename ? ($filename) : ())
Signature · 1 line
1.4.4.3.ge655-dirty
#9 lore raw

Re: [PATCH 2/8] gitweb: We do longer need the --parents flag in rev-list.

In reply to Robert Fitzsimons
Quoted 4 lines · Robert Fitzsimons

We only want to know the direct parents of a given commit object, these parents are available in the --header output of rev-list.  If --parents is supplied with --full-history the output includes merge commits that aren't relevant.

Actually --header output gives us original parents. Rewritten parents (available with --parents) include also grafts and shallow clone grafts. For parse_commit we want --parents, for parse_commits we don't want it because --parents affects --full-history.

The problem is that we cannot detect if git-rev-list was called with --parents and commit is root commit (parentless), or we didn't use --parents option.

In few other places we pass options specifying subroutine behavior as hash after all other requred parameters, e.g.
esc_html($line, -nbsp=>1),
parse_ls_tree_line($line, -z=>1), git_print_log($co{'comment'}, -final_empty_line=> 1, -remove_title => 1);
In this case it wouldn't work (unless we pass reference to array, via parse_commit_text( [ <$fd> ], -parents=>1);

Perhaps it would be better to use reference to hash of options as _first_ parameter, e.g. parse_commit_text({-parents=>1}, <$fd>);, and use something like if (ref($[0]) == 'HASH') { $opts = shift @_; } to get options.

So for now gitweb might not show what we want in very rare cases of repositories with grafts or shallow clones.

But apart from this small matter, this series is excellent work. Thanks!

Signature · 3 lines
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
author
#10 lore raw

PATCH 1/1 gitweb: Re-enable rev-list --parents for parse_commit.

Re-enable rev-list --parents for parse_commit which was removed in (208b2dff95bb48682c351099023a1cbb0e1edf26). rev-list --parents is not just used to return the parent headers in the commit object, it includes any grafts which are vaild for the commit.

Signed-off-by Robert Fitzsimons <[email protected]>

---

Quoted 4 lines

Actually --header output gives us original parents. Rewritten parents (available with --parents) include also grafts and shallow clone grafts. For parse_commit we want --parents, for parse_commits we don't want it because --parents affects --full-history.

Heres a patch the re-enables --parents for parse_commit.

Robert

 gitweb/gitweb.perl |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
gitweb/gitweb.perl +5 -5
index 65fcdb0..da12be7 100755
@@ -1271,7 +1271,7 @@ sub parse_tag {
 }
 
 sub parse_commit_text {
-	my ($commit_text) = @_;
+	my ($commit_text, $withparents) = @_;
 	my @commit_lines = split '\n', $commit_text;
 	my %co;
 

@@ -1281,13 +1281,12 @@ sub parse_commit_text {
 	if (!($header =~ m/^[0-9a-fA-F]{40}/)) {
 		return;
 	}
-	$co{'id'} = $header;
-	my @parents;
+	($co{'id'}, my @parents) = split ' ', $header;
 	while (my $line = shift @commit_lines) {
 		last if $line eq "\n";
 		if ($line =~ m/^tree ([0-9a-fA-F]{40})$/) {
 			$co{'tree'} = $1;
-		} elsif ($line =~ m/^parent ([0-9a-fA-F]{40})$/) {
+		} elsif ((!defined $withparents) && ($line =~ m/^parent ([0-9a-fA-F]{40})$/)) {
 			push @parents, $1;
 		} elsif ($line =~ m/^author (.*) ([0-9]+) (.*)$/) {
 			$co{'author'} = $1;

@@ -1373,12 +1372,13 @@ sub parse_commit {
 	local $/ = "\0";
 
 	open my $fd, "-|", git_cmd(), "rev-list",
+		"--parents",
 		"--header",
 		"--max-count=1",
 		$commit_id,
 		"--",
 		or die_error(undef, "Open git-rev-list failed");
-	%co = parse_commit_text(<$fd>);
+	%co = parse_commit_text(<$fd>, 1);
 	close $fd;
 
 	return %co;
Signature · 1 line
1.4.4.3.g6934