From c3288fb27efcca73ba87e27c2c2b41b4a1dfbd46 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 26 Jan 2017 04:27:02 +0000 Subject: repobrowse: simplify command generation for git commands This shortens the code quite a bit at a negligible performance cost, and the diffstat agrees. --- lib/PublicInbox/Git.pm | 10 +++++++--- lib/PublicInbox/RepobrowseGitAtom.pm | 9 ++++----- lib/PublicInbox/RepobrowseGitCommit.pm | 5 ++--- lib/PublicInbox/RepobrowseGitDiff.pm | 6 ++---- lib/PublicInbox/RepobrowseGitLog.pm | 11 ++--------- lib/PublicInbox/RepobrowseGitPatch.pm | 7 +++---- lib/PublicInbox/RepobrowseGitPlain.pm | 3 +-- lib/PublicInbox/RepobrowseGitSnapshot.pm | 9 ++++----- lib/PublicInbox/RepobrowseGitSummary.pm | 2 +- lib/PublicInbox/RepobrowseGitTree.pm | 3 +-- 10 files changed, 27 insertions(+), 38 deletions(-) diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm index 648aaaf0..caca3b09 100644 --- a/lib/PublicInbox/Git.pm +++ b/lib/PublicInbox/Git.pm @@ -50,6 +50,11 @@ sub err ($) { $buf; } +sub cmd { + my $self = shift; + [ 'git', "--git-dir=$self->{git_dir}", @_ ]; +} + sub _bidi_pipe { my ($self, $batch, $in, $out, $pid) = @_; return if $self->{$pid}; @@ -58,9 +63,8 @@ sub _bidi_pipe { pipe($in_r, $in_w) or fail($self, "pipe failed: $!"); pipe($out_r, $out_w) or fail($self, "pipe failed: $!"); - my @cmd = ('git', "--git-dir=$self->{git_dir}", qw(cat-file), $batch); my $redir = { 0 => fileno($out_r), 1 => fileno($in_w) }; - my $p = spawn(\@cmd, undef, $redir); + my $p = spawn(cmd($self, qw(cat-file), $batch), undef, $redir); defined $p or fail($self, "spawn failed: $!"); $self->{$pid} = $p; $out_w->autoflush(1); @@ -167,7 +171,7 @@ sub fail { sub popen { my ($self, @cmd) = @_; - my $cmd = [ 'git', "--git-dir=$self->{git_dir}" ]; + my $cmd = cmd($self); my ($env, $opt); if (ref $cmd[0]) { push @$cmd, @{$cmd[0]}; diff --git a/lib/PublicInbox/RepobrowseGitAtom.pm b/lib/PublicInbox/RepobrowseGitAtom.pm index 87fc60a7..c610d44d 100644 --- a/lib/PublicInbox/RepobrowseGitAtom.pm +++ b/lib/PublicInbox/RepobrowseGitAtom.pm @@ -145,11 +145,10 @@ sub call_git_atom { my $env = $req->{env}; my $q =$req->{'q'} = PublicInbox::RepobrowseGitQuery->new($env); my $h = $q->{h}; - my $git_dir = "--git-dir=$git->{git_dir}"; my $read_log = sub { - my $cmd = ['git', $git_dir, - qw(log --no-notes --no-color --abbrev-commit), - $git->abbrev, $ATOM_FMT, "-$max", $h, '--' ]; + my $cmd = $git->cmd(qw(log --no-notes --no-color + --abbrev-commit), $git->abbrev, + $ATOM_FMT, "-$max", $h, '--'); my $expath = $req->{expath}; push @$cmd, $expath if $expath ne ''; my $rdr = { 2 => $git->err_begin }; @@ -161,7 +160,7 @@ sub call_git_atom { $env->{'qspawn.response'} = $_[0]; return $read_log->() if $h ne ''; - my $cmd = [ 'git', $git_dir, qw(symbolic-ref --short HEAD) ]; + my $cmd = $git->cmd(qw(symbolic-ref --short HEAD)); my $rdr = { 2 => $git->err_begin }; my $qsp = PublicInbox::Qspawn->new($cmd, undef, undef, $rdr); $qsp->psgi_qx($env, undef, sub { diff --git a/lib/PublicInbox/RepobrowseGitCommit.pm b/lib/PublicInbox/RepobrowseGitCommit.pm index 328d2d40..6eadd2e5 100644 --- a/lib/PublicInbox/RepobrowseGitCommit.pm +++ b/lib/PublicInbox/RepobrowseGitCommit.pm @@ -131,10 +131,9 @@ sub call_git_commit { # RepobrowseBase calls this } my $git = $req->{repo_info}->{git}; - my $cmd = [ 'git', "--git-dir=$git->{git_dir}", qw(show - -z --numstat -p --encoding=UTF-8 + my $cmd = $git->cmd(qw(show -z --numstat -p --encoding=UTF-8 --no-notes --no-color -c), - $git->abbrev, GIT_FMT, $id, '--' ]; + $git->abbrev, GIT_FMT, $id, '--'); my $rdr = { 2 => $git->err_begin }; my $qsp = PublicInbox::Qspawn->new($cmd, undef, $rdr); $req->{'q'} = $q; diff --git a/lib/PublicInbox/RepobrowseGitDiff.pm b/lib/PublicInbox/RepobrowseGitDiff.pm index eb64d1fd..e2b7179f 100644 --- a/lib/PublicInbox/RepobrowseGitDiff.pm +++ b/lib/PublicInbox/RepobrowseGitDiff.pm @@ -40,10 +40,8 @@ sub call_git_diff { my $id2 = $q->{id2}; my $git = $req->{repo_info}->{git}; - my $cmd = [ 'git', "--git-dir=$git->{git_dir}", qw(diff-tree - -z --numstat -p --encoding=UTF-8 - --no-color -M -B -D -r), - $id2, $id, '--' ]; + my $cmd = $git->cmd(qw(diff-tree -z --numstat -p --encoding=UTF-8 + --no-color -M -B -D -r), $id2, $id, '--'); my $expath = $req->{expath}; push @$cmd, $expath if $expath ne ''; my $o = { nofollow => 1, noindex => 1 }; diff --git a/lib/PublicInbox/RepobrowseGitLog.pm b/lib/PublicInbox/RepobrowseGitLog.pm index 21c23fd3..85593cb8 100644 --- a/lib/PublicInbox/RepobrowseGitLog.pm +++ b/lib/PublicInbox/RepobrowseGitLog.pm @@ -130,15 +130,8 @@ sub call_git_log { my $h = $q->{h}; $h eq '' and $h = 'HEAD'; my $git = $repo_info->{git}; - my $git_dir = $git->{git_dir}; - - # n.b. no need to escape $h, this -debug line will never - # be seen if $h is invalid - # XXX but we should probably validate refnames before execve... - $req->{-debug} = "git log --git-dir=$git_dir $h --"; - my $cmd = [ 'git', "--git-dir=$git_dir", - qw(log --no-notes --no-color --abbrev-commit), - $git->abbrev, $LOG_FMT, "-$max", $h, '--' ]; + my $cmd = $git->cmd(qw(log --no-notes --no-color --abbrev-commit), + $git->abbrev, $LOG_FMT, "-$max", $h, '--'); my $rdr = { 2 => $git->err_begin }; my $title = "log: $repo_info->{repo} (" . utf8_html($h). ')'; $req->{lhtml} = $self->html_start($req, $title) . "\n\n"; diff --git a/lib/PublicInbox/RepobrowseGitPatch.pm b/lib/PublicInbox/RepobrowseGitPatch.pm index dc6cd793..5ae768e8 100644 --- a/lib/PublicInbox/RepobrowseGitPatch.pm +++ b/lib/PublicInbox/RepobrowseGitPatch.pm @@ -24,12 +24,11 @@ sub call_git_patch { # limit scope, don't take extra args to avoid wasting server # resources buffering: my $range = "$id~1..$id^0"; - my @cmd = ('git', "--git-dir=$git->{git_dir}", @CMD, - $sig." $range", $range, '--'); + my $cmd = $git->cmd(@CMD, $sig." $range", $range, '--'); my $expath = $req->{expath}; - push @cmd, $expath if $expath ne ''; + push @$cmd, $expath if $expath ne ''; - my $qsp = PublicInbox::Qspawn->new(\@cmd); + my $qsp = PublicInbox::Qspawn->new($cmd); $qsp->psgi_return($env, undef, sub { my ($r) = @_; my $h = ['Content-Type', 'text/plain; charset=UTF-8']; diff --git a/lib/PublicInbox/RepobrowseGitPlain.pm b/lib/PublicInbox/RepobrowseGitPlain.pm index 80324498..24cc70b0 100644 --- a/lib/PublicInbox/RepobrowseGitPlain.pm +++ b/lib/PublicInbox/RepobrowseGitPlain.pm @@ -82,8 +82,7 @@ sub git_tree_plain { $req->{tpfx} = $pfx; $req->{tstart} = "$title".$t; - my $cmd = [ 'git', "--git-dir=$git->{git_dir}", - qw(ls-tree --name-only -z), $git->abbrev, $hex ]; + my $cmd = $git->cmd(qw(ls-tree --name-only -z), $git->abbrev, $hex); my $rdr = { 2 => $git->err_begin }; my $qsp = PublicInbox::Qspawn->new($cmd, undef, $rdr); my $env = $req->{env}; diff --git a/lib/PublicInbox/RepobrowseGitSnapshot.pm b/lib/PublicInbox/RepobrowseGitSnapshot.pm index a9751b97..450fdad6 100644 --- a/lib/PublicInbox/RepobrowseGitSnapshot.pm +++ b/lib/PublicInbox/RepobrowseGitSnapshot.pm @@ -65,8 +65,8 @@ sub call_git_snapshot ($$) { # invoked by PublicInbox::RepobrowseBase::call delete $env->{'repobrowse.tree_cb'}; delete $env->{'qspawn.quiet'}; my $pfx = "$repo_info->{snapshot_pfx}-$ref/"; - my $cmd = [ 'git', "--git-dir=$git->{git_dir}", 'archive', - "--prefix=$pfx", "--format=$fmt", $tree ]; + my $cmd = $git->cmd('archive', + "--prefix=$pfx", "--format=$fmt", $tree); my $rdr = { 2 => $git->err_begin }; my $qsp = PublicInbox::Qspawn->new($cmd, undef, $rdr); $qsp->psgi_return($env, undef, sub { @@ -80,8 +80,7 @@ sub call_git_snapshot ($$) { # invoked by PublicInbox::RepobrowseBase::call }); }; - my @cmd = ('git', "--git-dir=$git->{git_dir}", - qw(rev-parse --verify --revs-only)); + my $cmd = $git->cmd(qw(rev-parse --verify --revs-only)); # try prefixing "v" or "V" for tag names to get the tree my @refs = ("V$ref", "v$ref", $ref); $env->{'qspawn.quiet'} = 1; @@ -98,7 +97,7 @@ sub call_git_snapshot ($$) { # invoked by PublicInbox::RepobrowseBase::call } my $rdr = { 2 => $git->err_begin }; my $r = pop @refs; - my $qsp = PublicInbox::Qspawn->new([@cmd, $r], undef, $rdr); + my $qsp = PublicInbox::Qspawn->new([@$cmd, $r], undef, $rdr); $qsp->psgi_qx($env, undef, $env->{'repobrowse.tree_cb'}); }; sub { diff --git a/lib/PublicInbox/RepobrowseGitSummary.pm b/lib/PublicInbox/RepobrowseGitSummary.pm index f571a4f5..b6b96028 100644 --- a/lib/PublicInbox/RepobrowseGitSummary.pm +++ b/lib/PublicInbox/RepobrowseGitSummary.pm @@ -16,7 +16,7 @@ sub call_git_summary { # n.b. we would use %(HEAD) in for-each-ref --format if we could # rely on git 1.9.0+, but it's too soon for that in early 2017... - my $cmd = [ 'git', "--git-dir=$git->{git_dir}", qw(symbolic-ref HEAD) ]; + my $cmd = $git->cmd(qw(symbolic-ref HEAD)); my $rdr = { 2 => $git->err_begin }; my $qsp = PublicInbox::Qspawn->new($cmd, undef, $rdr); sub { diff --git a/lib/PublicInbox/RepobrowseGitTree.pm b/lib/PublicInbox/RepobrowseGitTree.pm index a2e38017..c242fd1a 100644 --- a/lib/PublicInbox/RepobrowseGitTree.pm +++ b/lib/PublicInbox/RepobrowseGitTree.pm @@ -189,8 +189,7 @@ sub git_tree_sed ($) { sub git_tree_show { my ($req, $hex, $q) = @_; my $git = $req->{repo_info}->{git}; - my $cmd = [ 'git', "--git-dir=$git->{git_dir}", qw(ls-tree -l -z), - $git->abbrev, $hex ]; + my $cmd = $git->cmd(qw(ls-tree -l -z), $git->abbrev, $hex); my $rdr = { 2 => $git->err_begin }; my $qsp = PublicInbox::Qspawn->new($cmd, undef, $rdr); my $t = cur_path($req, $q); -- cgit v1.2.3-24-ge0c7