Merge branch 'jc/triangle-push-fixup'

Earlier remote.pushdefault (and per-branch branch.*.pushremote)
were introduced as an additional mechanism to choose what
repository to push into when "git push" did not say it from the
command line, to help people who push to a repository that is
different from where they fetch from.  This attempts to finish that
topic by teaching the default mechanism to choose branch in the
remote repository to be updated by such a push.

The 'current', 'matching' and 'nothing' modes (specified by the
push.default configuration variable) extend to such a "triangular"
workflow naturally, but 'upstream' and 'simple' have to be updated.

. 'upstream' is about pushing back to update the branch in the
  remote repository that the current branch fetches from and
  integrates with, it errors out in a triangular workflow.

. 'simple' is meant to help new people by avoiding mistakes, and
  will be the safe default in Git 2.0.

  In a non-triangular workflow, it will continue to act as a cross
  between 'upstream' and 'current' in that it pushes to the current
  branch's @{upstream} only when it is set to the same name as the
  current branch (e.g. your 'master' forks from the 'master' from
  the central repository).

  In a triangular workflow, this series tentatively defines it as
  the same as 'current', but we may have to tighten it to avoid
  surprises in some way.

* jc/triangle-push-fixup:
  t/t5528-push-default: test pushdefault workflows
  t/t5528-push-default: generalize test_push_*
  push: change `simple` to accommodate triangular workflows
  config doc: rewrite push.default section
  t/t5528-push-default: remove redundant test_config lines

3 files changed, 142 insertions, 46 deletions

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b4d4887bd7..8d6859fa65 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1844,39 +1844,59 @@ pull.twohead::
 	The default merge strategy to use when pulling a single branch.
 
 push.default::
-	Defines the action `git push` should take if no refspec is given
-	on the command line, no refspec is configured in the remote, and
-	no refspec is implied by any of the options given on the command
-	line. Possible values are:
+	Defines the action `git push` should take if no refspec is
+	explicitly given.  Different values are well-suited for
+	specific workflows; for instance, in a purely central workflow
+	(i.e. the fetch source is equal to the push destination),
+	`upstream` is probably what you want.  Possible values are:
 +
 --
-* `nothing` - do not push anything.
-* `matching` - push all branches having the same name in both ends.
-  This is for those who prepare all the branches into a publishable
-  shape and then push them out with a single command.  It is not
-  appropriate for pushing into a repository shared by multiple users,
-  since locally stalled branches will attempt a non-fast forward push
-  if other users updated the branch.
-  +
-  This is currently the default, but Git 2.0 will change the default
-  to `simple`.
-* `upstream` - push the current branch to its upstream branch
-  (`tracking` is a deprecated synonym for this).
-  With this, `git push` will update the same remote ref as the one which
-  is merged by `git pull`, making `push` and `pull` symmetrical.
-  See "branch.<name>.merge" for how to configure the upstream branch.
-* `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. It will become the default
-  in Git 2.0.
-* `current` - push the current branch to a branch of the same name.
---
+
+* `nothing` - do not push anything (error out) unless a refspec is
+  explicitly given. This is primarily meant for people who want to
+  avoid mistakes by always being explicit.
+
+* `current` - push the current branch to update a branch with the same
+  name on the receiving end.  Works in both central and non-central
+  workflows.
+
+* `upstream` - push the current branch back to the branch whose
+  changes are usually integrated into the current branch (which is
+  called `@{upstream}`).  This mode only makes sense if you are
+  pushing to the same repository you would normally pull from
+  (i.e. central workflow).
+
+* `simple` - in centralized workflow, work like `upstream` with an
+  added safety to refuse to push if the upstream branch's name is
+  different from the local one.
 +
-The `simple`, `current` and `upstream` modes are for those who want to
-push out a single branch after finishing work, even when the other
-branches are not yet ready to be pushed out. If you are working with
-other people to push into the same shared repository, you would want
-to use one of these.
+When pushing to a remote that is different from the remote you normally
+pull from, work as `current`.  This is the safest option and is suited
+for beginners.
++
+This mode will become the default in Git 2.0.
+
+* `matching` - push all branches having the same name on both ends.
+  This makes the repository you are pushing to remember the set of
+  branches that will be pushed out (e.g. if you always push 'maint'
+  and 'master' there and no other branches, the repository you push
+  to will have these two branches, and your local 'maint' and
+  'master' will be pushed there).
++
+To use this mode effectively, you have to make sure _all_ the
+branches you would push out are ready to be pushed out before
+running 'git push', as the whole point of this mode is to allow you
+to push all of the branches in one go.  If you usually finish work
+on only one branch and push out the result, while other branches are
+unfinished, this mode is not for you.  Also this mode is not
+suitable for pushing into a shared central repository, as other
+people may add new branches there, or update the tip of existing
+branches outside your control.
++
+This is currently the default, but Git 2.0 will change the default
+to `simple`.
+
+--
 
 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 2d84d10720..a2580a94f5 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -120,10 +120,11 @@ static const char message_detached_head_die[] =
 	   "\n"
 	   "    git push %s HEAD:<name-of-remote-branch>\n");
 
-static void setup_push_upstream(struct remote *remote, int simple)
+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)
@@ -137,18 +138,29 @@ static void setup_push_upstream(struct remote *remote, int simple)
 	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 (simple && strcmp(branch->refname, branch->merge[0]->src))
-		die_push_simple(branch, remote);
+
+	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"
@@ -173,9 +185,16 @@ static void warn_unspecified_push_default_configuration(void)
 	warning("%s\n", _(warn_unspecified_push_default_msg));
 }
 
+static int is_workflow_triangular(struct remote *remote)
+{
+	struct remote *fetch_remote = remote_get(NULL);
+	return (fetch_remote && 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_triangular(remote);
 
 	switch (push_default) {
 	default:
@@ -188,18 +207,18 @@ static void setup_default_push_refspecs(struct remote *remote)
 		break;
 
 	case PUSH_DEFAULT_SIMPLE:
-		setup_push_upstream(remote, 1);
+		if (triangular)
+			setup_push_current(remote, branch);
+		else
+			setup_push_upstream(remote, branch, triangular);
 		break;
 
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream(remote, 0);
+		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 4736da8f36..6a5ac3add4 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -15,17 +15,19 @@ test_expect_success 'setup bare remotes' '
 
 # $1 = local revision
 # $2 = remote revision (tested to be equal to the local one)
+# $3 = [optional] repo to check for actual output (repo1 by default)
 check_pushed_commit () {
 	git log -1 --format='%h %s' "$1" >expect &&
-	git --git-dir=repo1 log -1 --format='%h %s' "$2" >actual &&
+	git --git-dir="${3:-repo1}" log -1 --format='%h %s' "$2" >actual &&
 	test_cmp expect actual
 }
 
 # $1 = push.default value
 # $2 = expected target branch for the push
+# $3 = [optional] repo to check for actual output (repo1 by default)
 test_push_success () {
 	git -c push.default="$1" push &&
-	check_pushed_commit HEAD "$2"
+	check_pushed_commit HEAD "$2" "$3"
 }
 
 # $1 = push.default value
@@ -37,6 +39,26 @@ test_push_failure () {
 	test_cmp expect actual
 }
 
+# $1 = success or failure
+# $2 = push.default value
+# $3 = branch to check for actual output (master or foo)
+# $4 = [optional] switch to triangular workflow
+test_pushdefault_workflow () {
+	workflow=central
+	pushdefault=parent1
+	if test -n "${4-}"; then
+		workflow=triangular
+		pushdefault=parent2
+	fi
+	test_expect_success "push.default = $2 $1 in $workflow workflows" "
+		test_config branch.master.remote parent1 &&
+		test_config branch.master.merge refs/heads/foo &&
+		test_config remote.pushdefault $pushdefault &&
+		test_commit commit-for-$2${4+-triangular} &&
+		test_push_$1 $2 $3 ${4+repo2}
+	"
+}
+
 test_expect_success '"upstream" pushes to configured upstream' '
 	git checkout master &&
 	test_config branch.master.remote parent1 &&
@@ -48,7 +70,6 @@ test_expect_success '"upstream" pushes to configured upstream' '
 test_expect_success '"upstream" does not push on unconfigured remote' '
 	git checkout master &&
 	test_unconfig branch.master.remote &&
-	test_config push.default upstream &&
 	test_commit three &&
 	test_push_failure upstream
 '
@@ -57,7 +78,6 @@ test_expect_success '"upstream" does not push on unconfigured branch' '
 	git checkout master &&
 	test_config branch.master.remote parent1 &&
 	test_unconfig branch.master.merge &&
-	test_config push.default upstream
 	test_commit four &&
 	test_push_failure upstream
 '
@@ -115,4 +135,41 @@ test_expect_success 'push to existing branch, upstream configured with different
 	test_cmp expect-other-name actual-other-name
 '
 
+# We are on 'master', which integrates with 'foo' from parent1
+# remote (set in test_pushdefault_workflow helper).  Push to
+# parent1 in centralized, and push to parent2 in triangular workflow.
+# The parent1 repository has 'master' and 'foo' branches, while
+# the parent2 repository has only 'master' branch.
+#
+# test_pushdefault_workflow() arguments:
+# $1 = success or failure
+# $2 = push.default value
+# $3 = branch to check for actual output (master or foo)
+# $4 = [optional] switch to triangular workflow
+
+# update parent1's master (which is not our upstream)
+test_pushdefault_workflow success current master
+
+# update parent1's foo (which is our upstream)
+test_pushdefault_workflow success upstream foo
+
+# upsream is foo which is not the name of the current branch
+test_pushdefault_workflow failure simple master
+
+# master and foo are updated
+test_pushdefault_workflow success matching master
+
+# master is updated
+test_pushdefault_workflow success current master triangular
+
+# upstream mode cannot be used in triangular
+test_pushdefault_workflow failure upstream foo triangular
+
+# in triangular, 'simple' works as 'current' and update the branch
+# with the same name.
+test_pushdefault_workflow success simple master triangular
+
+# master is updated (parent2 does not have foo)
+test_pushdefault_workflow success matching master triangular
+
 test_done