Re: [PATCH] refspec: allow @ on the left-hand side of refspecs
- From
- Brandon Williams <bmwill@google.com>
- Date
- Jul 30, 2018, 17:50 UTC
- Message-ID
- <20180730175051.GA154732@google.com>
- In-Reply-To
- <20180729192803.1047050-1-sandals@crustytoothpaste.net>
On 07/29, brian m. carlson wrote:
Show 12 quoted lines
> The object ID parsing machinery is aware of "@" as a synonym for "HEAD" > and this is documented accordingly in gitrevisions(7). The push > documentation describes the source portion of a refspec as "any > arbitrary 'SHA-1 expression'"; however, "@" is not allowed on the > left-hand side of a refspec, since we attempt to check for it being a > valid ref name and fail (since it is not). > > Teach the refspec machinery about this alias and silently substitute > "HEAD" when we see "@". This handles the fact that HEAD is a symref and > preserves its special behavior. We need not handle other arbitrary > object ID expressions (such as "@^") when pushing because the revision > machinery already handles that for us.
So this claims that using "@^" should work despite not accounting for it explicitly or am I misreading? Unless I'm mistaken, it looks like we don't really support arbitrary rev syntax in refspecs since "HEAD^" doesn't work either.
Show 7 quoted lines
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > I probably type "git push upstream HEAD" from five to thirty times a > day, many of those where I typo "HEAD", so I decided to implement the > shorter form. This design handles @ as HEAD in both fetch and push, > whereas alternate solutions would not.
I'm always a fan of finding shortcuts and reducing how much I type, so thank you :)
Show 30 quoted lines
>
> check_refname_format explicitly rejects "@"; I tried at first to simply
> ignore that with a flag, but we end up calling that from several other
> places in the codebase and rejecting it and all of those places would
> have needed updating.
>
> I thought about putting the if/else logic in a function, but since it's
> just four lines, I decided not to. However, if people think it would be
> tidier, I can do so.
>
> Note that the test portion of the patch is best read with git diff -w;
> the current version is very noisy.
>
> refspec.c | 6 ++-
> t/t5516-fetch-push.sh | 104 +++++++++++++++++++++---------------------
> 2 files changed, 58 insertions(+), 52 deletions(-)
>
> diff --git a/refspec.c b/refspec.c
> index e8010dce0c..57c2f65104 100644
> --- a/refspec.c
> +++ b/refspec.c
> @@ -62,8 +62,12 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet
> return 0;
> }
>
> + if (llen == 1 && lhs[0] == '@')
> + item->src = xstrdup("HEAD");
> + else
> + item->src = xstrndup(lhs, llen);
> +This is probably the easiest place to put the aliasing logic so I don't have any issue with including it here.
-- Brandon Williams