* Re: [PATCH v5 1/2] config: let git_config_parse_key() validate quietly
From: Harald Nordgren @ 2026-06-02 16:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk
In-Reply-To: <xmqqtsrlujah.fsf@gitster.g>
> Perhaps the updated "git_config_parse_key()" in this patch should be
> renamed to be a file-scape static internal helper, and the existing
> "git_config_parse_key()" should become a thin wrapper around that
> new helper function, retaining the current external interface,
> requiring no changes to existing callers.
I want to remember a discussion on one of my earlier topics, a few
months back, where someone else suggested instead of introducing two
thin wrappers over a helper, we should update the callers instead.
But for me either way is fine, maybe here it makes more sense, because
of the repeated NULL/0/1 parameters.
Harald
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Ramsay Jones @ 2026-06-02 16:23 UTC (permalink / raw)
To: Junio C Hamano, Patrick Steinhardt; +Cc: git
In-Reply-To: <xmqqldcxvziw.fsf@gitster.g>
On 02/06/2026 2:32 pm, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
>> We're about to extend our documentation to recommend b4 for sending
>> patch series ot the mailing list. Prepare for this by introducing a b4
>> configuration so that the tool knows to honor our preferences. For now,
>> this configuration does two things:
>>
>> - It configures "send-same-thread = shallow", which tells b4 to always
>> send subsequent versions of the same patch series as a reply to the
>> cover letter of the first version.
>>
>> - It configures "prep-cover-template", which tells b4 to use a custom
>> template for the cover letter. The most important change compared to
>> the default template is that our custom template also includes a
>> range-diff.
>>
>> There's potentially more things that we may want to configure going
>> forward, like for example auto-configuration of folks to Cc on certain
>> patches. But these two tweaks feel like a good place to start.
>>
>> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>> ---
>> .b4-config | 3 +++
>> .b4-cover-template | 11 +++++++++++
>> 2 files changed, 14 insertions(+)
>
> Shipping a sample like ".b4-config.sample" that users who opt-in can
> copy-and-edit into the final name ".b4-config" is OK, but I'd rather
> not to ship the configuration files that the users would want to edit
> (hence making the tree dirty).
>
Hmm, for those of us not in the know, perhaps mention the b4 documentation
at 'b4.docs.kernel.org' (which includes how to install b4 ... ;) ).
ATB,
Ramsay Jones
^ permalink raw reply
* [PATCH] http: preserve wwwauth_headers across redirects
From: Aaron Plattner @ 2026-06-02 16:11 UTC (permalink / raw)
To: git; +Cc: Aaron Plattner, Rahul Rameshbabu
When cURL follows a redirect, it calls the CURLOPT_HEADERFUNCTION for
each header received including ones from a redirect. http_request() sets
fwrite_wwwauth() as the header function, which will record the wwwauth[]
entries for the last step in the redirection chain.
However, when http_request_recoverable() sees that cURL followed a
redirect, it attempts to update the credentials for the request from the
new URL using credential_from_url(). The first thing that does is call
credential_clear(), which clears everything including wwwauth_headers.
If the new URL should use a credential helper rather than credentials
embedded in the URL, this loses the list of authentication methods that
the server provided in the redirect.
For example, I have a server that supports HTTP but always redirects to
HTTPS before handling requests. This redirect breaks OAuth
authentication:
$ git ls-remote http://server/git
=> Send header: GET /git/info/refs?service=git-upload-pack HTTP/1.1
<= Recv header: HTTP/1.1 302 Found
<= Recv header: Location: https://server.nvidia.com/git/info/refs?service=git-upload-pack
== Info: Issue another request to this URL: 'https://server.nvidia.com/git/info/refs?service=git-upload-pack'
=> Send header: GET /git/info/refs?service=git-upload-pack HTTP/1.1
<= Recv header: HTTP/1.1 401 Unauthorized
<= Recv header: WWW-Authenticate: Bearer error="invalid_request", error_description="No bearer token found in the request", msal-tenant-id="<tenant>", msal-client-id="<client>"
trace: run_command: 'git credential-cache --timeout 7200 get'
trace: start_command: /bin/sh -c 'git credential-cache --timeout 7200 get' 'git credential-cache --timeout 7200 get'
trace: built-in: git credential-cache --timeout 7200 get
trace: run_command: 'git credential-msal get'
trace: start_command: /bin/sh -c 'git credential-msal get' 'git credential-msal get'
trace: exec: git-credential-msal get
trace: run_command: git-credential-msal get
trace: start_command: /usr/bin/git-credential-msal get
Username for 'https://server.nvidia.com': ^C
When git invokes the credential helper, it doesn't include the wwwauth[]
array, so git-credential-msal doesn't think that OAuth is supported [1].
Fix the problem by preserving the wwwauth_headers strvec across the call
to credential_from_url().
[1] https://github.com/Binary-Eater/git-credential-msal/blob/trunk/src/git_credential_msal/main.py#L69
Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
---
http.c | 14 ++++++++++++
t/lib-httpd/apache.conf | 1 +
t/t5563-simple-http-auth.sh | 45 +++++++++++++++++++++++++++++++++++++
3 files changed, 60 insertions(+)
diff --git a/http.c b/http.c
index ea9b16861b..cac8c9bfc9 100644
--- a/http.c
+++ b/http.c
@@ -2425,7 +2425,21 @@ static int http_request_recoverable(const char *url,
if (options->effective_url && options->base_url) {
if (update_url_from_redirect(options->base_url,
url, options->effective_url)) {
+ struct strvec wwwauth_headers = STRVEC_INIT;
+
+ /*
+ * Preserve wwwauth_headers across the call to
+ * credential_from_url(): if the effective URL doesn't
+ * specify its own credentials, a credential helper
+ * might need the wwwauth[] array from the server's
+ * redirect response in order to authenticate.
+ */
+ strvec_pushv(&wwwauth_headers,
+ http_auth.wwwauth_headers.v);
credential_from_url(&http_auth, options->base_url->buf);
+ strvec_pushv(&http_auth.wwwauth_headers,
+ wwwauth_headers.v);
+ strvec_clear(&wwwauth_headers);
url = options->effective_url->buf;
}
}
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 40a690b0bb..664f23fc6c 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -202,6 +202,7 @@ RewriteRule ^/dumb-redir/(.*)$ /dumb/$1 [R=301]
RewriteRule ^/smart-redir-perm/(.*)$ /smart/$1 [R=301]
RewriteRule ^/smart-redir-temp/(.*)$ /smart/$1 [R=302]
RewriteRule ^/smart-redir-auth/(.*)$ /auth/smart/$1 [R=301]
+RewriteRule ^/custom_auth_redir/(.*)$ /custom_auth/$1 [R=302]
RewriteRule ^/smart-redir-limited/(.*)/info/refs$ /smart/$1/info/refs [R=301]
RewriteRule ^/ftp-redir/(.*)$ ftp://localhost:1000/$1 [R=302]
diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
index a7d475dd68..349ae4ab39 100755
--- a/t/t5563-simple-http-auth.sh
+++ b/t/t5563-simple-http-auth.sh
@@ -557,6 +557,51 @@ test_expect_success 'access using bearer auth' '
EOF
'
+test_expect_success 'bearer auth after redirect preserves wwwauth headers' '
+ test_when_finished "per_test_cleanup" &&
+
+ set_credential_reply get <<-EOF &&
+ capability[]=authtype
+ authtype=Bearer
+ credential=YS1naXQtdG9rZW4=
+ EOF
+
+ cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+ id=1 creds=Bearer YS1naXQtdG9rZW4=
+ EOF
+
+ cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
+ id=1 status=200
+ id=default response=WWW-Authenticate: FooBar param1="value1" param2="value2"
+ id=default response=WWW-Authenticate: Bearer authorize_uri="id.example.com" p=1 q=0
+ id=default response=WWW-Authenticate: Basic realm="example.com"
+ EOF
+
+ test_config_global credential.helper test-helper &&
+ test_config_global credential.useHttpPath true &&
+ git ls-remote "$HTTPD_URL/custom_auth_redir/repo.git" &&
+
+ expect_credential_query get <<-EOF &&
+ capability[]=authtype
+ capability[]=state
+ protocol=http
+ host=$HTTPD_DEST
+ path=custom_auth/repo.git
+ wwwauth[]=FooBar param1="value1" param2="value2"
+ wwwauth[]=Bearer authorize_uri="id.example.com" p=1 q=0
+ wwwauth[]=Basic realm="example.com"
+ EOF
+
+ expect_credential_query store <<-EOF
+ capability[]=authtype
+ authtype=Bearer
+ credential=YS1naXQtdG9rZW4=
+ protocol=http
+ host=$HTTPD_DEST
+ path=custom_auth/repo.git
+ EOF
+'
+
test_expect_success 'access using bearer auth with invalid credentials' '
test_when_finished "per_test_cleanup" &&
--
2.54.0
^ permalink raw reply related
* Re: [PATCH 2/2] Documentation/MyFirstContribution: recommend the use of b4
From: Weijie Yuan @ 2026-06-02 16:09 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano
In-Reply-To: <20260602-pks-b4-v1-2-a7ae5a49e9cf@pks.im>
Hi Patrick and Junio,
Just so happens that I just submitted my first patch. At this point, I
may or should be the target audience for this document.
I personally watched Patrick's videos with Scott Chacon[1] first, and I
reviewed them many times until I could do those "manual" git operations.
> +Contributors are encouraged to use `b4`, which automates much of the
> +bookkeeping that is otherwise done by hand.
So for statement like this and with my personal experience, I would say
b4 is a more suitable option for senior contributors, as they already
know, for example, what Message-ID and range-diffs are. But apparently,
whose who use forges may not know.
Back to the patch, I think regarding b4 as a more advanced contribution
way for those who had contributed via mailing lists for more than one
time is a better expression or formulation. Here I mean "b4 prep", other
usage like "b4 mbox" and "b4 am" are of course more basic, and be
mentioned as tips when interacting with Git mailing list.
A bit too wordy, in conclusion: Suggest that new contributors master
classic git operations first. When they are familiar with those process,
b4 might be a good option.
Thanks!
--
[1] https://www.youtube.com/watch?v=mjYac9SwIK0&t=1s (Part 1)
^ permalink raw reply
* Re: [PATCH v2 0/2] Small updates to SubmittingPatches
From: Derrick Stolee @ 2026-06-02 15:24 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <20260602144304.3341000-1-gitster@pobox.com>
On 6/2/2026 10:43 AM, Junio C Hamano wrote:
> Recently I gave some advice on how a cover letter should
> try to sell the idea to widest possible audience, and then
> I realized that we do not seem to teach how in our guides.
>
> Here is a small series to do so.
>
> In this round, a few typos have been corrected, and improvements are
> made thanks to help from Christian, Stolee, and Patrick.
This version LGTM.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: SZEDER Gábor @ 2026-06-02 15:24 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood, git
In-Reply-To: <ah7N5bKAiAORtNkp@pks.im>
On Tue, Jun 02, 2026 at 02:34:45PM +0200, Patrick Steinhardt wrote:
> On Tue, Jun 02, 2026 at 05:27:41PM +0900, Junio C Hamano wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > I wouldn't mind that outcome much, either. What triggered this series is
> > > that I'm always annoyed that it's "builtin/init-db.c" instead of
> > > "builtin/init.c", and the same for `cmd_init_db()`. But I intentionally
> > > constructed the series in a way that the first commit can be picked
> > > as-is, so that we can adjust our code to the modern world while not
> > > doing the deprecation dance.
> > >
> > > So I'd be equally happy if we just drop the second commit in this
> > > series.
> >
> > I'd actually find myself annoyed by such a rename when looking for
> > builtin/init-db.c only to find it gone---much like how a previous
> > rename made ll-merge difficult to locate.
> >
> > My point is that while static names may annoy some, renaming them
> > does not resolve the annoyance; it merely shifts it to someone else.
> >
> > So, if the primary motivation is just the first patch, I would be
> > less inclined to support this series.
>
> That's entirely fair. My take on this is a bit different, as I think
> it's beneficial to accept a short-term adjustment for core contributors
> in favor of making stuff easier to discover/maintain going forward.
That's not a short-term adjustment, but it will be an ongoing
annoyance, because 'git log builtin/init.c' will cut off at the rename
barrier.
^ permalink raw reply
* Re: [PATCH 1/2] SubmittingPatches: separate typofixes section
From: Christian Couder @ 2026-06-02 15:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqbjdtuidp.fsf@gitster.g>
On Tue, Jun 2, 2026 at 4:28 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
> > s/typofies/typofixes/
> >
> > Thanks.
>
> Thanks. It is amusing to see I cannot say typofixes when I talk
> about them ;-)
I wondered a bit if it was a case of satiric or ironical misspelling,
but it looks like in English the usual words for them are "grammer"
and "speling", like in:
"I am an expert in English grammer and speling." ;-)
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Patrick Steinhardt @ 2026-06-02 14:58 UTC (permalink / raw)
To: phillip.wood; +Cc: Junio C Hamano, Kristoffer Haugsbakk, git
In-Reply-To: <e6e225e7-f915-4ed0-900d-03a7767fb36f@gmail.com>
On Tue, Jun 02, 2026 at 02:12:58PM +0100, Phillip Wood wrote:
> On 02/06/2026 13:34, Patrick Steinhardt wrote:
> >
> > That's entirely fair. My take on this is a bit different, as I think
> > it's beneficial to accept a short-term adjustment for core contributors
> > in favor of making stuff easier to discover/maintain going forward.
> > > A new contributor would probably be quick to learn that every
> > `cmd_foo()` entry point is named exactly the same as the subcommand
> > name, but they will then eventually trip over the few exceptions like
> > `cmd_init_db()` where that assumption doesn't hold.
>
> Yes, those exceptions to the rule are annoying. Though they mostly exist for
> a good reason (code sharing between builtin commands), it would be nice to
> minimize them where we can.
Either minimize, or if we don't want to (or can't) the next best thing
is to pick the variant that most folks will know about. Which would be
the case if we just picked the first patch in this series.
Patrick
^ permalink raw reply
* Re: [PATCH 1/2] b4: introduce configuration for the Git project
From: Patrick Steinhardt @ 2026-06-02 14:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqldcxvziw.fsf@gitster.g>
On Tue, Jun 02, 2026 at 10:32:23PM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > We're about to extend our documentation to recommend b4 for sending
> > patch series ot the mailing list. Prepare for this by introducing a b4
> > configuration so that the tool knows to honor our preferences. For now,
> > this configuration does two things:
> >
> > - It configures "send-same-thread = shallow", which tells b4 to always
> > send subsequent versions of the same patch series as a reply to the
> > cover letter of the first version.
> >
> > - It configures "prep-cover-template", which tells b4 to use a custom
> > template for the cover letter. The most important change compared to
> > the default template is that our custom template also includes a
> > range-diff.
> >
> > There's potentially more things that we may want to configure going
> > forward, like for example auto-configuration of folks to Cc on certain
> > patches. But these two tweaks feel like a good place to start.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > .b4-config | 3 +++
> > .b4-cover-template | 11 +++++++++++
> > 2 files changed, 14 insertions(+)
>
> Shipping a sample like ".b4-config.sample" that users who opt-in can
> copy-and-edit into the final name ".b4-config" is OK, but I'd rather
> not to ship the configuration files that the users would want to edit
> (hence making the tree dirty).
I think shipping this as-is makes sense though, as it allows us to make
b4 behave the way we want it to without the user having to do anything.
If users actually want to reconfigure those values they can by saying
`git config set b4.<foobar>`, as the repository-local configuration will
override whatever `.b4-config` has.
Patrick
^ permalink raw reply
* [ANNOUNCE] Git Rev News edition 135
From: Christian Couder @ 2026-06-02 14:49 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jakub Narebski, Markus Jansen, Kaartic Sivaraam,
Štěpán Němec, Taylor Blau,
Johannes Schindelin, Derrick Stolee, Elijah Newren, Jeff King,
Matthias A., JAYATHEERTH K, Lucas Seiki Oshiro, Justin Tobler,
Pablo, Chandra Pratap, karthik nayak, Siddharth Asthana,
Siddharth Shrimali, Tian Yuchen, Ayush Chandekar, Bello Olamide,
lwn
Hi everyone,
The 135th edition of Git Rev News is now published:
https://git.github.io/rev_news/2026/05/31/edition-135/
Thanks a lot to Matthias Aßhauer and Štěpán Němec who helped this month!
Enjoy,
Christian, Jakub, Markus and Kaartic.
PS: An issue for the next edition is already opened and contributions
are welcome:
https://github.com/git/git.github.io/issues/846
^ permalink raw reply
* Re: [PATCH 1/2] SubmittingPatches: separate typofixes section
From: Kristoffer Haugsbakk @ 2026-06-02 14:46 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <20260602090808.87837-2-gitster@pobox.com>
On Tue, Jun 2, 2026, at 11:08, Junio C Hamano wrote:
> The existing text said something about tests (with [[tests]] to make
> it easier to refer to it from elsewhere) and then flowed into a
> different topic of typofixes, but it was unclear where the latter
> started. Add a similar [[typofies]] marker to the document.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
Imperative mood?
^ permalink raw reply
* [PATCH v2 2/2] SubmittingPatches: describe cover letter
From: Junio C Hamano @ 2026-06-02 14:43 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Derrick Stolee
In-Reply-To: <20260602144304.3341000-1-gitster@pobox.com>
We talk about how a commit log message should look like, but do not
give advice on writing the cover letter to sell a series to the
widest possible audience.
Helped-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/SubmittingPatches | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index dec8aea4cb..df9f722bfe 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -472,6 +472,30 @@ highlighted above.
Only capitalize the very first letter of the trailer, i.e. favor
"Signed-off-by" over "Signed-Off-By" and "Acked-by:" over "Acked-By".
+[[cover-letter]]
+=== Cover Letter
+
+The purpose of your cover letter is to sell your changes, explain what
+they are about, and get your target audience interested enough to read
+the patches.
+
+. Every code change comes with risk of regression and maintenance cost.
+ The cover letter should clearly communicate why the value of your
+ proposed change is worth applying. You can also describe how the risk
+ is reduced by the design choices you made while writing the patches.
+
+. Make sure your target audience can understand what the patches are
+ about and why they are needed without prior context.
+
+. For a second or subsequent iteration of the same topic, make sure
+ people who missed the earlier discussion can still understand what
+ the patches are about, so they can judge if the topic is worth their
+ time to read and comment on.
+
+. To help those who are familiar with earlier iterations, give a
+ summary of changes since the previous rounds.
+
+
[[ai]]
=== Use of Artificial Intelligence (AI)
--
2.54.0-591-g9032776dcc
^ permalink raw reply related
* [PATCH v2 1/2] SubmittingPatches: separate typofixes section
From: Junio C Hamano @ 2026-06-02 14:43 UTC (permalink / raw)
To: git
In-Reply-To: <20260602144304.3341000-1-gitster@pobox.com>
The existing text said something about tests (with [[tests]] to make
it easier to refer to it from elsewhere) and then flowed into a
different topic of typofixes, but it was unclear where the latter
started. Add a similar [[typofixes]] marker to the document.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/SubmittingPatches | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index d570184ec8..dec8aea4cb 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -237,6 +237,7 @@ Do not forget to update the documentation to describe the updated
behavior and make sure that the resulting documentation set formats
well (try the Documentation/doc-diff script).
+[[typofixes]]
We currently have a liberal mixture of US and UK English norms for
spelling and grammar, which is somewhat unfortunate. A huge patch that
touches the files all over the place only to correct the inconsistency
--
2.54.0-591-g9032776dcc
^ permalink raw reply related
* [PATCH v2 0/2] Small updates to SubmittingPatches
From: Junio C Hamano @ 2026-06-02 14:43 UTC (permalink / raw)
To: git
In-Reply-To: <20260602090808.87837-1-gitster@pobox.com>
Recently I gave some advice on how a cover letter should
try to sell the idea to widest possible audience, and then
I realized that we do not seem to teach how in our guides.
Here is a small series to do so.
In this round, a few typos have been corrected, and improvements are
made thanks to help from Christian, Stolee, and Patrick.
1/2: SubmittingPatches: separate typofixes section
2/2: SubmittingPatches: describe cover letter
Documentation/SubmittingPatches | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
--
2.54.0-591-g9032776dcc
^ permalink raw reply
* Re: [PATCH 1/2] SubmittingPatches: separate typofixes section
From: Junio C Hamano @ 2026-06-02 14:28 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <CAP8UFD0ij4BTVTie1dXwTC8M_9gAvroXebFLmQuY7eUCgHrJhA@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> On Tue, Jun 2, 2026 at 11:13 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> The existing text said something about tests (with [[tests]] to make
>> it easier to refer to it from elsewhere) and then flowed into a
>> different topic of typofixes, but it was unclear where the latter
>> started. Add a similar [[typofies]] marker to the document.
>
> s/typofies/typofixes/
>
> Thanks.
Thanks. It is amusing to see I cannot say typofixes when I talk
about them ;-)
^ permalink raw reply
* Re: [PATCH 1/2] SubmittingPatches: separate typofixes section
From: Christian Couder @ 2026-06-02 14:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20260602090808.87837-2-gitster@pobox.com>
On Tue, Jun 2, 2026 at 11:13 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> The existing text said something about tests (with [[tests]] to make
> it easier to refer to it from elsewhere) and then flowed into a
> different topic of typofixes, but it was unclear where the latter
> started. Add a similar [[typofies]] marker to the document.
s/typofies/typofixes/
Thanks.
^ permalink raw reply
* Re: [PATCH v5 2/2] config: improve diagnostic for "set" with missing value
From: Junio C Hamano @ 2026-06-02 14:18 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
In-Reply-To: <e5a2070ee1598bc345556b4afd01ae6d40fab633.1780407557.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 11fc976f3a..ed122d1100 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -469,6 +469,61 @@ test_expect_success 'invalid key' '
> test_must_fail git config inval.2key blabla
> '
>
> +test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
> + test_must_fail git config set pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
This is a syntax error of the command line, but the lhs of '=' makes
us suspect that the user may have meant to assign to that variable.
Makes perfect sense.
> +test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
> + test_must_fail git config pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
Ditto, the syntax may be an implicit "get" with bogus variable name,
or an implicit "set" with variable name and its value concatenated
into one argument with '='. The message seems to be assuming the
latter, which is OK to me.
> +test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
> + test_must_fail git config set foo=bar 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
> + test_grep ! "did you mean" err
> +'
OK.
> +test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
> + test_must_fail git config set foo.1bar=baz 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
> + test_grep ! "did you mean" err
> +'
OK. The above two should always give us the same error (except for
the actual bogus names given by the user to the command).
> +test_expect_success 'set with 1 arg: digit-led section name is valid' '
> + test_must_fail git config set 1foo.bar=baz 2>err &&
> + test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
> + test_grep "did you mean .git config set 1foo\\.bar baz." err
> +'
OK.
> +test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
> + test_must_fail git config set foo.some.b_r=baz 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
> + test_grep ! "did you mean" err
> +'
This is the third one that should be identical to earlier two that
gave a bogus variable name.
> +test_expect_success 'set with 1 arg of valid key reports missing value' '
> + test_must_fail git config set pull.rebase 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase." err &&
> + test_grep ! "did you mean" err
> +'
Did we see this already? No, this is different from the earlier one
that had "=false". This is a bog standard "you said set but did not
say what value to set to". Good.
> +test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
> + test_must_fail git config set pull.rebase=false true 2>err &&
> + test_grep ! "did you mean" err
> +'
OK. Do we want to see that the bogus variable name reported?
> +test_expect_success '"=" inside subsection is valid' '
> + test_when_finished "rm -f subsection.cfg" &&
> + git config set -f subsection.cfg foo.bar=baz.boo qux &&
> + echo qux >expect &&
> + git config get -f subsection.cfg foo.bar=baz.boo >actual &&
> + test_cmp expect actual
> +'
Excellent.
^ permalink raw reply
* Re: [PATCH v5 1/2] config: let git_config_parse_key() validate quietly
From: Junio C Hamano @ 2026-06-02 14:08 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
In-Reply-To: <d938ebf95a817c00a415670c08b839747d711d29.1780407557.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Add a "quiet" parameter that suppresses the error() calls, and let
> store_key be NULL to skip the canonical-copy allocation. Existing
> callers pass 0 for quiet.
Hmph.
The way this patch did this may have been easier to implement, but
is a bit different from what I had in mind when I suggested to
"refactor" the existing logic.
Perhaps the updated "git_config_parse_key()" in this patch should be
renamed to be a file-scape static internal helper, and the existing
"git_config_parse_key()" should become a thin wrapper around that
new helper function, retaining the current external interface,
requiring no changes to existing callers.
Then in the next step, config.[ch] can add a new entry point that
serves the purpose of is_valid_key() in the previous iteration,
perhaps call it is_valid_git_config_key() or something like that
(Patrick or others may want to suggest a better word order in its
name). That way, we do not have to sprinkle many calls into
this (rather ugly) version of git_config_parse_key() with overly
wide interface that repeats meaningless NULL/0/1 parameters that no
callers want to use (other than for the purpose of differenciating
the real git_config_parse_key() calls from the new calls made to the
same function to ask "is this a valid key or not, yes/no?".
Thanks.
^ permalink raw reply
* Re: [PATCH v3] doc: fix typos via codespell
From: Junio C Hamano @ 2026-06-02 13:57 UTC (permalink / raw)
To: Andrew Kreimer; +Cc: git
In-Reply-To: <20260602111552.6084-1-algonell@gmail.com>
Andrew Kreimer <algonell@gmail.com> writes:
> There are some typos in the documentation, comments, etc.
> Fix them via codespell.
>
> Signed-off-by: Andrew Kreimer <algonell@gmail.com>
> ---
> v3:
> - Address test breaking changes (strings bounded by single quotes).
> - Thank you for your patience (extreme noise/gain ratio).
Thanks, but this is wrong.
>
> t/t1700-split-index.sh | 2 +-
> t/t3909-stash-pathspec-file.sh | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
[v3] should not be "on top of" [v2], but the above shows that
apparently this is vastly different from [v2], which had
Documentation/SubmittingPatches | 2 +-
Documentation/git-sparse-checkout.adoc | 2 +-
Documentation/technical/build-systems.adoc | 6 +++---
builtin/pack-objects.c | 2 +-
commit-graph.h | 2 +-
compat/precompose_utf8.c | 2 +-
hook.h | 2 +-
meson_options.txt | 2 +-
midx-write.c | 2 +-
odb/source.h | 2 +-
packfile.h | 2 +-
path.h | 2 +-
reftable/system.h | 2 +-
t/README | 2 +-
t/chainlint.pl | 2 +-
t/chainlint/chain-break-false.expect | 2 +-
t/chainlint/chain-break-false.test | 2 +-
t/t1700-split-index.sh | 2 +-
t/t3909-stash-pathspec-file.sh | 6 +++---
t/t4052-stat-output.sh | 2 +-
t/t4067-diff-partial-clone.sh | 2 +-
t/t9150/svk-merge.dump | 10 +++++-----
t/t9151/svn-mergeinfo.dump | 18 +++++++++---------
t/unit-tests/clar/README.md | 2 +-
24 files changed, 40 insertions(+), 40 deletions(-)
Until the topic is merged to 'next', a new iteration of patch(es)
should cleanly apply to the base that [v2] was meant to apply, but
should pretend as if [v2] never existed.
> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> index 869fb4a14e..887e72a5fa 100755
> --- a/t/t1700-split-index.sh
> +++ b/t/t1700-split-index.sh
> @@ -502,7 +502,7 @@ test_expect_success 'do not refresh null base index' '
> git checkout main &&
> git update-index --split-index &&
> test_commit more &&
> - # must not write a new shareindex, or we won't catch the problem
> + # must not write a new shareindex, or we will not catch the problem
The committed code never had "we won't" (what was in 'seen' does not
count), and this patch clearly shows that this is to fix-up the
breakage the previous round caused. We do not want that.
I'll squash the fix-up I already had into [v2] that I have queued,
which should be sufficient to get to the state this [v3] should have
been, I think.
Thanks.
^ permalink raw reply
* Re: [PATCH 2/2] builtin/init-db: deprecate alias for git-init(1)
From: Junio C Hamano @ 2026-06-02 13:50 UTC (permalink / raw)
To: Phillip Wood; +Cc: Patrick Steinhardt, phillip.wood, Kristoffer Haugsbakk, git
In-Reply-To: <336a4202-a55f-4223-b654-985d47233653@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
>> I was wondering whether we want to call `you_still_use_that()` here. I
>> found it to be a bit heavy-handed as it's so trivial to replace with
>> git-init(1), but on the other hand it's a trivial thing to do.
>
> I agree you_still_use_that() is too heavy handed, I was thinking of
> something like
>
> warning(_("this command is deprecated, please use \"git init\""
> "instead");
>
> but that would mean we need to add a separate cmd_init_db() function
> that prints the warning and then calls cmd_init().
If we do plan to remove it in the future, then something like that
may be needed.
But it is not like having "init-db" hidden but accessible in the
command table is hurting anything. Other than that those who want
to create their own
[alias "init-db"] command = foo
that is, and I'd see it a bit crazy.
The "init-db" form is hidden from "git help" listing, and we know
whenever we suggest to run "git init" we do not say "git init-db",
so if we do not have to remove it in the future, I do not think we
even need such a warning().
^ permalink raw reply
* Re: [PATCH 2/2] SubmittingPatches: describe cover letter
From: Junio C Hamano @ 2026-06-02 13:43 UTC (permalink / raw)
To: Derrick Stolee; +Cc: git
In-Reply-To: <fd588cff-be2b-4422-9c01-cef06b2ea5fd@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
>> +. Make sure your target audience can understand what the patches are
>> + about and why they are needed without prior context.
>
> The thing that I like to say about the cover letter is that this is
> your opportunity to communicate why the value of your change is worth
> the risk of regressions and the cost of maintenance. Perhaps:
>
> . Every code change comes with risk of regression and maintenance cost.
> The cover letter should clearly communicate why the value of your
> proposed change is worth applying. You can also describe how the risk
> is reduced by the design choices you made while writing the patches.
>
> Or something similar may be helpful? I may just be over explaining.
Yeah, it may be a bit on the heavy side, but complements what I
wanted to achieve with this update very well. I wanted to encourage
writing for wider audience, without leaving those "not in the know"
behind. What you wrote above is more about what to write, which is
very much appreciated. I think it fits well as the 0th item before
the three-bullet list.
>> +. For a second or subsequent iteration of the same topic, make sure
>> + people who missed the earlier discussion can still understand what
>> + the patches are about, so they can judge if the topic is worth their
>> + time to read and comment on.
>> +
>> +. To help those who are familiar with earlier iterations, give a
>> + summary of changes since the previous rounds.
>
> I find these updates to be particularly helpful, even for GitGitGadget
> PRs that include a range-diff automatically. It's good to double-check
> the human description of the update against the computed diff.
Oh, absolutely.
A GitGitGadget generated cover letter that lack any human input but
just range-diff dump is often very hard to read, and the receiving
end is better off pretending there was no useful information in the
cover letter. "git diff @{-1}..." after applying the patches to the
same base is sadly a lot easier to read than "git range-diff @{-1}..."
for many series.
^ permalink raw reply
* Re: [PATCH v11 0/6] branch: prune-merged
From: Harald Nordgren @ 2026-06-02 13:41 UTC (permalink / raw)
To: phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <9b44d867-219a-4ca3-b8ae-67fdac1c72f6@gmail.com>
> Hi Harald
>
> Just a quick note to say I've not forgotten about this, hopefully I
> should have time to review it later in the week now I'm back on the list.
>
> Thanks
>
> Phillip
Great to hear! Thanks!
Harald
^ permalink raw reply
* [PATCH v5 2/2] config: improve diagnostic for "set" with missing value
From: Harald Nordgren via GitGitGadget @ 2026-06-02 13:39 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2302.v5.git.git.1780407557.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git config set pull.rebase=false" currently fails with "wrong
number of arguments", and the implicit form "git config
pull.rebase=false" fails with "invalid key". Neither points at
the real problem: the value is missing.
Report that directly, and when the argument has the shape
"<valid-key>=<value>", also suggest the split form:
$ git config set pull.rebase=false
error: missing value to set to the variable 'pull.rebase=false'
hint: did you mean "git config set pull.rebase false"?
When the prefix before "=" is not a valid key, drop the hint:
$ git config set foo=bar
error: missing value to set to a variable with an invalid name 'foo=bar'
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/config.c | 32 ++++++++++++++++++++++++++-
t/t1300-config.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index b3188cd8d4..a2d46d0ce1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "abspath.h"
+#include "advice.h"
#include "config.h"
#include "color.h"
#include "date.h"
@@ -210,6 +211,26 @@ static void check_argc(int argc, int min, int max)
exit(129);
}
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
+ if (prefix && !git_config_parse_key(prefix, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
+ } else if (!git_config_parse_key(arg, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
+ arg);
+ }
+ free(prefix);
+ exit(129);
+}
+
static void show_config_origin(const struct config_display_options *opts,
const struct key_value_info *kvi,
struct strbuf *buf)
@@ -1133,6 +1154,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc == 1)
+ die_missing_set_value(argv[0]);
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1371,6 +1394,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
int ret = 0;
+ int actions_implicit;
struct key_value_info default_kvi = KVI_INIT;
argc = parse_options(argc, argv, prefix, opts,
@@ -1385,7 +1409,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
exit(129);
}
- if (actions == 0)
+ actions_implicit = (actions == 0);
+ if (actions_implicit)
switch (argc) {
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ -1394,6 +1419,11 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
error(_("no action specified"));
exit(129);
}
+ if (actions_implicit && argc == 1) {
+ const char *last_dot = strrchr(argv[0], '.');
+ if (last_dot && strchr(last_dot + 1, '='))
+ die_missing_set_value(argv[0]);
+ }
if (display_opts.omit_values &&
!(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
error(_("--name-only is only applicable to --list or --get-regexp"));
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 11fc976f3a..ed122d1100 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -469,6 +469,61 @@ test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
+test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
+ test_must_fail git config set pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
+ test_must_fail git config pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
+ test_must_fail git config set foo=bar 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
+ test_must_fail git config set foo.1bar=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: digit-led section name is valid' '
+ test_must_fail git config set 1foo.bar=baz 2>err &&
+ test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
+ test_grep "did you mean .git config set 1foo\\.bar baz." err
+'
+
+test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
+ test_must_fail git config set foo.some.b_r=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'correct key' '
git config 123456.a123 987
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 1/2] config: let git_config_parse_key() validate quietly
From: Harald Nordgren via GitGitGadget @ 2026-06-02 13:39 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2302.v5.git.git.1780407557.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Add a "quiet" parameter that suppresses the error() calls, and let
store_key be NULL to skip the canonical-copy allocation. Existing
callers pass 0 for quiet.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/config.c | 2 +-
config.c | 34 ++++++++++++++++++++++------------
config.h | 2 +-
submodule-config.c | 2 +-
4 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc..b3188cd8d4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -555,7 +555,7 @@ static int get_value(const struct config_location_options *opts,
goto free_strings;
}
} else {
- if (git_config_parse_key(key_, &key, NULL)) {
+ if (git_config_parse_key(key_, &key, NULL, 0)) {
ret = CONFIG_INVALID_KEY;
goto free_strings;
}
diff --git a/config.c b/config.c
index a1b92fe083..81b31c5155 100644
--- a/config.c
+++ b/config.c
@@ -536,11 +536,14 @@ static inline int iskeychar(int c)
* -2 if there is no section name in the key.
*
* store_key - pointer to char* which will hold a copy of the key with
- * lowercase section and variable name
+ * lowercase section and variable name, can be NULL to skip
+ * allocation when only validation is needed
* baselen - pointer to size_t which will hold the length of the
* section + subsection part, can be NULL
+ * quiet - when non-zero, suppress error() reports on rejection
*/
-int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
+int git_config_parse_key(const char *key, char **store_key, size_t *baselen_,
+ int quiet)
{
size_t i, baselen;
int dot;
@@ -552,12 +555,14 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
*/
if (last_dot == NULL || last_dot == key) {
- error(_("key does not contain a section: %s"), key);
+ if (!quiet)
+ error(_("key does not contain a section: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
if (!last_dot[1]) {
- error(_("key does not contain variable name: %s"), key);
+ if (!quiet)
+ error(_("key does not contain variable name: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
@@ -568,7 +573,8 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
/*
* Validate the key and while at it, lower case it for matching.
*/
- *store_key = xmallocz(strlen(key));
+ if (store_key)
+ *store_key = xmallocz(strlen(key));
dot = 0;
for (i = 0; key[i]; i++) {
@@ -579,21 +585,25 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
if (!dot || i > baselen) {
if (!iskeychar(c) ||
(i == baselen + 1 && !isalpha(c))) {
- error(_("invalid key: %s"), key);
+ if (!quiet)
+ error(_("invalid key: %s"), key);
goto out_free_ret_1;
}
c = tolower(c);
} else if (c == '\n') {
- error(_("invalid key (newline): %s"), key);
+ if (!quiet)
+ error(_("invalid key (newline): %s"), key);
goto out_free_ret_1;
}
- (*store_key)[i] = c;
+ if (store_key)
+ (*store_key)[i] = c;
}
return 0;
out_free_ret_1:
- FREE_AND_NULL(*store_key);
+ if (store_key)
+ FREE_AND_NULL(*store_key);
return -CONFIG_INVALID_KEY;
}
@@ -609,7 +619,7 @@ static int config_parse_pair(const char *key, const char *value,
if (!strlen(key))
return error(_("empty config key"));
- if (git_config_parse_key(key, &canonical_name, NULL))
+ if (git_config_parse_key(key, &canonical_name, NULL, 0))
return -1;
ret = (fn(canonical_name, value, &ctx, data) < 0) ? -1 : 0;
@@ -1708,7 +1718,7 @@ static int configset_find_element(struct config_set *set, const char *key,
* `key` may come from the user, so normalize it before using it
* for querying entries from the hashmap.
*/
- ret = git_config_parse_key(key, &normalized_key, NULL);
+ ret = git_config_parse_key(key, &normalized_key, NULL, 0);
if (ret)
return ret;
@@ -3001,7 +3011,7 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,
validate_comment_string(comment);
/* parse-key returns negative; flip the sign to feed exit(3) */
- ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
+ ret = 0 - git_config_parse_key(key, &store.key, &store.baselen, 0);
if (ret)
goto out_free;
diff --git a/config.h b/config.h
index bf47fb3afc..2c66d334c1 100644
--- a/config.h
+++ b/config.h
@@ -341,7 +341,7 @@ int repo_config_set_worktree_gently(struct repository *, const char *, const cha
*/
void repo_config_set(struct repository *, const char *, const char *);
-int git_config_parse_key(const char *, char **, size_t *);
+int git_config_parse_key(const char *, char **, size_t *, int quiet);
/*
* The following macros specify flag bits that alter the behavior
diff --git a/submodule-config.c b/submodule-config.c
index a81897b4e0..a319956f7a 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -970,7 +970,7 @@ int print_config_from_gitmodules(struct repository *repo, const char *key)
int ret;
char *store_key;
- ret = git_config_parse_key(key, &store_key, NULL);
+ ret = git_config_parse_key(key, &store_key, NULL, 0);
if (ret < 0)
return CONFIG_INVALID_KEY;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 0/2] config: suggest the correct form when key contains "="
From: Harald Nordgren via GitGitGadget @ 2026-06-02 13:39 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren
In-Reply-To: <pull.2302.v4.git.git.1779823288005.gitgitgadget@gmail.com>
* New commit config: let git_config_parse_key() validate quietly adds a
quiet parameter (and an optional store_key) so callers can validate
without writing to stderr.
* Validation in die_missing_set_value() now routes through
git_config_parse_key(key, NULL, NULL, 1) instead of the previous local
helper.
* Added tests for 1foo.bar=baz and foo.some.b_r=baz.
Harald Nordgren (2):
config: let git_config_parse_key() validate quietly
config: improve diagnostic for "set" with missing value
builtin/config.c | 34 ++++++++++++++++++++++++++--
config.c | 34 ++++++++++++++++++----------
config.h | 2 +-
submodule-config.c | 2 +-
t/t1300-config.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 111 insertions(+), 16 deletions(-)
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v5
Pull-Request: https://github.com/git/git/pull/2302
Range-diff vs v4:
-: ---------- > 1: d938ebf95a config: let git_config_parse_key() validate quietly
1: 780b99409c ! 2: e5a2070ee1 config: improve diagnostic for "set" with missing value
@@ builtin/config.c: static void check_argc(int argc, int min, int max)
exit(129);
}
-+static int is_valid_key(const char *key)
-+{
-+ const char *last_dot = strrchr(key, '.');
-+
-+ return last_dot && isalpha(last_dot[1]);
-+}
-+
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
-+ if (prefix && is_valid_key(prefix)) {
++ if (prefix && !git_config_parse_key(prefix, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
-+ } else if (is_valid_key(arg)) {
++ } else if (!git_config_parse_key(arg, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
@@ t/t1300-config.sh: test_expect_success 'invalid key' '
+ test_grep ! "did you mean" err
+'
+
++test_expect_success 'set with 1 arg: digit-led section name is valid' '
++ test_must_fail git config set 1foo.bar=baz 2>err &&
++ test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
++ test_grep "did you mean .git config set 1foo\\.bar baz." err
++'
++
++test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
++ test_must_fail git config set foo.some.b_r=baz 2>err &&
++ test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
++ test_grep ! "did you mean" err
++'
++
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
--
gitgitgadget
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox