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 8f35c1adb0383ec1d3ec695b34bef294f3b20e41..d45350381150a422fd7afd0375d80c66b8019f2d 100644 (file)
--- 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 2b3fd50787ba2d6629bc50e6341dfa270b67eada..39f22e19e09c4be3942f373e864beee284af1f39 100644 (file)
--- 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 1e8dc17f076c39bad147d57a37ce31118d5742fd..94bac688596bb9e759a918064ea7fc3c95236e61 100644 (file)
--- 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 c99a11cbe375c656684353275b7ac844a7b4a718..1ef6bdb1c4ceb04a172da880c6bc7f1c514754a4 100644 (file)
--- 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 0b1a33e0c324ebda2d2b30daba7c8025976882e2..fb771bd476a010e6fe1d4ee896c34d5ba7fb4464 100644 (file)
--- 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 (file)
index 0000000..e8cdff4
--- /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 d12b925d1d7b337b12de2cc4465f18e70b87849a..162c1acddc0163619493d0c14f5b592cee8850fe 100755 (executable)
--- 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);