{"thread":{"id":"12212","subject":"[PATCH] fix \"git push $there +HEAD\"","startedAt":"2008-02-20T10:40:11Z","lastAt":"2008-02-20T18:16:13Z","messageCount":4,"participants":["Junio C Hamano","Johannes Schindelin","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"69365","messageId":"7vprurc3n8.fsf@gitster.siamese.dyndns.org","threadId":"12212","inReplyTo":null,"subject":"[PATCH] fix \"git push $there +HEAD\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-20T10:40:11Z","receivedAt":"2008-02-20T10:40:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"An earlier commit 47d996a (push: support pushing HEAD to real\nbranch name) added support for \"git push $there HEAD\" by\nintroducing a rewrite rule for the refspecs obtained from the\ncommand line. However, unlike the usual refspecs, it did not\nallow prefixing with '+' to mean forcing the branch.\n\nThis refactors the rewriting rule into a separate function, and\nteaches it to pay attention to a possible '+' prefix.\n\nSigned-off-by: Junio C Hamano \n---\n\n * If we were to do the \"remote.*.push = HEAD\" I mentioned\n earlier for defeating the default \"matching\" behaviour while\n pushing into shared repositories, we would need to apply the\n same rewriting rule for the refspecs obtained from remote at\n the beginning of do_push(), and that is what triggered this\n refactoring.\n\n builtin-push.c | 40 +++++++++++++++++++++++++++++++---------\n t/t5516-fetch-push.sh | 21 +++++++++++++++++++++\n 2 files changed, 52 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-push.c b/builtin-push.c\nindex 9f727c0..0336ef8 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -28,6 +28,36 @@ static void add_refspec(const char *ref)\n \trefspec_nr = nr;\n }\n \n+static const char *replace_HEAD(const char *ref)\n+{\n+\tstatic int checked_HEAD;\n+\tstatic char *plus_HEAD;\n+\tint force = 0;\n+\tconst char *s = ref;\n+\n+\tif (s[0] == '+') {\n+\t\tforce = 1;\n+\t\ts++;\n+\t}\n+\tif (strcmp(\"HEAD\", s))\n+\t\treturn ref;\n+\tif (!checked_HEAD) {\n+\t\tunsigned char sha1[20];\n+\t\tsize_t len;\n+\n+\t\tchecked_HEAD = 1;\n+\t\ts = resolve_ref(s, sha1, 1, NULL);\n+\t\tif (!s || prefixcmp(s, \"refs/heads/\"))\n+\t\t\tdie(\"HEAD cannot be resolved to branch.\");\n+\t\tlen = strlen(s);\n+\t\tplus_HEAD = xmalloc(len + 2);\n+\t\tplus_HEAD[0] = '+';\n+\t\tstrcpy(plus_HEAD + 1, s);\n+\t}\n+\n+\treturn plus_HEAD + !force;\n+}\n+\n static void set_refspecs(const char **refs, int nr)\n {\n \tint i;\n@@ -44,15 +74,7 @@ static void set_refspecs(const char **refs, int nr)\n \t\t\tstrcat(tag, refs[i]);\n \t\t\tref = tag;\n \t\t}\n-\t\tif (!strcmp(\"HEAD\", ref)) {\n-\t\t\tunsigned char sha1_dummy[20];\n-\t\t\tref = resolve_ref(ref, sha1_dummy, 1, NULL);\n-\t\t\tif (!ref)\n-\t\t\t\tdie(\"HEAD cannot be resolved.\");\n-\t\t\tif (prefixcmp(ref, \"refs/heads/\"))\n-\t\t\t\tdie(\"HEAD cannot be resolved to branch.\");\n-\t\t\tref = xstrdup(ref + 11);\n-\t\t}\n+\t\tref = replace_HEAD(ref);\n \t\tadd_refspec(ref);\n \t}\n }\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 9d2dc33..3370d53 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -271,6 +271,27 @@ test_expect_success 'push with HEAD nonexisting at remote' '\n \tcheck_push_result $the_commit heads/local\n '\n \n+test_expect_success 'push with +HEAD' '\n+\n+\tmk_test heads/master &&\n+\tgit checkout master &&\n+\tgit branch -D local &&\n+\tgit checkout -b local &&\n+\tgit push testrepo master local &&\n+\tcheck_push_result $the_commit heads/master &&\n+\tcheck_push_result $the_commit heads/local &&\n+\n+\t# Without force rewinding should fail\n+\tgit reset --hard HEAD^ &&\n+\t! git push testrepo HEAD &&\n+\tcheck_push_result $the_commit heads/local &&\n+\n+\t# With force rewinding should succeed\n+\tgit push testrepo +HEAD &&\n+\tcheck_push_result $the_first_commit heads/local\n+\n+'\n+\n test_expect_success 'push with dry-run' '\n \n \tmk_test heads/master &&\n"},{"id":"69368","messageId":"alpine.LSU.1.00.0802201301230.17164@racer.site","threadId":"12212","inReplyTo":"7vprurc3n8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fix \"git push $there +HEAD\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-20T13:02:19Z","receivedAt":"2008-02-20T13:02:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 20 Feb 2008, Junio C Hamano wrote:\n\n> An earlier commit 47d996a (push: support pushing HEAD to real branch \n> name) added support for \"git push $there HEAD\" by introducing a rewrite \n> rule for the refspecs obtained from the command line. However, unlike \n> the usual refspecs, it did not allow prefixing with '+' to mean forcing \n> the branch.\n\nHeh. I realised that yesterday, and put it in my TODO list. Thanks for \ndoing it for me! ;-)\n\nThe patch looks obviously correct to me.\n\nCiao,\nDscho\n"},{"id":"69384","messageId":"alpine.LNX.1.00.0802201226560.19024@iabervon.org","threadId":"12212","inReplyTo":"7vprurc3n8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fix \"git push $there +HEAD\"","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-20T17:59:24Z","receivedAt":"2008-02-20T17:59:24Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 20 Feb 2008, Junio C Hamano wrote:\n\n> An earlier commit 47d996a (push: support pushing HEAD to real\n> branch name) added support for \"git push $there HEAD\" by\n> introducing a rewrite rule for the refspecs obtained from the\n> command line. However, unlike the usual refspecs, it did not\n> allow prefixing with '+' to mean forcing the branch.\n> \n> This refactors the rewriting rule into a separate function, and\n> teaches it to pay attention to a possible '+' prefix.\n> \n> Signed-off-by: Junio C Hamano \n> ---\n> \n> * If we were to do the \"remote.*.push = HEAD\" I mentioned\n> earlier for defeating the default \"matching\" behaviour while\n> pushing into shared repositories, we would need to apply the\n> same rewriting rule for the refspecs obtained from remote at\n> the beginning of do_push(), and that is what triggered this\n> refactoring.\n\nIf that's the ultimate goal, I think it would be better to have \"\" \ntreated as \":>\" instead of as \":\" in \nmatch_refs(); and not do any special rewriting on the front end. How about \nthis:\n\n-----\n\ncommit a5de00049f4072e3ea54ff05bc265a4b48a5ce74\nAuthor: Daniel Barkalow \nDate: Wed Feb 20 12:54:05 2008 -0500\n\n Resolve value supplied for no-colon push refspecs\n \n When pushing a refspec like \"HEAD\", we used to treat it as\n \"HEAD:HEAD\", which didn't work without rewriting. Instead, we should\n resolve the ref. If it's a symref, further require it to point to a\n branch, to avoid doing anything especially unexpected. Also remove the\n rewriting previously added in builtin-push.\n \n Since the code for \"HEAD\" uses the regular refspec parsing, it\n automatically handles \"+HEAD\" without anything special.\n \n Passes the included test written by Junio Hamano.\n \n Signed-off-by: Daniel Barkalow \n\ndiff --git a/builtin-push.c b/builtin-push.c\nindex c8cb63e..41df717 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -44,15 +44,6 @@ static void set_refspecs(const char **refs, int nr)\n \t\t\tstrcat(tag, refs[i]);\n \t\t\tref = tag;\n \t\t}\n-\t\tif (!strcmp(\"HEAD\", ref)) {\n-\t\t\tunsigned char sha1_dummy[20];\n-\t\t\tref = resolve_ref(ref, sha1_dummy, 1, NULL);\n-\t\t\tif (!ref)\n-\t\t\t\tdie(\"HEAD cannot be resolved.\");\n-\t\t\tif (prefixcmp(ref, \"refs/heads/\"))\n-\t\t\t\tdie(\"HEAD cannot be resolved to branch.\");\n-\t\t\tref = xstrdup(ref + 11);\n-\t\t}\n \t\tadd_refspec(ref);\n \t}\n }\ndiff --git a/remote.c b/remote.c\nindex 6b56473..d2b19a8 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -643,9 +643,17 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\terrs = 1;\n \n \tif (!dst_value) {\n+\t\tunsigned char sha1[20];\n+\t\tint flag;\n+\n \t\tif (!matched_src)\n \t\t\treturn errs;\n-\t\tdst_value = matched_src->name;\n+\t\tdst_value = resolve_ref(matched_src->name, sha1, 1, &flag);\n+\t\tif (!dst_value ||\n+\t\t ((flag & REF_ISSYMREF) && \n+\t\t prefixcmp(dst_value, \"refs/heads/\")))\n+\t\t\tdie(\"%s cannot be resolved to branch.\",\n+\t\t\t matched_src->name);\n \t}\n \n \tswitch (count_refspec_match(dst_value, dst, &matched_dst)) {\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 9d2dc33..3370d53 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -271,6 +271,27 @@ test_expect_success 'push with HEAD nonexisting at remote' '\n \tcheck_push_result $the_commit heads/local\n '\n \n+test_expect_success 'push with +HEAD' '\n+\n+\tmk_test heads/master &&\n+\tgit checkout master &&\n+\tgit branch -D local &&\n+\tgit checkout -b local &&\n+\tgit push testrepo master local &&\n+\tcheck_push_result $the_commit heads/master &&\n+\tcheck_push_result $the_commit heads/local &&\n+\n+\t# Without force rewinding should fail\n+\tgit reset --hard HEAD^ &&\n+\t! git push testrepo HEAD &&\n+\tcheck_push_result $the_commit heads/local &&\n+\n+\t# With force rewinding should succeed\n+\tgit push testrepo +HEAD &&\n+\tcheck_push_result $the_first_commit heads/local\n+\n+'\n+\n test_expect_success 'push with dry-run' '\n \n \tmk_test heads/master &&\n"},{"id":"69386","messageId":"7vwsoz5w9e.fsf@gitster.siamese.dyndns.org","threadId":"12212","inReplyTo":"alpine.LNX.1.00.0802201226560.19024@iabervon.org","subject":"Re: [PATCH] fix \"git push $there +HEAD\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-20T18:16:13Z","receivedAt":"2008-02-20T18:16:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> ..., I think it would be better to have \"\" \n> treated as \":>\" instead of as \":\" in \n> match_refs(); and not do any special rewriting on the front end.\n\nWonderful. That's hooking at the right layer.\n"}]}