Re: git config set --file, key-value pair without '= value', gives Segmentation fault

From: Han Jiang <jhcarl0814@gmail.com>
Date: Jul 31, 2024, 23:46 UTC
Message-ID: <CANrWfmQ89+NN0_O+=g51DkXHJANPYYMYN_zK8Os9JBiuZiHHQA@mail.gmail.com>
In-Reply-To: <ZqrKeG+alFldB7OY@nand.local>

Show 9 quoted lines

> but I am not sure exactly how we want to handle synonymous Boolean
> values here.  Regardless of how "true" value is spelled in the
> configuration file, e.g.
>
>         [section]               [section]
>                 key                     key = true
>
> I wonder if "git config set --value=yes [--fixed-value] section.key
> false" should replace either of them to false.

'key' can be selected by `--value='!'` 'key =' can be selected by `--value='^$'` or `--value='' --fixed-value` commands for demonstration:

cd '/'; cd '/'; rm --force --recursive -- './test_git'; mkdir "$_"; cd "$_";
cat >'./config.txt' <<'EOF'
[section]
    key
    key =
    key = value1
    key = value2
EOF
git config set --file './config.txt' --value='^$' 'section.key' 'value3'
git config get --file './config.txt' --show-origin --show-scope --all
'section.key'
git config get --file './config.txt' --show-origin --show-scope 'section.key'

cat >'./config.txt' <<'EOF'
[section]
    key
    key =
    key = value1
    key = value2
EOF
git config set --file './config.txt' --value='^$' --fixed-value
'section.key' 'value4'
git config get --file './config.txt' --show-origin --show-scope --all
'section.key'
git config get --file './config.txt' --show-origin --show-scope 'section.key'

cat >'./config.txt' <<'EOF'
[section]
    key
    key =
    key = value1
    key = value2
EOF
git config set --file './config.txt' --value='' 'section.key' 'value5'
git config get --file './config.txt' --show-origin --show-scope --all
'section.key'
git config get --file './config.txt' --show-origin --show-scope 'section.key'

cat >'./config.txt' <<'EOF'
[section]
    key
    key =
    key = value1
    key = value2
EOF
git config set --file './config.txt' --value='' --fixed-value
'section.key' 'value6'
git config get --file './config.txt' --show-origin --show-scope --all
'section.key'
git config get --file './config.txt' --show-origin --show-scope 'section.key'

cat >'./config.txt' <<'EOF'
[section]
    key
    key =
    key = value1
    key = value2
EOF
git config set --file './config.txt' --value='!' 'section.key' 'value7'
git config get --file './config.txt' --show-origin --show-scope --all
'section.key'
git config get --file './config.txt' --show-origin --show-scope 'section.key'

> It would especially true if the command is invoked with --type=bool
> but it seems that the --type option does not participate in the
> matching with the current value.

> I think I could be convinced that --type=bool should treat "", "true",
> "1", "on", and so on as matching here, but I'm not sure.

It seems that `--regexp` and `--value=<regexp>` and `--fixed-value` take effect before `--default=<default>` which is before `--type=<type>`, so creating a special case (treating 'key' as 'key = true' if `--type=bool` is present in downstream process, and when `--default=<default>` is present, requiring users to remember which has higher priority or which takes effect first) would be too surprising. (Currently you can use `--value=abcd` to kick 'key' away then `--default=false` to turn 'key' into false, use `--value=abcd` to kick 'key=' away then `--default=true` to turn 'key=' into true.)

On Thu, Aug 1, 2024 at 11:36 AM Taylor Blau <me@ttaylorr.com> wrote:

Show 41 quoted lines

>
> On Wed, Jul 31, 2024 at 03:47:04PM -0700, Junio C Hamano wrote:
> > I would have actually expected the fix that follow your analysis (by
> > the way, I found it really well done) would say something more like
> >
> >       if (strcmp(key, store->key))
> >               return 0; /* not ours */
> > +     if (!value)
> > +             value = "true";
> >       if (store->fixed_value)
> >               return !strcmp(store->fixed_value, value);
> >
> > but I am not sure exactly how we want to handle synonymous Boolean
> > values here.  Regardless of how "true" value is spelled in the
> > configuration file, e.g.
> >
> >         [section]               [section]
> >                 key                     key = true
> >
> > I wonder if "git config set --value=yes [--fixed-value] section.key
> > false" should replace either of them to false.
>
> I think it gets tricky when we talk about the implicit true value
> here for exactly that reason. Do we take true to mean just "true"? Or
> "true" and "yes", "1", "", and so on?
>
> I tried to match the behavior a few lines down in that function, where
> we only call regexec() if value is non-NULL.
>
> > It would especially true if the command is invoked with --type=bool
> > but it seems that the --type option does not participate in the
> > matching with the current value.
>
> I think I could be convinced that --type=bool should treat "", "true",
> "1", "on", and so on as matching here, but I'm not sure.
>
> > > I'd like to hear from Stolee (CC'd), who is the author of c90702a1f6
> > > before submitting this as a standalone patch.
>
> Thanks,
> Taylor