Re: [PATCH 1/2 v2] push: better error messages when push.default = tracking

Matthieu Moy [off-list ref] writes:

For all the other cases covered in this patch, the sequence that lead to
this situation would be like this:

	git checkout somebranch
        hack hack hack including commits
        git push

and it is very clear that the user wants to push the current branch to the
corresponding place but the user is getting an error because there is no
"corresponding place" mapping established yet.

I agree "push --set-upstream" is a very good advice to give under that
scenario---it would push the history s/he wanted to push right now, while
establishing the mapping for later use, both at the same time with a
single command.

However, I don't think that applies to this case with detached HEAD; it is
more likely that the user came here this way:

	git checkout somebranch~4 ;# the tip 3 are not quite ready
        hack hack quickfix to do only the sure part of the tip 3 did
        commit and test
        git push

Maybe the user needed to quickly push out a minimum fix out of the more
elaborate work, in which case, what would follow in the workflow is first
to:

	git push origin HEAD:somebranch

in order to unblock others.  This will then be followed by a more
leisurely:

	git rebase HEAD somebranch

to get back to the more elaborate work that is not yet presentable.

It is not likely that the end user wanted to:

	git checkout $not_a_branch_tip ;# detached
        hack hack hack including commits
        git push origin an_unrelated_branch

and wanted to omit "where and what" part.  We are talking about "push the
current branch only to corresponding destination" people, so if that
unrelated branch were already ready for external consumption, they would
have already pushed it out at the end of the session when they were on
that branch (and seen the other advice you are adding in this patch).

That is why I suggested to advice an explicit push, without checkout nor
set upstream, in my original review message.  IOW, I think the message
should instead suggest:

	If you want to push the history leading to the current (detached)
	state now, use

	    git push $remote HEAD:the-branch-you-want-to-push-to

A common scenario is to create a new branch and push it (checkout -b &&
push [--set-upstream]). In this case, the user was getting "The current
branch %s has no upstream branch.", which doesn't help much.

Provide the user a command to push the current branch. To avoid the
situation in the future, suggest --set-upstream.

While we're there, also improve the error message in the "detached HEAD"
case. We mention explicitly "detached HEAD" since this is the keyword to
look for in documentations.

Signed-off-by: Matthieu Moy <redacted>
---

I applied Junio's suggestion to suggest pushing HEAD in detached HEAD
state. I don't care very much either way indeed (and I didn't want to
make the message too heavy, just give the user a way to do something).

hope that's OK for inclusion.

 builtin/push.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 31da418..1b493fb 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -64,14 +64,24 @@ static void set_refspecs(const char **refs, int nr)
 	}
 }
 
-static void setup_push_upstream(void)
+static void setup_push_upstream(struct remote *remote)
 {
 	struct strbuf refspec = STRBUF_INIT;
 	struct branch *branch = branch_get(NULL);
 	if (!branch)
-		die("You are not currently on a branch.");
+		die("You are not currently on a branch.\n"
+		    "To push the history leading to the current (detached HEAD)\n"
+		    "state now, use\n"
+		    "\n"
+		    "    git push %s HEAD:<name-of-remote-branch>\n",
+		    remote->name);
 	if (!branch->merge_nr || !branch->merge)
-		die("The current branch %s has no upstream branch.",
+		die("The current branch %s has no upstream branch.\n"
+		    "To push the current branch and set the remote as upstream, use\n"
+		    "\n"
+		    "    git push --set-upstream %s %s\n",
+		    branch->name,
+		    remote->name,
 		    branch->name);
 	if (branch->merge_nr != 1)
 		die("The current branch %s has multiple upstream branches, "

@@ -80,7 +90,7 @@ static void setup_push_upstream(void)
 	add_refspec(refspec.buf);
 }
 
-static void setup_default_push_refspecs(void)
+static void setup_default_push_refspecs(struct remote *remote)
 {
 	switch (push_default) {
 	default:

@@ -89,7 +99,7 @@ static void setup_default_push_refspecs(void)
 		break;
 
 	case PUSH_DEFAULT_UPSTREAM:
-		setup_push_upstream();
+		setup_push_upstream(remote);
 		break;
 
 	case PUSH_DEFAULT_CURRENT:

@@ -175,7 +185,7 @@ static int do_push(const char *repo, int flags)
 			refspec = remote->push_refspec;
 			refspec_nr = remote->push_refspec_nr;
 		} else if (!(flags & TRANSPORT_PUSH_MIRROR))
-			setup_default_push_refspecs();
+			setup_default_push_refspecs(remote);
 	}
 	errs = 0;
 	if (remote->pushurl_nr) {

-- 
1.7.4.1.176.g6b069.dirty

Signed-off-by: Matthieu Moy <redacted>
---
No change since v2

 builtin/push.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 1b493fb..c3c2feb 100644
--- a/builtin/push.c
+++ b/builtin/push.c

@@ -157,7 +157,14 @@ static int do_push(const char *repo, int flags)
 	if (!remote) {
 		if (repo)
 			die("bad repository '%s'", repo);
-		die("No destination configured to push to.");
+		die("No configured push destination.\n"
+		    "Either specify the URL from the command-line or configure a remote repository using\n"
+		    "\n"
+		    "    git remote add <name> <url>\n"
+		    "\n"
+		    "and then push using the remote name\n"
+		    "\n"
+		    "    git push <name>\n");
 	}
 
 	if (remote->mirror)

-- 
1.7.4.1.176.g6b069.dirty