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

[PATCHv2 3/6] send-pack.c: add --atomic command line argument

From
Stefan Beller <sbeller@google.com>
Date
Dec 16, 2014, 18:49 UTC
Message-ID
<1418755747-22506-3-git-send-email-sbeller@google.com>
In-Reply-To
<1418755747-22506-1-git-send-email-sbeller@google.com>
From: Ronnie Sahlberg <sahlberg@google.com>

This adds support to send-pack to negotiate and use atomic pushes iff the server supports it. Atomic pushes are activated by a new command line flag --atomic.

In order to do this we also need to change the semantics for send_pack() slightly. The existing send_pack() function actually doesn't send all the refs back to the server when multiple refs are involved, for example when using --all. Several of the failure modes for pushes can already be detected locally in the send_pack client based on the information from the initial server side list of all the refs as generated by receive-pack. Any such refs that we thus know would fail to push are thus pruned from the list of refs we send to the server to update.

For atomic pushes, we have to deal thus with both failures that are detected locally as well as failures that are reported back from the server. In order to do so we treat all local failures as push failures too.

We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can flag all refs that we would normally have tried to push to the server but we did not due to local failures. This is to improve the error message back to the end user to flag that "these refs failed to update since the atomic push operation failed."

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Notes:
    Changes v1 -> v2:
     * Now we only need a very small change in the existing code and have
       a new static function, which cares about error reporting.
    	  Junio wrote:
    	  > Hmph.  Is "atomic push" so special that it deserves a separate
    	  > parameter?  When we come up with yet another mode of failure, would
    	  > we add another parameter to the callers to this function?
     * error messages are worded differently (lower case!),
     * use of error function instead of fprintf
    
     * undashed the printed error message ("atomic push failed");
 Documentation/git-send-pack.txt |  7 ++++++-
 builtin/send-pack.c             |  6 +++++-
 remote.h                        |  3 ++-
 send-pack.c                     | 36 ++++++++++++++++++++++++++++++++++--
 transport.c                     |  4 ++++
 5 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 2a0de42..45c7725 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another repository
 SYNOPSIS
 --------
 [verse]
-'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]
+'git send-pack' [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic] [<host>:]<directory> [<ref>...]
 
 DESCRIPTION
 -----------
@@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a flush packet.
 	Send a "thin" pack, which records objects in deltified form based
 	on objects not included in the pack to reduce network traffic.
 
+--atomic::
+	Use an atomic transaction for updating the refs. If any of the refs
+	fails to update then the entire push will fail without changing any
+	refs.
+
 <host>::
 	A remote host to house the repository.  When this
 	part is specified, 'git-receive-pack' is invoked via
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index b564a77..b961e5a 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -13,7 +13,7 @@
 #include "sha1-array.h"
 
 static const char send_pack_usage[] =
-"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
+"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [--atomic] [<host>:]<directory> [<ref>...]\n"
 "  --all and explicit <ref> specification are mutually exclusive.";
 
 static struct send_pack_args args;
@@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 				args.use_thin_pack = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--atomic")) {
+				args.atomic = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--stateless-rpc")) {
 				args.stateless_rpc = 1;
 				continue;
diff --git a/remote.h b/remote.h
index 8b62efd..f346524 100644
--- a/remote.h
+++ b/remote.h
@@ -115,7 +115,8 @@ struct ref {
 		REF_STATUS_REJECT_SHALLOW,
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
-		REF_STATUS_EXPECTING_REPORT
+		REF_STATUS_EXPECTING_REPORT,
+		REF_STATUS_ATOMIC_PUSH_FAILED
 	} status;
 	char *remote_status;
 	struct ref *peer_ref; /* when renaming */
diff --git a/send-pack.c b/send-pack.c
index 1c4ac75..71b1915 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -279,6 +279,30 @@ free_return:
 	return update_seen;
 }
 
+
+static int atomic_push_failure(struct send_pack_args *args,
+			       struct ref *remote_refs,
+			       struct ref *failing_ref)
+{
+	struct ref *ref;
+	/* Mark other refs as failed */
+	for (ref = remote_refs; ref; ref = ref->next) {
+		if (!ref->peer_ref && !args->send_mirror)
+			continue;
+
+		switch (ref->status) {
+		case REF_STATUS_EXPECTING_REPORT:
+			ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
+			continue;
+		default:
+			; /* do nothing */
+		}
+	}
+	error("atomic push failed for ref %s. "
+	      "status: %d\n", failing_ref->name, failing_ref->status);
+	return -1;
+}
+
 int send_pack(struct send_pack_args *args,
 	      int fd[], struct child_process *conn,
 	      struct ref *remote_refs,
@@ -371,9 +395,17 @@ int send_pack(struct send_pack_args *args,
 	 * the pack data.
 	 */
 	for (ref = remote_refs; ref; ref = ref->next) {
-		if (no_ref_update_to_be_sent(ref, args))
+		int reject_reason;
+		if ((reject_reason = no_ref_update_to_be_sent(ref, args))) {
+			/* When we know the server would reject a ref update if
+			 * we were to send it and we're trying to send the refs
+			 * atomically, abort the whole operation */
+			if (use_atomic && reject_reason == 2)
+				return atomic_push_failure(args,
+							   remote_refs,
+							   ref);
 			continue;
-
+		}
 		if (!ref->deletion)
 			need_pack_data = 1;
 
diff --git a/transport.c b/transport.c
index 70d38e4..c67feee 100644
--- a/transport.c
+++ b/transport.c
@@ -728,6 +728,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 						 ref->deletion ? NULL : ref->peer_ref,
 						 "remote failed to report status", porcelain);
 		break;
+	case REF_STATUS_ATOMIC_PUSH_FAILED:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+						 "atomic push failed", porcelain);
+		break;
 	case REF_STATUS_OK:
 		print_ok_ref_status(ref, porcelain);
 		break;

-- 
2.2.0.31.gad78000.dirty
Previous: Junio C HamanoNext: Junio C Hamano
Message 17 of 56 in “Add a flag to push atomically”
  1. 0/5 Add a flag to push atomicallyStefan Beller, Dec 15, 2014
  2. 1/5 receive-pack.c: add protocol support to negotiate atomic-pushStefan Beller, Dec 15, 2014
  3. Junio C HamanoDec 15, 2014
  4. Stefan BellerDec 15, 2014
  5. 2/5 send-pack.c: add an --atomic-push command line argumentStefan Beller, Dec 15, 2014
  6. Junio C HamanoDec 15, 2014
  7. 3/5 receive-pack.c: use a single ref_transaction for atomic pushesStefan Beller, Dec 15, 2014
  8. Junio C HamanoDec 15, 2014
  9. 4/5 push.c: add an --atomic-push argumentStefan Beller, Dec 15, 2014
  10. Junio C HamanoDec 15, 2014
  11. 5/5 t5543-atomic-push.sh: add basic tests for atomic pushesStefan Beller, Dec 15, 2014
  12. Junio C HamanoDec 15, 2014
  13. Junio C HamanoDec 15, 2014
  14. 1/6 receive-pack.c: add protocol support to negotiate atomic-pushStefan Beller, Dec 16, 2014
  15. 2/6 send-pack: Invert the return value of ref_update_to_be_sentStefan Beller, Dec 16, 2014
  16. Junio C HamanoDec 16, 2014
  17. 3/6 send-pack.c: add --atomic command line argumentStefan Beller, Dec 16, 2014
  18. Junio C HamanoDec 16, 2014
  19. 4/6 receive-pack.c: use a single ref_transaction for atomic pushesStefan Beller, Dec 16, 2014
  20. Eric SunshineDec 16, 2014
  21. Eric SunshineDec 16, 2014
  22. Junio C HamanoDec 16, 2014
  23. 5/6 push.c: add an --atomic-push argumentStefan Beller, Dec 16, 2014
  24. Eric SunshineDec 16, 2014
  25. Junio C HamanoDec 16, 2014
  26. Junio C HamanoDec 16, 2014
  27. 6/6 t5543-atomic-push.sh: add basic tests for atomic pushesStefan Beller, Dec 16, 2014
  28. receive-pack: refuse all commands if one fails in atomic modeStefan Beller, Dec 16, 2014
  29. Junio C HamanoDec 16, 2014
  30. Eric SunshineDec 16, 2014
  31. Junio C HamanoDec 16, 2014
  32. Stefan BellerDec 16, 2014
  33. Junio C HamanoDec 16, 2014
  34. Stefan BellerDec 16, 2014
  35. Junio C HamanoDec 16, 2014
  36. Stefan BellerDec 16, 2014
  37. Junio C HamanoDec 16, 2014
  38. 0/6 atomic pushesStefan Beller, Dec 17, 2014
  39. 1/6 receive-pack.c: add protocol support to negotiate atomicStefan Beller, Dec 17, 2014
  40. Eric SunshineDec 19, 2014
  41. 2/6 send-pack: Rename ref_update_to_be_sent to check_to_send_updateStefan Beller, Dec 17, 2014
  42. Junio C HamanoDec 17, 2014
  43. 3/6 send-pack.c: add --atomic command line argumentStefan Beller, Dec 17, 2014
  44. Junio C HamanoDec 17, 2014
  45. Eric SunshineDec 19, 2014
  46. 4/6 receive-pack.c: use a single ref_transaction for atomic pushesStefan Beller, Dec 17, 2014
  47. Junio C HamanoDec 17, 2014
  48. Stefan BellerDec 17, 2014
  49. Junio C HamanoDec 18, 2014
  50. 4/6 receive-pack.c: use a single ref_transaction for atomic pushesStefan Beller, Dec 18, 2014
  51. Eric SunshineDec 18, 2014
  52. 4/6 receive-pack.c: use a single ref_transaction for atomic pushesStefan Beller, Dec 19, 2014
  53. Eric SunshineDec 19, 2014
  54. 5/6 push.c: add an --atomic argumentStefan Beller, Dec 17, 2014
  55. Eric SunshineDec 19, 2014
  56. 6/6 t5543-atomic-push.sh: add basic tests for atomic pushesStefan Beller, Dec 17, 2014

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.