[PATCH] Give better 'pull' advice when pushing non-ff updates to current branch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Suppose a user configured a local branch to track an upstream branch by
a different name or didn't set an upstream branch at all. In these
cases, issuing 'git pull' without specifying a remote repository or
refspec can be dangerous. In the first case, 'git pull --rebase' could
rewrite published history. In the second, 'git pull' without argument
will fail.

Modify 'git push's non-fast-forward advice to account for these cases.
Instruct users who push a non-fast-forward update to their current
branch to 'git pull <repository> <refspec>' when the branch is untracked
or tracks to a different repo or refspec then the one they specified.
Otherwise, instruct users to 'git pull'. Make both types of advice
configurable, so that users who disable one won't disable the other on
accident. Finally, offer users who configure a branch for octopus
merges, i.e. where 'branch->merge_nr > 1', the simple 'git pull' advice.

Signed-off-by: Christopher Tiwald <christiwald@xxxxxxxxx>
---
Sent this out a while back [1] but I think it went unnoticed in the
'push default' discussion. I wanted to wait until ct/advise-push-default
hit master before resending.

This patch clarifies the 'git pull' advice offered when a push to the
current branch fails for a non-fast-forward error. I think the patch is
reasonable up to the 'push default' change, possibly longer, but I'm not
totally happy with "pushNonFFCurrentUntracked" and "pushNonFFCurrentTracked".
I think they're both too long as variables and prohibit usability. I'm at
a loss for shorter names that convey as much information.

[1] http://thread.gmane.org/gmane.comp.version-control.git/194175/focus=195142
--
Christopher Tiwald



 Documentation/config.txt |    9 +++++++--
 advice.c                 |    6 ++++--
 advice.h                 |    3 ++-
 builtin/push.c           |   48 +++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fb386ab..fd72120 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -141,9 +141,14 @@ advice.*::
 		Set this variable to 'false' if you want to disable
 		'pushNonFFCurrent', 'pushNonFFDefault', and
 		'pushNonFFMatching' simultaneously.
-	pushNonFFCurrent::
+	pushNonFFCurrentUntracked::
 		Advice shown when linkgit:git-push[1] fails due to a
-		non-fast-forward update to the current branch.
+		non-fast-forward update to the current branch and that
+		branch doesn't match the tracked remote and refspec.
+	pushNonFFCurrentTracked::
+		Advice shown when linkgit:git-push[1] fails due to a
+		non-fast-forward update to the current branch and that
+		branch matches the tracked remote and refspec.
 	pushNonFFDefault::
 		Advice to set 'push.default' to 'upstream' or 'current'
 		when you ran linkgit:git-push[1] and pushed 'matching
diff --git a/advice.c b/advice.c
index a492eea..828a41b 100644
--- a/advice.c
+++ b/advice.c
@@ -1,7 +1,8 @@
 #include "cache.h"
 
 int advice_push_nonfastforward = 1;
-int advice_push_non_ff_current = 1;
+int advice_push_non_ff_current_untracked = 1;
+int advice_push_non_ff_current_tracked = 1;
 int advice_push_non_ff_default = 1;
 int advice_push_non_ff_matching = 1;
 int advice_status_hints = 1;
@@ -15,7 +16,8 @@ static struct {
 	int *preference;
 } advice_config[] = {
 	{ "pushnonfastforward", &advice_push_nonfastforward },
-	{ "pushnonffcurrent", &advice_push_non_ff_current },
+	{ "pushnonffcurrentuntracked", &advice_push_non_ff_current_untracked },
+	{ "pushnonffcurrenttracked", &advice_push_non_ff_current_tracked },
 	{ "pushnonffdefault", &advice_push_non_ff_default },
 	{ "pushnonffmatching", &advice_push_non_ff_matching },
 	{ "statushints", &advice_status_hints },
diff --git a/advice.h b/advice.h
index f3cdbbf..c18809f 100644
--- a/advice.h
+++ b/advice.h
@@ -4,7 +4,8 @@
 #include "git-compat-util.h"
 
 extern int advice_push_nonfastforward;
-extern int advice_push_non_ff_current;
+extern int advice_push_non_ff_current_untracked;
+extern int advice_push_non_ff_current_tracked;
 extern int advice_push_non_ff_default;
 extern int advice_push_non_ff_matching;
 extern int advice_status_hints;
diff --git a/builtin/push.c b/builtin/push.c
index 6936713..e6614e9 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -134,12 +134,18 @@ static void setup_default_push_refspecs(struct remote *remote)
 	}
 }
 
-static const char message_advice_pull_before_push[] =
+static const char message_advice_tracked_pull_before_push[] =
 	N_("Updates were rejected because the tip of your current branch is behind\n"
 	   "its remote counterpart. Merge the remote changes (e.g. 'git pull')\n"
 	   "before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
+static const char message_advice_untracked_pull_before_push[] =
+	N_("Updates were rejected because the tip of your current branch is behind\n"
+	   "its remote counterpart. Merge the remote changes to your local branch\n"
+	   "(e.g. 'git pull <repository> <refspec>') before pushing again.\n"
+	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
+
 static const char message_advice_use_upstream[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
 	   "counterpart. If you did not intend to push that branch, you may want to\n"
@@ -152,11 +158,20 @@ static const char message_advice_checkout_pull_push[] =
 	   "(e.g. 'git pull') before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
-static void advise_pull_before_push(void)
+static void advise_tracked_pull_before_push(void)
+{
+	if (!advice_push_non_ff_current_tracked ||
+	    !advice_push_nonfastforward)
+		return;
+	advise(_(message_advice_tracked_pull_before_push));
+}
+
+static void advise_untracked_pull_before_push(void)
 {
-	if (!advice_push_non_ff_current || !advice_push_nonfastforward)
+	if (!advice_push_non_ff_current_untracked ||
+	    !advice_push_nonfastforward)
 		return;
-	advise(_(message_advice_pull_before_push));
+	advise(_(message_advice_untracked_pull_before_push));
 }
 
 static void advise_use_upstream(void)
@@ -177,6 +192,16 @@ static int push_with_options(struct transport *transport, int flags)
 {
 	int err;
 	int nonfastforward;
+	struct branch *branch;
+	struct strbuf buf = STRBUF_INIT;
+
+	branch = branch_get(NULL);
+
+	if (branch) {
+		strbuf_addstr(&buf, transport->remote->name);
+		strbuf_addstr(&buf, "/");
+		strbuf_addstr(&buf, branch->name);
+	}
 
 	transport_set_verbosity(transport, verbosity, progress);
 
@@ -201,7 +226,18 @@ static int push_with_options(struct transport *transport, int flags)
 	default:
 		break;
 	case NON_FF_HEAD:
-		advise_pull_before_push();
+		/* Branches configured for octopus merges should advise
+		 * just 'git pull' */
+		if (branch->remote_name &&
+		    branch->merge &&
+		    branch->merge_nr == 1 &&
+		    !strcmp(transport->remote->name, branch->remote_name) &&
+		    !strcmp(strbuf_detach(&buf, NULL),
+			    prettify_refname(branch->merge[0]->dst))) {
+			advise_tracked_pull_before_push();
+		}
+		else
+			advise_untracked_pull_before_push();
 		break;
 	case NON_FF_OTHER:
 		if (default_matching_used)
@@ -211,6 +247,8 @@ static int push_with_options(struct transport *transport, int flags)
 		break;
 	}
 
+	strbuf_release(&buf);
+
 	return 1;
 }
 
-- 
1.7.10.228.g7061d
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]