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

[WIP 1/7] http: use --stdin and --keep when downloading pack

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Feb 23, 2019, 23:38 UTC
Message-ID
<5ed3a4442e38165d0c45d53de5c8620b58a01015.1550963965.git.jonathantanmy@google.com>
In-Reply-To
<cover.1550963965.git.jonathantanmy@google.com>

When Git fetches a pack using dumb HTTP, it does at least 2 things differently from when it fetches using fetch-pack or receive-pack: (1) it reuses the server's name for the packfile (which incorporates a hash) for the packfile, and (2) it does not create a .keep file to avoid race conditions with repack.

A subsequent patch will allow downloading packs over HTTP(S) as part of a fetch. These downloads will not necessarily be from a Git repository, and thus may not have a hash as part of its name. Also, generating a .keep file will be necessary to avoid race conditions with repack (until the fetch has successfully written the new refs).

Thus, teach http to pass --stdin and --keep to index-pack, the former so that we have no reliance on the server's name for the packfile, and the latter so that we have the .keep file.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 http-push.c   |  7 ++++++-
 http-walker.c |  5 ++++-
 http.c        | 42 ++++++++++++++++++++----------------------
 http.h        |  2 +-
 4 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/http-push.c b/http-push.c
index b22c7caea0..409b266b0c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -586,11 +586,16 @@ static void finish_request(struct transfer_request *request)
 			fprintf(stderr, "Unable to get pack file %s\n%s",
 				request->url, curl_errorstr);
 		} else {
+			char *lockfile;
+
 			preq = (struct http_pack_request *)request->userData;
 
 			if (preq) {
-				if (finish_http_pack_request(preq) == 0)
+				if (finish_http_pack_request(preq,
+							     &lockfile) == 0) {
+					unlink(lockfile);
 					fail = 0;
+				}
 				release_http_pack_request(preq);
 			}
 		}
diff --git a/http-walker.c b/http-walker.c
index 8ae5d76c6a..804dc82304 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -425,6 +425,7 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo, unsigne
 	int ret;
 	struct slot_results results;
 	struct http_pack_request *preq;
+	char *lockfile;
 
 	if (fetch_indices(walker, repo))
 		return -1;
@@ -457,7 +458,9 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo, unsigne
 		goto abort;
 	}
 
-	ret = finish_http_pack_request(preq);
+	ret = finish_http_pack_request(preq, &lockfile);
+	if (!ret)
+		unlink(lockfile);
 	release_http_pack_request(preq);
 	if (ret)
 		return ret;
diff --git a/http.c b/http.c
index a32ad36ddf..5f8e602cd2 100644
--- a/http.c
+++ b/http.c
@@ -2200,13 +2200,13 @@ void release_http_pack_request(struct http_pack_request *preq)
 	free(preq);
 }
 
-int finish_http_pack_request(struct http_pack_request *preq)
+int finish_http_pack_request(struct http_pack_request *preq, char **lockfile)
 {
 	struct packed_git **lst;
 	struct packed_git *p = preq->target;
-	char *tmp_idx;
-	size_t len;
 	struct child_process ip = CHILD_PROCESS_INIT;
+	int tmpfile_fd;
+	int ret = 0;
 
 	close_pack_index(p);
 
@@ -2218,35 +2218,33 @@ int finish_http_pack_request(struct http_pack_request *preq)
 		lst = &((*lst)->next);
 	*lst = (*lst)->next;
 
-	if (!strip_suffix(preq->tmpfile.buf, ".pack.temp", &len))
-		BUG("pack tmpfile does not end in .pack.temp?");
-	tmp_idx = xstrfmt("%.*s.idx.temp", (int)len, preq->tmpfile.buf);
+	tmpfile_fd = xopen(preq->tmpfile.buf, O_RDONLY);
 
 	argv_array_push(&ip.args, "index-pack");
-	argv_array_pushl(&ip.args, "-o", tmp_idx, NULL);
-	argv_array_push(&ip.args, preq->tmpfile.buf);
+	argv_array_push(&ip.args, "--stdin");
+	argv_array_pushf(&ip.args, "--keep=git %"PRIuMAX, (uintmax_t)getpid());
 	ip.git_cmd = 1;
-	ip.no_stdin = 1;
-	ip.no_stdout = 1;
+	ip.in = tmpfile_fd;
+	ip.out = -1;
 
-	if (run_command(&ip)) {
-		unlink(preq->tmpfile.buf);
-		unlink(tmp_idx);
-		free(tmp_idx);
-		return -1;
+	if (start_command(&ip)) {
+		ret = -1;
+		goto cleanup;
 	}
 
-	unlink(sha1_pack_index_name(p->sha1));
+	*lockfile = index_pack_lockfile(ip.out);
+	close(ip.out);
 
-	if (finalize_object_file(preq->tmpfile.buf, sha1_pack_name(p->sha1))
-	 || finalize_object_file(tmp_idx, sha1_pack_index_name(p->sha1))) {
-		free(tmp_idx);
-		return -1;
+	if (finish_command(&ip)) {
+		ret = -1;
+		goto cleanup;
 	}
 
 	install_packed_git(the_repository, p);
-	free(tmp_idx);
-	return 0;
+cleanup:
+	close(tmpfile_fd);
+	unlink(preq->tmpfile.buf);
+	return ret;
 }
 
 struct http_pack_request *new_http_pack_request(
diff --git a/http.h b/http.h
index 4eb4e808e5..20d1c85d0b 100644
--- a/http.h
+++ b/http.h
@@ -212,7 +212,7 @@ struct http_pack_request {
 
 extern struct http_pack_request *new_http_pack_request(
 	struct packed_git *target, const char *base_url);
-extern int finish_http_pack_request(struct http_pack_request *preq);
+int finish_http_pack_request(struct http_pack_request *preq, char **lockfile);
 extern void release_http_pack_request(struct http_pack_request *preq);
 
 /* Helpers for fetching object */

-- 
2.19.0.271.gfe8321ec05.dirty
Previous: Jonathan TanNext: Jonathan Tan
Message 2 of 45 in “[WIP 0/7] CDN offloading of fetch response”
  1. Jonathan TanFeb 23, 2019
  2. 1/7 http: use --stdin and --keep when downloading packJonathan Tan, Feb 23, 2019
  3. 2/7 http: improve documentation of http_pack_requestJonathan Tan, Feb 23, 2019
  4. 3/7 http-fetch: support fetching packfiles by URLJonathan Tan, Feb 23, 2019
  5. 4/7 Documentation: order protocol v2 sectionsJonathan Tan, Feb 23, 2019
  6. 5/7 Documentation: add Packfile URIs design docJonathan Tan, Feb 23, 2019
  7. 6/7 upload-pack: refactor reading of pack-objects outJonathan Tan, Feb 23, 2019
  8. 7/7 upload-pack: send part of packfile response as uriJonathan Tan, Feb 23, 2019
  9. Junio C HamanoFeb 24, 2019
  10. Christian CouderFeb 25, 2019
  11. Jonathan NiederFeb 26, 2019
  12. Christian CouderFeb 26, 2019
  13. Josh SteadmonMar 1, 2019
  14. Jonathan TanMar 1, 2019
  15. Christian CouderFeb 25, 2019
  16. Jonathan NiederFeb 25, 2019
  17. Christian CouderFeb 26, 2019
  18. Ævar Arnfjörð BjarmasonFeb 26, 2019
  19. Christian CouderMar 4, 2019
  20. Jonathan NiederFeb 28, 2019
  21. Christian CouderMar 4, 2019
  22. 0/8 CDN offloading of fetch responseJonathan Tan, Mar 8, 2019
  23. 1/8 http: use --stdin when getting dumb HTTP packJonathan Tan, Mar 8, 2019
  24. 2/8 http: improve documentation of http_pack_requestJonathan Tan, Mar 8, 2019
  25. 3/8 http-fetch: support fetching packfiles by URLJonathan Tan, Mar 8, 2019
  26. 4/8 Documentation: order protocol v2 sectionsJonathan Tan, Mar 8, 2019
  27. 5/8 Documentation: add Packfile URIs design docJonathan Tan, Mar 8, 2019
  28. Jeff KingApr 23, 2019
  29. Jonathan TanApr 23, 2019
  30. Ævar Arnfjörð BjarmasonApr 23, 2019
  31. Jonathan NiederApr 23, 2019
  32. Ævar Arnfjörð BjarmasonApr 23, 2019
  33. Jonathan NiederApr 23, 2019
  34. Jonathan NiederApr 23, 2019
  35. Ævar Arnfjörð BjarmasonApr 23, 2019
  36. Jonathan NiederApr 23, 2019
  37. Ævar Arnfjörð BjarmasonApr 24, 2019
  38. Junio C HamanoApr 24, 2019
  39. 6/8 upload-pack: refactor reading of pack-objects outJonathan Tan, Mar 8, 2019
  40. 7/8 fetch-pack: support more than one pack lockfileJonathan Tan, Mar 8, 2019
  41. 8/8 upload-pack: send part of packfile response as uriJonathan Tan, Mar 8, 2019
  42. Josh SteadmonMar 19, 2019
  43. Jeff KingApr 23, 2019
  44. Jonathan TanApr 23, 2019
  45. Ævar Arnfjörð BjarmasonApr 24, 2019

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.