Re: [PATCH] fix "git push $there +HEAD"

On Wed, 20 Feb 2008, Junio C Hamano wrote:

An earlier commit 47d996a (push: support pushing HEAD to real
branch name) added support for "git push $there HEAD" by
introducing a rewrite rule for the refspecs obtained from the
command line.  However, unlike the usual refspecs, it did not
allow prefixing with '+' to mean forcing the branch.

This refactors the rewriting rule into a separate function, and
teaches it to pay attention to a possible '+' prefix.

Signed-off-by: Junio C Hamano <redacted>
---

 * If we were to do the "remote.*.push = HEAD" I mentioned
   earlier for defeating the default "matching" behaviour while
   pushing into shared repositories, we would need to apply the
   same rewriting rule for the refspecs obtained from remote at
   the beginning of do_push(), and that is what triggered this
   refactoring.

If that's the ultimate goal, I think it would be better to have "<src>" 
treated as "<src>:<resolve of <src>>" instead of as "<src>:<src>" in 
match_refs(); and not do any special rewriting on the front end. How about 
this:

-----

commit a5de00049f4072e3ea54ff05bc265a4b48a5ce74
Author: Daniel Barkalow [off-list ref]
Date:   Wed Feb 20 12:54:05 2008 -0500

    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.
    
    Passes the included test written by Junio Hamano.
    
    Signed-off-by: Daniel Barkalow [off-list ref]

diff --git a/builtin-push.c b/builtin-push.c
index c8cb63e..41df717 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 6b56473..d2b19a8 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 9d2dc33..3370d53 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh

@@ -271,6 +271,27 @@ 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 dry-run' '
 
 	mk_test heads/master &&