* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-24 16:20 ` D. Ben Knoble
@ 2025-11-24 21:19 ` Junio C Hamano
2025-11-24 23:55 ` Jeff King
2025-11-24 22:57 ` Jeff King
2025-11-25 20:16 ` Johannes Schindelin
2 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2025-11-24 21:19 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Ran Ari-Gur, git, Jeff King, raa.lkml@gmail.com
"D. Ben Knoble" <ben.knoble@gmail.com> writes:
> On Mon, Nov 24, 2025 at 12:23 AM Ran Ari-Gur <ran.arigur+git@samsara.com> wrote:
>>
>> Hi,
>>
>> There's a small regression in Git v2.52.0; it used to be that a command of the
>> form
>>
>> git clone '-c KEY=VALUE' ...
>>
>> or
>>
>> git clone '--config= KEY=VALUE' ...
>>
>> would trim whitespace around KEY, making the command equivalent to this:
>>
>> git clone --config=KEY=VALUE ...
Hmph, as documented in "git help clone",
`-c` `<key>=<value>`::
`--config` `<key>=<value>`::
Set a configuration variable in the newly-created repository;
this takes effect immediately after the repository is
initialized, but before the remote history is fetched or any
files checked out. The _<key>_ is in the same format as expected by
linkgit:git-config[1] (e.g., `core.eol=true`).
I do not offhand know if the option really used to behave as the
original report described, but if
git clone '-c KEY=VALUE'
git clone '--config KEY=VALUE'
does not complain-and-barf in the first place, I think that is a
bug. The above option description clearly asks the user to give the
dashed option (either "-c" or "--config") and "<key>=<value>" as two
separate arguments on the command line.
Interestingly, unlike other long options described nearby, we do not
seem to even list "--config=K=V" form, and that is a documentation
bug---other options like "server-option" is described to use "="
after it before its value, and to parse the "--config K=V", the code
uses the same mechanism.
Also, if the user writes
git clone -c ' KEY=VALUE'
git clone --config ' KEY=VALUE'
and we behaved as if it were "KEY=VALUE", that is another bug. As
documented, "key" is in the format as expected by "git config", and
we never allowed leading or trailing whitespaces around the key
names.
So I dunno.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-24 21:19 ` Junio C Hamano
@ 2025-11-24 23:55 ` Jeff King
2025-11-25 1:27 ` Junio C Hamano
0 siblings, 1 reply; 14+ messages in thread
From: Jeff King @ 2025-11-24 23:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Ran Ari-Gur, git, raa.lkml@gmail.com
On Mon, Nov 24, 2025 at 01:19:22PM -0800, Junio C Hamano wrote:
> Hmph, as documented in "git help clone",
>
> `-c` `<key>=<value>`::
> `--config` `<key>=<value>`::
> Set a configuration variable in the newly-created repository;
> this takes effect immediately after the repository is
> initialized, but before the remote history is fetched or any
> files checked out. The _<key>_ is in the same format as expected by
> linkgit:git-config[1] (e.g., `core.eol=true`).
>
> I do not offhand know if the option really used to behave as the
> original report described, but if
>
> git clone '-c KEY=VALUE'
> git clone '--config KEY=VALUE'
>
> does not complain-and-barf in the first place, I think that is a
> bug. The above option description clearly asks the user to give the
> dashed option (either "-c" or "--config") and "<key>=<value>" as two
> separate arguments on the command line.
I was surprised that a single "-c foo" argument would work, but it makes
sense: it is the "stuck" form of the short option "-c". So:
git cmd -cfoo
should be the equivalent of:
git cmd -c foo
whenever "-c" takes an option. It is just surprising to read because of
the leading space in the value.
Using the long option as a single string, like:
git clone '--config KEY=VALUE'
did not ever work (and should not), because there is no option of that
name. It is only the stuck form:
git clone '--config= KEY=VALUE'
which again makes sense from the config parser's perspective. It's just
funny that the first character of the option value is a space.
So I don't think there are any errors in the option-parser side. It's
just that we were overly lenient with trimming space in interpretation
of " KEY=VALUE" itself. Which has now either been corrected, or
erroneously broken, depending on your view. ;)
> Interestingly, unlike other long options described nearby, we do not
> seem to even list "--config=K=V" form, and that is a documentation
> bug---other options like "server-option" is described to use "="
> after it before its value, and to parse the "--config K=V", the code
> uses the same mechanism.
I don't think we're very consistent here. Look at --reference, --origin,
--branch, and others. I don't know if we have an existing style
recommendation here (though we do recommend the "stuck" form in gitcli,
which perhaps argues that we should be using that in our documentation).
So I don't know that I'd call it a bug, but it may be a good long-term
project to make the presentation of options more consistent.
> Also, if the user writes
>
> git clone -c ' KEY=VALUE'
> git clone --config ' KEY=VALUE'
>
> and we behaved as if it were "KEY=VALUE", that is another bug. As
> documented, "key" is in the format as expected by "git config", and
> we never allowed leading or trailing whitespaces around the key
> names.
So yes, we did allow that until recently, along with:
git clone -c ' foo.bar = baz'
which keeps the space in the value "baz", but otherwise sets foo.bar.
I agree it was certainly surprising. Despite the real-world report that
started this thread, it is oddball enough that I do not think we want to
continue supporting it even for historical reasons. It is not quite at
the level of https://xkcd.com/1172/, but especially the form that the OP
showed looks like a mistaken invocation that happened to work (and would
not work for any other option in general).
-Peff
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-24 23:55 ` Jeff King
@ 2025-11-25 1:27 ` Junio C Hamano
2025-11-25 22:03 ` Junio C Hamano
2025-11-26 14:53 ` Jeff King
0 siblings, 2 replies; 14+ messages in thread
From: Junio C Hamano @ 2025-11-25 1:27 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Ran Ari-Gur, git, raa.lkml@gmail.com
Jeff King <peff@peff.net> writes:
> I was surprised that a single "-c foo" argument would work, but it makes
> sense: it is the "stuck" form of the short option "-c". So:
>
> git cmd -cfoo
>
> should be the equivalent of:
>
> git cmd -c foo
>
> whenever "-c" takes an option. It is just surprising to read because of
> the leading space in the value.
Ahh, OK, so
git cmd '-c foo.bar=baz'
was doing
git cmd --config=' foo.bar=baz'
or an easier-to-read form to express in the "stuck" form of the
short option
git cmd -c' foo.bar=baz'
which I totally missed. I agree that the option parser is doing the
right thing for that case, including passing " foo=bar" with a
leading space as its value.
> So yes, we did allow that until recently, along with:
>
> git clone -c ' foo.bar = baz'
>
> which keeps the space in the value "baz", but otherwise sets foo.bar.
>
> I agree it was certainly surprising. Despite the real-world report that
> started this thread, it is oddball enough that I do not think we want to
> continue supporting it even for historical reasons. It is not quite at
> the level of https://xkcd.com/1172/, but especially the form that the OP
> showed looks like a mistaken invocation that happened to work (and would
> not work for any other option in general).
After you explained the "that's stuck form with leading whitespace
in the value" I missed, I wasn't so sure. "The value is supposed to
be a configuration variable, followed by an equal sign, followed by
its value; what good does it do if we retained the leading
whitespace---stripping is a usability feature" would work as an
argument in this particular case, even though it may not work in
general. Of course, the right thing to do when "git clone -c"
option was introduced would have been to notice that the stripping
of spaces is unwelcome complication of the UI and reject/correct it,
but it is way too late for that now.
The right right thing to do at this point may be to fix the
regression and at the same time mark the "feature" as deprecated,
and remove it following the usual deprecation procedure, but that
certainly sounds like an unnecessary waste of engineering effort.
So, I dunno.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-25 1:27 ` Junio C Hamano
@ 2025-11-25 22:03 ` Junio C Hamano
2025-11-26 15:02 ` Jeff King
2025-11-26 14:53 ` Jeff King
1 sibling, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2025-11-25 22:03 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Ran Ari-Gur, git, raa.lkml@gmail.com
Junio C Hamano <gitster@pobox.com> writes:
> The right right thing to do at this point may be to fix the
> regression and at the same time mark the "feature" as deprecated,
> and remove it following the usual deprecation procedure, but that
> certainly sounds like an unnecessary waste of engineering effort.
>
> So, I dunno.
The first step of the "right right thing" may look something like
this. As this thread analyzed so far, this awkward lenience exists
only in "clone -c <key>=<value>" in that the keyname is trimmed, so
isolating the damage within the clone's code path would be the right
approach, if we want to keep this awkward lenience alive a little
bit longer.
This function receives the string_list that accumulated
the "-c<STRING>" and "--config <STRING>" command line parameters
(plus some internally generated ones related to submodules) and
feeds them one at a time to git_config_parse_parameter() that
expects the <key>=<value> pair to be fed.
builtin/clone.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git c/builtin/clone.c w/builtin/clone.c
index c990f398ef..4ea8c92a6b 100644
--- c/builtin/clone.c
+++ w/builtin/clone.c
@@ -779,7 +779,26 @@ static void write_config(struct string_list *config)
int i;
for (i = 0; i < config->nr; i++) {
- if (git_config_parse_parameter(config->items[i].string,
+ /*
+ * NEEDSWORK: a backward compatibility wart that made
+ * us tolerate (note the leading whitespace before
+ * the variable name)
+ *
+ * $ git clone '-c foo.bar=baz'
+ *
+ * and treated as if the leading whitespace before the
+ * variable name did not exist. Apparently a third
+ * party tool "Bamboo" relies on this past stupidity
+ * of ours.
+ *
+ * Eventually we should deprecate and remove this.
+ */
+ const char *trimleft = config->items[i].string;
+
+ while (*trimleft && isspace(*trimleft))
+ trimleft++;
+
+ if (git_config_parse_parameter(trimleft,
write_one_config, NULL) < 0)
die(_("unable to write parameters to config file"));
}
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-25 22:03 ` Junio C Hamano
@ 2025-11-26 15:02 ` Jeff King
2025-11-26 17:06 ` Junio C Hamano
0 siblings, 1 reply; 14+ messages in thread
From: Jeff King @ 2025-11-26 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Ran Ari-Gur, git, raa.lkml@gmail.com
On Tue, Nov 25, 2025 at 02:03:56PM -0800, Junio C Hamano wrote:
> The first step of the "right right thing" may look something like
> this. As this thread analyzed so far, this awkward lenience exists
> only in "clone -c <key>=<value>" in that the keyname is trimmed, so
> isolating the damage within the clone's code path would be the right
> approach, if we want to keep this awkward lenience alive a little
> bit longer.
That's not entirely true. It is in any code that calls
git_config_parse_parameter(), which includes the old-style parser for
GIT_CONFIG_PARAMETERS.
So:
$ GIT_CONFIG_PARAMETERS="' foo.bar =baz'" git.v2.51.0 config foo.bar
baz
$ GIT_CONFIG_PARAMETERS="' foo.bar =baz'" git.v2.52.0 config foo.bar
error: invalid key: foo.bar
fatal: unable to parse command-line config
That doesn't trigger via "git -c", because we use the "new" form these
days (so it started rejecting the extra whitespace in 2021). And you'd
only see it if you hand-crafted the variable, or an old version of Git
set parameters that were then parsed by a newer one.
So whether that is a case we care about is up for debate. But if we are
going to accommodate backwards compatibility, we have to decide where to
draw the line.
> diff --git c/builtin/clone.c w/builtin/clone.c
> index c990f398ef..4ea8c92a6b 100644
> --- c/builtin/clone.c
> +++ w/builtin/clone.c
> @@ -779,7 +779,26 @@ static void write_config(struct string_list *config)
> int i;
>
> for (i = 0; i < config->nr; i++) {
> - if (git_config_parse_parameter(config->items[i].string,
> + /*
> + * NEEDSWORK: a backward compatibility wart that made
> + * us tolerate (note the leading whitespace before
> + * the variable name)
> + *
> + * $ git clone '-c foo.bar=baz'
> + *
> + * and treated as if the leading whitespace before the
> + * variable name did not exist. Apparently a third
> + * party tool "Bamboo" relies on this past stupidity
> + * of ours.
> + *
> + * Eventually we should deprecate and remove this.
> + */
> + const char *trimleft = config->items[i].string;
> +
> + while (*trimleft && isspace(*trimleft))
> + trimleft++;
The old code actually trimmed both sides. So:
$ GIT_CONFIG_PARAMETERS="'foo.bar =baz'" git.v2.51.0 config foo.bar
baz
$ GIT_CONFIG_PARAMETERS="'foo.bar =baz'" git.v2.52.0 config foo.bar
error: invalid key: foo.bar
fatal: unable to parse command-line config
And I think the latter would still fail with your patch. Again, that
might not matter to us, if all we care about is making:
git clone '-c foo.bar=baz' ...
work as before. But I'm still skeptical that is worthwhile (especially
given that nobody noticed the same change to "git -c" a few years ago).
-Peff
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-26 15:02 ` Jeff King
@ 2025-11-26 17:06 ` Junio C Hamano
2025-11-30 13:49 ` Jeff King
0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2025-11-26 17:06 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Ran Ari-Gur, git, raa.lkml@gmail.com
Jeff King <peff@peff.net> writes:
> That doesn't trigger via "git -c", because we use the "new" form these
> days (so it started rejecting the extra whitespace in 2021). And you'd
> only see it if you hand-crafted the variable, or an old version of Git
> set parameters that were then parsed by a newer one.
>
> So whether that is a case we care about is up for debate. But if we are
> going to accommodate backwards compatibility, we have to decide where to
> draw the line.
I was hoping we already drew the line above the "clone" thing ;-)
> The old code actually trimmed both sides. So:
>
> $ GIT_CONFIG_PARAMETERS="'foo.bar =baz'" git.v2.51.0 config foo.bar
> baz
>
> $ GIT_CONFIG_PARAMETERS="'foo.bar =baz'" git.v2.52.0 config foo.bar
> error: invalid key: foo.bar
> fatal: unable to parse command-line config
>
> And I think the latter would still fail with your patch. Again, that
> might not matter to us, if all we care about is making:
>
> git clone '-c foo.bar=baz' ...
>
> work as before. But I'm still skeptical that is worthwhile (especially
> given that nobody noticed the same change to "git -c" a few years ago).
True.
I do not think I can convince myself to care about this deeply
enough.
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-26 17:06 ` Junio C Hamano
@ 2025-11-30 13:49 ` Jeff King
2025-11-30 18:11 ` Junio C Hamano
0 siblings, 1 reply; 14+ messages in thread
From: Jeff King @ 2025-11-30 13:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Ran Ari-Gur, git, raa.lkml@gmail.com
On Wed, Nov 26, 2025 at 09:06:45AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > That doesn't trigger via "git -c", because we use the "new" form these
> > days (so it started rejecting the extra whitespace in 2021). And you'd
> > only see it if you hand-crafted the variable, or an old version of Git
> > set parameters that were then parsed by a newer one.
> >
> > So whether that is a case we care about is up for debate. But if we are
> > going to accommodate backwards compatibility, we have to decide where to
> > draw the line.
>
> I was hoping we already drew the line above the "clone" thing ;-)
OK. :) I am OK with that, but I wanted to make sure we were doing it
consciously.
> > And I think the latter would still fail with your patch. Again, that
> > might not matter to us, if all we care about is making:
> >
> > git clone '-c foo.bar=baz' ...
> >
> > work as before. But I'm still skeptical that is worthwhile (especially
> > given that nobody noticed the same change to "git -c" a few years ago).
>
> True.
>
> I do not think I can convince myself to care about this deeply
> enough.
That's about where I'm at, though I'm a little worried by Dscho's
mention that apparently git-lfs has the same problem. So maybe it's more
widespread than I am giving it credit for?
If we draw the line at "-c foo=bar" as a single argument (which is what
it sounds like git-lfs is doing, too) then your simple "trim" patch
would be enough.
I dunno. I certainly do not want to get into a deprecation period and
all of that mess. Maybe the breakage in v2.52.0 would be enough for
callers to notice and fix their invocations, and we could just quietly
remove the hack later? But then, I am not sure what makes "later" any
better than "now".
-Peff
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-30 13:49 ` Jeff King
@ 2025-11-30 18:11 ` Junio C Hamano
0 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2025-11-30 18:11 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Ran Ari-Gur, git, raa.lkml@gmail.com
Jeff King <peff@peff.net> writes:
> That's about where I'm at, though I'm a little worried by Dscho's
> mention that apparently git-lfs has the same problem. So maybe it's more
> widespread than I am giving it credit for?
Reading <https://github.com/git-for-windows/git/issues/5972>, I
think the mention of LFS in Dscho's message on this list was a
red-herring, as corrected by Dscho himself at
https://github.com/git-for-windows/git/issues/5972#issuecomment-3577520017
I do not know if buggily constructed command line by Atlassian
Bamboo is something we want to bend over backwards to help papering
over, but probably not.
> I dunno. I certainly do not want to get into a deprecation period and
> all of that mess. Maybe the breakage in v2.52.0 would be enough for
> callers to notice and fix their invocations, and we could just quietly
> remove the hack later? But then, I am not sure what makes "later" any
> better than "now".
Yes. Let's write it off as an inadvertent bugfix ;-)
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-25 1:27 ` Junio C Hamano
2025-11-25 22:03 ` Junio C Hamano
@ 2025-11-26 14:53 ` Jeff King
2025-11-26 17:08 ` Junio C Hamano
1 sibling, 1 reply; 14+ messages in thread
From: Jeff King @ 2025-11-26 14:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Ran Ari-Gur, git, raa.lkml@gmail.com
On Mon, Nov 24, 2025 at 05:27:01PM -0800, Junio C Hamano wrote:
> > So yes, we did allow that until recently, along with:
> >
> > git clone -c ' foo.bar = baz'
> >
> > which keeps the space in the value "baz", but otherwise sets foo.bar.
> >
> > I agree it was certainly surprising. Despite the real-world report that
> > started this thread, it is oddball enough that I do not think we want to
> > continue supporting it even for historical reasons. It is not quite at
> > the level of https://xkcd.com/1172/, but especially the form that the OP
> > showed looks like a mistaken invocation that happened to work (and would
> > not work for any other option in general).
>
> After you explained the "that's stuck form with leading whitespace
> in the value" I missed, I wasn't so sure. "The value is supposed to
> be a configuration variable, followed by an equal sign, followed by
> its value; what good does it do if we retained the leading
> whitespace---stripping is a usability feature" would work as an
> argument in this particular case, even though it may not work in
> general. Of course, the right thing to do when "git clone -c"
> option was introduced would have been to notice that the stripping
> of spaces is unwelcome complication of the UI and reject/correct it,
> but it is way too late for that now.
>
> The right right thing to do at this point may be to fix the
> regression and at the same time mark the "feature" as deprecated,
> and remove it following the usual deprecation procedure, but that
> certainly sounds like an unnecessary waste of engineering effort.
I agree that is the most conservative choice, but I'd also be
comfortable just calling this a bug that was fixed. The leading space
was accepted only by "git clone -c" and not "git -c". And of course
there is almost no other option in all of Git where doing "-o foo" as a
single argument would do the right thing[1].
I would be more sympathetic if the original report was "it is useful for
so-and-so reason to do this whitespace stripping". But it really sounds
like the problem was some caller doing something like (in perl
pseudo-code):
system("git", "clone", "-c $key", $repo);
instead of:
system("git", "clone", "-c", $key, $repo);
which is just a bug that happened to work in this limited instance.
So my inclination would be to leave it be, because I do not think it
merits the time. But if somebody else wants to go for it, I will not
stop them. ;)
-Peff
[1] Given our recent discussion of strtol(), I actually wonder if "-x
10" works for "-x" that takes a numeric option (because strtol would
suck up the leading whitespace). So maybe this kind of error is
silently lurking in more places.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-26 14:53 ` Jeff King
@ 2025-11-26 17:08 ` Junio C Hamano
0 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2025-11-26 17:08 UTC (permalink / raw)
To: Jeff King; +Cc: D. Ben Knoble, Ran Ari-Gur, git, raa.lkml@gmail.com
Jeff King <peff@peff.net> writes:
> I would be more sympathetic if the original report was "it is useful for
> so-and-so reason to do this whitespace stripping". But it really sounds
> like the problem was some caller doing something like (in perl
> pseudo-code):
>
> system("git", "clone", "-c $key", $repo);
>
> instead of:
>
> system("git", "clone", "-c", $key, $repo);
>
> which is just a bug that happened to work in this limited instance.
>
> So my inclination would be to leave it be, because I do not think it
> merits the time. But if somebody else wants to go for it, I will not
> stop them. ;)
;-) I might, as it would consume my time as well as theirs.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-24 16:20 ` D. Ben Knoble
2025-11-24 21:19 ` Junio C Hamano
@ 2025-11-24 22:57 ` Jeff King
2025-11-25 20:16 ` Johannes Schindelin
2 siblings, 0 replies; 14+ messages in thread
From: Jeff King @ 2025-11-24 22:57 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Ran Ari-Gur, git, raa.lkml@gmail.com
On Mon, Nov 24, 2025 at 11:20:05AM -0500, D. Ben Knoble wrote:
> Thanks! As far as backward compatibility, I think this behavior has
> been around since 2010's 8b1fa77867 (Allow passing of configuration
> parameters in the command line, 2010-03-26) which morphed via
> 572e4f6a0c (Use strbufs instead of open-coded string manipulation,
> 2010-03-26) into the strbuf_trim(pair[0]) that you pointed to as
> disappearing.
>
> Interestingly, I note that we dropped the trim around pair[1] in
> 06eb708f33 (config: always parse GIT_CONFIG_PARAMETERS during
> git_config, 2011-05-24), but I don't see that discussed in the commit
> message either. I tried a handful of mailing list searches around
> 20110524224955.GC24527@sigill.intra.peff.net, but didn't find any
> relevant discussion (though my lore-search skills are mediocre).
Yeah, there's really not much (any) discussion in that thread. I don't
recall why I would have removed the trim on the value side, but I don't
think it was an intentional choice. I don't think either trim (key or
value) really makes much sense. I'm kind of puzzled why we had them.
I thought it first it was to be lenient in the environment list. This
code was originally for "git -c foo.bar=baz", and we are not even
parsing it directly there. It gets shoved into GIT_CONFIG_PARAMETERS and
then re-parsed from there. So I think it was an attempt to be lenient
about writing:
GIT_CONFIG_PARAMETERS="foo.bar=baz other.key=whatever"
But it predates that! The environment passing came in 2b64fc894d (pass
"git -c foo=bar" params through environment, 2010-08-23). And it always
shell-quotes the names, like:
GIT_CONFIG_PARAMETERS="'foo.bar=baz' 'other.key=whatever'"
so the extra whitespace would need to be inside the shell quotes to
matter. So it seems like it really was about allowing:
git -c ' foo.bar=baz ' ...
to work. Which seems odd. And as an added bonus, that was already
broken! In 1ff21c05ba (config: store "git -c" variables using more
robust format, 2021-01-12) we switched to a different format which does
not call git_config_parse_parameter() at all, and does not do the extra
trim. (The old code is still there to read the non-robust format, but
new Git will never write it).
So this recent refactoring of the function is left affecting only "git
clone -c", which does not pass through the environment (we write the
variables out directly into the newly-cloned repo's config).
While it is a change of behavior, I'm tempted to say that it was not
something that was ever intended to work, and not worth going back now
to restore.
-Peff
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] `git clone '-c KEY=VALUE'` no longer works
2025-11-24 16:20 ` D. Ben Knoble
2025-11-24 21:19 ` Junio C Hamano
2025-11-24 22:57 ` Jeff King
@ 2025-11-25 20:16 ` Johannes Schindelin
2 siblings, 0 replies; 14+ messages in thread
From: Johannes Schindelin @ 2025-11-25 20:16 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Ran Ari-Gur, git, Jeff King, raa.lkml@gmail.com
[-- Attachment #1: Type: text/plain, Size: 4080 bytes --]
Hi Ran & Ben,
On Mon, 24 Nov 2025, D. Ben Knoble wrote:
> On Mon, Nov 24, 2025 at 12:23 AM Ran Ari-Gur <ran.arigur+git@samsara.com> wrote:
> >
> > There's a small regression in Git v2.52.0; it used to be that a command of the
> > form
> >
> > git clone '-c KEY=VALUE' ...
> >
> > or
> >
> > git clone '--config= KEY=VALUE' ...
> >
> > would trim whitespace around KEY, making the command equivalent to this:
> >
> > git clone --config=KEY=VALUE ...
> >
> > The relevant code was here:
> > https://github.com/git/git/blob/v2.51.2/config.c#L649
> >
> > That functionality was removed in this refactoring commit:
> > https://github.com/git/git/commit/dcecac2580ef871186fdc4e9efc87815a4ce4c66
> >
> > As a result, a command like the above will now fail, with an error such as this:
> >
> > error: invalid key: advice.detachedHead=false
> > fatal: unable to write parameters to config file
> >
> > because config keys are not allowed to contain whitespace.
> >
> > I believe this change was unintentional; it was not mentioned in the commit
> > message or the release notes.
> >
> > This probably isn't a common case, and the project where I ran into this issue
> > has already fixed it on their end (they now pass -c and KEY=VALUE as separate
> > arguments); but since Git aims to ensure backward-compatibility where possible,
> > I figured I should report it.
This has also been reported in
https://github.com/git-for-windows/git/issues/5972 as breaking Git LFS.
> Thanks! As far as backward compatibility, I think this behavior has
> been around since 2010's 8b1fa77867 (Allow passing of configuration
> parameters in the command line, 2010-03-26) which morphed via
> 572e4f6a0c (Use strbufs instead of open-coded string manipulation,
> 2010-03-26) into the strbuf_trim(pair[0]) that you pointed to as
> disappearing.
>
> Interestingly, I note that we dropped the trim around pair[1] in
> 06eb708f33 (config: always parse GIT_CONFIG_PARAMETERS during
> git_config, 2011-05-24), but I don't see that discussed in the commit
> message either. I tried a handful of mailing list searches around
> 20110524224955.GC24527@sigill.intra.peff.net, but didn't find any
> relevant discussion (though my lore-search skills are mediocre).
I am awfully pinched on time right now, but I _think_ that this could be
the start of a fix:
-- snipsnap --
diff --git a/config.c b/config.c
index f1def0dcfba..2b2efe479dc 100644
--- a/config.c
+++ b/config.c
@@ -637,7 +637,7 @@ int git_config_parse_parameter(const char *text,
kvi_from_param(&kvi);
- string_list_split(&pair, text, "=", 1);
+ string_list_split_f(&pair, text, "=", 1, STRING_LIST_SPLIT_TRIM_FIRST);
if (!pair.nr)
return error(_("bogus config parameter: %s"), text);
diff --git a/string-list.c b/string-list.c
index 08dc00984cc..9b5f1d71a9d 100644
--- a/string-list.c
+++ b/string-list.c
@@ -326,6 +326,13 @@ static int split_string(struct string_list *list, const char *string, const char
else if (!in_place && !list->strdup_strings)
BUG("string_list_split() called without strdup_strings");
+ if (flags & STRING_LIST_SPLIT_TRIM_FIRST) {
+ if (flags & STRING_LIST_SPLIT_TRIM)
+ flags &= ~STRING_LIST_SPLIT_TRIM_FIRST;
+ else
+ flags |= STRING_LIST_SPLIT_TRIM;
+ }
+
for (;;) {
char *end;
@@ -345,6 +352,9 @@ static int split_string(struct string_list *list, const char *string, const char
if (!end)
return count;
p = end + 1;
+
+ if (flags & STRING_LIST_SPLIT_TRIM_FIRST)
+ flags &= ~STRING_LIST_SPLIT_TRIM;
}
}
diff --git a/string-list.h b/string-list.h
index fa6ba07853c..938707bf09a 100644
--- a/string-list.h
+++ b/string-list.h
@@ -297,6 +297,8 @@ enum {
STRING_LIST_SPLIT_TRIM = (1 << 0),
/* omit adding empty string piece to the resulting list */
STRING_LIST_SPLIT_NONEMPTY = (1 << 1),
+ /* trim only the first */
+ STRING_LIST_SPLIT_TRIM_FIRST = (1 << 2),
};
int string_list_split_f(struct string_list *, const char *string,
^ permalink raw reply related [flat|nested] 14+ messages in thread