[PATCH 2/2] fetch-pack: accept "pack" output for packfile URIs
- From
Ted Nyman <tnyman@openai.com>
- Date
- Jul 13, 2026, 22:34 UTC
- Message-ID
- <alVoA5-fDDPwKPZZ@com-76773>
- In-Reply-To
- <cover.1783982021.git.tnyman@openai.com>
When "index-pack --keep" creates a .keep file, it reports "keep<TAB><hash>". If the file already exists, index-pack leaves it untouched and reports "pack<TAB><hash>" instead.
Since dd4b732df7 (upload-pack: send part of packfile response as uri, 2020-06-10), fetch-pack has accepted only the "keep" form for packs downloaded through packfile URIs. A concurrent fetch can install the same pack and create its .keep file before another process reaches index-pack. The latter process then fails even though index-pack completed successfully.
Accept both successful forms. Add a path to pack_lockfiles only for the "keep" form, so cleanup removes only a keep file created by the current process and preserves a pre-existing one.
Add a regression test which pre-creates a keep file and verifies that a fetch succeeds without changing it.
Signed-off-by: Ted Nyman <tnyman@openai.com> --- fetch-pack.c | 36 ++++++++++++++++++++---------------- t/t5702-protocol-v2.sh | 31 +++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 16 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c index 120e01f3cf..a16b80177a 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1887,9 +1887,12 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, } for (i = 0; i < packfile_uris.nr; i++) { + int created_keep = 0; int j; struct child_process cmd = CHILD_PROCESS_INIT; - char packname[GIT_MAX_HEXSZ + 1]; + char packname[GIT_MAX_HEXSZ + 6]; + const char *packhash; + const int packname_len = the_hash_algo->hexsz + 6; const char *uri = packfile_uris.items[i].string + the_hash_algo->hexsz + 1; @@ -1907,16 +1910,16 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, if (start_command(&cmd)) die("fetch-pack: unable to spawn http-fetch"); - if (read_in_full(cmd.out, packname, 5) < 0 || - memcmp(packname, "keep\t", 5)) - die("fetch-pack: expected keep then TAB at start of http-fetch output"); - - if (read_in_full(cmd.out, packname, - the_hash_algo->hexsz + 1) < 0 || - packname[the_hash_algo->hexsz] != '\n') - die("fetch-pack: expected hash then LF at end of http-fetch output"); - - packname[the_hash_algo->hexsz] = '\0'; + if (read_in_full(cmd.out, packname, packname_len) != packname_len || + packname[packname_len - 1] != '\n') + die("fetch-pack: expected pack or keep, TAB, hash, " + "then LF in http-fetch output"); + packname[packname_len - 1] = '\0'; + if (skip_prefix(packname, "keep\t", &packhash)) + created_keep = 1; + else if (!skip_prefix(packname, "pack\t", &packhash)) + die("fetch-pack: expected pack or keep, TAB, hash, " + "then LF in http-fetch output"); parse_gitmodules_oids(cmd.out, &fsck_options.gitmodules_found); @@ -1925,16 +1928,17 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, if (finish_command(&cmd)) die("fetch-pack: unable to finish http-fetch"); - if (memcmp(packfile_uris.items[i].string, packname, + if (memcmp(packfile_uris.items[i].string, packhash, the_hash_algo->hexsz)) die("fetch-pack: pack downloaded from %s does not match expected hash %.*s", uri, (int) the_hash_algo->hexsz, packfile_uris.items[i].string); - string_list_append_nodup(pack_lockfiles, - xstrfmt("%s/pack/pack-%s.keep", - repo_get_object_directory(the_repository), - packname)); + if (created_keep) + string_list_append_nodup(pack_lockfiles, + xstrfmt("%s/pack/pack-%s.keep", + repo_get_object_directory(the_repository), + packhash)); } string_list_clear(&packfile_uris, 0); strvec_clear(&index_pack_args); diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 9f6cf4142d..1861eb7d7c 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@ -1291,6 +1291,37 @@ test_expect_success 'packfile URIs with fetch instead of clone' ' fetch "$HTTPD_URL/smart/http_parent" ' +test_expect_success 'packfile URI preserves an existing keep file' ' + P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && + rm -rf "$P" http_child keep.expect && + + git init "$P" && + git -C "$P" config uploadpack.allowsidebandall true && + + echo my-blob >"$P/my-blob" && + git -C "$P" add my-blob && + git -C "$P" commit -m x && + configure_exclusion "$P" my-blob >h && + + git init http_child && + packhash=$(cat packh) && + keep="http_child/.git/objects/pack/pack-$packhash.keep" && + echo pre-existing >"$keep" && + cp "$keep" keep.expect && + + GIT_TEST_SIDEBAND_ALL=1 \ + git -C http_child -c protocol.version=2 \ + -c fetch.uriprotocols=http,https \ + fetch "$HTTPD_URL/smart/http_parent" && + + test_path_is_file \ + "http_child/.git/objects/pack/pack-$packhash.pack" && + test_path_is_file \ + "http_child/.git/objects/pack/pack-$packhash.idx" && + test_cmp keep.expect "$keep" && + git -C http_child cat-file -e "$(cat h)" +' + test_expect_success 'fetching with valid packfile URI but invalid hash fails' ' P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && rm -rf "$P" http_child log &&
-- 2.55.0