{"thread":{"id":"59021","subject":"[PATCH] get_oid_basic: detached HEAD and @{-1}","startedAt":"2022-12-31T14:13:47Z","lastAt":"2022-12-31T14:13:47Z","messageCount":1,"participants":["Rubén Justo"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"469706","messageId":"c18c0328-9292-aa02-2922-f139e578ac07@gmail.com","threadId":"59021","inReplyTo":null,"subject":"[PATCH] get_oid_basic: detached HEAD and @{-1}","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2022-12-31T14:13:36Z","receivedAt":"2022-12-31T14:13:47Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"We discourage the use of branch names that resemble commit ids; but\nfor convenience not as a limitation:\n\n\t$ git commit --allow-empty -m first\n\t[... 000001] first\n\t$ oid=$(git rev-parse HEAD)\n\t$ git commit --allow-empty -m second\n\t[... 000002] second\n\t$ git checkout -b $oid\n\tSwitched to a new branch '000001...'\n\nWith this usage, removing that risky-named branch and trying to re-use\nthe branch name again, the result expected is an error but:\n\n\t$ git checkout -\n\t$ git branch -d $oid\n\tDeleted branch 000001... (was 000002).\n\t$ git checkout $oid\n\t... You are in 'detached HEAD' state...\n\tHEAD is now at 000001...\n\nWith @{-1} shortcuts the result is the same, but here we can do better\nand give an error as a result:  from the reflog we know @{-1} refers\nto a branch named '000001...' which pointed to commit '000002...'.\n\nLet's use this unused-yet information to avoid the ambiguity in the\nresult.\n\nSigned-off-by: Rubén Justo <rjusto@gmail.com>\n---\n object-name.c                         | 20 ++++++++++++++------\n t/t3204-branch-name-interpretation.sh | 10 ++++++++++\n 2 files changed, 24 insertions(+), 6 deletions(-)\n\ndiff --git a/object-name.c b/object-name.c\nindex 2dd1a0f56e..3fae19de2f 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -878,7 +878,8 @@ static inline int push_mark(const char *string, int len)\n }\n \n static enum get_oid_result get_oid_1(struct repository *r, const char *name, int len, struct object_id *oid, unsigned lookup_flags);\n-static int interpret_nth_prior_checkout(struct repository *r, const char *name, int namelen, struct strbuf *buf);\n+static int interpret_nth_prior_checkout(struct repository *r, const char *name,\n+\t\tint namelen, struct strbuf *buf, struct object_id *ooid);\n \n static int get_oid_basic(struct repository *r, const char *str, int len,\n \t\t\t struct object_id *oid, unsigned int flags)\n@@ -940,10 +941,12 @@ static int get_oid_basic(struct repository *r, const char *str, int len,\n \n \tif (nth_prior) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstruct object_id ooid;\n \t\tint detached;\n \n-\t\tif (interpret_nth_prior_checkout(r, str, len, &buf) > 0) {\n-\t\t\tdetached = (buf.len == r->hash_algo->hexsz && !get_oid_hex(buf.buf, oid));\n+\t\tif (interpret_nth_prior_checkout(r, str, len, &buf, &ooid) > 0) {\n+\t\t\tdetached = (buf.len == r->hash_algo->hexsz &&\n+\t\t\t\t!get_oid_hex(buf.buf, oid)) && !oidcmp(oid, &ooid);\n \t\t\tstrbuf_release(&buf);\n \t\t\tif (detached)\n \t\t\t\treturn 0;\n@@ -1383,9 +1386,10 @@ static int get_oid_oneline(struct repository *r,\n struct grab_nth_branch_switch_cbdata {\n \tint remaining;\n \tstruct strbuf *sb;\n+\tstruct object_id *ooid;\n };\n \n-static int grab_nth_branch_switch(struct object_id *ooid UNUSED,\n+static int grab_nth_branch_switch(struct object_id *ooid,\n \t\t\t\t  struct object_id *noid UNUSED,\n \t\t\t\t  const char *email UNUSED,\n \t\t\t\t  timestamp_t timestamp UNUSED,\n@@ -1405,6 +1409,8 @@ static int grab_nth_branch_switch(struct object_id *ooid UNUSED,\n \t\tlen = target - match;\n \t\tstrbuf_reset(cb->sb);\n \t\tstrbuf_add(cb->sb, match, len);\n+\t\tif (cb->ooid)\n+\t\t\toidcpy(cb->ooid, ooid);\n \t\treturn 1; /* we are done */\n \t}\n \treturn 0;\n@@ -1416,7 +1422,8 @@ static int grab_nth_branch_switch(struct object_id *ooid UNUSED,\n  */\n static int interpret_nth_prior_checkout(struct repository *r,\n \t\t\t\t\tconst char *name, int namelen,\n-\t\t\t\t\tstruct strbuf *buf)\n+\t\t\t\t\tstruct strbuf *buf,\n+\t\t\t\t\tstruct object_id *ooid)\n {\n \tlong nth;\n \tint retval;\n@@ -1438,6 +1445,7 @@ static int interpret_nth_prior_checkout(struct repository *r,\n \t\treturn -1;\n \tcb.remaining = nth;\n \tcb.sb = buf;\n+\tcb.ooid = ooid;\n \n \tretval = refs_for_each_reflog_ent_reverse(get_main_ref_store(r),\n \t\t\t\"HEAD\", grab_nth_branch_switch, &cb);\n@@ -1621,7 +1629,7 @@ int repo_interpret_branch_name(struct repository *r,\n \t\tnamelen = strlen(name);\n \n \tif (!options->allowed || (options->allowed & INTERPRET_BRANCH_LOCAL)) {\n-\t\tlen = interpret_nth_prior_checkout(r, name, namelen, buf);\n+\t\tlen = interpret_nth_prior_checkout(r, name, namelen, buf, NULL);\n \t\tif (!len) {\n \t\t\treturn len; /* syntax Ok, not enough switches */\n \t\t} else if (len > 0) {\ndiff --git a/t/t3204-branch-name-interpretation.sh b/t/t3204-branch-name-interpretation.sh\nindex 3399344f25..5839884ae4 100755\n--- a/t/t3204-branch-name-interpretation.sh\n+++ b/t/t3204-branch-name-interpretation.sh\n@@ -167,4 +167,14 @@ test_expect_success 'modify branch upstream via \"@{-1}\" and \"@{-1}@{upstream}\"'\n \ttest_must_fail git config branch.upstream-other.merge\n '\n \n+test_expect_success '@{-1} might look erroneously like a detached HEAD' '\n+\toid=$(git rev-parse HEAD) &&\n+\tgit checkout -b $oid &&\n+\ttest_commit new-oid &&\n+\tgit checkout - &&\n+\tgit branch -D $oid &&\n+\ttest_must_fail git checkout @{-1} &&\n+\ttest_must_fail git rev-parse @{-1}\n+'\n+\n test_done\n-- \n2.39.0\n\n"}]}