From c71ed75a8b1d59793e033407cf38b03ddc1d0179 Mon Sep 17 00:00:00 2001 From: Ted Nyman Date: Sun, 26 Jul 2026 17:28:38 -0700 Subject: [PATCH 1/6] http-fetch: correct --index-pack-arg documentation The --packfile mode accepts one --index-pack-arg= option per argument passed to index-pack, but its documentation and option dependency errors still refer to the plural --index-pack-args form. Correct the spelling and describe the repeatable per-argument form. Signed-off-by: Ted Nyman Signed-off-by: Junio C Hamano --- Documentation/git-http-fetch.adoc | 9 +++++---- http-fetch.c | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Documentation/git-http-fetch.adoc b/Documentation/git-http-fetch.adoc index 2200f073c4..12036e65e9 100644 --- a/Documentation/git-http-fetch.adoc +++ b/Documentation/git-http-fetch.adoc @@ -50,11 +50,12 @@ commit-id:: URL and uses index-pack to generate corresponding .idx and .keep files. The hash is used to determine the name of the temporary file and is arbitrary. The output of index-pack is printed to stdout. Requires - --index-pack-args. + one or more --index-pack-arg options. ---index-pack-args=:: - For internal use only. The command to run on the contents of the - downloaded pack. Arguments are URL-encoded separated by spaces. +--index-pack-arg=:: + For internal use only. The first instance specifies the command run on + the contents of the downloaded pack. Subsequent instances specify its + arguments. --recover:: Verify that everything reachable from target is fetched. Used after diff --git a/http-fetch.c b/http-fetch.c index f9b6ecb061..601a77c3c1 100644 --- a/http-fetch.c +++ b/http-fetch.c @@ -155,7 +155,7 @@ int cmd_main(int argc, const char **argv) if (packfile) { if (!index_pack_args.nr) - die(_("the option '%s' requires '%s'"), "--packfile", "--index-pack-args"); + die(_("the option '%s' requires '%s'"), "--packfile", "--index-pack-arg"); fetch_single_packfile(&packfile_hash, argv[arg], index_pack_args.v); @@ -164,7 +164,7 @@ int cmd_main(int argc, const char **argv) } if (index_pack_args.nr) - die(_("the option '%s' requires '%s'"), "--index-pack-args", "--packfile"); + die(_("the option '%s' requires '%s'"), "--index-pack-arg", "--packfile"); if (commits_on_stdin) { commits = walker_targets_stdin(&commit_id, &write_ref); From f0d866a2eafcd012fb8fda8edd6b32da68859d7b Mon Sep 17 00:00:00 2001 From: Ted Nyman Date: Sun, 26 Jul 2026 17:28:39 -0700 Subject: [PATCH 2/6] http: avoid closing index-pack input twice finish_http_pack_request() passes its staging-file descriptor to index-pack through child_process.in. start_command() takes ownership of a supplied descriptor and closes it, even when starting the child fails. Do not close the descriptor again after run_command() returns. Signed-off-by: Ted Nyman Signed-off-by: Junio C Hamano --- http.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/http.c b/http.c index b4e7b8d00b..930e0d227f 100644 --- a/http.c +++ b/http.c @@ -2704,13 +2704,8 @@ int finish_http_pack_request(struct http_pack_request *preq) else ip.no_stdout = 1; - if (run_command(&ip)) { + if (run_command(&ip)) ret = -1; - goto cleanup; - } - -cleanup: - close(tmpfile_fd); unlink(preq->tmpfile.buf); return ret; } From 85f4f04f820c9aa6cfdfb4594ceb9b41515a8245 Mon Sep 17 00:00:00 2001 From: Ted Nyman Date: Sun, 26 Jul 2026 17:28:40 -0700 Subject: [PATCH 3/6] http: accept HTTP 416 for complete partial packs A resumed pack request may already have all bytes of the remote pack. A server can respond to the resulting Range request with HTTP 416 instead of returning an empty response. Accept that response in each pack-download caller and let index-pack validate the completed staging file. This can happen without concurrent downloads when a previous attempt completed the transfer but failed before indexing it. Add a regression test that seeds a complete partial pack and checks that http-fetch indexes it after the server returns HTTP 416. Signed-off-by: Ted Nyman Signed-off-by: Junio C Hamano --- http-fetch.c | 3 ++- http-push.c | 3 ++- http-walker.c | 3 ++- t/t5550-http-fetch-dumb.sh | 19 +++++++++++++++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/http-fetch.c b/http-fetch.c index 601a77c3c1..05f68f306a 100644 --- a/http-fetch.c +++ b/http-fetch.c @@ -70,7 +70,8 @@ static void fetch_single_packfile(struct object_id *packfile_hash, if (start_active_slot(preq->slot)) { run_active_slot(preq->slot); - if (results.curl_result != CURLE_OK) { + if (results.curl_result != CURLE_OK && + results.http_code != 416) { struct url_info url; char *nurl = url_normalize(preq->url, &url); if (!nurl || !git_env_bool("GIT_TRACE_REDACT", 1)) { diff --git a/http-push.c b/http-push.c index 3c23cbba27..03dc8102a1 100644 --- a/http-push.c +++ b/http-push.c @@ -595,7 +595,8 @@ static void finish_request(struct transfer_request *request) } else if (request->state == RUN_FETCH_PACKED) { int fail = 1; - if (request->curl_result != CURLE_OK) { + if (request->curl_result != CURLE_OK && + request->http_code != 416) { fprintf(stderr, "Unable to get pack file %s\n%s", request->url, curl_errorstr); } else { diff --git a/http-walker.c b/http-walker.c index b58a3b2a92..abafca84d6 100644 --- a/http-walker.c +++ b/http-walker.c @@ -451,7 +451,8 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo, if (start_active_slot(preq->slot)) { run_active_slot(preq->slot); - if (results.curl_result != CURLE_OK) { + if (results.curl_result != CURLE_OK && + results.http_code != 416) { error("Unable to get pack file %s\n%s", preq->url, curl_errorstr); goto abort; diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index b0080bf204..621f2fbf73 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -293,6 +293,25 @@ test_expect_success 'http-fetch --packfile' ' git -C packfileclient cat-file -e "$HASH" ' +test_expect_success 'http-fetch --packfile accepts an already complete partial' ' + git init packfileclient-complete && + p=$(cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git && + ls objects/pack/pack-*.pack) && + packhash=$(basename "$p" .pack) && + packhash=${packhash#pack-} && + tmpfile="packfileclient-complete/.git/objects/pack/pack-$packhash.pack.temp" && + cp "$HTTPD_DOCUMENT_ROOT_PATH/repo_pack.git/$p" "$tmpfile" && + chmod u+w "$tmpfile" && + GIT_TRACE_CURL="$TRASH_DIRECTORY/complete.trace" \ + git -C packfileclient-complete http-fetch --packfile="$packhash" \ + --index-pack-arg=index-pack \ + --index-pack-arg=--stdin --index-pack-arg=--keep \ + "$HTTPD_URL/dumb/repo_pack.git/$p" >out && + test_grep "416 Requested Range Not Satisfiable" complete.trace && + test_path_is_missing "$tmpfile" && + git -C packfileclient-complete cat-file -e "$HASH" +' + test_expect_success 'fetch notices corrupt pack' ' cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git && (cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git && From 5e855d9b426dd01552ecbfb47062b90fe53b3df5 Mon Sep 17 00:00:00 2001 From: Ted Nyman Date: Sun, 26 Jul 2026 17:28:41 -0700 Subject: [PATCH 4/6] http: avoid concurrent appends to partial packs Pack requests stage downloads in a predictable partial-pack file so an interrupted transfer can be resumed. Both packfile URI and ordinary dumb HTTP requests use this staging path. Opening it in append mode forces each write to the current end of the file, so concurrent responses can append duplicate data and corrupt the pack. Open the partial pack read-write without O_APPEND and seek once to its current end. Each downloader then retains the offset matching the Range it requested. Because the staging key must uniquely identify immutable pack contents, overlapping responses write the same bytes at the same offsets instead of extending the file with duplicate data. Duplicate the staging descriptor for index-pack instead of reopening the path after closing the stream. Another downloader may unlink the staging path before indexing begins, but index-pack can still read the retained descriptor. Exercise resumed transfers and overlapping 200 and 206 responses, and clarify the staging-key documentation. Signed-off-by: Ted Nyman Signed-off-by: Junio C Hamano --- Documentation/git-http-fetch.adoc | 5 +- http.c | 34 ++++--- t/t5550-http-fetch-dumb.sh | 164 ++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 16 deletions(-) diff --git a/Documentation/git-http-fetch.adoc b/Documentation/git-http-fetch.adoc index 12036e65e9..45e0d3d07c 100644 --- a/Documentation/git-http-fetch.adoc +++ b/Documentation/git-http-fetch.adoc @@ -48,8 +48,9 @@ commit-id:: line (which is not expected in this case), 'git http-fetch' fetches the packfile directly at the given URL and uses index-pack to generate corresponding .idx and .keep files. - The hash is used to determine the name of the temporary file and is - arbitrary. The output of index-pack is printed to stdout. Requires + The hash is used to determine the name of the temporary file. It need + not be the pack hash, but it must uniquely identify the pack contents + for resumption. The output of index-pack is printed to stdout. Requires one or more --index-pack-arg options. --index-pack-arg=:: diff --git a/http.c b/http.c index 930e0d227f..4965e89248 100644 --- a/http.c +++ b/http.c @@ -2688,10 +2688,13 @@ int finish_http_pack_request(struct http_pack_request *preq) int tmpfile_fd; int ret = 0; + /* Another downloader may unlink the staging path while we index it. */ + tmpfile_fd = xdup(fileno(preq->packfile)); fclose(preq->packfile); preq->packfile = NULL; - - tmpfile_fd = xopen(preq->tmpfile.buf, O_RDONLY); + if (lseek(tmpfile_fd, 0, SEEK_SET) < 0) + die_errno("unable to seek local file %s for pack", + preq->tmpfile.buf); ip.git_cmd = 1; ip.in = tmpfile_fd; @@ -2733,22 +2736,30 @@ struct http_pack_request *new_http_pack_request( struct http_pack_request *new_direct_http_pack_request( const unsigned char *packed_git_hash, char *url) { - off_t prev_posn = 0; + off_t prev_posn; struct http_pack_request *preq; + int fd; CALLOC_ARRAY(preq, 1); strbuf_init(&preq->tmpfile, 0); - preq->url = url; odb_pack_name(the_repository, &preq->tmpfile, packed_git_hash, "pack"); strbuf_addstr(&preq->tmpfile, ".temp"); - preq->packfile = fopen(preq->tmpfile.buf, "a"); - if (!preq->packfile) { - error("Unable to open local file %s for pack", - preq->tmpfile.buf); + fd = open(preq->tmpfile.buf, O_RDWR | O_CREAT, 0666); + if (fd < 0) { + error_errno("unable to open local file %s for pack", + preq->tmpfile.buf); goto abort; } + prev_posn = lseek(fd, 0, SEEK_END); + if (prev_posn < 0) { + error_errno("unable to seek local file %s for pack", + preq->tmpfile.buf); + close(fd); + goto abort; + } + preq->packfile = xfdopen(fd, "w"); preq->slot = get_active_slot(); preq->headers = object_request_headers(); @@ -2757,12 +2768,7 @@ struct http_pack_request *new_direct_http_pack_request( curl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url); curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, preq->headers); - /* - * If there is data present from a previous transfer attempt, - * resume where it left off - */ - prev_posn = ftello(preq->packfile); - if (prev_posn>0) { + if (prev_posn > 0) { if (http_is_verbose) fprintf(stderr, "Resuming fetch of pack %s at byte %"PRIuMAX"\n", diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 621f2fbf73..76cfcb8006 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -312,6 +312,170 @@ test_expect_success 'http-fetch --packfile accepts an already complete partial' git -C packfileclient-complete cat-file -e "$HASH" ' +test_expect_success 'http-fetch --packfile resumes a partial download' ' + git init packfileclient-resume && + p=$(cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git && + ls objects/pack/pack-*.pack) && + tmpfile="packfileclient-resume/.git/objects/pack/pack-$ARBITRARY.pack.temp" && + test_copy_bytes 64 <"$HTTPD_DOCUMENT_ROOT_PATH/repo_pack.git/$p" >"$tmpfile" && + GIT_TRACE_CURL="$TRASH_DIRECTORY/resume.trace" \ + git -C packfileclient-resume http-fetch --packfile="$ARBITRARY" \ + --index-pack-arg=index-pack --index-pack-arg=--stdin \ + --index-pack-arg=--keep \ + "$HTTPD_URL/dumb/repo_pack.git/$p" >out && + test_grep "Range: bytes=64-" resume.trace && + test_path_is_missing "$tmpfile" && + git -C packfileclient-resume cat-file -e "$HASH" +' + +test_expect_success PERL,PIPE 'concurrent http-fetch --packfile cannot corrupt an overlapping download' ' + git init packfileclient-overlap && + blob=$(test-tool genrandom pack-overlap 2m | + git -C "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git \ + hash-object -w --stdin) && + packhash=$(printf "%s\n" "$blob" | + git -C "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git \ + pack-objects "$TRASH_DIRECTORY/overlap-pack") && + pack="$TRASH_DIRECTORY/overlap-pack-$packhash.pack" && + tmpfile="packfileclient-overlap/.git/objects/pack/pack-$packhash.pack.temp" && + mkfifo server-ready first-ready && + exec 7<>server-ready && + exec 8<>first-ready && + write_script slow-pack-server "$PERL_PATH" <<-\EOF && + use strict; + use warnings; + use IO::Socket::INET; + + my ($packfile, $server_ready, $first_ready) = @ARGV; + my $completed = 0; + END { + if (!$completed) { + signal_ready($server_ready, "failed"); + signal_ready($first_ready, "failed"); + } + } + + $SIG{ALRM} = sub { die "timed out serving concurrent pack requests\n" }; + alarm 60; + + open(my $in, "<:raw", $packfile) or die "open $packfile: $!"; + my $pack = do { local $/; <$in> }; + close($in) or die "close $packfile: $!"; + my $server = IO::Socket::INET->new(LocalAddr => "127.0.0.1", + LocalPort => 0, Proto => "tcp", Listen => 2, ReuseAddr => 1) + or die "listen: $!"; + + sub signal_ready { + my ($file, $value) = @_; + open(my $out, ">", $file) or die "open $file: $!"; + print $out "$value\n" or die "write $file: $!"; + close($out) or die "close $file: $!"; + } + + sub write_all { + my ($out, $data) = @_; + my $offset = 0; + while ($offset < length($data)) { + my $written = syswrite($out, $data, + length($data) - $offset, $offset); + defined($written) && $written or die "write response: $!"; + $offset += $written; + } + } + + sub start_response { + my $out = $server->accept() or die "accept: $!"; + <$out> or die "read request: $!"; + my $start = 0; + while (<$out>) { + last if /^\r?\n$/; + $start = $1 if /^Range: bytes=(\d+)-/i; + } + $start < length($pack) or die "invalid range $start"; + my $length = length($pack) - $start; + my $middle = int($length / 2); + my $status = $start ? "206 Partial Content" : "200 OK"; + my $headers = "HTTP/1.1 $status\r\n" . + "Content-Length: $length\r\n" . + ($start ? "Content-Range: bytes $start-" . + (length($pack) - 1) . "/" . length($pack) . "\r\n" : "") . + "Connection: close\r\n\r\n"; + write_all($out, $headers); + write_all($out, substr($pack, $start, $middle)); + return ($out, $start + $middle); + } + + signal_ready($server_ready, $server->sockport()); + my ($first, $first_pos) = start_response(); + signal_ready($first_ready, "ready"); + my ($second, $second_pos) = start_response(); + write_all($first, substr($pack, $first_pos)); + write_all($second, substr($pack, $second_pos)); + close($first) or die "close first response: $!"; + close($second) or die "close second response: $!"; + $completed = 1; + alarm 0; + EOF + { + "$TRASH_DIRECTORY/slow-pack-server" "$pack" \ + "$TRASH_DIRECTORY/server-ready" \ + "$TRASH_DIRECTORY/first-ready" >server.log 2>&1 & + server_pid=$! + } && + test_when_finished " + kill $server_pid 2>/dev/null || : + wait $server_pid 2>/dev/null || : + exec 7>&- + exec 8>&- + rm -f server-ready first-ready slow-pack-server + " && + read port <&7 && + url="http://127.0.0.1:$port/pack" && + { + ( + if ! GIT_TRACE_CURL="$TRASH_DIRECTORY/overlap-first.trace" \ + GIT_TRACE_CURL_NO_DATA=1 \ + git -C packfileclient-overlap http-fetch --packfile="$packhash" \ + --index-pack-arg=index-pack \ + --index-pack-arg=--stdin --index-pack-arg=--keep \ + "$url" >first.out + then + echo failed >"$TRASH_DIRECTORY/first-ready" && + exit 1 + fi + ) & + first_pid=$! + } && + test_when_finished " + kill $first_pid 2>/dev/null || : + wait $first_pid 2>/dev/null || : + " && + read ready <&8 && + test "$ready" = ready && + test_path_is_file "$tmpfile" && + { + GIT_TRACE_CURL="$TRASH_DIRECTORY/overlap-second.trace" \ + GIT_TRACE_CURL_NO_DATA=1 \ + git -C packfileclient-overlap http-fetch --packfile="$packhash" \ + --index-pack-arg=index-pack \ + --index-pack-arg=--stdin --index-pack-arg=--keep \ + "$url" >second.out & + second_pid=$! + } && + test_when_finished " + kill $second_pid 2>/dev/null || : + wait $second_pid 2>/dev/null || : + " && + wait "$second_pid" && + wait "$first_pid" && + wait "$server_pid" && + printf "keep\t%s\npack\t%s\n" "$packhash" "$packhash" | sort >expect && + sort first.out second.out >actual && + test_cmp expect actual && + test_path_is_missing "$tmpfile" && + git -C packfileclient-overlap cat-file -e "$blob" +' + test_expect_success 'fetch notices corrupt pack' ' cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git && (cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git && From e92a518a741ead85634c43f871e7e095bcf66f1f Mon Sep 17 00:00:00 2001 From: Ted Nyman Date: Sun, 26 Jul 2026 17:28:42 -0700 Subject: [PATCH 5/6] http: permit unlinking partial packs on Windows On Windows, an open file must permit FILE_SHARE_DELETE before another process can unlink it. MinGW's non-append O_RDWR open enables that sharing mode only for an existing file; adding O_CREAT falls back to _wopen(), which cannot set it. First try opening the partial pack without O_CREAT. If it does not exist, create it exclusively, close that descriptor, and retry through the existing-file path. A racing creator retries after EEXIST. This ensures that every retained descriptor permits another downloader to unlink the staging path. Add an unlink-while-indexing test that does not require FIFOs and can therefore run on MinGW. Signed-off-by: Ted Nyman Signed-off-by: Junio C Hamano --- http.c | 17 ++++++++++++++++- t/t5550-http-fetch-dumb.sh | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/http.c b/http.c index 4965e89248..a530995363 100644 --- a/http.c +++ b/http.c @@ -2746,7 +2746,22 @@ struct http_pack_request *new_direct_http_pack_request( odb_pack_name(the_repository, &preq->tmpfile, packed_git_hash, "pack"); strbuf_addstr(&preq->tmpfile, ".temp"); - fd = open(preq->tmpfile.buf, O_RDWR | O_CREAT, 0666); + /* + * MinGW's non-append O_RDWR open grants FILE_SHARE_DELETE only for an + * existing file; reopen a newly created file so others may unlink it. + */ + for (;;) { + fd = open(preq->tmpfile.buf, O_RDWR); + if (fd >= 0 || errno != ENOENT) + break; + fd = open(preq->tmpfile.buf, O_RDWR | O_CREAT | O_EXCL, 0666); + if (fd >= 0) { + close(fd); + continue; + } + if (errno != EEXIST) + break; + } if (fd < 0) { error_errno("unable to open local file %s for pack", preq->tmpfile.buf); diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 76cfcb8006..1660baa0d1 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -328,6 +328,27 @@ test_expect_success 'http-fetch --packfile resumes a partial download' ' git -C packfileclient-resume cat-file -e "$HASH" ' +test_expect_success 'http-fetch --packfile permits unlink while indexing' ' + git init packfileclient-unlink && + p=$(cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git && + ls objects/pack/pack-*.pack) && + tmpfile="packfileclient-unlink/.git/objects/pack/pack-$ARBITRARY.pack.temp" && + write_script git-unlink-index-pack <<-\EOF && + test -f "$GIT_TEST_PACK_TEMP" || exit 1 + rm "$GIT_TEST_PACK_TEMP" || exit 1 + exec git index-pack "$@" + EOF + test_when_finished "rm -f git-unlink-index-pack" && + PATH="$TRASH_DIRECTORY:$PATH" \ + GIT_TEST_PACK_TEMP="$TRASH_DIRECTORY/$tmpfile" \ + git -C packfileclient-unlink http-fetch --packfile="$ARBITRARY" \ + --index-pack-arg=unlink-index-pack \ + --index-pack-arg=--stdin --index-pack-arg=--keep \ + "$HTTPD_URL/dumb/repo_pack.git/$p" >out && + test_path_is_missing "$tmpfile" && + git -C packfileclient-unlink cat-file -e "$HASH" +' + test_expect_success PERL,PIPE 'concurrent http-fetch --packfile cannot corrupt an overlapping download' ' git init packfileclient-overlap && blob=$(test-tool genrandom pack-overlap 2m | From c4244bdfe6d28ed552e10f1e37387978549b36ed Mon Sep 17 00:00:00 2001 From: Ted Nyman Date: Sun, 26 Jul 2026 17:28:43 -0700 Subject: [PATCH 6/6] 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 Signed-off-by: Junio C Hamano --- fetch-pack.c | 33 ++++++++++++++++++--------------- t/t5702-protocol-v2.sh | 31 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index 29c41132ee..e9f24fbd63 100644 --- 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 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 &&