* [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
* 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
* 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
* 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: 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 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: [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 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 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 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 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: 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: 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: 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
page: | 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