From: Eric Wong <e@80x24.org>
Date: Sat, 15 Feb 2025 11:10:11 +0000 (+0000)
Subject: lg2: use cfgwr_commit to write to configs using libgit2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a8073f6c1136d16d5aafd6d084eb0f6aca63c76e;p=thirdparty%2Fpublic-inbox.git

lg2: use cfgwr_commit to write to configs using libgit2

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).
---

diff --git a/MANIFEST b/MANIFEST
index 8f35c1adb..d45350381 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 2b3fd5078..39f22e19e 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 1e8dc17f0..94bac6885 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 c99a11cbe..1ef6bdb1c 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 0b1a33e0c..fb771bd47 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 000000000..e8cdff4ae
--- /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 d12b925d1..162c1acdd 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);