[PATCH 0/8] use libgit2 for writing configs

[PATCH 0/8] use libgit2 for writing configs

[Eric Wong]

1/8 was something I noticed while hunting for
`git config' invocations, and 2/8 was noticed when
working on 1/8.

3-5 came about while testing 8/8

Currently there's no noticeable speedup in the entire test suite
from 8/8 since we've been avoiding using `git config' too much
to setup configs, but that will probably change in the future.
The t/lg2_cfg.t test does show a ~%4 speedup compared to
setting TEST_VALIDATE_GIT_BEHAVIOR to force git(1) use instead
of the new libgit2 $cfgrw_commit subroutine.

Eric Wong (8):
  favor run_wait() over CORE::system()
  edit: use autodie and favor flush over autoflush
  import: clobber $? if global init.defaultBranch fails
  test_common: key2sub: detect syntax errors early
  t/watch_v1_v2_mix_no_modify: load PublicInbox::Config
  cfgwrite: new module to batch commit writes
  rename Gcf2 -> Lg2 for general libgit2 use
  lg2: use cfgwr_commit to write to configs using libgit2

 Documentation/public-inbox-tuning.pod     |  2 +-
 MANIFEST                                  |  6 ++-
 lib/PublicInbox/CfgWr.pm                  | 60 +++++++++++++++++++++++
 lib/PublicInbox/ExtSearchIdx.pm           |  5 +-
 lib/PublicInbox/Gcf2Client.pm             | 10 ++--
 lib/PublicInbox/Import.pm                 |  1 +
 lib/PublicInbox/LEI.pm                    |  1 +
 lib/PublicInbox/LeiMirror.pm              | 23 ++++-----
 lib/PublicInbox/LeiSavedSearch.pm         |  5 +-
 lib/PublicInbox/{Gcf2.pm => Lg2.pm}       | 13 +++--
 lib/PublicInbox/MultiGit.pm               |  4 +-
 lib/PublicInbox/SearchView.pm             |  2 +-
 lib/PublicInbox/Spawn.pm                  |  2 +-
 lib/PublicInbox/TestCommon.pm             |  3 +-
 lib/PublicInbox/ViewVCS.pm                |  2 +-
 lib/PublicInbox/{gcf2_libgit2.h => lg2.h} | 60 ++++++++++++++++++++++-
 script/public-inbox-edit                  | 17 +++----
 script/public-inbox-init                  | 16 +++---
 t/gcf2.t                                  | 12 ++---
 t/gcf2_client.t                           |  2 +-
 t/lg2_cfg.t                               | 60 +++++++++++++++++++++++
 t/watch_v1_v2_mix_no_modify.t             |  1 +
 xt/check-run.t                            |  1 +
 23 files changed, 245 insertions(+), 63 deletions(-)
 create mode 100644 lib/PublicInbox/CfgWr.pm
 rename lib/PublicInbox/{Gcf2.pm => Lg2.pm} (92%)
 rename lib/PublicInbox/{gcf2_libgit2.h => lg2.h} (64%)
 create mode 100644 t/lg2_cfg.t

[PATCH 1/8] favor run_wait() over CORE::system()

[Eric Wong]

run_wait() can use vfork(2) which saves memory in large
processes, such as public-inbox-extindex when dealing with giant
messages.  While vfork is unlikely to matter for real-world uses
of public-inbox-edit, PublicInbox::Spawn is a sunk cost treewide
our `make check-run' test target avoids spawning new Perl
processes for most things in script/*, so there can be a small
savings for testing.
---
 lib/PublicInbox/ExtSearchIdx.pm | 5 +++--
 script/public-inbox-edit        | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index d1a16c84..7cf600c1 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -24,7 +24,7 @@ use File::Glob qw(bsd_glob GLOB_NOSORT);
 use PublicInbox::SQLiteUtil;
 use PublicInbox::Isearch;
 use PublicInbox::MultiGit;
-use PublicInbox::Spawn ();
+use PublicInbox::Spawn qw(run_wait);
 use PublicInbox::Search;
 use PublicInbox::SearchIdx qw(prepare_stack is_ancestor is_bad_blob
 	update_checkpoint);
@@ -1266,8 +1266,9 @@ sub idx_init { # similar to V2Writable
 	if ($git_midx && ($opt->{'multi-pack-index'} // 1)) {
 		my $cmd = $self->git->cmd('multi-pack-index');
 		push @$cmd, '--no-progress' if ($opt->{quiet}//0) > 1;
+		push @$cmd, 'write';
 		my $lk = $self->lock_for_scope;
-		system(@$cmd, 'write');
+		run_wait $cmd;
 		# ignore errors, fairly new command, may not exist
 	}
 	$self->parallel_init($self->{indexlevel});
diff --git a/script/public-inbox-edit b/script/public-inbox-edit
index 88115d7c..c76579e4 100755
--- a/script/public-inbox-edit
+++ b/script/public-inbox-edit
@@ -15,6 +15,7 @@ PublicInbox::Admin::check_require('-index');
 use PublicInbox::Eml;
 use PublicInbox::InboxWritable qw(eml_from_path);
 use PublicInbox::Import;
+use PublicInbox::Spawn qw(run_wait);
 
 my $help = <<'EOF';
 usage: public-inbox-edit -m MESSAGE-ID [--all] [INBOX_DIRS]
@@ -159,7 +160,7 @@ foreach my $to_edit (values %$found) {
 
 	# run the editor, respecting spaces/quote
 retry_edit:
-	if (system(qw(sh -c), $editor.' "$@"', $editor, $edit_fn)) {
+	if (run_wait [qw(sh -c), $editor.' "$@"', $editor, $edit_fn]) {
 		if (!(-t STDIN) && !$opt->{force}) {
 			die "E: $editor failed: $?\n";
 		}

[PATCH 2/8] edit: use autodie and favor flush over autoflush

[Eric Wong]

Using a single ->flush reduces iops for the (default) non-raw
case and autodie for `open' makes error messages more
consistent.
---
 script/public-inbox-edit | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/script/public-inbox-edit b/script/public-inbox-edit
index c76579e4..a363f0c8 100755
--- a/script/public-inbox-edit
+++ b/script/public-inbox-edit
@@ -5,7 +5,8 @@
 # Used for editing messages in a public-inbox.
 # Supports v2 inboxes only, for now.
 use strict;
-use warnings;
+use v5.10.1; # TODO: check unicode_strings compat
+use autodie qw(open);
 use Getopt::Long qw(:config gnu_getopt no_ignore_case auto_abbrev);
 use PublicInbox::AdminEdit;
 use File::Temp 0.19 (); # 0.19 for TMPDIR
@@ -142,7 +143,6 @@ my %tmpopt = (
 
 foreach my $to_edit (values %$found) {
 	my $edit_fh = File::Temp->new(%tmpopt);
-	$edit_fh->autoflush(1);
 	my $edit_fn = $edit_fh->filename;
 	my ($ibx, $smsg) = @{$to_edit->[0]};
 	my $old_raw = $ibx->msg_by_smsg($smsg);
@@ -151,12 +151,11 @@ foreach my $to_edit (values %$found) {
 	my $tmp = $$old_raw;
 	if (!$opt->{raw}) {
 		my $oid = $smsg->{blob};
-		print $edit_fh "From mboxrd\@$oid Thu Jan  1 00:00:00 1970\n"
-			or die "failed to write From_ line: $!";
+		print $edit_fh "From mboxrd\@$oid Thu Jan  1 00:00:00 1970\n";
 		$tmp =~ s/^(>*From )/>$1/gm;
 	}
-	print $edit_fh $tmp or
-		die "failed to write tempfile for editing: $!";
+	print $edit_fh $tmp;
+	$edit_fh->flush or die "E: flush($edit_fn): $!";
 
 	# run the editor, respecting spaces/quote
 retry_edit:
@@ -183,8 +182,7 @@ retry_edit:
 
 	# reread the edited file, not using $edit_fh since $EDITOR may
 	# rename/relink $edit_fn
-	open my $new_fh, '<', $edit_fn or
-		die "can't read edited file ($edit_fn): $!\n";
+	open my $new_fh, '<', $edit_fn;
 	my $new_raw = PublicInbox::IO::read_all $new_fh;
 
 	if (!$opt->{raw}) {

[PATCH 3/8] import: clobber $? if global init.defaultBranch fails

[Eric Wong]

We don't want $? influencing further error checking downstream,
such as thw _wq_done_wait call in public-inbox-clone when
git-config(1) invocations which set $?=0 are replaced with
in-process libgit2 config function calls.
---
 lib/PublicInbox/Import.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index e6d725fc..35985f74 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -29,6 +29,7 @@ sub default_branch () {
 		my $h = run_qx([git_exe,qw(config --global init.defaultBranch)],
 				 { GIT_CONFIG => undef });
 		chomp $h;
+		$? = 0;
 		$h eq '' ? 'refs/heads/master' : "refs/heads/$h";
 	}
 }

[PATCH 4/8] test_common: key2sub: detect syntax errors early

[Eric Wong]

Instead of attempting to use an `undef' $sub as a coderef;
detect compile errors early and die immediately if `eval'
raises an exception.
---
 lib/PublicInbox/TestCommon.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index cbcfc008..e1178149 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -371,6 +371,7 @@ sub main {
 }
 1;
 EOF
+		die "E: $f failed: $@" if $@;
 		$pkg->can('main');
 	}
 }

[PATCH 5/8] t/watch_v1_v2_mix_no_modify: load PublicInbox::Config

[Eric Wong]

For testing with TEST_RUN_MODE=0 (which spawns new processes
instead of reusing them), we can't rely on packages being loaded
by subprocesses influencing the main Perl process.  Thus we must
load PublicInbox::Config explicitly to prevent failures with
this run mode.
---
 t/watch_v1_v2_mix_no_modify.t | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/watch_v1_v2_mix_no_modify.t b/t/watch_v1_v2_mix_no_modify.t
index 759fd0bb..d0e2a78c 100644
--- a/t/watch_v1_v2_mix_no_modify.t
+++ b/t/watch_v1_v2_mix_no_modify.t
@@ -9,6 +9,7 @@ use PublicInbox::InboxIdle;
 use PublicInbox::DS qw(now);
 use PublicInbox::IO qw(write_file);
 use PublicInbox::Eml;
+use PublicInbox::Config;
 my $tmpdir = tmpdir;
 local $ENV{HOME} = "$tmpdir";
 local $ENV{PI_CONFIG} = "$tmpdir/.public-inbox/config";

[PATCH 6/8] cfgwrite: new module to batch commit writes

[Eric Wong]

Hoisting git config writing code into a separate module will
make it easier to use libgit2 to avoid process spawning overhead
in a future change.
---
 MANIFEST                          |  1 +
 lib/PublicInbox/CfgWr.pm          | 54 +++++++++++++++++++++++++++++++
 lib/PublicInbox/LeiMirror.pm      | 23 ++++++-------
 lib/PublicInbox/LeiSavedSearch.pm |  5 +--
 lib/PublicInbox/MultiGit.pm       |  3 +-
 script/public-inbox-init          | 16 +++++----
 6 files changed, 78 insertions(+), 24 deletions(-)
 create mode 100644 lib/PublicInbox/CfgWr.pm

diff --git a/MANIFEST b/MANIFEST
index 2ca7755c..f0b42826 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -164,6 +164,7 @@ lib/PublicInbox/AdminEdit.pm
 lib/PublicInbox/AltId.pm
 lib/PublicInbox/Aspawn.pm
 lib/PublicInbox/AutoReap.pm
+lib/PublicInbox/CfgWr.pm
 lib/PublicInbox/Cgit.pm
 lib/PublicInbox/CidxComm.pm
 lib/PublicInbox/CidxLogP.pm
diff --git a/lib/PublicInbox/CfgWr.pm b/lib/PublicInbox/CfgWr.pm
new file mode 100644
index 00000000..2b3fd507
--- /dev/null
+++ b/lib/PublicInbox/CfgWr.pm
@@ -0,0 +1,54 @@
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+
+# config writer, may use libgit2 in the future
+package PublicInbox::CfgWr;
+use v5.12;
+use PublicInbox::Git qw(git_exe);
+use PublicInbox::Spawn qw(run_die run_wait);
+
+sub new {
+	my ($cls, $f) = @_;
+	bless { -f => $f }, $cls;
+}
+
+sub set {
+	my ($self, $k, $v) = @_;
+	push @{$self->{todo}}, [ $k, $v ];
+	$self;
+}
+
+sub add {
+	my ($self, $k, $v) = @_;
+	push @{$self->{todo}}, [ '--add', $k, $v ];
+	$self;
+}
+
+sub replace_all {
+	my ($self, $k, $v, $re) = @_;
+	push @{$self->{todo}}, [ '--replace-all', $k, $v, $re ];
+	$self;
+}
+
+sub unset_all {
+	my ($self, $k) = @_;
+	push @{$self->{todo}}, [ '--unset-all', $k ];
+	$self;
+}
+
+sub commit {
+	my ($self, $opt) = @_;
+	my @x = (git_exe, 'config', '-f', $self->{-f});
+	for my $c (@{delete $self->{todo} // []}) {
+		unshift @$c, @x;
+		if ($c->[scalar(@x)] eq '--unset-all') {
+			run_wait $c, undef, $opt;
+			# ignore ret=5 if no matches (see git-config(1))
+			die "E: @$c \$?=$?" if ($? && ($? >> 8) != 5);
+		} else {
+			run_die $c, undef, $opt;
+		}
+	}
+}
+
+1;
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 0f14ceba..51d0d9ac 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -19,6 +19,7 @@ use PublicInbox::Config qw(glob2re);
 use PublicInbox::Inbox;
 use PublicInbox::LeiCurl;
 use PublicInbox::OnDestroy;
+use PublicInbox::CfgWr;
 use PublicInbox::SHA qw(sha256_hex sha_all);
 use POSIX qw(strftime);
 use PublicInbox::Admin qw(fmt_localtime);
@@ -415,16 +416,13 @@ 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_exe, "--git-dir=$osdir", qw(config -f), $f ];
+		my $cfgwr = PublicInbox::CfgWr->new($f);
 
 		# clobber settings from previous run atomically
 		for ("remotes.$grp", 'fetch.hideRefs') {
-			my $c = [ @$cmd, '--unset-all', $_ ];
-			$self->{lei}->qerr("# @$c");
-			next if $self->{dry_run};
-			run_wait($c, undef, $opt);
-			die "E: @$c \$?=$?" if ($? && ($? >> 8) != 5);
+			$cfgwr->unset_all($_) if !$self->{dry_run};
 		}
+		$cfgwr->commit($opt);
 
 		# permanent configs:
 		my $cfg = PublicInbox::Config->git_config_dump($f);
@@ -436,12 +434,10 @@ sub fgrp_fetch_all {
 				my ($k, $v) = split(/=/, $_, 2);
 				$k = "remote.$rn.$k";
 				next if ($cfg->{$k} // '') eq $v;
-				my $c = [@$cmd, $k, $v];
-				$fgrp->{lei}->qerr("# @$c");
-				next if $fgrp->{dry_run};
-				run_die($c, undef, $opt);
+				$cfgwr->set($k, $v) if !$fgrp->{dry_run};
 			}
 		}
+		$cfgwr->commit($opt);
 
 		if (!$self->{dry_run}) {
 			# update the config atomically via O_APPEND while
@@ -460,7 +456,7 @@ sub fgrp_fetch_all {
 			write_file '>>', $f, @buf;
 			unlink("$f.lock");
 		}
-		$cmd  = [ @git, "--git-dir=$osdir", @fetch, $grp ];
+		my $cmd = [ @git, "--git-dir=$osdir", @fetch, $grp ];
 		push @$old, @$new;
 		my $end = on_destroy \&fgrpv_done, $old;
 		start_cmd($self, $cmd, $opt, $end);
@@ -809,9 +805,8 @@ sub update_ent {
 	$cur = $self->{-local_manifest}->{$key}->{owner} // "\0";
 	return if $cur eq $new;
 	utf8::encode($new); # to octets
-	my $cmd = [ git_exe, qw(config -f), "$dst/config",
-			'gitweb.owner', $new ];
-	start_cmd($self, $cmd, { 2 => $self->{lei}->{2} });
+	PublicInbox::CfgWr->new(
+		"$dst/config")->set('gitweb.owner', $new)->commit;
 }
 
 sub v1_done { # called via OnDestroy
diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm
index 612d1f43..574cf9aa 100644
--- a/lib/PublicInbox/LeiSavedSearch.pm
+++ b/lib/PublicInbox/LeiSavedSearch.pm
@@ -10,6 +10,7 @@ use PublicInbox::Git qw(git_exe);
 use PublicInbox::OverIdx;
 use PublicInbox::LeiSearch;
 use PublicInbox::Config;
+use PublicInbox::CfgWr;
 use PublicInbox::Spawn qw(run_die);
 use PublicInbox::ContentHash qw(git_sha);
 use PublicInbox::MID qw(mids_for_index);
@@ -179,9 +180,9 @@ EOM
 sub description { $_[0]->{qstr} } # for WWW
 
 sub cfg_set { # called by LeiXSearch
-	my ($self, @args) = @_;
+	my ($self, $k, $v) = @_;
 	my $lk = $self->lock_for_scope; # git-config doesn't wait
-	run_die([git_exe, qw(config -f), $self->{'-f'}, @args]);
+	PublicInbox::CfgWr->new($self->{-f})->set($k, $v)->commit;
 }
 
 # drop-in for LeiDedupe API
diff --git a/lib/PublicInbox/MultiGit.pm b/lib/PublicInbox/MultiGit.pm
index 9be3a876..c99a11cb 100644
--- a/lib/PublicInbox/MultiGit.pm
+++ b/lib/PublicInbox/MultiGit.pm
@@ -7,6 +7,7 @@ use strict;
 use v5.10.1;
 use PublicInbox::Spawn qw(run_die run_qx);
 use PublicInbox::Import;
+use PublicInbox::CfgWr;
 use PublicInbox::Git qw(git_exe);
 use File::Temp 0.19;
 use List::Util qw(max);
@@ -116,7 +117,7 @@ sub epoch_cfg_set {
 		chomp(my $x = run_qx(\@cmd));
 		return if $x eq $v;
 	}
-	run_die [@cmd, $v];
+	PublicInbox::CfgWr->new($f)->set('include.path', $v)->commit;
 }
 
 sub add_epoch {
diff --git a/script/public-inbox-init b/script/public-inbox-init
index b9e234e3..b959fd91 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -163,7 +163,8 @@ my $pi_config_tmp = $fh->filename;
 close($fh);
 
 my $pfx = "publicinbox.$name";
-my @x = (qw/git config/, "--file=$pi_config_tmp");
+require PublicInbox::CfgWr;
+my $cfgwr = PublicInbox::CfgWr->new($pi_config_tmp);
 
 $inboxdir = PublicInbox::Config::rel2abs_collapsed($inboxdir);
 die "`\\n' not allowed in `$inboxdir'\n" if index($inboxdir, "\n") >= 0;
@@ -225,15 +226,15 @@ require PublicInbox::Spawn;
 PublicInbox::Spawn->import(qw(run_die));
 
 for my $addr (grep { !$seen{lc $_} } @address) {
-	run_die([@x, "--add", "$pfx.address", $addr]);
+	$cfgwr->add("$pfx.address", $addr);
 }
-run_die([@x, "$pfx.url", $http_url]) if
+$cfgwr->set("$pfx.url", $http_url) if
 	(!$old_ibx || !grep(/\Q$http_url\E/, @{$old_ibx->{url} // []}));
-run_die([@x, "$pfx.inboxdir", $inboxdir]) if
+$cfgwr->set("$pfx.inboxdir", $inboxdir) if
 	(!$old_ibx || ($old_ibx->{inboxdir} ne $inboxdir));
-run_die([@x, "$pfx.indexlevel", $indexlevel]) if defined($indexlevel) &&
+$cfgwr->set("$pfx.indexlevel", $indexlevel) if defined($indexlevel) &&
 	(!$old_ibx || (($old_ibx->{indexlevel} // '') ne $indexlevel));
-run_die([@x, "$pfx.newsgroup", $ng]) if $ng ne '' &&
+$cfgwr->set("$pfx.newsgroup", $ng) if $ng ne '' &&
 	(!$old_ibx || (($old_ibx->{newsgroup} // '') ne $ng));
 
 for my $kv (@c_extra) {
@@ -242,8 +243,9 @@ for my $kv (@c_extra) {
 	# but that's too new to depend on in 2021.  Perl quotemeta
 	# seems compatible enough for POSIX ERE which git uses
 	my $re = '^'.quotemeta($v).'$';
-	run_die([@x, qw(--replace-all), "$pfx.$k", $v, $re]);
+	$cfgwr->replace_all("$pfx.$k", $v, $re);
 }
+$cfgwr->commit;
 
 # needed for git prior to v2.1.0
 chmod($perm & 07777, $pi_config_tmp);

[PATCH 7/8] rename Gcf2 -> Lg2 for general libgit2 use

[Eric Wong]

We'll be exploring libgit2 for writing (and possibly reading) config
files, so the `gcf2' (git-cat-file-2) name isn't appropriate
anymore for the package name.  We'll still keep `gcf2_' for
subroutine names related to git-cat-file-like behavior.
---
 Documentation/public-inbox-tuning.pod     |  2 +-
 MANIFEST                                  |  4 ++--
 lib/PublicInbox/Gcf2Client.pm             | 10 +++++-----
 lib/PublicInbox/{Gcf2.pm => Lg2.pm}       | 13 ++++++-------
 lib/PublicInbox/SearchView.pm             |  2 +-
 lib/PublicInbox/Spawn.pm                  |  2 +-
 lib/PublicInbox/TestCommon.pm             |  2 +-
 lib/PublicInbox/ViewVCS.pm                |  2 +-
 lib/PublicInbox/{gcf2_libgit2.h => lg2.h} |  2 +-
 t/gcf2.t                                  | 12 ++++++------
 t/gcf2_client.t                           |  2 +-
 11 files changed, 26 insertions(+), 27 deletions(-)
 rename lib/PublicInbox/{Gcf2.pm => Lg2.pm} (92%)
 rename lib/PublicInbox/{gcf2_libgit2.h => lg2.h} (98%)

diff --git a/Documentation/public-inbox-tuning.pod b/Documentation/public-inbox-tuning.pod
index b56c2b10..3f2c0861 100644
--- a/Documentation/public-inbox-tuning.pod
+++ b/Documentation/public-inbox-tuning.pod
@@ -71,7 +71,7 @@ public-inbox processes.
 If libgit2 development files are installed and L<Inline::C>
 is enabled (described above), per-inbox C<git cat-file --batch>
 processes are replaced with a single L<perl(1)> process running
-C<PublicInbox::Gcf2::loop> in read-only daemons.  libgit2 use
+C<PublicInbox::Lg2::gcf2_loop> in read-only daemons.  libgit2 use
 will be available in public-inbox 1.7.0+
 
 More (optional) L<Inline::C> use will be introduced in the future
diff --git a/MANIFEST b/MANIFEST
index f0b42826..8f35c1ad 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -203,7 +203,6 @@ lib/PublicInbox/Filter/Mirror.pm
 lib/PublicInbox/Filter/RubyLang.pm
 lib/PublicInbox/Filter/SubjectTag.pm
 lib/PublicInbox/Filter/Vger.pm
-lib/PublicInbox/Gcf2.pm
 lib/PublicInbox/Gcf2Client.pm
 lib/PublicInbox/GetlineResponse.pm
 lib/PublicInbox/Git.pm
@@ -294,6 +293,7 @@ lib/PublicInbox/LeiUp.pm
 lib/PublicInbox/LeiViewText.pm
 lib/PublicInbox/LeiWatch.pm
 lib/PublicInbox/LeiXSearch.pm
+lib/PublicInbox/Lg2.pm
 lib/PublicInbox/Limiter.pm
 lib/PublicInbox/Linkify.pm
 lib/PublicInbox/Listener.pm
@@ -388,8 +388,8 @@ lib/PublicInbox/XapHelperCxx.pm
 lib/PublicInbox/Xapcmd.pm
 lib/PublicInbox/XhcMset.pm
 lib/PublicInbox/XhcMsetIterator.pm
-lib/PublicInbox/gcf2_libgit2.h
 lib/PublicInbox/khashl.h
+lib/PublicInbox/lg2.h
 lib/PublicInbox/xap_helper.h
 lib/PublicInbox/xh_cidx.h
 lib/PublicInbox/xh_mset.h
diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 07ff7dcb..17bf34c7 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -1,12 +1,12 @@
 # Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
-# connects public-inbox processes to PublicInbox::Gcf2::loop()
+# connects public-inbox processes to PublicInbox::Lg2::gcf2_loop()
 package PublicInbox::Gcf2Client;
 use v5.12;
 use parent qw(PublicInbox::DS);
 use PublicInbox::Git;
-use PublicInbox::Gcf2; # fails if Inline::C or libgit2-dev isn't available
+use PublicInbox::Lg2; # fails if Inline::C or libgit2-dev isn't available
 use PublicInbox::Spawn qw(spawn);
 use Socket qw(AF_UNIX SOCK_STREAM);
 use PublicInbox::Syscall qw(EPOLLIN);
@@ -14,10 +14,10 @@ use PublicInbox::IO;
 use autodie qw(socketpair);
 
 # fields:
-#	sock => socket to Gcf2::loop
+#	sock => socket to Lg2::gcf2_loop
 # The rest of these fields are compatible with what PublicInbox::Git
 # uses code-sharing
-#	pid => PID of Gcf2::loop process
+#	pid => PID of Lg2::gcf2_loop process
 #	pid.owner => process which spawned {pid}
 #	in => same as {sock}, for compatibility with PublicInbox::Git
 #	inflight => array (see PublicInbox::Git)
@@ -30,7 +30,7 @@ sub new  {
 	$s1->blocking(0);
 	$opt->{0} = $opt->{1} = $s2;
 	my $cmd = [$^X, $^W ? ('-w') : (),
-			qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop]];
+			qw[-MPublicInbox::Lg2 -e PublicInbox::Lg2::gcf2_loop]];
 	$self->{inflight} = [];
 	PublicInbox::IO::attach_pid($s1, spawn($cmd, $env, $opt),
 			\&PublicInbox::Git::gcf_drain, $self->{inflight});
diff --git a/lib/PublicInbox/Gcf2.pm b/lib/PublicInbox/Lg2.pm
similarity index 92%
rename from lib/PublicInbox/Gcf2.pm
rename to lib/PublicInbox/Lg2.pm
index acc2091c..0ee9b354 100644
--- a/lib/PublicInbox/Gcf2.pm
+++ b/lib/PublicInbox/Lg2.pm
@@ -1,9 +1,8 @@
 # Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
-# backend for a git-cat-file-workalike based on libgit2,
-# other libgit2 stuff may go here, too.
-package PublicInbox::Gcf2;
+# including backend for a git-cat-file-workalike based on libgit2,
+package PublicInbox::Lg2;
 use v5.12;
 use PublicInbox::Spawn qw(which run_qx); # may set PERL_INLINE_DIRECTORY
 use Fcntl qw(SEEK_SET);
@@ -35,7 +34,7 @@ BEGIN {
 		die "E: libgit2 not installed: $err\n" if $?;
 		$vals->{$k} = $val;
 	}
-	my $f = "$dir/gcf2_libgit2.h";
+	my $f = "$dir/lg2.h";
 	$c_src = PublicInbox::IO::try_cat $f or die "cat $f: $!";
 	# append pkg-config results to the source to ensure Inline::C
 	# can rebuild if there's changes (it doesn't seem to detect
@@ -62,7 +61,7 @@ EOM
 		seek($fh, 0, SEEK_SET);
 		my @msg = <$fh>;
 		truncate($fh, 0);
-		die "Inline::C Gcf2 build failed:\n", $err, "\n", @msg;
+		die "Inline::C Lg2 build failed:\n", $err, "\n", @msg;
 	}
 }
 
@@ -86,9 +85,9 @@ sub add_alt ($$) {
 	1;
 }
 
-# Usage: $^X -MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop [EXPIRE-TIMEOUT]
+# Usage: $^X -MPublicInbox::Lg2 -e PublicInbox::Lg2::gcf2_loop [EXPIRE-TIMEOUT]
 # (see lib/PublicInbox/Gcf2Client.pm)
-sub loop (;$) {
+sub gcf2_loop (;$) {
 	my $exp = $_[0] || $ARGV[0] || 60; # seconds
 	my $gcf2 = new();
 	my (%seen, $check_at);
diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index 8ae422f9..8464ae1b 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -169,7 +169,7 @@ sub path2inc ($) {
 	if (my $short = $rmap_inc{$full}) {
 		return $short;
 	} elsif (!scalar(keys %rmap_inc) && -e $full) {
-		# n.b. $INC{'PublicInbox::Gcf2'} is undef if libgit2-dev
+		# n.b. $INC{'PublicInbox::Lg2'} is undef if libgit2-dev
 		# doesn't exist
 		my $f;
 		%rmap_inc = map {;
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index d818af0f..f79623b7 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -330,7 +330,7 @@ EOM
 			$all_libc = undef;
 		}
 	}
-	if (defined $all_libc) { # set for Gcf2
+	if (defined $all_libc) { # set for Lg2
 		$ENV{PERL_INLINE_DIRECTORY} = $inline_dir;
 		%RLIMITS = rlimit_map();
 		*send_cmd4 = sub ($$$$;$) {
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index e1178149..a9967735 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -266,7 +266,7 @@ sub require_mods (@) {
 			eval "require $mod";
 		}
 		if ($@) {
-			diag "require $mod: $@" if $mod =~ /Gcf2/;
+			diag "require $mod: $@" if $mod =~ /Lg2/;
 			push @need, $mod;
 		} elsif ($mod eq 'IO::Socket::SSL' &&
 			# old versions of IO::Socket::SSL aren't supported
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index e1e129b1..552e3241 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -129,7 +129,7 @@ sub cmt_title { # git->cat_async callback
 
 sub do_cat_async {
 	my ($arg, $cb, @req) = @_;
-	# favor git(1) over Gcf2 (libgit2) for SHA-256 support
+	# favor git(1) over Lg2 (libgit2) for SHA-256 support
 	my $ctx = ref $arg eq 'ARRAY' ? $arg->[0] : $arg;
 	$ctx->{git}->cat_async($_, $cb, $arg) for @req;
 	if ($ctx->{env}->{'pi-httpd.async'}) {
diff --git a/lib/PublicInbox/gcf2_libgit2.h b/lib/PublicInbox/lg2.h
similarity index 98%
rename from lib/PublicInbox/gcf2_libgit2.h
rename to lib/PublicInbox/lg2.h
index e1f0ef39..0b1a33e0 100644
--- a/lib/PublicInbox/gcf2_libgit2.h
+++ b/lib/PublicInbox/lg2.h
@@ -29,7 +29,7 @@ SV *new()
 
 	ref = newSViv((IV)odb);
 	self = newRV_noinc(ref);
-	sv_bless(self, gv_stashpv("PublicInbox::Gcf2", GV_ADD));
+	sv_bless(self, gv_stashpv("PublicInbox::Lg2", GV_ADD));
 	SvREADONLY_on(ref);
 
 	return self;
diff --git a/t/gcf2.t b/t/gcf2.t
index 9f9e8e20..fc687058 100644
--- a/t/gcf2.t
+++ b/t/gcf2.t
@@ -10,14 +10,14 @@ use Fcntl qw(:seek);
 use IO::Handle ();
 use POSIX qw(_exit);
 use Cwd qw(abs_path);
-require_mods('PublicInbox::Gcf2');
-use_ok 'PublicInbox::Gcf2';
+require_mods('PublicInbox::Lg2');
+use_ok 'PublicInbox::Lg2';
 use PublicInbox::Syscall qw($F_SETPIPE_SZ);
 use PublicInbox::Import;
 my ($tmpdir, $for_destroy) = tmpdir();
 
-my $gcf2 = PublicInbox::Gcf2::new();
-is(ref($gcf2), 'PublicInbox::Gcf2', '::new works');
+my $gcf2 = PublicInbox::Lg2::new();
+is(ref($gcf2), 'PublicInbox::Lg2', '::new works');
 my $COPYING = 'dba13ed2ddf783ee8118c6a581dbf75305f816a3';
 open my $agpl, '<', 'COPYING' or BAIL_OUT "AGPL-3 missing: $!";
 $agpl = do { local $/; <$agpl> };
@@ -102,7 +102,7 @@ SKIP: {
 	ok($gcf2->cat_oid(fileno($fh), $COPYING), 'cat_oid normal');
 	$ck_copying->('regular file');
 
-	$gcf2 = PublicInbox::Gcf2::new();
+	$gcf2 = PublicInbox::Lg2::new();
 	$gcf2->add_alternate("$tmpdir/objects");
 	open $fh, '+>', undef or BAIL_OUT "open: $!";
 	ok($gcf2->cat_oid(fileno($fh), $COPYING), 'cat_oid alternate');
@@ -150,7 +150,7 @@ if ($nr) {
 	close $r;
 	my $broken = fileno($w);
 	for (1..$nr) {
-		my $obj = PublicInbox::Gcf2::new();
+		my $obj = PublicInbox::Lg2::new();
 		if (defined($objdir)) {
 			$obj->add_alternate($objdir);
 			for (1..$cat) {
diff --git a/t/gcf2_client.t b/t/gcf2_client.t
index 33ee2c91..78921280 100644
--- a/t/gcf2_client.t
+++ b/t/gcf2_client.t
@@ -8,7 +8,7 @@ use autodie qw(open close);
 use PublicInbox::Import;
 use PublicInbox::DS;
 
-require_mods('PublicInbox::Gcf2');
+require_mods('PublicInbox::Lg2');
 use_ok 'PublicInbox::Gcf2Client';
 my ($tmpdir, $for_destroy) = tmpdir();
 my $git_a = "$tmpdir/a.git";

[PATCH 8/8] lg2: use cfgwr_commit to write to configs using libgit2

[Eric Wong]

libgit2 allows us to avoid process spawning overhead when
writing to the config file.  While the performance difference is
unlikely to matter in real-world use, it should improve
maintainability of tests by allowing us to write tests with
`public-inbox-init' with a smaller performance penalty (as
opposed to using `PublicInbox::IO::write_file' to setup
configs).
---
 MANIFEST                    |  1 +
 lib/PublicInbox/CfgWr.pm    |  6 ++++
 lib/PublicInbox/LEI.pm      |  1 +
 lib/PublicInbox/MultiGit.pm |  1 +
 lib/PublicInbox/lg2.h       | 58 ++++++++++++++++++++++++++++++++++-
 t/lg2_cfg.t                 | 60 +++++++++++++++++++++++++++++++++++++
 xt/check-run.t              |  1 +
 7 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 t/lg2_cfg.t

diff --git a/MANIFEST b/MANIFEST
index 8f35c1ad..d4535038 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -550,6 +550,7 @@ t/lei_saved_search.t
 t/lei_store.t
 t/lei_to_mail.t
 t/lei_xsearch.t
+t/lg2_cfg.t
 t/linkify.t
 t/main-bin/spamc
 t/mbox_lock.t
diff --git a/lib/PublicInbox/CfgWr.pm b/lib/PublicInbox/CfgWr.pm
index 2b3fd507..39f22e19 100644
--- a/lib/PublicInbox/CfgWr.pm
+++ b/lib/PublicInbox/CfgWr.pm
@@ -6,6 +6,10 @@ package PublicInbox::CfgWr;
 use v5.12;
 use PublicInbox::Git qw(git_exe);
 use PublicInbox::Spawn qw(run_die run_wait);
+my $cfgwr_commit = eval {
+	require PublicInbox::Lg2;
+	PublicInbox::Lg2->can('cfgwr_commit');
+};
 
 sub new {
 	my ($cls, $f) = @_;
@@ -38,6 +42,8 @@ sub unset_all {
 
 sub commit {
 	my ($self, $opt) = @_;
+	my $todo = delete $self->{todo} // return;
+	return $cfgwr_commit->($self->{-f}, $todo) if $cfgwr_commit;
 	my @x = (git_exe, 'config', '-f', $self->{-f});
 	for my $c (@{delete $self->{todo} // []}) {
 		unshift @$c, @x;
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 1e8dc17f..94bac688 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -20,6 +20,7 @@ use Fcntl qw(SEEK_SET);
 use PublicInbox::Config;
 use PublicInbox::Syscall qw(EPOLLIN);
 use PublicInbox::Spawn qw(run_wait popen_rd run_qx);
+eval { require PublicInbox::Lg2 }; # placate FindBin
 use PublicInbox::Lock;
 use PublicInbox::Eml;
 use PublicInbox::Git qw(git_exe);
diff --git a/lib/PublicInbox/MultiGit.pm b/lib/PublicInbox/MultiGit.pm
index c99a11cb..1ef6bdb1 100644
--- a/lib/PublicInbox/MultiGit.pm
+++ b/lib/PublicInbox/MultiGit.pm
@@ -116,6 +116,7 @@ sub epoch_cfg_set {
 	if (-r $f) {
 		chomp(my $x = run_qx(\@cmd));
 		return if $x eq $v;
+		$? = 0; # don't influence _wq_done_wait in -clone
 	}
 	PublicInbox::CfgWr->new($f)->set('include.path', $v)->commit;
 }
diff --git a/lib/PublicInbox/lg2.h b/lib/PublicInbox/lg2.h
index 0b1a33e0..fb771bd4 100644
--- a/lib/PublicInbox/lg2.h
+++ b/lib/PublicInbox/lg2.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
+ * Copyright (C) all contributors <meta@public-inbox.org>
  * License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
  *
  * libgit2 for Inline::C
@@ -140,3 +140,59 @@ int cat_oid(SV *self, int fd, SV *oidsv)
 
 	return rc == GIT_OK;
 }
+
+#define CFG_OP_EQ(op, cmd0, dlen) \
+	(dlen == (sizeof(op) - 1) && !memcmp(op, cmd0, sizeof(op) - 1))
+
+/* leaks on internal bugs: */
+#define FETCH_CSTR(var, ary, i) do { \
+	SV **s = av_fetch(ary, i, 0); \
+	if (!s) croak("BUG: " #var " = ary[%d] is NULL", i); \
+	var = SvPV_nolen(*s); \
+} while (0)
+
+void cfgwr_commit(const char *f, AV *todo)
+{
+	I32 i, todo_max = av_len(todo);
+	SV **sv;
+	git_config *cfg;
+	const git_error *e;
+	const char *cmd0, *name, *val, *re;
+	int rc = git_config_open_ondisk(&cfg, f);
+	STRLEN dlen;
+	AV *cmd;
+
+	for (i = 0; rc == GIT_OK && i <= todo_max; i++) {
+		sv = av_fetch(todo, i, 0);
+		/* leaks on internal bugs: */
+		if (!SvROK(*sv)) croak("BUG: not a ref");
+		cmd = (AV *)SvRV(*sv);
+		sv = av_fetch(cmd, 0, 0);
+		if (!sv) croak("BUG: cmd0 = $todo->[%d]->[0] is NULL", i);
+		cmd0 = SvPV(*sv, dlen);
+		if (CFG_OP_EQ("--add", cmd0, dlen)) {
+			FETCH_CSTR(name, cmd, 1);
+			FETCH_CSTR(val, cmd, 2);
+			rc = git_config_set_multivar(cfg, name, "$^", val);
+		} else if (CFG_OP_EQ("--unset-all", cmd0, dlen)) {
+			FETCH_CSTR(name, cmd, 1);
+			rc = git_config_delete_multivar(cfg, name, ".*");
+			if (rc == GIT_ENOTFOUND)
+				rc = GIT_OK;
+		} else if (CFG_OP_EQ("--replace-all", cmd0, dlen)) {
+			FETCH_CSTR(name, cmd, 1);
+			FETCH_CSTR(val, cmd, 2);
+			FETCH_CSTR(re, cmd, 3);
+			rc = git_config_set_multivar(cfg, name, re, val);
+		} else {
+			name = cmd0;
+			FETCH_CSTR(val, cmd, 1);
+			rc = git_config_set_string(cfg, name, val);
+		}
+	}
+	e = rc == GIT_OK ? NULL : giterr_last();
+	git_config_free(cfg);
+	if (rc != GIT_OK)
+		croak("config -f %s (%d %s)",
+			f, rc, e ? e->message : "unknown");
+}
diff --git a/t/lg2_cfg.t b/t/lg2_cfg.t
new file mode 100644
index 00000000..e8cdff4a
--- /dev/null
+++ b/t/lg2_cfg.t
@@ -0,0 +1,60 @@
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use PublicInbox::TestCommon;
+require_mods 'PublicInbox::Lg2';
+use PublicInbox::Git qw(git_exe);
+use PublicInbox::Spawn qw(run_die run_wait);
+use_ok 'PublicInbox::Lg2';
+use PublicInbox::Config;
+my $tmpdir = tmpdir;
+my $f = "$tmpdir/cfg";
+my $cfgwr_commit = $ENV{TEST_VALIDATE_GIT_BEHAVIOR} ? sub {
+	my ($file, $todo) = @_;
+	my @x = (git_exe, 'config', '-f', $file);
+	for my $c (@$todo) {
+		unshift @$c, @x;
+		if ($c->[scalar(@x)] eq '--unset-all') {
+			run_wait $c;
+			# ignore ret=5 if no matches (see git-config(1))
+			die "E: @$c \$?=$?" if ($? && ($? >> 8) != 5);
+		} else {
+			run_die $c;
+		}
+	}
+} : PublicInbox::Lg2->can('cfgwr_commit');
+
+my $cfg; # for explain() if needed
+my $get = sub {
+	my (@key) = @_;
+	$cfg = PublicInbox::Config->new($f);
+	@key > 1 ? (map { $_ => $cfg->{$_} } @key) : $cfg->{$key[0]};
+};
+
+$cfgwr_commit->($f, [ [ qw(a.b a) ] ]);
+is $get->('a.b'), 'a', 'config set works';
+
+$cfgwr_commit->($f, [ [ qw(--add a.b a) ] ]);
+is_xdeeply $get->('a.b'), [ qw(a a) ], 'config --add works to append';
+$cfgwr_commit->($f, [ [ qw(--add x.b c) ] ]);
+my %cfg = $get->('x.b', 'a.b');
+is $cfg{'x.b'}, 'c', 'config --add works to create';
+is_xdeeply $cfg{'a.b'}, [ qw(a a) ], 'config --add left existing alone';
+
+$cfgwr_commit->($f, [ [ qw(--unset-all a.b) ] ]);
+is $get->('a.b'), undef, 'config --unset-all works';
+
+$cfgwr_commit->($f, [ [ qw(--unset-all bogus.entry) ] ]);
+is $get->('bogus.entry'), undef, 'config --unset-all non-match';
+
+$cfgwr_commit->($f, [ [ qw(x.b d) ] ]);
+is $get->('x.b'), 'd', 'set clobbers existing value';
+
+eval { $cfgwr_commit->($f, []) };
+ok !$@, 'no exception or errors on empty todo';
+
+$cfgwr_commit->($f, [ [ qw(x.b d) ], [ qw(--add x.b e) ],
+	[ qw(--add x.b f) ] ]);
+is_xdeeply $get->('x.b'), [ qw(d e f) ], 'multiple changes chained';
+
+done_testing;
diff --git a/xt/check-run.t b/xt/check-run.t
index d12b925d..162c1acd 100755
--- a/xt/check-run.t
+++ b/xt/check-run.t
@@ -14,6 +14,7 @@ use v5.12;
 use IO::Handle; # ->autoflush
 use PublicInbox::TestCommon;
 use PublicInbox::Spawn;
+eval { require PublicInbox::Lg2 }; # placate FindBin
 use PublicInbox::DS; # already loaded by Spawn via PublicInbox::IO
 use Getopt::Long qw(:config gnu_getopt no_ignore_case auto_abbrev);
 use Errno qw(EINTR);