[PATCH v6 5/6] http: permit unlinking partial packs on Windows

Ted Nyman <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <87a20ac80fcaaaa3f69a24a650f5c135b24956ff.1785111375.git.tnyman@openai.com>
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 <[email protected]>
---
 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 ad07ef3549..a0d399b274 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 86b9d87ef5..b5758f1c9c 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 |
-- 
2.55.0.openai.131.g83a728de1eb6
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.