A re-roll of v2[1] which fixes issues pointed out with v2, plus some others I noticed along the way. There are now no longer any changes to the "public" Git::config_regxp() API. Instead it's left bitrotting in our tree at the end of this, having 0 in-tree users (instead of the 1 currently). We also handle the -c foo.bar (NULL value) case correctly, per what Jeff King pointed out. I also found a related issue with GIT_TEST_PERL_FATAL_WARNINGS=true, which we can now turn on for git-send-email.perl. 1. https://lore.kernel.org/git/cover-00.10-00000000000-20210520T081826Z-avarab@xxxxxxxxx/ Ævar Arnfjörð Bjarmason (13): send-email tests: support GIT_TEST_PERL_FATAL_WARNINGS=true send-email tests: test for boolean variables without a value send-email: remove non-working support for "sendemail.smtpssl" send-email: refactor sendemail.smtpencryption config parsing send-email: copy "config_regxp" into git-send-email.perl send-email: lazily load config for a big speedup send-email: lazily shell out to "git var" send-email: use function syntax instead of barewords send-email: get rid of indirect object syntax send-email: lazily load modules for a big speedup perl: lazily load some common Git.pm setup code send-email: move trivial config handling to Perl perl: nano-optimize by replacing Cwd::cwd() with Cwd::getcwd() Documentation/config/sendemail.txt | 3 - git-send-email.perl | 174 +++++++++++++++++++---------- perl/Git.pm | 35 +++--- t/t9001-send-email.sh | 29 +++++ 4 files changed, 160 insertions(+), 81 deletions(-) Range-diff against v2: -: ---------- > 1: 71f890dc60 send-email tests: support GIT_TEST_PERL_FATAL_WARNINGS=true -: ---------- > 2: 707c2ca556 send-email tests: test for boolean variables without a value 1: 8474acae68 = 3: 3bbd48dab2 send-email: remove non-working support for "sendemail.smtpssl" 2: b87f53adbe = 4: bed0f98d68 send-email: refactor sendemail.smtpencryption config parsing -: ---------- > 5: c12f69a411 send-email: copy "config_regxp" into git-send-email.perl 3: 1b27a393ae ! 6: d1c233d251 send-email: lazily load config for a big speedup @@ git-send-email.perl: sub read_config { next unless defined $v; next if $configured->{$setting}++; $$target = $v; -@@ git-send-email.perl: sub read_config { - } +@@ git-send-email.perl: sub config_regexp { + return @ret; } +# Save ourselves a lot of work of shelling out to 'git config' (it +# parses 'bool' etc.) by only doing so for config keys that exist. +my %known_config_keys; +{ -+ my @known_config_keys = Git::config_regexp("^sende?mail[.]"); ++ my @known_config_keys = config_regexp("^sende?mail[.]"); + @known_config_keys{@known_config_keys} = (); +} + @@ git-send-email.perl: sub read_config { my $rc = GetOptions( "identity=s" => \$identity, "no-identity" => \$no_identity, -@@ git-send-email.perl: sub read_config { +@@ git-send-email.perl: sub config_regexp { # Now we know enough to read the config { my %configured; @@ git-send-email.perl: sub read_config { } # Begin by accumulating all the variables (defined above), that we will end up -@@ git-send-email.perl: sub read_config { +@@ git-send-email.perl: sub config_regexp { usage(); } --if ($forbid_sendmail_variables && (scalar Git::config_regexp("^sendmail[.]")) != 0) { +-if ($forbid_sendmail_variables && (scalar config_regexp("^sendmail[.]")) != 0) { +if ($forbid_sendmail_variables && grep { /^sendmail/s } keys %known_config_keys) { die __("fatal: found configuration options for 'sendmail'\n" . "git-send-email is configured with the sendemail.* options - note the 'e'.\n" . "Set sendemail.forbidSendmailVariables to false to disable this check.\n"); - - ## perl/Git.pm ## -@@ perl/Git.pm: sub config_regexp { - } catch Git::Error::Command with { - my $E = shift; - if ($E->value() == 1) { -- my @matches = (); -- return @matches; -+ # Key(s) not found. -+ return; - } else { - throw $E; - } 4: acee22b77d ! 7: 4326c2f99c send-email: lazily shell out to "git var" @@ Commit message Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> ## git-send-email.perl ## -@@ git-send-email.perl: sub read_config { +@@ git-send-email.perl: sub config_regexp { } my ($repoauthor, $repocommitter); 5: f317cd1c01 = 8: e1fc71e3f9 send-email: use function syntax instead of barewords 6: fc27024f83 = 9: a806ce06f1 send-email: get rid of indirect object syntax 7: f86f5453d7 ! 10: aa11439789 send-email: lazily load modules for a big speedup @@ git-send-email.perl @@ use 5.008; use strict; - use warnings; + use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : (); -use POSIX qw/strftime/; -use Term::ReadLine; use Getopt::Long; @@ git-send-email.perl: sub do_edit { # SMTP password masked system "stty echo"; -@@ git-send-email.perl: sub read_config { +@@ git-send-email.perl: sub config_regexp { } sub parse_address_line { 8: 86641377c0 = 11: b3b342b173 perl: lazily load some common Git.pm setup code 9: 895c9e29a9 ! 12: 950dc0f53d send-email: move trivial config handling to Perl @@ Commit message Optimize the startup time of git-send-email by using an amended config_regexp() function to retrieve the list of config keys and - values we're interested in. See the earlier "send-email: lazily load - config for a big speedup" commit for why changing its interface is OK. + values we're interested in. For boolean keys we can handle the [true|false] case ourselves, and the "--get" case didn't need any parsing. Let's leave "--path" and - other "--bool" cases to "git config". As noted in a preceding commit - we're free to change the config_regexp() function, it's only used by - "git send-email". + other "--bool" cases to "git config". I'm not bothering with the + "undef" or "" case (true and false, respectively), let's just punt on + those and others and have "git config --type=bool" handle it. This brings the runtime of "git send-email" from ~60-~70ms to a very steady ~40ms on my test box. We now run just one "git config" @@ Commit message Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> + diff --git a/git-send-email.perl b/git-send-email.perl + index 1e9273fd4f..1ea4d9589d 100755 + --- a/git-send-email.perl + +++ b/git-send-email.perl + @@ -324,7 +324,11 @@ sub read_config { + my $target = $config_bool_settings{$setting}; + my $key = "$prefix.$setting"; + next unless exists $known_keys->{$key}; + - my $v = Git::config_bool(@repo, $key); + + my $v = (@{$known_keys->{$key}} == 1 && + + (defined $known_keys->{$key}->[0] && + + $known_keys->{$key}->[0] =~ /^(?:true|false)$/s)) + + ? $known_keys->{$key}->[0] eq 'true' + + : Git::config_bool(@repo, $key); + next unless defined $v; + next if $configured->{$setting}++; + $$target = $v; + @@ -353,14 +357,12 @@ sub read_config { + my $key = "$prefix.$setting"; + next unless exists $known_keys->{$key}; + if (ref($target) eq "ARRAY") { + - my @values = Git::config(@repo, $key); + - next unless @values; + + my @values = @{$known_keys->{$key}}; + next if $configured->{$setting}++; + @$target = @values; + } + else { + - my $v = Git::config(@repo, $key); + - next unless defined $v; + + my $v = $known_keys->{$key}->[0]; + next if $configured->{$setting}++; + $$target = $v; + } + @@ -371,12 +373,19 @@ sub config_regexp { + my ($regex) = @_; + my @ret; + eval { + - @ret = Git::command( + + my $ret = Git::command( + 'config', + - '--name-only', + + '--null', + '--get-regexp', + $regex, + ); + + @ret = map { + + # We must always return ($k, $v) here, since + + # empty config values will be just "key\0", + + # not "key\nvalue\0". + + my ($k, $v) = split /\n/, $_, 2; + + ($k, $v); + + } split /\0/, $ret; + 1; + } or do { + # If we have no keys we're OK, otherwise re-throw + @@ -389,8 +398,10 @@ sub config_regexp { + # parses 'bool' etc.) by only doing so for config keys that exist. + my %known_config_keys; + { + - my @known_config_keys = config_regexp("^sende?mail[.]"); + - @known_config_keys{@known_config_keys} = (); + + my @kv = config_regexp("^sende?mail[.]"); + + while (my ($k, $v) = splice @kv, 0, 2) { + + push @{$known_config_keys{$k}} => $v; + + } + } + + # sendemail.identity yields to --identity. We must parse this + ## git-send-email.perl ## @@ git-send-email.perl: sub read_config { my $target = $config_bool_settings{$setting}; @@ git-send-email.perl: sub read_config { next unless exists $known_keys->{$key}; - my $v = Git::config_bool(@repo, $key); + my $v = (@{$known_keys->{$key}} == 1 && -+ $known_keys->{$key}->[0] =~ /^(?:true|false)$/s) ++ (defined $known_keys->{$key}->[0] && ++ $known_keys->{$key}->[0] =~ /^(?:true|false)$/s)) + ? $known_keys->{$key}->[0] eq 'true' + : Git::config_bool(@repo, $key); next unless defined $v; @@ git-send-email.perl: sub read_config { next if $configured->{$setting}++; $$target = $v; } -@@ git-send-email.perl: sub read_config { +@@ git-send-email.perl: sub config_regexp { + my ($regex) = @_; + my @ret; + eval { +- @ret = Git::command( ++ my $ret = Git::command( + 'config', +- '--name-only', ++ '--null', + '--get-regexp', + $regex, + ); ++ @ret = map { ++ # We must always return ($k, $v) here, since ++ # empty config values will be just "key\0", ++ # not "key\nvalue\0". ++ my ($k, $v) = split /\n/, $_, 2; ++ ($k, $v); ++ } split /\0/, $ret; + 1; + } or do { + # If we have no keys we're OK, otherwise re-throw +@@ git-send-email.perl: sub config_regexp { # parses 'bool' etc.) by only doing so for config keys that exist. my %known_config_keys; { -- my @known_config_keys = Git::config_regexp("^sende?mail[.]"); +- my @known_config_keys = config_regexp("^sende?mail[.]"); - @known_config_keys{@known_config_keys} = (); -+ my @kv = Git::config_regexp("^sende?mail[.]"); ++ my @kv = config_regexp("^sende?mail[.]"); + while (my ($k, $v) = splice @kv, 0, 2) { + push @{$known_config_keys{$k}} => $v; + } } # sendemail.identity yields to --identity. We must parse this - - ## perl/Git.pm ## -@@ perl/Git.pm: sub config_int { - =item config_regexp ( RE ) - - Retrieve the list of configuration key names matching the regular --expression C<RE>. The return value is a list of strings matching --this regex. -+expression C<RE>. The return value is an ARRAY of key-value pairs. - - =cut - - sub config_regexp { - my ($self, $regex) = _maybe_self(@_); - try { -- my @cmd = ('config', '--name-only', '--get-regexp', $regex); -+ my @cmd = ('config', '--null', '--get-regexp', $regex); - unshift @cmd, $self if $self; -- my @matches = command(@cmd); -- return @matches; -+ my $data = command(@cmd); -+ my (@kv) = map { split /\n/, $_, 2 } split /\0/, $data; -+ return @kv; - } catch Git::Error::Command with { - my $E = shift; - if ($E->value() == 1) { 10: 97455f993d = 13: c1d7ea664a perl: nano-optimize by replacing Cwd::cwd() with Cwd::getcwd() -- 2.32.0.rc0.406.g05cb3eebfc
