Re: ds/maintenance-part-3 (was Re: What's cooking in git.git (Nov 2020, #02; Mon, 9))

[Junio C Hamano <gitster@pobox.com>]

Emily Shaffer <emilyshaffer@google.com> writes:

> Because 'git maintenance unregister' spins a child process to call 'git
> config --unset maintenance.repo <cwd>', it actually fails if "cwd"
> contains a POSIX regular expression special character:
>
>   git config [<file-option>] --unset name [value_regex]

Good find.  And it is even worse that value_regex uses ERE, not BRE,
which means even an otherwise innocuous letter like '+' cannot be
used without quoting.

> You can demo it for yourself like so:
>
>   git init repro+for+maintenance
>   git maintenance register
>   git maintenance unregister
>   echo $?	# returns '5'
>   git config --list --global
>
> I see two paths forward:
>

0. Quote the value_regex properly, instead of blindly using a value
   that comes from the environment.

> 1. Teach 'git config' to learn either which regex parser to use
> (including fixed), or at least to learn "value isn't a regex", or
>
> 2. Don't spin a child process in 'git maintenance [un]register' and
> instead just call the config API.

> I'd suggest #2. The config API is very nice, and seems to have a simple
> way to add or remove configs to your global file in just a couple of
> lines. If there's a reason why it's not simpler to do it that way, it's
> my fault for missing the review :)

My short-to-mid-term preference is to do #1 to allow a value to be
spelled literally (i.e. remove entry with _this_ value, and add this
one instead), and optionally do #2 as an optimization that is not
essential.  I do not offhand know how you can make #2 alone fly
without doing some form of #1, as I think the same value_regex that
ought to be ERE to specify entries to be replaced needs to be used
under the cover even if you use "config API" anyway.

Thanks.