* Re: [PATCH v3 1/2] branch: suggest <remote>/<branch> on upstream slip
From: Harald Nordgren @ 2026-06-28 7:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git
In-Reply-To: <xmqqfr272lq7.fsf@gitster.g>
Let's do it!
Harald
^ permalink raw reply
* Re: [PATCH v3 1/2] branch: suggest <remote>/<branch> on upstream slip
From: Junio C Hamano @ 2026-06-28 7:00 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
In-Reply-To: <9883c28482be4ad43f0f999c2e6be9f9dd9fb13b.1782583345.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1572a4f9ef..dede60d27b 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -706,6 +706,29 @@ static int edit_branch_description(const char *branch_name)
> return 0;
> }
>
> +static void die_if_upstream_looks_like_remote(const char *new_upstream, const char *branch_name)
> +{
> + struct strbuf remote_ref = STRBUF_INIT;
> + int code;
> +
> + if (strchr(new_upstream, '/') ||
> + !remote_is_configured(remote_get(new_upstream), 0))
> + return;
> +
> + strbuf_addf(&remote_ref, "refs/remotes/%s/%s", new_upstream, branch_name);
> + if (!refs_ref_exists(get_main_ref_store(the_repository), remote_ref.buf)) {
> + strbuf_release(&remote_ref);
> + return;
> + }
> +
> + code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
> + advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
> + _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
> + new_upstream, branch_name);
> + strbuf_release(&remote_ref);
> + exit(code);
> +}
> +
> int cmd_branch(int argc,
> const char **argv,
> const char *prefix,
> @@ -957,6 +980,15 @@ int cmd_branch(int argc,
> if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
> if (!argc || branch_checked_out(branch->refname))
> die(_("no commit on branch '%s' yet"), branch->name);
> + /*
> + * Check the advice up front to avoid the ref
> + * lookups when the hint is off. The helper still
> + * calls advise_if_enabled() so the hint carries the
> + * standard "disable this message" instructions.
> + */
> + if (argc == 1 &&
> + advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
> + die_if_upstream_looks_like_remote(new_upstream, argv[0]);
> die(_("branch '%s' does not exist"), branch->name);
> }
Hmph, something like adding a single liner in the caller, like this. ...
code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+ /* use _if_enabled here to show the hint on how to disable */
advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
_("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
new_upstream, branch_name);
strbuf_release(&remote_ref);
exit(code);
... was what I meant, because the most puzzling piece is that the
function calls _if_enabled form there, when the caller is presumably
already checked _enabled() and leaves the reader wondering if there
are other callers of this function that does not check before
calling it.
But this is so tiny a thing that once the code is written, it is
probably not worth the churn to redo it. Let's declare victory and
mark the topic ready for 'next'?
^ permalink raw reply
* Re: [PATCH] http: accept https:// proxies again
From: Junio C Hamano @ 2026-06-28 5:10 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Aliwoto, Johannes Schindelin
In-Reply-To: <xmqq8q7z4eg3.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> From this function nothing returns an error anymore, and looking at
> the preimage of 663d7abe (http: reject unsupported proxy URL
> schemes, 2026-05-05) that is the source of the bug, the original did
> not do anything when the corresponding code did not find and set any
> proxy settings, either.
>
> So perhaps it is a better fix to make it just a function that
> returns void with early returns?
Nah, I was being stupid. Disregard the above.
The whole point of 663d7abe was that we wanted to reject what we did
not recognise, and we cannot do so without returning "good/bad" from
that function. The bug was that we did recognise https:// but still
returned -1 because of the bug, which the patch in the thread fixed.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values
From: Tian Yuchen @ 2026-06-28 3:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, cirnovskyv, szeder.dev, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <eabb8169-2c13-4961-9b21-f44b1fa66f70@malon.dev>
On 6/28/26 11:19, Tian Yuchen wrote:
> Let it be noticed by repo_config_values() function to
> catch offending callers for now, and once the codebase becomes ready
> to use one repo_config_values per repository, this function does not
> have to change.
And
> Wouldn't we rather want to try to be more strict and say
>
> if (!repo || !repo->initialized)
> BUG("repo must be an initialied repository");
>
> here? Aren't all the callers of this function supposed to be
> dealing with an already initialized repository?
In my opinion, these two suggestions are not entirely consistent, and I
think we need to determine the most appropriate approach.
Regards, yuchen
^ permalink raw reply
* Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values
From: Tian Yuchen @ 2026-06-28 3:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, cirnovskyv, szeder.dev, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <xmqqv7b34snt.fsf@gitster.g>
On 6/28/26 04:47, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
>
>> Hi all,
>>
>> Apologies again for the duplicate...
>>
>> On 6/28/26 00:08, Tian Yuchen wrote:
>>
>>> +const char *repo_excludes_file(struct repository *repo)
>>> +{
>>> + if (!repo || !repo->initialized)
>>> + return NULL;
>
> I might already have said this, but I am not sure why want to be as
> loose as this code. It is not limited to this line, but I think we
> saw plenty of other "We know we must get an already initialized
> thing here, and the subsequent operation we perform on that thing
> will cause us to die() later, so let's return silently and early
> to avoid hitting die()" attempts to sweep problems under the rug.
>
> Wouldn't we rather want to try to be more strict and say
>
> if (!repo || !repo->initialized)
> BUG("repo must be an initialied repository");
>
> here? Aren't all the callers of this function supposed to be
> dealing with an already initialized repository?
>
That makes sense, but from my point of view...
'repo_config_values()' already has a check for 'repo->initialized'. If
we're absolutely certain that the 'repo' is initialized, wouldn't it be
better to simply remove all the checks inside the getter and leave the
judgment to 'repo_config_values()'?
This also aligns to some extent with the previous flag getters: since an
uninitialized 'repo' will trigger a BUG() in 'repo_config_values()', but
"reading and writing these flags when the 'repo' is uninitialized" is
sometimes a valid operation that's why we choose to intercept
'repo->initialized' _before_ 'repo_config_values()' and fallback to the
hard-coded values.
This gives the impression that _'repo_config_values()' is the function
responsible for checking_, and the way flags are handled is an exception
to this approach, which I think is more consistent and self-explanatory.
What do you think?
>
>>> + if (!repo_config_values(repo)->excludes_file)
>>> + repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
>>> +
>>> + return repo_config_values(repo)->excludes_file;
>>> +}
>>
>> One more thing:
>>
>> I deliberately didn't write a comment for the getter because it will
>> probably be merged with comments from the previous several patches in
>> some form in the near future... I'm not sure if it would be more
>> appropriate to write a separate patch to add the corresponding comments
>> then.
>
> That's very sensible.
>
Thanks.
regards, yuchen
^ permalink raw reply
* Re: [PATCH v2 5/6] t: convert grep assertions to test_grep
From: Junio C Hamano @ 2026-06-28 2:03 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Michael Montalbo via GitGitGadget, git, D. Ben Knoble,
Eric Sunshine, Michael Montalbo
In-Reply-To: <xmqqldbz4f1a.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> SZEDER Gábor <szeder.dev@gmail.com> writes:
>>
>>> I think in this case checking the file3's contents is wrong, because
>>> at this point file3 should not exist in the first place. I've sent a
>>> patch to fix this long ago, but apparently didn't manage to follow
>>> through back then.
>>>
>>> https://lore.kernel.org/git/20211010172809.1472914-1-szeder.dev@gmail.com/
>>
>> Thanks. I guess the test_grep can be extended to catch this case,
>> where
>>
>> test_grep ! -e pattern1 -e pattern2 file
>>
>> does not find any hits, but only because 'file' is missing, as an
>> error, ...
>
> Wait. The necessary check is already there, isn't it?
>
> test_grep () {
> eval "last_arg=\${$#}"
>
> test -f "$last_arg" ||
> BUG "test_grep requires a file to read as the last parameter"
>
> So why don't we see it every time we run that test that inspects
> file3's contents with Michael's series merged in? Puzzled...
Ah, of course. Michael sidesteps this mechanism by not using
"test_grep !", with
! grep dirty file3 && # lint-ok: file may not exist after --quit
and if we realize that "may not exist" is actually "never exists",
then your other patch from 5 years ago would become the most
sensible fix for this line.
It may not be a bad idea to go through "# lint-ok:" introduced by
Michael's series with finer toothed comb (there are only a handful
of them) and see if there are similar "look, the file we are
grepping in never exists with correctly running Git" gotchas.
Thanks.
^ permalink raw reply
* Re: [PATCH] http: accept https:// proxies again
From: Junio C Hamano @ 2026-06-28 1:54 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Aliwoto, Johannes Schindelin
In-Reply-To: <pull.2161.git.1782580676734.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> http.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/http.c b/http.c
> index 8e5a4d8bcf..8c0f831365 100644
> --- a/http.c
> +++ b/http.c
> @@ -802,6 +802,8 @@ static int set_curl_proxy_type(CURL *result, const char *protocol)
> if (has_proxy_cert_password())
> curl_easy_setopt(result, CURLOPT_PROXY_KEYPASSWD,
> proxy_cert_auth.password);
> +
> + return 0;
> }
>
> return -1;
That lack of "return 0" is so glaringly obvious when you point it
out like this patch does, and it is surprising it has been missed
initially.
From this function nothing returns an error anymore, and looking at
the preimage of 663d7abe (http: reject unsupported proxy URL
schemes, 2026-05-05) that is the source of the bug, the original did
not do anything when the corresponding code did not find and set any
proxy settings, either.
So perhaps it is a better fix to make it just a function that
returns void with early returns?
Thanks.
^ permalink raw reply
* Re: [PATCH] t3420-rebase-autostash: don't try to grep non-existing files
From: Junio C Hamano @ 2026-06-28 1:45 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Michael Montalbo, Denton Liu
In-Reply-To: <aj90x3DsER5HASUS@szeder.dev>
SZEDER Gábor <szeder.dev@gmail.com> writes:
> On Sun, Oct 10, 2021 at 07:28:09PM +0200, SZEDER Gábor wrote:
>> Several tests in 't3420-rebase-autostash.sh' start various rebase
>> processes that are expected to fail because of merge conflicts. The
>> tests [1] checking that 'git rebase --quit' and autostash work
>> together as expected after such a failure then run '! grep ...' to
>> ensure that the dirty contents of the file is gone. However, due to
>> the test repo's history and the choice of upstream branch that file
>> shouldn't exist in the conflicted state at all, and thus it shouldn't
>> exist after the subsequent 'git rebase --quit' either. Consequently,
>> this 'grep' doesn't fail as expected, i.e. because it can't find the
>> dirty content, but instead it fails, because it can't open the file.
>>
>> Thighten this check by using 'test_path_is_missing' instead, thereby
>> avoiding unexpected errors from 'grep' as well.
>>
>> Previously 2745817028 (t3420-rebase-autostash: don't try to grep
>> non-existing files, 2018-08-22) fixed a couple of similar issues; this
>> one was added later in 9b2df3e8d0 (rebase: save autostash entry into
>> stash reflog on --quit, 2020-04-28).
>>
>> [1] This patch modifies only a single test, but that test is run
>> several times with different strategies ('--apply', '--merge', and
>> '--interactive'), hence the plural "tests".
>>
>> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
>> ---
>> t/t3420-rebase-autostash.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
>> index 43fcb68f27..bbe82d2c0c 100755
>> --- a/t/t3420-rebase-autostash.sh
>> +++ b/t/t3420-rebase-autostash.sh
>> @@ -200,7 +200,7 @@ testrebase () {
>> git rebase --quit &&
>> test_when_finished git stash drop &&
>> test_path_is_missing $dotest/autostash &&
>> - ! grep dirty file3 &&
>> + test_path_is_missing file3 &&
>> git stash show -p >actual &&
>> test_cmp expect actual &&
>> git reset --hard &&
>> --
>> 2.33.0.1279.g1a260bf8c2
>
> It appears that this patch might have fallen quite deep through the
> cracks... ;)
Yeah, that indeed seems to be the case. It is surprising that
nobody even had any comment on it back then.
> But the issue this patch is addressing is still there, and the patch
> still applies cleanly after almost 5 years.
Will take a look and queue. Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/2] Silence po catalog output under "make -s"
From: Junio C Hamano @ 2026-06-28 1:43 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Harald Nordgren, git, Harald Nordgren via GitGitGadget
In-Reply-To: <40b7eee4-6b45-449f-a3a0-0ae415097041@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Am 26.06.26 um 21:27 schrieb Harald Nordgren:
>> What should I expect here, will it be merged to master now?
>
> These patches are cooking in my respective j6t-testing branches in my
> repositories[1][2]. I'll ask for inclusion in the Git repository in the
> coming weeks (but certainly not for v2.55).
>
> -- Hannes
>
> [1] https://github.com/j6t/git-gui/commits/j6t-testing/
> [2] https://github.com/j6t/gitk/commits/j6t-testing/
Thanks.
^ permalink raw reply
* Re: [PATCH v2 5/6] t: convert grep assertions to test_grep
From: Junio C Hamano @ 2026-06-28 1:41 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Michael Montalbo via GitGitGadget, git, D. Ben Knoble,
Eric Sunshine, Michael Montalbo
In-Reply-To: <xmqq4iio59uv.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>> I think in this case checking the file3's contents is wrong, because
>> at this point file3 should not exist in the first place. I've sent a
>> patch to fix this long ago, but apparently didn't manage to follow
>> through back then.
>>
>> https://lore.kernel.org/git/20211010172809.1472914-1-szeder.dev@gmail.com/
>
> Thanks. I guess the test_grep can be extended to catch this case,
> where
>
> test_grep ! -e pattern1 -e pattern2 file
>
> does not find any hits, but only because 'file' is missing, as an
> error, ...
Wait. The necessary check is already there, isn't it?
test_grep () {
eval "last_arg=\${$#}"
test -f "$last_arg" ||
BUG "test_grep requires a file to read as the last parameter"
So why don't we see it every time we run that test that inspects
file3's contents with Michael's series merged in? Puzzled...
^ permalink raw reply
* Re: 2.54.0: fyi: endless loop at 100% CPU
From: Michael Montalbo @ 2026-06-27 23:03 UTC (permalink / raw)
To: Michael Montalbo, git, Steffen Nurpmeso
In-Reply-To: <20260627201558.Bw6A-jbx@steffen%sdaoden.eu>
On Sat, Jun 27, 2026 at 1:16 PM Steffen Nurpmeso <steffen@sdaoden.eu> wrote:
>
> Thanks for these pointers, i did not know about such configuration
> variables. I will set them like you show.
No problem! Just to clarify, I'm not sure you should actually use those
configuration values verbatim. I was more pointing in the direction of
potentially relevant options for debugging / working around the issue.
^ permalink raw reply
* RE: Security Vulnerability in Git 2.54.0/OpenSSL 3.5.6 Status
From: Person, Tim @ 2026-06-27 21:17 UTC (permalink / raw)
To: Todd Zullinger; +Cc: git@vger.kernel.org
In-Reply-To: <20260627210718.zl0eH_Sc@teonanacatl.net>
Todd,
Thank you for the reply, the explanation, and the information about who to contact.
Thanks,
Tim
-----Original Message-----
From: Todd Zullinger <tmz@pobox.com>
Sent: Saturday, June 27, 2026 2:07 PM
To: Person, Tim <Tim.Person@personent.com>
Cc: git@vger.kernel.org
Subject: Re: Security Vulnerability in Git 2.54.0/OpenSSL 3.5.6 Status
[You don't often get email from tmz@pobox.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
[CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.]
Hi,
Person, Tim wrote:
> I am writing to determine when Git plans to release an update
> installer to patch the security vulnerability in Git 2.54.0 because of
> the included OpenSSL executable.
> This vulnerability is rated "Critical" in the CVE
> (https://www/
> .cve.org%2FCVERecord%3Fid%3DCVE-2026-34182&data=05%7C02%7CTim.Person%4
> 0personentcloud.mail.onmicrosoft.com%7C350b58458bd84a5312f308ded490243
> 8%7Ce2de18dc8323462e8c47561025ebc66c%7C0%7C0%7C639181913006654964%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C40000%7C%7C%7C&sdata=caMSGrA%2FfpxkKs2o%2Bg1dE9JuEQQlOK3IBt8BzbZ%2F7GM%3D&reserved=0). An updated version of the OpenSSL.exe fixing this problem has been available since 06/12/2026. I am just wondering if/when you plan to address this major security issue.
The Git project does not distribute any binaries. You likely want to direct this to the Git for Windows project¹.
That said, it's not even clear to me that the CVE you reference affects git's usage of OpenSSL.
From a little skimming, the issue affects use of CMS (which is something like the successor to S/MIME, as far as I can tell).
The only place where git gets close to that area is if you configure it to use x509 as gpg.program. And then git uses gpgsm, which is not affected by the CVE in OpenSSL.
¹ https://gitforwindows.org/
--
Todd
^ permalink raw reply
* Re: Security Vulnerability in Git 2.54.0/OpenSSL 3.5.6 Status
From: Todd Zullinger @ 2026-06-27 21:07 UTC (permalink / raw)
To: Person, Tim; +Cc: git@vger.kernel.org
In-Reply-To: <SN4P221MB0713994458A94BFCB51F7AC494EA2@SN4P221MB0713.NAMP221.PROD.OUTLOOK.COM>
Hi,
Person, Tim wrote:
> I am writing to determine when Git plans to release an
> update installer to patch the security vulnerability in
> Git 2.54.0 because of the included OpenSSL executable.
> This vulnerability is rated "Critical" in the CVE
> (https://www.cve.org/CVERecord?id=CVE-2026-34182). An
> updated version of the OpenSSL.exe fixing this problem has
> been available since 06/12/2026. I am just wondering
> if/when you plan to address this major security issue.
The Git project does not distribute any binaries. You
likely want to direct this to the Git for Windows project¹.
That said, it's not even clear to me that the CVE you
reference affects git's usage of OpenSSL.
From a little skimming, the issue affects use of CMS (which
is something like the successor to S/MIME, as far as I can
tell).
The only place where git gets close to that area is if you
configure it to use x509 as gpg.program. And then git uses
gpgsm, which is not affected by the CVE in OpenSSL.
¹ https://gitforwindows.org/
--
Todd
^ permalink raw reply
* Re: [PATCH GSoC v14 05/13] fetch-pack: prepare function to be moved
From: Pablo Sabater @ 2026-06-27 21:06 UTC (permalink / raw)
To: Karthik Nayak
Cc: git, chandrapratap3519, chriscool, eric.peijian, gitster,
jltobler, peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <CAOLa=ZScS3Gmm5BAgJF69phpaDXGnP_j9jx+bMhn_tfF65RXEg@mail.gmail.com>
El vie, 26 jun 2026 a las 18:54, Karthik Nayak
(<karthik.188@gmail.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> The subject doesn't really give much insight into what the patch does.
> Perhaps something like:
>
> fetch-pack: use repo config in `write_fetch_command_and_capabilities()`
> fetch-pack: drop static variable use in
> `write_fetch_command_and_capabilities()`
>
> > `write_fetch_command_and_capabilities()` will be refactored and moved in
> > subsequent commits where it will become a more general-purpose function,
> > making it more accessible to additional commands in the future.
> >
> > To move `write_fetch_command_and_capabilities()` to `connect.c`, we
> > previously need to adjust how `advertise_sid` is managed. Currently in
>
> I don't think 'previously' makes sense here.
>
> > `fetch_pack.c`, `advertise_sid` is a static variable, modified using
> > `repo_config_get_bool()`.
> >
>
> Perhaps:
>
> To move `write_fetch_command_and_capabilities()` to `connect.c`,
> drop the usage of file static variable `advertise_sid` within the
> function. Currently, `advertise_sid` is modified...
>
> >
> > Initialize `advertise_sid` at the begining by directly using
> > `repo_config_get_bool()`. This change is safe because:
> >
> > In the original `fetch-pack.c` code, there are only two places that write
> > `advertise_sid`:
> >
>
> This needs to be modified no? This is from the prev patch, where we
> moved and refactored in the same patch, this no longer is the case.
>
> > 1. In function `do_fetch_pack()`:
> > if (!server_supports("session_id"))
> > advertise_sid = 0;
> > 2. In function `fetch_pack_config()`:
> > repo_config_get_bool("transfer.advertisesid", &advertise_sid);
> >
> > About 1, since `do_fetch_pack()` is only relevant for protocol v1, this
> > assignment can be ignored, as `write_fetch_command_and_capabilities()`
> > is only used in v2.
> >
> > About 2, `repo_config_get_bool()` is from `config.h` and it's an
> > out-of-box dependency of `connect.c`, so we can reuse it directly.
> >
> > Helped-by: Jonathan Tan <jonathantanmy@google.com>
> > Helped-by: Christian Couder <chriscool@tuxfamily.org>
> > Signed-off-by: Calvin Wan <calvinwan@google.com>
> > Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> > ---
> > fetch-pack.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/fetch-pack.c b/fetch-pack.c
> > index f13951d154..ad07603755 100644
> > --- a/fetch-pack.c
> > +++ b/fetch-pack.c
> > @@ -1380,6 +1380,9 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
> > const struct string_list *server_options)
> > {
> > const char *hash_name;
> > + int advertise_sid;
> > +
> > + repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
> >
> > ensure_server_supports_v2("fetch");
> > packet_buf_write(req_buf, "command=fetch");
> > @@ -1395,7 +1398,7 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
> > }
> >
> > if (server_feature_v2("object-format", &hash_name)) {
> > - int hash_algo = hash_algo_by_name(hash_name);
> > + const unsigned int hash_algo = hash_algo_by_name(hash_name);
> >
>
> Agreed with Chandra, this needs to be assessed.
>
> > if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
> > die(_("mismatched algorithms: client %s; server %s"),
> > the_hash_algo->name, hash_name);
> >
> > --
> > 2.54.0
I'll reword the commit message with all you and Chandra reviewed.
Thanks for the feedback,
Pablo.
^ permalink raw reply
* Re: [PATCH GSoC v14 06/13] fetch-pack: move function to connect.c
From: Pablo Sabater @ 2026-06-27 21:04 UTC (permalink / raw)
To: Chandra Pratap
Cc: git, chriscool, eric.peijian, gitster, jltobler, karthik.188,
peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <CA+J6zkRm_F4MQ2K8Ayv8PGJOx+pNAg73+p-4VdOgkxeuKAkKew@mail.gmail.com>
El vie, 26 jun 2026 a las 14:14, Chandra Pratap
(<chandrapratap3519@gmail.com>) escribió:
>
> On Thu, 25 Jun 2026 at 17:43, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
> >
> > write_fetch_command_and_capabilities will be refactored in a subsequent
>
> Nit: The paragraph below and the preceding patches refer to this function
> as `write_fetch_command_and_capabilities()`. It will be nice to maintain
> consistency throughout this series.
I'll do that.
>
> > commit where it will become a more general-purpose function, making it
> > more accessible to additional commands in the future.
> >
> > Move `write_fetch_command_and_capabilities()` to `connect.c`, where
> > there are similar purpose functions.
> >
> > Because string_list is only used as a pointer, use a forward
> > declaration [1].
> >
> > [1]: https://lore.kernel.org/git/Z0RIqUAoEob8lGfM@pks.im/
> >
> > Helped-by: Jonathan Tan <jonathantanmy@google.com>
> > Helped-by: Christian Couder <chriscool@tuxfamily.org>
> > Signed-off-by: Calvin Wan <calvinwan@google.com>
> > Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> > ---
> [snip]
Thanks for the feedback,
Pablo.
^ permalink raw reply
* Re: [PATCH GSoC v14 06/13] fetch-pack: move function to connect.c
From: Pablo Sabater @ 2026-06-27 21:03 UTC (permalink / raw)
To: Karthik Nayak
Cc: git, chandrapratap3519, chriscool, eric.peijian, gitster,
jltobler, peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <CAOLa=ZRhCXBQN7CqwLyE2F9u+oqAsqvcFP+fuiyw4SVaSDfT6Q@mail.gmail.com>
El vie, 26 jun 2026 a las 18:57, Karthik Nayak
(<karthik.188@gmail.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> Nit: could we mention the function in the subject?
Yes, I'll mention it.
>
> > write_fetch_command_and_capabilities will be refactored in a
> > subsequent
>
> Super Nit: Some parts of your patches use backticks for quoting code or
> filenames and others skip the convention. It would be nice to be
> consistent.
Sorry about that, I'll stick to backticks always.
>
> > commit where it will become a more general-purpose function, making it
> > more accessible to additional commands in the future.
> >
> > Move `write_fetch_command_and_capabilities()` to `connect.c`, where
> > there are similar purpose functions.
> >
> > Because string_list is only used as a pointer, use a forward
> > declaration [1].
> >
> > [1]: https://lore.kernel.org/git/Z0RIqUAoEob8lGfM@pks.im/
> >
> > Helped-by: Jonathan Tan <jonathantanmy@google.com>
> > Helped-by: Christian Couder <chriscool@tuxfamily.org>
> > Signed-off-by: Calvin Wan <calvinwan@google.com>
> > Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> > ---
> > connect.c | 34 ++++++++++++++++++++++++++++++++++
> > connect.h | 4 ++++
> > fetch-pack.c | 34 ----------------------------------
> > 3 files changed, 38 insertions(+), 34 deletions(-)
> >
[snip]
Thanks for the feedback,
Pablo.
^ permalink raw reply
* Re: [PATCH GSoC v14 07/13] connect: refactor packet writing
From: Pablo Sabater @ 2026-06-27 20:58 UTC (permalink / raw)
To: Karthik Nayak
Cc: git, chandrapratap3519, chriscool, eric.peijian, gitster,
jltobler, peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <CAOLa=ZTdcg47nmZs2t1FvyOgG9S4Ap3RaK+C0Dhku6cG+wj_Kw@mail.gmail.com>
El vie, 26 jun 2026 a las 19:03, Karthik Nayak
(<karthik.188@gmail.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> The subject is bit too generic no? Maybe we can talk about the function?
> Perhaps:
>
> connect: make `write_fetch_command_and_capabilities()` more generic
Okay, I'll use that as the subject.
>
> > Refactor `write_fetch_command_and_capabilities()`, enabling it to serve
> > both fetch and additional commands.
> >
> > In this context, "command" refers to the "operations" supported by
> > Git's wire protocol https://git-scm.com/docs/protocol-v2, such as a Git
> > subcommand (e.g., git-fetch(1)) or a server-side operation like
> > "object-info" as implemented in commit a2ba162
> > (object-info: support for retrieving object info, 2021-04-20).
> >
> > Refactor the function signature to accept a command instead of the
> > hardcoded "fetch".
>
> [snip]
Thanks for the feedback,
Pablo.
^ permalink raw reply
* Re: [PATCH GSoC v14 11/13] cat-file: add remote-object-info to batch-command
From: Pablo Sabater @ 2026-06-27 20:51 UTC (permalink / raw)
To: Karthik Nayak
Cc: git, chandrapratap3519, chriscool, eric.peijian, gitster,
jltobler, peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <CAOLa=ZSCKbwckV-j+DyUqOkDkfYcW5xSCPza562mq+OJtQc7DA@mail.gmail.com>
El sáb, 27 jun 2026 a las 15:14, Karthik Nayak
(<karthik.188@gmail.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> [snip]
>
> > diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
> > index 86b9181599..aba20eb770 100644
> > --- a/Documentation/git-cat-file.adoc
> > +++ b/Documentation/git-cat-file.adoc
> > @@ -169,6 +169,13 @@ info <object>::
> > Print object info for object reference `<object>`. This corresponds to the
> > output of `--batch-check`.
> >
> > +remote-object-info <remote> <object>...::
> > + Print object info for object references `<object>` at specified
> > + `<remote>` without downloading objects from the remote.
> > + Raise an error when the `object-info` capability is not supported by the remote.
> > + Raise an error when no object references are provided.
> > + This command may be combined with `--buffer`.
> > +
> > flush::
> > Used with `--buffer` to execute all preceding commands that were issued
> > since the beginning or since the last flush was issued. When `--buffer`
> > @@ -312,7 +319,8 @@ newline. The available atoms are:
> > The full hex representation of the object name.
> >
> > `objecttype`::
> > - The type of the object (the same as `cat-file -t` reports).
> > + The type of the object (the same as `cat-file -t` reports). See
> > + `CAVEATS` below. Not supported by `remote-object-info`.
> >
>
> Do we have to keep adding 'Not supported by `remote-object-info`' to
> each type? Can't we do the inverse and only add 'Supported by
> `remote-object-info`' to `objectsize`. This avoid having to add this
> line to every new type.
Yes, I will do that.
>
> > If no format is specified, the default format is `%(objectname)
> > -%(objecttype) %(objectsize)`.
> > +%(objecttype) %(objectsize)`, except for `remote-object-info` commands which use
> > +`%(objectname) %(objectsize)` for now because "%(objecttype)" is not supported yet.
>
> Nit: I would drop the 'for now' here, since we don't know when the changes
> for 'objecttype' will land.
Okay, I'll drop it.
>
> [snip]
>
> > enum batch_mode {
> > BATCH_MODE_CONTENTS,
> > @@ -633,6 +649,81 @@ static void batch_one_object(const char *obj_name,
> > object_context_release(&ctx);
> > }
> >
> > +static int get_remote_info(struct batch_options *opt,
> > + int argc,
> > + const char **argv,
> > + struct object_info **remote_object_info,
> > + struct oid_array *object_info_oids)
> > +{
> > + int retval = 0;
> > + struct remote *remote = NULL;
> > + struct object_id oid;
> > + struct string_list object_info_options = STRING_LIST_INIT_NODUP;
> > + struct transport *gtransport;
> > +
> > + /*
> > + * Change the format to "%(objectname) %(objectsize)" when
>
> Nit: perhaps prepend a "TODO"
I'll add it.
>
> > + * remote-object-info command is used. Once we start supporting objecttype
> > + * the default format should change to DEFAULT_FORMAT.
> > + */
> > + if (!opt->format)
> > + opt->format = "%(objectname) %(objectsize)";
> > +
> > + remote = remote_get(argv[0]);
> > + if (!remote)
> > + die(_("must supply valid remote when using remote-object-info"));
> > +
> > + oid_array_clear(object_info_oids);
> > + for (size_t i = 1; i < argc; i++) {
> > + if (get_oid_hex(argv[i], &oid)) {
> > + size_t len = strlen(argv[i]);
> > +
> > + if (len < the_hash_algo->hexsz && len >= 4) {
> > + size_t j;
> > + for (j = 0; j < len; j++)
> > + if (!isxdigit(argv[i][j]))
> > + break;
> > + if (j == len)
> > + die(_("remote-object-info does not support "
> > + "short oids, %d characters required"),
> > + (int)the_hash_algo->hexsz);
> > + }
> > + die(_("not a valid object name '%s'"), argv[i]);
> > + }
> > + oid_array_append(object_info_oids, &oid);
> > + }
> > +
> > + if (!object_info_oids->nr)
> > + die(_("remote-object-info requires objects"));
> > +
> > + gtransport = transport_get(remote, NULL);
> > +
> > + if (!gtransport->smart_options) {
> > + retval = -1;
> > + goto cleanup;
> > + }
> > +
> > + CALLOC_ARRAY(*remote_object_info, object_info_oids->nr);
> > + gtransport->smart_options->object_info = 1;
> > + gtransport->smart_options->object_info_oids = object_info_oids;
> > +
> > + /* 'objectsize' is the only option currently supported */
> > + if (!strstr(opt->format, "%(objectsize)"))
> > + die(_("%s is currently not supported with remote-object-info"), opt->format);
> > +
>
> Aren't we setting the opt->format ourselves in this function? Why do we
> need to check it?
We only set `opt->format` when the user does not provide a custom format.
The `strstr` check catches cases like `%(objecttype)` alone, but is
not sufficient for mixed formats like `%(objecttype) %(objectsize)`.
This is fixed in [12/13] with the allow-list.
>
> > + string_list_append(&object_info_options, "size");
> > +
> > + if (object_info_options.nr > 0) {
> > + gtransport->smart_options->object_info_options = &object_info_options;
> > + gtransport->smart_options->object_info_data = *remote_object_info;
> > + retval = transport_fetch_refs(gtransport, NULL);
> > + }
> > +cleanup:
> > + string_list_clear(&object_info_options, 0);
> > + transport_disconnect(gtransport);
> > + return retval;
> > +}
> > +
> > struct object_cb_data {
> > struct batch_options *opt;
> > struct expand_data *expand;
> > @@ -714,6 +805,57 @@ static void parse_cmd_mailmap(struct batch_options *opt UNUSED,
> > load_mailmap();
> > }
> >
> > +static void parse_cmd_remote_object_info(struct batch_options *opt,
> > + const char *line, struct strbuf *output,
> > + struct expand_data *data)
> > +{
> > + int count;
> > + const char **argv;
> > + char *line_to_split;
> > + struct object_info *remote_object_info = NULL;
> > + struct oid_array object_info_oids = OID_ARRAY_INIT;
> > +
> > + if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
> > + die(_("remote-object-info command too long"));
> > +
> > + line_to_split = xstrdup(line);
> > + count = split_cmdline(line_to_split, &argv);
> > + if (count < 0)
> > + die(_("split remote-object-info command"));
>
> We should be using `split_cmdline_strerror()` here
Ok, I'll use it.
>
> > + if (count - 1 > MAX_ALLOWED_OBJ_LIMIT)
> > + die(_("remote-object-info supports at most %d objects"),
> > + MAX_ALLOWED_OBJ_LIMIT);
> > +
> > + if (get_remote_info(opt, count, argv, &remote_object_info,
> > + &object_info_oids))
> > + goto cleanup;
> > +
> > + data->skip_object_info = 1;
> > + for (size_t i = 0; i < object_info_oids.nr; i++) {
> > + data->oid = object_info_oids.oid[i];
> > + if (remote_object_info[i].sizep) {
> > + /*
> > + * When reaching here, it means remote-object-info can retrieve
> > + * information from server without downloading them.
> > + */
> > + data->size = *remote_object_info[i].sizep;
> > + opt->batch_mode = BATCH_MODE_INFO;
> > + batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
> > + } else {
> > + report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing");
> > + }
> > + }
> > + data->skip_object_info = 0;
> > +
> > +cleanup:
> > + for (size_t i = 0; i < object_info_oids.nr; i++)
> > + free_object_info_contents(&remote_object_info[i]);
> > + free(line_to_split);
> > + free(argv);
> > + free(remote_object_info);
> > + oid_array_clear(&object_info_oids);
> > +}
> > +
>
> [snip]
>
> > diff --git a/t/meson.build b/t/meson.build
> > index 3219264fe7..54d21111a3 100644
> > --- a/t/meson.build
> > +++ b/t/meson.build
> > @@ -170,6 +170,7 @@ integration_tests = [
> > 't1014-read-tree-confusing.sh',
> > 't1015-read-index-unmerged.sh',
> > 't1016-compatObjectFormat.sh',
> > + 't1017-cat-file-remote-object-info.sh',
> > 't1020-subdirectory.sh',
> > 't1022-read-tree-partial-clone.sh',
> > 't1050-large.sh',
>
> [snip]
Thanks for the feedback,
Pablo.
^ permalink raw reply
* Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values
From: Junio C Hamano @ 2026-06-27 20:47 UTC (permalink / raw)
To: Tian Yuchen
Cc: git, cirnovskyv, szeder.dev, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <04d1a7d5-ef83-4728-b816-5cdf1cb4aa25@malon.dev>
Tian Yuchen <cat@malon.dev> writes:
> Hi all,
>
> Apologies again for the duplicate...
>
> On 6/28/26 00:08, Tian Yuchen wrote:
>
>> +const char *repo_excludes_file(struct repository *repo)
>> +{
>> + if (!repo || !repo->initialized)
>> + return NULL;
I might already have said this, but I am not sure why want to be as
loose as this code. It is not limited to this line, but I think we
saw plenty of other "We know we must get an already initialized
thing here, and the subsequent operation we perform on that thing
will cause us to die() later, so let's return silently and early
to avoid hitting die()" attempts to sweep problems under the rug.
Wouldn't we rather want to try to be more strict and say
if (!repo || !repo->initialized)
BUG("repo must be an initialied repository");
here? Aren't all the callers of this function supposed to be
dealing with an already initialized repository?
>> + if (!repo_config_values(repo)->excludes_file)
>> + repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
>> +
>> + return repo_config_values(repo)->excludes_file;
>> +}
>
> One more thing:
>
> I deliberately didn't write a comment for the getter because it will
> probably be merged with comments from the previous several patches in
> some form in the near future... I'm not sure if it would be more
> appropriate to write a separate patch to add the corresponding comments
> then.
That's very sensible.
^ permalink raw reply
* Re: 2.54.0: fyi: endless loop at 100% CPU
From: Steffen Nurpmeso @ 2026-06-27 20:15 UTC (permalink / raw)
To: Michael Montalbo; +Cc: git, Steffen Nurpmeso
In-Reply-To: <CAC2Qwm+48Gpj=AWHzx-nO00bwVfuYoGiwd=3gExbybcOyHC45Q@mail.gmail.com>
Michael Montalbo wrote in
<CAC2Qwm+48Gpj=AWHzx-nO00bwVfuYoGiwd=3gExbybcOyHC45Q@mail.gmail.com>:
|Steffen Nurpmeso <steffen@sdaoden.eu> writes:
|> I have no idea and i am not looking either, but my scripted update
|> of tracked repos stuck, and i can a hundred percent reproduce an
|> endless loop that consumes hundred percent CPU by doing
|>
|> git ls-remote https://gitlab.xiph.org/xiph/opus.git
|
|Hello, thank you for the report.
|
|When I tried reproducing this locally I was able to get a response
|eventually, though there was what seemed to be a stall mid-way
|through the response from the server. After looking closer, the linked
|repo appears to be behind Anubis[1] which may be rate-limiting
|and/or blocking the requests from your script. FWIW, running:
|
|GIT_TRACE_CURL=1 git ls-remote https://gitlab.xiph.org/xiph/opus.git 2>&1
It now works for me. I cannot reproduce it no more.
(Fwiw i got the same behavior for all repositories there, i track
more from there.)
(And no "network hang", that is, whatever, but it really busy
looped!)
|locally showed the TLS handshake starting then pausing for a significant
|period of time before eventually completing the request successfully.
|Maybe running the command with the trace will show something on your
|end?
|
|Also, here are some other potentially relevant configuration options \
|[2][3]:
| git -c http.version=HTTP/1.1 \
| -c http.lowSpeedLimit=1000 \
| -c http.lowSpeedTime=10
|
|[1] https://anubis.techaro.lol/
|[2] https://git-scm.com/docs/git-config#Documentation/git-config.txt-htt\
|pversion
|[3] https://git-scm.com/docs/git-config#Documentation/git-config.txt-htt\
|plowSpeedLimit
Thanks for these pointers, i did not know about such configuration
variables. I will set them like you show. Before i only had
[http]
sslVerify = true
#sslCAInfo = /home/steffen/sec.arena/tls.git/cacert.pem
sslTry = true
--End of <CAC2Qwm+48Gpj=AWHzx-nO00bwVfuYoGiwd=3gExbybcOyHC45Q@mail.gmail\
.com>
Thank you. Ciao!
--steffen
|
|Der Kragenbaer, The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
^ permalink raw reply
* Re: [PATCH GSoC v14 10/13] transport: add client support for object-info
From: Pablo Sabater @ 2026-06-27 19:44 UTC (permalink / raw)
To: Karthik Nayak
Cc: git, chandrapratap3519, chriscool, eric.peijian, gitster,
jltobler, peff, toon
In-Reply-To: <CAOLa=ZT8u32YUqhnq-pMCCK8Qzx+7k-3E-+yAoM_miJ7BQjxTA@mail.gmail.com>
El sáb, 27 jun 2026 a las 2:22, Karthik Nayak
(<karthik.188@gmail.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> > From: Calvin Wan <calvinwan@google.com>
> >
> > Sometimes, it is beneficial to retrieve information about an object
> > without downloading it entirely. The server-side logic for this
> > functionality was implemented in commit "a2ba162cda (object-info:
> > support for retrieving object info, 2021-04-20)." And the wire
> > format is documented at
> > https://git-scm.com/docs/protocol-v2#_object_info.
> >
> > This commit introduces client functions to interact with the server.
> >
> > Currently, the client supports requesting a list of object IDs with
> > the 'size' feature from a v2 server. If the server does not advertise
> > this feature (i.e., transfer.advertiseobjectinfo is set to false),
> > the client will return an error and exit.
> >
> > Notice that the entire request is written into req_buf before being
> > sent to the remote. This approach follows the pattern used in the
> > `send_fetch_request()` logic within fetch-pack.c.
> > Streaming the request is not addressed in this patch.
> >
> > Helped-by: Jonathan Tan <jonathantanmy@google.com>
> > Helped-by: Christian Couder <chriscool@tuxfamily.org>
> > Signed-off-by: Calvin Wan <calvinwan@google.com>
> > Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> > ---
> > Makefile | 1 +
> > fetch-object-info.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > fetch-object-info.h | 22 +++++++++++++
> > fetch-pack.c | 3 ++
> > fetch-pack.h | 2 ++
> > meson.build | 1 +
> > transport-helper.c | 11 +++++--
> > transport.c | 28 ++++++++++++++++-
> > transport.h | 11 +++++++
> > 9 files changed, 166 insertions(+), 3 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 1cec251f43..ec4df39a6b 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1159,6 +1159,7 @@ LIB_OBJS += ewah/ewah_rlw.o
> > LIB_OBJS += exec-cmd.o
> > LIB_OBJS += fetch-negotiator.o
> > LIB_OBJS += fetch-pack.o
> > +LIB_OBJS += fetch-object-info.o
> > LIB_OBJS += fmt-merge-msg.o
> > LIB_OBJS += fsck.o
> > LIB_OBJS += fsmonitor.o
> > diff --git a/fetch-object-info.c b/fetch-object-info.c
> > new file mode 100644
> > index 0000000000..9c4ae9bd11
> > --- /dev/null
> > +++ b/fetch-object-info.c
> > @@ -0,0 +1,90 @@
> > +#include "git-compat-util.h"
> > +#include "gettext.h"
> > +#include "hex.h"
> > +#include "pkt-line.h"
> > +#include "connect.h"
> > +#include "oid-array.h"
> > +#include "odb.h"
> > +#include "fetch-object-info.h"
> > +#include "string-list.h"
> > +
> > +/* Sends git-cat-file object-info command and its arguments into the request buffer. */
>
> This file doesn't know about git-cat-file(1), nor should it care about
> it, right? Since git-cat-file(1) is simply a user of this function.
> Would it make more sense to structure the document around what this
> function is supposed to do?
>
> Also the comment for `fetch_object_info` seems to be similar, perhaps
> worthwhile changing both without referring to git-cat-file(1).
> Theoretically there could be other users in the future too.
`git cat-file` is currently the only consumer of `object-info` but you
are right, this file shouldn't know about it nor mention it in a
comment, I'll drop it.
>
> > +static void send_object_info_request(const int fd_out, struct object_info_args *args)
> > +{
> > + struct strbuf req_buf = STRBUF_INIT;
> > +
> > + write_command_and_capabilities(&req_buf, "object-info", args->server_options);
> > +
> > + if (unsorted_string_list_has_string(args->object_info_options, "size"))
> > + packet_buf_write(&req_buf, "size");
> > +
>
> Okay so if the user requests 'size', we forward that to the server. But
> what about the 'else' condition? Should we BUG() out?
In patch [13/13] the options are built dynamically and filtered
against what the server advertises, so by the time we reach here only
valid options remain. For this commit there is no client support yet
so there is no way of this to fail and in the next commit [11/13]:
get_remote_info()
[snip]
string_list_append(&object_info_options, "size");
if (object_info_options.nr > 0) {
gtransport->smart_options->object_info_options =
&object_info_options;
gtransport->smart_options->object_info_data =
*remote_object_info;
retval = transport_fetch_refs(gtransport, NULL);
}
[snip]
get_remote_info() appends "size" always to object_info_options and
this is changed in commit [12/13] with the allow-list where only asked
object-info is appended. I could move that check to this commit as if
size always gets appended the `if` below will always be true.
Also because size is always appended there can't be an else condition
to BUG() out.
>
> > + if (args->oids)
> > + for (size_t i = 0; i < args->oids->nr; i++)
> > + packet_buf_write(&req_buf, "oid %s", oid_to_hex(&args->oids->oid[i]));
> > +
> > + packet_buf_flush(&req_buf);
> > + if (write_in_full(fd_out, req_buf.buf, req_buf.len) < 0)
> > + die_errno(_("unable to write request to remote"));
> > +
>
> We write out all the oids, flush and write to the fd. Okay.
>
> > + strbuf_release(&req_buf);
> > +}
> > +
> > +int fetch_object_info(const enum protocol_version version, struct object_info_args *args,
> > + struct packet_reader *reader, struct object_info *object_info_data,
> > + const int stateless_rpc, const int fd_out)
> > +{
> > + int size_index = -1;
> > +
> > + switch (version) {
> > + case protocol_v2:
> > + if (!server_supports_v2("object-info"))
> > + die(_("object-info capability is not enabled on the server"));
> > + send_object_info_request(fd_out, args);
>
> So if the server does support 'object-info', we call
> `send_object_info_request()`. Makes sense.
>
> > + break;
> > + case protocol_v1:
> > + case protocol_v0:
> > + die(_("unsupported protocol version. expected v2"));
> > + case protocol_unknown_version:
> > + BUG("unknown protocol version");
> > + }
> > +
>
> Now that we've sent the request, we should start parsing the response.
>
> > + for (size_t i = 0; i < args->object_info_options->nr; i++) {
> > + if (packet_reader_read(reader) != PACKET_READ_NORMAL) {
> > + check_stateless_delimiter(stateless_rpc, reader,
> > + "stateless delimiter expected");
> > + return -1;
> > + }
> > +
> > + if (!string_list_has_string(args->object_info_options, reader->line))
> > + return -1;
> > +
> > + if (!strcmp(reader->line, "size")) {
> > + size_index = i;
> > + for (size_t j = 0; j < args->oids->nr; j++)
> > + object_info_data[j].sizep = xcalloc(1, sizeof(*object_info_data[j].sizep));
> > + }
> > + }
> > +
>
> So this function seems to iterate over the list of options to only find
> and store the indexes. If the server does support size we also allocate
> the pointers to store the size.
>
> Shouldn't we similarly BUG() if there is anything apart from 'size' here?
Because only size is appended and the server size support comes with
the series, there shouldn't be a scenario where anything else appears.
1. Full response with size.
2. OID unrecognized by the server.
We can quickly check this because the server response when the OID is
unrecognized is `OID SP` if there's nothing after the SP the object is
unrecognized.
>
> > + for (size_t i = 0; packet_reader_read(reader) == PACKET_READ_NORMAL && i < args->oids->nr; i++) {
> > + struct string_list object_info_values = STRING_LIST_INIT_DUP;
> > +
> > + string_list_split(&object_info_values, reader->line, " ", -1);
> > + if (0 <= size_index) {
> > + if (!strcmp(object_info_values.items[1 + size_index].string, "")) {
> > + FREE_AND_NULL(object_info_data[i].sizep);
> > + string_list_clear(&object_info_values, 0);
> > + continue;
> > + }
> > + if (strtoul_szt(object_info_values.items[1 + size_index].string,
> > + 10, object_info_data[i].sizep))
>
> This is now no longer correctly aligned.
I will fix it.
>
> > + die("object-info: ref %s has invalid size %s",
> > + object_info_values.items[0].string,
> > + object_info_values.items[1 + size_index].string);
> > + }
> > +
> > + string_list_clear(&object_info_values, 0);
> > + }
> > + check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected");
> > +
>
> We parse each line and obtain the size and parse it into
> `object_info_data[i].sizep`. If the value is missing, we simply continue
> the iteration.
Related to that above, if `size` is missing we FREE_AND_NULL so later
on `parse_cmd_remote_object_info()` can detect that and print
"missing" for that oid. This "missing" behavior follows what the local
option `info` does for unrecognized OIDs.
parse_cmd_remote_object_info()
[snip]
for (size_t i = 0; i < object_info_oids.nr; i++) {
data->oid = object_info_oids.oid[i];
if (remote_object_info[i].sizep) {
/*
* When reaching here, it means remote-object-info can retrieve
* information from server without downloading them.
*/
data->size = *remote_object_info[i].sizep;
opt->batch_mode = BATCH_MODE_INFO;
batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
} else {
report_object_status(opt,
oid_to_hex(&data->oid), &data->oid, "missing");
}
}
[snip]
>
> I think we had this discussion off the list about how this means that
> oids which do not have a size will not error out but rather display a
> missing info value.
The following commit [11/13] that uses the functions introduced in
this commit, will print "oid missing" in case of the object being
unrecognized by the server, the no error out (empty string) would be
in the scenario where the object is recognized by the server but the
capability is not. Because this same series introduces the capability
advertisement at [9/13] commit we can be sure that size will always be
supported.
Subsequent commits with the allow-list aim to tackle this with the
empty-strings when a server doesn't support a capability.
>
> The argument for not error'ing out was better user experience where the
> command would complete without exiting. I still think we should error
> out, because:
>
> 1. Without error'ing out, we'd have to display to the user a missing
> value token. There is contention around what this token should be,
> as such a token shouldn't be a valid value for the info type being
> displayed. Future options must be considered here.
I believe that better user experience was my argument last time we
discussed and I still believe that. But a stronger argument is that
the local option `info` doesn't die either but rather it prints "OID
missing" and the next commit where the cmd-related functions are
implemented tries to be consistent with it for unrecognized OIDs.
>
> 2. What will the error code of such a situation be? Do we consider it a
> success or a failure? Is there a situation where an object can have a
> missing size?
There are two different scenarios to consider here:
1. OID unrecognized by the server: the server responds with `oid SP`
(no value after the space).
In this case, the next commit [11/13] prints "<OID> missing" using
`report_object_status()`, which is consistent with how the local
`info` command handles missing objects in `--batch-command`.
2. OID recognized but a capability is not supported by the server:
subsequent commits [12/13] and [13/13] introduce an allow-list that
filters options against what the server advertises.
Unsupported placeholders return an empty string, following how
`for-each-ref`handles known but inapplicable atoms.
For the exit code, both cases return 0. In case 1, this matches the
local `info` behavior. In case 2, we are not failing to fetch, we
simply see that the server does not support the requested capabilities
and skip the request to the server. For the missing token (your point
1), we reuse the existing "missing" token from
`report_object_status()`, which is what the local code path already
uses, so no new token is introduced.
>
> > + return 0;
> > +}
> > diff --git a/fetch-object-info.h b/fetch-object-info.h
> > new file mode 100644
> > index 0000000000..d35284bd6b
> > --- /dev/null
> > +++ b/fetch-object-info.h
> > @@ -0,0 +1,22 @@
> > +#ifndef FETCH_OBJECT_INFO_H
> > +#define FETCH_OBJECT_INFO_H
> > +
> > +#include "pkt-line.h"
> > +#include "protocol.h"
> > +#include "odb.h"
> > +
> > +struct object_info_args {
> > + struct string_list *object_info_options;
> > + const struct string_list *server_options;
> > + struct oid_array *oids;
> > +};
> > +
> > +/*
> > + * Sends git-cat-file object-info command into the request buf and read the
> > + * results from packets.
> > + */
> > +int fetch_object_info(enum protocol_version version, struct object_info_args *args,
> > + struct packet_reader *reader, struct object_info *object_info_data,
> > + int stateless_rpc, int fd_out);
> > +
> > +#endif /* FETCH_OBJECT_INFO_H */
> > diff --git a/fetch-pack.c b/fetch-pack.c
> > index cdebd3476f..a86c93fc52 100644
> > --- a/fetch-pack.c
> > +++ b/fetch-pack.c
> > @@ -1742,6 +1742,9 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
> > if (args->depth > 0 || args->deepen_since || args->deepen_not)
> > args->deepen = 1;
> >
> > + if (args->object_info)
> > + state = FETCH_SEND_REQUEST;
> > +
> > while (state != FETCH_DONE) {
> > switch (state) {
> > case FETCH_CHECK_LOCAL:
> > diff --git a/fetch-pack.h b/fetch-pack.h
> > index 6d0dec7f41..5a428f11ed 100644
> > --- a/fetch-pack.h
> > +++ b/fetch-pack.h
> > @@ -16,6 +16,7 @@ struct fetch_pack_args {
> > const struct string_list *deepen_not;
> > struct list_objects_filter_options filter_options;
> > const struct string_list *server_options;
> > + struct object_info *object_info_data;
> >
> > /*
> > * If not NULL, during packfile negotiation, fetch-pack will send "have"
> > @@ -43,6 +44,7 @@ struct fetch_pack_args {
> > unsigned reject_shallow_remote:1;
> > unsigned deepen:1;
> > unsigned refetch:1;
> > + unsigned object_info:1;
> >
> > /*
> > * Indicate that the remote of this request is a promisor remote. The
> > diff --git a/meson.build b/meson.build
> > index 3247697f74..145c6882eb 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -347,6 +347,7 @@ libgit_sources = [
> > 'exec-cmd.c',
> > 'fetch-negotiator.c',
> > 'fetch-pack.c',
> > + 'fetch-object-info.c',
> > 'fmt-merge-msg.c',
> > 'fsck.c',
> > 'fsmonitor.c',
> > diff --git a/transport-helper.c b/transport-helper.c
> > index f195070788..c77599f6fb 100644
> > --- a/transport-helper.c
> > +++ b/transport-helper.c
> > @@ -727,8 +727,8 @@ static int fetch_refs(struct transport *transport,
> >
> > /*
> > * If we reach here, then the server, the client, and/or the transport
> > - * helper does not support protocol v2. --negotiate-only requires
> > - * protocol v2.
> > + * helper does not support protocol v2. --negotiate-only and cat-file
> > + * remote-object-info require protocol v2.
>
> This is not really true as of this commit. This only comes into effect
> in the next commit. So shouldn't this be added there? That said, I would
> modify this to only talk about object-info requiring v2 and drop the
> reference to cat-file.
Agreed, I will do that, thanks.
>
> > */
> > if (data->transport_options.acked_commits) {
> > warning(_("--negotiate-only requires protocol v2"));
> > @@ -744,6 +744,13 @@ static int fetch_refs(struct transport *transport,
> > free_refs(dummy);
> > }
> >
> > + /* fail the command explicitly to avoid further commands input. */
> > + if (transport->smart_options->object_info)
> > + die(_("remote-object-info requires protocol v2"));
> > +
> > + if (!data->get_refs_list_called)
> > + get_refs_list_using_list(transport, 0);
> > +
>
> Don't we already do this right above? Why is this needed again?
True, I'll drop the second `if`.
>
> > count = 0;
> > for (i = 0; i < nr_heads; i++)
> > if (!(to_fetch[i]->status & REF_STATUS_UPTODATE))
> > diff --git a/transport.c b/transport.c
> > index 0f5ec30247..7d3246e12b 100644
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -9,6 +9,7 @@
> > #include "hook.h"
> > #include "pkt-line.h"
> > #include "fetch-pack.h"
> > +#include "fetch-object-info.h"
> > #include "remote.h"
> > #include "connect.h"
> > #include "send-pack.h"
> > @@ -467,8 +468,33 @@ static int fetch_refs_via_pack(struct transport *transport,
> > args.negotiation_restrict_tips = data->options.negotiation_restrict_tips;
> > args.negotiation_include_tips = data->options.negotiation_include_tips;
> > args.reject_shallow_remote = transport->smart_options->reject_shallow;
> > + args.object_info = transport->smart_options->object_info;
> > +
>
> Hmm, so we piggy-back on top of the `fetch_refs_via_pack()` function...
>
> > + if (transport->smart_options->object_info
> > + && transport->smart_options->object_info_oids->nr > 0) {
> > + struct packet_reader reader;
> > + struct object_info_args obj_info_args = { 0 };
> > +
> > + obj_info_args.server_options = transport->server_options;
> > + obj_info_args.oids = transport->smart_options->object_info_oids;
> > + obj_info_args.object_info_options = transport->smart_options->object_info_options;
> > + string_list_sort(obj_info_args.object_info_options);
> > +
> > + connect_setup(transport, 0);
> > + packet_reader_init(&reader, data->fd[0], NULL, 0,
> > + PACKET_READ_CHOMP_NEWLINE |
> > + PACKET_READ_GENTLE_ON_EOF |
> > + PACKET_READ_DIE_ON_ERR_PACKET);
> > +
> > + data->version = discover_version(&reader);
> > + transport->hash_algo = reader.hash_algo;
> > +
> > + ret = fetch_object_info(data->version, &obj_info_args, &reader,
> > + data->options.object_info_data, transport->stateless_rpc,
> > + data->fd[1]);
> > + goto cleanup;
> >
>
> ... and we jump to exit when we only want object info information. This
> skips the call to fetch_pack(). I'm a bit uneasy with this. Ideally we
> should be adding a new function to the vtable to only fetch object info.
> While this works, this doesn't fit the contract of what this function is
> supposed to do. See the comment around `fetch_refs` in `struct
> transport_vtable`. Shouldn't we update that documentation at the very
> least?
Now that you point that out, piggy-backing doesn't look very good,
while it works, updating the documentation leaves this in a bad
situation, so I will move it to a new function and add it to the
vtable for the next version. Thanks for noticing it.
>
> > - if (!data->finished_handshake) {
> > + } else if (!data->finished_handshake) {
> > int i;
> > int must_list_refs = 0;
> > for (i = 0; i < nr_heads; i++) {
> > diff --git a/transport.h b/transport.h
> > index 7e5867cffa..bd60b10af4 100644
> > --- a/transport.h
> > +++ b/transport.h
> > @@ -6,6 +6,7 @@
> > #include "list-objects-filter-options.h"
> > #include "string-list.h"
> > #include "connect.h"
> > +#include "odb.h"
> >
> > struct git_transport_options {
> > unsigned thin : 1;
> > @@ -31,6 +32,12 @@ struct git_transport_options {
> > */
> > unsigned connectivity_checked:1;
> >
> > + /*
> > + * Transport will attempt to retrieve only object-info.
> > + * If object-info is not supported, the operation will error and exit.
> > + */
> > + unsigned object_info : 1;
> > +
>
> According to our style, this should be `unsigned object_info:1`.
I'll fix it to properly follow the style, I kept it like this because
other bit fields on these files seem to be outdated in that style.
>
> > int depth;
> > const char *deepen_since;
> > const struct string_list *deepen_not;
> > @@ -55,6 +62,10 @@ struct git_transport_options {
> > * common commits to this oidset instead of fetching any packfiles.
> > */
> > struct oidset *acked_commits;
> > +
> > + struct oid_array *object_info_oids;
> > + struct object_info *object_info_data;
> > + struct string_list *object_info_options;
> > };
> >
> > enum transport_family {
> >
> > --
> > 2.54.0
>
> Nit: I do think the commit message doesn't sufficiently capture the
> entirety of this patch. We do not talk about:
>
> 1. How we piggy-back on top of `fetch_refs_via_pack()` and don't
> introduce our own pointer in the vtable.
> 2. The changes made to 'transport-helper.c' and why that is required.
> 3. How we don't error out when there is no size value provided for an
> OID and what the implication of this is.
I will extend the commit message to include all 3 points (I'll change
the piggy-backing to have its own function).
Thanks for the review,
Pablo.
^ permalink raw reply
* Re: 2.54.0: fyi: endless loop at 100% CPU
From: Michael Montalbo @ 2026-06-27 19:39 UTC (permalink / raw)
To: steffen; +Cc: git
Steffen Nurpmeso <steffen@sdaoden.eu> writes:
> I have no idea and i am not looking either, but my scripted update
> of tracked repos stuck, and i can a hundred percent reproduce an
> endless loop that consumes hundred percent CPU by doing
>
> git ls-remote https://gitlab.xiph.org/xiph/opus.git
Hello, thank you for the report.
When I tried reproducing this locally I was able to get a response
eventually, though there was what seemed to be a stall mid-way
through the response from the server. After looking closer, the linked
repo appears to be behind Anubis[1] which may be rate-limiting
and/or blocking the requests from your script. FWIW, running:
GIT_TRACE_CURL=1 git ls-remote https://gitlab.xiph.org/xiph/opus.git 2>&1
locally showed the TLS handshake starting then pausing for a significant
period of time before eventually completing the request successfully.
Maybe running the command with the trace will show something on your
end?
Also, here are some other potentially relevant configuration options [2][3]:
git -c http.version=HTTP/1.1 \
-c http.lowSpeedLimit=1000 \
-c http.lowSpeedTime=10
[1] https://anubis.techaro.lol/
[2] https://git-scm.com/docs/git-config#Documentation/git-config.txt-httpversion
[3] https://git-scm.com/docs/git-config#Documentation/git-config.txt-httplowSpeedLimit
^ permalink raw reply
* Security Vulnerability in Git 2.54.0/OpenSSL 3.5.6 Status
From: Person, Tim @ 2026-06-27 19:18 UTC (permalink / raw)
To: git@vger.kernel.org
Good afternoon,
I am writing to determine when Git plans to release an update installer to patch the security vulnerability in Git 2.54.0 because of the included OpenSSL executable. This vulnerability is rated "Critical" in the CVE (https://www.cve.org/CVERecord?id=CVE-2026-34182). An updated version of the OpenSSL.exe fixing this problem has been available since 06/12/2026. I am just wondering if/when you plan to address this major security issue.
Respectfully,
Tim Person
^ permalink raw reply
* [PATCH v3 2/2] push: suggest <remote> <branch> for a slash slip
From: Harald Nordgren via GitGitGadget @ 2026-06-27 18:02 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2331.v3.git.git.1782583345.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
When pushing the 'main' branch to the remote 'origin', i.e.,
$ git push origin main
it is easy to mistakenly write
$ git push origin/main
That is parsed as the repository to push to, and since 'origin/main'
is neither a configured remote nor a path it dies with:
fatal: 'origin/main' does not appear to be a git repository
Often 'origin/main' does not exist as a repository, so the command
fails without doing any harm, but it gives no hint that a space was
meant instead of a slash and can leave the user puzzled.
When the argument is not an existing path or configured remote but
its part before the first slash names one, suggest the intended
'<remote> <branch>' form:
$ git push origin main
The suggestion is shown as advice so it can be silenced with
advice.pushRepoLooksLikeRef.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/config/advice.adoc | 5 +++++
advice.c | 1 +
advice.h | 1 +
builtin/push.c | 37 +++++++++++++++++++++++++++++++-
t/t5529-push-errors.sh | 31 ++++++++++++++++++++++++++
5 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db58918..fa77a5110e 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -90,6 +90,11 @@ all advice messages.
Shown when linkgit:git-push[1] rejects a forced update of
a branch when its remote-tracking ref has updates that we
do not have locally.
+ pushRepoLooksLikeRef::
+ Shown when the repository given to linkgit:git-push[1] is not
+ a configured remote but looks like a `<remote>/<branch>` ref,
+ suggesting that the remote and branch be given as separate
+ arguments.
pushUnqualifiedRefname::
Shown when linkgit:git-push[1] gives up trying to
guess based on the source and destination refs what
diff --git a/advice.c b/advice.c
index 0018501b7b..63bf8b0c5f 100644
--- a/advice.c
+++ b/advice.c
@@ -69,6 +69,7 @@ static struct {
[ADVICE_PUSH_NON_FF_CURRENT] = { "pushNonFFCurrent" },
[ADVICE_PUSH_NON_FF_MATCHING] = { "pushNonFFMatching" },
[ADVICE_PUSH_REF_NEEDS_UPDATE] = { "pushRefNeedsUpdate" },
+ [ADVICE_PUSH_REPO_LOOKS_LIKE_REF] = { "pushRepoLooksLikeRef" },
[ADVICE_PUSH_UNQUALIFIED_REF_NAME] = { "pushUnqualifiedRefName" },
[ADVICE_PUSH_UPDATE_REJECTED] = { "pushUpdateRejected" },
[ADVICE_PUSH_UPDATE_REJECTED_ALIAS] = { "pushNonFastForward" }, /* backwards compatibility */
diff --git a/advice.h b/advice.h
index 8def280688..66f6cd6a77 100644
--- a/advice.h
+++ b/advice.h
@@ -36,6 +36,7 @@ enum advice_type {
ADVICE_PUSH_NON_FF_CURRENT,
ADVICE_PUSH_NON_FF_MATCHING,
ADVICE_PUSH_REF_NEEDS_UPDATE,
+ ADVICE_PUSH_REPO_LOOKS_LIKE_REF,
ADVICE_PUSH_UNQUALIFIED_REF_NAME,
ADVICE_PUSH_UPDATE_REJECTED,
ADVICE_PUSH_UPDATE_REJECTED_ALIAS,
diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..1b2ad3b8df 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -8,6 +8,7 @@
#include "advice.h"
#include "branch.h"
#include "config.h"
+#include "dir.h"
#include "environment.h"
#include "gettext.h"
#include "hex.h"
@@ -662,6 +663,29 @@ static int push_multiple(struct string_list *list,
return result;
}
+static void die_if_repo_looks_like_ref(const char *repo)
+{
+ const char *slash = strchr(repo, '/');
+ struct strbuf name = STRBUF_INIT;
+ int code;
+
+ if (!slash || !slash[1] || file_exists(repo))
+ return;
+
+ strbuf_add(&name, repo, slash - repo);
+ if (!remote_is_configured(remote_get(name.buf), 0)) {
+ strbuf_release(&name);
+ return;
+ }
+
+ code = die_message(_("'%s' is not a valid push target"), repo);
+ advise_if_enabled(ADVICE_PUSH_REPO_LOOKS_LIKE_REF,
+ _("Did you mean to use: git push %s %s?"),
+ name.buf, slash + 1);
+ strbuf_release(&name);
+ exit(code);
+}
+
int cmd_push(int argc,
const char **argv,
const char *prefix,
@@ -744,6 +768,17 @@ int cmd_push(int argc,
if (repo) {
if (!add_remote_or_group(repo, &remote_group)) {
+ struct remote *r;
+
+ /*
+ * Check the advice up front to avoid the remote
+ * lookup when the hint is off. The helper still
+ * calls advise_if_enabled() so the hint carries the
+ * standard "disable this message" instructions.
+ */
+ if (advice_enabled(ADVICE_PUSH_REPO_LOOKS_LIKE_REF))
+ die_if_repo_looks_like_ref(repo);
+
/*
* Not a configured remote name or group name.
* Try treating it as a direct URL or path, e.g.
@@ -753,7 +788,7 @@ int cmd_push(int argc,
* from the URL so the loop below can handle it
* identically to a named remote.
*/
- struct remote *r = pushremote_get(repo);
+ r = pushremote_get(repo);
if (!r)
die(_("bad repository '%s'"), repo);
string_list_append(&remote_group, r->name);
diff --git a/t/t5529-push-errors.sh b/t/t5529-push-errors.sh
index 80b06a0cd2..2294645902 100755
--- a/t/t5529-push-errors.sh
+++ b/t/t5529-push-errors.sh
@@ -54,6 +54,37 @@ test_expect_success 'detect empty remote with targeted refspec' '
grep "fatal: bad repository ${SQ}${SQ}" stderr
'
+test_expect_success 'suggest <remote> <branch> for a <remote>/<branch> slip' '
+ test_must_fail git push origin/main 2>stderr &&
+ test_grep "${SQ}origin/main${SQ} is not a valid push target" stderr &&
+ test_grep "hint: Did you mean to use: git push origin main?" stderr &&
+ test_must_fail git -c advice.pushRepoLooksLikeRef=false push origin/main 2>stderr &&
+ test_grep ! "Did you mean" stderr
+'
+
+test_expect_success 'suggest <remote> <branch> when the branch has slashes' '
+ test_must_fail git push origin/feature/x 2>stderr &&
+ test_grep "hint: Did you mean to use: git push origin feature/x?" stderr
+'
+
+test_expect_success 'no suggestion when prefix is not a configured remote' '
+ test_must_fail git push not-a-remote/main 2>stderr &&
+ test_grep ! "Did you mean" stderr
+'
+
+test_expect_success 'no suggestion for a trailing slash with no branch' '
+ test_must_fail git push origin/ 2>stderr &&
+ test_grep ! "Did you mean" stderr
+'
+
+test_expect_success 'no suggestion when the argument is an existing path' '
+ test_when_finished "rm -rf origin" &&
+ git init --bare origin/main &&
+ git push origin/main HEAD:refs/heads/pushed 2>stderr &&
+ test_grep ! "Did you mean" stderr &&
+ git -C origin/main rev-parse --verify refs/heads/pushed
+'
+
test_expect_success 'detect ambiguous refs early' '
git branch foo &&
git tag foo &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/2] branch: suggest <remote>/<branch> on upstream slip
From: Harald Nordgren via GitGitGadget @ 2026-06-27 18:02 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2331.v3.git.git.1782583345.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
When setting the upstream of the current branch to the 'main' branch
of the remote 'origin', i.e.,
$ git branch --set-upstream-to origin/main
it is easy to mistakenly write
$ git branch --set-upstream-to origin main
That is parsed as a request to set the upstream of the local branch
'main' to 'origin'. When 'main' does not exist, the command dies
with:
fatal: branch 'main' does not exist
pointing at a branch the user never meant to name. When 'main' does
exist, it instead dies with:
fatal: the requested upstream branch 'origin' does not exist
leaving the user equally puzzled.
When the operated-on branch is missing and '<remote>/<branch>' names
a real remote-tracking ref, suggest the intended form:
$ git branch --set-upstream-to=origin/main
The suggestion is gated on '<remote>/<branch>' existing so it only
appears when a slipped slash is the likely explanation.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/branch.c | 32 ++++++++++++++++++++++++++++++++
t/t3200-branch.sh | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+)
diff --git a/builtin/branch.c b/builtin/branch.c
index 1572a4f9ef..dede60d27b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -706,6 +706,29 @@ static int edit_branch_description(const char *branch_name)
return 0;
}
+static void die_if_upstream_looks_like_remote(const char *new_upstream, const char *branch_name)
+{
+ struct strbuf remote_ref = STRBUF_INIT;
+ int code;
+
+ if (strchr(new_upstream, '/') ||
+ !remote_is_configured(remote_get(new_upstream), 0))
+ return;
+
+ strbuf_addf(&remote_ref, "refs/remotes/%s/%s", new_upstream, branch_name);
+ if (!refs_ref_exists(get_main_ref_store(the_repository), remote_ref.buf)) {
+ strbuf_release(&remote_ref);
+ return;
+ }
+
+ code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+ advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
+ _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
+ new_upstream, branch_name);
+ strbuf_release(&remote_ref);
+ exit(code);
+}
+
int cmd_branch(int argc,
const char **argv,
const char *prefix,
@@ -957,6 +980,15 @@ int cmd_branch(int argc,
if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
if (!argc || branch_checked_out(branch->refname))
die(_("no commit on branch '%s' yet"), branch->name);
+ /*
+ * Check the advice up front to avoid the ref
+ * lookups when the hint is off. The helper still
+ * calls advise_if_enabled() so the hint carries the
+ * standard "disable this message" instructions.
+ */
+ if (argc == 1 &&
+ advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
+ die_if_upstream_looks_like_remote(new_upstream, argv[0]);
die(_("branch '%s' does not exist"), branch->name);
}
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index e7829c2c4b..e2682a83a0 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1022,6 +1022,44 @@ test_expect_success '--set-upstream-to fails on a missing dst branch' '
test_cmp expect err
'
+test_expect_success '--set-upstream-to suggests <remote>/<branch> on slip' '
+ test_when_finished "git remote remove slip-remote" &&
+ git remote add slip-remote . &&
+ git update-ref refs/remotes/slip-remote/slip-feature HEAD &&
+ test_must_fail git branch --set-upstream-to slip-remote slip-feature 2>err &&
+ test_grep "takes a single <remote>/<branch> argument" err &&
+ test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip-feature?" err &&
+ test_must_fail git -c advice.setUpstreamFailure=false \
+ branch --set-upstream-to slip-remote slip-feature 2>err &&
+ test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to does not suggest when no matching remote ref' '
+ test_when_finished "git remote remove slip-remote" &&
+ git remote add slip-remote . &&
+ test_must_fail git branch --set-upstream-to slip-remote no-such-branch 2>err &&
+ test_grep "branch ${SQ}no-such-branch${SQ} does not exist" err &&
+ test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to to a local branch is not mistaken for a slip' '
+ git branch slip-local-upstream &&
+ git branch slip-local-target &&
+ git branch --set-upstream-to=slip-local-upstream slip-local-target 2>err &&
+ test_grep ! "Did you mean" err &&
+ echo refs/heads/slip-local-upstream >expect &&
+ git config branch.slip-local-target.merge >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--set-upstream-to slip suggestion keeps a slashed branch name' '
+ test_when_finished "git remote remove slip-remote" &&
+ git remote add slip-remote . &&
+ git update-ref refs/remotes/slip-remote/slip/feature HEAD &&
+ test_must_fail git branch --set-upstream-to slip-remote slip/feature 2>err &&
+ test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip/feature?" err
+'
+
test_expect_success '--set-upstream-to fails on a missing src branch' '
test_must_fail git branch --set-upstream-to does-not-exist main 2>err &&
test_grep "the requested upstream branch '"'"'does-not-exist'"'"' does not exist" err
--
gitgitgadget
^ permalink raw reply related
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