{"thread":{"id":"53457","subject":"[PATCH] wt-status: expand, not dwim, a \"detached from\" ref","startedAt":"2020-05-13T00:41:05Z","lastAt":"2020-09-01T22:28:30Z","messageCount":17,"participants":["Jonathan Tan","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"397648","messageId":"20200513004058.34456-1-jonathantanmy@google.com","threadId":"53457","inReplyTo":null,"subject":"[PATCH] wt-status: expand, not dwim, a \"detached from\" ref","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-05-13T00:40:58Z","receivedAt":"2020-05-13T00:41:05Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"When a user checks out the upstream branch of HEAD and then runs \"git\nbranch\", like this:\n\n  git checkout @{u}\n  git branch\n\nan error message is printed:\n\n  fatal: HEAD does not point to a branch\n\n(This error message when running \"git branch\" persists even after\nchecking out other things - it only stops after checking out a branch.)\n\nThis is because \"git branch\" attempts to DWIM \"@{u}\" when determining\nthe \"HEAD detached\" message, but that doesn't work because HEAD no\nlonger points to a branch.\n\nTherefore, when calculating the status of a worktree, only expand such a\nstring, not DWIM it. Such strings would not match a ref, and \"git\nbranch\" would report \"HEAD detached at <hash>\" instead.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\nI tried to track down other uses of wt_status_get_detached_from()\n(called from wt_status_get_state() with get_detached_from=1) but there\nseemed to be quite a few, so I stopped.\n\nOne alternative is to plumb down a flag from dwim_ref() to\ninterpret_branch_mark() that indicates that (1) don't read HEAD when\nprocessing, and (2) when encountering a ref of the form \"<optional\nref>@{u}\", don't die when the optional ref doesn't exist, is not a\nbranch, or does not have an upstream. (We cannot change the\nfunctionality outright because other parts of Git rely on such a message\nbeing printed.) But this seems too complicated just for diagnosis.\n---\n t/t3203-branch-output.sh | 10 ++++++++++\n wt-status.c              |  2 +-\n 2 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\nindex 71818b90f0..c5d3d739e5 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -209,6 +209,16 @@ EOF\n \ttest_i18ncmp expect actual\n '\n \n+test_expect_success 'git branch shows detached HEAD properly after checking out upstream branch' '\n+\tgit init upstream &&\n+\ttest_commit -C upstream foo &&\n+\n+\tgit clone upstream downstream &&\n+\tgit -C downstream checkout @{u} &&\n+\tgit -C downstream branch >actual &&\n+\ttest_i18ngrep \"HEAD detached at [0-9a-f]\\\\+\" actual\n+'\n+\n test_expect_success 'git branch `--sort` option' '\n \tcat >expect <<-\\EOF &&\n \t* (HEAD detached from fromtag)\ndiff --git a/wt-status.c b/wt-status.c\nindex 98dfa6f73f..f84ebc3e2c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1562,7 +1562,7 @@ static void wt_status_get_detached_from(struct repository *r,\n \t\treturn;\n \t}\n \n-\tif (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&\n+\tif (expand_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&\n \t    /* sha1 is a commit? match without further lookup */\n \t    (oideq(&cb.noid, &oid) ||\n \t     /* perhaps sha1 is a tag, try to dereference to a commit */\n-- \n2.26.2.645.ge9eca65c58-goog\n\n"},{"id":"397704","messageId":"xmqqftc4wg96.fsf@gitster.c.googlers.com","threadId":"53457","inReplyTo":"20200513004058.34456-1-jonathantanmy@google.com","subject":"Re: [PATCH] wt-status: expand, not dwim, a \"detached from\" ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-13T05:33:41Z","receivedAt":"2020-05-13T05:33:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan <jonathantanmy@google.com> writes:\n\n> When a user checks out the upstream branch of HEAD and then runs \"git\n> branch\", like this:\n>\n>   git checkout @{u}\n>   git branch\n>\n> an error message is printed:\n>\n>   fatal: HEAD does not point to a branch\n\nI had to spend unreasonable amount of time reproducing this.\n\n    $ git branch --show-current\n    master\n    $ git checkout -b -t naster master\n    $ git branch --show-current\n    naster\n    $ git checkout @{u}\n    $ git branch --show-current\n    master\n    $ git branch\n    * master\n      naster\n\nWhat was missing was that the current branch must be forked from a\nremote-tracking branch; with a fork from a local branch, the last\nstep, i.e. \"git branch\", works just fine.\n\n> (This error message when running \"git branch\" persists even after\n> checking out other things - it only stops after checking out a branch.)\n\nCorrect.  And it is also worth pointing out that this is not \"git\nbranch\"; the users would see it primarily as a problem with \"git\nstatus\", which would die the same way.\n\n> This is because \"git branch\" attempts to DWIM \"@{u}\" when determining\n> the \"HEAD detached\" message, but that doesn't work because HEAD no\n> longer points to a branch.\n>\n> Therefore, when calculating the status of a worktree, only expand such a\n> string, not DWIM it. Such strings would not match a ref, and \"git\n> branch\" would report \"HEAD detached at <hash>\" instead.\n\nOK.  \"Detached at <hash>\" is somewhat unsatisfactory (you most\nlikely have detached from refs/remotes/origin/<something>), but\nit is much better than \"fatal\" X-<.\n\n> One alternative is to plumb down a flag from dwim_ref() to\n> interpret_branch_mark() that indicates that (1) don't read HEAD when\n> processing, and (2) when encountering a ref of the form \"<optional\n> ref>@{u}\", don't die when the optional ref doesn't exist, is not a\n> branch, or does not have an upstream. (We cannot change the\n> functionality outright because other parts of Git rely on such a message\n> being printed.)\n\nThanks for a good analysis.  That likely is a good direction for a\nlonger term.  @{upstream} is \"tell me the upstream of the branch\nthat is currently checked out\", right?  So it is reasonable to\nexpect that it has no good answer in a detached HEAD state.  And\nwhen on a branch, that branch may not have an upstream, and we\nshould prepare for such a case, too.\n\n> diff --git a/wt-status.c b/wt-status.c\n> index 98dfa6f73f..f84ebc3e2c 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1562,7 +1562,7 @@ static void wt_status_get_detached_from(struct repository *r,\n>  \t\treturn;\n>  \t}\n>  \n> -\tif (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&\n> +\tif (expand_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&\n>  \t    /* sha1 is a commit? match without further lookup */\n>  \t    (oideq(&cb.noid, &oid) ||\n>  \t     /* perhaps sha1 is a tag, try to dereference to a commit */\n\nHmph, I just did this:\n\n    $ git branch -b -t next origin/next\n    $ git checkout next@{upstream}\n    $ git status\n\nIt used to say \"HEAD detached at origin/next\" without this change,\nbut now it says \"HEAD detached at 046d49d455\", which smells like a\nregression.\n"},{"id":"397713","messageId":"xmqq5zczx4mn.fsf@gitster.c.googlers.com","threadId":"53457","inReplyTo":"xmqqftc4wg96.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] wt-status: expand, not dwim, a \"detached from\" ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-13T14:59:28Z","receivedAt":"2020-05-13T15:00:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> OK.  \"Detached at <hash>\" is somewhat unsatisfactory (you most\n> likely have detached from refs/remotes/origin/<something>), but\n> it is much better than \"fatal\" X-<.\n>\n>> One alternative is to plumb down a flag from dwim_ref() to\n>> interpret_branch_mark() that indicates that (1) don't read HEAD when\n>> processing, and (2) when encountering a ref of the form \"<optional\n>> ref>@{u}\", don't die when the optional ref doesn't exist, is not a\n>> branch, or does not have an upstream. (We cannot change the\n>> functionality outright because other parts of Git rely on such a message\n>> being printed.)\n>\n> Thanks for a good analysis.  That likely is a good direction for a\n> longer term.  @{upstream} is \"tell me the upstream of the branch\n> that is currently checked out\", right?  So it is reasonable to\n> expect that it has no good answer in a detached HEAD state.  And\n> when on a branch, that branch may not have an upstream, and we\n> should prepare for such a case, too.\n\nWell, I have to take this back.\n\nThe real cause of the problem is that we record in reflog that we\nswitched to @{u}, but when get_detached_from() tries to \"dwim\" it,\nthe \"current branch\" that is implicitly there to be applied to @{u}\nis different one from what used to compute which branch to check\nout, isn't it?  And it probably is not limited to @{upstream} but\nother @{<stuff>} that implicitly apply to the \"current branch\",\ne.g. @{push}.\n\nThis causes a few potential problems:\n\n - The \"current\" branch may be different from the one when such a\n   reflog entry that refers to @{<stuff>} when we later yank\n   @{<stuff>} out of the reflog and try to use it.  This is the\n   problem the patch under discussion tries to hide, but the patch\n   also breaks cases that are working fine.\n\n - Even if the user gave the branch name (i.e. 'next@{upstream}' in\n   the example this patch would have introduced a regression) or if\n   we updated get_detached_from() to correctly infer the branch that\n   was current when the reflog entry in which we found @{upstream}\n   was recorded, the upstream for the branch may have been\n   reconfigured between the time when the reflog entry was written\n   and get_detached_from() is called.  'next@{upstream}' may have\n   been 'origin/next' back then but it may be a different\n   remote-tracking branch now.  This issue is not solved by the\n   patch---the issue is unfixable unless we log the changes to the\n   branch configuration so that we can figure out what was the\n   upstream of 'next' back then, which we won't do.\n\nAssuming that is the root cause, I think the right solution likely\nlies along these three lines:\n\n (1) record not @{<stuff>} (or <branch>@{<stuff>} for that matter),\n     but the actual starting point in the reflog (e.g. in the\n     example this patch would have introduced a regression,\n     i.e. next@{u}, we should record 'origin/next'.  In the example\n     this patch would have used degraded output to prevent dying,\n     i.e. @{u}, we should also record 'origin/next')---this also\n     will fix the \"the branch's upstream may be different now\"\n     problem.\n\n (2) a patch like yours to use expand_ref() as a fallback, but only\n     for the \"@{<stuff>}\" notation that depends on what the \"then\n     current\" branch was---this is a mere band-aid, like the patch\n     under discussion is, but at least it would not regress the\n     cases that are \"working\".  \"The upstream may be different now\"\n     problem is still there.\n\n (3) when we have to interpret @{<stuff>} found in the reflog, go\n     back to see which branch was current, and compute @{<stuff>}\n     relative to that branch---this would \"fix\" more cases than (2)\n     would, but it won't fix \"the upstream can change\" problem, and\n     I think the trade-off is not all that great.\n\nI think the combination of (1) and (2) is likely to be the best way\nto go.  (1) will remove the root cause, and (2) will allow us to\ncope with reflog entries before (1) is applied.\n"},{"id":"398160","messageId":"20200518222447.15967-1-jonathantanmy@google.com","threadId":"53457","inReplyTo":"xmqq5zczx4mn.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] wt-status: expand, not dwim, a \"detached from\" ref","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-05-18T22:24:47Z","receivedAt":"2020-05-18T22:24:53Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":">  (1) record not @{<stuff>} (or <branch>@{<stuff>} for that matter),\n>      but the actual starting point in the reflog (e.g. in the\n>      example this patch would have introduced a regression,\n>      i.e. next@{u}, we should record 'origin/next'.  In the example\n>      this patch would have used degraded output to prevent dying,\n>      i.e. @{u}, we should also record 'origin/next')---this also\n>      will fix the \"the branch's upstream may be different now\"\n>      problem.\n\nThis sounds reasonable. I took a look at this.\n\nThe part that converts the user-given refname (e.g. \"@{u}\") into an OID\nis the invocation of get_oid_mb() in parse_branchname_arg() in\nbuiltin/checkout.c, and get_oid_mb() eventually calls repo_dwim_ref()\nwhich has access to the absolute branch name (\"origin/master\"). I did\nnot try plumbing it all the way, but I tried overriding \"arg\" with\n\"refs/remotes/origin/master\" after the call to get_oid_mb() and it\nworked.\n\nFor reference, the stack between get_oid_mb() and repo_dwim_ref() is as\nfollows (the line numbers may not be accurate because of some debug\nstatements I added):\n\n  repo_dwim_ref (refs.c:597)                                                                                \n  get_oid_basic (sha1-name.c:875)                                                                           \n  get_oid_1 (sha1-name.c:1195)                                                                              \n  get_oid_with_context_1 (sha1-name.c:1812)                                                         \n  get_oid_with_context (sha1-name.c:1959)\n  repo_get_oid (sha1-name.c:1610)\n  repo_get_oid_mb (sha1-name.c:1382)\n\nBesides the increase in complicatedness of all the listed functions that\nwe would need in order to plumb the absolute branch name through, I\nhaven't checked if the absolute branch name is the one that we should\nuse whenever we write to the reflog, or if there are some times that we\nstill want to use the user-specified name. I'll take a further look, but\nany ideas are welcome.\n"},{"id":"404568","messageId":"20200827014723.GA750502@google.com","threadId":"53457","inReplyTo":"20200513004058.34456-1-jonathantanmy@google.com","subject":"Re: [PATCH] wt-status: expand, not dwim, a \"detached from\" ref","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2020-08-27T01:47:23Z","receivedAt":"2020-08-27T01:47:29Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJonathan Tan wrote:\n\n> When a user checks out the upstream branch of HEAD and then runs \"git\n> branch\", like this:\n>\n>   git checkout @{u}\n>   git branch\n>\n> an error message is printed:\n>\n>   fatal: HEAD does not point to a branch\n\nInteresting.  Even worse, it happens when I run \"git status\".\n\n[...]\n> This is because \"git branch\" attempts to DWIM \"@{u}\" when determining\n> the \"HEAD detached\" message, but that doesn't work because HEAD no\n> longer points to a branch.\n>\n> Therefore, when calculating the status of a worktree, only expand such a\n> string, not DWIM it. Such strings would not match a ref, and \"git\n> branch\" would report \"HEAD detached at <hash>\" instead.\n[...]\n> -\tif (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&\n> +\tif (expand_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&\n>  \t    /* sha1 is a commit? match without further lookup */\n\nAlas, as Junio mentions downthread, this patch produces a regression\nin behavior: before,\n\n\t$ git checkout --quiet master@{u}\n\t$ git status | head -1\n\tHEAD detached at origin/master\n\nafter,\n\n\t$ git checkout --quiet master@{u}\n\t$ git status | head -1\n\tHEAD detached at 675a4aaf3b2\n\nThis ends up being a bit subtle.\n\nThe current branch can be in one of three states: I can be on a branch\n(HEAD symref pointing to a ref refs/heads/branchname), detached (HEAD\npsuedoref pointing to a commit), or on a branch yet to be born.\n__git_ps1 in contrib/completion/git-prompt.sh, for example, is able to\nshow these three states.\n\nSince b397ea4863a (status: show more info than \"currently not on any\nbranch\", 2013-03-13), \"git status\", on the other hand, gives richer\ninformation.  In the detached case, naming the commit that HEAD points\nto doesn't really give a user enough information to orient themself,\nso we look at the reflog (!) to find out what the user had intended to\ncheck out.  Reflog entries look like\n\n\tcheckout: moving from master to v1.0\n\nand the \"v1.0\" can tell us that v1.0 is the tag that we detached HEAD\non.\n\nThis strikes me as always having been strange in a few ways:\n\nOn one hand, this is using the reflog.  reflog entries are\nhistorically meant to be human-readable.  They are a raw string, not\nstructured data.  They can be translated to a different human\nlanguage.  Other tools interacting with git repositories are not\nguaranteed to use the same string.  Changes such as the introduction\nof \"git switch\" could be expected to lead to writing \"switch:\" instead\nof \"checkout:\" some day.\n\nBeyond that, it involves guesswork.  As b397ea4863a explains,\n\n\tWhen it cannot figure out the original ref, it shows an\n\tabbreviated SHA-1.\n\nThe reflog entry contains the parameter passed to \"git checkout\" in\nthe form the user provided --- it is not a full ref name or string\nmeant to be appended to refs/heads/ but it can be arbitrary syntax\nsupported by \"git checkout\".  At the time that the checkout happened,\nwe know what *commit* that name resolved to, but we do not know what\nref it resolved to:\n\n- refs in the search path can have been created or deleted since then\n- with @{u} syntax, the named branch's upstream can have been\n  reconfigured\n- and so on\n\nIf we want to know what ref HEAD was detached at, wouldn't we want\nsome reliable source of that information that records it at the time\nit was known?\n\nAnother example is if I used \"git checkout -\" syntax.  In that case,\nthe reflog records the commit id that I am checking out, instead of\nrecording \"-\".  That's because (after \"-\" is replaced by \"@{-1}\") the\n\"magic branch\" handler strbuf_branchname replaces @{-1} with with the\nbranch name being checked out.  That branch name *also* comes from the\nreflog, this time from the <old> side of the\n\n\tcheckout: moving from <old> to <new>\n\nline, and <old> is populated as a simple commit id or branch name read\nfrom HEAD.  So this creates a bit of an inconsistency: if I run \"git\nstatus\", \"git checkout -\", \"git checkout -\" again, and then \"git\nstatus\" again, the first \"git status\" tells me what ref I had used to\ndetach HEAD but the second does not.\n\nOkay, so much for background.\n\nThe motivating error producing\n\n\tfatal: HEAD does not point to a branch\n\nis a special case of the \"we do not know what ref it resolved to\"\nproblem described above: the string @{upstream} represents the\nupstream of the branch that HEAD pointed to *at the time of the\ncheckout*, but since then HEAD has been detached.\n\n[...]\n> One alternative is to plumb down a flag from dwim_ref() to\n> interpret_branch_mark() that indicates that (1) don't read HEAD when\n> processing, and (2) when encountering a ref of the form \"<optional\n> ref>@{u}\", don't die when the optional ref doesn't exist, is not a\n> branch, or does not have an upstream.\n\nGiven the context above, two possibilities seem appealing:\n\n A. Like you're hinting, could dwim_ref get a variant that returns -1\n    instead of die()ing on failure?  That way, we could fulfill the\n    intent described in b397ea48:\n\n\tWhen it cannot figure out the original ref, it shows an abbreviated\n\tSHA-1.\n\n B. Alternatively, in the @{u} case could we switch together the <old>\n    and <new> pieces of the context from the\n\n\tcheckout: moving from master to @{upstream}\n\n    reflog line to make \"master@{upstream}\"?  It's still possible for\n    the upstream to have changed since then, but at least in the\n    common case this would match the lookup that happened at checkout\n    time.\n\nThoughts?\n\nThanks,\nJonathan\n"},{"id":"404569","messageId":"xmqqtuwosusc.fsf@gitster.c.googlers.com","threadId":"53457","inReplyTo":"20200827014723.GA750502@google.com","subject":"Re: [PATCH] wt-status: expand, not dwim, a \"detached from\" ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-27T02:10:43Z","receivedAt":"2020-08-27T02:10:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Given the context above, two possibilities seem appealing:\n>\n>  A. Like you're hinting, could dwim_ref get a variant that returns -1\n>     instead of die()ing on failure?  That way, we could fulfill the\n>     intent described in b397ea48:\n>\n> \tWhen it cannot figure out the original ref, it shows an abbreviated\n> \tSHA-1.\n>\n>  B. Alternatively, in the @{u} case could we switch together the <old>\n>     and <new> pieces of the context from the\n>\n> \tcheckout: moving from master to @{upstream}\n>\n>     reflog line to make \"master@{upstream}\"?  It's still possible for\n>     the upstream to have changed since then, but at least in the\n>     common case this would match the lookup that happened at checkout\n>     time.\n\nAh, blast from the past.  Thanks for resurrecting.\n\nIf we are allowed to change what goes to reflog, can we do even\nbetter than recording master@{upstream} at the time \"checkout\"\nrecords the HEAD movement?  \"checkout: moving from next to master\"\nwould be far better than \"moving from next to next@{upstream}\" or\n\"moving from next to @{upstream}\".  \n\nCan we even change the phrasing altogether, like \"checkout: moving\nfrom next to detached e9b77c...\"?  That would produce even more\nprecise report.\n\n"},{"id":"404739","messageId":"cover.1598662525.git.jonathantanmy@google.com","threadId":"53457","inReplyTo":"20200513004058.34456-1-jonathantanmy@google.com","subject":"[PATCH v2 0/2] Fix for git checkout @{u} (non-local) then git status","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-08-29T01:02:25Z","receivedAt":"2020-08-29T01:02:37Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"As Jonathan Nieder wrote [1]:\n\n>  A. Like you're hinting, could dwim_ref get a variant that returns -1\n>     instead of die()ing on failure?  That way, we could fulfill the\n>     intent described in b397ea48:\n> \n> \tWhen it cannot figure out the original ref, it shows an abbreviated\n> \tSHA-1.\n\nHere are some patches that do this. As discussed, this is not the\ncomplete solution, but at least now we can handle unresolvable marks.\n\nI've also switched the commit messages and test from mentioning \"git\nbranch\" to mentioning \"git status\".\n\n[1] https://lore.kernel.org/git/20200827014723.GA750502@google.com/\n\nJonathan Tan (2):\n  sha1-name: replace unsigned int with option struct\n  wt-status: tolerate dangling marks\n\n cache.h           | 27 +++++++++++++++++++--------\n refs.c            | 17 +++++++++++------\n refs.h            |  3 ++-\n revision.c        |  3 ++-\n sha1-name.c       | 45 +++++++++++++++++++++++++++++----------------\n t/t7508-status.sh | 12 ++++++++++++\n wt-status.c       |  2 +-\n 7 files changed, 76 insertions(+), 33 deletions(-)\n\n-- \n2.28.0.402.g5ffc5be6b7-goog\n\n"},{"id":"404740","messageId":"1ce44900a08857332ee70b916c3d9e7e76751221.1598662525.git.jonathantanmy@google.com","threadId":"53457","inReplyTo":"cover.1598662525.git.jonathantanmy@google.com","subject":"[PATCH v2 1/2] sha1-name: replace unsigned int with option struct","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-08-29T01:02:26Z","receivedAt":"2020-08-29T01:02:37Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"In preparation for a future patch adding a boolean parameter to\nrepo_interpret_branch_name(), which might be easily confused with an\nexisting unsigned int parameter, refactor repo_interpret_branch_name()\nto take an option struct instead of the unsigned int parameter.\n\nThe static function interpret_branch_mark() is also updated to take the\noption struct in preparation for that future patch, since it will also\nmake use of the to-be-introduced boolean parameter.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n cache.h     | 20 ++++++++++++--------\n refs.c      |  3 ++-\n revision.c  |  3 ++-\n sha1-name.c | 29 ++++++++++++++++++-----------\n 4 files changed, 34 insertions(+), 21 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4cad61ffa4..4f16a57ba4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1557,21 +1557,25 @@ int parse_oid_hex_any(const char *hex, struct object_id *oid, const char **end);\n  *\n  * If the input was ok but there are not N branch switches in the\n  * reflog, it returns 0.\n- *\n- * If \"allowed\" is non-zero, it is a treated as a bitfield of allowable\n- * expansions: local branches (\"refs/heads/\"), remote branches\n- * (\"refs/remotes/\"), or \"HEAD\". If no \"allowed\" bits are set, any expansion is\n- * allowed, even ones to refs outside of those namespaces.\n  */\n #define INTERPRET_BRANCH_LOCAL (1<<0)\n #define INTERPRET_BRANCH_REMOTE (1<<1)\n #define INTERPRET_BRANCH_HEAD (1<<2)\n+struct interpret_branch_name_options {\n+\t/*\n+\t * If \"allowed\" is non-zero, it is a treated as a bitfield of allowable\n+\t * expansions: local branches (\"refs/heads/\"), remote branches\n+\t * (\"refs/remotes/\"), or \"HEAD\". If no \"allowed\" bits are set, any expansion is\n+\t * allowed, even ones to refs outside of those namespaces.\n+\t */\n+\tunsigned allowed;\n+};\n int repo_interpret_branch_name(struct repository *r,\n \t\t\t       const char *str, int len,\n \t\t\t       struct strbuf *buf,\n-\t\t\t       unsigned allowed);\n-#define interpret_branch_name(str, len, buf, allowed) \\\n-\trepo_interpret_branch_name(the_repository, str, len, buf, allowed)\n+\t\t\t       const struct interpret_branch_name_options *options);\n+#define interpret_branch_name(str, len, buf, options) \\\n+\trepo_interpret_branch_name(the_repository, str, len, buf, options)\n \n int validate_headref(const char *ref);\n \ndiff --git a/refs.c b/refs.c\nindex 3ee3afaf41..cf09cd039f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -601,7 +601,8 @@ static char *substitute_branch_name(struct repository *r,\n \t\t\t\t    const char **string, int *len)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tint ret = repo_interpret_branch_name(r, *string, *len, &buf, 0);\n+\tstruct interpret_branch_name_options options = { 0 } ;\n+\tint ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);\n \n \tif (ret == *len) {\n \t\tsize_t size;\ndiff --git a/revision.c b/revision.c\nindex 96630e3186..1247ee4ec8 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -315,13 +315,14 @@ static void add_pending_object_with_path(struct rev_info *revs,\n \t\t\t\t\t const char *name, unsigned mode,\n \t\t\t\t\t const char *path)\n {\n+\tstruct interpret_branch_name_options options = { 0 };\n \tif (!obj)\n \t\treturn;\n \tif (revs->no_walk && (obj->flags & UNINTERESTING))\n \t\trevs->no_walk = 0;\n \tif (revs->reflog_info && obj->type == OBJ_COMMIT) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n-\t\tint len = interpret_branch_name(name, 0, &buf, 0);\n+\t\tint len = interpret_branch_name(name, 0, &buf, &options);\n \n \t\tif (0 < len && name[len] && buf.len)\n \t\t\tstrbuf_addstr(&buf, name + len);\ndiff --git a/sha1-name.c b/sha1-name.c\nindex 0b8cb5247a..a7a9de66c4 100644\n--- a/sha1-name.c\n+++ b/sha1-name.c\n@@ -1427,9 +1427,12 @@ static int reinterpret(struct repository *r,\n \tstruct strbuf tmp = STRBUF_INIT;\n \tint used = buf->len;\n \tint ret;\n+\tstruct interpret_branch_name_options options = {\n+\t\t.allowed = allowed\n+\t};\n \n \tstrbuf_add(buf, name + len, namelen - len);\n-\tret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, allowed);\n+\tret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, &options);\n \t/* that data was not interpreted, remove our cruft */\n \tif (ret < 0) {\n \t\tstrbuf_setlen(buf, used);\n@@ -1471,7 +1474,7 @@ static int interpret_branch_mark(struct repository *r,\n \t\t\t\t int (*get_mark)(const char *, int),\n \t\t\t\t const char *(*get_data)(struct branch *,\n \t\t\t\t\t\t\t struct strbuf *),\n-\t\t\t\t unsigned allowed)\n+\t\t\t\t const struct interpret_branch_name_options *options)\n {\n \tint len;\n \tstruct branch *branch;\n@@ -1496,7 +1499,7 @@ static int interpret_branch_mark(struct repository *r,\n \tif (!value)\n \t\tdie(\"%s\", err.buf);\n \n-\tif (!branch_interpret_allowed(value, allowed))\n+\tif (!branch_interpret_allowed(value, options->allowed))\n \t\treturn -1;\n \n \tset_shortened_ref(r, buf, value);\n@@ -1506,7 +1509,7 @@ static int interpret_branch_mark(struct repository *r,\n int repo_interpret_branch_name(struct repository *r,\n \t\t\t       const char *name, int namelen,\n \t\t\t       struct strbuf *buf,\n-\t\t\t       unsigned allowed)\n+\t\t\t       const struct interpret_branch_name_options *options)\n {\n \tchar *at;\n \tconst char *start;\n@@ -1515,7 +1518,7 @@ int repo_interpret_branch_name(struct repository *r,\n \tif (!namelen)\n \t\tnamelen = strlen(name);\n \n-\tif (!allowed || (allowed & INTERPRET_BRANCH_LOCAL)) {\n+\tif (!options->allowed || (options->allowed & INTERPRET_BRANCH_LOCAL)) {\n \t\tlen = interpret_nth_prior_checkout(r, name, namelen, buf);\n \t\tif (!len) {\n \t\t\treturn len; /* syntax Ok, not enough switches */\n@@ -1523,7 +1526,8 @@ int repo_interpret_branch_name(struct repository *r,\n \t\t\tif (len == namelen)\n \t\t\t\treturn len; /* consumed all */\n \t\t\telse\n-\t\t\t\treturn reinterpret(r, name, namelen, len, buf, allowed);\n+\t\t\t\treturn reinterpret(r, name, namelen, len, buf,\n+\t\t\t\t\t\t   options->allowed);\n \t\t}\n \t}\n \n@@ -1531,22 +1535,22 @@ int repo_interpret_branch_name(struct repository *r,\n \t     (at = memchr(start, '@', namelen - (start - name)));\n \t     start = at + 1) {\n \n-\t\tif (!allowed || (allowed & INTERPRET_BRANCH_HEAD)) {\n+\t\tif (!options->allowed || (options->allowed & INTERPRET_BRANCH_HEAD)) {\n \t\t\tlen = interpret_empty_at(name, namelen, at - name, buf);\n \t\t\tif (len > 0)\n \t\t\t\treturn reinterpret(r, name, namelen, len, buf,\n-\t\t\t\t\t\t   allowed);\n+\t\t\t\t\t\t   options->allowed);\n \t\t}\n \n \t\tlen = interpret_branch_mark(r, name, namelen, at - name, buf,\n \t\t\t\t\t    upstream_mark, branch_get_upstream,\n-\t\t\t\t\t    allowed);\n+\t\t\t\t\t    options);\n \t\tif (len > 0)\n \t\t\treturn len;\n \n \t\tlen = interpret_branch_mark(r, name, namelen, at - name, buf,\n \t\t\t\t\t    push_mark, branch_get_push,\n-\t\t\t\t\t    allowed);\n+\t\t\t\t\t    options);\n \t\tif (len > 0)\n \t\t\treturn len;\n \t}\n@@ -1557,7 +1561,10 @@ int repo_interpret_branch_name(struct repository *r,\n void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)\n {\n \tint len = strlen(name);\n-\tint used = interpret_branch_name(name, len, sb, allowed);\n+\tstruct interpret_branch_name_options options = {\n+\t\t.allowed = allowed\n+\t};\n+\tint used = interpret_branch_name(name, len, sb, &options);\n \n \tif (used < 0)\n \t\tused = 0;\n-- \n2.28.0.402.g5ffc5be6b7-goog\n\n"},{"id":"404741","messageId":"59b91a206d9d7bf64825cb48c747730e28b10a79.1598662525.git.jonathantanmy@google.com","threadId":"53457","inReplyTo":"cover.1598662525.git.jonathantanmy@google.com","subject":"[PATCH v2 2/2] wt-status: tolerate dangling marks","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-08-29T01:02:27Z","receivedAt":"2020-08-29T01:02:39Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"When a user checks out the upstream branch of HEAD, the upstream branch\nnot being a local branch, and then runs \"git status\", like this:\n\n  git clone $URL client\n  cd client\n  git checkout @{u}\n  git status\n\nno status is printed, but instead an error message:\n\n  fatal: HEAD does not point to a branch\n\n(This error message when running \"git branch\" persists even after\nchecking out other things - it only stops after checking out a branch.)\n\nThis is because \"git status\" reads the reflog when determining the \"HEAD\ndetached\" message, and thus attempts to DWIM \"@{u}\", but that doesn't\nwork because HEAD no longer points to a branch.\n\nTherefore, when calculating the status of a worktree, tolerate dangling\nmarks. This is done by adding an additional parameter to\nrepo_dwim_ref().\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n cache.h           |  7 +++++++\n refs.c            | 16 ++++++++++------\n refs.h            |  3 ++-\n sha1-name.c       | 16 +++++++++++-----\n t/t7508-status.sh | 12 ++++++++++++\n wt-status.c       |  2 +-\n 6 files changed, 43 insertions(+), 13 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4f16a57ba4..cee8aa5dc3 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1569,6 +1569,13 @@ struct interpret_branch_name_options {\n \t * allowed, even ones to refs outside of those namespaces.\n \t */\n \tunsigned allowed;\n+\n+\t/*\n+\t * If ^{upstream} or ^{push} (or equivalent) is requested, and the\n+\t * branch in question does not have such a reference, return -1 instead\n+\t * of die()-ing.\n+\t */\n+\tunsigned nonfatal_dangling_mark : 1;\n };\n int repo_interpret_branch_name(struct repository *r,\n \t\t\t       const char *str, int len,\ndiff --git a/refs.c b/refs.c\nindex cf09cd039f..b6f1a2f452 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -598,10 +598,13 @@ const char *git_default_branch_name(void)\n  * to name a branch.\n  */\n static char *substitute_branch_name(struct repository *r,\n-\t\t\t\t    const char **string, int *len)\n+\t\t\t\t    const char **string, int *len,\n+\t\t\t\t    int nonfatal_dangling_mark)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct interpret_branch_name_options options = { 0 } ;\n+\tstruct interpret_branch_name_options options = {\n+\t\t.nonfatal_dangling_mark = nonfatal_dangling_mark\n+\t};\n \tint ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);\n \n \tif (ret == *len) {\n@@ -615,9 +618,10 @@ static char *substitute_branch_name(struct repository *r,\n }\n \n int repo_dwim_ref(struct repository *r, const char *str, int len,\n-\t\t  struct object_id *oid, char **ref)\n+\t\t  struct object_id *oid, char **ref, int nonfatal_dangling_mark)\n {\n-\tchar *last_branch = substitute_branch_name(r, &str, &len);\n+\tchar *last_branch = substitute_branch_name(r, &str, &len,\n+\t\t\t\t\t\t   nonfatal_dangling_mark);\n \tint   refs_found  = expand_ref(r, str, len, oid, ref);\n \tfree(last_branch);\n \treturn refs_found;\n@@ -625,7 +629,7 @@ int repo_dwim_ref(struct repository *r, const char *str, int len,\n \n int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)\n {\n-\treturn repo_dwim_ref(the_repository, str, len, oid, ref);\n+\treturn repo_dwim_ref(the_repository, str, len, oid, ref, 0);\n }\n \n int expand_ref(struct repository *repo, const char *str, int len,\n@@ -666,7 +670,7 @@ int repo_dwim_log(struct repository *r, const char *str, int len,\n \t\t  struct object_id *oid, char **log)\n {\n \tstruct ref_store *refs = get_main_ref_store(r);\n-\tchar *last_branch = substitute_branch_name(r, &str, &len);\n+\tchar *last_branch = substitute_branch_name(r, &str, &len, 0);\n \tconst char **p;\n \tint logs_found = 0;\n \tstruct strbuf path = STRBUF_INIT;\ndiff --git a/refs.h b/refs.h\nindex 29e28124cd..b94a7fd4f7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -149,7 +149,8 @@ struct strvec;\n void expand_ref_prefix(struct strvec *prefixes, const char *prefix);\n \n int expand_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n-int repo_dwim_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n+int repo_dwim_ref(struct repository *r, const char *str, int len,\n+\t\t  struct object_id *oid, char **ref, int nonfatal_dangling_mark);\n int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);\n int dwim_log(const char *str, int len, struct object_id *oid, char **ref);\ndiff --git a/sha1-name.c b/sha1-name.c\nindex a7a9de66c4..0b23b86ceb 100644\n--- a/sha1-name.c\n+++ b/sha1-name.c\n@@ -809,7 +809,7 @@ static int get_oid_basic(struct repository *r, const char *str, int len,\n \n \tif (len == r->hash_algo->hexsz && !get_oid_hex(str, oid)) {\n \t\tif (warn_ambiguous_refs && warn_on_object_refname_ambiguity) {\n-\t\t\trefs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref);\n+\t\t\trefs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref, 0);\n \t\t\tif (refs_found > 0) {\n \t\t\t\twarning(warn_msg, len, str);\n \t\t\t\tif (advice_object_name_warning)\n@@ -860,11 +860,11 @@ static int get_oid_basic(struct repository *r, const char *str, int len,\n \n \tif (!len && reflog_len)\n \t\t/* allow \"@{...}\" to mean the current branch reflog */\n-\t\trefs_found = repo_dwim_ref(r, \"HEAD\", 4, oid, &real_ref);\n+\t\trefs_found = repo_dwim_ref(r, \"HEAD\", 4, oid, &real_ref, 0);\n \telse if (reflog_len)\n \t\trefs_found = repo_dwim_log(r, str, len, oid, &real_ref);\n \telse\n-\t\trefs_found = repo_dwim_ref(r, str, len, oid, &real_ref);\n+\t\trefs_found = repo_dwim_ref(r, str, len, oid, &real_ref, 0);\n \n \tif (!refs_found)\n \t\treturn -1;\n@@ -1496,8 +1496,14 @@ static int interpret_branch_mark(struct repository *r,\n \t\tbranch = branch_get(NULL);\n \n \tvalue = get_data(branch, &err);\n-\tif (!value)\n-\t\tdie(\"%s\", err.buf);\n+\tif (!value) {\n+\t\tif (options->nonfatal_dangling_mark) {\n+\t\t\tstrbuf_release(&err);\n+\t\t\treturn -1;\n+\t\t} else {\n+\t\t\tdie(\"%s\", err.buf);\n+\t\t}\n+\t}\n \n \tif (!branch_interpret_allowed(value, options->allowed))\n \t\treturn -1;\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex e81759319f..45e1f6ff68 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -846,6 +846,18 @@ test_expect_success 'status refreshes the index' '\n \ttest_cmp expect output\n '\n \n+test_expect_success 'status shows detached HEAD properly after checking out non-local upstream branch' '\n+\ttest_when_finished rm -rf upstream downstream actual &&\n+\n+\ttest_create_repo upstream &&\n+\ttest_commit -C upstream foo &&\n+\n+\tgit clone upstream downstream &&\n+\tgit -C downstream checkout @{u} &&\n+\tgit -C downstream status >actual &&\n+\ttest_i18ngrep \"HEAD detached at [0-9a-f]\\\\+\" actual\n+'\n+\n test_expect_success 'setup status submodule summary' '\n \ttest_create_repo sm && (\n \t\tcd sm &&\ndiff --git a/wt-status.c b/wt-status.c\nindex 7ce58b8aae..ae16faf40d 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1569,7 +1569,7 @@ static void wt_status_get_detached_from(struct repository *r,\n \t\treturn;\n \t}\n \n-\tif (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&\n+\tif (repo_dwim_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&\n \t    /* sha1 is a commit? match without further lookup */\n \t    (oideq(&cb.noid, &oid) ||\n \t     /* perhaps sha1 is a tag, try to dereference to a commit */\n-- \n2.28.0.402.g5ffc5be6b7-goog\n\n"},{"id":"404752","messageId":"xmqqsgc5i96h.fsf@gitster.c.googlers.com","threadId":"53457","inReplyTo":"1ce44900a08857332ee70b916c3d9e7e76751221.1598662525.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 1/2] sha1-name: replace unsigned int with option struct","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-29T18:44:22Z","receivedAt":"2020-08-29T18:44:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan <jonathantanmy@google.com> writes:\n\n> In preparation for a future patch adding a boolean parameter to\n> repo_interpret_branch_name(), which might be easily confused with an\n> existing unsigned int parameter, refactor repo_interpret_branch_name()\n> to take an option struct instead of the unsigned int parameter.\n\nMakes sense.\n\n>  #define INTERPRET_BRANCH_LOCAL (1<<0)\n>  #define INTERPRET_BRANCH_REMOTE (1<<1)\n>  #define INTERPRET_BRANCH_HEAD (1<<2)\n> +struct interpret_branch_name_options {\n> +\t/*\n> +\t * If \"allowed\" is non-zero, it is a treated as a bitfield of allowable\n> +\t * expansions: local branches (\"refs/heads/\"), remote branches\n> +\t * (\"refs/remotes/\"), or \"HEAD\". If no \"allowed\" bits are set, any expansion is\n> +\t * allowed, even ones to refs outside of those namespaces.\n> +\t */\n> +\tunsigned allowed;\n> +};\n>  int repo_interpret_branch_name(struct repository *r,\n>  \t\t\t       const char *str, int len,\n>  \t\t\t       struct strbuf *buf,\n> -\t\t\t       unsigned allowed);\n> -#define interpret_branch_name(str, len, buf, allowed) \\\n> -\trepo_interpret_branch_name(the_repository, str, len, buf, allowed)\n> +\t\t\t       const struct interpret_branch_name_options *options);\n> +#define interpret_branch_name(str, len, buf, options) \\\n> +\trepo_interpret_branch_name(the_repository, str, len, buf, options)\n\nI was debating myself if we want to have \n\n    #define IBN_OPTIONS_INIT { 0 }\n\nor something similar (perhaps \"#define IOI(abit) { .allowed = (abit) }\"),\nbut it probably is not worth it given that we have only 3 local\nsites that define it, 1 always initializes the field to 0, and the\nother just relay the value passed by its caller.\n\n> ...\n> diff --git a/sha1-name.c b/sha1-name.c\n> index 0b8cb5247a..a7a9de66c4 100644\n> --- a/sha1-name.c\n> +++ b/sha1-name.c\n> @@ -1427,9 +1427,12 @@ static int reinterpret(struct repository *r,\n>  \tstruct strbuf tmp = STRBUF_INIT;\n>  \tint used = buf->len;\n>  \tint ret;\n> +\tstruct interpret_branch_name_options options = {\n> +\t\t.allowed = allowed\n> +\t};\n>  \n>  \tstrbuf_add(buf, name + len, namelen - len);\n> -\tret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, allowed);\n> +\tret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, &options);\n\n> @@ -1557,7 +1561,10 @@ int repo_interpret_branch_name(struct repository *r,\n>  void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)\n>  {\n>  \tint len = strlen(name);\n> -\tint used = interpret_branch_name(name, len, sb, allowed);\n> +\tstruct interpret_branch_name_options options = {\n> +\t\t.allowed = allowed\n> +\t};\n> +\tint used = interpret_branch_name(name, len, sb, &options);\n\nThese are quite straight-forward rewrites.  Looking good.\n\nThanks.\n"},{"id":"404754","messageId":"xmqqo8mti8od.fsf@gitster.c.googlers.com","threadId":"53457","inReplyTo":"59b91a206d9d7bf64825cb48c747730e28b10a79.1598662525.git.jonathantanmy@google.com","subject":"Re: [PATCH v2 2/2] wt-status: tolerate dangling marks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-29T18:55:14Z","receivedAt":"2020-08-29T18:55:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan <jonathantanmy@google.com> writes:\n\n> +\n> +\t/*\n> +\t * If ^{upstream} or ^{push} (or equivalent) is requested, and the\n> +\t * branch in question does not have such a reference, return -1 instead\n> +\t * of die()-ing.\n> +\t */\n> +\tunsigned nonfatal_dangling_mark : 1;\n\nMicronit; I would have avoided \"or equivalent\" as the point of\nparenthetical comment was not to say these two modifiers upstream\nand push (and other forms that spell differently but invokes exactly\none of these two features) are special, but to say that these two\nare merely examples, and any other ^{modifiers} we have or we will\nadd in the future would honor this bit.  Perhaps \"(and the like)\"?\n\n>  };\n>  int repo_interpret_branch_name(struct repository *r,\n>  \t\t\t       const char *str, int len,\n> diff --git a/refs.c b/refs.c\n> index cf09cd039f..b6f1a2f452 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -598,10 +598,13 @@ const char *git_default_branch_name(void)\n>   * to name a branch.\n>   */\n>  static char *substitute_branch_name(struct repository *r,\n> -\t\t\t\t    const char **string, int *len)\n> +\t\t\t\t    const char **string, int *len,\n> +\t\t\t\t    int nonfatal_dangling_mark)\n>  {\n>  \tstruct strbuf buf = STRBUF_INIT;\n> -\tstruct interpret_branch_name_options options = { 0 } ;\n> +\tstruct interpret_branch_name_options options = {\n> +\t\t.nonfatal_dangling_mark = nonfatal_dangling_mark\n> +\t};\n>  \tint ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);\n>  \n>  \tif (ret == *len) {\n> @@ -615,9 +618,10 @@ static char *substitute_branch_name(struct repository *r,\n>  }\n>  \n>  int repo_dwim_ref(struct repository *r, const char *str, int len,\n> -\t\t  struct object_id *oid, char **ref)\n> +\t\t  struct object_id *oid, char **ref, int nonfatal_dangling_mark)\n>  {\n> -\tchar *last_branch = substitute_branch_name(r, &str, &len);\n> +\tchar *last_branch = substitute_branch_name(r, &str, &len,\n> +\t\t\t\t\t\t   nonfatal_dangling_mark);\n>  \tint   refs_found  = expand_ref(r, str, len, oid, ref);\n>  \tfree(last_branch);\n>  \treturn refs_found;\n> @@ -625,7 +629,7 @@ int repo_dwim_ref(struct repository *r, const char *str, int len,\n>  \n>  int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)\n>  {\n> -\treturn repo_dwim_ref(the_repository, str, len, oid, ref);\n> +\treturn repo_dwim_ref(the_repository, str, len, oid, ref, 0);\n>  }\n>  \n>  int expand_ref(struct repository *repo, const char *str, int len,\n> @@ -666,7 +670,7 @@ int repo_dwim_log(struct repository *r, const char *str, int len,\n>  \t\t  struct object_id *oid, char **log)\n>  {\n>  \tstruct ref_store *refs = get_main_ref_store(r);\n> -\tchar *last_branch = substitute_branch_name(r, &str, &len);\n> +\tchar *last_branch = substitute_branch_name(r, &str, &len, 0);\n>  \tconst char **p;\n>  \tint logs_found = 0;\n>  \tstruct strbuf path = STRBUF_INIT;\n\nAmong these callers that reach substitute_branch_name(), how were\nthose that can specify the new bit chosen?\n\nFor example, what is the reasoning behind making dwim_ref() unable\nto ask the \"do so gently\" variant, while allowing repo_dwim_ref()\nto?\n\nI am NOT necessarily saying these two functions MUST be able to\naccess the same set of features and the only difference between them\nMUST be kept to the current \"repo_* variant can work on an arbitrary\nrepository, while the variant without repo_* would work on the\nprimary repository only\".  As long as there is a good reason to make\ntheir power diverge, it is OK---I just do not see why and I'd like\nto know.\n\nThe same question about not allowing the gentler variant while\ndrimming the reflog.\n\nThanks.\n"},{"id":"404818","messageId":"20200831171732.1199755-1-jonathantanmy@google.com","threadId":"53457","inReplyTo":"xmqqo8mti8od.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 2/2] wt-status: tolerate dangling marks","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-08-31T17:17:32Z","receivedAt":"2020-08-31T17:17:39Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> Jonathan Tan <jonathantanmy@google.com> writes:\n> \n> > +\n> > +\t/*\n> > +\t * If ^{upstream} or ^{push} (or equivalent) is requested, and the\n> > +\t * branch in question does not have such a reference, return -1 instead\n> > +\t * of die()-ing.\n> > +\t */\n> > +\tunsigned nonfatal_dangling_mark : 1;\n> \n> Micronit; I would have avoided \"or equivalent\" as the point of\n> parenthetical comment was not to say these two modifiers upstream\n> and push (and other forms that spell differently but invokes exactly\n> one of these two features) are special, but to say that these two\n> are merely examples, and any other ^{modifiers} we have or we will\n> add in the future would honor this bit.  Perhaps \"(and the like)\"?\n\n\"(and the like)\" sounds good.\n\n> Among these callers that reach substitute_branch_name(), how were\n> those that can specify the new bit chosen?\n\nI just did the minimal change to fix the bug in the test.\n\n> For example, what is the reasoning behind making dwim_ref() unable\n> to ask the \"do so gently\" variant, while allowing repo_dwim_ref()\n> to?\n> \n> I am NOT necessarily saying these two functions MUST be able to\n> access the same set of features and the only difference between them\n> MUST be kept to the current \"repo_* variant can work on an arbitrary\n> repository, while the variant without repo_* would work on the\n> primary repository only\".  As long as there is a good reason to make\n> their power diverge, it is OK---I just do not see why and I'd like\n> to know.\n\nMaybe add the following to the end of the last paragraph of the commit\nmessage:\n\n  (dwim_ref() is unchanged because I expect more and more code to use\n  repo_dwim_ref(), and to reduce the size of the diff.)\n\nIf you prefer not to make this change locally, let me know and I'll\nresend one with the updated commit message and the \"(and the like)\"\nchange above.\n\n> The same question about not allowing the gentler variant while\n> drimming the reflog.\n\nSame as above - I only did the minimal change to fix the bug.\nAdmittedly, if a reflog-accessing command could fail in the same way\n(dangling mark), we would need to fix repo_dwim_log() and then we could\nsimplify substitute_branch_name() to not take the nonfatal_dangling_mark\nparameter (since all dangling marks would now be nonfatal), but I\nhaven't looked beyond this.\n"},{"id":"404820","messageId":"xmqqzh6ag1ih.fsf@gitster.c.googlers.com","threadId":"53457","inReplyTo":"20200831171732.1199755-1-jonathantanmy@google.com","subject":"Re: [PATCH v2 2/2] wt-status: tolerate dangling marks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-31T17:37:26Z","receivedAt":"2020-08-31T17:37:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan <jonathantanmy@google.com> writes:\n\n>> For example, what is the reasoning behind making dwim_ref() unable\n>> to ask the \"do so gently\" variant, while allowing repo_dwim_ref()\n>> to?\n>> \n>> I am NOT necessarily saying these two functions MUST be able to\n>> access the same set of features and the only difference between them\n>> MUST be kept to the current \"repo_* variant can work on an arbitrary\n>> repository, while the variant without repo_* would work on the\n>> primary repository only\".  As long as there is a good reason to make\n>> their power diverge, it is OK---I just do not see why and I'd like\n>> to know.\n>\n> Maybe add the following to the end of the last paragraph of the commit\n> message:\n>\n>   (dwim_ref() is unchanged because I expect more and more code to use\n>   repo_dwim_ref(), and to reduce the size of the diff.)\n\nIf that is the reasoning, I totally disagree.  We may be staring\nproblems in submodules too long to think everybody should use the\nvariant with repo_ prefix, but a single repository operations will\ncontinue to exist, and when you know you only need to access the\ncurrent repository, not having to use the function with longer name\nwithout having to pass an extra parameter is a plus.\n\nI even wonder why dwim_ref() is not defined like so in a header:\n\n\t#define dwim_ref(s, l, o, r) \\\n\t\trepo_dwim_ref(the_repository, (s), (l), (o), (r))\n\nWhich would force a change like the one we are discussing to keep\nthem in parallel and keep the promise that only difference between\nthe two is that dwim_ref() works with the_repository and the other\ncan take an arbitrary repository.  Perhaps that can be a preliminary\nclean-up patch before these two patches ;-)\n\nDoing so would mean that coders have to remember one fewer thing\nthan \"dwim_ref(), not only cannot take a repository pointer, cannot\nbe told to report error gently\".  The worst part is that we know\nthat the difference will GROW, because the purpose of adding the\nextra option argument to the API is exactly to make it easy to do\nso.\n\nReducing the size of the diff is a good justification only when the\nend result is the same.  If it burdens future developers more, that\nis \"I write less at the expense of forcing others to write more\",\nwhich is not quite the same thing.\n\nThanks.\n"},{"id":"404888","messageId":"1ce44900a08857332ee70b916c3d9e7e76751221.1598998707.git.jonathantanmy@google.com","threadId":"53457","inReplyTo":"cover.1598998706.git.jonathantanmy@google.com","subject":"[PATCH v3 1/3] sha1-name: replace unsigned int with option struct","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-09-01T22:28:07Z","receivedAt":"2020-09-01T22:28:21Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"In preparation for a future patch adding a boolean parameter to\nrepo_interpret_branch_name(), which might be easily confused with an\nexisting unsigned int parameter, refactor repo_interpret_branch_name()\nto take an option struct instead of the unsigned int parameter.\n\nThe static function interpret_branch_mark() is also updated to take the\noption struct in preparation for that future patch, since it will also\nmake use of the to-be-introduced boolean parameter.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n cache.h     | 20 ++++++++++++--------\n refs.c      |  3 ++-\n revision.c  |  3 ++-\n sha1-name.c | 29 ++++++++++++++++++-----------\n 4 files changed, 34 insertions(+), 21 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4cad61ffa4..4f16a57ba4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1557,21 +1557,25 @@ int parse_oid_hex_any(const char *hex, struct object_id *oid, const char **end);\n  *\n  * If the input was ok but there are not N branch switches in the\n  * reflog, it returns 0.\n- *\n- * If \"allowed\" is non-zero, it is a treated as a bitfield of allowable\n- * expansions: local branches (\"refs/heads/\"), remote branches\n- * (\"refs/remotes/\"), or \"HEAD\". If no \"allowed\" bits are set, any expansion is\n- * allowed, even ones to refs outside of those namespaces.\n  */\n #define INTERPRET_BRANCH_LOCAL (1<<0)\n #define INTERPRET_BRANCH_REMOTE (1<<1)\n #define INTERPRET_BRANCH_HEAD (1<<2)\n+struct interpret_branch_name_options {\n+\t/*\n+\t * If \"allowed\" is non-zero, it is a treated as a bitfield of allowable\n+\t * expansions: local branches (\"refs/heads/\"), remote branches\n+\t * (\"refs/remotes/\"), or \"HEAD\". If no \"allowed\" bits are set, any expansion is\n+\t * allowed, even ones to refs outside of those namespaces.\n+\t */\n+\tunsigned allowed;\n+};\n int repo_interpret_branch_name(struct repository *r,\n \t\t\t       const char *str, int len,\n \t\t\t       struct strbuf *buf,\n-\t\t\t       unsigned allowed);\n-#define interpret_branch_name(str, len, buf, allowed) \\\n-\trepo_interpret_branch_name(the_repository, str, len, buf, allowed)\n+\t\t\t       const struct interpret_branch_name_options *options);\n+#define interpret_branch_name(str, len, buf, options) \\\n+\trepo_interpret_branch_name(the_repository, str, len, buf, options)\n \n int validate_headref(const char *ref);\n \ndiff --git a/refs.c b/refs.c\nindex 3ee3afaf41..cf09cd039f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -601,7 +601,8 @@ static char *substitute_branch_name(struct repository *r,\n \t\t\t\t    const char **string, int *len)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tint ret = repo_interpret_branch_name(r, *string, *len, &buf, 0);\n+\tstruct interpret_branch_name_options options = { 0 } ;\n+\tint ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);\n \n \tif (ret == *len) {\n \t\tsize_t size;\ndiff --git a/revision.c b/revision.c\nindex 96630e3186..1247ee4ec8 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -315,13 +315,14 @@ static void add_pending_object_with_path(struct rev_info *revs,\n \t\t\t\t\t const char *name, unsigned mode,\n \t\t\t\t\t const char *path)\n {\n+\tstruct interpret_branch_name_options options = { 0 };\n \tif (!obj)\n \t\treturn;\n \tif (revs->no_walk && (obj->flags & UNINTERESTING))\n \t\trevs->no_walk = 0;\n \tif (revs->reflog_info && obj->type == OBJ_COMMIT) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n-\t\tint len = interpret_branch_name(name, 0, &buf, 0);\n+\t\tint len = interpret_branch_name(name, 0, &buf, &options);\n \n \t\tif (0 < len && name[len] && buf.len)\n \t\t\tstrbuf_addstr(&buf, name + len);\ndiff --git a/sha1-name.c b/sha1-name.c\nindex 0b8cb5247a..a7a9de66c4 100644\n--- a/sha1-name.c\n+++ b/sha1-name.c\n@@ -1427,9 +1427,12 @@ static int reinterpret(struct repository *r,\n \tstruct strbuf tmp = STRBUF_INIT;\n \tint used = buf->len;\n \tint ret;\n+\tstruct interpret_branch_name_options options = {\n+\t\t.allowed = allowed\n+\t};\n \n \tstrbuf_add(buf, name + len, namelen - len);\n-\tret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, allowed);\n+\tret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, &options);\n \t/* that data was not interpreted, remove our cruft */\n \tif (ret < 0) {\n \t\tstrbuf_setlen(buf, used);\n@@ -1471,7 +1474,7 @@ static int interpret_branch_mark(struct repository *r,\n \t\t\t\t int (*get_mark)(const char *, int),\n \t\t\t\t const char *(*get_data)(struct branch *,\n \t\t\t\t\t\t\t struct strbuf *),\n-\t\t\t\t unsigned allowed)\n+\t\t\t\t const struct interpret_branch_name_options *options)\n {\n \tint len;\n \tstruct branch *branch;\n@@ -1496,7 +1499,7 @@ static int interpret_branch_mark(struct repository *r,\n \tif (!value)\n \t\tdie(\"%s\", err.buf);\n \n-\tif (!branch_interpret_allowed(value, allowed))\n+\tif (!branch_interpret_allowed(value, options->allowed))\n \t\treturn -1;\n \n \tset_shortened_ref(r, buf, value);\n@@ -1506,7 +1509,7 @@ static int interpret_branch_mark(struct repository *r,\n int repo_interpret_branch_name(struct repository *r,\n \t\t\t       const char *name, int namelen,\n \t\t\t       struct strbuf *buf,\n-\t\t\t       unsigned allowed)\n+\t\t\t       const struct interpret_branch_name_options *options)\n {\n \tchar *at;\n \tconst char *start;\n@@ -1515,7 +1518,7 @@ int repo_interpret_branch_name(struct repository *r,\n \tif (!namelen)\n \t\tnamelen = strlen(name);\n \n-\tif (!allowed || (allowed & INTERPRET_BRANCH_LOCAL)) {\n+\tif (!options->allowed || (options->allowed & INTERPRET_BRANCH_LOCAL)) {\n \t\tlen = interpret_nth_prior_checkout(r, name, namelen, buf);\n \t\tif (!len) {\n \t\t\treturn len; /* syntax Ok, not enough switches */\n@@ -1523,7 +1526,8 @@ int repo_interpret_branch_name(struct repository *r,\n \t\t\tif (len == namelen)\n \t\t\t\treturn len; /* consumed all */\n \t\t\telse\n-\t\t\t\treturn reinterpret(r, name, namelen, len, buf, allowed);\n+\t\t\t\treturn reinterpret(r, name, namelen, len, buf,\n+\t\t\t\t\t\t   options->allowed);\n \t\t}\n \t}\n \n@@ -1531,22 +1535,22 @@ int repo_interpret_branch_name(struct repository *r,\n \t     (at = memchr(start, '@', namelen - (start - name)));\n \t     start = at + 1) {\n \n-\t\tif (!allowed || (allowed & INTERPRET_BRANCH_HEAD)) {\n+\t\tif (!options->allowed || (options->allowed & INTERPRET_BRANCH_HEAD)) {\n \t\t\tlen = interpret_empty_at(name, namelen, at - name, buf);\n \t\t\tif (len > 0)\n \t\t\t\treturn reinterpret(r, name, namelen, len, buf,\n-\t\t\t\t\t\t   allowed);\n+\t\t\t\t\t\t   options->allowed);\n \t\t}\n \n \t\tlen = interpret_branch_mark(r, name, namelen, at - name, buf,\n \t\t\t\t\t    upstream_mark, branch_get_upstream,\n-\t\t\t\t\t    allowed);\n+\t\t\t\t\t    options);\n \t\tif (len > 0)\n \t\t\treturn len;\n \n \t\tlen = interpret_branch_mark(r, name, namelen, at - name, buf,\n \t\t\t\t\t    push_mark, branch_get_push,\n-\t\t\t\t\t    allowed);\n+\t\t\t\t\t    options);\n \t\tif (len > 0)\n \t\t\treturn len;\n \t}\n@@ -1557,7 +1561,10 @@ int repo_interpret_branch_name(struct repository *r,\n void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)\n {\n \tint len = strlen(name);\n-\tint used = interpret_branch_name(name, len, sb, allowed);\n+\tstruct interpret_branch_name_options options = {\n+\t\t.allowed = allowed\n+\t};\n+\tint used = interpret_branch_name(name, len, sb, &options);\n \n \tif (used < 0)\n \t\tused = 0;\n-- \n2.28.0.402.g5ffc5be6b7-goog\n\n"},{"id":"404889","messageId":"cover.1598998706.git.jonathantanmy@google.com","threadId":"53457","inReplyTo":"20200513004058.34456-1-jonathantanmy@google.com","subject":"[PATCH v3 0/3] Fix for git checkout @{u} (non-local) then git status","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-09-01T22:28:06Z","receivedAt":"2020-09-01T22:28:21Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Junio writes [1]:\n\n> I even wonder why dwim_ref() is not defined like so in a header:\n> \n> \t#define dwim_ref(s, l, o, r) \\\n> \t\trepo_dwim_ref(the_repository, (s), (l), (o), (r))\n> \n> Which would force a change like the one we are discussing to keep\n> them in parallel and keep the promise that only difference between\n> the two is that dwim_ref() works with the_repository and the other\n> can take an arbitrary repository.  Perhaps that can be a preliminary\n> clean-up patch before these two patches ;-)\n\nOK done - that's patch 2 here. (I used \"static inline\" instead of a\npreprocessor #define because I prefer not to use the preprocessor whenever\npossible, but switching to #define is fine too.)\n\n> Reducing the size of the diff is a good justification only when the\n> end result is the same.  If it burdens future developers more, that\n> is \"I write less at the expense of forcing others to write more\",\n> which is not quite the same thing.\n\nOK - that makes sense.\n\n[1] https://lore.kernel.org/git/xmqqzh6ag1ih.fsf@gitster.c.googlers.com/\n\nJonathan Tan (3):\n  sha1-name: replace unsigned int with option struct\n  refs: move dwim_ref() to header file\n  wt-status: tolerate dangling marks\n\n archive.c             |  4 ++--\n branch.c              |  2 +-\n builtin/checkout.c    |  4 ++--\n builtin/fast-export.c |  2 +-\n builtin/log.c         |  2 +-\n builtin/merge.c       |  2 +-\n builtin/reset.c       |  2 +-\n builtin/rev-parse.c   |  2 +-\n builtin/show-branch.c |  2 +-\n builtin/stash.c       |  2 +-\n bundle.c              |  2 +-\n cache.h               | 27 ++++++++++++++++++--------\n commit.c              |  2 +-\n refs.c                | 20 +++++++++----------\n refs.h                | 12 ++++++++++--\n remote.c              |  2 +-\n revision.c            |  3 ++-\n sha1-name.c           | 45 ++++++++++++++++++++++++++++---------------\n t/t7508-status.sh     | 12 ++++++++++++\n wt-status.c           |  2 +-\n 20 files changed, 98 insertions(+), 53 deletions(-)\n\nRange-diff against v2:\n-:  ---------- > 1:  6b3e3077e6 refs: move dwim_ref() to header file\n1:  59b91a206d ! 2:  8f489d9633 wt-status: tolerate dangling marks\n    @@ Commit message\n     \n         Therefore, when calculating the status of a worktree, tolerate dangling\n         marks. This is done by adding an additional parameter to\n    -    repo_dwim_ref().\n    +    dwim_ref() and repo_dwim_ref().\n     \n         Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n     \n    + ## archive.c ##\n    +@@ archive.c: static void parse_treeish_arg(const char **argv,\n    + \t\tconst char *colon = strchrnul(name, ':');\n    + \t\tint refnamelen = colon - name;\n    + \n    +-\t\tif (!dwim_ref(name, refnamelen, &oid, &ref))\n    ++\t\tif (!dwim_ref(name, refnamelen, &oid, &ref, 0))\n    + \t\t\tdie(_(\"no such ref: %.*s\"), refnamelen, name);\n    + \t} else {\n    +-\t\tdwim_ref(name, strlen(name), &oid, &ref);\n    ++\t\tdwim_ref(name, strlen(name), &oid, &ref, 0);\n    + \t}\n    + \n    + \tif (get_oid(name, &oid))\n    +\n    + ## branch.c ##\n    +@@ branch.c: void create_branch(struct repository *r,\n    + \t\tdie(_(\"Not a valid object name: '%s'.\"), start_name);\n    + \t}\n    + \n    +-\tswitch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref)) {\n    ++\tswitch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref, 0)) {\n    + \tcase 0:\n    + \t\t/* Not branching from any existing branch */\n    + \t\tif (explicit_tracking)\n    +\n    + ## builtin/checkout.c ##\n    +@@ builtin/checkout.c: static void setup_branch_path(struct branch_info *branch)\n    + \t * If this is a ref, resolve it; otherwise, look up the OID for our\n    + \t * expression.  Failure here is okay.\n    + \t */\n    +-\tif (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname))\n    ++\tif (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname, 0))\n    + \t\trepo_get_oid_committish(the_repository, branch->name, &branch->oid);\n    + \n    + \tstrbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);\n    +@@ builtin/checkout.c: static void die_expecting_a_branch(const struct branch_info *branch_info)\n    + \tstruct object_id oid;\n    + \tchar *to_free;\n    + \n    +-\tif (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free) == 1) {\n    ++\tif (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free, 0) == 1) {\n    + \t\tconst char *ref = to_free;\n    + \n    + \t\tif (skip_prefix(ref, \"refs/tags/\", &ref))\n    +\n    + ## builtin/fast-export.c ##\n    +@@ builtin/fast-export.c: static void get_tags_and_duplicates(struct rev_cmdline_info *info)\n    + \t\tif (e->flags & UNINTERESTING)\n    + \t\t\tcontinue;\n    + \n    +-\t\tif (dwim_ref(e->name, strlen(e->name), &oid, &full_name) != 1)\n    ++\t\tif (dwim_ref(e->name, strlen(e->name), &oid, &full_name, 0) != 1)\n    + \t\t\tcontinue;\n    + \n    + \t\tif (refspecs.nr) {\n    +\n    + ## builtin/log.c ##\n    +@@ builtin/log.c: static char *find_branch_name(struct rev_info *rev)\n    + \t\treturn NULL;\n    + \tref = rev->cmdline.rev[positive].name;\n    + \ttip_oid = &rev->cmdline.rev[positive].item->oid;\n    +-\tif (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref) &&\n    ++\tif (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref, 0) &&\n    + \t    skip_prefix(full_ref, \"refs/heads/\", &v) &&\n    + \t    oideq(tip_oid, &branch_oid))\n    + \t\tbranch = xstrdup(v);\n    +\n    + ## builtin/merge.c ##\n    +@@ builtin/merge.c: static void merge_name(const char *remote, struct strbuf *msg)\n    + \tif (!remote_head)\n    + \t\tdie(_(\"'%s' does not point to a commit\"), remote);\n    + \n    +-\tif (dwim_ref(remote, strlen(remote), &branch_head, &found_ref) > 0) {\n    ++\tif (dwim_ref(remote, strlen(remote), &branch_head, &found_ref, 0) > 0) {\n    + \t\tif (starts_with(found_ref, \"refs/heads/\")) {\n    + \t\t\tstrbuf_addf(msg, \"%s\\t\\tbranch '%s' of .\\n\",\n    + \t\t\t\t    oid_to_hex(&branch_head), remote);\n    +\n    + ## builtin/reset.c ##\n    +@@ builtin/reset.c: int cmd_reset(int argc, const char **argv, const char *prefix)\n    + \t\t\tchar *ref = NULL;\n    + \t\t\tint err;\n    + \n    +-\t\t\tdwim_ref(rev, strlen(rev), &dummy, &ref);\n    ++\t\t\tdwim_ref(rev, strlen(rev), &dummy, &ref, 0);\n    + \t\t\tif (ref && !starts_with(ref, \"refs/\"))\n    + \t\t\t\tref = NULL;\n    + \n    +\n    + ## builtin/rev-parse.c ##\n    +@@ builtin/rev-parse.c: static void show_rev(int type, const struct object_id *oid, const char *name)\n    + \t\t\tstruct object_id discard;\n    + \t\t\tchar *full;\n    + \n    +-\t\t\tswitch (dwim_ref(name, strlen(name), &discard, &full)) {\n    ++\t\t\tswitch (dwim_ref(name, strlen(name), &discard, &full, 0)) {\n    + \t\t\tcase 0:\n    + \t\t\t\t/*\n    + \t\t\t\t * Not found -- not a ref.  We could\n    +\n    + ## builtin/show-branch.c ##\n    +@@ builtin/show-branch.c: int cmd_show_branch(int ac, const char **av, const char *prefix)\n    + \t\t\tdie(Q_(\"only %d entry can be shown at one time.\",\n    + \t\t\t       \"only %d entries can be shown at one time.\",\n    + \t\t\t       MAX_REVS), MAX_REVS);\n    +-\t\tif (!dwim_ref(*av, strlen(*av), &oid, &ref))\n    ++\t\tif (!dwim_ref(*av, strlen(*av), &oid, &ref, 0))\n    + \t\t\tdie(_(\"no such ref %s\"), *av);\n    + \n    + \t\t/* Has the base been specified? */\n    +\n    + ## builtin/stash.c ##\n    +@@ builtin/stash.c: static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n    + \tend_of_rev = strchrnul(revision, '@');\n    + \tstrbuf_add(&symbolic, revision, end_of_rev - revision);\n    + \n    +-\tret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref);\n    ++\tret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref, 0);\n    + \tstrbuf_release(&symbolic);\n    + \tswitch (ret) {\n    + \tcase 0: /* Not found, but valid ref */\n    +\n    + ## bundle.c ##\n    +@@ bundle.c: static int write_bundle_refs(int bundle_fd, struct rev_info *revs)\n    + \n    + \t\tif (e->item->flags & UNINTERESTING)\n    + \t\t\tcontinue;\n    +-\t\tif (dwim_ref(e->name, strlen(e->name), &oid, &ref) != 1)\n    ++\t\tif (dwim_ref(e->name, strlen(e->name), &oid, &ref, 0) != 1)\n    + \t\t\tgoto skip_write_ref;\n    + \t\tif (read_ref_full(e->name, RESOLVE_REF_READING, &oid, &flag))\n    + \t\t\tflag = 0;\n    +\n      ## cache.h ##\n     @@ cache.h: struct interpret_branch_name_options {\n      \t * allowed, even ones to refs outside of those namespaces.\n    @@ cache.h: struct interpret_branch_name_options {\n      int repo_interpret_branch_name(struct repository *r,\n      \t\t\t       const char *str, int len,\n     \n    + ## commit.c ##\n    +@@ commit.c: struct commit *get_fork_point(const char *refname, struct commit *commit)\n    + \tstruct commit *ret = NULL;\n    + \tchar *full_refname;\n    + \n    +-\tswitch (dwim_ref(refname, strlen(refname), &oid, &full_refname)) {\n    ++\tswitch (dwim_ref(refname, strlen(refname), &oid, &full_refname, 0)) {\n    + \tcase 0:\n    + \t\tdie(\"No such ref: '%s'\", refname);\n    + \tcase 1:\n    +\n      ## refs.c ##\n     @@ refs.c: const char *git_default_branch_name(void)\n       * to name a branch.\n    @@ refs.c: static char *substitute_branch_name(struct repository *r,\n      \tint   refs_found  = expand_ref(r, str, len, oid, ref);\n      \tfree(last_branch);\n      \treturn refs_found;\n    -@@ refs.c: int repo_dwim_ref(struct repository *r, const char *str, int len,\n    - \n    - int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)\n    - {\n    --\treturn repo_dwim_ref(the_repository, str, len, oid, ref);\n    -+\treturn repo_dwim_ref(the_repository, str, len, oid, ref, 0);\n    - }\n    - \n    - int expand_ref(struct repository *repo, const char *str, int len,\n     @@ refs.c: int repo_dwim_log(struct repository *r, const char *str, int len,\n      \t\t  struct object_id *oid, char **log)\n      {\n    @@ refs.h: struct strvec;\n     +int repo_dwim_ref(struct repository *r, const char *str, int len,\n     +\t\t  struct object_id *oid, char **ref, int nonfatal_dangling_mark);\n      int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n    - int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);\n    + static inline int dwim_ref(const char *str, int len, struct object_id *oid,\n    +-\t\t\t   char **ref)\n    ++\t\t\t   char **ref, int nonfatal_dangling_mark)\n    + {\n    +-\treturn repo_dwim_ref(the_repository, str, len, oid, ref);\n    ++\treturn repo_dwim_ref(the_repository, str, len, oid, ref,\n    ++\t\t\t     nonfatal_dangling_mark);\n    + }\n      int dwim_log(const char *str, int len, struct object_id *oid, char **ref);\n    + \n    +\n    + ## remote.c ##\n    +@@ remote.c: static void set_merge(struct branch *ret)\n    + \t\t    strcmp(ret->remote_name, \".\"))\n    + \t\t\tcontinue;\n    + \t\tif (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]),\n    +-\t\t\t     &oid, &ref) == 1)\n    ++\t\t\t     &oid, &ref, 0) == 1)\n    + \t\t\tret->merge[i]->dst = ref;\n    + \t\telse\n    + \t\t\tret->merge[i]->dst = xstrdup(ret->merge_name[i]);\n     \n      ## sha1-name.c ##\n     @@ sha1-name.c: static int get_oid_basic(struct repository *r, const char *str, int len,\n    @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,\n      \t}\n      \n     -\tif (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&\n    -+\tif (repo_dwim_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&\n    ++\tif (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&\n      \t    /* sha1 is a commit? match without further lookup */\n      \t    (oideq(&cb.noid, &oid) ||\n      \t     /* perhaps sha1 is a tag, try to dereference to a commit */\n-- \n2.28.0.402.g5ffc5be6b7-goog\n\n"},{"id":"404890","messageId":"6b3e3077e6db3aff3295c66b6a030ac1de8288cc.1598998707.git.jonathantanmy@google.com","threadId":"53457","inReplyTo":"cover.1598998706.git.jonathantanmy@google.com","subject":"[PATCH v3 2/3] refs: move dwim_ref() to header file","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-09-01T22:28:08Z","receivedAt":"2020-09-01T22:28:26Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"This makes it clear that dwim_ref() is just repo_dwim_ref() without the\nfirst parameter.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n refs.c | 5 -----\n refs.h | 8 +++++++-\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex cf09cd039f..0fa6067340 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -623,11 +623,6 @@ int repo_dwim_ref(struct repository *r, const char *str, int len,\n \treturn refs_found;\n }\n \n-int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)\n-{\n-\treturn repo_dwim_ref(the_repository, str, len, oid, ref);\n-}\n-\n int expand_ref(struct repository *repo, const char *str, int len,\n \t       struct object_id *oid, char **ref)\n {\ndiff --git a/refs.h b/refs.h\nindex 29e28124cd..8cbef96a8d 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -1,6 +1,8 @@\n #ifndef REFS_H\n #define REFS_H\n \n+#include \"cache.h\"\n+\n struct object_id;\n struct ref_store;\n struct repository;\n@@ -151,7 +153,11 @@ void expand_ref_prefix(struct strvec *prefixes, const char *prefix);\n int expand_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n int repo_dwim_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n-int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);\n+static inline int dwim_ref(const char *str, int len, struct object_id *oid,\n+\t\t\t   char **ref)\n+{\n+\treturn repo_dwim_ref(the_repository, str, len, oid, ref);\n+}\n int dwim_log(const char *str, int len, struct object_id *oid, char **ref);\n \n /*\n-- \n2.28.0.402.g5ffc5be6b7-goog\n\n"},{"id":"404891","messageId":"8f489d963333acfb08cdfadea267c9583e3e586f.1598998707.git.jonathantanmy@google.com","threadId":"53457","inReplyTo":"cover.1598998706.git.jonathantanmy@google.com","subject":"[PATCH v3 3/3] wt-status: tolerate dangling marks","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2020-09-01T22:28:09Z","receivedAt":"2020-09-01T22:28:30Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"When a user checks out the upstream branch of HEAD, the upstream branch\nnot being a local branch, and then runs \"git status\", like this:\n\n  git clone $URL client\n  cd client\n  git checkout @{u}\n  git status\n\nno status is printed, but instead an error message:\n\n  fatal: HEAD does not point to a branch\n\n(This error message when running \"git branch\" persists even after\nchecking out other things - it only stops after checking out a branch.)\n\nThis is because \"git status\" reads the reflog when determining the \"HEAD\ndetached\" message, and thus attempts to DWIM \"@{u}\", but that doesn't\nwork because HEAD no longer points to a branch.\n\nTherefore, when calculating the status of a worktree, tolerate dangling\nmarks. This is done by adding an additional parameter to\ndwim_ref() and repo_dwim_ref().\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n archive.c             |  4 ++--\n branch.c              |  2 +-\n builtin/checkout.c    |  4 ++--\n builtin/fast-export.c |  2 +-\n builtin/log.c         |  2 +-\n builtin/merge.c       |  2 +-\n builtin/reset.c       |  2 +-\n builtin/rev-parse.c   |  2 +-\n builtin/show-branch.c |  2 +-\n builtin/stash.c       |  2 +-\n bundle.c              |  2 +-\n cache.h               |  7 +++++++\n commit.c              |  2 +-\n refs.c                | 14 +++++++++-----\n refs.h                |  8 +++++---\n remote.c              |  2 +-\n sha1-name.c           | 16 +++++++++++-----\n t/t7508-status.sh     | 12 ++++++++++++\n wt-status.c           |  2 +-\n 19 files changed, 60 insertions(+), 29 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex fb39706120..0de6048bfc 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -397,10 +397,10 @@ static void parse_treeish_arg(const char **argv,\n \t\tconst char *colon = strchrnul(name, ':');\n \t\tint refnamelen = colon - name;\n \n-\t\tif (!dwim_ref(name, refnamelen, &oid, &ref))\n+\t\tif (!dwim_ref(name, refnamelen, &oid, &ref, 0))\n \t\t\tdie(_(\"no such ref: %.*s\"), refnamelen, name);\n \t} else {\n-\t\tdwim_ref(name, strlen(name), &oid, &ref);\n+\t\tdwim_ref(name, strlen(name), &oid, &ref, 0);\n \t}\n \n \tif (get_oid(name, &oid))\ndiff --git a/branch.c b/branch.c\nindex 7095f78058..9c9dae1eae 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -281,7 +281,7 @@ void create_branch(struct repository *r,\n \t\tdie(_(\"Not a valid object name: '%s'.\"), start_name);\n \t}\n \n-\tswitch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref)) {\n+\tswitch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref, 0)) {\n \tcase 0:\n \t\t/* Not branching from any existing branch */\n \t\tif (explicit_tracking)\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex bba64108af..1f10cc93dd 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -651,7 +651,7 @@ static void setup_branch_path(struct branch_info *branch)\n \t * If this is a ref, resolve it; otherwise, look up the OID for our\n \t * expression.  Failure here is okay.\n \t */\n-\tif (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname))\n+\tif (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname, 0))\n \t\trepo_get_oid_committish(the_repository, branch->name, &branch->oid);\n \n \tstrbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);\n@@ -1345,7 +1345,7 @@ static void die_expecting_a_branch(const struct branch_info *branch_info)\n \tstruct object_id oid;\n \tchar *to_free;\n \n-\tif (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free) == 1) {\n+\tif (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free, 0) == 1) {\n \t\tconst char *ref = to_free;\n \n \t\tif (skip_prefix(ref, \"refs/tags/\", &ref))\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 9f37895d4c..1b8fca3ee0 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -943,7 +943,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)\n \t\tif (e->flags & UNINTERESTING)\n \t\t\tcontinue;\n \n-\t\tif (dwim_ref(e->name, strlen(e->name), &oid, &full_name) != 1)\n+\t\tif (dwim_ref(e->name, strlen(e->name), &oid, &full_name, 0) != 1)\n \t\t\tcontinue;\n \n \t\tif (refspecs.nr) {\ndiff --git a/builtin/log.c b/builtin/log.c\nindex b58f8da09e..4ec7f57cf4 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1061,7 +1061,7 @@ static char *find_branch_name(struct rev_info *rev)\n \t\treturn NULL;\n \tref = rev->cmdline.rev[positive].name;\n \ttip_oid = &rev->cmdline.rev[positive].item->oid;\n-\tif (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref) &&\n+\tif (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref, 0) &&\n \t    skip_prefix(full_ref, \"refs/heads/\", &v) &&\n \t    oideq(tip_oid, &branch_oid))\n \t\tbranch = xstrdup(v);\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 74829a838e..2af70b5605 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -500,7 +500,7 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \tif (!remote_head)\n \t\tdie(_(\"'%s' does not point to a commit\"), remote);\n \n-\tif (dwim_ref(remote, strlen(remote), &branch_head, &found_ref) > 0) {\n+\tif (dwim_ref(remote, strlen(remote), &branch_head, &found_ref, 0) > 0) {\n \t\tif (starts_with(found_ref, \"refs/heads/\")) {\n \t\t\tstrbuf_addf(msg, \"%s\\t\\tbranch '%s' of .\\n\",\n \t\t\t\t    oid_to_hex(&branch_head), remote);\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 8ae69d6f2b..c635b062c3 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -423,7 +423,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tchar *ref = NULL;\n \t\t\tint err;\n \n-\t\t\tdwim_ref(rev, strlen(rev), &dummy, &ref);\n+\t\t\tdwim_ref(rev, strlen(rev), &dummy, &ref, 0);\n \t\t\tif (ref && !starts_with(ref, \"refs/\"))\n \t\t\t\tref = NULL;\n \ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 669dd2fd6f..ed200c8af1 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -136,7 +136,7 @@ static void show_rev(int type, const struct object_id *oid, const char *name)\n \t\t\tstruct object_id discard;\n \t\t\tchar *full;\n \n-\t\t\tswitch (dwim_ref(name, strlen(name), &discard, &full)) {\n+\t\t\tswitch (dwim_ref(name, strlen(name), &discard, &full, 0)) {\n \t\t\tcase 0:\n \t\t\t\t/*\n \t\t\t\t * Not found -- not a ref.  We could\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex 7eae5f3801..d6d2dabeca 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -741,7 +741,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\tdie(Q_(\"only %d entry can be shown at one time.\",\n \t\t\t       \"only %d entries can be shown at one time.\",\n \t\t\t       MAX_REVS), MAX_REVS);\n-\t\tif (!dwim_ref(*av, strlen(*av), &oid, &ref))\n+\t\tif (!dwim_ref(*av, strlen(*av), &oid, &ref, 0))\n \t\t\tdie(_(\"no such ref %s\"), *av);\n \n \t\t/* Has the base been specified? */\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 4bdfaf8397..3f811f3050 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -185,7 +185,7 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n \tend_of_rev = strchrnul(revision, '@');\n \tstrbuf_add(&symbolic, revision, end_of_rev - revision);\n \n-\tret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref);\n+\tret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref, 0);\n \tstrbuf_release(&symbolic);\n \tswitch (ret) {\n \tcase 0: /* Not found, but valid ref */\ndiff --git a/bundle.c b/bundle.c\nindex 995a940dfd..cb0e5931ac 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -403,7 +403,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)\n \n \t\tif (e->item->flags & UNINTERESTING)\n \t\t\tcontinue;\n-\t\tif (dwim_ref(e->name, strlen(e->name), &oid, &ref) != 1)\n+\t\tif (dwim_ref(e->name, strlen(e->name), &oid, &ref, 0) != 1)\n \t\t\tgoto skip_write_ref;\n \t\tif (read_ref_full(e->name, RESOLVE_REF_READING, &oid, &flag))\n \t\t\tflag = 0;\ndiff --git a/cache.h b/cache.h\nindex 4f16a57ba4..cee8aa5dc3 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1569,6 +1569,13 @@ struct interpret_branch_name_options {\n \t * allowed, even ones to refs outside of those namespaces.\n \t */\n \tunsigned allowed;\n+\n+\t/*\n+\t * If ^{upstream} or ^{push} (or equivalent) is requested, and the\n+\t * branch in question does not have such a reference, return -1 instead\n+\t * of die()-ing.\n+\t */\n+\tunsigned nonfatal_dangling_mark : 1;\n };\n int repo_interpret_branch_name(struct repository *r,\n \t\t\t       const char *str, int len,\ndiff --git a/commit.c b/commit.c\nindex 4ce8cb38d5..119892abbc 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -921,7 +921,7 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)\n \tstruct commit *ret = NULL;\n \tchar *full_refname;\n \n-\tswitch (dwim_ref(refname, strlen(refname), &oid, &full_refname)) {\n+\tswitch (dwim_ref(refname, strlen(refname), &oid, &full_refname, 0)) {\n \tcase 0:\n \t\tdie(\"No such ref: '%s'\", refname);\n \tcase 1:\ndiff --git a/refs.c b/refs.c\nindex 0fa6067340..e06ee22a8a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -598,10 +598,13 @@ const char *git_default_branch_name(void)\n  * to name a branch.\n  */\n static char *substitute_branch_name(struct repository *r,\n-\t\t\t\t    const char **string, int *len)\n+\t\t\t\t    const char **string, int *len,\n+\t\t\t\t    int nonfatal_dangling_mark)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct interpret_branch_name_options options = { 0 } ;\n+\tstruct interpret_branch_name_options options = {\n+\t\t.nonfatal_dangling_mark = nonfatal_dangling_mark\n+\t};\n \tint ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);\n \n \tif (ret == *len) {\n@@ -615,9 +618,10 @@ static char *substitute_branch_name(struct repository *r,\n }\n \n int repo_dwim_ref(struct repository *r, const char *str, int len,\n-\t\t  struct object_id *oid, char **ref)\n+\t\t  struct object_id *oid, char **ref, int nonfatal_dangling_mark)\n {\n-\tchar *last_branch = substitute_branch_name(r, &str, &len);\n+\tchar *last_branch = substitute_branch_name(r, &str, &len,\n+\t\t\t\t\t\t   nonfatal_dangling_mark);\n \tint   refs_found  = expand_ref(r, str, len, oid, ref);\n \tfree(last_branch);\n \treturn refs_found;\n@@ -661,7 +665,7 @@ int repo_dwim_log(struct repository *r, const char *str, int len,\n \t\t  struct object_id *oid, char **log)\n {\n \tstruct ref_store *refs = get_main_ref_store(r);\n-\tchar *last_branch = substitute_branch_name(r, &str, &len);\n+\tchar *last_branch = substitute_branch_name(r, &str, &len, 0);\n \tconst char **p;\n \tint logs_found = 0;\n \tstruct strbuf path = STRBUF_INIT;\ndiff --git a/refs.h b/refs.h\nindex 8cbef96a8d..e03c106320 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -151,12 +151,14 @@ struct strvec;\n void expand_ref_prefix(struct strvec *prefixes, const char *prefix);\n \n int expand_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n-int repo_dwim_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n+int repo_dwim_ref(struct repository *r, const char *str, int len,\n+\t\t  struct object_id *oid, char **ref, int nonfatal_dangling_mark);\n int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);\n static inline int dwim_ref(const char *str, int len, struct object_id *oid,\n-\t\t\t   char **ref)\n+\t\t\t   char **ref, int nonfatal_dangling_mark)\n {\n-\treturn repo_dwim_ref(the_repository, str, len, oid, ref);\n+\treturn repo_dwim_ref(the_repository, str, len, oid, ref,\n+\t\t\t     nonfatal_dangling_mark);\n }\n int dwim_log(const char *str, int len, struct object_id *oid, char **ref);\n \ndiff --git a/remote.c b/remote.c\nindex c5ed74f91c..420150837b 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1558,7 +1558,7 @@ static void set_merge(struct branch *ret)\n \t\t    strcmp(ret->remote_name, \".\"))\n \t\t\tcontinue;\n \t\tif (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]),\n-\t\t\t     &oid, &ref) == 1)\n+\t\t\t     &oid, &ref, 0) == 1)\n \t\t\tret->merge[i]->dst = ref;\n \t\telse\n \t\t\tret->merge[i]->dst = xstrdup(ret->merge_name[i]);\ndiff --git a/sha1-name.c b/sha1-name.c\nindex a7a9de66c4..0b23b86ceb 100644\n--- a/sha1-name.c\n+++ b/sha1-name.c\n@@ -809,7 +809,7 @@ static int get_oid_basic(struct repository *r, const char *str, int len,\n \n \tif (len == r->hash_algo->hexsz && !get_oid_hex(str, oid)) {\n \t\tif (warn_ambiguous_refs && warn_on_object_refname_ambiguity) {\n-\t\t\trefs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref);\n+\t\t\trefs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref, 0);\n \t\t\tif (refs_found > 0) {\n \t\t\t\twarning(warn_msg, len, str);\n \t\t\t\tif (advice_object_name_warning)\n@@ -860,11 +860,11 @@ static int get_oid_basic(struct repository *r, const char *str, int len,\n \n \tif (!len && reflog_len)\n \t\t/* allow \"@{...}\" to mean the current branch reflog */\n-\t\trefs_found = repo_dwim_ref(r, \"HEAD\", 4, oid, &real_ref);\n+\t\trefs_found = repo_dwim_ref(r, \"HEAD\", 4, oid, &real_ref, 0);\n \telse if (reflog_len)\n \t\trefs_found = repo_dwim_log(r, str, len, oid, &real_ref);\n \telse\n-\t\trefs_found = repo_dwim_ref(r, str, len, oid, &real_ref);\n+\t\trefs_found = repo_dwim_ref(r, str, len, oid, &real_ref, 0);\n \n \tif (!refs_found)\n \t\treturn -1;\n@@ -1496,8 +1496,14 @@ static int interpret_branch_mark(struct repository *r,\n \t\tbranch = branch_get(NULL);\n \n \tvalue = get_data(branch, &err);\n-\tif (!value)\n-\t\tdie(\"%s\", err.buf);\n+\tif (!value) {\n+\t\tif (options->nonfatal_dangling_mark) {\n+\t\t\tstrbuf_release(&err);\n+\t\t\treturn -1;\n+\t\t} else {\n+\t\t\tdie(\"%s\", err.buf);\n+\t\t}\n+\t}\n \n \tif (!branch_interpret_allowed(value, options->allowed))\n \t\treturn -1;\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex e81759319f..45e1f6ff68 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -846,6 +846,18 @@ test_expect_success 'status refreshes the index' '\n \ttest_cmp expect output\n '\n \n+test_expect_success 'status shows detached HEAD properly after checking out non-local upstream branch' '\n+\ttest_when_finished rm -rf upstream downstream actual &&\n+\n+\ttest_create_repo upstream &&\n+\ttest_commit -C upstream foo &&\n+\n+\tgit clone upstream downstream &&\n+\tgit -C downstream checkout @{u} &&\n+\tgit -C downstream status >actual &&\n+\ttest_i18ngrep \"HEAD detached at [0-9a-f]\\\\+\" actual\n+'\n+\n test_expect_success 'setup status submodule summary' '\n \ttest_create_repo sm && (\n \t\tcd sm &&\ndiff --git a/wt-status.c b/wt-status.c\nindex 7ce58b8aae..a99b7a0c59 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1569,7 +1569,7 @@ static void wt_status_get_detached_from(struct repository *r,\n \t\treturn;\n \t}\n \n-\tif (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&\n+\tif (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&\n \t    /* sha1 is a commit? match without further lookup */\n \t    (oideq(&cb.noid, &oid) ||\n \t     /* perhaps sha1 is a tag, try to dereference to a commit */\n-- \n2.28.0.402.g5ffc5be6b7-goog\n\n"}]}