Resolve value supplied for no-colon push refspecs

When pushing a refspec like "HEAD", we used to treat it as
"HEAD:HEAD", which didn't work without rewriting. Instead, we should
resolve the ref. If it's a symref, further require it to point to a
branch, to avoid doing anything especially unexpected. Also remove the
rewriting previously added in builtin-push.

Since the code for "HEAD" uses the regular refspec parsing, it
automatically handles "+HEAD" without anything special.

[jc: added a further test to make sure that "remote.*.push = HEAD" works]

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

3 files changed, 52 insertions, 10 deletions

diff --git a/builtin-push.c b/builtin-push.c
index 9f727c00f..b68c6813b 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -44,15 +44,6 @@ static void set_refspecs(const char **refs, int nr)
 			strcat(tag, refs[i]);
 			ref = tag;
 		}
-		if (!strcmp("HEAD", ref)) {
-			unsigned char sha1_dummy[20];
-			ref = resolve_ref(ref, sha1_dummy, 1, NULL);
-			if (!ref)
-				die("HEAD cannot be resolved.");
-			if (prefixcmp(ref, "refs/heads/"))
-				die("HEAD cannot be resolved to branch.");
-			ref = xstrdup(ref + 11);
-		}
 		add_refspec(ref);
 	}
 }
diff --git a/remote.c b/remote.c
index 6b56473f5..8ee2487ed 100644
--- a/remote.c
+++ b/remote.c
@@ -643,9 +643,17 @@ static int match_explicit(struct ref *src, struct ref *dst,
 		errs = 1;
 
 	if (!dst_value) {
+		unsigned char sha1[20];
+		int flag;
+
 		if (!matched_src)
 			return errs;
-		dst_value = matched_src->name;
+		dst_value = resolve_ref(matched_src->name, sha1, 1, &flag);
+		if (!dst_value ||
+		    ((flag & REF_ISSYMREF) &&
+		     prefixcmp(dst_value, "refs/heads/")))
+			die("%s cannot be resolved to branch.",
+			    matched_src->name);
 	}
 
 	switch (count_refspec_match(dst_value, dst, &matched_dst)) {
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 9d2dc33cb..b0d97db71 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -271,6 +271,49 @@ test_expect_success 'push with HEAD nonexisting at remote' '
 	check_push_result $the_commit heads/local
 '
 
+test_expect_success 'push with +HEAD' '
+
+	mk_test heads/master &&
+	git checkout master &&
+	git branch -D local &&
+	git checkout -b local &&
+	git push testrepo master local &&
+	check_push_result $the_commit heads/master &&
+	check_push_result $the_commit heads/local &&
+
+	# Without force rewinding should fail
+	git reset --hard HEAD^ &&
+	! git push testrepo HEAD &&
+	check_push_result $the_commit heads/local &&
+
+	# With force rewinding should succeed
+	git push testrepo +HEAD &&
+	check_push_result $the_first_commit heads/local
+
+'
+
+test_expect_success 'push with config remote.*.push = HEAD' '
+
+	mk_test heads/local &&
+	git checkout master &&
+	git branch -f local $the_commit &&
+	(
+		cd testrepo &&
+		git checkout local &&
+		git reset --hard $the_first_commit
+	) &&
+	git config remote.there.url testrepo &&
+	git config remote.there.push HEAD &&
+	git config branch.master.remote there &&
+	git push &&
+	check_push_result $the_commit heads/master &&
+	check_push_result $the_first_commit heads/local
+'
+
+# clean up the cruft left with the previous one
+git config --remove-section remote.there
+git config --remove-section branch.master
+
 test_expect_success 'push with dry-run' '
 
 	mk_test heads/master &&