* [PATCH 1/3] connect: let write_command_and_capabilities() send a promisor-remote reply
2026-11-12 19:58 [PATCH 0/3] Fix promisor-remote reply missing from fetch Christian Couder
@ 2026-11-12 19:59 ` Christian Couder
2026-11-12 22:18 ` Junio C Hamano
2026-11-12 19:59 ` [PATCH 2/3] fetch-pack: ensure fetch sends " Christian Couder
2026-11-12 19:59 ` [PATCH 3/3] doc: gitprotocol-v2: send the promisor-remote reply with fetch Christian Couder
2 siblings, 1 reply; 6+ messages in thread
From: Christian Couder @ 2026-11-12 19:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Matt Cote, Christian Couder
In protocol v2, the `promisor-remote=...` reply is only written by
send_capabilities(), which is used to send the `ls-refs` and
`bundle-uri` protocol commands.
The request for the `fetch` protocol command is built by
write_command_and_capabilities() called by send_fetch_request() in
"fetch-pack.c" which cannot send that reply.
In a following commit, we will need send_fetch_request() to send that
reply.
Let's prepare for that by adding a new `promisor_remote_reply` argument
to write_command_and_capabilities(). When it is not NULL, a
`promisor-remote=...` capability with that value is written into the
request.
For now, all the callers pass NULL, so there is no change in behavior.
Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
connect.c | 6 +++++-
connect.h | 6 ++++--
fetch-object-info.c | 3 ++-
fetch-pack.c | 6 ++++--
4 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/connect.c b/connect.c
index 87d2e8bccd..32243948b4 100644
--- a/connect.c
+++ b/connect.c
@@ -724,7 +724,8 @@ int server_supports(const char *feature)
}
void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
- const struct string_list *server_options)
+ const struct string_list *server_options,
+ const char *promisor_remote_reply)
{
const char *hash_name;
int advertise_sid = 0;
@@ -754,6 +755,9 @@ void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
die(_("the server does not support algorithm '%s'"),
the_hash_algo->name);
}
+ if (promisor_remote_reply)
+ packet_buf_write(req_buf, "promisor-remote=%s",
+ promisor_remote_reply);
packet_buf_delim(req_buf);
}
diff --git a/connect.h b/connect.h
index 957e5fe2b9..8914d131c7 100644
--- a/connect.h
+++ b/connect.h
@@ -37,9 +37,11 @@ void check_stateless_delimiter(int stateless_rpc,
struct string_list;
/*
* Write a protocol v2 command request, along with the capability
- * advertisements, into req_buf.
+ * advertisements, into req_buf. When `promisor_remote_reply` is not
+ * NULL, a `promisor-remote` capability is sent with that value.
*/
void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
- const struct string_list *server_options);
+ const struct string_list *server_options,
+ const char *promisor_remote_reply);
#endif
diff --git a/fetch-object-info.c b/fetch-object-info.c
index 0a58308f9b..7fe6e4e864 100644
--- a/fetch-object-info.c
+++ b/fetch-object-info.c
@@ -18,7 +18,8 @@ static void send_object_info_request(const int fd_out,
{
struct strbuf req_buf = STRBUF_INIT;
- write_command_and_capabilities(&req_buf, "object-info", server_options);
+ write_command_and_capabilities(&req_buf, "object-info",
+ server_options, NULL);
if (ask_size)
packet_buf_write(&req_buf, "size");
diff --git a/fetch-pack.c b/fetch-pack.c
index 55d311c0c3..50c97f561e 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1386,7 +1386,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
int done_sent = 0;
struct strbuf req_buf = STRBUF_INIT;
- write_command_and_capabilities(&req_buf, "fetch", args->server_options);
+ write_command_and_capabilities(&req_buf, "fetch",
+ args->server_options, NULL);
if (args->use_thin_pack)
packet_buf_write(&req_buf, "thin-pack");
@@ -2269,7 +2270,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
the_repository, "%d",
negotiation_round);
strbuf_reset(&req_buf);
- write_command_and_capabilities(&req_buf, "fetch", server_options);
+ write_command_and_capabilities(&req_buf, "fetch",
+ server_options, NULL);
packet_buf_write(&req_buf, "wait-for-done");
--
2.56.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] fetch-pack: ensure fetch sends a promisor-remote reply
2026-11-12 19:58 [PATCH 0/3] Fix promisor-remote reply missing from fetch Christian Couder
2026-11-12 19:59 ` [PATCH 1/3] connect: let write_command_and_capabilities() send a promisor-remote reply Christian Couder
@ 2026-11-12 19:59 ` Christian Couder
2026-11-12 19:59 ` [PATCH 3/3] doc: gitprotocol-v2: send the promisor-remote reply with fetch Christian Couder
2 siblings, 0 replies; 6+ messages in thread
From: Christian Couder @ 2026-11-12 19:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Matt Cote, Christian Couder
In protocol v2, when the client accepts promisor remotes, the
`promisor-remote=...` reply is only written by send_capabilities(),
which is used to send the `ls-refs` and `bundle-uri` protocol commands.
The request for the `fetch` protocol command is built by
write_command_and_capabilities() called by send_fetch_request() in
"fetch-pack.c" which doesn't send that reply. do_fetch_pack_v2() calls
promisor_remote_reply(), but only to mark the accepted remotes, as
e15a6b2f8b (promisor-remote: change promisor_remote_reply()'s
signature, 2026-02-16) assumed that fetch-pack would not need to send
a reply.
With file:// and ssh:// URLs on a Git server, one upload-pack process
handles both `ls-refs` and `fetch`. The acceptance recorded during
`ls-refs` by promisor_remote_receive() in "serve.c" is still there at
`fetch` time, so the clone succeeds. But it fails when each request is
handled on its own, for example with HTTP where different upload-pack
processes are used, or with other server implementations like JGit.
It also fails when only object IDs are fetched, as no `ls-refs`
command is then sent. In these cases, the server doesn't know that the
client accepted a promisor remote, so it doesn't allow promisor
objects to be missing from the pack it sends, and it fails with "could
not fetch <oid> from promisor remote".
Let's fix that by always properly sending the `promisor-remote=...`
reply when sending a `fetch` request from do_fetch_pack_v2().
The `fetch` requests sent by negotiate_using_fetch() don't ask for a
pack, so they don't need that reply.
Let's also add tests in t5710 to check that the reply is sent with the
`fetch` request, and that fetching only an object ID works. For HTTP,
let's add a new t5711 test script, copying the setup from t5710.
Reported-by: Matt Cote <mcote@nvidia.com>
Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
fetch-pack.c | 13 ++-
t/meson.build | 1 +
t/t5710-promisor-remote-capability.sh | 40 +++++++
t/t5711-promisor-remote-capability-http.sh | 126 +++++++++++++++++++++
4 files changed, 176 insertions(+), 4 deletions(-)
create mode 100755 t/t5711-promisor-remote-capability-http.sh
diff --git a/fetch-pack.c b/fetch-pack.c
index 50c97f561e..2f1bb9d62c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1380,14 +1380,16 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
const struct ref *wants, struct oidset *common,
int *haves_to_send, int *in_vain,
int sideband_all, int seen_ack,
- struct oidset *negotiation_include_oids)
+ struct oidset *negotiation_include_oids,
+ const char *promisor_remote_reply)
{
int haves_added;
int done_sent = 0;
struct strbuf req_buf = STRBUF_INIT;
write_command_and_capabilities(&req_buf, "fetch",
- args->server_options, NULL);
+ args->server_options,
+ promisor_remote_reply);
if (args->use_thin_pack)
packet_buf_write(&req_buf, "thin-pack");
@@ -1696,11 +1698,12 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
int i;
struct strvec index_pack_args = STRVEC_INIT;
const char *promisor_remote_config;
+ char *accepted = NULL;
fsck_options_init(&fsck_options, the_repository, FSCK_OPTIONS_MISSING_GITMODULES);
if (server_feature_v2("promisor-remote", &promisor_remote_config))
- promisor_remote_reply(promisor_remote_config, NULL);
+ promisor_remote_reply(promisor_remote_config, &accepted);
if (args->filter_options.choice == LOFC_AUTO) {
struct strbuf errbuf = STRBUF_INIT;
@@ -1778,7 +1781,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
&haves_to_send, &in_vain,
reader.use_sideband,
seen_ack,
- &negotiation_include_oids)) {
+ &negotiation_include_oids,
+ accepted)) {
trace2_region_leave_printf("negotiation_v2", "round",
the_repository, "%d",
negotiation_round);
@@ -1928,6 +1932,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
if (negotiator)
negotiator->release(negotiator);
+ free(accepted);
fsck_options_clear(&fsck_options);
oidset_clear(&common);
oidset_clear(&negotiation_include_oids);
diff --git a/t/meson.build b/t/meson.build
index 1cf1d95f70..84644703f1 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -766,6 +766,7 @@ integration_tests = [
't5704-protocol-violations.sh',
't5705-session-id-in-capabilities.sh',
't5710-promisor-remote-capability.sh',
+ 't5711-promisor-remote-capability-http.sh',
't5730-protocol-v2-bundle-uri-file.sh',
't5731-protocol-v2-bundle-uri-git.sh',
't5732-protocol-v2-bundle-uri-http.sh',
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index 463e9e00b0..e5d6f9d1c6 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -999,6 +999,46 @@ test_expect_success "clone and fetch with --filter=auto" '
check_missing_objects server 1 "$oid"
'
+test_expect_success "clone sends the promisor-remote reply with fetch" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client trace" &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git clone \
+ -c remote.lop.promisor=true \
+ -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+ -c promisor.acceptfromserver=All \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that the reply is part of the fetch request
+ sed -n "/clone> command=fetch/,/clone> 0001/p" trace >request &&
+ test_grep "clone> promisor-remote=lop" request &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects server 1 "$oid"
+'
+
+test_expect_success "fetch by object ID sends the promisor-remote reply" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client trace" &&
+
+ git init client &&
+ git -C client remote add origin "$TRASH_DIRECTORY_URL/server" &&
+ git -C client config remote.lop.promisor true &&
+ git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" &&
+ git -C client config promisor.acceptfromserver All &&
+
+ # Fetching only an object ID skips the "ls-refs" command, so the
+ # reply can only be sent with "fetch"
+ head=$(git -C server rev-parse HEAD) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch --no-tags \
+ --filter="blob:limit=5k" origin "$head" &&
+ test_grep ! "fetch> command=ls-refs" trace &&
+ test_grep "fetch> promisor-remote=lop" trace &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects server 1 "$oid"
+'
+
test_expect_success "clone with promisor.advertise set to 'true' but don't delete the client" '
git -C server config promisor.advertise true &&
diff --git a/t/t5711-promisor-remote-capability-http.sh b/t/t5711-promisor-remote-capability-http.sh
new file mode 100755
index 0000000000..c08cca3e51
--- /dev/null
+++ b/t/t5711-promisor-remote-capability-http.sh
@@ -0,0 +1,126 @@
+#!/bin/sh
+
+test_description='handling of promisor remote advertisement over HTTP'
+
+. ./test-lib.sh
+
+if ! test_have_prereq PERL_TEST_HELPERS
+then
+ skip_all='skipping promisor remote capabilities tests; Perl not available'
+ test_done
+fi
+
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+GIT_TEST_MULTI_PACK_INDEX=0
+GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL=0
+
+# The setup and helpers below are copied from t5710, except that the
+# server repository is in the HTTP document root.
+
+SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server.git"
+
+check_missing_objects () {
+ git -C "$1" rev-list --objects --all --missing=print > all.txt &&
+ perl -ne 'print if s/^[?]//' all.txt >missing.txt &&
+ test_line_count = "$2" missing.txt &&
+ test "$3" = "$(cat missing.txt)"
+}
+
+initialize_server () {
+ # Repack everything first
+ git -C "$SERVER" -c repack.writebitmaps=false repack -a -d &&
+
+ # Repack without the largest object and create a promisor pack on server
+ git -C "$SERVER" -c repack.writebitmaps=false repack -a -d \
+ --filter=blob:limit=5k --filter-to="$(pwd)/pack" &&
+ promisor_file=$(ls "$SERVER"/objects/pack/*.pack | sed "s/\.pack/.promisor/") &&
+ >"$promisor_file" &&
+
+ # Check objects missing on the server
+ check_missing_objects "$SERVER" 1 "$1"
+}
+
+copy_to_lop () {
+ oid_path="$(test_oid_to_path $1)" &&
+ path="$SERVER/objects/$oid_path" &&
+ path2="lop/objects/$oid_path" &&
+ mkdir -p $(dirname "$path2") &&
+ cp "$path" "$path2"
+}
+
+# On Windows, `pwd` returns a path like 'D:/foo/bar'. Prepend '/' to turn
+# it into '/D:/foo/bar', which is what git expects in file:// URLs on Windows.
+# On Unix, the path already starts with '/', so this is a no-op.
+pwd_path=$(pwd)
+case "$pwd_path" in
+[a-zA-Z]:*) pwd_path="/$pwd_path" ;;
+esac
+
+TRASH_DIRECTORY_URL="file://$pwd_path"
+
+test_expect_success 'setup: create "template" repository' '
+ git init template &&
+ test_commit -C template 1 &&
+ test_commit -C template 2 &&
+ test_commit -C template 3 &&
+ test-tool genrandom foo 10k >template/foo &&
+ git -C template add foo &&
+ git -C template commit -m foo
+'
+
+# A bare repo will act as a server repo with unpacked objects.
+test_expect_success 'setup: create bare "server" repository' '
+ git clone --bare --no-local template "$SERVER" &&
+ mv "$SERVER"/objects/pack/pack-* . &&
+ packfile=$(ls pack-*.pack) &&
+ git -C "$SERVER" unpack-objects --strict <"$packfile"
+'
+
+test_expect_success "setup for testing promisor remote advertisement" '
+ # Create another bare repo called "lop" (for Large Object Promisor)
+ git init --bare lop &&
+
+ # Copy the largest object from server to lop
+ obj="HEAD:foo" &&
+ oid="$(git -C "$SERVER" rev-parse $obj)" &&
+ copy_to_lop "$oid" &&
+
+ initialize_server "$oid" &&
+
+ # Configure lop as promisor remote for server
+ git -C "$SERVER" remote add lop "$TRASH_DIRECTORY_URL/lop" &&
+ git -C "$SERVER" config remote.lop.promisor true &&
+
+ git -C lop config uploadpack.allowFilter true &&
+ git -C lop config uploadpack.allowAnySHA1InWant true &&
+ git -C "$SERVER" config uploadpack.allowFilter true &&
+ git -C "$SERVER" config uploadpack.allowAnySHA1InWant true &&
+ git -C "$SERVER" config promisor.advertise true
+'
+
+# Over HTTP, each request is handled by a separate upload-pack process,
+# so the server knows that the client accepted "lop" only if the reply
+# is sent with the "fetch" request itself.
+test_expect_success "clone over HTTP with an accepted promisor remote" '
+ test_when_finished "rm -rf client trace" &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 clone \
+ -c remote.lop.promisor=true \
+ -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+ -c promisor.acceptfromserver=All \
+ --filter="blob:limit=5k" "$HTTPD_URL/smart/server.git" client &&
+
+ # Check that the reply is part of the fetch request
+ sed -n "/clone> command=fetch/,/clone> 0001/p" trace >request &&
+ test_grep "clone> promisor-remote=lop" request &&
+
+ # Check that the largest object was lazily fetched from lop
+ test_cmp template/foo client/foo &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects "$SERVER" 1 "$oid"
+'
+
+test_done
--
2.56.0
^ permalink raw reply related [flat|nested] 6+ messages in thread