{"thread":{"id":"59360","subject":"[PATCH RESEND 0/2] status: improve info for detached HEAD","startedAt":"2023-03-08T19:21:25Z","lastAt":"2023-03-10T16:29:26Z","messageCount":5,"participants":["Roy Eldar","Junio C Hamano","Roy E"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"473212","messageId":"20230308192050.1291-1-royeldar0@gmail.com","threadId":"59360","inReplyTo":null,"subject":"[PATCH RESEND 0/2] status: improve info for detached HEAD","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2023-03-08T19:20:48Z","receivedAt":"2023-03-08T19:21:25Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When a repository is cloned using \"git clone -b\" and a tag is specified,\nHEAD is automatically detached. As a result, \"git status\" shows\n\"currently not on any branch\", which is not very useful.\n\nTeach \"git status\" to generate the \"HEAD detached at\" message in this\ncase as well, in a similar way to when a tag is checked out.\n\nIn the case of \"git checkout\", the name of the ref that was checked out\nis retrieved from the reflog; for \"git clone\", the name of the ref isn't\npresent in the reflog entry, so we use the abbreviated hash instead.\nThis is also consistent with the \"detached HEAD\" advice.\n\nRoy Eldar (2):\n t7508: test status output for detached HEAD after clone\n status: improve info for detached HEAD after clone\n\n t/t7508-status.sh | 12 ++++++++++++\n wt-status.c | 7 +++++++\n 2 files changed, 19 insertions(+)\n\n-- \n2.30.2\n\n"},{"id":"473213","messageId":"20230308192050.1291-2-royeldar0@gmail.com","threadId":"59360","inReplyTo":"20230308192050.1291-1-royeldar0@gmail.com","subject":"[PATCH RESEND 1/2] t7508: test status output for detached HEAD after clone","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2023-03-08T19:20:49Z","receivedAt":"2023-03-08T19:21:36Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"After cloning a repository, HEAD might be detached: for example, when\n\"--branch\" specifies a non-branch (e.g. a tag). In this case, running\n\"git status\" prints 'Not currently on any branch'.\n\nSigned-off-by: Roy Eldar \n---\n t/t7508-status.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex aed07c5b62..d279157d28 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -885,6 +885,18 @@ test_expect_success 'status shows detached HEAD properly after checking out non-\n \tgrep -E \"HEAD detached at [0-9a-f]+\" actual\n '\n \n+test_expect_success 'status shows detached HEAD properly after cloning a repository' '\n+\ttest_when_finished rm -rf upstream downstream actual &&\n+\n+\tgit init upstream &&\n+\ttest_commit -C upstream foo &&\n+\tgit -C upstream tag test_tag &&\n+\n+\tgit clone -b test_tag upstream downstream &&\n+\tgit -C downstream status >actual &&\n+\tgrep -E \"Not currently on any branch.\" actual\n+'\n+\n test_expect_success 'setup status submodule summary' '\n \ttest_create_repo sm && (\n \t\tcd sm &&\n-- \n2.30.2\n\n"},{"id":"473214","messageId":"20230308192050.1291-3-royeldar0@gmail.com","threadId":"59360","inReplyTo":"20230308192050.1291-1-royeldar0@gmail.com","subject":"[PATCH RESEND 2/2] status: improve info for detached HEAD after clone","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2023-03-08T19:20:50Z","receivedAt":"2023-03-08T19:22:03Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When a remote ref or a tag is checked out, HEAD is automatically\ndetached, and \"git status\" says 'HEAD detached at ...', instead of\n'Not currently on any branch.'; this is done by traversing the reflog\nand parsing an entry like 'checkout: moving from ... to ...'.\n\nIn certain situations, HEAD can be detached after \"git clone\": for\nexample, when \"--branch\" specifies a non-branch (e.g. a tag). It is\npreferable to avoid displaying 'Not currently on any branch.', so\n'HEAD detached at $sha1' is shown instead.\n\nSigned-off-by: Roy Eldar \n---\n t/t7508-status.sh | 2 +-\n wt-status.c | 7 +++++++\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex d279157d28..0ab5bdc1e0 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -894,7 +894,7 @@ test_expect_success 'status shows detached HEAD properly after cloning a reposit\n \n \tgit clone -b test_tag upstream downstream &&\n \tgit -C downstream status >actual &&\n-\tgrep -E \"Not currently on any branch.\" actual\n+\tgrep -E \"HEAD detached at [0-9a-f]+\" actual\n '\n \n test_expect_success 'setup status submodule summary' '\ndiff --git a/wt-status.c b/wt-status.c\nindex 3162241a57..f0a5fb578a 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1632,6 +1632,13 @@ static int grab_1st_switch(struct object_id *ooid UNUSED,\n \tstruct grab_1st_switch_cbdata *cb = cb_data;\n \tconst char *target = NULL, *end;\n \n+\tif (skip_prefix(message, \"clone: from \", &message)) {\n+\t\toidcpy(&cb->noid, noid);\n+\t\tstrbuf_reset(&cb->buf);\n+\t\tstrbuf_add_unique_abbrev(&cb->buf, noid, DEFAULT_ABBREV);\n+\t\treturn 1;\n+\t}\n+\n \tif (!skip_prefix(message, \"checkout: moving from \", &message))\n \t\treturn 0;\n \ttarget = strstr(message, \" to \");\n-- \n2.30.2\n\n"},{"id":"473220","messageId":"xmqqsfefrn4q.fsf@gitster.g","threadId":"59360","inReplyTo":"20230308192050.1291-3-royeldar0@gmail.com","subject":"Re: [PATCH RESEND 2/2] status: improve info for detached HEAD after clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-08T20:42:13Z","receivedAt":"2023-03-08T20:42:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roy Eldar writes:\n\n> diff --git a/wt-status.c b/wt-status.c\n> index 3162241a57..f0a5fb578a 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1632,6 +1632,13 @@ static int grab_1st_switch(struct object_id *ooid UNUSED,\n> \tstruct grab_1st_switch_cbdata *cb = cb_data;\n> \tconst char *target = NULL, *end;\n> \n> +\tif (skip_prefix(message, \"clone: from \", &message)) {\n> +\t\toidcpy(&cb->noid, noid);\n> +\t\tstrbuf_reset(&cb->buf);\n> +\t\tstrbuf_add_unique_abbrev(&cb->buf, noid, DEFAULT_ABBREV);\n> +\t\treturn 1;\n> +\t}\n> +\n> \tif (!skip_prefix(message, \"checkout: moving from \", &message))\n> \t\treturn 0;\n> \ttarget = strstr(message, \" to \");\n\nTwo comments:\n\n - The original seems to duplicate the logic already in use for\n parsing @{-1} in object-name.c::grab_nth_branch_switch().\n\n - Adding new code here would mean that the result of parsing @{-1}\n and what wt_status_get_detached_from() will report becomes\n inconsistent, no? After such a clone, \"git checkout @{-1}\" would\n say \"there is no @{-1}\" but \"git status\" would say it was\n detached from some hexadecimal object.\n\nThinking about the latter, I think it does not add much value to say\nthat we detached from something that is not a ref, so not adding\n\"clone: from \" logic to grab_nth_branch_switch() is probably the\nright thing to do anyway.\n\nBut then does it even make sense to have this new logic here?\n\nYes, the head may be detached at some object that is not a local or\nremote branch. But what is so bad about reporting the fact\nfaithfully, i.e., that we are not on any branch? What commit object\nwe are at can be seen by \"git show\" or \"git rev-parse HEAD\" or any\nother usual ways anyway, so...\n\nI personally do not very much appreciate the extra info that is\ngiven by saying \"HEAD detached at X\" and \"HEAD detached from X\",\ncompared to saying just \"Not currently on any branch\", especially\nwhen these X are not concrete branch names or tag names but just\nhexadecimal string that needs to be fed to \"git describe\" to be\nturned into something that makes sense to humans, and that is\nprobably the reason why I am not a good judge about the change this\npatch makes. Others who like the \"detached at/from X\" may be better\njudges to decide if this change makes sense.\n\nThanks.\n\n"},{"id":"473357","messageId":"CAOfFamk5wWFuUgn4uEo0JV0siLzH=ybDB_Yr-9oL3OM4taozuA@mail.gmail.com","threadId":"59360","inReplyTo":"xmqqsfefrn4q.fsf@gitster.g","subject":"Re: [PATCH RESEND 2/2] status: improve info for detached HEAD after clone","fromName":"Roy E","fromEmail":"royeldar0@gmail.com","sentAt":"2023-03-10T16:25:26Z","receivedAt":"2023-03-10T16:29:26Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Hi,\n\nFirst of all, thanks for the thoughtful response.\n\nJunio C Hamano writes:\n\n> - Adding new code here would mean that the result of parsing @{-1}\n> and what wt_status_get_detached_from() will report becomes\n> inconsistent, no?\n\nIf I understand correctly, the result of parsing @{-1} is the commit\nchecked out before the current one, so grab_nth_branch_switch() gets\nthe commit we've moved _from_, whereas wt-status::grab_1st_switch()\ngets the commit we've moved _to_. After a clone, there is no commit\nwe've moved _from_.\n\n> Yes, the head may be detached at some object that is not a local or\n> remote branch. But what is so bad about reporting the fact\n> faithfully, i.e., that we are not on any branch?\n\nI thought that we try to avoid showing \"Not currently on any branch\"\nas this message is not very user-friendly (see commit b397ea4).\nFurthermore, showing \"HEAD detached at X\" where X is the abbreviated\nhash is more consistent with the behavior of the detached HEAD advice\nin \"git clone\", which says\n\n Note: switching to 'X'\n\n> I personally do not very much appreciate the extra info that is\n> given by saying \"HEAD detached at X\" and \"HEAD detached from X\",\n> compared to saying just \"Not currently on any branch\", especially\n> when these X are not concrete branch names or tag names but just\n> hexadecimal string that needs to be fed to \"git describe\" to be\n> turned into something that makes sense to humans\n\nIt might be better to show \"HEAD detached at X\" where X is the concrete\ntag name which was cloned; but since \"grab_1st_switch\" digs in the\nreflog for that information, one cannot figure out the tag name that\nwas used when the repository was cloned. I didn't want to complicate\nthe current logic too much, and IMHO showing the abbreviated hash is\nthe best thing we can do, and it is already what we do in certain cases\n(e.g. after \"git checkout --detach\").\n"}]}