Inconsistent/buggy behaviour of "git config --add"

Inconsistent/buggy behaviour of "git config --add"

[Tim Landscheidt]

Hi,

with Git 2.44.0 on Fedora 39, the behaviour of "git config
--add", i. e., adding multiple lines to a configuration key,
is inconsistent and/or buggy:

| # git config section.key value0
| # git config --add section.key value1
| # cat .git/config
| [core]
|         repositoryformatversion = 0
|         filemode = true
|         bare = false
|         logallrefupdates = true
| [section]
|         key = value0
|         key = value1
| # git config section.key --add value2
| # cat .git/config
| [core]
|         repositoryformatversion = 0
|         filemode = true
|         bare = false
|         logallrefupdates = true
| [section]
|         key = value0
|         key = value1
|         key = --add
| # git config section.key --add
| warning: section.key has multiple values
| error: cannot overwrite multiple values with a single value
|        Use a regexp, --add or --replace-all to change section.key.
| #

So on one hand, "--add" must be given before the key to add
a line, but if on the other hand one passes the option after
the key and before the value, it is literally taken as the
value and the value does not seem to be interpreted as a
value-pattern, either.  However, if the value is missing,
Git correctly recognizes that this does not make sense.

My expectation of least surprise is that "git config
section.key --add value" should be equivalent to "git config
--add section.key value".

If that is not possible, I would expect "git config
section.key --add value2" to mean "change the values of
section.key to '--add' where they currently match the
value-pattern of 'value2'".

Tim

Re: Inconsistent/buggy behaviour of "git config --add"

[Junio C Hamano]

Tim Landscheidt <tim@tim-landscheidt.de> writes:

> | # git config section.key value0
> | # git config --add section.key value1

The action verb --add comes immediately after "git config" (and
possibly file-option and type option), so this is a request to
append "key = value1" in the "[section]" (if there is no existing
section.key, then "[section]" would have to be created at the same
time, but in this case there already is one).

> | # cat .git/config
> | [core]
> |         repositoryformatversion = 0
> |         filemode = true
> |         bare = false
> |         logallrefupdates = true
> | [section]
> |         key = value0
> |         key = value1

So this makes perfect sense.

> | # git config section.key --add value2

No action verb immediately after "git config" (possibly after
file-option and type option).  This should be taken as

    git config <name> <value> <value-pattern>

where

    <name> = section.key
    <value> = --add
    <value-pattern> = value2

As we lack --replace-all, the default behaviour is to replace a
single existing entry of "section.key" with existing value "value2",
with the new value "--add", or if there is no such existing entry,
add one such entry.

> | # cat .git/config
> | [core]
> |         repositoryformatversion = 0
> |         filemode = true
> |         bare = false
> |         logallrefupdates = true
> | [section]
> |         key = value0
> |         key = value1
> |         key = --add

which seems to be what the code did.

> | # git config section.key --add

No action verb immediately after "git config" (possibly after
file-option and type option).  This should be taken as

    git config <name> <value>

where

    <name> = section.key
    <value> = --add

and is an attempt to replace existing section.key with the new value
"--add", but because we have already three such entries, we get 

> | warning: section.key has multiple values
> | error: cannot overwrite multiple values with a single value
> |        Use a regexp, --add or --replace-all to change section.key.
> | #

which sounds sensible.

> So on one hand, "--add" must be given before the key to add
> a line, but if on the other hand one passes the option after
> the key and before the value, it is literally taken as the
> value and the value does not seem to be interpreted as a
> value-pattern, either.  However, if the value is missing,
> Git correctly recognizes that this does not make sense.

Not really.  I agree that the "git config" syntax is messy, but I
followed your example with "git config --help" (especially its
SYNOPSIS section) in hand, and reached the above explanation, which
your conjecutre does not quite match.

> My expectation of least surprise is that "git config
> section.key --add value" should be equivalent to "git config
> --add section.key value".

You cannot have "--add" as a value by doing so.

> If that is not possible, I would expect "git config
> section.key --add value2" to mean "change the values of
> section.key to '--add' where they currently match the
> value-pattern of 'value2'".

I think your expectation needs to be updated in this particular
case, but there is a discussion to revamp the UI started elsewhere,
which stops the double-dashed action verbs and instead trigger
different actions as subcommands of "git config", which will
hopefully make things easier to understand.

Re: Inconsistent/buggy behaviour of "git config --add"

[Tim Landscheidt]

Junio C Hamano <gitster@pobox.com> wrote:

> […]

>> So on one hand, "--add" must be given before the key to add
>> a line, but if on the other hand one passes the option after
>> the key and before the value, it is literally taken as the
>> value and the value does not seem to be interpreted as a
>> value-pattern, either.  However, if the value is missing,
>> Git correctly recognizes that this does not make sense.

> Not really.  I agree that the "git config" syntax is messy, but I
> followed your example with "git config --help" (especially its
> SYNOPSIS section) in hand, and reached the above explanation, which
> your conjecutre does not quite match.

I respectfully disagree (somewhat).  git-config(1) reads:

| […]

|        Multiple lines can be added to an option by using
|        the --add option. If you want to update or unset an
|        option which can occur on multiple lines, a
|        value-pattern (which is an extended regular
|        expression, unless the --fixed-value option is
|        given) needs to be given. Only the existing values
|        that match the pattern are updated or unset. If you
|        want to handle the lines that do not match the
|        pattern, just prepend a single exclamation mark in
|        front (see also the section called “EXAMPLES”), but
|        note that this only works when the --fixed-value
|        option is not in use.

| […]

|        --replace-all
|            Default behavior is to replace at most one
|            line. This replaces all lines matching the key
|            (and optionally the value-pattern).

This says quite clearly if one wants to /add/ a value,
"--add" must be given; the default is to /replace/ an
existing value.

The only indication that this might be false is the
description of "--add" itself which gives another and
different explanation for the behaviour of "--replace-all":

|        --add
|            Adds a new line to the option without altering
|            any existing values. This is the same as
|            providing ^$ as the value-pattern in
|            --replace-all.

| […]

So the first section states that "only the existing values
that match the pattern are updated or unset", and here it
says that if there are no matches, a new line is added.

>> My expectation of least surprise is that "git config
>> section.key --add value" should be equivalent to "git config
>> --add section.key value".

> You cannot have "--add" as a value by doing so.

> […]

The standard way to achieve this would be to use "--",
i. e. "git config section.key --add -- --add" (apparently
even part of POSIX as "Utility Syntax Guidelines", but also
commonly used elsewhere in Git).

Tim

Re: Inconsistent/buggy behaviour of "git config --add"

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

On 2024-03-24 at 18:57:21, Tim Landscheidt wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> >> My expectation of least surprise is that "git config
> >> section.key --add value" should be equivalent to "git config
> >> --add section.key value".
> 
> > You cannot have "--add" as a value by doing so.
> 
> > […]
> 
> The standard way to achieve this would be to use "--",
> i. e. "git config section.key --add -- --add" (apparently
> even part of POSIX as "Utility Syntax Guidelines", but also
> commonly used elsewhere in Git).

POSIX mandates that all options come before all non-option arguments.
So by that rule, `git config section.key --add value` means that `--add`
is a non-option argument, not the option `--add`.

Certainly some GNU tools do break the rule, and Git begrudgingly allows
it in some cases, but in utilities in general, it isn't allowed.  Many
non-Linux OSes and non-GNU tools don't allow options to appear after
non-option arguments at all.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: Inconsistent/buggy behaviour of "git config --add"

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1516 bytes --]

On Sun, Mar 24, 2024 at 06:57:21PM +0000, Tim Landscheidt wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
[snip]
> >> My expectation of least surprise is that "git config
> >> section.key --add value" should be equivalent to "git config
> >> --add section.key value".
> 
> > You cannot have "--add" as a value by doing so.
> 
> > […]
> 
> The standard way to achieve this would be to use "--",
> i. e. "git config section.key --add -- --add" (apparently
> even part of POSIX as "Utility Syntax Guidelines", but also
> commonly used elsewhere in Git).

I think the whole design of git-config(1) is just not great, and I'm not
surprised at all that folks run into issues like the one you describe
here. I agree that it is quite surprising that modes like "--add" look
like a flag, but don't really behave like one in the general case.

This is part of the reason why I have recently proposed to convert this
command to use proper subcommands [1]. This introduces things like `git
config set`, `git config list`, `git config get`, `git config unset`
that (in my opinion) behave a lot more obvious. Also, I certainly agree
with you that those subcommands should respect "--" as many other of our
Git commands already do.

I plan to reroll this patch series soonish. Please feel free to provide
your feedback on it so that we can iterate on the proposed UI/UX and
make it match user's expectations better.

Patrick

[1]: https://lore.kernel.org/git/cover.1710198711.git.ps@pks.im/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Inconsistent/buggy behaviour of "git config --add"

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> I plan to reroll this patch series soonish. Please feel free to provide
> your feedback on it so that we can iterate on the proposed UI/UX and
> make it match user's expectations better.

;-)