We can pass `-c NAME=VALUE' args directly to git-config without
needing a temporary directory nor file. Furthermore, this opens
the door to us being able to correctly handle `-c NAME=VALUE'
after `delete $lei->{cfg}' if we need to reload the config
during a command.
This tightens up error-checking for `lei config' and ensures we
can make config settings changes while using `-c NAME=VALUE'
instead of editing the temporary file.
The non-obvious part was avoiding the use of the -f/--file arg for
`git config' for read-only operations and include relying on
`-c include.path=$ABS_PATH'. This is done by parsing the
switches to be passed to `git config' to determine if it's a
read-only operation or not.
---
lib/PublicInbox/Config.pm | 52 +++++++++++++++-----
lib/PublicInbox/LEI.pm | 95 +++++++++++++++++++-----------------
lib/PublicInbox/LeiConfig.pm | 23 ++++++---
lib/PublicInbox/LeiMirror.pm | 5 +-
t/lei.t | 17 ++++++-
5 files changed, 123 insertions(+), 69 deletions(-)
diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index f6236d84..533f4a52 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -22,7 +22,7 @@ sub _array ($) { ref($_[0]) eq 'ARRAY' ? $_[0] : [ $_[0] ] }
# returns key-value pairs of config directives in a hash
# if keys may be multi-value, the value is an array ref containing all values
sub new {
- my ($class, $file, $errfh) = @_;
+ my ($class, $file, $lei) = @_;
$file //= default_file();
my $self;
my $set_dedupe;
@@ -36,7 +36,7 @@ sub new {
$self = $DEDUPE->{$file} and return $self;
$set_dedupe = 1;
}
- $self = git_config_dump($class, $file, $errfh);
+ $self = git_config_dump($class, $file, $lei);
$self->{'-f'} = $file;
}
# caches
@@ -174,13 +174,34 @@ sub config_fh_parse ($$$) {
\%rv;
}
+sub tmp_cmd_opt ($$) {
+ my ($env, $opt) = @_;
+ # quiet global and system gitconfig if supported by installed git,
+ # but normally harmless if too noisy (NOGLOBAL no longer exists)
+ $env->{GIT_CONFIG_NOSYSTEM} = 1;
+ $env->{GIT_CONFIG_GLOBAL} = '/dev/null'; # git v2.32+
+ $opt->{-C} = '/'; # avoid $worktree/.git/config on MOST systems :P
+}
+
sub git_config_dump {
- my ($class, $file, $errfh) = @_;
- return bless {}, $class unless -e $file;
- my $cmd = [ qw(git config -z -l --includes), "--file=$file" ];
- my $fh = popen_rd($cmd, undef, { 2 => $errfh // 2 });
+ my ($class, $file, $lei) = @_;
+ my @opt_c = map { ('-c', $_) } @{$lei->{opt}->{c} // []};
+ $file = undef if !-e $file;
+ # XXX should we set {-f} if !-e $file?
+ return bless {}, $class if (!@opt_c && !defined($file));
+ my %env;
+ my $opt = { 2 => $lei->{2} // 2 };
+ if (@opt_c) {
+ 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));
+ push(@cmd, '-f', $file) if !@opt_c && defined($file);
+ my $fh = popen_rd(\@cmd, \%env, $opt);
my $rv = config_fh_parse($fh, "\0", "\n");
- close $fh or die "@$cmd failed: \$?=$?\n";
+ close $fh or die "@cmd failed: \$?=$?\n";
+ $rv->{-opt_c} = \@opt_c if @opt_c; # for ->urlmatch
+ $rv->{-f} = $file;
bless $rv, $class;
}
@@ -544,14 +565,23 @@ sub _fill_ei ($$) {
$es;
}
+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');
+ @opt_c ? tmp_cmd_opt($env, $opt) : push(@cmd, '-f', $f);
+ \@cmd;
+}
+
sub urlmatch {
my ($self, $key, $url, $try_git) = @_;
state $urlmatch_broken; # requires git 1.8.5
return if $urlmatch_broken;
- my $file = $self->{'-f'} // default_file();
- my $cmd = [qw/git config -z --includes --get-urlmatch/,
- "--file=$file", $key, $url ];
- my $fh = popen_rd($cmd);
+ my (%env, %opt);
+ my $cmd = $self->config_cmd(\%env, \%opt);
+ push @$cmd, qw(-z --includes --get-urlmatch), $key, $url;
+ my $fh = popen_rd($cmd, \%env, \%opt);
local $/ = "\0";
my $val = <$fh>;
if (!close($fh)) {
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 488006e0..8b62def2 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -281,7 +281,8 @@ our %CMD = ( # sorted in order of importance/use:
"use a patch to generate a query for `lei q --stdin'",
qw(stdin| in-format|F=s want|w=s@ uri debug), @net_opt, @c_opt ],
'config' => [ '[...]', sub {
- 'git-config(1) wrapper for '._config_path($_[0]);
+ 'git-config(1) wrapper for '._config_path($_[0]). "\n" .
+ '-l/--list and other common git-config uses are supported'
}, qw(config-file|system|global|file|f=s), # for conflict detection
qw(edit|e c=s@ C=s@), pass_through('git config') ],
'inspect' => [ 'ITEMS...|--stdin', 'inspect lei/store and/or local external',
@@ -456,6 +457,7 @@ my %OPTDESC = (
'z|0' => 'use NUL \\0 instead of newline (CR) to delimit lines',
'signal|s=s' => [ 'SIG', 'signal to send lei-daemon (default: TERM)' ],
+'edit|e config' => 'open an editor to modify the lei config file',
); # %OPTDESC
my %CONFIG_KEYS = (
@@ -760,36 +762,6 @@ sub optparse ($$$) {
$err ? fail($self, "usage: lei $cmd $proto\nE: $err") : 1;
}
-sub _tmp_cfg { # for lei -c <name>=<value> ...
- my ($self) = @_;
- my $cfg = _lei_cfg($self, 1);
- require File::Temp;
- my $ft = File::Temp->new(TEMPLATE => 'lei_cfg-XXXX', TMPDIR => 1);
- my $tmp = { '-f' => $ft->filename, -tmp => $ft };
- $ft->autoflush(1);
- print $ft <<EOM or return fail($self, "$tmp->{-f}: $!");
-[include]
- path = $cfg->{-f}
-EOM
- $tmp = $self->{cfg} = bless { %$cfg, %$tmp }, ref($cfg);
- for (@{$self->{opt}->{c}}) {
- /\A([^=\.]+\.[^=]+)(?:=(.*))?\z/ or return fail($self, <<EOM);
-`-c $_' is not of the form -c <name>=<value>'
-EOM
- my ($name, $value) = ($1, $2 // 1);
- _config($self, '--add', $name, $value) or return;
- if (defined(my $v = $tmp->{$name})) {
- if (ref($v) eq 'ARRAY') {
- push @$v, $value;
- } else {
- $tmp->{$name} = [ $v, $value ];
- }
- } else {
- $tmp->{$name} = $value;
- }
- }
-}
-
sub lazy_cb ($$$) { # $pfx is _complete_ or lei_
my ($self, $cmd, $pfx) = @_;
my $ucmd = $cmd;
@@ -819,7 +791,6 @@ sub dispatch {
}
if (my $cb = lazy_cb(__PACKAGE__, $cmd, 'lei_')) {
optparse($self, $cmd, \@argv) or return;
- $self->{opt}->{c} and (_tmp_cfg($self) // return);
if (my $chdir = $self->{opt}->{C}) {
for my $d (@$chdir) {
next if $d eq ''; # same as git(1)
@@ -844,17 +815,20 @@ sub _lei_cfg ($;$) {
my $f = _config_path($self);
my @st = stat($f);
my $cur_st = @st ? pack('dd', $st[10], $st[7]) : ''; # 10:ctime, 7:size
- my ($sto, $sto_dir, $watches, $lne);
- if (my $cfg = $PATH2CFG{$f}) { # reuse existing object in common case
- return ($self->{cfg} = $cfg) if $cur_st eq $cfg->{-st};
+ my ($sto, $sto_dir, $watches, $lne, $cfg);
+ if ($cfg = $PATH2CFG{$f}) { # reuse existing object in common case
+ ($cur_st eq $cfg->{-st} && !$self->{opt}->{c}) and
+ return ($self->{cfg} = $cfg);
+ # reuse some fields below if they match:
($sto, $sto_dir, $watches, $lne) =
@$cfg{qw(-lei_store leistore.dir -watches
-lei_note_event)};
}
if (!@st) {
- unless ($creat) {
- delete $self->{cfg};
- return bless {}, 'PublicInbox::Config';
+ unless ($creat) { # any commands which write to cfg must creat
+ $cfg = PublicInbox::Config->git_config_dump(
+ '/dev/null', $self);
+ return ($self->{cfg} = $cfg);
}
my ($cfg_dir) = ($f =~ m!(.*?/)[^/]+\z!);
File::Path::mkpath($cfg_dir);
@@ -863,9 +837,8 @@ sub _lei_cfg ($;$) {
$cur_st = pack('dd', $st[10], $st[7]);
qerr($self, "# $f created") if $self->{cmd} ne 'config';
}
- my $cfg = PublicInbox::Config->git_config_dump($f, $self->{2});
+ $cfg = PublicInbox::Config->git_config_dump($f, $self);
$cfg->{-st} = $cur_st;
- $cfg->{'-f'} = $f;
if ($sto && canonpath_harder($sto_dir // store_path($self))
eq canonpath_harder($cfg->{'leistore.dir'} //
store_path($self))) {
@@ -877,7 +850,7 @@ sub _lei_cfg ($;$) {
# FIXME: use inotify/EVFILT_VNODE to detect unlinked configs
delete(@PATH2CFG{grep(!-f, keys %PATH2CFG)});
}
- $self->{cfg} = $PATH2CFG{$f} = $cfg;
+ $self->{cfg} = $self->{opt}->{c} ? $cfg : ($PATH2CFG{$f} = $cfg);
refresh_watches($self);
$cfg;
}
@@ -898,11 +871,41 @@ sub _lei_store ($;$) {
sub _config {
my ($self, @argv) = @_;
my $err_ok = ($argv[0] // '') eq '+e' ? shift(@argv) : undef;
- my %env = (%{$self->{env}}, GIT_CONFIG => undef);
+ my %env;
+ my %opt = map { $_ => $self->{$_} } (0..2);
my $cfg = _lei_cfg($self, 1);
- my $cmd = [ qw(git config -f), $cfg->{'-f'}, @argv ];
- my %rdr = map { $_ => $self->{$_} } (0..2);
- waitpid(spawn($cmd, \%env, \%rdr), 0);
+ my $opt_c = delete local $cfg->{-opt_c};
+ my @file_arg;
+ if ($opt_c) {
+ my ($set, $get, $nondash);
+ for (@argv) { # order matters for git-config
+ if (!$nondash) {
+ if (/\A--(?:add|rename-section|remove-section|
+ replace-all|
+ unset-all|unset)\z/x) {
+ ++$set;
+ } elsif ($_ eq '-l' || $_ eq '--list' ||
+ /\A--get/) {
+ ++$get;
+ } elsif (/\A-/) { # -z and such
+ } else {
+ ++$nondash;
+ }
+ } else {
+ ++$nondash;
+ }
+ }
+ if ($set || ($nondash//0) > 1 && !$get) {
+ @file_arg = ('-f', $cfg->{-f});
+ $env{GIT_CONFIG} = $file_arg[1];
+ } else { # OK, we can use `-c n=v' for read-only
+ $cfg->{-opt_c} = $opt_c;
+ $env{GIT_CONFIG} = undef;
+ }
+ }
+ my $cmd = $cfg->config_cmd(\%env, \%opt);
+ push @$cmd, @file_arg, @argv;
+ waitpid(spawn($cmd, \%env, \%opt), 0);
$? == 0 ? 1 : ($err_ok ? undef : fail($self, $?));
}
@@ -1545,7 +1548,7 @@ sub sto_done_request {
sub cfg_dump ($$) {
my ($lei, $f) = @_;
- my $ret = eval { PublicInbox::Config->git_config_dump($f, $lei->{2}) };
+ my $ret = eval { PublicInbox::Config->git_config_dump($f, $lei) };
return $ret if !$@;
warn($@);
undef;
diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm
index fd4b0eca..76fc43e7 100644
--- a/lib/PublicInbox/LeiConfig.pm
+++ b/lib/PublicInbox/LeiConfig.pm
@@ -1,8 +1,7 @@
-# Copyright (C) 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>
-package PublicInbox::LeiConfig;
-use strict;
-use v5.10.1;
+package PublicInbox::LeiConfig; # subclassed by LeiEditSearch
+use v5.12;
use PublicInbox::PktOp;
use Fcntl qw(SEEK_SET);
use autodie qw(open seek);
@@ -41,10 +40,18 @@ sub lei_config {
my ($lei, @argv) = @_;
$lei->{opt}->{'config-file'} and return $lei->fail(
"config file switches not supported by `lei config'");
- return $lei->_config(@argv) unless $lei->{opt}->{edit};
- my $f = $lei->_lei_cfg(1)->{-f};
- my $self = bless { lei => $lei, -f => $f }, __PACKAGE__;
- cfg_do_edit($self);
+ if ($lei->{opt}->{edit}) {
+ @argv and return $lei->fail(
+'--edit must be used without other arguments');
+ $lei->{opt}->{c} and return $lei->fail(
+"`-c $lei->{opt}->{c}->[0]' not allowed with --edit");
+ my $f = $lei->_lei_cfg(1)->{-f};
+ cfg_do_edit(bless { lei => $lei, -f => $f }, __PACKAGE__);
+ } elsif (@argv) { # let git-config do error-checking
+ $lei->_config(@argv);
+ } else {
+ $lei->_help('no options given');
+ }
}
1;
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index bed034f1..fed6b668 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -193,7 +193,8 @@ sub _write_inbox_config {
} elsif (!$!{EEXIST}) {
die "open($f): $!";
}
- my $cfg = PublicInbox::Config->git_config_dump($f, $self->{lei}->{2});
+ my $cfg = PublicInbox::Config->git_config_dump($f,
+ { 2 => $self->{lei}->{2} });
my $ibx = $self->{ibx} = {}; # for indexing
for my $sec (grep(/\Apublicinbox\./, @{$cfg->{-section_order}})) {
for (qw(address newsgroup nntpmirror)) {
@@ -238,7 +239,7 @@ sub index_cloned_inbox {
}
# force synchronous awaitpid for v2:
local $PublicInbox::DS::in_loop = 0;
- my $cfg = PublicInbox::Config->new(undef, $lei->{2});
+ my $cfg = PublicInbox::Config->new(undef, { 2 => $lei->{2} });
my $env = PublicInbox::Admin::index_prepare($opt, $cfg);
local %ENV = (%ENV, %$env) if $env;
PublicInbox::Admin::progress_prepare($opt, $lei->{2});
diff --git a/t/lei.t b/t/lei.t
index 1199ca75..3ac804a8 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -40,10 +40,21 @@ my $test_help = sub {
lei_ok(qw(config -h));
like($lei_out, qr! \Q$home\E/\.config/lei/config\b!,
'actual path shown in config -h');
+ my $exp_help = qr/\Q$lei_out\E/s;
+ ok(!lei('config'), 'config w/o args fails');
+ like($lei_err, $exp_help, 'config w/o args shows our help in stderr');
lei_ok(qw(config -h), { XDG_CONFIG_HOME => '/XDC' },
\'config with XDG_CONFIG_HOME');
like($lei_out, qr! /XDC/lei/config\b!, 'XDG_CONFIG_HOME in config -h');
is($lei_err, '', 'no errors from config -h');
+
+ lei_ok(qw(-c foo.bar config dash.c works));
+ lei_ok(qw(config dash.c));
+ is($lei_out, "works\n", 'config set w/ -c');
+
+ lei_ok(qw(-c foo.bar config --add dash.c add-works));
+ lei_ok(qw(config --get-all dash.c));
+ is($lei_out, "works\nadd-works\n", 'config --add w/ -c');
};
my $ok_err_info = sub {
@@ -101,9 +112,11 @@ my $test_config = sub {
is($lei_out, "tr00\n", "-c string value passed as-is");
lei_ok(qw(-c imap.debug=a -c imap.debug=b config --get-all imap.debug));
is($lei_out, "a\nb\n", '-c and --get-all work together');
-
- lei_ok([qw(config -e)], { VISUAL => 'cat', EDITOR => 'cat' });
+ my $env = { VISUAL => 'cat', EDITOR => 'cat' };
+ lei_ok([qw(config -e)], $env);
is($lei_out, "[a]\n\tb = c\n", '--edit works');
+ ok(!lei([qw(-c a.b=c config -e)], $env), '-c conflicts with -e');
+ like($lei_err, qr/not allowed/, 'error message shown');
};
my $test_completion = sub {
