From: Eric Wong <e@80x24.org>
Date: Thu, 9 May 2024 00:39:01 +0000 (+0000)
Subject: treewide: reduce $PATH checks for `git' executable
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=39c390da4f5793bdd08a3634ca34ed9c9bda0700;p=thirdparty%2Fpublic-inbox.git

treewide: reduce $PATH checks for `git' executable

Repeatedly checking $PATH for `git' when we need to call it
multiple times in quick succession doesn't seem useful.  So
avoid some expensive stat(2) syscalls to make things less bad
for systems which require expensive CPU vulnerability
mitigations.

This also saves a bunch of memory allocations since we do the
$PATH lookup in pure Perl to avoid doing the uncacheable lookup
in a vfork-ed child.
---

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index d63006106..49659a2e6 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -13,6 +13,7 @@ use v5.10.1;
 use parent qw(Exporter);
 our @EXPORT_OK = qw(glob2re rel2abs_collapsed);
 use PublicInbox::Inbox;
+use PublicInbox::Git qw(git_exe);
 use PublicInbox::Spawn qw(popen_rd run_qx);
 our $LD_PRELOAD = $ENV{LD_PRELOAD}; # only valid at startup
 our $DEDUPE; # set to {} to dedupe or clear cache
@@ -188,7 +189,7 @@ sub git_config_dump {
 		unshift(@opt_c, '-c', "include.path=$file") if defined($file);
 		tmp_cmd_opt(\%env, $opt);
 	}
-	my @cmd = ('git', @opt_c, qw(config -z -l --includes));
+	my @cmd = (git_exe, @opt_c, qw(config -z -l --includes));
 	push(@cmd, '-f', $file) if !@opt_c && defined($file);
 	my $fh = popen_rd(\@cmd, \%env, $opt);
 	my $rv = config_fh_parse($fh, "\0", "\n");
@@ -608,7 +609,7 @@ sub config_cmd {
 	my ($self, $env, $opt) = @_;
 	my $f = $self->{-f} // default_file();
 	my @opt_c = @{$self->{-opt_c} // []};
-	my @cmd = ('git', @opt_c, 'config');
+	my @cmd = (git_exe, @opt_c, 'config');
 	@opt_c ? tmp_cmd_opt($env, $opt) : push(@cmd, '-f', $f);
 	\@cmd;
 }
diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 774fa47bb..883dbea3d 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -1288,10 +1288,10 @@ sub idx_init { # similar to V2Writable
 		$self->{mg}->write_alternates($mode, $alt, $new);
 	my $restore = $self->with_umask;
 	if ($git_midx && ($opt->{'multi-pack-index'} // 1)) {
-		my @cmd = ('multi-pack-index');
-		push @cmd, '--no-progress' if ($opt->{quiet}//0) > 1;
+		my $cmd = $self->git->cmd('multi-pack-index');
+		push @$cmd, '--no-progress' if ($opt->{quiet}//0) > 1;
 		my $lk = $self->lock_for_scope;
-		system('git', "--git-dir=$ALL", @cmd, 'write');
+		system(@$cmd, 'write');
 		# ignore errors, fairly new command, may not exist
 	}
 	$self->parallel_init($self->{indexlevel});
diff --git a/lib/PublicInbox/Fetch.pm b/lib/PublicInbox/Fetch.pm
index b0f1437cb..814d6e8ed 100644
--- a/lib/PublicInbox/Fetch.pm
+++ b/lib/PublicInbox/Fetch.pm
@@ -12,6 +12,7 @@ use PublicInbox::LeiCurl;
 use PublicInbox::LeiMirror;
 use PublicInbox::SHA qw(sha_all);
 use File::Temp ();
+use PublicInbox::Git qw(git_exe);
 
 sub new { bless {}, __PACKAGE__ }
 
@@ -19,7 +20,7 @@ sub remote_url ($$) {
 	my ($lei, $dir) = @_;
 	my $rn = $lei->{opt}->{'try-remote'} // [ 'origin', '_grokmirror' ];
 	for my $r (@$rn) {
-		my $cmd = [ qw(git config), "remote.$r.url" ];
+		my $cmd = [ git_exe, 'config', "remote.$r.url" ];
 		my $url = run_qx($cmd, undef, { -C => $dir, 2 => $lei->{2} });
 		next if $?;
 		$url =~ s!/*\n!!s;
@@ -92,7 +93,7 @@ sub do_manifest ($$$) {
 
 sub get_fingerprint2 {
 	my ($git_dir) = @_;
-	my $rd = popen_rd([qw(git show-ref)], undef, { -C => $git_dir });
+	my $rd = popen_rd([git_exe, 'show-ref'], undef, { -C => $git_dir });
 	sha_all(256, $rd)->digest; # ignore show-ref errors
 }
 
@@ -132,8 +133,8 @@ sub do_fetch { # main entry point
 				warn "W: $edir missing remote.*.url\n";
 				my $o = { -C => $edir };
 				$o->{1} = $o->{2} = $lei->{2};
-				run_wait([qw(git config -l)], undef, $o) and
-					$lei->child_error($?);
+				run_wait([git_exe, qw(config -l)], undef, $o)
+					and $lei->child_error($?);
 			}
 		}
 		@epochs = grep { !$skip->{$_} } @epochs if $skip;
@@ -188,7 +189,7 @@ EOM
 		my $opt = {}; # for spawn
 		if (-d $d) {
 			$fp2->[0] = get_fingerprint2($d) if $fp2;
-			$cmd = [ @$torsocks, 'git', "--git-dir=$d",
+			$cmd = [ @$torsocks, git_exe, "--git-dir=$d",
 			       PublicInbox::LeiMirror::fetch_args($lei, $opt)];
 		} else {
 			my $e_uri = $ibx_uri->clone;
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index aea389e85..a9a821adf 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -10,6 +10,7 @@ package PublicInbox::Git;
 use strict;
 use v5.10.1;
 use parent qw(Exporter PublicInbox::DS);
+use PublicInbox::DS qw(now);
 use autodie qw(socketpair read);
 use POSIX ();
 use Socket qw(AF_UNIX SOCK_STREAM);
@@ -25,7 +26,7 @@ use PublicInbox::SHA qw(sha_all);
 our %HEXLEN2SHA = (40 => 1, 64 => 256);
 our %OFMT2HEXLEN = (sha1 => 40, sha256 => 64);
 our @EXPORT_OK = qw(git_unquote git_quote %HEXLEN2SHA %OFMT2HEXLEN
-			$ck_unlinked_packs);
+			$ck_unlinked_packs git_exe);
 our $in_cleanup;
 our $async_warn; # true in read-only daemons
 
@@ -54,7 +55,11 @@ my %ESC_GIT = map { $GIT_ESC{$_} => $_ } keys %GIT_ESC;
 my $EXE_ST = ''; # pack('dd', st_dev, st_ino); # no `q' in some 32-bit builds
 my ($GIT_EXE, $GIT_VER);
 
-sub check_git_exe () {
+sub git_exe () {
+	my $now = now;
+	state $next_check = $now - 10;
+	return $GIT_EXE if $now < $next_check;
+	$next_check = $now + 10;
 	$GIT_EXE = which('git') // die "git not found in $ENV{PATH}";
 	my @st = stat(_) or die "stat($GIT_EXE): $!"; # can't do HiRes w/ _
 	my $st = pack('dd', $st[0], $st[1]);
@@ -69,8 +74,8 @@ sub check_git_exe () {
 	$GIT_EXE;
 }
 
-sub git_version {
-	check_git_exe();
+sub git_version () {
+	git_exe;
 	$GIT_VER;
 }
 
@@ -174,7 +179,7 @@ sub _sock_cmd {
 
 	# git 2.31.0+ supports -c core.abbrev=no, don't bother with
 	# core.abbrev=64 since not many releases had SHA-256 prior to 2.31
-	my $abbr = $GIT_VER lt v2.31.0 ? 40 : 'no';
+	my $abbr = git_version lt v2.31.0 ? 40 : 'no';
 	my @cmd = ($GIT_EXE, "--git-dir=$gd", '-c', "core.abbrev=$abbr",
 			'cat-file', "--$batch");
 	if ($err_c) {
@@ -287,8 +292,7 @@ sub cat_async_wait ($) {
 
 sub batch_prepare ($) {
 	my ($self) = @_;
-	check_git_exe();
-	if ($GIT_VER ge BATCH_CMD_VER) {
+	if (git_version ge BATCH_CMD_VER) {
 		$self->{-bc} = 1;
 		_sock_cmd($self, 'batch-command', 1);
 	} else {
@@ -344,8 +348,7 @@ sub ck {
 sub check_async_begin ($) {
 	my ($self) = @_;
 	cleanup($self) if alternates_changed($self);
-	check_git_exe();
-	if ($GIT_VER ge BATCH_CMD_VER) {
+	if (git_version ge BATCH_CMD_VER) {
 		$self->{-bc} = 1;
 		_sock_cmd($self, 'batch-command', 1);
 	} else {
@@ -421,15 +424,15 @@ sub async_err ($$$$$) {
 
 sub cmd {
 	my $self = shift;
-	[ $GIT_EXE // check_git_exe(), "--git-dir=$self->{git_dir}", @_ ]
+	[ git_exe(), "--git-dir=$self->{git_dir}", @_ ]
 }
 
 # $git->popen(qw(show f00)); # or
 # $git->popen(qw(show f00), { GIT_CONFIG => ... }, { 2 => ... });
 sub popen {
 	my ($self, $cmd) = splice(@_, 0, 2);
-	$cmd = [ 'git', "--git-dir=$self->{git_dir}",
-		ref($cmd) ? @$cmd : ($cmd, grep { defined && !ref } @_) ];
+	$cmd = $self->cmd(ref($cmd) ? @$cmd :
+			($cmd, grep { defined && !ref } @_));
 	popen_rd($cmd, grep { !defined || ref } @_); # env and opt
 }
 
@@ -577,9 +580,8 @@ sub cloneurl {
 # templates/this--description in git.git
 sub manifest_entry {
 	my ($self, $epoch, $default_desc) = @_;
-	check_git_exe();
 	my $gd = $self->{git_dir};
-	my @git = ($GIT_EXE, "--git-dir=$gd");
+	my @git = (git_exe, "--git-dir=$gd");
 	my $sr = popen_rd([@git, 'show-ref']);
 	my $own = popen_rd([@git, qw(config gitweb.owner)]);
 	my $mod = popen_rd([@git, @MODIFIED_DATE]);
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index ed34d548e..fefc282a0 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -73,8 +73,8 @@ sub gfi_start {
 			die "fatal: ls-tree -r -z --name-only $ref: \$?=$?" if $?;
 			$self->{-tree} = { map { $_ => 1 } split(/\0/, $t) };
 		}
-		my $gfi = [ 'git', "--git-dir=$git->{git_dir}", qw(fast-import
-				--quiet --done --date-format=raw) ];
+		my $gfi = $git->cmd(qw(fast-import
+					--quiet --done --date-format=raw));
 		my $pid = spawn($gfi, undef, { 0 => $s2, 1 => $s2 });
 		$self->{nchg} = 0;
 		$self->{io} = PublicInbox::IO::attach_pid($io, $pid);
@@ -161,7 +161,7 @@ sub _update_git_info ($$) {
 	# for compatibility with existing ssoma installations
 	# we can probably remove this entirely by 2020
 	my $git_dir = $self->{git}->{git_dir};
-	my @cmd = ('git', "--git-dir=$git_dir");
+	my @cmd = @{$self->{git}->cmd};
 	my $index = "$git_dir/ssoma.index";
 	if (-e $index && !$ENV{FAST}) {
 		my $env = { GIT_INDEX_FILE => $index };
@@ -631,7 +631,7 @@ sub replace_oids {
 	chomp(my $cmt = $self->get_mark(":$mark")) if $nreplace;
 	$self->{nchg} = 0; # prevent _update_git_info until update-ref:
 	$self->done;
-	my @git = ('git', "--git-dir=$git->{git_dir}");
+	my @git = @{$git->cmd};
 
 	run_die([@git, qw(update-ref), $old, $tmp]) if $nreplace;
 
diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index 00697097b..7b2ea4347 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -36,14 +36,13 @@ sub solver_user_cb { # called by solver when done
 	ref($res) eq 'ARRAY' or return $lei->child_error(0, $$log_buf);
 	$lei->qerr($$log_buf);
 	my ($git, $oid, $type, $size, $di) = @$res;
-	my $gd = $git->{git_dir};
 
 	# don't try to support all the git-show(1) options for non-blob,
 	# this is just a convenience:
-	$type ne 'blob' and
-		warn "# $oid is a $type of $size bytes in:\n#\t$gd\n";
-
-	my $cmd = [ 'git', "--git-dir=$gd", 'show', $oid ];
+	$type ne 'blob' and warn <<EOM;
+# $oid is a $type of $size bytes in:\n#\t$git->{git_dir}
+EOM
+	my $cmd = $git->cmd('show', $oid);
 	my $rdr = { 1 => $lei->{1}, 2 => $lei->{2} };
 	run_wait($cmd, $lei->{env}, $rdr) and $lei->child_error($?);
 }
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 08e61e4b2..e7c265bdb 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -24,6 +24,7 @@ use POSIX qw(strftime);
 use PublicInbox::Admin qw(fmt_localtime);
 use autodie qw(chdir chmod close open pipe readlink
 		seek symlink sysopen sysseek truncate unlink);
+use PublicInbox::Git qw(git_exe);
 
 our $LIVE; # pid => callback
 our $FGRP_TODO; # objstore -> [[ to resume ], [ to clone ]]
@@ -105,7 +106,7 @@ E: confused by scraping <$uri>, got ambiguous results:
 
 sub clone_cmd {
 	my ($lei, $opt) = @_;
-	my @cmd = qw(git);
+	my @cmd = (git_exe);
 	$opt->{$_} = $lei->{$_} for (0..2);
 	# we support "-c $key=$val" for arbitrary git config options
 	# e.g.: git -c http.proxy=socks5h://127.0.0.1:9050
@@ -291,7 +292,7 @@ sub upr { # feed `git update-ref --stdin -z' verbosely
 sub start_update_ref {
 	my ($fgrp) = @_;
 	pipe(my $r, my $w);
-	my $cmd = [ 'git', "--git-dir=$fgrp->{cur_dst}",
+	my $cmd = [ git_exe, "--git-dir=$fgrp->{cur_dst}",
 		qw(update-ref --stdin -z) ];
 	my $pack = on_destroy \&satellite_done, $fgrp;
 	start_cmd($fgrp, $cmd, { 0 => $r, 2 => $fgrp->{lei}->{2} }, $pack);
@@ -353,7 +354,7 @@ sub satellite_done {
 
 sub pack_refs {
 	my ($self, $git_dir) = @_;
-	my $cmd = [ 'git', "--git-dir=$git_dir", qw(pack-refs --all --prune) ];
+	my $cmd = [git_exe, "--git-dir=$git_dir", qw(pack-refs --all --prune)];
 	start_cmd($self, $cmd, { 2 => $self->{lei}->{2} });
 }
 
@@ -374,14 +375,15 @@ sub fgrpv_done {
 		my $rn = $fgrp->{-remote};
 		my %opt = ( 2 => $fgrp->{lei}->{2} );
 		my $update_ref = on_destroy \&fgrp_update, $fgrp;
-		my $src = [ 'git', "--git-dir=$fgrp->{-osdir}", 'for-each-ref',
+		my $src = [ git_exe, "--git-dir=$fgrp->{-osdir}",
+			'for-each-ref',
 			"--format=refs/%(refname:lstrip=3)%00%(objectname)",
 			"refs/remotes/$rn/" ];
 		open(my $sfh, '+>', undef);
 		$fgrp->{srcfh} = $sfh;
 		start_cmd($fgrp, $src, { %opt, 1 => $sfh }, $update_ref);
-		my $dst = [ 'git', "--git-dir=$fgrp->{cur_dst}", 'for-each-ref',
-			'--format=%(refname)%00%(objectname)' ];
+		my $dst = [ git_exe, "--git-dir=$fgrp->{cur_dst}",
+			'for-each-ref', '--format=%(refname)%00%(objectname)' ];
 		open(my $dfh, '+>', undef);
 		$fgrp->{dstfh} = $dfh;
 		start_cmd($fgrp, $dst, { %opt, 1 => $dfh }, $update_ref);
@@ -399,7 +401,7 @@ sub fgrp_fetch_all {
 	# system argv limits:
 	my $grp = 'fgrptmp';
 
-	my @git = (@{$self->{-torsocks}}, 'git');
+	my @git = (@{$self->{-torsocks}}, git_exe);
 	my $j = $self->{lei}->{opt}->{jobs};
 	my $opt = {};
 	my @fetch = do {
@@ -413,7 +415,7 @@ sub fgrp_fetch_all {
 		my ($old, $new) = @$fgrp_old_new;
 		@$old = sort { $b->{-sort} <=> $a->{-sort} } @$old;
 		# $new is ordered by {references}
-		my $cmd = ['git', "--git-dir=$osdir", qw(config -f), $f ];
+		my $cmd = [ git_exe, "--git-dir=$osdir", qw(config -f), $f ];
 
 		# clobber settings from previous run atomically
 		for ("remotes.$grp", 'fetch.hideRefs') {
@@ -541,7 +543,7 @@ sub cmp_fp_do {
 		return if $cur_ent->{fingerprint} eq $new;
 	}
 	my $dst = $self->{cur_dst} // $self->{dst};
-	my $cmd = ['git', "--git-dir=$dst", 'show-ref'];
+	my $cmd = [git_exe, "--git-dir=$dst", 'show-ref'];
 	my $opt = { 2 => $self->{lei}->{2} };
 	open($opt->{1}, '+>', undef);
 	$self->{-show_ref} = $opt->{1};
@@ -555,7 +557,7 @@ sub resume_fetch {
 	my ($self, $uri, $fini) = @_;
 	return if !keep_going($self);
 	my $dst = $self->{cur_dst} // $self->{dst};
-	my @git = ('git', "--git-dir=$dst");
+	my @git = (git_exe, "--git-dir=$dst");
 	my $opt = { 2 => $self->{lei}->{2} };
 	my $rn = 'random'.int(rand(1 << 30));
 	for ("url=$uri", "fetch=+refs/*:refs/*", 'mirror=true') {
@@ -755,7 +757,7 @@ sub update_ent {
 	my $cur = $self->{-local_manifest}->{$key}->{fingerprint} // "\0";
 	my $dst = $self->{cur_dst} // $self->{dst};
 	if (defined($new) && $new ne $cur) {
-		my $cmd = ['git', "--git-dir=$dst", 'show-ref'];
+		my $cmd = [git_exe, "--git-dir=$dst", 'show-ref'];
 		my $opt = { 2 => $self->{lei}->{2} };
 		open($opt->{1}, '+>', undef);
 		$self->{-show_ref_up} = $opt->{1};
@@ -766,7 +768,7 @@ sub update_ent {
 	$cur = $self->{-local_manifest}->{$key}->{head} // "\0";
 	if (defined($new) && $new ne $cur) {
 		# n.b. grokmirror writes raw contents to $dst/HEAD w/o locking
-		my $cmd = [ 'git', "--git-dir=$dst" ];
+		my $cmd = [ git_exe, "--git-dir=$dst" ];
 		if ($new =~ s/\Aref: //) {
 			push @$cmd, qw(symbolic-ref HEAD), $new;
 		} elsif ($new =~ /\A[a-f0-9]{40,}\z/) {
@@ -811,7 +813,8 @@ sub update_ent {
 	$cur = $self->{-local_manifest}->{$key}->{owner} // "\0";
 	return if $cur eq $new;
 	utf8::encode($new); # to octets
-	my $cmd = [ qw(git config -f), "$dst/config", 'gitweb.owner', $new ];
+	my $cmd = [ git_exe, qw(config -f), "$dst/config",
+			'gitweb.owner', $new ];
 	start_cmd($self, $cmd, { 2 => $self->{lei}->{2} });
 }
 
diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm
index 357283300..66359dd44 100644
--- a/lib/PublicInbox/LeiRediff.pm
+++ b/lib/PublicInbox/LeiRediff.pm
@@ -119,17 +119,16 @@ EOM
 			map { $_->git_path('objects')."\n" } @{$self->{gits}};
 		$rw = PublicInbox::Git->new($d);
 	}
-	my $w = popen_wr(['git', "--git-dir=$rw->{git_dir}",
-			qw(fast-import --quiet --done --date-format=raw)],
+	my $w = popen_wr($rw->cmd(qw(fast-import
+				--quiet --done --date-format=raw)),
 			$lei->{env}, { 2 => $lei->{2} });
 	print $w $ta, "\n", $tb, "\ndone\n" or die "print fast-import: $!";
 	$w->close or die "close w fast-import: \$?=$? \$!=$!";
 
-	my $cmd = [ 'diff' ];
+	my $cmd = $rw->cmd('diff');
 	_lei_diff_prepare($lei, $cmd);
-	$lei->qerr("# git @$cmd");
+	$lei->qerr("# git @$cmd[2..$#$cmd]");
 	push @$cmd, qw(A B);
-	unshift @$cmd, 'git', "--git-dir=$rw->{git_dir}";
 	run_wait($cmd, $lei->{env}, { 2 => $lei->{2}, 1 => $lei->{1} }) and
 		$lei->child_error($?); # for git diff --exit-code
 	undef;
diff --git a/lib/PublicInbox/RepoAtom.pm b/lib/PublicInbox/RepoAtom.pm
index ab0f2fcc6..eb0ed3c77 100644
--- a/lib/PublicInbox/RepoAtom.pm
+++ b/lib/PublicInbox/RepoAtom.pm
@@ -94,11 +94,10 @@ xmlns="http://www.w3.org/1999/xhtml"><pre style="white-space:pre-wrap">
 sub srv_tags_atom {
 	my ($ctx) = @_;
 	my $max = 50; # TODO configurable
-	my @cmd = ('git', "--git-dir=$ctx->{git}->{git_dir}",
-			qw(for-each-ref --sort=-creatordate), "--count=$max",
-			'--perl', $EACH_REF_FMT, 'refs/tags');
+	my $cmd = $ctx->{git}->cmd(qw(for-each-ref --sort=-creatordate),
+			"--count=$max", '--perl', $EACH_REF_FMT, 'refs/tags');
 	$ctx->{-feed_title} = "$ctx->{git}->{nick} tags";
-	my $qsp = PublicInbox::Qspawn->new(\@cmd);
+	my $qsp = PublicInbox::Qspawn->new($cmd);
 	$ctx->{-is_tag} = 1;
 	$qsp->psgi_yield($ctx->{env}, undef, \&atom_ok, $ctx);
 }
@@ -107,20 +106,19 @@ sub srv_atom {
 	my ($ctx, $path) = @_;
 	return if index($path, '//') >= 0 || index($path, '/') == 0;
 	my $max = 50; # TODO configurable
-	my @cmd = ('git', "--git-dir=$ctx->{git}->{git_dir}",
-			qw(log --no-notes --no-color --no-abbrev),
-			$ATOM_FMT, "-$max");
+	my $cmd = $ctx->{git}->cmd(qw(log --no-notes --no-color --no-abbrev),
+				$ATOM_FMT, "-$max");
 	my $tip = $ctx->{qp}->{h}; # same as cgit
 	$ctx->{-feed_title} = $ctx->{git}->{nick};
 	$ctx->{-feed_title} .= " $path" if $path ne '';
 	if (defined($tip)) {
-		push @cmd, $tip;
+		push @$cmd, $tip;
 		$ctx->{-feed_title} .= ", $tip";
 	}
 	# else: let git decide based on HEAD if $tip isn't defined
-	push @cmd, '--';
-	push @cmd, $path if $path ne '';
-	my $qsp = PublicInbox::Qspawn->new(\@cmd, undef,
+	push @$cmd, '--';
+	push @$cmd, $path if $path ne '';
+	my $qsp = PublicInbox::Qspawn->new($cmd, undef,
 					{ quiet => 1, 2 => $ctx->{lh} });
 	$qsp->psgi_yield($ctx->{env}, undef, \&atom_ok, $ctx);
 }
diff --git a/lib/PublicInbox/RepoSnapshot.pm b/lib/PublicInbox/RepoSnapshot.pm
index 4c3725692..bff97bc82 100644
--- a/lib/PublicInbox/RepoSnapshot.pm
+++ b/lib/PublicInbox/RepoSnapshot.pm
@@ -50,15 +50,13 @@ sub ver_check { # git->check_async callback
 			delete($ctx->{env}->{'qspawn.wcb'})->(r(404));
 	} else { # found, done:
 		$ctx->{etag} = $oid;
-		my @cfg;
+		my $cmd = $ctx->{git}->cmd;
 		if (my $cmd = $FMT_CFG{$ctx->{snap_fmt}}) {
-			@cfg = ('-c', "tar.$ctx->{snap_fmt}.command=$cmd");
+			push @$cmd, '-c', "tar.$ctx->{snap_fmt}.command=$cmd";
 		}
-		my $qsp = PublicInbox::Qspawn->new(['git', @cfg,
-				"--git-dir=$ctx->{git}->{git_dir}", 'archive',
-				"--prefix=$ctx->{snap_pfx}/",
-				"--format=$ctx->{snap_fmt}", $treeish], undef,
-				{ quiet => 1 });
+		push @$cmd, 'archive', "--prefix=$ctx->{snap_pfx}/",
+				"--format=$ctx->{snap_fmt}", $treeish;
+		my $qsp = PublicInbox::Qspawn->new($cmd, undef, { quiet => 1 });
 		$qsp->psgi_yield($ctx->{env}, undef, \&archive_hdr, $ctx);
 	}
 }
diff --git a/lib/PublicInbox/RepoTree.pm b/lib/PublicInbox/RepoTree.pm
index 5c73531a8..4c85f9a87 100644
--- a/lib/PublicInbox/RepoTree.pm
+++ b/lib/PublicInbox/RepoTree.pm
@@ -51,8 +51,8 @@ sub find_missing {
 		$res->[0] = 404;
 		return delete($ctx->{-wcb})->($res);
 	}
-	my $cmd = ['git', "--git-dir=$ctx->{git}->{git_dir}",
-		qw(log --no-color -1), '--pretty=%H %h %s (%as)' ];
+	my $cmd = $ctx->{git}->cmd(qw(log --no-color -1),
+				'--pretty=%H %h %s (%as)');
 	push @$cmd, $ctx->{qp}->{h} if defined($ctx->{qp}->{h});
 	push @$cmd, '--';
 	push @$cmd, $ctx->{-path};
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index d3a7a0c08..4fd493d95 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -1003,8 +1003,7 @@ sub prepare_stack ($$) {
 sub is_ancestor ($$$) {
 	my ($git, $cur, $tip) = @_;
 	return 0 unless $git->check($cur);
-	my $cmd = [ 'git', "--git-dir=$git->{git_dir}",
-		qw(merge-base --is-ancestor), $cur, $tip ];
+	my $cmd = $git->cmd(qw(merge-base --is-ancestor), $cur, $tip);
 	run_wait($cmd) == 0;
 }
 
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index aeff5d1d6..3a67ab541 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -168,7 +168,7 @@ sub require_git_http_backend (;$) {
 	my ($nr) = @_;
 	state $ok = do {
 		require PublicInbox::Git;
-		my $git = PublicInbox::Git::check_git_exe() or plan
+		my $git = PublicInbox::Git::git_exe() or plan
 			skip_all => 'nothing in public-inbox works w/o git';
 		my $rdr = { 1 => \my $out, 2 => \my $err };
 		xsys([$git, qw(http-backend)], undef, $rdr);
diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 43f37f602..15a731581 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -1071,8 +1071,8 @@ sub unindex_todo ($$$) {
 	return if $before == $after;
 
 	# ensure any blob can not longer be accessed via dumb HTTP
-	run_die(['git', "--git-dir=$unit->{git}->{git_dir}",
-		qw(-c gc.reflogExpire=now gc --prune=all --quiet)]);
+	run_die($unit->{git}->cmd(qw(-c gc.reflogExpire=now gc
+				--prune=all --quiet)));
 }
 
 sub sync_ranges ($$) {
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index f47c2703e..83a836986 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -106,7 +106,7 @@ sub stream_large_blob ($$) {
 	my ($ctx, $res) = @_;
 	$ctx->{-res} = $res;
 	my ($git, $oid, $type, $size, $di) = @$res;
-	my $cmd = ['git', "--git-dir=$git->{git_dir}", 'cat-file', $type, $oid];
+	my $cmd = $git->cmd('cat-file', $type, $oid);
 	my $qsp = PublicInbox::Qspawn->new($cmd);
 	$ctx->{env}->{'qspawn.wcb'} = $ctx->{-wcb};
 	$qsp->psgi_yield($ctx->{env}, undef, \&stream_blob_parse_hdr, $ctx);
@@ -368,10 +368,9 @@ sub stream_patch_parse_hdr { # {parse_hdr} for Qspawn
 sub show_patch ($$) {
 	my ($ctx, $res) = @_;
 	my ($git, $oid) = @$res;
-	my @cmd = ('git', "--git-dir=$git->{git_dir}",
-		qw(format-patch -1 --stdout -C),
+	my $cmd = $git->cmd(qw(format-patch -1 --stdout -C),
 		"--signature=git format-patch -1 --stdout -C $oid", $oid);
-	my $qsp = PublicInbox::Qspawn->new(\@cmd);
+	my $qsp = PublicInbox::Qspawn->new($cmd);
 	$ctx->{env}->{'qspawn.wcb'} = $ctx->{-wcb};
 	$ctx->{patch_oid} = $oid;
 	$qsp->psgi_yield($ctx->{env}, undef, \&stream_patch_parse_hdr, $ctx);
@@ -400,8 +399,8 @@ sub show_other ($$) { # just in case...
 	my ($git, $oid, $type, $size) = @$res;
 	$size > $MAX_SIZE and return html_page($ctx, 200,
 		ascii_html($type)." $oid is too big to show\n". dbg_log($ctx));
-	my $cmd = ['git', "--git-dir=$git->{git_dir}",
-		qw(show --encoding=UTF-8 --no-color --no-abbrev), $oid ];
+	my $cmd = $git->cmd(qw(show --encoding=UTF-8
+			--no-color --no-abbrev), $oid);
 	my $qsp = PublicInbox::Qspawn->new($cmd);
 	$qsp->{qsp_err} = \($ctx->{-qsp_err} = '');
 	$qsp->psgi_qx($ctx->{env}, undef, \&show_other_result, $ctx);
@@ -487,8 +486,7 @@ sub show_tree ($$) { # also used by RepoTree
 	my ($git, $oid, undef, $size) = @$res;
 	$size > $MAX_SIZE and return html_page($ctx, 200,
 			"tree $oid is too big to show\n". dbg_log($ctx));
-	my $cmd = [ 'git', "--git-dir=$git->{git_dir}",
-		qw(ls-tree -z -l --no-abbrev), $oid ];
+	my $cmd = $git->cmd(qw(ls-tree -z -l --no-abbrev), $oid);
 	my $qsp = PublicInbox::Qspawn->new($cmd);
 	$ctx->{tree_oid} = $oid;
 	$qsp->{qsp_err} = \($ctx->{-qsp_err} = '');