Re: [PATCH v5 00/13] send-email: various optimizations to speed up by >2x

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: May 31, 2021, 09:53 UTC
Message-ID: <874kej2pjw.fsf@evledraar.gmail.com>
In-Reply-To: <YLR4tMOvkR81+ACq@coredump.intra.peff.net>

On Mon, May 31 2021, Jeff King wrote:

Show 30 quoted lines

> On Fri, May 28, 2021 at 11:23:39AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> Hopefully the final iteration. Updates a commit message to explain why
>> I moved away from File::Spec::Functions, rebases on master, and
>> explains and deals with the "undef in config" issue Jeff King noted.
>
> Thanks. The solution was less invasive than I feared.
>
> I guess here:
>
>>     @@ git-send-email.perl: sub read_config {
>>      -			my @values = Git::config(@repo, $key);
>>      -			next unless @values;
>>      +			my @values = @{$known_keys->{$key}};
>>     ++			@values = grep { defined } @values;
>>       			next if $configured->{$setting}++;
>>       			@$target = @values;
>>       		}
>>       		else {
>>      -			my $v = Git::config(@repo, $key);
>>     --			next unless defined $v;
>>      +			my $v = $known_keys->{$key}->[0];
>>     + 			next unless defined $v;
>>       			next if $configured->{$setting}++;
>>       			$$target = $v;
>>     - 		}
>
> we'd ignore such undef values, whereas presumably before they'd have
> triggered an error via Git::config(). I don't think it matters all that
> much either way, though.

They didn't error before, they were treated the same as the empty string.

This is because Git.pm uses "git config --get" to retrieve them, but we now use "--list --null".

This ultimately comes down to a limitation of git-config's one-shot CLI interface. You get the empty string and zero exit code for both of:

    git -c foo.bar= config --get foo.bar
    git -c foo.bar config --get foo.bar

Ignoring them here for list values is technically a behavior change if we were treating this as a black box, but in reality we know that these could only conceivable be useful for the bool values, for list values like "list of e-mail addresses" it makes no sense to have that in a the middle of a list, and we were implicitly ignoring them anyway before when we processed the empty string.