fetch-pack: accept "pack" output for packfile URIs

When index-pack finds an existing keep file it reports pack rather than
keep. Accept either result from http-fetch, and only register a keep
lockfile when this fetch created it.

Read the pack/keep prefix and hash without consuming any following fsck
output, validate the reported pack hash against the advertised hash, and
exercise a packfile URI fetch with a pre-existing keep file.

Signed-off-by: Ted Nyman <tnyman@openai.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/fetch-pack.c b/fetch-pack.c
index 29c41132ee04957d582c899146cc7afbfe626ee9..e9f24fbd6301b1a76a08b72b3540534c26593795 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1887,9 +1887,10 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
        }
 
        for (i = 0; i < packfile_uris.nr; i++) {
+               bool created_keep;
                int j;
                struct child_process cmd = CHILD_PROCESS_INIT;
-               char packname[GIT_MAX_HEXSZ + 1];
+               char packhash[GIT_MAX_HEXSZ + 1];
                const char *uri = packfile_uris.items[i].string +
                        the_hash_algo->hexsz + 1;
 
@@ -1907,16 +1908,17 @@ 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, packhash, 5) != 5 ||
+                   (memcmp(packhash, "keep\t", 5) &&
+                    memcmp(packhash, "pack\t", 5)))
+                       die("fetch-pack: expected pack or keep then TAB at start of http-fetch output");
+               created_keep = !memcmp(packhash, "keep\t", 5);
 
-               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, packhash,
+                                the_hash_algo->hexsz + 1) != the_hash_algo->hexsz + 1 ||
+                   packhash[the_hash_algo->hexsz] != '\n')
+                       die("fetch-pack: expected hash then LF in http-fetch output");
+               packhash[the_hash_algo->hexsz] = '\0';
 
                parse_gitmodules_oids(cmd.out, &fsck_options.gitmodules_found);
 
@@ -1925,16 +1927,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 9f6cf4142d5b83bd5b3ab9a3a08d3084fd8774d3..1861eb7d7ccdd1de029c21947c6c4144aa296741 100755 (executable)
--- 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 &&