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

From: Junio C Hamano <[email-masked]>
Date: Tue, 17 Oct 2017 16:05:43 +0900

Jonathan Nieder <[email-masked]> writes:

> And in that spirit, I think the patch you replied with aims to go in
> the right direction, by providing the core functionality when in a
> repository while avoiding breaking such a script outside of one
> (though I do not understand it fully yet).

Given that, is it safe for me to ignore this earlier one

> For what it's worth, I don't agree with this repurposing of
> "check-ref-format --branch" at all.

as reacting to the patch without reading what it does?

>>                                        (e.g. a wrapper to "git
>> clone" that wants to verify the name it is going to give to the "-b"
>> option), and a check desired in such a context is different from
>> (and is stricter than) feeding refs/heads/$name to the same command
>> without the "--branch" option.
>
> Can you say more about this example?  E.g. why is this hypothetical
> wrapper unable to rely on "git clone -b"'s own error handling?

If I have to guess what you meant, perhaps the Porcelain wanted to
diagnose bad parameter that would be rejected _before_ letting clone
spend cycles and possibly network bandwidth?

But this was my way of rephrasing an earlier example you used while
objecting to Peff's change

    For example, if you have a script with an $opt_branch variable
    that defaults to "master", it may do

            resolved_branch=$(git check-ref-format --branch "$opt_branch")

    even though it is in a mode that not going to have to use
    $resolved_branch and it is not running from a repository.

so my answer to the question in the message I am directly responding
to would be "You tell me." ;-)

> --symbolic-full-name seems like a good fit.  Do you remember why
> check-ref-format was introduced instead?

It really serves two purposes, and at this point, I do not think it
really matters why it also does the @{-1} expansion as well as name
validation.  604e0cb5 ("Documentation: describe check-ref-format
--branch", 2009-10-12) happened 8 years ago, and since then it has
been advertised long enough as if the 80% primary purpose of "c-r-f
--branch" were to expand @{-1} to a branch name; even though the
text hints that it also does the usual checks, by definition
validation of the result of expanding @{-1} ought to succeed; after
all that was the branch we _were_ on ;-).