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

Re: [PATCH 3/6] push: change `simple` to accommodate triangular workflows

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 20, 2013, 02:57 UTC
Message-ID
<7vk3lpwkt6.fsf@alter.siamese.dyndns.org>
In-Reply-To
<7v38sdzx8o.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 5 quoted lines
> Without any configuration the current branch is pushed out, which
> loosens the safety we implemented in the current 'safer upstream'.
>
> I am not convinced this is a good change.  I am not convinced this is
> a bad change, either, yet, but this loosening smells bad.

Provided that we would want to keep the "Push the current one to the same name but you have to have it set up as your integration source" safety for central workflow (which I am starting to think we should), we would want something like this on top of your entire series, I think. The behaviour change can be seen in the revert of one test you made to the test that expects "simple" to fail due to the safety.

This patch is somewhat minimal in that it does not address other issues I raised in the review of the series; it only addresses the "simple must be safe" issue.

 builtin/push.c          | 60 +++++++++++++++++++++++++------------------------
 t/t5528-push-default.sh |  2 +-
 2 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 783bacf..84c4a90 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -120,29 +120,11 @@ static const char message_detached_head_die[] =
 	   "\n"
 	   "    git push %s HEAD:<name-of-remote-branch>\n");
 
-static void setup_push_simple(struct remote *remote)
-{
-	struct branch *branch = branch_get(NULL);
-	if (!branch)
-		die(_(message_detached_head_die), remote->name);
-	if (!branch->merge_nr || !branch->merge || !branch->remote_name)
-		/* No upstream configured */
-		goto end;
-	if (branch->merge_nr != 1)
-		die(_("The current branch %s has multiple upstream branches, "
-		    "refusing to push."), branch->name);
-	if (!strcmp(branch->remote_name, remote->name) &&
-		strcmp(branch->refname, branch->merge[0]->src))
-		/* Central workflow safety feature */
-		die_push_simple(branch, remote);
-end:
-	add_refspec(branch->name);
-}
-
-static void setup_push_upstream(struct remote *remote)
+static void setup_push_upstream(struct remote *remote, struct branch *branch,
+				int triangular)
 {
 	struct strbuf refspec = STRBUF_INIT;
-	struct branch *branch = branch_get(NULL);
+
 	if (!branch)
 		die(_(message_detached_head_die), remote->name);
 	if (!branch->merge_nr || !branch->merge || !branch->remote_name)
@@ -156,16 +138,29 @@ static void setup_push_upstream(struct remote *remote)
 	if (branch->merge_nr != 1)
 		die(_("The current branch %s has multiple upstream branches, "
 		    "refusing to push."), branch->name);
-	if (strcmp(branch->remote_name, remote->name))
+	if (triangular)
 		die(_("You are pushing to remote '%s', which is not the upstream of\n"
 		      "your current branch '%s', without telling me what to push\n"
 		      "to update which remote branch."),
 		    remote->name, branch->name);
 
+	if (push_default == PUSH_DEFAULT_SIMPLE) {
+		/* Additional safety */
+		if (strcmp(branch->refname, branch->merge[0]->src))
+			die_push_simple(branch, remote);
+	}
+
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);
 }
 
+static void setup_push_current(struct remote *remote, struct branch *branch)
+{
+	if (!branch)
+		die(_(message_detached_head_die), remote->name);
+	add_refspec(branch->name);
+}
+
 static char warn_unspecified_push_default_msg[] =
 N_("push.default is unset; its implicit value is changing in\n"
    "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
@@ -190,9 +185,16 @@ static void warn_unspecified_push_default_configuration(void)
 	warning("%s\n", _(warn_unspecified_push_default_msg));
 }
 
+static int is_workflow_triagular(struct remote *remote)
+{
+	struct remote *fetch_remote = remote_get(NULL);
+	return (fetch_remote != remote);
+}
+
 static void setup_default_push_refspecs(struct remote *remote)
 {
-	struct branch *branch;
+	struct branch *branch = branch_get(NULL);
+	int triangular = is_workflow_triagular(remote);
 
 	switch (push_default) {
 	default:
@@ -205,18 +207,18 @@ static void setup_default_push_refspecs(struct remote *remote)
 		break;
 
 	case PUSH_DEFAULT_SIMPLE:
-		setup_push_simple(remote);
+		if (triangular)
+			setup_push_current(remote, branch);
+		else
+			setup_push_upstream(remote, branch, triangular);
 		break;
 
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream(remote);
+		setup_push_upstream(remote, branch, triangular);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:
-		branch = branch_get(NULL);
-		if (!branch)
-			die(_(message_detached_head_die), remote->name);
-		add_refspec(branch->name);
+		setup_push_current(remote, branch);
 		break;
 
 	case PUSH_DEFAULT_NOTHING:
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index eabc09d..36f5a63 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -107,7 +107,7 @@ test_expect_success 'push from/to new branch with current creates remote branch'
 test_expect_success 'push to existing branch, with no upstream configured' '
 	test_config branch.master.remote repo1 &&
 	git checkout master &&
-	test_push_success simple master &&
+	test_push_failure simple &&
 	test_push_failure upstream
 '
Previous: Junio C HamanoNext: Ramkumar Ramachandra
Message 10 of 24 in “push.default in the triangular world”
  1. 0/6 push.default in the triangular worldRamkumar Ramachandra, Jun 19, 2013
  2. 1/6 t/t5528-push-default: remove redundant test_config linesRamkumar Ramachandra, Jun 19, 2013
  3. Junio C HamanoJun 19, 2013
  4. 2/6 config doc: rewrite push.default sectionRamkumar Ramachandra, Jun 19, 2013
  5. Junio C HamanoJun 19, 2013
  6. Junio C HamanoJun 20, 2013
  7. Johan HerlandJun 20, 2013
  8. 3/6 push: change `simple` to accommodate triangular workflowsRamkumar Ramachandra, Jun 19, 2013
  9. Junio C HamanoJun 19, 2013
  10. Junio C HamanoJun 20, 2013
  11. Ramkumar RamachandraJun 20, 2013
  12. Junio C HamanoJun 20, 2013
  13. Philip OakleyJun 20, 2013
  14. Junio C HamanoJun 20, 2013
  15. Ramkumar RamachandraJun 20, 2013
  16. Junio C HamanoJun 20, 2013
  17. Junio C HamanoJun 20, 2013
  18. Junio C HamanoJun 20, 2013
  19. 4/6 push: remove dead code in setup_push_upstream()Ramkumar Ramachandra, Jun 19, 2013
  20. Junio C HamanoJun 19, 2013
  21. 5/6 t/t5528-push-default: generalize test_push_*Ramkumar Ramachandra, Jun 19, 2013
  22. Junio C HamanoJun 19, 2013
  23. 6/6 t/t5528-push-default: test pushdefault workflowsRamkumar Ramachandra, Jun 19, 2013
  24. Junio C HamanoJun 19, 2013

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.