Re: [PATCH] check-ref-format: require a repository for --branch

From: Jeff King <peff@peff.net>
Date: Aug 18, 2017, 06:20 UTC
Message-ID: <20170818062021.gqhsose5kepth55h@sigill.intra.peff.net>
In-Reply-To: <xmqqy3qhevaq.fsf@gitster.mtv.corp.google.com>

On Thu, Aug 17, 2017 at 02:30:53PM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> > I'm not sure I buy that. What does "turning it into a branch name" even
> > mean when you are not in a repository? Clearly @{-1} must fail. And
> > everything else is just going to output the exact input you provided.
> 
> This "just going to output the exact input" is not entirely correct;
> there is just one use case for it.
> 
> "git check-ref-format --branch a..b" would fail with a helpful error
> message, while the command run with "a.b" would tell you the name is
> safe.

Well, yes. It's checking the syntax, as well. But you don't need --branch for that.

Show 9 quoted lines

> Use of 'check-ref-format --branch "@{-1}"' *IS* a nonsense, whether
> it is inside or outside a repository; it is OK to fail it outside a
> repository and I would say it is even OK to fail it inside a
> repository.  After all "check-ref-format" is about checking if the
> string is a sensible name to use.
> 
> I think calling interpret_branch_name() in the codepath is a mistake
> and we should instead report if "refs/heads/@{-1}" literally is a
> valid refname we could create instead.

I don't think it's nonsense. It's the reason the feature was added in the first place. See your own a31dca0393 (check-ref-format --branch: give Porcelain a way to grok branch shorthand, 2009-03-21). Without that interpretation, it does nothing that you could not equally well do as:

  git check-ref-format refs/heads/$name

I _do_ think it's a misfeature to put it in check-ref-format. It should be part of rev-parse. Which admittedly is a kitchen sink, but this kind of resolving is one of the main things it should be doing. And in fact you can already do:

  git rev-parse --symbolic-full-name @{-1}

But I stopped short of suggesting we remove the feature entirely. It would obviously require a deprecation period.

-Peff