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

[PATCH 2/4] push: introduce new push.default mode "simple"

From
Matthieu Moy <matthieu.moy@imag.fr>
Date
Apr 20, 2012, 14:59 UTC
Message-ID
<1334933944-13446-3-git-send-email-Matthieu.Moy@imag.fr>
In-Reply-To
<1334933944-13446-1-git-send-email-Matthieu.Moy@imag.fr>

When calling "git push" without argument, we want to allow Git to do something simple to explain and safe. push.default=matching is unsafe when use to push to shared repositories, and hard to explain to beginners in some context. It is debatable whether 'upstream' or 'current' is the safest or the easiest to explain, so introduce a new mode called 'simple' that is the intersection of them: push the upstream branch, but only if it has the same name remotely. If not, give an error that suggest the right command to push explicitely to 'upstream' or 'current'.

A question is whether to allow pushing when no upstream is configured. An argument in favor of allowing the push is that it makes the new mode work in more cases. On the other hand, refusing to push when no upstream is configured encourages the user to set the upstream, which will be beneficial on the next pull. Lacking better argument, we chose to deny the push, because it will be easier to change in the future is someone shows us wrong.

Original-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Except for the broken-ness, this adds the last line in the warning message:
"To chose either option permanently, read about push.default in git-config(1)"
 Documentation/config.txt |    3 +++
 builtin/push.c           |   32 +++++++++++++++++++++--
 cache.h                  |    1 +
 config.c                 |    4 ++-
 t/t5528-push-default.sh  |   63 +++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 97 insertions(+), 6 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 368a770..05d1472 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1697,6 +1697,9 @@ push.default::
   This option allows publishing a branch to a remote repository using
   the same naming convention locally and remotely, in a more
   conservative and safer way than `matching`.
+* `simple` - like `upstream`, but refuses to push if the upstream
+  branch's name is different from the local one. This is the safest
+  option and is well-suited for beginners.
 
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
diff --git a/builtin/push.c b/builtin/push.c
index 6936713..ba0d6a0 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -76,7 +76,7 @@ static int push_url_of_remote(struct remote *remote, const char ***url_p)
 	return remote->url_nr;
 }
 
-static void setup_push_upstream(struct remote *remote)
+static void setup_push_upstream(struct remote *remote, int simple)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
@@ -103,6 +103,30 @@ static void setup_push_upstream(struct remote *remote)
 		      "your current branch '%s', without telling me what to push\n"
 		      "to update which remote branch."),
 		    remote->name, branch->name);
+	if (simple && strcmp(branch->refname, branch->merge[0]->src)) {
+		/*
+		 * There's no point in using shorten_unambiguous_ref here,
+		 * as the ambiguity would be on the remote side, not what
+		 * we have locally. Plus, this is supposed to be the simple
+		 * mode. If the user is doing something crazy like setting
+		 * upstream to a non-branch, we should probably be showing
+		 * them the big ugly fully qualified ref.
+		 */
+		const char *short_up = skip_prefix(branch->merge[0]->src, "refs/heads/");
+		die(_("The upstream branch of your current branch does not match\n"
+		      "the name of your current branch.  To push to the upstream branch\n"
+		      "on the remote, use\n"
+		      "\n"
+		      "    git push %s HEAD:%s\n"
+		      "\n"
+		      "To push to the branch of the same name on the remote, use\n"
+		      "\n"
+		      "    git push %s %s\n"
+		      "\n"
+		      "To chose either option permanently, read about push.default in git-config(1)\n"),
+		    remote->name, short_up ? short_up : branch->merge[0]->src,
+		    remote->name, branch->name);
+	}
 
 	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
 	add_refspec(refspec.buf);
@@ -119,8 +143,12 @@ static void setup_default_push_refspecs(struct remote *remote)
 		add_refspec(":");
 		break;
 
+	case PUSH_DEFAULT_SIMPLE:
+		setup_push_upstream(remote, 1);
+		break;
+
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream(remote);
+		setup_push_upstream(remote, 0);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:
diff --git a/cache.h b/cache.h
index d8f6f1e..5e419a1 100644
--- a/cache.h
+++ b/cache.h
@@ -580,6 +580,7 @@ enum rebase_setup_type {
 enum push_default_type {
 	PUSH_DEFAULT_NOTHING = 0,
 	PUSH_DEFAULT_MATCHING,
+	PUSH_DEFAULT_SIMPLE,
 	PUSH_DEFAULT_UPSTREAM,
 	PUSH_DEFAULT_CURRENT,
 	PUSH_DEFAULT_UNSPECIFIED
diff --git a/config.c b/config.c
index 68d3294..024bc74 100644
--- a/config.c
+++ b/config.c
@@ -829,6 +829,8 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_NOTHING;
 		else if (!strcmp(value, "matching"))
 			push_default = PUSH_DEFAULT_MATCHING;
+		else if (!strcmp(value, "simple"))
+			push_default = PUSH_DEFAULT_SIMPLE;
 		else if (!strcmp(value, "upstream"))
 			push_default = PUSH_DEFAULT_UPSTREAM;
 		else if (!strcmp(value, "tracking")) /* deprecated */
@@ -837,7 +839,7 @@ static int git_default_push_config(const char *var, const char *value)
 			push_default = PUSH_DEFAULT_CURRENT;
 		else {
 			error("Malformed value for %s: %s", var, value);
-			return error("Must be one of nothing, matching, "
+			return error("Must be one of simple, nothing, matching, "
 				     "tracking or current.");
 		}
 		return 0;
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index c334c51..949dbdf 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -13,6 +13,22 @@ test_expect_success 'setup bare remotes' '
 	git push parent2 HEAD
 '
 
+# $1 = local revision
+# $2 = remote repository
+# $3 = remote revision (tested to be equal to the local one)
+check_pushed_commit () {
+	git rev-parse "$1" > expect &&
+	git --git-dir="$2" rev-parse "$3" > actual &&
+	test_cmp expect actual
+}
+
+# $1 = push.default value
+# $2 = expected target branch for the push
+test_push_success () {
+	git -c push.default="$1" push &&
+	check_pushed_commit HEAD repo1 "$2"
+}
+
 test_expect_success '"upstream" pushes to configured upstream' '
 	git checkout master &&
 	test_config branch.master.remote parent1 &&
@@ -20,9 +36,7 @@ test_expect_success '"upstream" pushes to configured upstream' '
 	test_config push.default upstream &&
 	test_commit two &&
 	git push &&
-	echo two >expect &&
-	git --git-dir=repo1 log -1 --format=%s foo >actual &&
-	test_cmp expect actual
+	check_pushed_commit HEAD repo1 foo
 '
 
 test_expect_success '"upstream" does not push on unconfigured remote' '
@@ -51,4 +65,47 @@ test_expect_success '"upstream" does not push when remotes do not match' '
 	test_must_fail git push parent2
 '
 
+test_expect_success 'push from/to new branch with upstream, matching and simple' '
+	git checkout -b new-branch &&
+	test_must_fail git -c push.default=simple push &&
+	test_must_fail git -c push.default=matching push &&
+	test_must_fail git -c push.default=upstream push
+'
+
+test_expect_success 'push from/to new branch with current creates remote branch' '
+	test_config branch.new-branch.remote repo1 &&
+	git checkout new-branch &&
+	test_push_success current new-branch
+'
+
+test_expect_success 'push to existing branch, with no upstream configured' '
+	test_config branch.master.remote repo1 &&
+	git checkout master &&
+	test_must_fail git -c push.default=simple push &&
+	test_must_fail git -c push.default=upstream push
+'
+
+test_expect_success 'push to existing branch, upstream configured with same name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/master &&
+	git checkout master &&
+	test_commit six &&
+	test_push_success upstream master &&
+	test_commit seven &&
+	test_push_success simple master &&
+	check_pushed_commit HEAD repo1 master
+'
+
+test_expect_success 'push to existing branch, upstream configured with different name' '
+	test_config branch.master.remote repo1 &&
+	test_config branch.master.merge refs/heads/other-name &&
+	git checkout master &&
+	test_commit eight &&
+	test_push_success upstream other-name &&
+	test_commit nine &&
+	test_must_fail git -c push.default=simple push &&
+	test_push_success current master &&
+	test_must_fail check_pushed_commit HEAD repo1 other-name
+'
+
 test_done

-- 
1.7.10.140.g8c333
Previous: Junio C HamanoNext: Jeff King
Message 70 of 152 in “[ANNOUNCE] Git 1.7.10-rc3”
  1. Junio C HamanoMar 28, 2012
  2. Jeff KingMar 29, 2012
  3. Junio C HamanoMar 29, 2012
  4. Jeff KingMar 29, 2012
  5. Junio C HamanoMar 30, 2012
  6. push.default: current vs upstreamJeff King, Mar 30, 2012
  7. Junio C HamanoMar 30, 2012
  8. Jeff KingMar 30, 2012
  9. Junio C HamanoMar 30, 2012
  10. Jeff KingMar 30, 2012
  11. Junio C HamanoMar 30, 2012
  12. Jeff KingMar 30, 2012
  13. Junio C HamanoMar 30, 2012
  14. Nathan GrayMar 31, 2012
  15. Seth RobertsonMar 31, 2012
  16. Junio C HamanoApr 1, 2012
  17. Nathan GrayApr 1, 2012
  18. Matthieu MoyApr 2, 2012
  19. Junio C HamanoApr 2, 2012
  20. Matthieu MoyApr 2, 2012
  21. Junio C HamanoApr 2, 2012
  22. Matthieu MoyApr 2, 2012
  23. Junio C HamanoApr 2, 2012
  24. Matthieu MoyApr 2, 2012
  25. Junio C HamanoApr 2, 2012
  26. demerphqApr 2, 2012
  27. Matthieu MoyApr 2, 2012
  28. demerphqApr 4, 2012
  29. Jeff KingApr 5, 2012
  30. Matthieu MoyApr 5, 2012
  31. Jeff KingApr 6, 2012
  32. Matthieu MoyApr 6, 2012
  33. Jeff KingApr 6, 2012
  34. Junio C HamanoApr 6, 2012
  35. Jehan BingApr 6, 2012
  36. Michael HaggertyApr 7, 2012
  37. Jeff KingApr 7, 2012
  38. Andrew SayersApr 7, 2012
  39. Jeff KingApr 12, 2012
  40. Junio C HamanoApr 12, 2012
  41. Andrew SayersApr 12, 2012
  42. Junio C HamanoApr 12, 2012
  43. Jeff KingApr 12, 2012
  44. Philip OakleyApr 12, 2012
  45. Junio C HamanoApr 13, 2012
  46. Andrew SayersApr 17, 2012
  47. Junio C HamanoApr 8, 2012
  48. Matthieu MoyApr 11, 2012
  49. Junio C HamanoApr 11, 2012
  50. Jeff KingApr 12, 2012
  51. Matthieu MoyApr 12, 2012
  52. Jeff KingApr 12, 2012
  53. Matthieu MoyApr 12, 2012
  54. Junio C HamanoApr 12, 2012
  55. Junio C HamanoApr 19, 2012
  56. Matthieu MoyApr 19, 2012
  57. Junio C HamanoApr 19, 2012
  58. 0/3 push.default upcomming changeMatthieu Moy, Apr 19, 2012
  59. 1/3 push: introduce new push.default mode "simple"Matthieu Moy, Apr 19, 2012
  60. Jeff KingApr 19, 2012
  61. Matthieu MoyApr 20, 2012
  62. 0/4 push.default upcomming changeMatthieu Moy, Apr 20, 2012
  63. 1/4 Documentation: explain push.default option a bit moreMatthieu Moy, Apr 20, 2012
  64. Jeff KingApr 20, 2012
  65. Junio C HamanoApr 20, 2012
  66. Michael HaggertyApr 21, 2012
  67. Junio C HamanoApr 21, 2012
  68. Michael HaggertyApr 21, 2012
  69. Junio C HamanoApr 21, 2012
  70. 2/4 push: introduce new push.default mode "simple"Matthieu Moy, Apr 20, 2012
  71. Jeff KingApr 20, 2012
  72. Zbigniew Jędrzejewski-SzmekApr 22, 2012
  73. Junio C HamanoApr 20, 2012
  74. Matthieu MoyApr 23, 2012
  75. 3/4 t5570: use explicit push refspecMatthieu Moy, Apr 20, 2012
  76. 4/4 push: start warning upcoming default change for push.defaultMatthieu Moy, Apr 20, 2012
  77. Jeff KingApr 20, 2012
  78. Matthieu MoyApr 22, 2012
  79. Junio C HamanoApr 23, 2012
  80. 0/7 push.default upcomming changeMatthieu Moy, Apr 23, 2012
  81. 1/7 Documentation: explain push.default option a bit moreMatthieu Moy, Apr 23, 2012
  82. Junio C HamanoApr 23, 2012
  83. Philip OakleyApr 23, 2012
  84. Junio C HamanoApr 23, 2012
  85. Philip OakleyApr 23, 2012
  86. 2/7 Undocument deprecated alias 'push.default=tracking'Matthieu Moy, Apr 23, 2012
  87. Junio C HamanoApr 23, 2012
  88. Ævar Arnfjörð BjarmasonJan 31, 2013
  89. Junio C HamanoJan 31, 2013
  90. Jonathan NiederJan 31, 2013
  91. Jonathan NiederJan 31, 2013
  92. Junio C HamanoJan 31, 2013
  93. Junio C HamanoJan 31, 2013
  94. Jonathan NiederJan 31, 2013
  95. Junio C HamanoJan 31, 2013
  96. Jonathan NiederJan 31, 2013
  97. Junio C HamanoJan 31, 2013
  98. Junio C HamanoJan 31, 2013
  99. Jonathan NiederJan 31, 2013
  100. Matthieu MoyJan 31, 2013
  101. Junio C HamanoJan 31, 2013
  102. Junio C HamanoJan 31, 2013
  103. Jonathan NiederJan 31, 2013
  104. Junio C HamanoFeb 1, 2013
  105. Jonathan NiederJan 31, 2013
  106. Philip OakleyJan 31, 2013
  107. 3/7 t5528-push-default.sh: add helper functionsMatthieu Moy, Apr 23, 2012
  108. Junio C HamanoApr 23, 2012
  109. Matthieu MoyApr 23, 2012
  110. Junio C HamanoApr 23, 2012
  111. Matthieu MoyApr 23, 2012
  112. Matthieu MoyApr 23, 2012
  113. Junio C HamanoApr 23, 2012
  114. Junio C HamanoApr 23, 2012
  115. 2/3 fixup! t5528-push-default.sh: add helper functionsJunio C Hamano, Apr 23, 2012
  116. 3/3 push: suggested updates to push configuration documentationJunio C Hamano, Apr 23, 2012
  117. 4/7 push: introduce new push.default mode "simple"Matthieu Moy, Apr 23, 2012
  118. Michael HaggertyApr 23, 2012
  119. Matthieu MoyApr 23, 2012
  120. Junio C HamanoApr 23, 2012
  121. Matthieu MoyApr 23, 2012
  122. 5/7 t5570: use explicit push refspecMatthieu Moy, Apr 23, 2012
  123. 6/7 push: document the future default change for push.default (matching -> simple)Matthieu Moy, Apr 23, 2012
  124. 7/7 push: start warning upcoming default change for push.defaultMatthieu Moy, Apr 23, 2012
  125. 0/7 push.default upcomming changeMatthieu Moy, Apr 24, 2012
  126. 1/7 Documentation: explain push.default option a bit moreMatthieu Moy, Apr 24, 2012
  127. 2/7 Undocument deprecated alias 'push.default=tracking'Matthieu Moy, Apr 24, 2012
  128. 3/7 t5528-push-default.sh: add helper functionsMatthieu Moy, Apr 24, 2012
  129. 4/7 push: introduce new push.default mode "simple"Matthieu Moy, Apr 24, 2012
  130. Junio C HamanoApr 25, 2012
  131. Matthieu MoyApr 25, 2012
  132. 5/7 t5570: use explicit push refspecMatthieu Moy, Apr 24, 2012
  133. 6/7 push: document the future default change for push.default (matching -> simple)Matthieu Moy, Apr 24, 2012
  134. 7/7 push: start warning upcoming default change for push.defaultMatthieu Moy, Apr 24, 2012
  135. Junio C HamanoApr 24, 2012
  136. 2/3 t5570: use explicit push refspecMatthieu Moy, Apr 19, 2012
  137. 3/3 push: start warning upcoming default change for push.defaultMatthieu Moy, Apr 19, 2012
  138. t5541: warning message is given even with --quietJunio C Hamano, Apr 26, 2012
  139. Matthieu MoyApr 26, 2012
  140. Give better 'pull' advice when pushing non-ff updates to current branchChristopher Tiwald, Apr 11, 2012
  141. Dmitry PotapovApr 6, 2012
  142. demerphqApr 6, 2012
  143. Dmitry PotapovApr 6, 2012
  144. demerphqApr 6, 2012
  145. Dmitry PotapovApr 6, 2012
  146. Junio C HamanoMar 30, 2012
  147. Jeff KingApr 3, 2012
  148. Jeff KingApr 3, 2012
  149. Junio C HamanoApr 3, 2012
  150. Junio C HamanoApr 3, 2012
  151. Jeff KingApr 5, 2012
  152. Felipe ContrerasApr 8, 2012

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.