Re: [PATCH 1/2] sha1_name.c: signal if @{-N} was a true branch nameor a detached head
On Thu, May 09, 2013 at 10:08:24AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net 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
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
Re: [PATCH 1/2] sha1_name.c: signal if @{-N} was a true branch nameor a detached head
Jeff King p...@peff.net 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
On Wed, May 8, 2013 at 5:12 PM, Junio C Hamano gits...@pobox.com 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 gits...@pobox.com -- 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
[PATCH 1/2] sha1_name.c: signal if @{-N} was a true branch nameor a detached head
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 gits...@pobox.com --- * This is a preparatory step for the beginning of a much larger series. Peff is Cc'ed because one of the most tricky issue involves what d46a8301930a (fix parsing of @{-1}@{u} combination, 2010-01-28) did. sha1_name.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 3820f28..1473bb6 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -862,6 +862,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1, struct grab_nth_branch_switch_cbdata { int remaining; struct strbuf buf; + int detached; }; static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1, @@ -880,9 +881,14 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1, if (!match || !target) return 0; if (--(cb-remaining) == 0) { + unsigned char sha1[20]; + len = target - match; strbuf_reset(cb-buf); strbuf_add(cb-buf, match, len); + cb-detached = (len == 40 + !get_sha1_hex(match, sha1) + !hashcmp(osha1, sha1)); return 1; /* we are done */ } return 0; @@ -891,8 +897,12 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1, /* * Parse @{-N} syntax, return the number of characters parsed * if successful; otherwise signal an error with negative value. + * The string in buf.buf is either a branch name (needs to be + * prefixed with refs/heads/ if the caller wants to make it + * a fully spelled refname) or 40-hex object name of the detached + * HEAD, and *detached is set to true for the latter. */ -static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf) +static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf, int *detached) { long nth; int retval; @@ -917,6 +927,8 @@ static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf) if (0 for_each_reflog_ent_reverse(HEAD, grab_nth_branch_switch, cb)) { strbuf_reset(buf); strbuf_add(buf, cb.buf.buf, cb.buf.len); + if (detached) + *detached = cb.detached; retval = brace - name + 1; } @@ -992,7 +1004,7 @@ int interpret_branch_name(const char *name, struct strbuf *buf) char *cp; struct branch *upstream; int namelen = strlen(name); - int len = interpret_nth_prior_checkout(name, buf); + int len = interpret_nth_prior_checkout(name, buf, NULL); int tmp_len; if (!len) -- 1.8.3-rc1-182-gc61d106 -- 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