[PATCH] wt-status: expand, not dwim, a "detached from" ref

[PATCH] wt-status: expand, not dwim, a "detached from" ref

[Jonathan Tan]

When a user checks out the upstream branch of HEAD and then runs "git
branch", like this:

  git checkout @{u}
  git branch

an error message is printed:

  fatal: HEAD does not point to a branch

(This error message when running "git branch" persists even after
checking out other things - it only stops after checking out a branch.)

This is because "git branch" attempts to DWIM "@{u}" when determining
the "HEAD detached" message, but that doesn't work because HEAD no
longer points to a branch.

Therefore, when calculating the status of a worktree, only expand such a
string, not DWIM it. Such strings would not match a ref, and "git
branch" would report "HEAD detached at <hash>" instead.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
I tried to track down other uses of wt_status_get_detached_from()
(called from wt_status_get_state() with get_detached_from=1) but there
seemed to be quite a few, so I stopped.

One alternative is to plumb down a flag from dwim_ref() to
interpret_branch_mark() that indicates that (1) don't read HEAD when
processing, and (2) when encountering a ref of the form "<optional
ref>@{u}", don't die when the optional ref doesn't exist, is not a
branch, or does not have an upstream. (We cannot change the
functionality outright because other parts of Git rely on such a message
being printed.) But this seems too complicated just for diagnosis.
---
 t/t3203-branch-output.sh | 10 ++++++++++
 wt-status.c              |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 71818b90f0..c5d3d739e5 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -209,6 +209,16 @@ EOF
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'git branch shows detached HEAD properly after checking out upstream branch' '
+	git init upstream &&
+	test_commit -C upstream foo &&
+
+	git clone upstream downstream &&
+	git -C downstream checkout @{u} &&
+	git -C downstream branch >actual &&
+	test_i18ngrep "HEAD detached at [0-9a-f]\\+" actual
+'
+
 test_expect_success 'git branch `--sort` option' '
 	cat >expect <<-\EOF &&
 	* (HEAD detached from fromtag)
diff --git a/wt-status.c b/wt-status.c
index 98dfa6f73f..f84ebc3e2c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1562,7 +1562,7 @@ static void wt_status_get_detached_from(struct repository *r,
 		return;
 	}
 
-	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
+	if (expand_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
 	    /* sha1 is a commit? match without further lookup */
 	    (oideq(&cb.noid, &oid) ||
 	     /* perhaps sha1 is a tag, try to dereference to a commit */
-- 
2.26.2.645.ge9eca65c58-goog

Re: [PATCH] wt-status: expand, not dwim, a "detached from" ref

[Junio C Hamano]

Jonathan Tan <jonathantanmy@google.com> writes:

> When a user checks out the upstream branch of HEAD and then runs "git
> branch", like this:
>
>   git checkout @{u}
>   git branch
>
> an error message is printed:
>
>   fatal: HEAD does not point to a branch

I had to spend unreasonable amount of time reproducing this.

    $ git branch --show-current
    master
    $ git checkout -b -t naster master
    $ git branch --show-current
    naster
    $ git checkout @{u}
    $ git branch --show-current
    master
    $ git branch
    * master
      naster

What was missing was that the current branch must be forked from a
remote-tracking branch; with a fork from a local branch, the last
step, i.e. "git branch", works just fine.

> (This error message when running "git branch" persists even after
> checking out other things - it only stops after checking out a branch.)

Correct.  And it is also worth pointing out that this is not "git
branch"; the users would see it primarily as a problem with "git
status", which would die the same way.

> This is because "git branch" attempts to DWIM "@{u}" when determining
> the "HEAD detached" message, but that doesn't work because HEAD no
> longer points to a branch.
>
> Therefore, when calculating the status of a worktree, only expand such a
> string, not DWIM it. Such strings would not match a ref, and "git
> branch" would report "HEAD detached at <hash>" instead.

OK.  "Detached at <hash>" is somewhat unsatisfactory (you most
likely have detached from refs/remotes/origin/<something>), but
it is much better than "fatal" X-<.

> One alternative is to plumb down a flag from dwim_ref() to
> interpret_branch_mark() that indicates that (1) don't read HEAD when
> processing, and (2) when encountering a ref of the form "<optional
> ref>@{u}", don't die when the optional ref doesn't exist, is not a
> branch, or does not have an upstream. (We cannot change the
> functionality outright because other parts of Git rely on such a message
> being printed.)

Thanks for a good analysis.  That likely is a good direction for a
longer term.  @{upstream} is "tell me the upstream of the branch
that is currently checked out", right?  So it is reasonable to
expect that it has no good answer in a detached HEAD state.  And
when on a branch, that branch may not have an upstream, and we
should prepare for such a case, too.

> diff --git a/wt-status.c b/wt-status.c
> index 98dfa6f73f..f84ebc3e2c 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1562,7 +1562,7 @@ static void wt_status_get_detached_from(struct repository *r,
>  		return;
>  	}
>  
> -	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
> +	if (expand_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
>  	    /* sha1 is a commit? match without further lookup */
>  	    (oideq(&cb.noid, &oid) ||
>  	     /* perhaps sha1 is a tag, try to dereference to a commit */

Hmph, I just did this:

    $ git branch -b -t next origin/next
    $ git checkout next@{upstream}
    $ git status

It used to say "HEAD detached at origin/next" without this change,
but now it says "HEAD detached at 046d49d455", which smells like a
regression.

Re: [PATCH] wt-status: expand, not dwim, a "detached from" ref

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> OK.  "Detached at <hash>" is somewhat unsatisfactory (you most
> likely have detached from refs/remotes/origin/<something>), but
> it is much better than "fatal" X-<.
>
>> One alternative is to plumb down a flag from dwim_ref() to
>> interpret_branch_mark() that indicates that (1) don't read HEAD when
>> processing, and (2) when encountering a ref of the form "<optional
>> ref>@{u}", don't die when the optional ref doesn't exist, is not a
>> branch, or does not have an upstream. (We cannot change the
>> functionality outright because other parts of Git rely on such a message
>> being printed.)
>
> Thanks for a good analysis.  That likely is a good direction for a
> longer term.  @{upstream} is "tell me the upstream of the branch
> that is currently checked out", right?  So it is reasonable to
> expect that it has no good answer in a detached HEAD state.  And
> when on a branch, that branch may not have an upstream, and we
> should prepare for such a case, too.

Well, I have to take this back.

The real cause of the problem is that we record in reflog that we
switched to @{u}, but when get_detached_from() tries to "dwim" it,
the "current branch" that is implicitly there to be applied to @{u}
is different one from what used to compute which branch to check
out, isn't it?  And it probably is not limited to @{upstream} but
other @{<stuff>} that implicitly apply to the "current branch",
e.g. @{push}.

This causes a few potential problems:

 - The "current" branch may be different from the one when such a
   reflog entry that refers to @{<stuff>} when we later yank
   @{<stuff>} out of the reflog and try to use it.  This is the
   problem the patch under discussion tries to hide, but the patch
   also breaks cases that are working fine.

 - Even if the user gave the branch name (i.e. 'next@{upstream}' in
   the example this patch would have introduced a regression) or if
   we updated get_detached_from() to correctly infer the branch that
   was current when the reflog entry in which we found @{upstream}
   was recorded, the upstream for the branch may have been
   reconfigured between the time when the reflog entry was written
   and get_detached_from() is called.  'next@{upstream}' may have
   been 'origin/next' back then but it may be a different
   remote-tracking branch now.  This issue is not solved by the
   patch---the issue is unfixable unless we log the changes to the
   branch configuration so that we can figure out what was the
   upstream of 'next' back then, which we won't do.

Assuming that is the root cause, I think the right solution likely
lies along these three lines:

 (1) record not @{<stuff>} (or <branch>@{<stuff>} for that matter),
     but the actual starting point in the reflog (e.g. in the
     example this patch would have introduced a regression,
     i.e. next@{u}, we should record 'origin/next'.  In the example
     this patch would have used degraded output to prevent dying,
     i.e. @{u}, we should also record 'origin/next')---this also
     will fix the "the branch's upstream may be different now"
     problem.

 (2) a patch like yours to use expand_ref() as a fallback, but only
     for the "@{<stuff>}" notation that depends on what the "then
     current" branch was---this is a mere band-aid, like the patch
     under discussion is, but at least it would not regress the
     cases that are "working".  "The upstream may be different now"
     problem is still there.

 (3) when we have to interpret @{<stuff>} found in the reflog, go
     back to see which branch was current, and compute @{<stuff>}
     relative to that branch---this would "fix" more cases than (2)
     would, but it won't fix "the upstream can change" problem, and
     I think the trade-off is not all that great.

I think the combination of (1) and (2) is likely to be the best way
to go.  (1) will remove the root cause, and (2) will allow us to
cope with reflog entries before (1) is applied.

Re: [PATCH] wt-status: expand, not dwim, a "detached from" ref

[Jonathan Tan]

>  (1) record not @{<stuff>} (or <branch>@{<stuff>} for that matter),
>      but the actual starting point in the reflog (e.g. in the
>      example this patch would have introduced a regression,
>      i.e. next@{u}, we should record 'origin/next'.  In the example
>      this patch would have used degraded output to prevent dying,
>      i.e. @{u}, we should also record 'origin/next')---this also
>      will fix the "the branch's upstream may be different now"
>      problem.

This sounds reasonable. I took a look at this.

The part that converts the user-given refname (e.g. "@{u}") into an OID
is the invocation of get_oid_mb() in parse_branchname_arg() in
builtin/checkout.c, and get_oid_mb() eventually calls repo_dwim_ref()
which has access to the absolute branch name ("origin/master"). I did
not try plumbing it all the way, but I tried overriding "arg" with
"refs/remotes/origin/master" after the call to get_oid_mb() and it
worked.

For reference, the stack between get_oid_mb() and repo_dwim_ref() is as
follows (the line numbers may not be accurate because of some debug
statements I added):

  repo_dwim_ref (refs.c:597)                                                                                
  get_oid_basic (sha1-name.c:875)                                                                           
  get_oid_1 (sha1-name.c:1195)                                                                              
  get_oid_with_context_1 (sha1-name.c:1812)                                                         
  get_oid_with_context (sha1-name.c:1959)
  repo_get_oid (sha1-name.c:1610)
  repo_get_oid_mb (sha1-name.c:1382)

Besides the increase in complicatedness of all the listed functions that
we would need in order to plumb the absolute branch name through, I
haven't checked if the absolute branch name is the one that we should
use whenever we write to the reflog, or if there are some times that we
still want to use the user-specified name. I'll take a further look, but
any ideas are welcome.

Re: [PATCH] wt-status: expand, not dwim, a "detached from" ref

[Jonathan Nieder]

Hi,

Jonathan Tan wrote:

> When a user checks out the upstream branch of HEAD and then runs "git
> branch", like this:
>
>   git checkout @{u}
>   git branch
>
> an error message is printed:
>
>   fatal: HEAD does not point to a branch

Interesting.  Even worse, it happens when I run "git status".

[...]
> This is because "git branch" attempts to DWIM "@{u}" when determining
> the "HEAD detached" message, but that doesn't work because HEAD no
> longer points to a branch.
>
> Therefore, when calculating the status of a worktree, only expand such a
> string, not DWIM it. Such strings would not match a ref, and "git
> branch" would report "HEAD detached at <hash>" instead.
[...]
> -	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
> +	if (expand_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
>  	    /* sha1 is a commit? match without further lookup */

Alas, as Junio mentions downthread, this patch produces a regression
in behavior: before,

	$ git checkout --quiet master@{u}
	$ git status | head -1
	HEAD detached at origin/master

after,

	$ git checkout --quiet master@{u}
	$ git status | head -1
	HEAD detached at 675a4aaf3b2

This ends up being a bit subtle.

The current branch can be in one of three states: I can be on a branch
(HEAD symref pointing to a ref refs/heads/branchname), detached (HEAD
psuedoref pointing to a commit), or on a branch yet to be born.
__git_ps1 in contrib/completion/git-prompt.sh, for example, is able to
show these three states.

Since b397ea4863a (status: show more info than "currently not on any
branch", 2013-03-13), "git status", on the other hand, gives richer
information.  In the detached case, naming the commit that HEAD points
to doesn't really give a user enough information to orient themself,
so we look at the reflog (!) to find out what the user had intended to
check out.  Reflog entries look like

	checkout: moving from master to v1.0

and the "v1.0" can tell us that v1.0 is the tag that we detached HEAD
on.

This strikes me as always having been strange in a few ways:

On one hand, this is using the reflog.  reflog entries are
historically meant to be human-readable.  They are a raw string, not
structured data.  They can be translated to a different human
language.  Other tools interacting with git repositories are not
guaranteed to use the same string.  Changes such as the introduction
of "git switch" could be expected to lead to writing "switch:" instead
of "checkout:" some day.

Beyond that, it involves guesswork.  As b397ea4863a explains,

	When it cannot figure out the original ref, it shows an
	abbreviated SHA-1.

The reflog entry contains the parameter passed to "git checkout" in
the form the user provided --- it is not a full ref name or string
meant to be appended to refs/heads/ but it can be arbitrary syntax
supported by "git checkout".  At the time that the checkout happened,
we know what *commit* that name resolved to, but we do not know what
ref it resolved to:

- refs in the search path can have been created or deleted since then
- with @{u} syntax, the named branch's upstream can have been
  reconfigured
- and so on

If we want to know what ref HEAD was detached at, wouldn't we want
some reliable source of that information that records it at the time
it was known?

Another example is if I used "git checkout -" syntax.  In that case,
the reflog records the commit id that I am checking out, instead of
recording "-".  That's because (after "-" is replaced by "@{-1}") the
"magic branch" handler strbuf_branchname replaces @{-1} with with the
branch name being checked out.  That branch name *also* comes from the
reflog, this time from the <old> side of the

	checkout: moving from <old> to <new>

line, and <old> is populated as a simple commit id or branch name read
from HEAD.  So this creates a bit of an inconsistency: if I run "git
status", "git checkout -", "git checkout -" again, and then "git
status" again, the first "git status" tells me what ref I had used to
detach HEAD but the second does not.

Okay, so much for background.

The motivating error producing

	fatal: HEAD does not point to a branch

is a special case of the "we do not know what ref it resolved to"
problem described above: the string @{upstream} represents the
upstream of the branch that HEAD pointed to *at the time of the
checkout*, but since then HEAD has been detached.

[...]
> One alternative is to plumb down a flag from dwim_ref() to
> interpret_branch_mark() that indicates that (1) don't read HEAD when
> processing, and (2) when encountering a ref of the form "<optional
> ref>@{u}", don't die when the optional ref doesn't exist, is not a
> branch, or does not have an upstream.

Given the context above, two possibilities seem appealing:

 A. Like you're hinting, could dwim_ref get a variant that returns -1
    instead of die()ing on failure?  That way, we could fulfill the
    intent described in b397ea48:

	When it cannot figure out the original ref, it shows an abbreviated
	SHA-1.

 B. Alternatively, in the @{u} case could we switch together the <old>
    and <new> pieces of the context from the

	checkout: moving from master to @{upstream}

    reflog line to make "master@{upstream}"?  It's still possible for
    the upstream to have changed since then, but at least in the
    common case this would match the lookup that happened at checkout
    time.

Thoughts?

Thanks,
Jonathan

Re: [PATCH] wt-status: expand, not dwim, a "detached from" ref

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Given the context above, two possibilities seem appealing:
>
>  A. Like you're hinting, could dwim_ref get a variant that returns -1
>     instead of die()ing on failure?  That way, we could fulfill the
>     intent described in b397ea48:
>
> 	When it cannot figure out the original ref, it shows an abbreviated
> 	SHA-1.
>
>  B. Alternatively, in the @{u} case could we switch together the <old>
>     and <new> pieces of the context from the
>
> 	checkout: moving from master to @{upstream}
>
>     reflog line to make "master@{upstream}"?  It's still possible for
>     the upstream to have changed since then, but at least in the
>     common case this would match the lookup that happened at checkout
>     time.

Ah, blast from the past.  Thanks for resurrecting.

If we are allowed to change what goes to reflog, can we do even
better than recording master@{upstream} at the time "checkout"
records the HEAD movement?  "checkout: moving from next to master"
would be far better than "moving from next to next@{upstream}" or
"moving from next to @{upstream}".  

Can we even change the phrasing altogether, like "checkout: moving
from next to detached e9b77c..."?  That would produce even more
precise report.

[PATCH v2 0/2] Fix for git checkout @{u} (non-local) then git status

[Jonathan Tan]

As Jonathan Nieder wrote [1]:

>  A. Like you're hinting, could dwim_ref get a variant that returns -1
>     instead of die()ing on failure?  That way, we could fulfill the
>     intent described in b397ea48:
> 
> 	When it cannot figure out the original ref, it shows an abbreviated
> 	SHA-1.

Here are some patches that do this. As discussed, this is not the
complete solution, but at least now we can handle unresolvable marks.

I've also switched the commit messages and test from mentioning "git
branch" to mentioning "git status".

[1] https://lore.kernel.org/git/20200827014723.GA750502@google.com/

Jonathan Tan (2):
  sha1-name: replace unsigned int with option struct
  wt-status: tolerate dangling marks

 cache.h           | 27 +++++++++++++++++++--------
 refs.c            | 17 +++++++++++------
 refs.h            |  3 ++-
 revision.c        |  3 ++-
 sha1-name.c       | 45 +++++++++++++++++++++++++++++----------------
 t/t7508-status.sh | 12 ++++++++++++
 wt-status.c       |  2 +-
 7 files changed, 76 insertions(+), 33 deletions(-)

-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v2 1/2] sha1-name: replace unsigned int with option struct

[Jonathan Tan]

In preparation for a future patch adding a boolean parameter to
repo_interpret_branch_name(), which might be easily confused with an
existing unsigned int parameter, refactor repo_interpret_branch_name()
to take an option struct instead of the unsigned int parameter.

The static function interpret_branch_mark() is also updated to take the
option struct in preparation for that future patch, since it will also
make use of the to-be-introduced boolean parameter.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 cache.h     | 20 ++++++++++++--------
 refs.c      |  3 ++-
 revision.c  |  3 ++-
 sha1-name.c | 29 ++++++++++++++++++-----------
 4 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/cache.h b/cache.h
index 4cad61ffa4..4f16a57ba4 100644
--- a/cache.h
+++ b/cache.h
@@ -1557,21 +1557,25 @@ int parse_oid_hex_any(const char *hex, struct object_id *oid, const char **end);
  *
  * If the input was ok but there are not N branch switches in the
  * reflog, it returns 0.
- *
- * If "allowed" is non-zero, it is a treated as a bitfield of allowable
- * expansions: local branches ("refs/heads/"), remote branches
- * ("refs/remotes/"), or "HEAD". If no "allowed" bits are set, any expansion is
- * allowed, even ones to refs outside of those namespaces.
  */
 #define INTERPRET_BRANCH_LOCAL (1<<0)
 #define INTERPRET_BRANCH_REMOTE (1<<1)
 #define INTERPRET_BRANCH_HEAD (1<<2)
+struct interpret_branch_name_options {
+	/*
+	 * If "allowed" is non-zero, it is a treated as a bitfield of allowable
+	 * expansions: local branches ("refs/heads/"), remote branches
+	 * ("refs/remotes/"), or "HEAD". If no "allowed" bits are set, any expansion is
+	 * allowed, even ones to refs outside of those namespaces.
+	 */
+	unsigned allowed;
+};
 int repo_interpret_branch_name(struct repository *r,
 			       const char *str, int len,
 			       struct strbuf *buf,
-			       unsigned allowed);
-#define interpret_branch_name(str, len, buf, allowed) \
-	repo_interpret_branch_name(the_repository, str, len, buf, allowed)
+			       const struct interpret_branch_name_options *options);
+#define interpret_branch_name(str, len, buf, options) \
+	repo_interpret_branch_name(the_repository, str, len, buf, options)
 
 int validate_headref(const char *ref);
 
diff --git a/refs.c b/refs.c
index 3ee3afaf41..cf09cd039f 100644
--- a/refs.c
+++ b/refs.c
@@ -601,7 +601,8 @@ static char *substitute_branch_name(struct repository *r,
 				    const char **string, int *len)
 {
 	struct strbuf buf = STRBUF_INIT;
-	int ret = repo_interpret_branch_name(r, *string, *len, &buf, 0);
+	struct interpret_branch_name_options options = { 0 } ;
+	int ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);
 
 	if (ret == *len) {
 		size_t size;
diff --git a/revision.c b/revision.c
index 96630e3186..1247ee4ec8 100644
--- a/revision.c
+++ b/revision.c
@@ -315,13 +315,14 @@ static void add_pending_object_with_path(struct rev_info *revs,
 					 const char *name, unsigned mode,
 					 const char *path)
 {
+	struct interpret_branch_name_options options = { 0 };
 	if (!obj)
 		return;
 	if (revs->no_walk && (obj->flags & UNINTERESTING))
 		revs->no_walk = 0;
 	if (revs->reflog_info && obj->type == OBJ_COMMIT) {
 		struct strbuf buf = STRBUF_INIT;
-		int len = interpret_branch_name(name, 0, &buf, 0);
+		int len = interpret_branch_name(name, 0, &buf, &options);
 
 		if (0 < len && name[len] && buf.len)
 			strbuf_addstr(&buf, name + len);
diff --git a/sha1-name.c b/sha1-name.c
index 0b8cb5247a..a7a9de66c4 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -1427,9 +1427,12 @@ static int reinterpret(struct repository *r,
 	struct strbuf tmp = STRBUF_INIT;
 	int used = buf->len;
 	int ret;
+	struct interpret_branch_name_options options = {
+		.allowed = allowed
+	};
 
 	strbuf_add(buf, name + len, namelen - len);
-	ret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, allowed);
+	ret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, &options);
 	/* that data was not interpreted, remove our cruft */
 	if (ret < 0) {
 		strbuf_setlen(buf, used);
@@ -1471,7 +1474,7 @@ static int interpret_branch_mark(struct repository *r,
 				 int (*get_mark)(const char *, int),
 				 const char *(*get_data)(struct branch *,
 							 struct strbuf *),
-				 unsigned allowed)
+				 const struct interpret_branch_name_options *options)
 {
 	int len;
 	struct branch *branch;
@@ -1496,7 +1499,7 @@ static int interpret_branch_mark(struct repository *r,
 	if (!value)
 		die("%s", err.buf);
 
-	if (!branch_interpret_allowed(value, allowed))
+	if (!branch_interpret_allowed(value, options->allowed))
 		return -1;
 
 	set_shortened_ref(r, buf, value);
@@ -1506,7 +1509,7 @@ static int interpret_branch_mark(struct repository *r,
 int repo_interpret_branch_name(struct repository *r,
 			       const char *name, int namelen,
 			       struct strbuf *buf,
-			       unsigned allowed)
+			       const struct interpret_branch_name_options *options)
 {
 	char *at;
 	const char *start;
@@ -1515,7 +1518,7 @@ int repo_interpret_branch_name(struct repository *r,
 	if (!namelen)
 		namelen = strlen(name);
 
-	if (!allowed || (allowed & INTERPRET_BRANCH_LOCAL)) {
+	if (!options->allowed || (options->allowed & INTERPRET_BRANCH_LOCAL)) {
 		len = interpret_nth_prior_checkout(r, name, namelen, buf);
 		if (!len) {
 			return len; /* syntax Ok, not enough switches */
@@ -1523,7 +1526,8 @@ int repo_interpret_branch_name(struct repository *r,
 			if (len == namelen)
 				return len; /* consumed all */
 			else
-				return reinterpret(r, name, namelen, len, buf, allowed);
+				return reinterpret(r, name, namelen, len, buf,
+						   options->allowed);
 		}
 	}
 
@@ -1531,22 +1535,22 @@ int repo_interpret_branch_name(struct repository *r,
 	     (at = memchr(start, '@', namelen - (start - name)));
 	     start = at + 1) {
 
-		if (!allowed || (allowed & INTERPRET_BRANCH_HEAD)) {
+		if (!options->allowed || (options->allowed & INTERPRET_BRANCH_HEAD)) {
 			len = interpret_empty_at(name, namelen, at - name, buf);
 			if (len > 0)
 				return reinterpret(r, name, namelen, len, buf,
-						   allowed);
+						   options->allowed);
 		}
 
 		len = interpret_branch_mark(r, name, namelen, at - name, buf,
 					    upstream_mark, branch_get_upstream,
-					    allowed);
+					    options);
 		if (len > 0)
 			return len;
 
 		len = interpret_branch_mark(r, name, namelen, at - name, buf,
 					    push_mark, branch_get_push,
-					    allowed);
+					    options);
 		if (len > 0)
 			return len;
 	}
@@ -1557,7 +1561,10 @@ int repo_interpret_branch_name(struct repository *r,
 void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 {
 	int len = strlen(name);
-	int used = interpret_branch_name(name, len, sb, allowed);
+	struct interpret_branch_name_options options = {
+		.allowed = allowed
+	};
+	int used = interpret_branch_name(name, len, sb, &options);
 
 	if (used < 0)
 		used = 0;
-- 
2.28.0.402.g5ffc5be6b7-goog

Re: [PATCH v2 1/2] sha1-name: replace unsigned int with option struct

[Junio C Hamano]

Jonathan Tan <jonathantanmy@google.com> writes:

> In preparation for a future patch adding a boolean parameter to
> repo_interpret_branch_name(), which might be easily confused with an
> existing unsigned int parameter, refactor repo_interpret_branch_name()
> to take an option struct instead of the unsigned int parameter.

Makes sense.

>  #define INTERPRET_BRANCH_LOCAL (1<<0)
>  #define INTERPRET_BRANCH_REMOTE (1<<1)
>  #define INTERPRET_BRANCH_HEAD (1<<2)
> +struct interpret_branch_name_options {
> +	/*
> +	 * If "allowed" is non-zero, it is a treated as a bitfield of allowable
> +	 * expansions: local branches ("refs/heads/"), remote branches
> +	 * ("refs/remotes/"), or "HEAD". If no "allowed" bits are set, any expansion is
> +	 * allowed, even ones to refs outside of those namespaces.
> +	 */
> +	unsigned allowed;
> +};
>  int repo_interpret_branch_name(struct repository *r,
>  			       const char *str, int len,
>  			       struct strbuf *buf,
> -			       unsigned allowed);
> -#define interpret_branch_name(str, len, buf, allowed) \
> -	repo_interpret_branch_name(the_repository, str, len, buf, allowed)
> +			       const struct interpret_branch_name_options *options);
> +#define interpret_branch_name(str, len, buf, options) \
> +	repo_interpret_branch_name(the_repository, str, len, buf, options)

I was debating myself if we want to have 

    #define IBN_OPTIONS_INIT { 0 }

or something similar (perhaps "#define IOI(abit) { .allowed = (abit) }"),
but it probably is not worth it given that we have only 3 local
sites that define it, 1 always initializes the field to 0, and the
other just relay the value passed by its caller.

> ...
> diff --git a/sha1-name.c b/sha1-name.c
> index 0b8cb5247a..a7a9de66c4 100644
> --- a/sha1-name.c
> +++ b/sha1-name.c
> @@ -1427,9 +1427,12 @@ static int reinterpret(struct repository *r,
>  	struct strbuf tmp = STRBUF_INIT;
>  	int used = buf->len;
>  	int ret;
> +	struct interpret_branch_name_options options = {
> +		.allowed = allowed
> +	};
>  
>  	strbuf_add(buf, name + len, namelen - len);
> -	ret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, allowed);
> +	ret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, &options);

> @@ -1557,7 +1561,10 @@ int repo_interpret_branch_name(struct repository *r,
>  void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
>  {
>  	int len = strlen(name);
> -	int used = interpret_branch_name(name, len, sb, allowed);
> +	struct interpret_branch_name_options options = {
> +		.allowed = allowed
> +	};
> +	int used = interpret_branch_name(name, len, sb, &options);

These are quite straight-forward rewrites.  Looking good.

Thanks.

[PATCH v2 2/2] wt-status: tolerate dangling marks

[Jonathan Tan]

When a user checks out the upstream branch of HEAD, the upstream branch
not being a local branch, and then runs "git status", like this:

  git clone $URL client
  cd client
  git checkout @{u}
  git status

no status is printed, but instead an error message:

  fatal: HEAD does not point to a branch

(This error message when running "git branch" persists even after
checking out other things - it only stops after checking out a branch.)

This is because "git status" reads the reflog when determining the "HEAD
detached" message, and thus attempts to DWIM "@{u}", but that doesn't
work because HEAD no longer points to a branch.

Therefore, when calculating the status of a worktree, tolerate dangling
marks. This is done by adding an additional parameter to
repo_dwim_ref().

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 cache.h           |  7 +++++++
 refs.c            | 16 ++++++++++------
 refs.h            |  3 ++-
 sha1-name.c       | 16 +++++++++++-----
 t/t7508-status.sh | 12 ++++++++++++
 wt-status.c       |  2 +-
 6 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/cache.h b/cache.h
index 4f16a57ba4..cee8aa5dc3 100644
--- a/cache.h
+++ b/cache.h
@@ -1569,6 +1569,13 @@ struct interpret_branch_name_options {
 	 * allowed, even ones to refs outside of those namespaces.
 	 */
 	unsigned allowed;
+
+	/*
+	 * If ^{upstream} or ^{push} (or equivalent) is requested, and the
+	 * branch in question does not have such a reference, return -1 instead
+	 * of die()-ing.
+	 */
+	unsigned nonfatal_dangling_mark : 1;
 };
 int repo_interpret_branch_name(struct repository *r,
 			       const char *str, int len,
diff --git a/refs.c b/refs.c
index cf09cd039f..b6f1a2f452 100644
--- a/refs.c
+++ b/refs.c
@@ -598,10 +598,13 @@ const char *git_default_branch_name(void)
  * to name a branch.
  */
 static char *substitute_branch_name(struct repository *r,
-				    const char **string, int *len)
+				    const char **string, int *len,
+				    int nonfatal_dangling_mark)
 {
 	struct strbuf buf = STRBUF_INIT;
-	struct interpret_branch_name_options options = { 0 } ;
+	struct interpret_branch_name_options options = {
+		.nonfatal_dangling_mark = nonfatal_dangling_mark
+	};
 	int ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);
 
 	if (ret == *len) {
@@ -615,9 +618,10 @@ static char *substitute_branch_name(struct repository *r,
 }
 
 int repo_dwim_ref(struct repository *r, const char *str, int len,
-		  struct object_id *oid, char **ref)
+		  struct object_id *oid, char **ref, int nonfatal_dangling_mark)
 {
-	char *last_branch = substitute_branch_name(r, &str, &len);
+	char *last_branch = substitute_branch_name(r, &str, &len,
+						   nonfatal_dangling_mark);
 	int   refs_found  = expand_ref(r, str, len, oid, ref);
 	free(last_branch);
 	return refs_found;
@@ -625,7 +629,7 @@ int repo_dwim_ref(struct repository *r, const char *str, int len,
 
 int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)
 {
-	return repo_dwim_ref(the_repository, str, len, oid, ref);
+	return repo_dwim_ref(the_repository, str, len, oid, ref, 0);
 }
 
 int expand_ref(struct repository *repo, const char *str, int len,
@@ -666,7 +670,7 @@ int repo_dwim_log(struct repository *r, const char *str, int len,
 		  struct object_id *oid, char **log)
 {
 	struct ref_store *refs = get_main_ref_store(r);
-	char *last_branch = substitute_branch_name(r, &str, &len);
+	char *last_branch = substitute_branch_name(r, &str, &len, 0);
 	const char **p;
 	int logs_found = 0;
 	struct strbuf path = STRBUF_INIT;
diff --git a/refs.h b/refs.h
index 29e28124cd..b94a7fd4f7 100644
--- a/refs.h
+++ b/refs.h
@@ -149,7 +149,8 @@ struct strvec;
 void expand_ref_prefix(struct strvec *prefixes, const char *prefix);
 
 int expand_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
-int repo_dwim_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
+int repo_dwim_ref(struct repository *r, const char *str, int len,
+		  struct object_id *oid, char **ref, int nonfatal_dangling_mark);
 int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
 int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);
 int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
diff --git a/sha1-name.c b/sha1-name.c
index a7a9de66c4..0b23b86ceb 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -809,7 +809,7 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 
 	if (len == r->hash_algo->hexsz && !get_oid_hex(str, oid)) {
 		if (warn_ambiguous_refs && warn_on_object_refname_ambiguity) {
-			refs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref);
+			refs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref, 0);
 			if (refs_found > 0) {
 				warning(warn_msg, len, str);
 				if (advice_object_name_warning)
@@ -860,11 +860,11 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 
 	if (!len && reflog_len)
 		/* allow "@{...}" to mean the current branch reflog */
-		refs_found = repo_dwim_ref(r, "HEAD", 4, oid, &real_ref);
+		refs_found = repo_dwim_ref(r, "HEAD", 4, oid, &real_ref, 0);
 	else if (reflog_len)
 		refs_found = repo_dwim_log(r, str, len, oid, &real_ref);
 	else
-		refs_found = repo_dwim_ref(r, str, len, oid, &real_ref);
+		refs_found = repo_dwim_ref(r, str, len, oid, &real_ref, 0);
 
 	if (!refs_found)
 		return -1;
@@ -1496,8 +1496,14 @@ static int interpret_branch_mark(struct repository *r,
 		branch = branch_get(NULL);
 
 	value = get_data(branch, &err);
-	if (!value)
-		die("%s", err.buf);
+	if (!value) {
+		if (options->nonfatal_dangling_mark) {
+			strbuf_release(&err);
+			return -1;
+		} else {
+			die("%s", err.buf);
+		}
+	}
 
 	if (!branch_interpret_allowed(value, options->allowed))
 		return -1;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index e81759319f..45e1f6ff68 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -846,6 +846,18 @@ test_expect_success 'status refreshes the index' '
 	test_cmp expect output
 '
 
+test_expect_success 'status shows detached HEAD properly after checking out non-local upstream branch' '
+	test_when_finished rm -rf upstream downstream actual &&
+
+	test_create_repo upstream &&
+	test_commit -C upstream foo &&
+
+	git clone upstream downstream &&
+	git -C downstream checkout @{u} &&
+	git -C downstream status >actual &&
+	test_i18ngrep "HEAD detached at [0-9a-f]\\+" actual
+'
+
 test_expect_success 'setup status submodule summary' '
 	test_create_repo sm && (
 		cd sm &&
diff --git a/wt-status.c b/wt-status.c
index 7ce58b8aae..ae16faf40d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1569,7 +1569,7 @@ static void wt_status_get_detached_from(struct repository *r,
 		return;
 	}
 
-	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
+	if (repo_dwim_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&
 	    /* sha1 is a commit? match without further lookup */
 	    (oideq(&cb.noid, &oid) ||
 	     /* perhaps sha1 is a tag, try to dereference to a commit */
-- 
2.28.0.402.g5ffc5be6b7-goog

Re: [PATCH v2 2/2] wt-status: tolerate dangling marks

[Junio C Hamano]

Jonathan Tan <jonathantanmy@google.com> writes:

> +
> +	/*
> +	 * If ^{upstream} or ^{push} (or equivalent) is requested, and the
> +	 * branch in question does not have such a reference, return -1 instead
> +	 * of die()-ing.
> +	 */
> +	unsigned nonfatal_dangling_mark : 1;

Micronit; I would have avoided "or equivalent" as the point of
parenthetical comment was not to say these two modifiers upstream
and push (and other forms that spell differently but invokes exactly
one of these two features) are special, but to say that these two
are merely examples, and any other ^{modifiers} we have or we will
add in the future would honor this bit.  Perhaps "(and the like)"?

>  };
>  int repo_interpret_branch_name(struct repository *r,
>  			       const char *str, int len,
> diff --git a/refs.c b/refs.c
> index cf09cd039f..b6f1a2f452 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -598,10 +598,13 @@ const char *git_default_branch_name(void)
>   * to name a branch.
>   */
>  static char *substitute_branch_name(struct repository *r,
> -				    const char **string, int *len)
> +				    const char **string, int *len,
> +				    int nonfatal_dangling_mark)
>  {
>  	struct strbuf buf = STRBUF_INIT;
> -	struct interpret_branch_name_options options = { 0 } ;
> +	struct interpret_branch_name_options options = {
> +		.nonfatal_dangling_mark = nonfatal_dangling_mark
> +	};
>  	int ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);
>  
>  	if (ret == *len) {
> @@ -615,9 +618,10 @@ static char *substitute_branch_name(struct repository *r,
>  }
>  
>  int repo_dwim_ref(struct repository *r, const char *str, int len,
> -		  struct object_id *oid, char **ref)
> +		  struct object_id *oid, char **ref, int nonfatal_dangling_mark)
>  {
> -	char *last_branch = substitute_branch_name(r, &str, &len);
> +	char *last_branch = substitute_branch_name(r, &str, &len,
> +						   nonfatal_dangling_mark);
>  	int   refs_found  = expand_ref(r, str, len, oid, ref);
>  	free(last_branch);
>  	return refs_found;
> @@ -625,7 +629,7 @@ int repo_dwim_ref(struct repository *r, const char *str, int len,
>  
>  int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)
>  {
> -	return repo_dwim_ref(the_repository, str, len, oid, ref);
> +	return repo_dwim_ref(the_repository, str, len, oid, ref, 0);
>  }
>  
>  int expand_ref(struct repository *repo, const char *str, int len,
> @@ -666,7 +670,7 @@ int repo_dwim_log(struct repository *r, const char *str, int len,
>  		  struct object_id *oid, char **log)
>  {
>  	struct ref_store *refs = get_main_ref_store(r);
> -	char *last_branch = substitute_branch_name(r, &str, &len);
> +	char *last_branch = substitute_branch_name(r, &str, &len, 0);
>  	const char **p;
>  	int logs_found = 0;
>  	struct strbuf path = STRBUF_INIT;

Among these callers that reach substitute_branch_name(), how were
those that can specify the new bit chosen?

For example, what is the reasoning behind making dwim_ref() unable
to ask the "do so gently" variant, while allowing repo_dwim_ref()
to?

I am NOT necessarily saying these two functions MUST be able to
access the same set of features and the only difference between them
MUST be kept to the current "repo_* variant can work on an arbitrary
repository, while the variant without repo_* would work on the
primary repository only".  As long as there is a good reason to make
their power diverge, it is OK---I just do not see why and I'd like
to know.

The same question about not allowing the gentler variant while
drimming the reflog.

Thanks.

Re: [PATCH v2 2/2] wt-status: tolerate dangling marks

[Jonathan Tan]

> Jonathan Tan <jonathantanmy@google.com> writes:
> 
> > +
> > +	/*
> > +	 * If ^{upstream} or ^{push} (or equivalent) is requested, and the
> > +	 * branch in question does not have such a reference, return -1 instead
> > +	 * of die()-ing.
> > +	 */
> > +	unsigned nonfatal_dangling_mark : 1;
> 
> Micronit; I would have avoided "or equivalent" as the point of
> parenthetical comment was not to say these two modifiers upstream
> and push (and other forms that spell differently but invokes exactly
> one of these two features) are special, but to say that these two
> are merely examples, and any other ^{modifiers} we have or we will
> add in the future would honor this bit.  Perhaps "(and the like)"?

"(and the like)" sounds good.

> Among these callers that reach substitute_branch_name(), how were
> those that can specify the new bit chosen?

I just did the minimal change to fix the bug in the test.

> For example, what is the reasoning behind making dwim_ref() unable
> to ask the "do so gently" variant, while allowing repo_dwim_ref()
> to?
> 
> I am NOT necessarily saying these two functions MUST be able to
> access the same set of features and the only difference between them
> MUST be kept to the current "repo_* variant can work on an arbitrary
> repository, while the variant without repo_* would work on the
> primary repository only".  As long as there is a good reason to make
> their power diverge, it is OK---I just do not see why and I'd like
> to know.

Maybe add the following to the end of the last paragraph of the commit
message:

  (dwim_ref() is unchanged because I expect more and more code to use
  repo_dwim_ref(), and to reduce the size of the diff.)

If you prefer not to make this change locally, let me know and I'll
resend one with the updated commit message and the "(and the like)"
change above.

> The same question about not allowing the gentler variant while
> drimming the reflog.

Same as above - I only did the minimal change to fix the bug.
Admittedly, if a reflog-accessing command could fail in the same way
(dangling mark), we would need to fix repo_dwim_log() and then we could
simplify substitute_branch_name() to not take the nonfatal_dangling_mark
parameter (since all dangling marks would now be nonfatal), but I
haven't looked beyond this.

Re: [PATCH v2 2/2] wt-status: tolerate dangling marks

[Junio C Hamano]

Jonathan Tan <jonathantanmy@google.com> writes:

>> For example, what is the reasoning behind making dwim_ref() unable
>> to ask the "do so gently" variant, while allowing repo_dwim_ref()
>> to?
>> 
>> I am NOT necessarily saying these two functions MUST be able to
>> access the same set of features and the only difference between them
>> MUST be kept to the current "repo_* variant can work on an arbitrary
>> repository, while the variant without repo_* would work on the
>> primary repository only".  As long as there is a good reason to make
>> their power diverge, it is OK---I just do not see why and I'd like
>> to know.
>
> Maybe add the following to the end of the last paragraph of the commit
> message:
>
>   (dwim_ref() is unchanged because I expect more and more code to use
>   repo_dwim_ref(), and to reduce the size of the diff.)

If that is the reasoning, I totally disagree.  We may be staring
problems in submodules too long to think everybody should use the
variant with repo_ prefix, but a single repository operations will
continue to exist, and when you know you only need to access the
current repository, not having to use the function with longer name
without having to pass an extra parameter is a plus.

I even wonder why dwim_ref() is not defined like so in a header:

	#define dwim_ref(s, l, o, r) \
		repo_dwim_ref(the_repository, (s), (l), (o), (r))

Which would force a change like the one we are discussing to keep
them in parallel and keep the promise that only difference between
the two is that dwim_ref() works with the_repository and the other
can take an arbitrary repository.  Perhaps that can be a preliminary
clean-up patch before these two patches ;-)

Doing so would mean that coders have to remember one fewer thing
than "dwim_ref(), not only cannot take a repository pointer, cannot
be told to report error gently".  The worst part is that we know
that the difference will GROW, because the purpose of adding the
extra option argument to the API is exactly to make it easy to do
so.

Reducing the size of the diff is a good justification only when the
end result is the same.  If it burdens future developers more, that
is "I write less at the expense of forcing others to write more",
which is not quite the same thing.

Thanks.

[PATCH v3 0/3] Fix for git checkout @{u} (non-local) then git status

[Jonathan Tan]

Junio writes [1]:

> I even wonder why dwim_ref() is not defined like so in a header:
> 
> 	#define dwim_ref(s, l, o, r) \
> 		repo_dwim_ref(the_repository, (s), (l), (o), (r))
> 
> Which would force a change like the one we are discussing to keep
> them in parallel and keep the promise that only difference between
> the two is that dwim_ref() works with the_repository and the other
> can take an arbitrary repository.  Perhaps that can be a preliminary
> clean-up patch before these two patches ;-)

OK done - that's patch 2 here. (I used "static inline" instead of a
preprocessor #define because I prefer not to use the preprocessor whenever
possible, but switching to #define is fine too.)

> Reducing the size of the diff is a good justification only when the
> end result is the same.  If it burdens future developers more, that
> is "I write less at the expense of forcing others to write more",
> which is not quite the same thing.

OK - that makes sense.

[1] https://lore.kernel.org/git/xmqqzh6ag1ih.fsf@gitster.c.googlers.com/

Jonathan Tan (3):
  sha1-name: replace unsigned int with option struct
  refs: move dwim_ref() to header file
  wt-status: tolerate dangling marks

 archive.c             |  4 ++--
 branch.c              |  2 +-
 builtin/checkout.c    |  4 ++--
 builtin/fast-export.c |  2 +-
 builtin/log.c         |  2 +-
 builtin/merge.c       |  2 +-
 builtin/reset.c       |  2 +-
 builtin/rev-parse.c   |  2 +-
 builtin/show-branch.c |  2 +-
 builtin/stash.c       |  2 +-
 bundle.c              |  2 +-
 cache.h               | 27 ++++++++++++++++++--------
 commit.c              |  2 +-
 refs.c                | 20 +++++++++----------
 refs.h                | 12 ++++++++++--
 remote.c              |  2 +-
 revision.c            |  3 ++-
 sha1-name.c           | 45 ++++++++++++++++++++++++++++---------------
 t/t7508-status.sh     | 12 ++++++++++++
 wt-status.c           |  2 +-
 20 files changed, 98 insertions(+), 53 deletions(-)

Range-diff against v2:
-:  ---------- > 1:  6b3e3077e6 refs: move dwim_ref() to header file
1:  59b91a206d ! 2:  8f489d9633 wt-status: tolerate dangling marks
    @@ Commit message
     
         Therefore, when calculating the status of a worktree, tolerate dangling
         marks. This is done by adding an additional parameter to
    -    repo_dwim_ref().
    +    dwim_ref() and repo_dwim_ref().
     
         Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
     
    + ## archive.c ##
    +@@ archive.c: static void parse_treeish_arg(const char **argv,
    + 		const char *colon = strchrnul(name, ':');
    + 		int refnamelen = colon - name;
    + 
    +-		if (!dwim_ref(name, refnamelen, &oid, &ref))
    ++		if (!dwim_ref(name, refnamelen, &oid, &ref, 0))
    + 			die(_("no such ref: %.*s"), refnamelen, name);
    + 	} else {
    +-		dwim_ref(name, strlen(name), &oid, &ref);
    ++		dwim_ref(name, strlen(name), &oid, &ref, 0);
    + 	}
    + 
    + 	if (get_oid(name, &oid))
    +
    + ## branch.c ##
    +@@ branch.c: void create_branch(struct repository *r,
    + 		die(_("Not a valid object name: '%s'."), start_name);
    + 	}
    + 
    +-	switch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref)) {
    ++	switch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref, 0)) {
    + 	case 0:
    + 		/* Not branching from any existing branch */
    + 		if (explicit_tracking)
    +
    + ## builtin/checkout.c ##
    +@@ builtin/checkout.c: static void setup_branch_path(struct branch_info *branch)
    + 	 * If this is a ref, resolve it; otherwise, look up the OID for our
    + 	 * expression.  Failure here is okay.
    + 	 */
    +-	if (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname))
    ++	if (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname, 0))
    + 		repo_get_oid_committish(the_repository, branch->name, &branch->oid);
    + 
    + 	strbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
    +@@ builtin/checkout.c: static void die_expecting_a_branch(const struct branch_info *branch_info)
    + 	struct object_id oid;
    + 	char *to_free;
    + 
    +-	if (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free) == 1) {
    ++	if (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free, 0) == 1) {
    + 		const char *ref = to_free;
    + 
    + 		if (skip_prefix(ref, "refs/tags/", &ref))
    +
    + ## builtin/fast-export.c ##
    +@@ builtin/fast-export.c: static void get_tags_and_duplicates(struct rev_cmdline_info *info)
    + 		if (e->flags & UNINTERESTING)
    + 			continue;
    + 
    +-		if (dwim_ref(e->name, strlen(e->name), &oid, &full_name) != 1)
    ++		if (dwim_ref(e->name, strlen(e->name), &oid, &full_name, 0) != 1)
    + 			continue;
    + 
    + 		if (refspecs.nr) {
    +
    + ## builtin/log.c ##
    +@@ builtin/log.c: static char *find_branch_name(struct rev_info *rev)
    + 		return NULL;
    + 	ref = rev->cmdline.rev[positive].name;
    + 	tip_oid = &rev->cmdline.rev[positive].item->oid;
    +-	if (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref) &&
    ++	if (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref, 0) &&
    + 	    skip_prefix(full_ref, "refs/heads/", &v) &&
    + 	    oideq(tip_oid, &branch_oid))
    + 		branch = xstrdup(v);
    +
    + ## builtin/merge.c ##
    +@@ builtin/merge.c: static void merge_name(const char *remote, struct strbuf *msg)
    + 	if (!remote_head)
    + 		die(_("'%s' does not point to a commit"), remote);
    + 
    +-	if (dwim_ref(remote, strlen(remote), &branch_head, &found_ref) > 0) {
    ++	if (dwim_ref(remote, strlen(remote), &branch_head, &found_ref, 0) > 0) {
    + 		if (starts_with(found_ref, "refs/heads/")) {
    + 			strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
    + 				    oid_to_hex(&branch_head), remote);
    +
    + ## builtin/reset.c ##
    +@@ builtin/reset.c: int cmd_reset(int argc, const char **argv, const char *prefix)
    + 			char *ref = NULL;
    + 			int err;
    + 
    +-			dwim_ref(rev, strlen(rev), &dummy, &ref);
    ++			dwim_ref(rev, strlen(rev), &dummy, &ref, 0);
    + 			if (ref && !starts_with(ref, "refs/"))
    + 				ref = NULL;
    + 
    +
    + ## builtin/rev-parse.c ##
    +@@ builtin/rev-parse.c: static void show_rev(int type, const struct object_id *oid, const char *name)
    + 			struct object_id discard;
    + 			char *full;
    + 
    +-			switch (dwim_ref(name, strlen(name), &discard, &full)) {
    ++			switch (dwim_ref(name, strlen(name), &discard, &full, 0)) {
    + 			case 0:
    + 				/*
    + 				 * Not found -- not a ref.  We could
    +
    + ## builtin/show-branch.c ##
    +@@ builtin/show-branch.c: int cmd_show_branch(int ac, const char **av, const char *prefix)
    + 			die(Q_("only %d entry can be shown at one time.",
    + 			       "only %d entries can be shown at one time.",
    + 			       MAX_REVS), MAX_REVS);
    +-		if (!dwim_ref(*av, strlen(*av), &oid, &ref))
    ++		if (!dwim_ref(*av, strlen(*av), &oid, &ref, 0))
    + 			die(_("no such ref %s"), *av);
    + 
    + 		/* Has the base been specified? */
    +
    + ## builtin/stash.c ##
    +@@ builtin/stash.c: static int get_stash_info(struct stash_info *info, int argc, const char **argv)
    + 	end_of_rev = strchrnul(revision, '@');
    + 	strbuf_add(&symbolic, revision, end_of_rev - revision);
    + 
    +-	ret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref);
    ++	ret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref, 0);
    + 	strbuf_release(&symbolic);
    + 	switch (ret) {
    + 	case 0: /* Not found, but valid ref */
    +
    + ## bundle.c ##
    +@@ bundle.c: static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
    + 
    + 		if (e->item->flags & UNINTERESTING)
    + 			continue;
    +-		if (dwim_ref(e->name, strlen(e->name), &oid, &ref) != 1)
    ++		if (dwim_ref(e->name, strlen(e->name), &oid, &ref, 0) != 1)
    + 			goto skip_write_ref;
    + 		if (read_ref_full(e->name, RESOLVE_REF_READING, &oid, &flag))
    + 			flag = 0;
    +
      ## cache.h ##
     @@ cache.h: struct interpret_branch_name_options {
      	 * allowed, even ones to refs outside of those namespaces.
    @@ cache.h: struct interpret_branch_name_options {
      int repo_interpret_branch_name(struct repository *r,
      			       const char *str, int len,
     
    + ## commit.c ##
    +@@ commit.c: struct commit *get_fork_point(const char *refname, struct commit *commit)
    + 	struct commit *ret = NULL;
    + 	char *full_refname;
    + 
    +-	switch (dwim_ref(refname, strlen(refname), &oid, &full_refname)) {
    ++	switch (dwim_ref(refname, strlen(refname), &oid, &full_refname, 0)) {
    + 	case 0:
    + 		die("No such ref: '%s'", refname);
    + 	case 1:
    +
      ## refs.c ##
     @@ refs.c: const char *git_default_branch_name(void)
       * to name a branch.
    @@ refs.c: static char *substitute_branch_name(struct repository *r,
      	int   refs_found  = expand_ref(r, str, len, oid, ref);
      	free(last_branch);
      	return refs_found;
    -@@ refs.c: int repo_dwim_ref(struct repository *r, const char *str, int len,
    - 
    - int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)
    - {
    --	return repo_dwim_ref(the_repository, str, len, oid, ref);
    -+	return repo_dwim_ref(the_repository, str, len, oid, ref, 0);
    - }
    - 
    - int expand_ref(struct repository *repo, const char *str, int len,
     @@ refs.c: int repo_dwim_log(struct repository *r, const char *str, int len,
      		  struct object_id *oid, char **log)
      {
    @@ refs.h: struct strvec;
     +int repo_dwim_ref(struct repository *r, const char *str, int len,
     +		  struct object_id *oid, char **ref, int nonfatal_dangling_mark);
      int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
    - int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);
    + static inline int dwim_ref(const char *str, int len, struct object_id *oid,
    +-			   char **ref)
    ++			   char **ref, int nonfatal_dangling_mark)
    + {
    +-	return repo_dwim_ref(the_repository, str, len, oid, ref);
    ++	return repo_dwim_ref(the_repository, str, len, oid, ref,
    ++			     nonfatal_dangling_mark);
    + }
      int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
    + 
    +
    + ## remote.c ##
    +@@ remote.c: static void set_merge(struct branch *ret)
    + 		    strcmp(ret->remote_name, "."))
    + 			continue;
    + 		if (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]),
    +-			     &oid, &ref) == 1)
    ++			     &oid, &ref, 0) == 1)
    + 			ret->merge[i]->dst = ref;
    + 		else
    + 			ret->merge[i]->dst = xstrdup(ret->merge_name[i]);
     
      ## sha1-name.c ##
     @@ sha1-name.c: static int get_oid_basic(struct repository *r, const char *str, int len,
    @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
      	}
      
     -	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
    -+	if (repo_dwim_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&
    ++	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&
      	    /* sha1 is a commit? match without further lookup */
      	    (oideq(&cb.noid, &oid) ||
      	     /* perhaps sha1 is a tag, try to dereference to a commit */
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v3 1/3] sha1-name: replace unsigned int with option struct

[Jonathan Tan]

In preparation for a future patch adding a boolean parameter to
repo_interpret_branch_name(), which might be easily confused with an
existing unsigned int parameter, refactor repo_interpret_branch_name()
to take an option struct instead of the unsigned int parameter.

The static function interpret_branch_mark() is also updated to take the
option struct in preparation for that future patch, since it will also
make use of the to-be-introduced boolean parameter.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 cache.h     | 20 ++++++++++++--------
 refs.c      |  3 ++-
 revision.c  |  3 ++-
 sha1-name.c | 29 ++++++++++++++++++-----------
 4 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/cache.h b/cache.h
index 4cad61ffa4..4f16a57ba4 100644
--- a/cache.h
+++ b/cache.h
@@ -1557,21 +1557,25 @@ int parse_oid_hex_any(const char *hex, struct object_id *oid, const char **end);
  *
  * If the input was ok but there are not N branch switches in the
  * reflog, it returns 0.
- *
- * If "allowed" is non-zero, it is a treated as a bitfield of allowable
- * expansions: local branches ("refs/heads/"), remote branches
- * ("refs/remotes/"), or "HEAD". If no "allowed" bits are set, any expansion is
- * allowed, even ones to refs outside of those namespaces.
  */
 #define INTERPRET_BRANCH_LOCAL (1<<0)
 #define INTERPRET_BRANCH_REMOTE (1<<1)
 #define INTERPRET_BRANCH_HEAD (1<<2)
+struct interpret_branch_name_options {
+	/*
+	 * If "allowed" is non-zero, it is a treated as a bitfield of allowable
+	 * expansions: local branches ("refs/heads/"), remote branches
+	 * ("refs/remotes/"), or "HEAD". If no "allowed" bits are set, any expansion is
+	 * allowed, even ones to refs outside of those namespaces.
+	 */
+	unsigned allowed;
+};
 int repo_interpret_branch_name(struct repository *r,
 			       const char *str, int len,
 			       struct strbuf *buf,
-			       unsigned allowed);
-#define interpret_branch_name(str, len, buf, allowed) \
-	repo_interpret_branch_name(the_repository, str, len, buf, allowed)
+			       const struct interpret_branch_name_options *options);
+#define interpret_branch_name(str, len, buf, options) \
+	repo_interpret_branch_name(the_repository, str, len, buf, options)
 
 int validate_headref(const char *ref);
 
diff --git a/refs.c b/refs.c
index 3ee3afaf41..cf09cd039f 100644
--- a/refs.c
+++ b/refs.c
@@ -601,7 +601,8 @@ static char *substitute_branch_name(struct repository *r,
 				    const char **string, int *len)
 {
 	struct strbuf buf = STRBUF_INIT;
-	int ret = repo_interpret_branch_name(r, *string, *len, &buf, 0);
+	struct interpret_branch_name_options options = { 0 } ;
+	int ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);
 
 	if (ret == *len) {
 		size_t size;
diff --git a/revision.c b/revision.c
index 96630e3186..1247ee4ec8 100644
--- a/revision.c
+++ b/revision.c
@@ -315,13 +315,14 @@ static void add_pending_object_with_path(struct rev_info *revs,
 					 const char *name, unsigned mode,
 					 const char *path)
 {
+	struct interpret_branch_name_options options = { 0 };
 	if (!obj)
 		return;
 	if (revs->no_walk && (obj->flags & UNINTERESTING))
 		revs->no_walk = 0;
 	if (revs->reflog_info && obj->type == OBJ_COMMIT) {
 		struct strbuf buf = STRBUF_INIT;
-		int len = interpret_branch_name(name, 0, &buf, 0);
+		int len = interpret_branch_name(name, 0, &buf, &options);
 
 		if (0 < len && name[len] && buf.len)
 			strbuf_addstr(&buf, name + len);
diff --git a/sha1-name.c b/sha1-name.c
index 0b8cb5247a..a7a9de66c4 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -1427,9 +1427,12 @@ static int reinterpret(struct repository *r,
 	struct strbuf tmp = STRBUF_INIT;
 	int used = buf->len;
 	int ret;
+	struct interpret_branch_name_options options = {
+		.allowed = allowed
+	};
 
 	strbuf_add(buf, name + len, namelen - len);
-	ret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, allowed);
+	ret = repo_interpret_branch_name(r, buf->buf, buf->len, &tmp, &options);
 	/* that data was not interpreted, remove our cruft */
 	if (ret < 0) {
 		strbuf_setlen(buf, used);
@@ -1471,7 +1474,7 @@ static int interpret_branch_mark(struct repository *r,
 				 int (*get_mark)(const char *, int),
 				 const char *(*get_data)(struct branch *,
 							 struct strbuf *),
-				 unsigned allowed)
+				 const struct interpret_branch_name_options *options)
 {
 	int len;
 	struct branch *branch;
@@ -1496,7 +1499,7 @@ static int interpret_branch_mark(struct repository *r,
 	if (!value)
 		die("%s", err.buf);
 
-	if (!branch_interpret_allowed(value, allowed))
+	if (!branch_interpret_allowed(value, options->allowed))
 		return -1;
 
 	set_shortened_ref(r, buf, value);
@@ -1506,7 +1509,7 @@ static int interpret_branch_mark(struct repository *r,
 int repo_interpret_branch_name(struct repository *r,
 			       const char *name, int namelen,
 			       struct strbuf *buf,
-			       unsigned allowed)
+			       const struct interpret_branch_name_options *options)
 {
 	char *at;
 	const char *start;
@@ -1515,7 +1518,7 @@ int repo_interpret_branch_name(struct repository *r,
 	if (!namelen)
 		namelen = strlen(name);
 
-	if (!allowed || (allowed & INTERPRET_BRANCH_LOCAL)) {
+	if (!options->allowed || (options->allowed & INTERPRET_BRANCH_LOCAL)) {
 		len = interpret_nth_prior_checkout(r, name, namelen, buf);
 		if (!len) {
 			return len; /* syntax Ok, not enough switches */
@@ -1523,7 +1526,8 @@ int repo_interpret_branch_name(struct repository *r,
 			if (len == namelen)
 				return len; /* consumed all */
 			else
-				return reinterpret(r, name, namelen, len, buf, allowed);
+				return reinterpret(r, name, namelen, len, buf,
+						   options->allowed);
 		}
 	}
 
@@ -1531,22 +1535,22 @@ int repo_interpret_branch_name(struct repository *r,
 	     (at = memchr(start, '@', namelen - (start - name)));
 	     start = at + 1) {
 
-		if (!allowed || (allowed & INTERPRET_BRANCH_HEAD)) {
+		if (!options->allowed || (options->allowed & INTERPRET_BRANCH_HEAD)) {
 			len = interpret_empty_at(name, namelen, at - name, buf);
 			if (len > 0)
 				return reinterpret(r, name, namelen, len, buf,
-						   allowed);
+						   options->allowed);
 		}
 
 		len = interpret_branch_mark(r, name, namelen, at - name, buf,
 					    upstream_mark, branch_get_upstream,
-					    allowed);
+					    options);
 		if (len > 0)
 			return len;
 
 		len = interpret_branch_mark(r, name, namelen, at - name, buf,
 					    push_mark, branch_get_push,
-					    allowed);
+					    options);
 		if (len > 0)
 			return len;
 	}
@@ -1557,7 +1561,10 @@ int repo_interpret_branch_name(struct repository *r,
 void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 {
 	int len = strlen(name);
-	int used = interpret_branch_name(name, len, sb, allowed);
+	struct interpret_branch_name_options options = {
+		.allowed = allowed
+	};
+	int used = interpret_branch_name(name, len, sb, &options);
 
 	if (used < 0)
 		used = 0;
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v3 2/3] refs: move dwim_ref() to header file

[Jonathan Tan]

This makes it clear that dwim_ref() is just repo_dwim_ref() without the
first parameter.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 refs.c | 5 -----
 refs.h | 8 +++++++-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index cf09cd039f..0fa6067340 100644
--- a/refs.c
+++ b/refs.c
@@ -623,11 +623,6 @@ int repo_dwim_ref(struct repository *r, const char *str, int len,
 	return refs_found;
 }
 
-int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)
-{
-	return repo_dwim_ref(the_repository, str, len, oid, ref);
-}
-
 int expand_ref(struct repository *repo, const char *str, int len,
 	       struct object_id *oid, char **ref)
 {
diff --git a/refs.h b/refs.h
index 29e28124cd..8cbef96a8d 100644
--- a/refs.h
+++ b/refs.h
@@ -1,6 +1,8 @@
 #ifndef REFS_H
 #define REFS_H
 
+#include "cache.h"
+
 struct object_id;
 struct ref_store;
 struct repository;
@@ -151,7 +153,11 @@ void expand_ref_prefix(struct strvec *prefixes, const char *prefix);
 int expand_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
 int repo_dwim_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
 int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
-int dwim_ref(const char *str, int len, struct object_id *oid, char **ref);
+static inline int dwim_ref(const char *str, int len, struct object_id *oid,
+			   char **ref)
+{
+	return repo_dwim_ref(the_repository, str, len, oid, ref);
+}
 int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
 
 /*
-- 
2.28.0.402.g5ffc5be6b7-goog

[PATCH v3 3/3] wt-status: tolerate dangling marks

[Jonathan Tan]

When a user checks out the upstream branch of HEAD, the upstream branch
not being a local branch, and then runs "git status", like this:

  git clone $URL client
  cd client
  git checkout @{u}
  git status

no status is printed, but instead an error message:

  fatal: HEAD does not point to a branch

(This error message when running "git branch" persists even after
checking out other things - it only stops after checking out a branch.)

This is because "git status" reads the reflog when determining the "HEAD
detached" message, and thus attempts to DWIM "@{u}", but that doesn't
work because HEAD no longer points to a branch.

Therefore, when calculating the status of a worktree, tolerate dangling
marks. This is done by adding an additional parameter to
dwim_ref() and repo_dwim_ref().

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 archive.c             |  4 ++--
 branch.c              |  2 +-
 builtin/checkout.c    |  4 ++--
 builtin/fast-export.c |  2 +-
 builtin/log.c         |  2 +-
 builtin/merge.c       |  2 +-
 builtin/reset.c       |  2 +-
 builtin/rev-parse.c   |  2 +-
 builtin/show-branch.c |  2 +-
 builtin/stash.c       |  2 +-
 bundle.c              |  2 +-
 cache.h               |  7 +++++++
 commit.c              |  2 +-
 refs.c                | 14 +++++++++-----
 refs.h                |  8 +++++---
 remote.c              |  2 +-
 sha1-name.c           | 16 +++++++++++-----
 t/t7508-status.sh     | 12 ++++++++++++
 wt-status.c           |  2 +-
 19 files changed, 60 insertions(+), 29 deletions(-)

diff --git a/archive.c b/archive.c
index fb39706120..0de6048bfc 100644
--- a/archive.c
+++ b/archive.c
@@ -397,10 +397,10 @@ static void parse_treeish_arg(const char **argv,
 		const char *colon = strchrnul(name, ':');
 		int refnamelen = colon - name;
 
-		if (!dwim_ref(name, refnamelen, &oid, &ref))
+		if (!dwim_ref(name, refnamelen, &oid, &ref, 0))
 			die(_("no such ref: %.*s"), refnamelen, name);
 	} else {
-		dwim_ref(name, strlen(name), &oid, &ref);
+		dwim_ref(name, strlen(name), &oid, &ref, 0);
 	}
 
 	if (get_oid(name, &oid))
diff --git a/branch.c b/branch.c
index 7095f78058..9c9dae1eae 100644
--- a/branch.c
+++ b/branch.c
@@ -281,7 +281,7 @@ void create_branch(struct repository *r,
 		die(_("Not a valid object name: '%s'."), start_name);
 	}
 
-	switch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref)) {
+	switch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref, 0)) {
 	case 0:
 		/* Not branching from any existing branch */
 		if (explicit_tracking)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bba64108af..1f10cc93dd 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -651,7 +651,7 @@ static void setup_branch_path(struct branch_info *branch)
 	 * If this is a ref, resolve it; otherwise, look up the OID for our
 	 * expression.  Failure here is okay.
 	 */
-	if (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname))
+	if (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname, 0))
 		repo_get_oid_committish(the_repository, branch->name, &branch->oid);
 
 	strbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
@@ -1345,7 +1345,7 @@ static void die_expecting_a_branch(const struct branch_info *branch_info)
 	struct object_id oid;
 	char *to_free;
 
-	if (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free) == 1) {
+	if (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free, 0) == 1) {
 		const char *ref = to_free;
 
 		if (skip_prefix(ref, "refs/tags/", &ref))
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9f37895d4c..1b8fca3ee0 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -943,7 +943,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
 		if (e->flags & UNINTERESTING)
 			continue;
 
-		if (dwim_ref(e->name, strlen(e->name), &oid, &full_name) != 1)
+		if (dwim_ref(e->name, strlen(e->name), &oid, &full_name, 0) != 1)
 			continue;
 
 		if (refspecs.nr) {
diff --git a/builtin/log.c b/builtin/log.c
index b58f8da09e..4ec7f57cf4 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1061,7 +1061,7 @@ static char *find_branch_name(struct rev_info *rev)
 		return NULL;
 	ref = rev->cmdline.rev[positive].name;
 	tip_oid = &rev->cmdline.rev[positive].item->oid;
-	if (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref) &&
+	if (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref, 0) &&
 	    skip_prefix(full_ref, "refs/heads/", &v) &&
 	    oideq(tip_oid, &branch_oid))
 		branch = xstrdup(v);
diff --git a/builtin/merge.c b/builtin/merge.c
index 74829a838e..2af70b5605 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -500,7 +500,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	if (!remote_head)
 		die(_("'%s' does not point to a commit"), remote);
 
-	if (dwim_ref(remote, strlen(remote), &branch_head, &found_ref) > 0) {
+	if (dwim_ref(remote, strlen(remote), &branch_head, &found_ref, 0) > 0) {
 		if (starts_with(found_ref, "refs/heads/")) {
 			strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
 				    oid_to_hex(&branch_head), remote);
diff --git a/builtin/reset.c b/builtin/reset.c
index 8ae69d6f2b..c635b062c3 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -423,7 +423,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 			char *ref = NULL;
 			int err;
 
-			dwim_ref(rev, strlen(rev), &dummy, &ref);
+			dwim_ref(rev, strlen(rev), &dummy, &ref, 0);
 			if (ref && !starts_with(ref, "refs/"))
 				ref = NULL;
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 669dd2fd6f..ed200c8af1 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -136,7 +136,7 @@ static void show_rev(int type, const struct object_id *oid, const char *name)
 			struct object_id discard;
 			char *full;
 
-			switch (dwim_ref(name, strlen(name), &discard, &full)) {
+			switch (dwim_ref(name, strlen(name), &discard, &full, 0)) {
 			case 0:
 				/*
 				 * Not found -- not a ref.  We could
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 7eae5f3801..d6d2dabeca 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -741,7 +741,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			die(Q_("only %d entry can be shown at one time.",
 			       "only %d entries can be shown at one time.",
 			       MAX_REVS), MAX_REVS);
-		if (!dwim_ref(*av, strlen(*av), &oid, &ref))
+		if (!dwim_ref(*av, strlen(*av), &oid, &ref, 0))
 			die(_("no such ref %s"), *av);
 
 		/* Has the base been specified? */
diff --git a/builtin/stash.c b/builtin/stash.c
index 4bdfaf8397..3f811f3050 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -185,7 +185,7 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
 	end_of_rev = strchrnul(revision, '@');
 	strbuf_add(&symbolic, revision, end_of_rev - revision);
 
-	ret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref);
+	ret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref, 0);
 	strbuf_release(&symbolic);
 	switch (ret) {
 	case 0: /* Not found, but valid ref */
diff --git a/bundle.c b/bundle.c
index 995a940dfd..cb0e5931ac 100644
--- a/bundle.c
+++ b/bundle.c
@@ -403,7 +403,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 
 		if (e->item->flags & UNINTERESTING)
 			continue;
-		if (dwim_ref(e->name, strlen(e->name), &oid, &ref) != 1)
+		if (dwim_ref(e->name, strlen(e->name), &oid, &ref, 0) != 1)
 			goto skip_write_ref;
 		if (read_ref_full(e->name, RESOLVE_REF_READING, &oid, &flag))
 			flag = 0;
diff --git a/cache.h b/cache.h
index 4f16a57ba4..cee8aa5dc3 100644
--- a/cache.h
+++ b/cache.h
@@ -1569,6 +1569,13 @@ struct interpret_branch_name_options {
 	 * allowed, even ones to refs outside of those namespaces.
 	 */
 	unsigned allowed;
+
+	/*
+	 * If ^{upstream} or ^{push} (or equivalent) is requested, and the
+	 * branch in question does not have such a reference, return -1 instead
+	 * of die()-ing.
+	 */
+	unsigned nonfatal_dangling_mark : 1;
 };
 int repo_interpret_branch_name(struct repository *r,
 			       const char *str, int len,
diff --git a/commit.c b/commit.c
index 4ce8cb38d5..119892abbc 100644
--- a/commit.c
+++ b/commit.c
@@ -921,7 +921,7 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
 	struct commit *ret = NULL;
 	char *full_refname;
 
-	switch (dwim_ref(refname, strlen(refname), &oid, &full_refname)) {
+	switch (dwim_ref(refname, strlen(refname), &oid, &full_refname, 0)) {
 	case 0:
 		die("No such ref: '%s'", refname);
 	case 1:
diff --git a/refs.c b/refs.c
index 0fa6067340..e06ee22a8a 100644
--- a/refs.c
+++ b/refs.c
@@ -598,10 +598,13 @@ const char *git_default_branch_name(void)
  * to name a branch.
  */
 static char *substitute_branch_name(struct repository *r,
-				    const char **string, int *len)
+				    const char **string, int *len,
+				    int nonfatal_dangling_mark)
 {
 	struct strbuf buf = STRBUF_INIT;
-	struct interpret_branch_name_options options = { 0 } ;
+	struct interpret_branch_name_options options = {
+		.nonfatal_dangling_mark = nonfatal_dangling_mark
+	};
 	int ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);
 
 	if (ret == *len) {
@@ -615,9 +618,10 @@ static char *substitute_branch_name(struct repository *r,
 }
 
 int repo_dwim_ref(struct repository *r, const char *str, int len,
-		  struct object_id *oid, char **ref)
+		  struct object_id *oid, char **ref, int nonfatal_dangling_mark)
 {
-	char *last_branch = substitute_branch_name(r, &str, &len);
+	char *last_branch = substitute_branch_name(r, &str, &len,
+						   nonfatal_dangling_mark);
 	int   refs_found  = expand_ref(r, str, len, oid, ref);
 	free(last_branch);
 	return refs_found;
@@ -661,7 +665,7 @@ int repo_dwim_log(struct repository *r, const char *str, int len,
 		  struct object_id *oid, char **log)
 {
 	struct ref_store *refs = get_main_ref_store(r);
-	char *last_branch = substitute_branch_name(r, &str, &len);
+	char *last_branch = substitute_branch_name(r, &str, &len, 0);
 	const char **p;
 	int logs_found = 0;
 	struct strbuf path = STRBUF_INIT;
diff --git a/refs.h b/refs.h
index 8cbef96a8d..e03c106320 100644
--- a/refs.h
+++ b/refs.h
@@ -151,12 +151,14 @@ struct strvec;
 void expand_ref_prefix(struct strvec *prefixes, const char *prefix);
 
 int expand_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
-int repo_dwim_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
+int repo_dwim_ref(struct repository *r, const char *str, int len,
+		  struct object_id *oid, char **ref, int nonfatal_dangling_mark);
 int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref);
 static inline int dwim_ref(const char *str, int len, struct object_id *oid,
-			   char **ref)
+			   char **ref, int nonfatal_dangling_mark)
 {
-	return repo_dwim_ref(the_repository, str, len, oid, ref);
+	return repo_dwim_ref(the_repository, str, len, oid, ref,
+			     nonfatal_dangling_mark);
 }
 int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
 
diff --git a/remote.c b/remote.c
index c5ed74f91c..420150837b 100644
--- a/remote.c
+++ b/remote.c
@@ -1558,7 +1558,7 @@ static void set_merge(struct branch *ret)
 		    strcmp(ret->remote_name, "."))
 			continue;
 		if (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]),
-			     &oid, &ref) == 1)
+			     &oid, &ref, 0) == 1)
 			ret->merge[i]->dst = ref;
 		else
 			ret->merge[i]->dst = xstrdup(ret->merge_name[i]);
diff --git a/sha1-name.c b/sha1-name.c
index a7a9de66c4..0b23b86ceb 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -809,7 +809,7 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 
 	if (len == r->hash_algo->hexsz && !get_oid_hex(str, oid)) {
 		if (warn_ambiguous_refs && warn_on_object_refname_ambiguity) {
-			refs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref);
+			refs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref, 0);
 			if (refs_found > 0) {
 				warning(warn_msg, len, str);
 				if (advice_object_name_warning)
@@ -860,11 +860,11 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 
 	if (!len && reflog_len)
 		/* allow "@{...}" to mean the current branch reflog */
-		refs_found = repo_dwim_ref(r, "HEAD", 4, oid, &real_ref);
+		refs_found = repo_dwim_ref(r, "HEAD", 4, oid, &real_ref, 0);
 	else if (reflog_len)
 		refs_found = repo_dwim_log(r, str, len, oid, &real_ref);
 	else
-		refs_found = repo_dwim_ref(r, str, len, oid, &real_ref);
+		refs_found = repo_dwim_ref(r, str, len, oid, &real_ref, 0);
 
 	if (!refs_found)
 		return -1;
@@ -1496,8 +1496,14 @@ static int interpret_branch_mark(struct repository *r,
 		branch = branch_get(NULL);
 
 	value = get_data(branch, &err);
-	if (!value)
-		die("%s", err.buf);
+	if (!value) {
+		if (options->nonfatal_dangling_mark) {
+			strbuf_release(&err);
+			return -1;
+		} else {
+			die("%s", err.buf);
+		}
+	}
 
 	if (!branch_interpret_allowed(value, options->allowed))
 		return -1;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index e81759319f..45e1f6ff68 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -846,6 +846,18 @@ test_expect_success 'status refreshes the index' '
 	test_cmp expect output
 '
 
+test_expect_success 'status shows detached HEAD properly after checking out non-local upstream branch' '
+	test_when_finished rm -rf upstream downstream actual &&
+
+	test_create_repo upstream &&
+	test_commit -C upstream foo &&
+
+	git clone upstream downstream &&
+	git -C downstream checkout @{u} &&
+	git -C downstream status >actual &&
+	test_i18ngrep "HEAD detached at [0-9a-f]\\+" actual
+'
+
 test_expect_success 'setup status submodule summary' '
 	test_create_repo sm && (
 		cd sm &&
diff --git a/wt-status.c b/wt-status.c
index 7ce58b8aae..a99b7a0c59 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1569,7 +1569,7 @@ static void wt_status_get_detached_from(struct repository *r,
 		return;
 	}
 
-	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 &&
+	if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 &&
 	    /* sha1 is a commit? match without further lookup */
 	    (oideq(&cb.noid, &oid) ||
 	     /* perhaps sha1 is a tag, try to dereference to a commit */
-- 
2.28.0.402.g5ffc5be6b7-goog