git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/2] http: use unique tempfiles for packfile URI downloads

From
Taylor Blau <ttaylorr@openai.com>
Date
Jul 14, 2026, 04:06 UTC
Message-ID
<alW1tAnMtOznxrhK@com-79390>
In-Reply-To
<alVn-QmK3K91_tkH@com-76773>
On Mon, Jul 13, 2026 at 03:34:33PM -0700, Ted Nyman wrote:
Show 57 quoted lines
> Since 8d5d2a34df (http-fetch: support fetching packfiles by URL,
> 2020-06-10), packfile URI downloads have been staged at
> objects/pack/pack-<hash>.pack.temp.
>
> The path is derived from the advertised pack hash. Two processes
> fetching the same pack into a shared object database therefore open the
> same file for append. Their writes can corrupt the temporary pack. If
> one process arrives after the other has completed the download, it may
> instead try to resume at EOF, which some HTTP servers reject with 416.
>
> Use the tempfile API to give direct packfile URI downloads unique
> temporary files. Keep the deterministic path for ordinary dumb HTTP
> pack requests, which use it to resume a partial download left by an
> earlier invocation.
>
> This means that a packfile URI download cannot be resumed by a later
> invocation. A retry starts with an empty temporary file instead.
>
> Add a test which pauses one process after downloading the pack and
> starts another process using the same object database.
>
> Signed-off-by: Ted Nyman <tnyman@openai.com>
> ---
>  Documentation/git-http-fetch.adoc |  5 +-
>  http.c                            | 77 +++++++++++++++++++++----------
>  http.h                            |  1 +
>  t/t5550-http-fetch-dumb.sh        | 72 ++++++++++++++++++++++++++++-
>  4 files changed, 126 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/git-http-fetch.adoc b/Documentation/git-http-fetch.adoc
> index 2200f073c4..533bf381c4 100644
> --- a/Documentation/git-http-fetch.adoc
> +++ b/Documentation/git-http-fetch.adoc
> @@ -48,9 +48,8 @@ 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
> -	--index-pack-args.
> +	The hash is arbitrary. The output of index-pack is printed to stdout.
> +	Requires --index-pack-args.
>
>  --index-pack-args=<args>::
>  	For internal use only. The command to run on the contents of the
> diff --git a/http.c b/http.c
> index b4e7b8d00b..5a46e7c65c 100644
> --- a/http.c
> +++ b/http.c
> @@ -2668,7 +2668,10 @@ int http_get_info_packs(const char *base_url, struct packfile_list *packs)
>
>  void release_http_pack_request(struct http_pack_request *preq)
>  {
> -	if (preq->packfile) {
> +	if (preq->tempfile) {
> +		delete_tempfile(&preq->tempfile);
> +		preq->packfile = NULL;

We should be able to drop the assignment to NULL on the second line, since `delete_tempfile()` takes a double pointer to the 'struct packfile' and NULL's it out for us.

(The other callers appear to avoid explicitly setting `preq->tempfile` to NULL.)

The rest of the patch looks good to me.
Show 21 quoted lines
> diff --git a/http.h b/http.h
> index 729c51904d..2c900779f5 100644
> --- a/http.h
> +++ b/http.h
> @@ -224,6 +224,7 @@ struct http_pack_request {
>
>  	FILE *packfile;
>  	struct strbuf tmpfile;
> +	struct tempfile *tempfile;
>  	struct active_request_slot *slot;
>  	struct curl_slist *headers;
>  };
> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
> index b0080bf204..314a74c433 100755
> --- a/t/t5550-http-fetch-dumb.sh
> +++ b/t/t5550-http-fetch-dumb.sh
> @@ -293,6 +293,74 @@ test_expect_success 'http-fetch --packfile' '
>  	git -C packfileclient cat-file -e "$HASH"
>  '
>
> +test_expect_success PIPE 'concurrent http-fetch --packfile' '
Phew ;-).

This is definitely tricky to test, but what you wrote here looks plausibly correct to me.

Thanks, Taylor

Previous: Ted NymanNext: Jeff King
Message 9 of 57 in “packfile URIs: support concurrent downloads”
  1. 0/2 packfile URIs: support concurrent downloadsTed Nyman, Jul 13, 2026
  2. 1/2 http: use unique tempfiles for packfile URI downloadsTed Nyman, Jul 13, 2026
  3. Junio C HamanoJul 14, 2026
  4. Ted NymanJul 14, 2026
  5. Taylor BlauJul 14, 2026
  6. Jeff KingJul 14, 2026
  7. Junio C HamanoJul 14, 2026
  8. Ted NymanJul 14, 2026
  9. Taylor BlauJul 14, 2026
  10. Jeff KingJul 14, 2026
  11. Jeff KingJul 14, 2026
  12. 2/2 fetch-pack: accept "pack" output for packfile URIsTed Nyman, Jul 13, 2026
  13. Jeff KingJul 14, 2026
  14. Jeff KingJul 14, 2026
  15. Ted NymanJul 14, 2026
  16. Jeff KingJul 14, 2026
  17. Taylor BlauJul 14, 2026
  18. 0/2 packfile URIs: support concurrent downloadsTed Nyman, Jul 20, 2026
  19. 1/2 http: avoid concurrent appends to partial packsTed Nyman, Jul 20, 2026
  20. Junio C HamanoJul 21, 2026
  21. 2/2 fetch-pack: accept "pack" output for packfile URIsTed Nyman, Jul 20, 2026
  22. 0/3 packfile URIs: support concurrent downloadsTed Nyman, Jul 21, 2026
  23. 1/3 http-fetch: correct --index-pack-arg documentationTed Nyman, Jul 21, 2026
  24. 2/3 http: avoid concurrent appends to partial packsTed Nyman, Jul 21, 2026
  25. 3/3 fetch-pack: accept "pack" output for packfile URIsTed Nyman, Jul 21, 2026
  26. Junio C HamanoJul 24, 2026
  27. Jeff KingJul 25, 2026
  28. Jeff KingJul 25, 2026
  29. Jeff KingJul 25, 2026
  30. Jeff KingJul 25, 2026
  31. Junio C HamanoJul 25, 2026
  32. 0/3 packfile URIs: support concurrent downloadsTed Nyman, Jul 24, 2026
  33. 1/3 http-fetch: correct --index-pack-arg documentationTed Nyman, Jul 24, 2026
  34. Taylor BlauJul 24, 2026
  35. 2/3 http: avoid concurrent appends to partial packsTed Nyman, Jul 24, 2026
  36. 3/3 fetch-pack: accept "pack" output for packfile URIsTed Nyman, Jul 24, 2026
  37. Taylor BlauJul 24, 2026
  38. 0/3 packfile URIs: support concurrent downloadsTed Nyman, Jul 26, 2026
  39. 1/3 http-fetch: correct --index-pack-arg documentationTed Nyman, Jul 26, 2026
  40. 2/3 http: avoid concurrent appends to partial packsTed Nyman, Jul 26, 2026
  41. Jeff KingJul 26, 2026
  42. Ted NymanJul 26, 2026
  43. Jeff KingJul 26, 2026
  44. 3/3 fetch-pack: accept "pack" output for packfile URIsTed Nyman, Jul 26, 2026
  45. Jeff KingJul 26, 2026
  46. 0/6 packfile URIs: support concurrent downloadsTed Nyman, Jul 27, 2026
  47. 1/6 http-fetch: correct --index-pack-arg documentationTed Nyman, Jul 27, 2026
  48. 2/6 http: avoid closing index-pack input twiceTed Nyman, Jul 27, 2026
  49. Jeff KingAug 1, 2026
  50. 3/6 http: accept HTTP 416 for complete partial packsTed Nyman, Jul 27, 2026
  51. Jeff KingAug 1, 2026
  52. 4/6 http: avoid concurrent appends to partial packsTed Nyman, Jul 27, 2026
  53. 5/6 http: permit unlinking partial packs on WindowsTed Nyman, Jul 27, 2026
  54. 6/6 fetch-pack: accept "pack" output for packfile URIsTed Nyman, Jul 27, 2026
  55. Junio C HamanoJul 29, 2026
  56. Jeff KingAug 1, 2026
  57. Junio C HamanoAug 8, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.