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

[PATCH v2 3/5] receive-pack: quarantine objects until pre-receive accepts

From
Jeff King <peff@peff.net>
Date
Oct 3, 2016, 20:49 UTC
Message-ID
<20161003204914.2jn2ik252scsusry@sigill.intra.peff.net>
In-Reply-To
<20161003204851.klwspo6agykx6s3q@sigill.intra.peff.net>

When a client pushes objects to us, index-pack checks the objects themselves and then installs them into place. If we then reject the push due to a pre-receive hook, we cannot just delete the packfile; other processes may be depending on it. We have to do a normal reachability check at this point via `git gc`.

But such objects may hang around for weeks due to the gc.pruneExpire grace period. And worse, during that time they may be exploded from the pack into inefficient loose objects.

Instead, this patch teaches receive-pack to put the new objects into a "quarantine" temporary directory. We make these objects available to the connectivity check and to the pre-receive hook, and then install them into place only if it is successful (and otherwise remove them as tempfiles).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/receive-pack.c     | 41 ++++++++++++++++++++++++++++++++++++++++-
 t/t5547-push-quarantine.sh | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100755 t/t5547-push-quarantine.sh
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 896b16f..267d320 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -20,6 +20,7 @@
 #include "gpg-interface.h"
 #include "sigchain.h"
 #include "fsck.h"
+#include "tmp-objdir.h"
 
 static const char * const receive_pack_usage[] = {
 	N_("git receive-pack <git-dir>"),
@@ -86,6 +87,8 @@ static enum {
 } use_keepalive;
 static int keepalive_in_sec = 5;
 
+static struct tmp_objdir *tmp_objdir;
+
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
 	if (value) {
@@ -663,6 +666,9 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed,
 	} else
 		argv_array_pushf(&proc.env_array, "GIT_PUSH_OPTION_COUNT");
 
+	if (tmp_objdir)
+		argv_array_pushv(&proc.env_array, tmp_objdir_env(tmp_objdir));
+
 	if (use_sideband) {
 		memset(&muxer, 0, sizeof(muxer));
 		muxer.proc = copy_to_sideband;
@@ -762,6 +768,7 @@ static int run_update_hook(struct command *cmd)
 	proc.stdout_to_stderr = 1;
 	proc.err = use_sideband ? -1 : 0;
 	proc.argv = argv;
+	proc.env = tmp_objdir_env(tmp_objdir);
 
 	code = start_command(&proc);
 	if (code)
@@ -833,6 +840,7 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 		    !delayed_reachability_test(si, i))
 			sha1_array_append(&extra, si->shallow->sha1[i]);
 
+	opt.env = tmp_objdir_env(tmp_objdir);
 	setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
 	if (check_connected(command_singleton_iterator, cmd, &opt)) {
 		rollback_lock_file(&shallow_lock);
@@ -1240,12 +1248,17 @@ static void set_connectivity_errors(struct command *commands,
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		struct command *singleton = cmd;
+		struct check_connected_options opt = CHECK_CONNECTED_INIT;
+
 		if (shallow_update && si->shallow_ref[cmd->index])
 			/* to be checked in update_shallow_ref() */
 			continue;
+
+		opt.env = tmp_objdir_env(tmp_objdir);
 		if (!check_connected(command_singleton_iterator, &singleton,
-				     NULL))
+				     &opt))
 			continue;
+
 		cmd->error_string = "missing necessary objects";
 	}
 }
@@ -1428,6 +1441,7 @@ static void execute_commands(struct command *commands,
 	data.si = si;
 	opt.err_fd = err_fd;
 	opt.progress = err_fd && !quiet;
+	opt.env = tmp_objdir_env(tmp_objdir);
 	if (check_connected(iterate_receive_command_list, &data, &opt))
 		set_connectivity_errors(commands, si);
 
@@ -1444,6 +1458,19 @@ static void execute_commands(struct command *commands,
 		return;
 	}
 
+	/*
+	 * Now we'll start writing out refs, which means the objects need
+	 * to be in their final positions so that other processes can see them.
+	 */
+	if (tmp_objdir_migrate(tmp_objdir) < 0) {
+		for (cmd = commands; cmd; cmd = cmd->next) {
+			if (!cmd->error_string)
+				cmd->error_string = "unable to migrate objects to permanent storage";
+		}
+		return;
+	}
+	tmp_objdir = NULL;
+
 	check_aliased_updates(commands);
 
 	free(head_name_to_free);
@@ -1639,6 +1666,18 @@ static const char *unpack(int err_fd, struct shallow_info *si)
 		argv_array_push(&child.args, alt_shallow_file);
 	}
 
+	tmp_objdir = tmp_objdir_create();
+	if (!tmp_objdir)
+		return "unable to create temporary object directory";
+	child.env = tmp_objdir_env(tmp_objdir);
+
+	/*
+	 * Normally we just pass the tmp_objdir environment to the child
+	 * processes that do the heavy lifting, but we may need to see these
+	 * objects ourselves to set up shallow information.
+	 */
+	tmp_objdir_add_as_alternate(tmp_objdir);
+
 	if (ntohl(hdr.hdr_entries) < unpack_limit) {
 		argv_array_pushl(&child.args, "unpack-objects", hdr_arg, NULL);
 		if (quiet)
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
new file mode 100755
index 0000000..1e5d32d
--- /dev/null
+++ b/t/t5547-push-quarantine.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+test_description='check quarantine of objects during push'
+. ./test-lib.sh
+
+test_expect_success 'create picky dest repo' '
+	git init --bare dest.git &&
+	write_script dest.git/hooks/pre-receive <<-\EOF
+	while read old new ref; do
+		test "$(git log -1 --format=%s $new)" = reject && exit 1
+	done
+	exit 0
+	EOF
+'
+
+test_expect_success 'accepted objects work' '
+	test_commit ok &&
+	git push dest.git HEAD &&
+	commit=$(git rev-parse HEAD) &&
+	git --git-dir=dest.git cat-file commit $commit
+'
+
+test_expect_success 'rejected objects are not installed' '
+	test_commit reject &&
+	commit=$(git rev-parse HEAD) &&
+	test_must_fail git push dest.git reject &&
+	test_must_fail git --git-dir=dest.git cat-file commit $commit
+'
+
+test_expect_success 'rejected objects are removed' '
+	echo "incoming-*" >expect &&
+	(cd dest.git/objects && echo incoming-*) >actual &&
+	test_cmp expect actual
+'
+
+test_done

-- 
2.10.0.618.g82cc264
Previous: Jeff KingNext: Jeff King
Message 28 of 32 in “receive-pack: quarantine pushed objects”
  1. 0/6 receive-pack: quarantine pushed objectsJeff King, Sep 30, 2016
  2. 1/6 check_connected: accept an env argumentJeff King, Sep 30, 2016
  3. 2/6 sha1_file: always allow relative paths to alternatesJeff King, Sep 30, 2016
  4. René ScharfeOct 2, 2016
  5. Jeff KingOct 2, 2016
  6. Jeff KingOct 2, 2016
  7. Jeff KingOct 2, 2016
  8. 3/6 tmp-objdir: introduce API for temporary object directoriesJeff King, Sep 30, 2016
  9. Junio C HamanoSep 30, 2016
  10. Jeff KingSep 30, 2016
  11. David TurnerSep 30, 2016
  12. Jeff KingSep 30, 2016
  13. David TurnerSep 30, 2016
  14. 4/6 receive-pack: quarantine objects until pre-receive acceptsJeff King, Sep 30, 2016
  15. Jeff KingOct 1, 2016
  16. Ævar Arnfjörð BjarmasonApr 8, 2017
  17. Jeff KingApr 10, 2017
  18. 5/6 tmp-objdir: put quarantine information in the environmentJeff King, Sep 30, 2016
  19. 6/6 tmp-objdir: do not migrate files starting with '.'Jeff King, Sep 30, 2016
  20. Christian CouderOct 2, 2016
  21. Jeff KingOct 2, 2016
  22. Christian CouderOct 3, 2016
  23. 0/5 receive-pack: quarantine pushed objectsJeff King, Oct 3, 2016
  24. 1/5 check_connected: accept an env argumentJeff King, Oct 3, 2016
  25. Jakub NarębskiOct 5, 2016
  26. Jeff KingOct 5, 2016
  27. 2/5 tmp-objdir: introduce API for temporary object directoriesJeff King, Oct 3, 2016
  28. 3/5 receive-pack: quarantine objects until pre-receive acceptsJeff King, Oct 3, 2016
  29. 5/5 tmp-objdir: do not migrate files starting with '.'Jeff King, Oct 3, 2016
  30. 4/5 tmp-objdir: put quarantine information in the environmentJeff King, Oct 3, 2016
  31. Junio C HamanoOct 3, 2016
  32. Jeff KingOct 3, 2016

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.