On Sat, 2017-12-02 at 17:52 -0800, Junio C Hamano wrote:
> Kaartic Sivaraam <kaartic.sivaraam@xxxxxxxxx> writes:
>
> > > I have a mild suspicion that "git checkout -B @{-1}" would want to
> > > error out instead of creating a valid new branch whose name is
> > > 40-hex that happen to be the name of the commit object you were
> > > detached at previously.
> >
> > I thought this the other way round. Rather than letting the callers
> > error out when @{-N} didn't expand to a branch name, I thought we
> > should not be expanding @{-N} syntax for "check-ref-format --branch" at
> > all to make a "stronger guarantee" that the result is "always" a valid
> > branch name. Then I thought it might be too restrictive and didn't
> > mention it. So, I dunno.
> >
OK. Seems I was out of mind in the above reply. I have answered the
question about whether "branch=$(git check-ref-format @{-1}) && git
checkout -B $branch" should fail or not. Sorry, for the confusion in
case you mind it.
> >
> > > I am not sure if "check-ref-format --branch" should the same; it is
> > > more about the syntax and the 40-hex _is_ valid there, so...
> >
> > I'm not sure what you were trying to say here, sorry.
>
> The "am not sure if" comes from this question: should these two be
> equivalent?
>
> $ git check-ref-format --branch @{-1}
> $ git check-ref-format --branch $(git rev-parse --verify @{-1})
>
I could see how,
$ git rev-parse --verify @{-1}
$ git rev-parse --verify $(git check-ref-format --branch @{-1})
could be equivalent. I'm not sure how the previous two might be
equivalent except in the case when the previous thing checked out was
not a branch.
1. "git check-ref-format --branch @{-1}" returns the previous thing
checked out which might be a branch name or a commit hash
2. "git check-ref-format --branch $(git rev-parse --verify @{-1})"
returns the commit hash of the previous thing (the hash of the tip if
was a branch). IIUC, this thing never returns a branch name.
> If they should be equivalent (and I think the current implementation
> says they are), then the answer to "if ... should do the same?"
> becomes "no, we should not error out".
>
> Stepping back a bit, the mild suspicion above says
>
> $ git checkout HEAD^0
> ... do things ...
> $ git checkout -b temp
> ... do more things ...
> $ git checkout -B @{-1}
>
> that creates a new branch whose name is 40-hex of a commit that
> happens to be where we started the whole dance *is* a bug. No sane
> user expects that to happen, and the last step "checkout -B @{-1}"
> should result in an error instead [*1*].
>
> I was wondering if "git check-ref-format --branch @{-1}", when used
> in place of "checkout -B @{-1}" in the above sequence,
I guess you mean '... "git checkout -B $(git check-ref-format --branch
@{-1}", when used in place of "git checkout -B @{-1}" ...' ?
> should or
> should not fail. It really boils down to this question: what @{-1}
> expands to and what the user wants to do with it.
>
> In the context of getting a revision (i.e. "rev-parse @{-1}") where
> we are asking what the object name is, the value of the detached
> HEAD we were on previously is a valid answer we are "expanding @{-1}
> to". If we were on a concrete branch and @{-1} yields a concrete
> branch name, then rev-parse would turn that into an object name, and
> in the end, in either case, the object name is what we wanted to
> get. So we do not want to error this out.
>
Right.
> But a user of "git check-ref-format --branch" is not asking about
> the object name ("git rev-parse" would have been used otherwise).
> Which argues for erroring out "check-ref-format --branch @{-1}" if
> we were not on a branch in the previous state.
>
Exactly what I thought.
> And that argues for erroring out "check-ref-format --branch @{-1}"
> in such a case, i.e. declaring that the first two commands in this
> message are not equivalent.
>
As said before, IIUC, they give equivalent output to stdout only when
the previous thing checked out was not a branch.
--
Kaartic
