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 <tnyman@openai.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/http.c b/http.c
index 4965e892489ebf1acc07ca8192c9665a40bea750..a53099536397d4e4878c0c40a4576fa5436d47e6 100644 (file)
--- 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 76cfcb8006d7d3a4c03f39a03e766b203a1d768c..1660baa0d1a69c468c9d795d820f7e86a8a6a5ad 100755 (executable)
--- 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 |