[PATCH v4 0/3] packfile URIs: support concurrent downloads

[Ted Nyman <tnyman@openai.com>]

Packfile URI and dumb HTTP downloads stage packs at
objects/pack/pack-<hash>.pack.temp so an interrupted transfer can
resume. Opening that file in append mode forces every write to its
current end. Two Git processes fetching the same pack into one object
database can therefore append duplicate data and corrupt the pack.

The first patch separates the unrelated --index-pack-arg documentation
and error-message correction requested during review.

The second patch keeps the predictable staging name but removes append
mode. Each downloader seeks once to the current end, requests the
corresponding Range, and writes using its own descriptor offset. Since
the staging key must identify immutable pack contents, overlapping
responses write identical bytes at identical offsets. There is no need
for pwrite(2) or cross-process coordination, and resumption continues to
work for both packfile URI and ordinary dumb HTTP downloads.

A downloader can also find that the partial pack has completed and
request a range starting at EOF. Servers may respond with HTTP 416 in
that case. Treat the response as a completed download and let
index-pack validate the pack.

On MinGW, the non-append O_RDWR open grants FILE_SHARE_DELETE only for an
existing file. Create a missing staging file exclusively, close it, and
reopen it without O_CREAT so every retained descriptor permits another
downloader to unlink the path. Keep the open descriptor for index-pack;
it installs its own pack, so the shared staging file is only unlinked,
never renamed.

The third patch handles the related .keep race. When another process has
already created the keep file, index-pack reports "pack<TAB><hash>"
instead of "keep<TAB><hash>". Accept both successful forms and remove
only keep files created by the current process. Read only the prefix and
hash so any following fsck output remains available to fetch-pack.

The tests cover resumption, a completed partial returning 416,
overlapping 200 and 206 responses, unlinking the staging path while
index-pack holds its descriptor, and a pre-existing .keep file. The
unlink test does not require FIFOs, so it can exercise MinGW's sharing
behavior even though the concurrent-download tests are skipped there.

Changes since v3:

  * Match HTTP 416 in trace output from both older and current libcurl.
  * Add a timeout to the overlapping-download test server, notify FIFO
    waiters on server failures, and track the actual server process for
    cleanup.
  * Wait for the second downloader first so an early failure cannot
    leave the test server waiting for a request that will never arrive.
  * No production code changes.

These changes avoid false failures with older libcurl and prevent a
failed downloader from leaving the test server running indefinitely.

The v3 discussion is at:

  https://lore.kernel.org/git/cover.1784676106.git.tnyman@openai.com/

Ted Nyman (3):
  http-fetch: correct --index-pack-arg documentation
  http: avoid concurrent appends to partial packs
  fetch-pack: accept "pack" output for packfile URIs

 Documentation/git-http-fetch.adoc |  13 +-
 fetch-pack.c                      |  33 ++--
 http-fetch.c                      |   7 +-
 http-push.c                       |   3 +-
 http-walker.c                     |   3 +-
 http.c                            |  56 ++++---
 t/t5550-http-fetch-dumb.sh        | 250 ++++++++++++++++++++++++++++++
 t/t5702-protocol-v2.sh            |  31 ++++
 8 files changed, 350 insertions(+), 46 deletions(-)

Range-diff against v3:
1:  a6a40b8046 = 1:  a6a40b8046 http-fetch: correct --index-pack-arg documentation
2:  6c91054afc ! 2:  144c98cdfa http: avoid concurrent appends to partial packs
    @@ t/t5550-http-fetch-dumb.sh: test_expect_success 'http-fetch --packfile' '
     +	test_cmp expect second.out &&
     +	test_grep "Range: bytes=64-" first.trace &&
     +	test_grep "Range: bytes=[0-9]*-" second.trace &&
    -+	test_grep "HTTP/[0-9.]* 416" second.trace &&
    ++	test_grep "416 Requested Range Not Satisfiable" second.trace &&
     +	test_path_is_missing "$tmpfile" &&
     +	git -C packfileclient-concurrent cat-file -e "$HASH"
     +'
    @@ t/t5550-http-fetch-dumb.sh: test_expect_success 'http-fetch --packfile' '
     +	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: $!";
    @@ t/t5550-http-fetch-dumb.sh: test_expect_success 'http-fetch --packfile' '
     +	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
     +	{
    -+		(
    -+			if ! "$TRASH_DIRECTORY/slow-pack-server" "$pack" \
    -+				"$TRASH_DIRECTORY/server-ready" \
    -+				"$TRASH_DIRECTORY/first-ready"
    -+			then
    -+				echo failed >"$TRASH_DIRECTORY/server-ready" &&
    -+				echo failed >"$TRASH_DIRECTORY/first-ready" &&
    -+				exit 1
    -+			fi
    -+		) >server.log 2>&1 &
    ++		"$TRASH_DIRECTORY/slow-pack-server" "$pack" \
    ++			"$TRASH_DIRECTORY/server-ready" \
    ++			"$TRASH_DIRECTORY/first-ready" >server.log 2>&1 &
     +		server_pid=$!
     +	} &&
     +	test_when_finished "
    @@ t/t5550-http-fetch-dumb.sh: test_expect_success 'http-fetch --packfile' '
     +		kill $second_pid 2>/dev/null || :
     +		wait $second_pid 2>/dev/null || :
     +	" &&
    -+	wait "$server_pid" &&
    -+	wait "$first_pid" &&
     +	wait "$second_pid" &&
    ++	wait "$first_pid" &&
    ++	wait "$server_pid" &&
     +	test_grep "HTTP/[0-9.]* 200" overlap-first.trace &&
     +	test_grep "Range: bytes=[1-9][0-9]*-" overlap-second.trace &&
     +	test_grep "HTTP/[0-9.]* 206" overlap-second.trace &&
3:  1ee5d7e027 = 3:  d9063deb60 fetch-pack: accept "pack" output for packfile URIs

base-commit: 5d2e7709234afea1b6ddb25cd4f60d3d5fb3c200
-- 
2.55.0.openai.131.g83a728de1eb6