{"thread":{"id":"48974","subject":"[PATCH] refspec: allow @ on the left-hand side of refspecs","startedAt":"2018-07-29T19:28:12Z","lastAt":"2018-07-31T16:02:18Z","messageCount":4,"participants":["brian m. carlson","Brandon Williams"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"353833","messageId":"20180729192803.1047050-1-sandals@crustytoothpaste.net","threadId":"48974","inReplyTo":null,"subject":"[PATCH] refspec: allow @ on the left-hand side of refspecs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-29T19:28:03Z","receivedAt":"2018-07-29T19:28:12Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The object ID parsing machinery is aware of \"@\" as a synonym for \"HEAD\"\nand this is documented accordingly in gitrevisions(7).  The push\ndocumentation describes the source portion of a refspec as \"any\narbitrary 'SHA-1 expression'\"; however, \"@\" is not allowed on the\nleft-hand side of a refspec, since we attempt to check for it being a\nvalid ref name and fail (since it is not).\n\nTeach the refspec machinery about this alias and silently substitute\n\"HEAD\" when we see \"@\".  This handles the fact that HEAD is a symref and\npreserves its special behavior.  We need not handle other arbitrary\nobject ID expressions (such as \"@^\") when pushing because the revision\nmachinery already handles that for us.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\nI probably type \"git push upstream HEAD\" from five to thirty times a\nday, many of those where I typo \"HEAD\", so I decided to implement the\nshorter form.  This design handles @ as HEAD in both fetch and push,\nwhereas alternate solutions would not.\n\ncheck_refname_format explicitly rejects \"@\"; I tried at first to simply\nignore that with a flag, but we end up calling that from several other\nplaces in the codebase and rejecting it and all of those places would\nhave needed updating.\n\nI thought about putting the if/else logic in a function, but since it's\njust four lines, I decided not to.  However, if people think it would be\ntidier, I can do so.\n\nNote that the test portion of the patch is best read with git diff -w;\nthe current version is very noisy.\n\n refspec.c             |   6 ++-\n t/t5516-fetch-push.sh | 104 +++++++++++++++++++++---------------------\n 2 files changed, 58 insertions(+), 52 deletions(-)\n\ndiff --git a/refspec.c b/refspec.c\nindex e8010dce0c..57c2f65104 100644\n--- a/refspec.c\n+++ b/refspec.c\n@@ -62,8 +62,12 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet\n \t\treturn 0;\n \t}\n \n+\tif (llen == 1 && lhs[0] == '@')\n+\t\titem->src = xstrdup(\"HEAD\");\n+\telse\n+\t\titem->src = xstrndup(lhs, llen);\n+\n \titem->pattern = is_glob;\n-\titem->src = xstrndup(lhs, llen);\n \tflags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);\n \n \tif (fetch) {\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex a5077d8b7c..cbccbd2f8d 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -436,70 +436,72 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '\n \n '\n \n-test_expect_success 'push with HEAD' '\n+for ref in HEAD @\n+do\n+\ttest_expect_success \"push with $ref\" '\n \n-\tmk_test testrepo heads/master &&\n-\tgit checkout master &&\n-\tgit push testrepo HEAD &&\n-\tcheck_push_result testrepo $the_commit heads/master\n+\t\tmk_test testrepo heads/master &&\n+\t\tgit checkout master &&\n+\t\tgit push testrepo $ref &&\n+\t\tcheck_push_result testrepo $the_commit heads/master\n \n-'\n+\t'\n \n-test_expect_success 'push with HEAD nonexisting at remote' '\n+\ttest_expect_success \"push with $ref nonexisting at remote\" '\n \n-\tmk_test testrepo heads/master &&\n-\tgit checkout -b local master &&\n-\tgit push testrepo HEAD &&\n-\tcheck_push_result testrepo $the_commit heads/local\n-'\n+\t\tmk_test testrepo heads/master &&\n+\t\tgit checkout -B local master &&\n+\t\tgit push testrepo $ref &&\n+\t\tcheck_push_result testrepo $the_commit heads/local\n+\t'\n \n-test_expect_success 'push with +HEAD' '\n+\ttest_expect_success \"push with +$ref\" '\n \n-\tmk_test testrepo 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 testrepo $the_commit heads/master &&\n-\tcheck_push_result testrepo $the_commit heads/local &&\n+\t\tmk_test testrepo heads/master &&\n+\t\tgit checkout master &&\n+\t\tgit branch -D local &&\n+\t\tgit checkout -b local &&\n+\t\tgit push testrepo master local &&\n+\t\tcheck_push_result testrepo $the_commit heads/master &&\n+\t\tcheck_push_result testrepo $the_commit heads/local &&\n \n-\t# Without force rewinding should fail\n-\tgit reset --hard HEAD^ &&\n-\ttest_must_fail git push testrepo HEAD &&\n-\tcheck_push_result testrepo $the_commit heads/local &&\n+\t\t# Without force rewinding should fail\n+\t\tgit reset --hard HEAD^ &&\n+\t\ttest_must_fail git push testrepo $ref &&\n+\t\tcheck_push_result testrepo $the_commit heads/local &&\n \n-\t# With force rewinding should succeed\n-\tgit push testrepo +HEAD &&\n-\tcheck_push_result testrepo $the_first_commit heads/local\n+\t\t# With force rewinding should succeed\n+\t\tgit push testrepo +$ref &&\n+\t\tcheck_push_result testrepo $the_first_commit heads/local\n \n-'\n+\t'\n \n-test_expect_success 'push HEAD with non-existent, incomplete dest' '\n+\ttest_expect_success \"push $ref with non-existent, incomplete dest\" '\n \n-\tmk_test testrepo &&\n-\tgit checkout master &&\n-\tgit push testrepo HEAD:branch &&\n-\tcheck_push_result testrepo $the_commit heads/branch\n+\t\tmk_test testrepo &&\n+\t\tgit checkout master &&\n+\t\tgit push testrepo $ref:branch &&\n+\t\tcheck_push_result testrepo $the_commit heads/branch\n \n-'\n+\t'\n \n-test_expect_success 'push with config remote.*.push = HEAD' '\n-\n-\tmk_test testrepo heads/local &&\n-\tgit checkout master &&\n-\tgit branch -f local $the_commit &&\n-\t(\n-\t\tcd testrepo &&\n-\t\tgit checkout local &&\n-\t\tgit reset --hard $the_first_commit\n-\t) &&\n-\ttest_config remote.there.url testrepo &&\n-\ttest_config remote.there.push HEAD &&\n-\ttest_config branch.master.remote there &&\n-\tgit push &&\n-\tcheck_push_result testrepo $the_commit heads/master &&\n-\tcheck_push_result testrepo $the_first_commit heads/local\n-'\n+\ttest_expect_success \"push with config remote.*.push = $ref\" '\n+\t\tmk_test testrepo heads/local &&\n+\t\tgit checkout master &&\n+\t\tgit branch -f local $the_commit &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit checkout local &&\n+\t\t\tgit reset --hard $the_first_commit\n+\t\t) &&\n+\t\ttest_config remote.there.url testrepo &&\n+\t\ttest_config remote.there.push $ref &&\n+\t\ttest_config branch.master.remote there &&\n+\t\tgit push &&\n+\t\tcheck_push_result testrepo $the_commit heads/master &&\n+\t\tcheck_push_result testrepo $the_first_commit heads/local\n+\t'\n+done\n \n test_expect_success 'push with remote.pushdefault' '\n \tmk_test up_repo heads/master &&\n"},{"id":"353919","messageId":"20180730175051.GA154732@google.com","threadId":"48974","inReplyTo":"20180729192803.1047050-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH] refspec: allow @ on the left-hand side of refspecs","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-07-30T17:50:51Z","receivedAt":"2018-07-30T17:50:56Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 07/29, brian m. carlson wrote:\n> The object ID parsing machinery is aware of \"@\" as a synonym for \"HEAD\"\n> and this is documented accordingly in gitrevisions(7).  The push\n> documentation describes the source portion of a refspec as \"any\n> arbitrary 'SHA-1 expression'\"; however, \"@\" is not allowed on the\n> left-hand side of a refspec, since we attempt to check for it being a\n> valid ref name and fail (since it is not).\n> \n> Teach the refspec machinery about this alias and silently substitute\n> \"HEAD\" when we see \"@\".  This handles the fact that HEAD is a symref and\n> preserves its special behavior.  We need not handle other arbitrary\n> object ID expressions (such as \"@^\") when pushing because the revision\n> machinery already handles that for us.\n\nSo this claims that using \"@^\" should work despite not accounting for it\nexplicitly or am I misreading?  Unless I'm mistaken, it looks like we\ndon't really support arbitrary rev syntax in refspecs since \"HEAD^\"\ndoesn't work either.\n\n> \n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> I probably type \"git push upstream HEAD\" from five to thirty times a\n> day, many of those where I typo \"HEAD\", so I decided to implement the\n> shorter form.  This design handles @ as HEAD in both fetch and push,\n> whereas alternate solutions would not.\n\nI'm always a fan of finding shortcuts and reducing how much I type, so\nthank you :)\n\n> \n> check_refname_format explicitly rejects \"@\"; I tried at first to simply\n> ignore that with a flag, but we end up calling that from several other\n> places in the codebase and rejecting it and all of those places would\n> have needed updating.\n> \n> I thought about putting the if/else logic in a function, but since it's\n> just four lines, I decided not to.  However, if people think it would be\n> tidier, I can do so.\n> \n> Note that the test portion of the patch is best read with git diff -w;\n> the current version is very noisy.\n> \n>  refspec.c             |   6 ++-\n>  t/t5516-fetch-push.sh | 104 +++++++++++++++++++++---------------------\n>  2 files changed, 58 insertions(+), 52 deletions(-)\n> \n> diff --git a/refspec.c b/refspec.c\n> index e8010dce0c..57c2f65104 100644\n> --- a/refspec.c\n> +++ b/refspec.c\n> @@ -62,8 +62,12 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet\n>  \t\treturn 0;\n>  \t}\n>  \n> +\tif (llen == 1 && lhs[0] == '@')\n> +\t\titem->src = xstrdup(\"HEAD\");\n> +\telse\n> +\t\titem->src = xstrndup(lhs, llen);\n> +\n\nThis is probably the easiest place to put the aliasing logic so I don't\nhave any issue with including it here.\n\n-- \nBrandon Williams\n"},{"id":"353976","messageId":"20180730231451.GG945730@genre.crustytoothpaste.net","threadId":"48974","inReplyTo":"20180730175051.GA154732@google.com","subject":"Re: [PATCH] refspec: allow @ on the left-hand side of refspecs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-30T23:14:51Z","receivedAt":"2018-07-30T23:15:02Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Jul 30, 2018 at 10:50:51AM -0700, Brandon Williams wrote:\n> On 07/29, brian m. carlson wrote:\n> > The object ID parsing machinery is aware of \"@\" as a synonym for \"HEAD\"\n> > and this is documented accordingly in gitrevisions(7).  The push\n> > documentation describes the source portion of a refspec as \"any\n> > arbitrary 'SHA-1 expression'\"; however, \"@\" is not allowed on the\n> > left-hand side of a refspec, since we attempt to check for it being a\n> > valid ref name and fail (since it is not).\n> > \n> > Teach the refspec machinery about this alias and silently substitute\n> > \"HEAD\" when we see \"@\".  This handles the fact that HEAD is a symref and\n> > preserves its special behavior.  We need not handle other arbitrary\n> > object ID expressions (such as \"@^\") when pushing because the revision\n> > machinery already handles that for us.\n> \n> So this claims that using \"@^\" should work despite not accounting for it\n> explicitly or am I misreading?  Unless I'm mistaken, it looks like we\n> don't really support arbitrary rev syntax in refspecs since \"HEAD^\"\n> doesn't work either.\n\nCorrect, it does indeed work, at least for me:\n\ngenre ok % git push castro HEAD^:refs/heads/temp\nTotal 0 (delta 0), reused 0 (delta 0)\nTo https://git.crustytoothpaste.net/git/bmc/git.git\n * [new branch]            HEAD^ -> temp\n\ngenre ok % git push castro @^:refs/heads/temp\nTotal 0 (delta 0), reused 0 (delta 0)\nTo https://git.crustytoothpaste.net/git/bmc/git.git\n * [new branch]            @^ -> temp\n\nNote that in this case, I had to specify a full ref since it didn't\nexist on the remote and the left side wasn't a ref name.\n\nNow it doesn't work for fetches, only pushes.  Only the left side of a\npush refspec can be an arbitrary expression.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"354050","messageId":"20180731160213.GA192506@google.com","threadId":"48974","inReplyTo":"20180730231451.GG945730@genre.crustytoothpaste.net","subject":"Re: [PATCH] refspec: allow @ on the left-hand side of refspecs","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-07-31T16:02:13Z","receivedAt":"2018-07-31T16:02:18Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 07/30, brian m. carlson wrote:\n> On Mon, Jul 30, 2018 at 10:50:51AM -0700, Brandon Williams wrote:\n> > On 07/29, brian m. carlson wrote:\n> > > The object ID parsing machinery is aware of \"@\" as a synonym for \"HEAD\"\n> > > and this is documented accordingly in gitrevisions(7).  The push\n> > > documentation describes the source portion of a refspec as \"any\n> > > arbitrary 'SHA-1 expression'\"; however, \"@\" is not allowed on the\n> > > left-hand side of a refspec, since we attempt to check for it being a\n> > > valid ref name and fail (since it is not).\n> > > \n> > > Teach the refspec machinery about this alias and silently substitute\n> > > \"HEAD\" when we see \"@\".  This handles the fact that HEAD is a symref and\n> > > preserves its special behavior.  We need not handle other arbitrary\n> > > object ID expressions (such as \"@^\") when pushing because the revision\n> > > machinery already handles that for us.\n> > \n> > So this claims that using \"@^\" should work despite not accounting for it\n> > explicitly or am I misreading?  Unless I'm mistaken, it looks like we\n> > don't really support arbitrary rev syntax in refspecs since \"HEAD^\"\n> > doesn't work either.\n> \n> Correct, it does indeed work, at least for me:\n> \n> genre ok % git push castro HEAD^:refs/heads/temp\n> Total 0 (delta 0), reused 0 (delta 0)\n> To https://git.crustytoothpaste.net/git/bmc/git.git\n>  * [new branch]            HEAD^ -> temp\n> \n> genre ok % git push castro @^:refs/heads/temp\n> Total 0 (delta 0), reused 0 (delta 0)\n> To https://git.crustytoothpaste.net/git/bmc/git.git\n>  * [new branch]            @^ -> temp\n> \n> Note that in this case, I had to specify a full ref since it didn't\n> exist on the remote and the left side wasn't a ref name.\n\nThat's what I was missing, a full refspec! Thanks for the illustration.\n\n> \n> Now it doesn't work for fetches, only pushes.  Only the left side of a\n> push refspec can be an arbitrary expression.\n> -- \n> brian m. carlson: Houston, Texas, US\n> OpenPGP: https://keybase.io/bk2204\n\n\n\n-- \nBrandon Williams\n"}]}