Re: [PATCH 1/2] sha1_name.c: signal if @{-N} was a true branch nameor a detached head

2013-05-13 Thread Jeff King
On Thu, May 09, 2013 at 10:08:24AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > Since the point of marking the detached HEAD is to turn off things like
> > "@{-1}@{u}", we would want to be generous and err on the side of
> > assuming it is a branch if it _might_ be one.
> 
> I am not sure X and Y mesh well in your "Since X, we would want Y".
> It seems to argue for erring on the side of detached HEAD to me.

Thinking on it more, I don't see that one is actually better than the
other. If you claim a detached HEAD when there isn't one, the user says
"stupid git, that was a branch, and you should tell me its upstream".
But if you claim an undetached HEAD when there isn't one, asking for the
upstream provides wildly inaccurate results (e.g., "git checkout
@{-1}@{u}" taking you somewhere unexpected).

> Checking the "from" name $HEX against old_sha1 is a local and cheap
> measure I added there for the first level of disambiguation.  If
> they do not match, we _know_ we didn't come back from a detached
> HEAD state.
> 
> In order to err on the "favor branch when it could have been one",
> you could additionally look for the reflog .git/logs/refs/heads/$HEX
> when the "from" name $HEX matches old_sha1 (which is likely to be
> detached, but it is possible that we were on the $HEX branch when
> its tip was at $HEX) and making sure the tip of that $HEX branch
> once used to be at $HEX at the time recorded for @{-N} in the HEAD
> reflog in question.

I was thinking in terms of @{-1}@{u}, so that you could say "well, do we
have upstream config for such a branch currently?". Because even though
we are digging into history (and it _may_ have been a branch at the
time, but isn't now), if we are ultimately going to ask about the
upstream config (as it is _now_, not when the entry was made), then it
does not matter if the branch was detached or not: we are still going to
return failure either way.

But there are _other_ uses for @{-1}, and I am probably being too
focused on this one use-case.

So given all of the above, I think I am fine with the direction of the
series.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] sha1_name.c: signal if @{-N} was a true branch nameor a detached head

2013-05-09 Thread Eric Sunshine
On Wed, May 8, 2013 at 5:12 PM, Junio C Hamano  wrote:
> sha1_name.c: signal if @{-N} was a true branch nameor a detached head

s/nameor/name or/

> The original API read "checkout: moving from (.*) to ..." from the
> reflog of the HEAD, and returned the substring between "from" and
> "to", but there was no way, if the substring was a 40-hex string, to
> tell if we were on a detached HEAD at that commit object, or on a
> branch whose name happened to be the 40-hex string.
>
> At this point, we cannot afford to change the format recorded in the
> reflog, so introduce a heuristics to see if the 40-hex matches the
> object name of the commit we are switching out of.  This will
> unfortunately mishandle this case:
>
> HEX=$(git rev-parse master)
> git checkout -b $HEX master
> git checkout master
>
> where we were indeed on a non-detached $HEX branch (i.e. HEAD was
> pointing at refs/heads/$HEX, not storing $HEX), of course, but
> otherwise should be fairly reliable.
>
> Signed-off-by: Junio C Hamano 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] sha1_name.c: signal if @{-N} was a true branch nameor a detached head

2013-05-09 Thread Junio C Hamano
Jeff King  writes:

> Since the point of marking the detached HEAD is to turn off things like
> "@{-1}@{u}", we would want to be generous and err on the side of
> assuming it is a branch if it _might_ be one.

I am not sure X and Y mesh well in your "Since X, we would want Y".
It seems to argue for erring on the side of detached HEAD to me.

Checking the "from" name $HEX against old_sha1 is a local and cheap
measure I added there for the first level of disambiguation.  If
they do not match, we _know_ we didn't come back from a detached
HEAD state.

In order to err on the "favor branch when it could have been one",
you could additionally look for the reflog .git/logs/refs/heads/$HEX
when the "from" name $HEX matches old_sha1 (which is likely to be
detached, but it is possible that we were on the $HEX branch when
its tip was at $HEX) and making sure the tip of that $HEX branch
once used to be at $HEX at the time recorded for @{-N} in the HEAD
reflog in question.

That is far more expensive, though; I doubt it is worth doing.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] sha1_name.c: signal if @{-N} was a true branch nameor a detached head

2013-05-08 Thread Jeff King
On Wed, May 08, 2013 at 02:12:29PM -0700, Junio C Hamano wrote:

> The original API read "checkout: moving from (.*) to ..." from the
> reflog of the HEAD, and returned the substring between "from" and
> "to", but there was no way, if the substring was a 40-hex string, to
> tell if we were on a detached HEAD at that commit object, or on a
> branch whose name happened to be the 40-hex string.
> 
> At this point, we cannot afford to change the format recorded in the
> reflog, so introduce a heuristics to see if the 40-hex matches the
> object name of the commit we are switching out of.  This will
> unfortunately mishandle this case:
> 
>   HEX=$(git rev-parse master)
>   git checkout -b $HEX master
>   git checkout master

I do not think I've ever seen a 40-hex branch name in practice, but I
would think a branch named after the commit tip would be a reasonably
common reason to have one, and would trigger this case.

Since the point of marking the detached HEAD is to turn off things like
"@{-1}@{u}", we would want to be generous and err on the side of
assuming it is a branch if it _might_ be one. IOW, shouldn't we treat
the above sequence as a branch, and therefore mishandle:

  git checkout $HEX^0 master
  git checkout master

by erroneously assuming that we moved to the branch $HEX?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html