checkout & worktree: introduce a core.DWIMRemote setting

Introduce a core.DWIMRemote setting which can be used to designate a remote to prefer (via core.DWIMRemote=origin) when running e.g. "git checkout master" to mean origin/master, even though there's other remotes that have the "master" branch.

I want this because it's very handy to use this workflow to checkout a repository and create a topic branch, then get back to a "master" as retrieved from upstream:

    (
        rm -rf /tmp/tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git checkout master
    )

That will output:

Branch 'master' set up to track remote branch 'master' from 'origin'. Switched to a new branch 'master'

But as soon as a new remote is added (e.g. just to inspect something from someone else) the DWIMery goes away:

    (
        rm -rf /tmp/tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git remote add avar [email protected]:avar/tbdiff.git &&
        git fetch avar &&
        git checkout master
    )

Will output:

error: pathspec 'master' did not match any file(s) known to git.

The new core.DWIMRemote config allows me to say that whenever that ambiguity comes up I'd like to prefer "origin", and it'll still work as though the only remote I had was "origin".

I considered calling the config setting checkout.DWIMRemote, but then discovered that this behavior is also used by "worktree" for similar purposes, so it makes sense to have it under core.*. As noted in the documentation we may also want to use this in other commands in the future if they have similar DWIM behavior in the presence of one remote.

See also 70c9ac2f19 ("DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"", 2009-10-18) which introduced this DWIM feature to begin with, and 4e85333197 ("worktree: make add <path> <branch> dwim", 2017-11-26) which added it to git-worktree.

---
 Documentation/config.txt       | 15 +++++++++++++++
 Documentation/git-checkout.txt |  5 +++++
 Documentation/git-worktree.txt |  5 +++++
 builtin/checkout.c             |  3 ++-
 checkout.c                     | 17 +++++++++++++++--
 t/t2024-checkout-dwim.sh       | 10 ++++++++++
 t/t2025-worktree-add.sh        | 18 ++++++++++++++++++
 7 files changed, 70 insertions(+), 3 deletions(-)

index 2659153cb3..d4740fcdb9 100644
@@ -898,6 +898,21 @@ core.notesRef::
 This setting defaults to "refs/notes/commits", and it can be overridden by
 the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
 
+core.DWIMRemote::
+	Various git commands will look at references on the configured
+	remotes and DW[YI]M (Do What (You|I) Mean) if the reference
+	only exists on one remote. This setting allows for setting the
+	name of a special remote that should always win when it comes
+	to disambiguation. The typical use-case is to set this to
+	`origin`.
++
+Currently this is used by linkgit:git-checkout[1] when `git checkout
+<something>' will checkout the '<something>' branch on another remote,
+and linkgit:git-worktree[1] when 'git worktree add' similarly DWYM
+when a branch is unique across remotes, or this setting is set to a
+special remote. This setting might be used for other commands or
+functionality in the future when appropriate.
+
 core.sparseCheckout::
 	Enable "sparse checkout" feature. See section "Sparse checkout" in
 	linkgit:git-read-tree[1] for more information.

index ca5fc9c798..8ad7be6c53 100644
@@ -38,6 +38,11 @@ equivalent to
 $ git checkout -b <branch> --track <remote>/<branch>
 ------------
 +
+If the branch exists in multiple remotes the `core.DWIMRemote`
+variable can be used to pick the remote you really mean. Set it to
+e.g. `core.DWIMRemote=origin` to always checkout remote branches from
+there. See also `core.DWIMRemote` in linkgit:git-config[1].
++
 You could omit <branch>, in which case the command degenerates to
 "check out the current branch", which is a glorified no-op with
 rather expensive side-effects to show only the tracking information,

index 2755ca90e3..37db12f816 100644
@@ -60,6 +60,11 @@ with a matching name, treat as equivalent to:
 $ git worktree add --track -b <branch> <path> <remote>/<branch>
 ------------
 +
+It's also possible to use the `core.DWIMRemote` setting to designate a
+special remote this rule should be applied to, even if the branch
+isn't unique across all remotes. See `core.DWIMRemote` in
+linkgit:git-config[1].
++
 If `<commit-ish>` is omitted and neither `-b` nor `-B` nor `--detach` used,
 then, as a convenience, a new branch based at HEAD is created automatically,
 as if `-b $(basename <path>)` was specified.

index b49b582071..8622ad4ef1 100644
@@ -903,7 +903,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 	 *   (b) If <something> is _not_ a commit, either "--" is present
 	 *       or <something> is not a path, no -t or -b was given, and
 	 *       and there is a tracking branch whose name is <something>
-	 *       in one and only one remote, then this is a short-hand to
+	 *       in one and only one remote (or if the branch exists on the
+	 *       remote named in core.DWIMRemote), then this is a short-hand to
 	 *       fork local <something> from that remote-tracking branch.
 	 *
 	 *   (c) Otherwise, if "--" is present, treat it like case (1).

index ac42630f74..6e474fbfbb 100644
@@ -1,12 +1,15 @@
 #include "cache.h"
 #include "remote.h"
 #include "checkout.h"
+#include "config.h"
 
 struct tracking_name_data {
 	/* const */ char *src_ref;
 	char *dst_ref;
 	struct object_id *dst_oid;
 	int unique;
+	const char *dwim_remote;
+	char *dwim_dst_ref;
 };
 
 static int check_tracking_name(struct remote *remote, void *cb_data)

@@ -20,6 +23,8 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 		free(query.dst);
 		return 0;
 	}
+	if (cb->dwim_remote && !strcmp(remote->name, cb->dwim_remote))
+		cb->dwim_dst_ref = xstrdup(query.dst);
 	if (cb->dst_ref) {
 		free(query.dst);
 		cb->unique = 0;

@@ -31,13 +36,21 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 
 const char *unique_tracking_name(const char *name, struct object_id *oid)
 {
-	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	const char *dwim_remote = NULL;
+	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1, NULL, NULL };
+	if (!git_config_get_string_const("core.dwimremote", &dwim_remote))
+		cb_data.dwim_remote = dwim_remote;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
 	free(cb_data.src_ref);
-	if (cb_data.unique)
+	free((char *)dwim_remote);
+	if (cb_data.unique) {
+		free(cb_data.dwim_dst_ref);
 		return cb_data.dst_ref;
+	}
 	free(cb_data.dst_ref);
+	if (cb_data.dwim_dst_ref)
+		return cb_data.dwim_dst_ref;
 	return NULL;
 }

index 3e5ac81bd2..d8db2a14d7 100755
@@ -68,6 +68,16 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '
 	test_branch master
 '
 
+test_expect_success 'checkout of branch from multiple remotes succeeds with core.DWIMRemote #1' '
+	git checkout -B master &&
+	test_might_fail git branch -D foo &&
+
+	git -c core.DWIMRemote=repo_a checkout foo &&
+	test_branch foo &&
+	test_cmp_rev remotes/repo_a/foo HEAD &&
+	test_branch_upstream foo repo_a foo
+'
+
 test_expect_success 'checkout of branch from a single remote succeeds #1' '
 	git checkout -B master &&
 	test_might_fail git branch -D bar &&

index d0d2e4f7ec..6be4cd03da 100755
@@ -450,6 +450,24 @@ test_expect_success 'git worktree --no-guess-remote option overrides config' '
 	)
 '
 
+test_expect_success '"add" <path> <branch> dwims with core.DWIMRemote' '
+	test_when_finished rm -rf repo_upstream repo_dwim foo &&
+	setup_remote_repo repo_upstream repo_dwim &&
+	git init repo_dwim &&
+	(
+		cd repo_dwim &&
+		git remote add repo_upstream2 ../repo_upstream &&
+		git fetch repo_upstream2 &&
+		test_must_fail git worktree add ../foo foo &&
+		git -c core.DWIMRemote=repo_upstream worktree add ../foo foo
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_upstream foo &&
+		test_cmp_rev refs/remotes/repo_upstream/foo refs/heads/foo
+	)
+'
+
 post_checkout_hook () {
 	gitdir=${1:-.git}
 	test_when_finished "rm -f $gitdir/hooks/post-checkout" &&

2.17.0.290.gded63e768a

Do we anticipate more dwimy customizations to justify a new dwim group in config (i.e. dwim.remote instead of core.dwimRemote)?

Duy

A few observations:

1. DWIM has broad meaning; while this certainly falls within DWIM, it's also just a _default_, so should this instead be named "defaultRemote"?

2. Building on #1: How well is the term "DWIM" understood by non-power users? A term, such as "default" is more well known.

3. git-worktree learned --guess-remote and worktree.guessRemote in [1] and [2], which, on the surface, sound confusingly similar to "DWIMRemote". Even though I was well involved in the discussion and repeatedly reviewed the patch series which introduced those, it still took me an embarrassingly long time, and repeated re-reads of all commit messages involved, to grasp (or re-grasp) how "guess remote" and "DWIM remote" differ. If it was that difficult for me, as a person involved in the patch series, to figure out "guess remote" vs. "DWIM remote", then I worry that it may be too confusing in general. If the new config option had been named "defaultRemote", I don't think I'd have been confused at all.

It may sound as if I'm advocating the name "defaultRemote", but that wasn't the intention, and I'm not wedded to it; that name simply fell out naturally as I enumerated the above observations.

[1]: 71d6682d8c (worktree: add --guess-remote flag to add subcommand, 2017-11-29)
[2]: e92445a731 (add worktree.guessRemote config option, 2017-11-29)

I'm going off topic but I kinda dislike this term. First time I encountered it in the code I didn't even know what it meant. Since it has not been leaked to user documents (I only did a grep on Documentation/) perhaps a better term should be used, preferably not another acronym.
--
Duy

I've got no love for the DWIM term. And I think I should change it in v2, I just want some way to enable this functionality since this behavior has annoyed me for a long time.

I wonder though if something like "core.defaultRemote" might not bring up connotations about whether this would e.g. affect "push" in the minds of users (not that my initial suggestion is better).

So maybe something like checkout.implicitRemote would be better? And we could just break the rule that only git-checkout would use it, since git-worktree could be said to be doing something checkout-like, or just also add a worktree.implicitRemote.

I hadn't looked at this at all before today when I wrote the patch, and I've never used git-worktree (but maybe I should...), but my understanding of this --[no-]guess-remote functionality is that it effectively splits up the functionality that the "git checkout" does, and we'll unconditionally check out the branch, but the option controls whether or not we'd set up the equivalent of remote tracking for git-worktree.

But maybe I've completely misunderstood it.

Introduce a checkout.implicitRemote setting which can be used to designate a remote to prefer (via checkout.implicitRemote=origin) when running e.g. "git checkout master" to mean origin/master, even though there's other remotes that have the "master" branch.

I want this because it's very handy to use this workflow to checkout a repository and create a topic branch, then get back to a "master" as retrieved from upstream:

    (
        rm -rf /tmp/tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git checkout master
    )

That will output:

Branch 'master' set up to track remote branch 'master' from 'origin'. Switched to a new branch 'master'

But as soon as a new remote is added (e.g. just to inspect something from someone else) the DWIMery goes away:

    (
        rm -rf /tmp/tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git remote add avar [email protected]:avar/tbdiff.git &&
        git fetch avar &&
        git checkout master
    )

Will output:

error: pathspec 'master' did not match any file(s) known to git.

The new checkout.implicitRemote config allows me to say that whenever that ambiguity comes up I'd like to prefer "origin", and it'll still work as though the only remote I had was "origin".

I considered splitting this into checkout.implicitRemote and worktree.implicitRemote, but it's probably less confusing to break our own rules that anything shared between config should live in core.* than have two config settings, and I couldn't come up with a short name under core.* that made sense (core.implicitRemoteForCheckout?).

See also 70c9ac2f19 ("DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"", 2009-10-18) which introduced this DWIM feature to begin with, and 4e85333197 ("worktree: make add <path> <branch> dwim", 2017-11-26) which added it to git-worktree.

---

In v2 I've changed the config name to checkout.implicitRemote as suggested above, there's no worktree.implicitRemote, instead the config just explains that it's used for checkout-like things (and may be expanded in the future).

A branch-diff (new command from JS) follows:

1: 18f13ecd89 ! 1: 905a17f35f checkout & worktree: introduce a core.DWIMRemote setting
@@ -1,11 +1,11 @@
Author: Ævar Arnfjörð Bjarmason <[email protected]>

- checkout & worktree: introduce a core.DWIMRemote setting
+ checkout & worktree: introduce checkout.implicitRemote

- Introduce a core.DWIMRemote setting which can be used to designate a
- remote to prefer (via core.DWIMRemote=origin) when running e.g. "git
- checkout master" to mean origin/master, even though there's other
- remotes that have the "master" branch.
+ Introduce a checkout.implicitRemote setting which can be used to
+ designate a remote to prefer (via checkout.implicitRemote=origin) when
+ running e.g. "git checkout master" to mean origin/master, even though
+ there's other remotes that have the "master" branch.

I want this because it's very handy to use this workflow to checkout a repository and create a topic branch, then get back to a "master" as
@@ -41,16 +41,15 @@

error: pathspec 'master' did not match any file(s) known to git.

- The new core.DWIMRemote config allows me to say that whenever that
- ambiguity comes up I'd like to prefer "origin", and it'll still work
- as though the only remote I had was "origin".
+ The new checkout.implicitRemote config allows me to say that whenever
+ that ambiguity comes up I'd like to prefer "origin", and it'll still
+ work as though the only remote I had was "origin".

- I considered calling the config setting checkout.DWIMRemote, but then
- discovered that this behavior is also used by "worktree" for similar
- purposes, so it makes sense to have it under core.*. As noted in the
- documentation we may also want to use this in other commands in the
- future if they have similar DWIM behavior in the presence of one
- remote.
+ I considered splitting this into checkout.implicitRemote and
+ worktree.implicitRemote, but it's probably less confusing to break our
+ own rules that anything shared between config should live in core.*
+ than have two config settings, and I couldn't come up with a short
+ name under core.* that made sense (core.implicitRemoteForCheckout?).

See also 70c9ac2f19 ("DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"", 2009-10-18) which introduced this DWIM feature
@@ -63,27 +62,29 @@
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@
- This setting defaults to "refs/notes/commits", and it can be overridden by
- the GIT_NOTES_REF environment variable. See linkgit:git-notes[1].
+ browse HTML help (see -w option in linkgit:git-help[1]) or a
+ working repository in gitweb (see linkgit:git-instaweb[1]).

-+core.DWIMRemote::
-+ Various git commands will look at references on the configured
-+ remotes and DW[YI]M (Do What (You|I) Mean) if the reference
-+ only exists on one remote. This setting allows for setting the
-+ name of a special remote that should always win when it comes
-+ to disambiguation. The typical use-case is to set this to
++checkout.implicitRemote::
++ When you run 'git checkout <something>' and only have one
++ remote, it may implicitly fall back on checking out and
++ tracking e.g. 'origin/<something>'. This stops working as soon
++ as you have more than one remote with a '<something>'
++ reference. This setting allows for setting the name of a
++ special remote that should always win when it comes to
++ disambiguation. The typical use-case is to set this to
+ origin.
++
-+Currently this is used by linkgit:git-checkout[1] when `git checkout ++Currently this is used by linkgit:git-checkout[1] when 'git checkout
+<something>' will checkout the '<something>' branch on another remote,
-+and linkgit:git-worktree[1] when 'git worktree add' similarly DWYM -+when a branch is unique across remotes, or this setting is set to a -+special remote. This setting might be used for other commands or -+functionality in the future when appropriate.
++and by linkgit:git-worktree[1] when 'git worktree add' when referring ++to a remote branch. This setting might be used for other ++checkout-like commands or functionality in the future when ++appropriate.
+
- core.sparseCheckout::
- Enable "sparse checkout" feature. See section "Sparse checkout" in
- linkgit:git-read-tree[1] for more information.
+ clean.requireForce::
+ A boolean to make git-clean do nothing unless given -f,
+ -i or -n. Defaults to true.

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt --- a/Documentation/git-checkout.txt
@@ -92,10 +93,11 @@
$ git checkout -b <branch> --track <remote>/<branch>
------------
+
-+If the branch exists in multiple remotes the core.DWIMRemote ++If the branch exists in multiple remotes the checkout.implicitRemote
+variable can be used to pick the remote you really mean. Set it to
-+e.g. core.DWIMRemote=origin to always checkout remote branches from -+there. See also core.DWIMRemote in linkgit:git-config[1]. ++e.g. checkout.implicitRemote=origin to always checkout remote ++branches from there. See also checkout.implicitRemote in ++linkgit:git-config[1].
++
You could omit <branch>, in which case the command degenerates to "check out the current branch", which is a glorified no-op with
@@ -108,10 +110,10 @@
$ git worktree add --track -b <branch> <path> <remote>/<branch>
------------
+
-+It's also possible to use the core.DWIMRemote setting to designate a -+special remote this rule should be applied to, even if the branch -+isn't unique across all remotes. See core.DWIMRemote in -+linkgit:git-config[1].
++It's also possible to use the checkout.implicitRemote setting to ++designate a special remote this rule should be applied to, even if the ++branch isn't unique across all remotes. See checkout.implicitRemote ++in linkgit:git-config[1].
++
If <commit-ish> is omitted and neither -b nor -B nor --detach used, then, as a convenience, a new branch based at HEAD is created automatically,
@@ -125,11 +127,14 @@
* or <something> is not a path, no -t or -b was given, and
* and there is a tracking branch whose name is <something>
- * in one and only one remote, then this is a short-hand to
+- * fork local <something> from that remote-tracking branch.
+ * in one and only one remote (or if the branch exists on the
-+ * remote named in core.DWIMRemote), then this is a short-hand to
- * fork local <something> from that remote-tracking branch.
++ * remote named in checkout.implicitRemote), then this is a
++ * short-hand to fork local <something> from that
++ * remote-tracking branch.
*
* (c) Otherwise, if "--" is present, treat it like case (1).
+ *

diff --git a/checkout.c b/checkout.c
--- a/checkout.c
@@ -145,8 +150,8 @@
char *dst_ref;
struct object_id *dst_oid;
int unique;
-+ const char *dwim_remote;
-+ char *dwim_dst_ref;
++ const char *implicit_remote;
++ char *implicit_dst_ref;
};

static int check_tracking_name(struct remote *remote, void *cb_data)
@@ -154,8 +159,8 @@
free(query.dst);
return 0;
}
-+ if (cb->dwim_remote && !strcmp(remote->name, cb->dwim_remote))
-+ cb->dwim_dst_ref = xstrdup(query.dst);
++ if (cb->implicit_remote && !strcmp(remote->name, cb->implicit_remote))
++ cb->implicit_dst_ref = xstrdup(query.dst);
if (cb->dst_ref) {
free(query.dst);
cb->unique = 0;
@@ -164,23 +169,23 @@
const char *unique_tracking_name(const char *name, struct object_id *oid) {
- struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
-+ const char *dwim_remote = NULL;
++ const char *implicit_remote = NULL;
+ struct tracking_name_data cb_data = { NULL, NULL, NULL, 1, NULL, NULL };
-+ if (!git_config_get_string_const("core.dwimremote", &dwim_remote))
-+ cb_data.dwim_remote = dwim_remote;
++ if (!git_config_get_string_const("checkout.implicitremote", &implicit_remote))
++ cb_data.implicit_remote = implicit_remote;
cb_data.src_ref = xstrfmt("refs/heads/%s", name); cb_data.dst_oid = oid; for_each_remote(check_tracking_name, &cb_data); free(cb_data.src_ref);
- if (cb_data.unique)
-+ free((char *)dwim_remote);
++ free((char *)implicit_remote);
+ if (cb_data.unique) {
-+ free(cb_data.dwim_dst_ref);
++ free(cb_data.implicit_dst_ref);
return cb_data.dst_ref;
+ }
free(cb_data.dst_ref);
-+ if (cb_data.dwim_dst_ref)
-+ return cb_data.dwim_dst_ref;
++ if (cb_data.implicit_dst_ref)
++ return cb_data.implicit_dst_ref;
return NULL;
}

@@ -191,11 +196,11 @@
test_branch master
'

-+test_expect_success 'checkout of branch from multiple remotes succeeds with core.DWIMRemote #1' ' ++test_expect_success 'checkout of branch from multiple remotes succeeds with checkout.implicitRemote #1' '
+ git checkout -B master &&
+ test_might_fail git branch -D foo &&
+
-+ git -c core.DWIMRemote=repo_a checkout foo &&
++ git -c checkout.implicitRemote=repo_a checkout foo &&
+ test_branch foo &&
+ test_cmp_rev remotes/repo_a/foo HEAD &&
+ test_branch_upstream foo repo_a foo
@@ -212,7 +217,7 @@
)
'

-+test_expect_success '"add" <path> <branch> dwims with core.DWIMRemote' ' ++test_expect_success '"add" <path> <branch> dwims with checkout.implicitRemote' '
+ test_when_finished rm -rf repo_upstream repo_dwim foo &&
+ setup_remote_repo repo_upstream repo_dwim &&
+ git init repo_dwim &&
@@ -221,7 +226,7 @@
+ git remote add repo_upstream2 ../repo_upstream &&
+ git fetch repo_upstream2 &&
+ test_must_fail git worktree add ../foo foo &&
-+ git -c core.DWIMRemote=repo_upstream worktree add ../foo foo
++ git -c checkout.implicitRemote=repo_upstream worktree add ../foo foo
+ ) &&
+ (
+ cd foo &&

 Documentation/config.txt       | 17 +++++++++++++++++
 Documentation/git-checkout.txt |  6 ++++++
 Documentation/git-worktree.txt |  5 +++++
 builtin/checkout.c             |  6 ++++--
 checkout.c                     | 17 +++++++++++++++--
 t/t2024-checkout-dwim.sh       | 10 ++++++++++
 t/t2025-worktree-add.sh        | 18 ++++++++++++++++++
 7 files changed, 75 insertions(+), 4 deletions(-)

index 2659153cb3..83460bd443 100644
@@ -1084,6 +1084,23 @@ browser.<tool>.path::
 	browse HTML help (see `-w` option in linkgit:git-help[1]) or a
 	working repository in gitweb (see linkgit:git-instaweb[1]).
 
+checkout.implicitRemote::
+	When you run 'git checkout <something>' and only have one
+	remote, it may implicitly fall back on checking out and
+	tracking e.g. 'origin/<something>'. This stops working as soon
+	as you have more than one remote with a '<something>'
+	reference. This setting allows for setting the name of a
+	special remote that should always win when it comes to
+	disambiguation. The typical use-case is to set this to
+	`origin`.
++
+Currently this is used by linkgit:git-checkout[1] when 'git checkout
+<something>' will checkout the '<something>' branch on another remote,
+and by linkgit:git-worktree[1] when 'git worktree add' when referring
+to a remote branch.  This setting might be used for other
+checkout-like commands or functionality in the future when
+appropriate.
+
 clean.requireForce::
 	A boolean to make git-clean do nothing unless given -f,
 	-i or -n.   Defaults to true.

index ca5fc9c798..753aa4001f 100644
@@ -38,6 +38,12 @@ equivalent to
 $ git checkout -b <branch> --track <remote>/<branch>
 ------------
 +
+If the branch exists in multiple remotes the `checkout.implicitRemote`
+variable can be used to pick the remote you really mean. Set it to
+e.g. `checkout.implicitRemote=origin` to always checkout remote
+branches from there. See also `checkout.implicitRemote` in
+linkgit:git-config[1].
++
 You could omit <branch>, in which case the command degenerates to
 "check out the current branch", which is a glorified no-op with
 rather expensive side-effects to show only the tracking information,

index 2755ca90e3..35798a1132 100644
@@ -60,6 +60,11 @@ with a matching name, treat as equivalent to:
 $ git worktree add --track -b <branch> <path> <remote>/<branch>
 ------------
 +
+It's also possible to use the `checkout.implicitRemote` setting to
+designate a special remote this rule should be applied to, even if the
+branch isn't unique across all remotes. See `checkout.implicitRemote`
+in linkgit:git-config[1].
++
 If `<commit-ish>` is omitted and neither `-b` nor `-B` nor `--detach` used,
 then, as a convenience, a new branch based at HEAD is created automatically,
 as if `-b $(basename <path>)` was specified.

index b49b582071..495a5d7955 100644
@@ -903,8 +903,10 @@ static int parse_branchname_arg(int argc, const char **argv,
 	 *   (b) If <something> is _not_ a commit, either "--" is present
 	 *       or <something> is not a path, no -t or -b was given, and
 	 *       and there is a tracking branch whose name is <something>
-	 *       in one and only one remote, then this is a short-hand to
-	 *       fork local <something> from that remote-tracking branch.
+	 *       in one and only one remote (or if the branch exists on the
+	 *       remote named in checkout.implicitRemote), then this is a
+	 *       short-hand to fork local <something> from that
+	 *       remote-tracking branch.
 	 *
 	 *   (c) Otherwise, if "--" is present, treat it like case (1).
 	 *

index ac42630f74..2c898cfe19 100644
@@ -1,12 +1,15 @@
 #include "cache.h"
 #include "remote.h"
 #include "checkout.h"
+#include "config.h"
 
 struct tracking_name_data {
 	/* const */ char *src_ref;
 	char *dst_ref;
 	struct object_id *dst_oid;
 	int unique;
+	const char *implicit_remote;
+	char *implicit_dst_ref;
 };
 
 static int check_tracking_name(struct remote *remote, void *cb_data)

@@ -20,6 +23,8 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 		free(query.dst);
 		return 0;
 	}
+	if (cb->implicit_remote && !strcmp(remote->name, cb->implicit_remote))
+		cb->implicit_dst_ref = xstrdup(query.dst);
 	if (cb->dst_ref) {
 		free(query.dst);
 		cb->unique = 0;

@@ -31,13 +36,21 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 
 const char *unique_tracking_name(const char *name, struct object_id *oid)
 {
-	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	const char *implicit_remote = NULL;
+	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1, NULL, NULL };
+	if (!git_config_get_string_const("checkout.implicitremote", &implicit_remote))
+		cb_data.implicit_remote = implicit_remote;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
 	free(cb_data.src_ref);
-	if (cb_data.unique)
+	free((char *)implicit_remote);
+	if (cb_data.unique) {
+		free(cb_data.implicit_dst_ref);
 		return cb_data.dst_ref;
+	}
 	free(cb_data.dst_ref);
+	if (cb_data.implicit_dst_ref)
+		return cb_data.implicit_dst_ref;
 	return NULL;
 }

index 3e5ac81bd2..da6bd74bbc 100755
@@ -68,6 +68,16 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '
 	test_branch master
 '
 
+test_expect_success 'checkout of branch from multiple remotes succeeds with checkout.implicitRemote #1' '
+	git checkout -B master &&
+	test_might_fail git branch -D foo &&
+
+	git -c checkout.implicitRemote=repo_a checkout foo &&
+	test_branch foo &&
+	test_cmp_rev remotes/repo_a/foo HEAD &&
+	test_branch_upstream foo repo_a foo
+'
+
 test_expect_success 'checkout of branch from a single remote succeeds #1' '
 	git checkout -B master &&
 	test_might_fail git branch -D bar &&

index d0d2e4f7ec..6eb994ac12 100755
@@ -450,6 +450,24 @@ test_expect_success 'git worktree --no-guess-remote option overrides config' '
 	)
 '
 
+test_expect_success '"add" <path> <branch> dwims with checkout.implicitRemote' '
+	test_when_finished rm -rf repo_upstream repo_dwim foo &&
+	setup_remote_repo repo_upstream repo_dwim &&
+	git init repo_dwim &&
+	(
+		cd repo_dwim &&
+		git remote add repo_upstream2 ../repo_upstream &&
+		git fetch repo_upstream2 &&
+		test_must_fail git worktree add ../foo foo &&
+		git -c checkout.implicitRemote=repo_upstream worktree add ../foo foo
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_upstream foo &&
+		test_cmp_rev refs/remotes/repo_upstream/foo refs/heads/foo
+	)
+'
+
 post_checkout_hook () {
 	gitdir=${1:-.git}
 	test_when_finished "rm -f $gitdir/hooks/post-checkout" &&

2.17.0.290.gded63e768a

I wonder if it's difficult to add a dwim hook that allows us to replace the entire dwim logic with a hook? Doing this "preferring origin" in a shell script should be easy and it lets us play more with tweaking dwim logic. (Yeah sorry I'm getting off topic again)

Duy

Realistically the way we do hooks now would make the UI of this suck, i.e. you couldn't configure it globally or system-wide easily. Something like what I noted in
https://public-inbox.org/git/[email protected]/ would make it suck less, but that's a much bigger task.

I think for now this is a common enough case users run into that it makes sense to add it to the code, and if we ever do have more extensible hook features in the future we can just go and replace various options like these to use them.

I thought you would bring this up :) I've given some more thoughts on this topic and am willing to implement something like below, in a week or two. Would that help change your mind?

I proposed hooks.* config space in that same thread. Here is the extension to make it cover both of your points.

hooks.* can have multiple values. So you can specify hooks.post-checkout multiple times and all those scripts will run (in the same order you specified). Since we already have a search path for config (/etc/gitconfig -> $HOME/.gitconfig -> $REPO/config) this helps hooks management as well. Execution order is still the same: if you specify hooks.post-checkout in both /etc/gitconfig and .git/config, then the one in /etc/gitconfig is executed first, the one in .git second.

And here's something extra to make it possible to override the search
order: if you specify hooks.post-checkout = reset (reset is a random keyword) then _all_ post-checkout hooks before this point are ignored. So you can put this in .git/config

[hooks]
post-checkout = reset
post-checkout = ~/some-hook

and can be sure that post-checkout specified in $HOME and /etc will not affect you, only ~/some-hook will run. If you drop the second line then you have no post-checkout hooks. This is a workaround for a bigger problem (not being able to unset a config entry) but I think it's sufficient for this use case.

Duy

A few things:

1) I don't see a reason to hold back this feature in particular waiting for some way to do it via config / hooks. If we grow some compatible way to do it via hooks in the future, great, then we can just make this (and numerous other config values) historical aliases for that facility.

2) Let's not have some per-config type way to reset earlier config. I suggested a way to do it generally in
https://public-inbox.org/git/[email protected]/ I'm not saying we should go for that suggestion in particular, but that we should have something general.

3) Doing #2 will take a lot longer to implement than what you're suggesting.

4) I think such a facility should also be able to replace something like https://docs.gitlab.com/ee/administration/custom_hooks.html
which requires not just supporting hooks from the config, but executing some hooks on the FS in glob() order.

5) What you're describing above is just 1/2 of what we need to have a viable way to replace something like this checkout.implicitRemote with a hook while providing the same functionality to the end user.

If something ships as a config value like checkout.implicitRemote users can just turn it on in their ~/.gitconfig without any further config, or you can tell users in docs via one-liner to enable it.

We also need the other half which some method of shipping "standard" hooks with git, i.e. with your proposal something like (along with the general config reset):

[config]
reject = hooks.post-checkout
[hooks]
# Reads config from hooks.post-checkout-implicit-remote.* # (e.g. hooks.post-checkout-implicit-remote.remote = origin) post-checkout = git-hooks://post-checkout-implicit-remote-config

Only then will this be as easy to enable as git config --global.. (although you'll need two invocations of that, which is fine...)

6) The feature being discussed here would not be a post-checkout hook, but would need to be fairly integrated with the internals of checkout, see dwim_ok and the big comment in parse_branchname_arg().

I.e. the thing that makes this work is the Nth step in some fairly intricate fallback logic. It's not clear to me in the general case how we'd turn this into a hook without having N number of checkout-what-step-N-hooks, or requiring every checkout-what hook to implement a huge part of what checkout.c is doing now just to tweak some tiny aspect of it, such as this tweak of unique_tracking_name().

Introduce a checkout.implicitRemote setting which can be used to designate a remote to prefer (via checkout.implicitRemote=origin) when running e.g. "git checkout master" to mean origin/master, even though there's other remotes that have the "master" branch.

I want this because it's very handy to use this workflow to checkout a repository and create a topic branch, then get back to a "master" as retrieved from upstream:

    (
        rm -rf /tmp/tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git checkout master
    )

That will output:

Branch 'master' set up to track remote branch 'master' from 'origin'. Switched to a new branch 'master'

But as soon as a new remote is added (e.g. just to inspect something from someone else) the DWIMery goes away:

    (
        rm -rf /tmp/tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git remote add avar [email protected]:avar/tbdiff.git &&
        git fetch avar &&
        git checkout master
    )

Will output:

error: pathspec 'master' did not match any file(s) known to git.

The new checkout.implicitRemote config allows me to say that whenever that ambiguity comes up I'd like to prefer "origin", and it'll still work as though the only remote I had was "origin".

I considered splitting this into checkout.implicitRemote and worktree.implicitRemote, but it's probably less confusing to break our own rules that anything shared between config should live in core.* than have two config settings, and I couldn't come up with a short name under core.* that made sense (core.implicitRemoteForCheckout?).

See also 70c9ac2f19 ("DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"", 2009-10-18) which introduced this DWIM feature to begin with, and 4e85333197 ("worktree: make add <path> <branch> dwim", 2017-11-26) which added it to git-worktree.

---

Took me a while to get around to this, but this v3 addresses feedback by Eric about the docs. Thanks! interdiff:

1: 905a17f35f ! 1: b23d1b71e9 checkout & worktree: introduce checkout.implicitRemote
@@ -71,16 +71,15 @@
+ tracking e.g. 'origin/<something>'. This stops working as soon
+ as you have more than one remote with a '<something>'
+ reference. This setting allows for setting the name of a
-+ special remote that should always win when it comes to
++ preferred remote that should always win when it comes to
+ disambiguation. The typical use-case is to set this to
+ origin.
++
+Currently this is used by linkgit:git-checkout[1] when 'git checkout +<something>' will checkout the '<something>' branch on another remote,
-+and by linkgit:git-worktree[1] when 'git worktree add' when referring -+to a remote branch. This setting might be used for other -+checkout-like commands or functionality in the future when -+appropriate.
++and by linkgit:git-worktree[1] when 'git worktree add' refers to a ++remote branch. This setting might be used for other checkout-like ++commands or functionality in the future.
+
clean.requireForce::
A boolean to make git-clean do nothing unless given -f,
@@ -110,14 +109,14 @@
$ git worktree add --track -b <branch> <path> <remote>/<branch>
------------
+
-+It's also possible to use the checkout.implicitRemote setting to -+designate a special remote this rule should be applied to, even if the -+branch isn't unique across all remotes. See checkout.implicitRemote -+in linkgit:git-config[1].
++The checkout.implicitRemote setting can be used to to designate a ++preferred <remote> this rule should be applied to, even if the ++<branch> isn't unique across all remotes. See ++checkout.implicitRemote in linkgit:git-config[1].
++
If <commit-ish> is omitted and neither -b nor -B nor --detach used,
- then, as a convenience, a new branch based at HEAD is created automatically,
- as if -b $(basename <path>) was specified.
+ then, as a convenience, the new worktree is associated with a branch
+ (call it <branch>) named after $(basename <path>). If <branch>

diff --git a/builtin/checkout.c b/builtin/checkout.c --- a/builtin/checkout.c
@@ -235,6 +234,6 @@
+ )
+'
+
- post_checkout_hook () {
- gitdir=${1:-.git}
- test_when_finished "rm -f $gitdir/hooks/post-checkout" &&
+ test_expect_success 'git worktree add does not match remote' '
+ test_when_finished rm -rf repo_a repo_b foo &&
+ setup_remote_repo repo_a repo_b &&

 Documentation/config.txt       | 16 ++++++++++++++++
 Documentation/git-checkout.txt |  6 ++++++
 Documentation/git-worktree.txt |  5 +++++
 builtin/checkout.c             |  6 ++++--
 checkout.c                     | 17 +++++++++++++++--
 t/t2024-checkout-dwim.sh       | 10 ++++++++++
 t/t2025-worktree-add.sh        | 18 ++++++++++++++++++
 7 files changed, 74 insertions(+), 4 deletions(-)

index 84e2891aed..7f095a260a 100644
@@ -1098,6 +1098,22 @@ browser.<tool>.path::
 	browse HTML help (see `-w` option in linkgit:git-help[1]) or a
 	working repository in gitweb (see linkgit:git-instaweb[1]).
 
+checkout.implicitRemote::
+	When you run 'git checkout <something>' and only have one
+	remote, it may implicitly fall back on checking out and
+	tracking e.g. 'origin/<something>'. This stops working as soon
+	as you have more than one remote with a '<something>'
+	reference. This setting allows for setting the name of a
+	preferred remote that should always win when it comes to
+	disambiguation. The typical use-case is to set this to
+	`origin`.
++
+Currently this is used by linkgit:git-checkout[1] when 'git checkout
+<something>' will checkout the '<something>' branch on another remote,
+and by linkgit:git-worktree[1] when 'git worktree add' refers to a
+remote branch. This setting might be used for other checkout-like
+commands or functionality in the future.
+
 clean.requireForce::
 	A boolean to make git-clean do nothing unless given -f,
 	-i or -n.   Defaults to true.

index ca5fc9c798..753aa4001f 100644
@@ -38,6 +38,12 @@ equivalent to
 $ git checkout -b <branch> --track <remote>/<branch>
 ------------
 +
+If the branch exists in multiple remotes the `checkout.implicitRemote`
+variable can be used to pick the remote you really mean. Set it to
+e.g. `checkout.implicitRemote=origin` to always checkout remote
+branches from there. See also `checkout.implicitRemote` in
+linkgit:git-config[1].
++
 You could omit <branch>, in which case the command degenerates to
 "check out the current branch", which is a glorified no-op with
 rather expensive side-effects to show only the tracking information,

index afc6576a14..6febb21dca 100644
@@ -60,6 +60,11 @@ with a matching name, treat as equivalent to:
 $ git worktree add --track -b <branch> <path> <remote>/<branch>
 ------------
 +
+The `checkout.implicitRemote` setting can be used to to designate a
+preferred `<remote>` this rule should be applied to, even if the
+`<branch>` isn't unique across all remotes. See
+`checkout.implicitRemote` in linkgit:git-config[1].
++
 If `<commit-ish>` is omitted and neither `-b` nor `-B` nor `--detach` used,
 then, as a convenience, the new worktree is associated with a branch
 (call it `<branch>`) named after `$(basename <path>)`.  If `<branch>`

index 2b3b768eff..88e650f0f8 100644
@@ -909,8 +909,10 @@ static int parse_branchname_arg(int argc, const char **argv,
 	 *   (b) If <something> is _not_ a commit, either "--" is present
 	 *       or <something> is not a path, no -t or -b was given, and
 	 *       and there is a tracking branch whose name is <something>
-	 *       in one and only one remote, then this is a short-hand to
-	 *       fork local <something> from that remote-tracking branch.
+	 *       in one and only one remote (or if the branch exists on the
+	 *       remote named in checkout.implicitRemote), then this is a
+	 *       short-hand to fork local <something> from that
+	 *       remote-tracking branch.
 	 *
 	 *   (c) Otherwise, if "--" is present, treat it like case (1).
 	 *

index ac42630f74..2c898cfe19 100644
@@ -1,12 +1,15 @@
 #include "cache.h"
 #include "remote.h"
 #include "checkout.h"
+#include "config.h"
 
 struct tracking_name_data {
 	/* const */ char *src_ref;
 	char *dst_ref;
 	struct object_id *dst_oid;
 	int unique;
+	const char *implicit_remote;
+	char *implicit_dst_ref;
 };
 
 static int check_tracking_name(struct remote *remote, void *cb_data)

@@ -20,6 +23,8 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 		free(query.dst);
 		return 0;
 	}
+	if (cb->implicit_remote && !strcmp(remote->name, cb->implicit_remote))
+		cb->implicit_dst_ref = xstrdup(query.dst);
 	if (cb->dst_ref) {
 		free(query.dst);
 		cb->unique = 0;

@@ -31,13 +36,21 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 
 const char *unique_tracking_name(const char *name, struct object_id *oid)
 {
-	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	const char *implicit_remote = NULL;
+	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1, NULL, NULL };
+	if (!git_config_get_string_const("checkout.implicitremote", &implicit_remote))
+		cb_data.implicit_remote = implicit_remote;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
 	free(cb_data.src_ref);
-	if (cb_data.unique)
+	free((char *)implicit_remote);
+	if (cb_data.unique) {
+		free(cb_data.implicit_dst_ref);
 		return cb_data.dst_ref;
+	}
 	free(cb_data.dst_ref);
+	if (cb_data.implicit_dst_ref)
+		return cb_data.implicit_dst_ref;
 	return NULL;
 }

index 3e5ac81bd2..da6bd74bbc 100755
@@ -68,6 +68,16 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '
 	test_branch master
 '
 
+test_expect_success 'checkout of branch from multiple remotes succeeds with checkout.implicitRemote #1' '
+	git checkout -B master &&
+	test_might_fail git branch -D foo &&
+
+	git -c checkout.implicitRemote=repo_a checkout foo &&
+	test_branch foo &&
+	test_cmp_rev remotes/repo_a/foo HEAD &&
+	test_branch_upstream foo repo_a foo
+'
+
 test_expect_success 'checkout of branch from a single remote succeeds #1' '
 	git checkout -B master &&
 	test_might_fail git branch -D bar &&

index 2240498924..271a6413f0 100755
@@ -402,6 +402,24 @@ test_expect_success '"add" <path> <branch> dwims' '
 	)
 '
 
+test_expect_success '"add" <path> <branch> dwims with checkout.implicitRemote' '
+	test_when_finished rm -rf repo_upstream repo_dwim foo &&
+	setup_remote_repo repo_upstream repo_dwim &&
+	git init repo_dwim &&
+	(
+		cd repo_dwim &&
+		git remote add repo_upstream2 ../repo_upstream &&
+		git fetch repo_upstream2 &&
+		test_must_fail git worktree add ../foo foo &&
+		git -c checkout.implicitRemote=repo_upstream worktree add ../foo foo
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_upstream foo &&
+		test_cmp_rev refs/remotes/repo_upstream/foo refs/heads/foo
+	)
+'
+
 test_expect_success 'git worktree add does not match remote' '
 	test_when_finished rm -rf repo_a repo_b foo &&
 	setup_remote_repo repo_a repo_b &&

2.17.0.290.gded63e768a

I still plan to revisit per-worktree config support [1] at some point and finish it off. Or is it decided that we don't need a generic mechanism and will add a new level like this for each config var that is per-worktree? If defaultRemote sets a precedence, then it'll be the way to go from now on or we'll have another mess when some config does "foo.$worktree.bar" while others use per-worktree config files.

[1] https://public-inbox.org/git/[email protected]/

Duy

What do you think about including worktree in this at this time? I'm not very familiar with it and just included it because I worked my way backwards from the DWIM function, but I could exclude it if you think it's too much trouble, i.e. if you'd like to hold out for some facility like the per-worktree config Junio mentioned.

I think Junio was talking about the future. What is currently in the patch, both code and document, is fine. But if you're going to implement core.$worktree.defaultRemote at this time, maybe wait a bit. I could try to resurrect the per-worktree config topic in a month or so and if it does not work out, then everybody will add new config vars if they want per-worktree support.

Duy

v4 started as a simple bug-fix for this one-part series, but since it's not going to make 2.18.0 at this point I thought I'd do some more work on it. Comments on patches below:

Ævar Arnfjörð Bjarmason (9):
checkout tests: index should be clean after dwim checkout

Tests that would have revealed the bug in v3.

checkout.h: wrap the arguments to unique_tracking_name() checkout.[ch]: move struct declaration into *.h

Boring moving code around.

checkout.[ch]: introduce an *_INIT macro

Make checkout.h have a TRACKING_NAME_DATA_INIT for its struct.

checkout.[ch]: change "unique" member to "num_matches"
checkout: pass the "num_matches" up to callers builtin/checkout.c: use "ret" variable for return

Refactoring with no changes yet to make subsequent changes smaller.

checkout: add advice for ambiguous "checkout <branch>"

Even if checkout.defaultRemote is off we now print advice telling the user why their "git checkout branch" didn't work.

checkout & worktree: introduce checkout.defaultRemote

It's now called checkout.defaultRemote not checkout.implicitRemote on Junio's suggestion. On reflection that's better.

Improved tests for git-worktree (similar to the dwim checkout tests improvements earlier), and the the documentation for git-checkout & git-worktree.

I'm omitting the tbdiff because most of it's because of the new patches in this series. Better just to read them.

 Documentation/config.txt       | 26 +++++++++++++++
 Documentation/git-checkout.txt |  9 ++++++
 Documentation/git-worktree.txt |  9 ++++++
 advice.c                       |  2 ++
 advice.h                       |  1 +
 builtin/checkout.c             | 44 ++++++++++++++++++++------
 builtin/worktree.c             |  4 +--
 checkout.c                     | 37 +++++++++++++++-------
 checkout.h                     | 16 +++++++++-
 t/t2024-checkout-dwim.sh       | 58 +++++++++++++++++++++++++++++++++-
 t/t2025-worktree-add.sh        | 21 ++++++++++++
 11 files changed, 203 insertions(+), 24 deletions(-)

2.17.0.290.gded63e768a

Assert that whenever there's a DWIM checkout that the index should be clean afterwards, in addition to the correct branch being checked-out.

The way the DWIM checkout code in checkout.[ch] works is by looping over all remotes, and for each remote trying to find if a given reference name only exists on that remote, or if it exists anywhere else.

This is done by starting out with a unique = 1 tracking variable in a struct shared by the entire loop, which will get set to 0 if the data reference is not unique.

Thus if we find a match we know the dst_oid member of tracking_name_data must be correct, since it's associated with the only reference on the only remote that could have matched our query.

But if there was ever a mismatch there for some reason we might end up with the correct branch checked out, but at the wrong oid, which would show whatever the difference between the two staged in the index (checkout branch A, stage changes from the state of branch B).

So let's amend the tests (mostly added in) 399e4a1c56 ("t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'", 2013-04-21) to always assert that "status" is clean after we run "checkout", that's being done with "-uno" because there's going to be some untracked files related to the test itself which we don't care about.

Then if we ever run into this sort of regression, either in the existing code or with a new feature, we'll know.

---
 t/t2024-checkout-dwim.sh | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

index 3e5ac81bd2..29c1eada17 100755
@@ -23,6 +23,12 @@ test_branch_upstream () {
 	test_cmp expect.upstream actual.upstream
 }
 
+status_uno_is_clean() {
+	>status.expect &&
+	git status -uno --porcelain >status.actual &&
+	test_cmp status.expect status.actual
+}
+
 test_expect_success 'setup' '
 	test_commit my_master &&
 	git init repo_a &&

@@ -55,6 +61,7 @@ test_expect_success 'checkout of non-existing branch fails' '
 	test_might_fail git branch -D xyzzy &&
 
 	test_must_fail git checkout xyzzy &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/xyzzy &&
 	test_branch master
 '

@@ -64,8 +71,10 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '
 	test_might_fail git branch -D foo &&
 
 	test_must_fail git checkout foo &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/foo &&
-	test_branch master
+	test_branch master &&
+	status_uno_is_clean
 '
 
 test_expect_success 'checkout of branch from a single remote succeeds #1' '

@@ -73,6 +82,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #1' '
 	test_might_fail git branch -D bar &&
 
 	git checkout bar &&
+	status_uno_is_clean &&
 	test_branch bar &&
 	test_cmp_rev remotes/repo_a/bar HEAD &&
 	test_branch_upstream bar repo_a bar

@@ -83,6 +93,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '
 	test_might_fail git branch -D baz &&
 
 	git checkout baz &&
+	status_uno_is_clean &&
 	test_branch baz &&
 	test_cmp_rev remotes/other_b/baz HEAD &&
 	test_branch_upstream baz repo_b baz

@@ -90,6 +101,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '
 
 test_expect_success '--no-guess suppresses branch auto-vivification' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D bar &&
 
 	test_must_fail git checkout --no-guess bar &&

@@ -99,6 +111,7 @@ test_expect_success '--no-guess suppresses branch auto-vivification' '
 
 test_expect_success 'setup more remotes with unconventional refspecs' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	git init repo_c &&
 	(
 		cd repo_c &&

@@ -128,27 +141,33 @@ test_expect_success 'setup more remotes with unconventional refspecs' '
 
 test_expect_success 'checkout of branch from multiple remotes fails #2' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D bar &&
 
 	test_must_fail git checkout bar &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/bar &&
 	test_branch master
 '
 
 test_expect_success 'checkout of branch from multiple remotes fails #3' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D baz &&
 
 	test_must_fail git checkout baz &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/baz &&
 	test_branch master
 '
 
 test_expect_success 'checkout of branch from a single remote succeeds #3' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	git checkout spam &&
+	status_uno_is_clean &&
 	test_branch spam &&
 	test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD &&
 	test_branch_upstream spam repo_c spam

@@ -156,9 +175,11 @@ test_expect_success 'checkout of branch from a single remote succeeds #3' '
 
 test_expect_success 'checkout of branch from a single remote succeeds #4' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D eggs &&
 
 	git checkout eggs &&
+	status_uno_is_clean &&
 	test_branch eggs &&
 	test_cmp_rev refs/repo_d/eggs HEAD &&
 	test_branch_upstream eggs repo_d eggs

@@ -166,32 +187,38 @@ test_expect_success 'checkout of branch from a single remote succeeds #4' '
 
 test_expect_success 'checkout of branch with a file having the same name fails' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	>spam &&
 	test_must_fail git checkout spam &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/spam &&
 	test_branch master
 '
 
 test_expect_success 'checkout of branch with a file in subdir having the same name fails' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	>spam &&
 	mkdir sub &&
 	mv spam sub/spam &&
 	test_must_fail git -C sub checkout spam &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/spam &&
 	test_branch master
 '
 
 test_expect_success 'checkout <branch> -- succeeds, even if a file with the same name exists' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	>spam &&
 	git checkout spam -- &&
+	status_uno_is_clean &&
 	test_branch spam &&
 	test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD &&
 	test_branch_upstream spam repo_c spam

@@ -200,6 +227,7 @@ test_expect_success 'checkout <branch> -- succeeds, even if a file with the same
 test_expect_success 'loosely defined local base branch is reported correctly' '
 
 	git checkout master &&
+	status_uno_is_clean &&
 	git branch strict &&
 	git branch loose &&
 	git commit --allow-empty -m "a bit more" &&

@@ -210,7 +238,9 @@ test_expect_success 'loosely defined local base branch is reported correctly' '
 	test_config branch.loose.merge master &&
 
 	git checkout strict | sed -e "s/strict/BRANCHNAME/g" >expect &&
+	status_uno_is_clean &&
 	git checkout loose | sed -e "s/loose/BRANCHNAME/g" >actual &&
+	status_uno_is_clean &&
 
 	test_cmp expect actual
 '

2.17.0.290.gded63e768a

It might not be absolutely necessary to state, but it would be helpful to say that you are assuming to start a checkout (DWIM or otherwise) from a clean state; without the assumption, the readers need to think for a few breaths why "the index should be clean" is true.

The intention and the implementation of the change both mostly look good to me from a quick read.

Makes sense, will fix.

Slipped in, will remove. Thanks.

The line was too long already, and will be longer still when a later change adds another argument.

---
 checkout.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 9980711179..4cd4cd1c23 100644
@@ -8,6 +8,7 @@
  * tracking branch.  Return the name of the remote if such a branch
  * exists, NULL otherwise.
  */
-extern const char *unique_tracking_name(const char *name, struct object_id *oid);
+extern const char *unique_tracking_name(const char *name,
+					struct object_id *oid);
 
 #endif /* CHECKOUT_H */

2.17.0.290.gded63e768a

Move the tracking_name_data struct used in checkout.c into its corresponding header file. This wasn't done in 7c85a87c54 ("checkout:
factor out functions to new lib file", 2017-11-26) when checkout.[ch] were created, and is more consistent with the rest of the codebase.

---
 checkout.c | 7 -------
 checkout.h | 7 +++++++
 2 files changed, 7 insertions(+), 7 deletions(-)

index bdefc888ba..8d68f75ad1 100644
@@ -3,13 +3,6 @@
 #include "refspec.h"
 #include "checkout.h"
 
-struct tracking_name_data {
-	/* const */ char *src_ref;
-	char *dst_ref;
-	struct object_id *dst_oid;
-	int unique;
-};
-
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {
 	struct tracking_name_data *cb = cb_data;

index 4cd4cd1c23..04b52f9ffe 100644
@@ -3,6 +3,13 @@
 
 #include "cache.h"
 
+struct tracking_name_data {
+	/* const */ char *src_ref;
+	char *dst_ref;
+	struct object_id *dst_oid;
+	int unique;
+};
+
 /*
  * Check if the branch name uniquely matches a branch name on a remote
  * tracking branch.  Return the name of the remote if such a branch

2.17.0.290.gded63e768a

We seem to have plenty of structs defined in '.c' files, if they are only needed there. Its use also seems to be single purpose for the callback data, so I'm a bit puzzled how having this in a header file instead of the .c file would be helpful?

I feel like having only the "public" part in the header file also helps developers that are just looking for documentation of the functions they are looking at, by having less things to go through, that they wouldn't necessarily care about.

Yup, sounds like a sensible criterion to choose between <*.h> and <*.c>.

Yes this makes sense. Thanks both. I misunderstood the idiom. Makes sense in hindsight (& with both of your advices) only to put structs in the *.h if it's actually used outside of that API. Will move this around in v5.

Add an *_INIT macro for the tracking_name_data similar to what exists elsewhere in the codebase, e.g. OID_ARRAY_INIT in sha1-array.h. This will make it more idiomatic in later changes to add more fields to the struct & its initialization macro.

---
 checkout.c | 2 +-
 checkout.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

index 8d68f75ad1..629fc1d5c4 100644
@@ -25,7 +25,7 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 
 const char *unique_tracking_name(const char *name, struct object_id *oid)
 {
-	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);

index 04b52f9ffe..a61ec93e65 100644
@@ -10,6 +10,8 @@ struct tracking_name_data {
 	int unique;
 };
 
+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 1 }
+
 /*
  * Check if the branch name uniquely matches a branch name on a remote
  * tracking branch.  Return the name of the remote if such a branch

2.17.0.290.gded63e768a

Makes sense; Thomas's comment on 3/9 still stands at this point, as we have no outside users of the definition of the callback data yet at this point.

Internally track how many matches we find in the check_tracking_name() callback. Nothing uses this now, but it will be made use of in a later change.

---
 checkout.c | 4 ++--
 checkout.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

index 629fc1d5c4..7ce5306bc7 100644
@@ -14,9 +14,9 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 		free(query.dst);
 		return 0;
 	}
+	cb->num_matches++;
 	if (cb->dst_ref) {
 		free(query.dst);
-		cb->unique = 0;
 		return 0;
 	}
 	cb->dst_ref = query.dst;

@@ -30,7 +30,7 @@ const char *unique_tracking_name(const char *name, struct object_id *oid)
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
 	free(cb_data.src_ref);
-	if (cb_data.unique)
+	if (cb_data.num_matches == 1)
 		return cb_data.dst_ref;
 	free(cb_data.dst_ref);
 	return NULL;

index a61ec93e65..2decb9b820 100644
@@ -7,10 +7,10 @@ struct tracking_name_data {
 	/* const */ char *src_ref;
 	char *dst_ref;
 	struct object_id *dst_oid;
-	int unique;
+	int num_matches;
 };
 
-#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 1 }
+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0 }
 
 /*
  * Check if the branch name uniquely matches a branch name on a remote

2.17.0.290.gded63e768a

Pass the previously added "num_matches" struct value up to the callers of unique_tracking_name(). This will allow callers to optionally print better error messages in a later change.

---
 builtin/checkout.c | 16 +++++++++++-----
 builtin/worktree.c |  4 ++--
 checkout.c         |  5 ++++-
 checkout.h         |  3 ++-
 4 files changed, 19 insertions(+), 9 deletions(-)

index 2e1d2376d2..ec7cf93b4a 100644
@@ -239,7 +239,8 @@ static int checkout_merged(int pos, const struct checkout *state)
 }
 
 static int checkout_paths(const struct checkout_opts *opts,
-			  const char *revision)
+			  const char *revision,
+			  int *dwim_remotes_matched)
 {
 	int pos;
 	struct checkout state = CHECKOUT_INIT;

@@ -878,7 +879,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 				int dwim_new_local_branch_ok,
 				struct branch_info *new_branch_info,
 				struct checkout_opts *opts,
-				struct object_id *rev)
+				struct object_id *rev,
+				int *dwim_remotes_matched)
 {
 	struct tree **source_tree = &opts->source_tree;
 	const char **new_branch = &opts->new_branch;

@@ -972,7 +974,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 			recover_with_dwim = 0;
 
 		if (recover_with_dwim) {
-			const char *remote = unique_tracking_name(arg, rev);
+			const char *remote = unique_tracking_name(arg, rev,
+								  dwim_remotes_matched);
 			if (remote) {
 				*new_branch = arg;
 				arg = remote;

@@ -1109,6 +1112,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct branch_info new_branch_info;
 	char *conflict_style = NULL;
 	int dwim_new_local_branch = 1;
+	int dwim_remotes_matched = 0;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
 		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),

@@ -1219,7 +1223,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			opts.track == BRANCH_TRACK_UNSPECIFIED &&
 			!opts.new_branch;
 		int n = parse_branchname_arg(argc, argv, dwim_ok,
-					     &new_branch_info, &opts, &rev);
+					     &new_branch_info, &opts, &rev,
+					     &dwim_remotes_matched);
 		argv += n;
 		argc -= n;
 	}

@@ -1262,7 +1267,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 
 	UNLEAK(opts);
 	if (opts.patch_mode || opts.pathspec.nr)
-		return checkout_paths(&opts, new_branch_info.name);
+		return checkout_paths(&opts, new_branch_info.name,
+				      &dwim_remotes_matched);
 	else
 		return checkout_branch(&opts, &new_branch_info);
 }

index 5c7d2bb180..a763dbdccb 100644
@@ -412,7 +412,7 @@ static const char *dwim_branch(const char *path, const char **new_branch)
 	if (guess_remote) {
 		struct object_id oid;
 		const char *remote =
-			unique_tracking_name(*new_branch, &oid);
+			unique_tracking_name(*new_branch, &oid, NULL);
 		return remote;
 	}
 	return NULL;

@@ -484,7 +484,7 @@ static int add(int ac, const char **av, const char *prefix)
 
 		commit = lookup_commit_reference_by_name(branch);
 		if (!commit) {
-			remote = unique_tracking_name(branch, &oid);
+			remote = unique_tracking_name(branch, &oid, NULL);
 			if (remote) {
 				new_branch = branch;
 				branch = remote;

index 7ce5306bc7..c578782baa 100644
@@ -23,12 +23,15 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 	return 0;
 }
 
-const char *unique_tracking_name(const char *name, struct object_id *oid)
+const char *unique_tracking_name(const char *name, struct object_id *oid,
+				 int *dwim_remotes_matched)
 {
 	struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
+	if (dwim_remotes_matched)
+		*dwim_remotes_matched = cb_data.num_matches;
 	free(cb_data.src_ref);
 	if (cb_data.num_matches == 1)
 		return cb_data.dst_ref;

index 2decb9b820..4e518c801a 100644
@@ -18,6 +18,7 @@ struct tracking_name_data {
  * exists, NULL otherwise.
  */
 extern const char *unique_tracking_name(const char *name,
-					struct object_id *oid);
+					struct object_id *oid,
+					int *dwim_remotes_matched);
 
 #endif /* CHECKOUT_H */

2.17.0.290.gded63e768a

There is no point in doing this right now, but in later change the "ret" variable will be inspected. This change makes that meaningful change smaller.

---
 builtin/checkout.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

index ec7cf93b4a..423e056acd 100644
@@ -1266,9 +1266,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	}
 
 	UNLEAK(opts);
-	if (opts.patch_mode || opts.pathspec.nr)
-		return checkout_paths(&opts, new_branch_info.name,
-				      &dwim_remotes_matched);
-	else
+	if (opts.patch_mode || opts.pathspec.nr) {
+		int ret = checkout_paths(&opts, new_branch_info.name,
+					 &dwim_remotes_matched);
+		return ret;
+	} else {
 		return checkout_branch(&opts, &new_branch_info);
+	}
 }

2.17.0.290.gded63e768a

As the "checkout" documentation describes:

If <branch> is not found but there does exist a tracking branch in exactly one remote (call it <remote>) with a matching name, treat as equivalent to [...] <remote>/<branch.

This is a really useful feature, the problem is that when you another remote (e.g. a fork) git won't find a unique branch name anymore, and will instead print this nondescript message:

    $ git checkout master
    error: pathspec 'master' did not match any file(s) known to git

Now it will, on my git.git checkout, print:

    $ ./git --exec-path=$PWD checkout master
    error: pathspec 'master' did not match any file(s) known to git.
    hint: The argument 'master' matched more than one remote tracking branch.
    hint: We found 26 remotes with a reference that matched. So we fell back
    hint: on trying to resolve the argument as a path, but failed there too!
    hint:
    hint: Perhaps you meant fully qualify the branch name? E.g. origin/<name>
    hint: instead of <name>?

---
 Documentation/config.txt |  7 +++++++
 advice.c                 |  2 ++
 advice.h                 |  1 +
 builtin/checkout.c       | 11 +++++++++++
 t/t2024-checkout-dwim.sh | 14 ++++++++++++++
 5 files changed, 35 insertions(+)

index 7d8383433c..08d3e70989 100644
@@ -344,6 +344,13 @@ advice.*::
 		Advice shown when you used linkgit:git-checkout[1] to
 		move to the detach HEAD state, to instruct how to create
 		a local branch after the fact.
+	checkoutAmbiguousRemoteBranchName::
+		Advice shown when the argument to
+		linkgit:git-checkout[1] ambiguously resolves to a
+		remote tracking branch on more than one remote in
+		situations where an unambiguous argument would have
+		otherwise caused a remote-tracking branch to be
+		checked out.
 	amWorkDir::
 		Advice that shows the location of the patch file when
 		linkgit:git-am[1] fails to apply it.

index 370a56d054..75e7dede90 100644
@@ -21,6 +21,7 @@ int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
 int advice_graft_file_deprecated = 1;
+int advice_checkout_ambiguous_remote_branch_name = 1;
 
 static int advice_use_color = -1;
 static char advice_colors[][COLOR_MAXLEN] = {

@@ -72,6 +73,7 @@ static struct {
 	{ "ignoredhook", &advice_ignored_hook },
 	{ "waitingforeditor", &advice_waiting_for_editor },
 	{ "graftfiledeprecated", &advice_graft_file_deprecated },
+	{ "checkoutambiguousremotebranchname", &advice_checkout_ambiguous_remote_branch_name },
 
 	/* make this an alias for backward compatibility */
 	{ "pushnonfastforward", &advice_push_update_rejected }

index 9f5064e82a..4d11d51d43 100644
@@ -22,6 +22,7 @@ extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
 extern int advice_graft_file_deprecated;
+extern int advice_checkout_ambiguous_remote_branch_name;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))

index 423e056acd..710369a60a 100644
@@ -22,6 +22,7 @@
 #include "resolve-undo.h"
 #include "submodule-config.h"
 #include "submodule.h"
+#include "advice.h"
 
 static const char * const checkout_usage[] = {
 	N_("git checkout [<options>] <branch>"),

@@ -1269,6 +1270,16 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	if (opts.patch_mode || opts.pathspec.nr) {
 		int ret = checkout_paths(&opts, new_branch_info.name,
 					 &dwim_remotes_matched);
+		if (ret && dwim_remotes_matched > 1 &&
+		    advice_checkout_ambiguous_remote_branch_name)
+			advise(_("The argument '%s' matched more than one remote tracking branch.\n"
+				 "We found %d remotes with a reference that matched. So we fell back\n"
+				 "on trying to resolve the argument as a path, but failed there too!\n"
+				 "\n"
+				 "Perhaps you meant fully qualify the branch name? E.g. origin/<name>\n"
+				 "instead of <name>?"),
+			       argv[0],
+			       dwim_remotes_matched);
 		return ret;
 	} else {
 		return checkout_branch(&opts, &new_branch_info);

index 29c1eada17..14735f5bb8 100755
@@ -77,6 +77,20 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '
 	status_uno_is_clean
 '
 
+test_expect_success 'checkout of branch from multiple remotes fails with advice' '
+	git checkout -B master &&
+	test_might_fail git branch -D foo &&
+	test_must_fail git checkout foo 2>stderr &&
+	test_branch master &&
+	status_uno_is_clean &&
+	test_i18ngrep "^hint: " stderr &&
+	test_must_fail git -c advice.checkoutAmbiguousRemoteBranchName=false \
+		checkout foo 2>stderr &&
+	test_branch master &&
+	status_uno_is_clean &&
+	test_i18ngrep ! "^hint: " stderr
+'
+
 test_expect_success 'checkout of branch from a single remote succeeds #1' '
 	git checkout -B master &&
 	test_might_fail git branch -D bar &&

2.17.0.290.gded63e768a

Well, upon closer inspection adding *dwim_remotes_matched parameter to checkout_paths() done in an earlier step seems to be totally bogus and only serves the purpose of confusing reviewers. The function does not touch the pointer in any way---it does not use the pointer to return its findings, and it does not use an earlier findings to affect its behaviour by dereferencing it.

The dwim_remotes_matched is set by an earlier call to parse_branchname_arg(), which does gain an int* parameter in this series. And that addition _does_ make sense. That codepath is where the "do we have many remotes that could match, or none, or unique?" determination is made.

Yes, sorry. I'll fix that. This was a relic from an earlier version of this that never escaped into the wild where I first tried printing out this error printing it out near the report_path_error() codepath called from checkout_paths().

I.e. I was trying to avoid printing out the "error: pathspec 'master' did not match any file(s) known to git." error altogether. That's still arguably a good direction, since we *know* "master" would have otherwise matched a remote branch, so that's probably a more informative message than falling back to checking out pathspecs and failing, and complaining about there being no such pathspec.

But it was a pain to handle the various edge cases, e.g.:

    $ ./git --exec-path=$PWD checkout x y z
    error: pathspec 'x' did not match any file(s) known to git.
    error: pathspec 'y' did not match any file(s) known to git.
    error: pathspec 'z' did not match any file(s) known to git.

So I decided just to let checkout_paths() to its thing and then print out an error about dwim branches if applicable if it failed.

*nod*

That ideal behaviour makes sense.

Let's take a detour to a tangent, as this example does not have anything to do with the remote-tracking auto-dwimming.

Ideally what do we want to say in this case? What's allowed for 'x' (2 possibilities) is different from whats allowed for 'y' and 'z' (only 1 possibility)---do we want to complain that 'x' is not a rev noris a file (we do not say 'x' could be a misspelt rev name right now), and then 'y' and 'z' are not files (which is what we do now)?

That might be an improvement. I dunno. In any case, that is a tangent that we do not have to address with these patches.

In contrast, the command line without y and z gives three possibilities to 'x'. 'x' is not a rev, is not a remote-tracking branch name that only a single remote has, and is not a file. Now, if we are going to mention that we failed to interpret it as the latter two, perhaps we should also mention that it was not a rev (which could have been misspelt)?

Yeah, I think I get it now. If you want to silence the "error" from report_path_error() and replace it with something else, you would need to change checout_paths(), as this function is sort-of used as the last ditch effort after all else failed, and right now it is not aware of what exactly all these other failed efforts were.

Thanks. I'm looking at v6 reroll for queuing.

No:

    $ ./git --exec-path=$PWD checkout -p master
    No changes.
    $ ./git --exec-path=$PWD checkout master
    error: pathspec 'master' did not match any file(s) known to git.
    hint: The argument 'master' matched more than one remote tracking branch.
    [...]

I skimmed the rest of this thread and saw there were more specific suggestions. I'm open to moving this somewhere else. The "what is this and what do we do" logic in checkout.c is spread over multiple functions, and this seemed the most straightforward way to add this.

s/, the/. The/
s/you/& add/

s/meant/& to/

Thanks!

Will rephrase to make this less confusing.

Introduce a checkout.defaultRemote setting which can be used to designate a remote to prefer (via checkout.defaultRemote=origin) when running e.g. "git checkout master" to mean origin/master, even though there's other remotes that have the "master" branch.

I want this because it's very handy to use this workflow to checkout a repository and create a topic branch, then get back to a "master" as retrieved from upstream:

    (
        cd /tmp &&
        rm -rf tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git checkout master
    )

That will output:

Branch 'master' set up to track remote branch 'master' from 'origin'. Switched to a new branch 'master'

But as soon as a new remote is added (e.g. just to inspect something from someone else) the DWIMery goes away:

    (
        cd /tmp &&
        rm -rf tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git remote add avar [email protected]:avar/tbdiff.git &&
        git fetch avar &&
        git checkout master
    )

Will output (without the advice output added earlier in this series):

error: pathspec 'master' did not match any file(s) known to git.

The new checkout.defaultRemote config allows me to say that whenever that ambiguity comes up I'd like to prefer "origin", and it'll still work as though the only remote I had was "origin".

Also adjust the advice.checkoutAmbiguousRemoteBranchName message to mention this new config setting to the user, the full output on my git.git is now (the last paragraph is new):

    $ ./git --exec-path=$PWD checkout master
    error: pathspec 'master' did not match any file(s) known to git.
    hint: The argument 'master' matched more than one remote tracking branch.
    hint: We found 26 remotes with a reference that matched. So we fell back
    hint: on trying to resolve the argument as a path, but failed there too!
    hint:
    hint: Perhaps you meant fully qualify the branch name? E.g. origin/<name>
    hint: instead of <name>?
    hint:
    hint: If you'd like to always have checkouts of 'master' prefer one remote,
    hint: e.g. the 'origin' remote, consider setting checkout.defaultRemote=origin
    hint: in your config. See the 'git-config' manual page for details.

I considered splitting this into checkout.defaultRemote and worktree.defaultRemote, but it's probably less confusing to break our own rules that anything shared between config should live in core.* than have two config settings, and I couldn't come up with a short name under core.* that made sense (core.defaultRemoteForCheckout?).

See also 70c9ac2f19 ("DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"", 2009-10-18) which introduced this DWIM feature to begin with, and 4e85333197 ("worktree: make add <path> <branch> dwim", 2017-11-26) which added it to git-worktree.

---
 Documentation/config.txt       | 21 ++++++++++++++++++++-
 Documentation/git-checkout.txt |  9 +++++++++
 Documentation/git-worktree.txt |  9 +++++++++
 builtin/checkout.c             | 15 +++++++++++----
 checkout.c                     | 21 ++++++++++++++++++++-
 checkout.h                     |  5 ++++-
 t/t2024-checkout-dwim.sh       | 12 ++++++++++++
 t/t2025-worktree-add.sh        | 21 +++++++++++++++++++++
 8 files changed, 106 insertions(+), 7 deletions(-)

index 08d3e70989..e0d92217ac 100644
@@ -350,7 +350,10 @@ advice.*::
 		remote tracking branch on more than one remote in
 		situations where an unambiguous argument would have
 		otherwise caused a remote-tracking branch to be
-		checked out.
+		checked out. See the `checkout.defaultRemote`
+		configuration variable for how to set a given remote
+		to used by default in some situations where this
+		advice would be printed.
 	amWorkDir::
 		Advice that shows the location of the patch file when
 		linkgit:git-am[1] fails to apply it.

@@ -1105,6 +1108,22 @@ browser.<tool>.path::
 	browse HTML help (see `-w` option in linkgit:git-help[1]) or a
 	working repository in gitweb (see linkgit:git-instaweb[1]).
 
+checkout.defaultRemote::
+	When you run 'git checkout <something>' and only have one
+	remote, it may implicitly fall back on checking out and
+	tracking e.g. 'origin/<something>'. This stops working as soon
+	as you have more than one remote with a '<something>'
+	reference. This setting allows for setting the name of a
+	preferred remote that should always win when it comes to
+	disambiguation. The typical use-case is to set this to
+	`origin`.
++
+Currently this is used by linkgit:git-checkout[1] when 'git checkout
+<something>' will checkout the '<something>' branch on another remote,
+and by linkgit:git-worktree[1] when 'git worktree add' refers to a
+remote branch. This setting might be used for other checkout-like
+commands or functionality in the future.
+
 clean.requireForce::
 	A boolean to make git-clean do nothing unless given -f,
 	-i or -n.   Defaults to true.

index ca5fc9c798..8cb77bddeb 100644
@@ -38,6 +38,15 @@ equivalent to
 $ git checkout -b <branch> --track <remote>/<branch>
 ------------
 +
+If the branch exists in multiple remotes and one of them is named by
+the `checkout.defaultRemote` configuration variable, we'll use that
+one for the purposes of disambiguation, even if the `<branch>` isn't
+unique across all remotes. Set it to
+e.g. `checkout.defaultRemote=origin` to always checkout remote
+branches from there if `<branch> is ambiguous but exists on the
+'origin' remote. See also `checkout.defaultRemote` in
+linkgit:git-config[1].
++
 You could omit <branch>, in which case the command degenerates to
 "check out the current branch", which is a glorified no-op with
 rather expensive side-effects to show only the tracking information,

index afc6576a14..6638d5ca3d 100644
@@ -60,6 +60,15 @@ with a matching name, treat as equivalent to:
 $ git worktree add --track -b <branch> <path> <remote>/<branch>
 ------------
 +
+If the branch exists in multiple remotes and one of them is named by
+the `checkout.defaultRemote` configuration variable, we'll use that
+one for the purposes of disambiguation, even if the `<branch>` isn't
+unique across all remotes. Set it to
+e.g. `checkout.defaultRemote=origin` to always checkout remote
+branches from there if `<branch> is ambiguous but exists on the
+'origin' remote. See also `checkout.defaultRemote` in
+linkgit:git-config[1].
++
 If `<commit-ish>` is omitted and neither `-b` nor `-B` nor `--detach` used,
 then, as a convenience, the new worktree is associated with a branch
 (call it `<branch>`) named after `$(basename <path>)`.  If `<branch>`

index 710369a60a..78bbead000 100644
@@ -913,8 +913,10 @@ static int parse_branchname_arg(int argc, const char **argv,
 	 *   (b) If <something> is _not_ a commit, either "--" is present
 	 *       or <something> is not a path, no -t or -b was given, and
 	 *       and there is a tracking branch whose name is <something>
-	 *       in one and only one remote, then this is a short-hand to
-	 *       fork local <something> from that remote-tracking branch.
+	 *       in one and only one remote (or if the branch exists on the
+	 *       remote named in checkout.defaultRemote), then this is a
+	 *       short-hand to fork local <something> from that
+	 *       remote-tracking branch.
 	 *
 	 *   (c) Otherwise, if "--" is present, treat it like case (1).
 	 *

@@ -1277,9 +1279,14 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 				 "on trying to resolve the argument as a path, but failed there too!\n"
 				 "\n"
 				 "Perhaps you meant fully qualify the branch name? E.g. origin/<name>\n"
-				 "instead of <name>?"),
+				 "instead of <name>?\n"
+				 "\n"
+				 "If you'd like to always have checkouts of '%s' prefer one remote,\n"
+				 "e.g. the 'origin' remote, consider setting checkout.defaultRemote=origin\n"
+				 "in your config. See the 'git-config' manual page for details."),
 			       argv[0],
-			       dwim_remotes_matched);
+			       dwim_remotes_matched,
+			       argv[0]);
 		return ret;
 	} else {
 		return checkout_branch(&opts, &new_branch_info);

index c578782baa..e6cfe6d2ab 100644
@@ -2,6 +2,7 @@
 #include "remote.h"
 #include "refspec.h"
 #include "checkout.h"
+#include "config.h"
 
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {

@@ -15,6 +16,12 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 		return 0;
 	}
 	cb->num_matches++;
+	if (cb->default_remote && !strcmp(remote->name, cb->default_remote)) {
+		struct object_id *dst = xmalloc(sizeof(*cb->default_dst_oid));
+		cb->default_dst_ref = xstrdup(query.dst);
+		oidcpy(dst, cb->dst_oid);
+		cb->default_dst_oid = dst;
+	}
 	if (cb->dst_ref) {
 		free(query.dst);
 		return 0;

@@ -27,14 +34,26 @@ const char *unique_tracking_name(const char *name, struct object_id *oid,
 				 int *dwim_remotes_matched)
 {
 	struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
+	const char *default_remote = NULL;
+	if (!git_config_get_string_const("checkout.defaultremote", &default_remote))
+		cb_data.default_remote = default_remote;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
 	if (dwim_remotes_matched)
 		*dwim_remotes_matched = cb_data.num_matches;
 	free(cb_data.src_ref);
-	if (cb_data.num_matches == 1)
+	free((char *)default_remote);
+	if (cb_data.num_matches == 1) {
+		free(cb_data.default_dst_ref);
+		free(cb_data.default_dst_oid);
 		return cb_data.dst_ref;
+	}
 	free(cb_data.dst_ref);
+	if (cb_data.default_dst_ref) {
+		oidcpy(oid, cb_data.default_dst_oid);
+		free(cb_data.default_dst_oid);
+		return cb_data.default_dst_ref;
+	}
 	return NULL;
 }

index 4e518c801a..e9f1372f32 100644
@@ -8,9 +8,12 @@ struct tracking_name_data {
 	char *dst_ref;
 	struct object_id *dst_oid;
 	int num_matches;
+	const char *default_remote;
+	char *default_dst_ref;
+	struct object_id *default_dst_oid;
 };
 
-#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0 }
+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0, NULL, NULL, NULL }
 
 /*
  * Check if the branch name uniquely matches a branch name on a remote

index 14735f5bb8..ccd96ba85c 100755
@@ -91,6 +91,18 @@ test_expect_success 'checkout of branch from multiple remotes fails with advice'
 	test_i18ngrep ! "^hint: " stderr
 '
 
+test_expect_success 'checkout of branch from multiple remotes succeeds with checkout.defaultRemote #1' '
+	git checkout -B master &&
+	status_uno_is_clean &&
+	test_might_fail git branch -D foo &&
+
+	git -c checkout.defaultRemote=repo_a checkout foo &&
+	status_uno_is_clean &&
+	test_branch foo &&
+	test_cmp_rev remotes/repo_a/foo HEAD &&
+	test_branch_upstream foo repo_a foo
+'
+
 test_expect_success 'checkout of branch from a single remote succeeds #1' '
 	git checkout -B master &&
 	test_might_fail git branch -D bar &&

index d2e49f7632..be6e093142 100755
@@ -402,6 +402,27 @@ test_expect_success '"add" <path> <branch> dwims' '
 	)
 '
 
+test_expect_success '"add" <path> <branch> dwims with checkout.defaultRemote' '
+	test_when_finished rm -rf repo_upstream repo_dwim foo &&
+	setup_remote_repo repo_upstream repo_dwim &&
+	git init repo_dwim &&
+	(
+		cd repo_dwim &&
+		git remote add repo_upstream2 ../repo_upstream &&
+		git fetch repo_upstream2 &&
+		test_must_fail git worktree add ../foo foo &&
+		git -c checkout.defaultRemote=repo_upstream worktree add ../foo foo &&
+		>status.expect &&
+		git status -uno --porcelain >status.actual &&
+		test_cmp status.expect status.actual
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_upstream foo &&
+		test_cmp_rev refs/remotes/repo_upstream/foo refs/heads/foo
+	)
+'
+
 test_expect_success 'git worktree add does not match remote' '
 	test_when_finished rm -rf repo_a repo_b foo &&
 	setup_remote_repo repo_a repo_b &&

2.17.0.290.gded63e768a

Hi Ævar,

Sorry for chiming in late. I have a couple of thoughts:

I thought master is already there after the clone operation and you'd merely switch back to the local branch that was created at clone time?

    $ git clone [email protected]:trast/tbdiff.git && cd tbdiff
    $ git branch
    * master
    $ cat .git/config
...
[branch "master"]
    remote = origin
    merge = refs/heads/master

But the observation is right, we get that message. When do we do the setup for the master branch specifically?

s/meant fully/meant to fully/
s/? E.g./?\nFor example/

In builtin/submodule--helper.c there is get_default_remote() which also hardcodes "origin". I think that is a safe thing to do.

his new setting elevates one remote over all others, which may be enough for most setups and not confusing, too. Consider the following:

git clone https://kernel.googlesource.com/pub/scm/git/git && cd git git remote add gitster https://github.com/gitster/git
git remote add interesting-patches https://github.com/avar/git
git remote add my-github https://github.com/stefanbeller/git

git checkout master

This probably means I want to have origin/master (from kernel.org)

git checkout ab/checkout-implicit-remote

This probably wants to have it from gitster/ (as it is not found on kernel.org); I am not sure if it would want to look at interesting-patches/ that mirrors github, but probably if it were not to be found at gitster.

So maybe we rather want a setup to give a defined priority for the search order:

git config dwim.remoteSearchOrder origin gitster avar

Stepping back a bit, there is already an order in the config file for the remotes, and that order is used for example for 'fetch --all'.

I wonder if we want to take that order? (Or are the days of hand editing the config over and this is too arcane? We would need a config command to re order remotes). Then we could just have a boolean switch to use the config order on ambiguity. Although you might want to have a different order for fetching and looking for the right checkout.

core.dwimRemote ? It's a bit cryptic, though.

Stefan

This option started out as 'core.dwimRemote' in the very first version of this series[1], but someone argued against it for several reasons and suggested 'defaultRemote' instead[2].

[1]: https://public-inbox.org/git/[email protected]/
[2]: https://public-inbox.org/git/CAPig+cTZyYC-1_TxL2PrfOF6HAktUxxM+g5EXcByS5fCDMdCHg@mail.gmail.com/

What you're missing is this part:

git branch -m topic

I.e. we clone the repo, and have a "master" branch, we then rename "master" to "topic", now there's no local master branch. Then we checkout master either with only one remote or two.

Thanks, will fix.

I thought about this use-case, and if we want this in the future I think the most straightforward way is not to invent some new search order variable, but just make use of git config allowing multi-values, i.e.:

[checkout]
defaultRemote = origin
defaultRemote = gitster

Although I'm not interested in implementing that now, and unlike just having one special remote I don't think it's of interest to the vast majority of git users.

Covered by Eric's reply in <CAPig+cSk9Dt3ZLQRjWwpxqMyP3npu3KbEQxkNfjV5RxRtro82Q@mail.gmail.com>

I agree that splitting this into two variables would be needlessly confusing. 'checkout' and 'worktree add' are similar enough in spirit, that users only setting one of the configuration variables would end up confused at some point. Because the commands are so similar, I also feel like it would be okay to break our own rules here, and use the 'core.defaultRemote' name you suggested (I also can't come up with anything better in core.* right now).

I do think "checkout" in name is grately helpful. I do not see why it is a bad idea for the worktree codepath to pay attention to the checkout.defaultRemote configuration variable, especially when those who are discussing this thread agree "checkout" and "worktree add" are quite similar in end-users' minds.

This v5 should address all the comments to v4. Thanks all! It's one patch less because the struct isn't being moved around anymore.

tbdiff:

    1: 16d656ee3b ! 1: ab4529d9f5 checkout tests: index should be clean after dwim checkout
        @@ -29,6 +29,10 @@
             "checkout", that's being done with "-uno" because there's going to be
             some untracked files related to the test itself which we don't care
             about.
        +    
        +    In all these tests (DWIM or otherwise) we start with a clean index, so
        +    these tests are asserting that that's still the case after the
        +    "checkout", failed or otherwise.
             
             Then if we ever run into this sort of regression, either in the
             existing code or with a new feature, we'll know.
        @@ -65,12 +69,8 @@
          	test_must_fail git checkout foo &&
         +	status_uno_is_clean &&
          	test_must_fail git rev-parse --verify refs/heads/foo &&
        --	test_branch master
        -+	test_branch master &&
        -+	status_uno_is_clean
        - '
        - 
        - test_expect_success 'checkout of branch from a single remote succeeds #1' '
        + 	test_branch master
        + '
         @@
          	test_might_fail git branch -D bar &&
          
    2: 159cc0634b = 2: c8bbece403 checkout.h: wrap the arguments to unique_tracking_name()
    3: 3df4594e2d < -:  ------- checkout.[ch]: move struct declaration into *.h
    4: 35c6481208 < -:  ------- checkout.[ch]: introduce an *_INIT macro
    -:  ------- > 3: 4fc5ab27fa checkout.[ch]: introduce an *_INIT macro
    5: 69a834f010 ! 4: fbce6df584 checkout.[ch]: change "unique" member to "num_matches"
        @@ -11,6 +11,19 @@
         diff --git a/checkout.c b/checkout.c
         --- a/checkout.c
         +++ b/checkout.c
        +@@
        + 	/* const */ char *src_ref;
        + 	char *dst_ref;
        + 	struct object_id *dst_oid;
        +-	int unique;
        ++	int num_matches;
        + };
        + 
        +-#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 1 }
        ++#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0 }
        + 
        + static int check_tracking_name(struct remote *remote, void *cb_data)
        + {
         @@
          		free(query.dst);
          		return 0;
        @@ -31,20 +44,3 @@
          		return cb_data.dst_ref;
          	free(cb_data.dst_ref);
          	return NULL;
        -
        -diff --git a/checkout.h b/checkout.h
        ---- a/checkout.h
        -+++ b/checkout.h
        -@@
        - 	/* const */ char *src_ref;
        - 	char *dst_ref;
        - 	struct object_id *dst_oid;
        --	int unique;
        -+	int num_matches;
        - };
        - 
        --#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 1 }
        -+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0 }
        - 
        - /*
        -  * Check if the branch name uniquely matches a branch name on a remote
    6: 13547824dc ! 5: 6e016d43d7 checkout: pass the "num_matches" up to callers
        @@ -11,16 +11,6 @@
         diff --git a/builtin/checkout.c b/builtin/checkout.c
         --- a/builtin/checkout.c
         +++ b/builtin/checkout.c
        -@@
        - }
        - 
        - static int checkout_paths(const struct checkout_opts *opts,
        --			  const char *revision)
        -+			  const char *revision,
        -+			  int *dwim_remotes_matched)
        - {
        - 	int pos;
        - 	struct checkout state = CHECKOUT_INIT;
         @@
          				int dwim_new_local_branch_ok,
          				struct branch_info *new_branch_info,
        @@ -59,16 +49,6 @@
          		argv += n;
          		argc -= n;
          	}
        -@@
        - 
        - 	UNLEAK(opts);
        - 	if (opts.patch_mode || opts.pathspec.nr)
        --		return checkout_paths(&opts, new_branch_info.name);
        -+		return checkout_paths(&opts, new_branch_info.name,
        -+				      &dwim_remotes_matched);
        - 	else
        - 		return checkout_branch(&opts, &new_branch_info);
        - }
         
         diff --git a/builtin/worktree.c b/builtin/worktree.c
         --- a/builtin/worktree.c
    7: 6895b5c903 ! 6: 07b11b133d builtin/checkout.c: use "ret" variable for return
        @@ -16,12 +16,10 @@
          
          	UNLEAK(opts);
         -	if (opts.patch_mode || opts.pathspec.nr)
        --		return checkout_paths(&opts, new_branch_info.name,
        --				      &dwim_remotes_matched);
        +-		return checkout_paths(&opts, new_branch_info.name);
         -	else
         +	if (opts.patch_mode || opts.pathspec.nr) {
        -+		int ret = checkout_paths(&opts, new_branch_info.name,
        -+					 &dwim_remotes_matched);
        ++		int ret = checkout_paths(&opts, new_branch_info.name);
         +		return ret;
         +	} else {
          		return checkout_branch(&opts, &new_branch_info);
    8: 5cfc0896e5 ! 7: 97e84f6e1c checkout: add advice for ambiguous "checkout <branch>"
        @@ -8,9 +8,9 @@
                 exactly one remote (call it <remote>) with a matching name, treat
                 as equivalent to [...] <remote>/<branch.
             
        -    This is a really useful feature, the problem is that when you another
        -    remote (e.g. a fork) git won't find a unique branch name anymore, and
        -    will instead print this nondescript message:
        +    This is a really useful feature. The problem is that when you and
        +    another remote (e.g. a fork) git won't find a unique branch name
        +    anymore, and will instead print this nondescript message:
             
                 $ git checkout master
                 error: pathspec 'master' did not match any file(s) known to git
        @@ -23,8 +23,10 @@
                 hint: We found 26 remotes with a reference that matched. So we fell back
                 hint: on trying to resolve the argument as a path, but failed there too!
                 hint:
        -        hint: Perhaps you meant fully qualify the branch name? E.g. origin/<name>
        -        hint: instead of <name>?
        +        hint: If you meant to check out a remote tracking branch on e.g. 'origin'
        +        hint: you can do so by fully-qualifying the name with the --track option:
        +        hint:
        +        hint:     git checkout --track origin/<name>
             
             Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]>
         
        @@ -90,17 +92,19 @@
          static const char * const checkout_usage[] = {
          	N_("git checkout [<options>] <branch>"),
         @@
        + 	UNLEAK(opts);
          	if (opts.patch_mode || opts.pathspec.nr) {
        - 		int ret = checkout_paths(&opts, new_branch_info.name,
        - 					 &dwim_remotes_matched);
        + 		int ret = checkout_paths(&opts, new_branch_info.name);
         +		if (ret && dwim_remotes_matched > 1 &&
         +		    advice_checkout_ambiguous_remote_branch_name)
         +			advise(_("The argument '%s' matched more than one remote tracking branch.\n"
         +				 "We found %d remotes with a reference that matched. So we fell back\n"
         +				 "on trying to resolve the argument as a path, but failed there too!\n"
         +				 "\n"
        -+				 "Perhaps you meant fully qualify the branch name? E.g. origin/<name>\n"
        -+				 "instead of <name>?"),
        ++				 "If you meant to check out a remote tracking branch on e.g. 'origin'\n"
        ++				 "you can do so by fully-qualifying the name with the --track option:\n"
        ++				 "\n"
        ++				 "    git checkout --track origin/<name>"),
         +			       argv[0],
         +			       dwim_remotes_matched);
          		return ret;
        @@ -111,7 +115,7 @@
         --- a/t/t2024-checkout-dwim.sh
         +++ b/t/t2024-checkout-dwim.sh
         @@
        - 	status_uno_is_clean
        + 	test_branch master
          '
          
         +test_expect_success 'checkout of branch from multiple remotes fails with advice' '
    9: fad1df1edd ! 8: a5cc070ebf checkout & worktree: introduce checkout.defaultRemote
        @@ -57,12 +57,14 @@
                 hint: We found 26 remotes with a reference that matched. So we fell back
                 hint: on trying to resolve the argument as a path, but failed there too!
                 hint:
        -        hint: Perhaps you meant fully qualify the branch name? E.g. origin/<name>
        -        hint: instead of <name>?
        +        hint: If you meant to check out a remote tracking branch on e.g. 'origin'
        +        hint: you can do so by fully-qualifying the name with the --track option:
                 hint:
        -        hint: If you'd like to always have checkouts of 'master' prefer one remote,
        -        hint: e.g. the 'origin' remote, consider setting checkout.defaultRemote=origin
        -        hint: in your config. See the 'git-config' manual page for details.
        +        hint:     git checkout --track origin/<name>
        +        hint:
        +        hint: If you'd like to always have checkouts of an ambiguous <name> prefer
        +        hint: one remote, e.g. the 'origin' remote, consider setting
        +        hint: checkout.defaultRemote=origin in your config.
             
             I considered splitting this into checkout.defaultRemote and
             worktree.defaultRemote, but it's probably less confusing to break our
        @@ -128,7 +130,7 @@
         +one for the purposes of disambiguation, even if the `<branch>` isn't
         +unique across all remotes. Set it to
         +e.g. `checkout.defaultRemote=origin` to always checkout remote
        -+branches from there if `<branch> is ambiguous but exists on the
        ++branches from there if `<branch>` is ambiguous but exists on the
         +'origin' remote. See also `checkout.defaultRemote` in
         +linkgit:git-config[1].
         ++
        @@ -148,7 +150,7 @@
         +one for the purposes of disambiguation, even if the `<branch>` isn't
         +unique across all remotes. Set it to
         +e.g. `checkout.defaultRemote=origin` to always checkout remote
        -+branches from there if `<branch> is ambiguous but exists on the
        ++branches from there if `<branch>` is ambiguous but exists on the
         +'origin' remote. See also `checkout.defaultRemote` in
         +linkgit:git-config[1].
         ++
        @@ -173,22 +175,18 @@
          	 *   (c) Otherwise, if "--" is present, treat it like case (1).
          	 *
         @@
        - 				 "on trying to resolve the argument as a path, but failed there too!\n"
        + 				 "If you meant to check out a remote tracking branch on e.g. 'origin'\n"
        + 				 "you can do so by fully-qualifying the name with the --track option:\n"
          				 "\n"
        - 				 "Perhaps you meant fully qualify the branch name? E.g. origin/<name>\n"
        --				 "instead of <name>?"),
        -+				 "instead of <name>?\n"
        +-				 "    git checkout --track origin/<name>"),
        ++				 "    git checkout --track origin/<name>\n"
         +				 "\n"
        -+				 "If you'd like to always have checkouts of '%s' prefer one remote,\n"
        -+				 "e.g. the 'origin' remote, consider setting checkout.defaultRemote=origin\n"
        -+				 "in your config. See the 'git-config' manual page for details."),
        ++				 "If you'd like to always have checkouts of an ambiguous <name> prefer\n"
        ++				 "one remote, e.g. the 'origin' remote, consider setting\n"
        ++				 "checkout.defaultRemote=origin in your config."),
          			       argv[0],
        --			       dwim_remotes_matched);
        -+			       dwim_remotes_matched,
        -+			       argv[0]);
        + 			       dwim_remotes_matched);
          		return ret;
        - 	} else {
        - 		return checkout_branch(&opts, &new_branch_info);
         
         diff --git a/checkout.c b/checkout.c
         --- a/checkout.c
        @@ -198,6 +196,19 @@
          #include "refspec.h"
          #include "checkout.h"
         +#include "config.h"
        + 
        + struct tracking_name_data {
        + 	/* const */ char *src_ref;
        + 	char *dst_ref;
        + 	struct object_id *dst_oid;
        + 	int num_matches;
        ++	const char *default_remote;
        ++	char *default_dst_ref;
        ++	struct object_id *default_dst_oid;
        + };
        + 
        +-#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0 }
        ++#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0, NULL, NULL, NULL }
          
          static int check_tracking_name(struct remote *remote, void *cb_data)
          {
        @@ -243,31 +254,21 @@
          	return NULL;
          }
         
        -diff --git a/checkout.h b/checkout.h
        ---- a/checkout.h
        -+++ b/checkout.h
        -@@
        - 	char *dst_ref;
        - 	struct object_id *dst_oid;
        - 	int num_matches;
        -+	const char *default_remote;
        -+	char *default_dst_ref;
        -+	struct object_id *default_dst_oid;
        - };
        - 
        --#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0 }
        -+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0, NULL, NULL, NULL }
        - 
        - /*
        -  * Check if the branch name uniquely matches a branch name on a remote
        -
         diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
         --- a/t/t2024-checkout-dwim.sh
         +++ b/t/t2024-checkout-dwim.sh
         @@
        - 	test_i18ngrep ! "^hint: " stderr
        - '
        - 
        + 		checkout foo 2>stderr &&
        + 	test_branch master &&
        + 	status_uno_is_clean &&
        +-	test_i18ngrep ! "^hint: " stderr
        ++	test_i18ngrep ! "^hint: " stderr &&
        ++	# Make sure the likes of checkout -p don not print this hint
        ++	git checkout -p foo 2>stderr &&
        ++	test_i18ngrep ! "^hint: " stderr &&
        ++	status_uno_is_clean
        ++'
        ++
         +test_expect_success 'checkout of branch from multiple remotes succeeds with checkout.defaultRemote #1' '
         +	git checkout -B master &&
         +	status_uno_is_clean &&
        @@ -278,11 +279,9 @@
         +	test_branch foo &&
         +	test_cmp_rev remotes/repo_a/foo HEAD &&
         +	test_branch_upstream foo repo_a foo
        -+'
        -+
        + '
        + 
          test_expect_success 'checkout of branch from a single remote succeeds #1' '
        - 	git checkout -B master &&
        - 	test_might_fail git branch -D bar &&
         
         diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
         --- a/t/t2025-worktree-add.sh

Ævar Arnfjörð Bjarmason (8):
checkout tests: index should be clean after dwim checkout checkout.h: wrap the arguments to unique_tracking_name() checkout.[ch]: introduce an *_INIT macro checkout.[ch]: change "unique" member to "num_matches"
checkout: pass the "num_matches" up to callers builtin/checkout.c: use "ret" variable for return
checkout: add advice for ambiguous "checkout <branch>" checkout & worktree: introduce checkout.defaultRemote

 Documentation/config.txt       | 26 +++++++++++++++
 Documentation/git-checkout.txt |  9 ++++++
 Documentation/git-worktree.txt |  9 ++++++
 advice.c                       |  2 ++
 advice.h                       |  1 +
 builtin/checkout.c             | 41 ++++++++++++++++++-----
 builtin/worktree.c             |  4 +--
 checkout.c                     | 37 ++++++++++++++++++---
 checkout.h                     |  4 ++-
 t/t2024-checkout-dwim.sh       | 59 ++++++++++++++++++++++++++++++++++
 t/t2025-worktree-add.sh        | 21 ++++++++++++
 11 files changed, 197 insertions(+), 16 deletions(-)

2.17.0.290.gded63e768a

Assert that whenever there's a DWIM checkout that the index should be clean afterwards, in addition to the correct branch being checked-out.

The way the DWIM checkout code in checkout.[ch] works is by looping over all remotes, and for each remote trying to find if a given reference name only exists on that remote, or if it exists anywhere else.

This is done by starting out with a unique = 1 tracking variable in a struct shared by the entire loop, which will get set to 0 if the data reference is not unique.

Thus if we find a match we know the dst_oid member of tracking_name_data must be correct, since it's associated with the only reference on the only remote that could have matched our query.

But if there was ever a mismatch there for some reason we might end up with the correct branch checked out, but at the wrong oid, which would show whatever the difference between the two staged in the index (checkout branch A, stage changes from the state of branch B).

So let's amend the tests (mostly added in) 399e4a1c56 ("t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'", 2013-04-21) to always assert that "status" is clean after we run "checkout", that's being done with "-uno" because there's going to be some untracked files related to the test itself which we don't care about.

In all these tests (DWIM or otherwise) we start with a clean index, so these tests are asserting that that's still the case after the "checkout", failed or otherwise.

Then if we ever run into this sort of regression, either in the existing code or with a new feature, we'll know.

---
 t/t2024-checkout-dwim.sh | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

index 3e5ac81bd2..ed32828105 100755
@@ -23,6 +23,12 @@ test_branch_upstream () {
 	test_cmp expect.upstream actual.upstream
 }
 
+status_uno_is_clean() {
+	>status.expect &&
+	git status -uno --porcelain >status.actual &&
+	test_cmp status.expect status.actual
+}
+
 test_expect_success 'setup' '
 	test_commit my_master &&
 	git init repo_a &&

@@ -55,6 +61,7 @@ test_expect_success 'checkout of non-existing branch fails' '
 	test_might_fail git branch -D xyzzy &&
 
 	test_must_fail git checkout xyzzy &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/xyzzy &&
 	test_branch master
 '

@@ -64,6 +71,7 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '
 	test_might_fail git branch -D foo &&
 
 	test_must_fail git checkout foo &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/foo &&
 	test_branch master
 '

@@ -73,6 +81,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #1' '
 	test_might_fail git branch -D bar &&
 
 	git checkout bar &&
+	status_uno_is_clean &&
 	test_branch bar &&
 	test_cmp_rev remotes/repo_a/bar HEAD &&
 	test_branch_upstream bar repo_a bar

@@ -83,6 +92,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '
 	test_might_fail git branch -D baz &&
 
 	git checkout baz &&
+	status_uno_is_clean &&
 	test_branch baz &&
 	test_cmp_rev remotes/other_b/baz HEAD &&
 	test_branch_upstream baz repo_b baz

@@ -90,6 +100,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '
 
 test_expect_success '--no-guess suppresses branch auto-vivification' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D bar &&
 
 	test_must_fail git checkout --no-guess bar &&

@@ -99,6 +110,7 @@ test_expect_success '--no-guess suppresses branch auto-vivification' '
 
 test_expect_success 'setup more remotes with unconventional refspecs' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	git init repo_c &&
 	(
 		cd repo_c &&

@@ -128,27 +140,33 @@ test_expect_success 'setup more remotes with unconventional refspecs' '
 
 test_expect_success 'checkout of branch from multiple remotes fails #2' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D bar &&
 
 	test_must_fail git checkout bar &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/bar &&
 	test_branch master
 '
 
 test_expect_success 'checkout of branch from multiple remotes fails #3' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D baz &&
 
 	test_must_fail git checkout baz &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/baz &&
 	test_branch master
 '
 
 test_expect_success 'checkout of branch from a single remote succeeds #3' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	git checkout spam &&
+	status_uno_is_clean &&
 	test_branch spam &&
 	test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD &&
 	test_branch_upstream spam repo_c spam

@@ -156,9 +174,11 @@ test_expect_success 'checkout of branch from a single remote succeeds #3' '
 
 test_expect_success 'checkout of branch from a single remote succeeds #4' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D eggs &&
 
 	git checkout eggs &&
+	status_uno_is_clean &&
 	test_branch eggs &&
 	test_cmp_rev refs/repo_d/eggs HEAD &&
 	test_branch_upstream eggs repo_d eggs

@@ -166,32 +186,38 @@ test_expect_success 'checkout of branch from a single remote succeeds #4' '
 
 test_expect_success 'checkout of branch with a file having the same name fails' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	>spam &&
 	test_must_fail git checkout spam &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/spam &&
 	test_branch master
 '
 
 test_expect_success 'checkout of branch with a file in subdir having the same name fails' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	>spam &&
 	mkdir sub &&
 	mv spam sub/spam &&
 	test_must_fail git -C sub checkout spam &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/spam &&
 	test_branch master
 '
 
 test_expect_success 'checkout <branch> -- succeeds, even if a file with the same name exists' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	>spam &&
 	git checkout spam -- &&
+	status_uno_is_clean &&
 	test_branch spam &&
 	test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD &&
 	test_branch_upstream spam repo_c spam

@@ -200,6 +226,7 @@ test_expect_success 'checkout <branch> -- succeeds, even if a file with the same
 test_expect_success 'loosely defined local base branch is reported correctly' '
 
 	git checkout master &&
+	status_uno_is_clean &&
 	git branch strict &&
 	git branch loose &&
 	git commit --allow-empty -m "a bit more" &&

@@ -210,7 +237,9 @@ test_expect_success 'loosely defined local base branch is reported correctly' '
 	test_config branch.loose.merge master &&
 
 	git checkout strict | sed -e "s/strict/BRANCHNAME/g" >expect &&
+	status_uno_is_clean &&
 	git checkout loose | sed -e "s/loose/BRANCHNAME/g" >actual &&
+	status_uno_is_clean &&
 
 	test_cmp expect actual
 '

2.17.0.290.gded63e768a

The line was too long already, and will be longer still when a later change adds another argument.

---
 checkout.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 9980711179..4cd4cd1c23 100644
@@ -8,6 +8,7 @@
  * tracking branch.  Return the name of the remote if such a branch
  * exists, NULL otherwise.
  */
-extern const char *unique_tracking_name(const char *name, struct object_id *oid);
+extern const char *unique_tracking_name(const char *name,
+					struct object_id *oid);
 
 #endif /* CHECKOUT_H */

2.17.0.290.gded63e768a

Add an *_INIT macro for the tracking_name_data similar to what exists elsewhere in the codebase, e.g. OID_ARRAY_INIT in sha1-array.h. This will make it more idiomatic in later changes to add more fields to the struct & its initialization macro.

---
 checkout.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index bdefc888ba..80e430cda8 100644
@@ -10,6 +10,8 @@ struct tracking_name_data {
 	int unique;
 };
 
+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 1 }
+
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {
 	struct tracking_name_data *cb = cb_data;

@@ -32,7 +34,7 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 
 const char *unique_tracking_name(const char *name, struct object_id *oid)
 {
-	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);

2.17.0.290.gded63e768a

checkout.c: introduce...

Internally track how many matches we find in the check_tracking_name() callback. Nothing uses this now, but it will be made use of in a later change.

---
 checkout.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

index 80e430cda8..7662a39a62 100644
@@ -7,10 +7,10 @@ struct tracking_name_data {
 	/* const */ char *src_ref;
 	char *dst_ref;
 	struct object_id *dst_oid;
-	int unique;
+	int num_matches;
 };
 
-#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 1 }
+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0 }
 
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {

@@ -23,9 +23,9 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 		free(query.dst);
 		return 0;
 	}
+	cb->num_matches++;
 	if (cb->dst_ref) {
 		free(query.dst);
-		cb->unique = 0;
 		return 0;
 	}
 	cb->dst_ref = query.dst;

@@ -39,7 +39,7 @@ const char *unique_tracking_name(const char *name, struct object_id *oid)
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
 	free(cb_data.src_ref);
-	if (cb_data.unique)
+	if (cb_data.num_matches == 1)
 		return cb_data.dst_ref;
 	free(cb_data.dst_ref);
 	return NULL;

2.17.0.290.gded63e768a

checkout.c: change...

Pass the previously added "num_matches" struct value up to the callers of unique_tracking_name(). This will allow callers to optionally print better error messages in a later change.

---
 builtin/checkout.c | 10 +++++++---
 builtin/worktree.c |  4 ++--
 checkout.c         |  5 ++++-
 checkout.h         |  3 ++-
 4 files changed, 15 insertions(+), 7 deletions(-)

index 2e1d2376d2..72457fb7d5 100644
@@ -878,7 +878,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 				int dwim_new_local_branch_ok,
 				struct branch_info *new_branch_info,
 				struct checkout_opts *opts,
-				struct object_id *rev)
+				struct object_id *rev,
+				int *dwim_remotes_matched)
 {
 	struct tree **source_tree = &opts->source_tree;
 	const char **new_branch = &opts->new_branch;

@@ -972,7 +973,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 			recover_with_dwim = 0;
 
 		if (recover_with_dwim) {
-			const char *remote = unique_tracking_name(arg, rev);
+			const char *remote = unique_tracking_name(arg, rev,
+								  dwim_remotes_matched);
 			if (remote) {
 				*new_branch = arg;
 				arg = remote;

@@ -1109,6 +1111,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct branch_info new_branch_info;
 	char *conflict_style = NULL;
 	int dwim_new_local_branch = 1;
+	int dwim_remotes_matched = 0;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
 		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),

@@ -1219,7 +1222,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			opts.track == BRANCH_TRACK_UNSPECIFIED &&
 			!opts.new_branch;
 		int n = parse_branchname_arg(argc, argv, dwim_ok,
-					     &new_branch_info, &opts, &rev);
+					     &new_branch_info, &opts, &rev,
+					     &dwim_remotes_matched);
 		argv += n;
 		argc -= n;
 	}

index 5c7d2bb180..a763dbdccb 100644
@@ -412,7 +412,7 @@ static const char *dwim_branch(const char *path, const char **new_branch)
 	if (guess_remote) {
 		struct object_id oid;
 		const char *remote =
-			unique_tracking_name(*new_branch, &oid);
+			unique_tracking_name(*new_branch, &oid, NULL);
 		return remote;
 	}
 	return NULL;

@@ -484,7 +484,7 @@ static int add(int ac, const char **av, const char *prefix)
 
 		commit = lookup_commit_reference_by_name(branch);
 		if (!commit) {
-			remote = unique_tracking_name(branch, &oid);
+			remote = unique_tracking_name(branch, &oid, NULL);
 			if (remote) {
 				new_branch = branch;
 				branch = remote;

index 7662a39a62..ee3a7e9c05 100644
@@ -32,12 +32,15 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 	return 0;
 }
 
-const char *unique_tracking_name(const char *name, struct object_id *oid)
+const char *unique_tracking_name(const char *name, struct object_id *oid,
+				 int *dwim_remotes_matched)
 {
 	struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
+	if (dwim_remotes_matched)
+		*dwim_remotes_matched = cb_data.num_matches;
 	free(cb_data.src_ref);
 	if (cb_data.num_matches == 1)
 		return cb_data.dst_ref;

index 4cd4cd1c23..6b2073310c 100644
@@ -9,6 +9,7 @@
  * exists, NULL otherwise.
  */
 extern const char *unique_tracking_name(const char *name,
-					struct object_id *oid);
+					struct object_id *oid,
+					int *dwim_remotes_matched);
 
 #endif /* CHECKOUT_H */

2.17.0.290.gded63e768a

There is no point in doing this right now, but in later change the "ret" variable will be inspected. This change makes that meaningful change smaller.

---
 builtin/checkout.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

index 72457fb7d5..8c93c55cbc 100644
@@ -1265,8 +1265,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	}
 
 	UNLEAK(opts);
-	if (opts.patch_mode || opts.pathspec.nr)
-		return checkout_paths(&opts, new_branch_info.name);
-	else
+	if (opts.patch_mode || opts.pathspec.nr) {
+		int ret = checkout_paths(&opts, new_branch_info.name);
+		return ret;
+	} else {
 		return checkout_branch(&opts, &new_branch_info);
+	}
 }

2.17.0.290.gded63e768a

As the "checkout" documentation describes:

If <branch> is not found but there does exist a tracking branch in exactly one remote (call it <remote>) with a matching name, treat as equivalent to [...] <remote>/<branch.

This is a really useful feature. The problem is that when you and another remote (e.g. a fork) git won't find a unique branch name anymore, and will instead print this nondescript message:

    $ git checkout master
    error: pathspec 'master' did not match any file(s) known to git

Now it will, on my git.git checkout, print:

    $ ./git --exec-path=$PWD checkout master
    error: pathspec 'master' did not match any file(s) known to git.
    hint: The argument 'master' matched more than one remote tracking branch.
    hint: We found 26 remotes with a reference that matched. So we fell back
    hint: on trying to resolve the argument as a path, but failed there too!
    hint:
    hint: If you meant to check out a remote tracking branch on e.g. 'origin'
    hint: you can do so by fully-qualifying the name with the --track option:
    hint:
    hint:     git checkout --track origin/<name>

---
 Documentation/config.txt |  7 +++++++
 advice.c                 |  2 ++
 advice.h                 |  1 +
 builtin/checkout.c       | 13 +++++++++++++
 t/t2024-checkout-dwim.sh | 14 ++++++++++++++
 5 files changed, 37 insertions(+)

index ab641bf5a9..dfc0413a84 100644
@@ -344,6 +344,13 @@ advice.*::
 		Advice shown when you used linkgit:git-checkout[1] to
 		move to the detach HEAD state, to instruct how to create
 		a local branch after the fact.
+	checkoutAmbiguousRemoteBranchName::
+		Advice shown when the argument to
+		linkgit:git-checkout[1] ambiguously resolves to a
+		remote tracking branch on more than one remote in
+		situations where an unambiguous argument would have
+		otherwise caused a remote-tracking branch to be
+		checked out.
 	amWorkDir::
 		Advice that shows the location of the patch file when
 		linkgit:git-am[1] fails to apply it.

index 370a56d054..75e7dede90 100644
@@ -21,6 +21,7 @@ int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
 int advice_graft_file_deprecated = 1;
+int advice_checkout_ambiguous_remote_branch_name = 1;
 
 static int advice_use_color = -1;
 static char advice_colors[][COLOR_MAXLEN] = {

@@ -72,6 +73,7 @@ static struct {
 	{ "ignoredhook", &advice_ignored_hook },
 	{ "waitingforeditor", &advice_waiting_for_editor },
 	{ "graftfiledeprecated", &advice_graft_file_deprecated },
+	{ "checkoutambiguousremotebranchname", &advice_checkout_ambiguous_remote_branch_name },
 
 	/* make this an alias for backward compatibility */
 	{ "pushnonfastforward", &advice_push_update_rejected }

index 9f5064e82a..4d11d51d43 100644
@@ -22,6 +22,7 @@ extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
 extern int advice_graft_file_deprecated;
+extern int advice_checkout_ambiguous_remote_branch_name;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))

index 8c93c55cbc..4dfb8f1535 100644
@@ -22,6 +22,7 @@
 #include "resolve-undo.h"
 #include "submodule-config.h"
 #include "submodule.h"
+#include "advice.h"
 
 static const char * const checkout_usage[] = {
 	N_("git checkout [<options>] <branch>"),

@@ -1267,6 +1268,18 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	UNLEAK(opts);
 	if (opts.patch_mode || opts.pathspec.nr) {
 		int ret = checkout_paths(&opts, new_branch_info.name);
+		if (ret && dwim_remotes_matched > 1 &&
+		    advice_checkout_ambiguous_remote_branch_name)
+			advise(_("The argument '%s' matched more than one remote tracking branch.\n"
+				 "We found %d remotes with a reference that matched. So we fell back\n"
+				 "on trying to resolve the argument as a path, but failed there too!\n"
+				 "\n"
+				 "If you meant to check out a remote tracking branch on e.g. 'origin'\n"
+				 "you can do so by fully-qualifying the name with the --track option:\n"
+				 "\n"
+				 "    git checkout --track origin/<name>"),
+			       argv[0],
+			       dwim_remotes_matched);
 		return ret;
 	} else {
 		return checkout_branch(&opts, &new_branch_info);

index ed32828105..fef263a858 100755
@@ -76,6 +76,20 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '
 	test_branch master
 '
 
+test_expect_success 'checkout of branch from multiple remotes fails with advice' '
+	git checkout -B master &&
+	test_might_fail git branch -D foo &&
+	test_must_fail git checkout foo 2>stderr &&
+	test_branch master &&
+	status_uno_is_clean &&
+	test_i18ngrep "^hint: " stderr &&
+	test_must_fail git -c advice.checkoutAmbiguousRemoteBranchName=false \
+		checkout foo 2>stderr &&
+	test_branch master &&
+	status_uno_is_clean &&
+	test_i18ngrep ! "^hint: " stderr
+'
+
 test_expect_success 'checkout of branch from a single remote succeeds #1' '
 	git checkout -B master &&
 	test_might_fail git branch -D bar &&

2.17.0.290.gded63e768a

s/and/add/

Missing comma: s/)/),/

Perhaps s/nondescript/unhelpful/ ?

Missing commas: s/on e.g. 'origin'/on, e.g. 'origin',/

s/fully-qualifying/fully qualifying/

Aside from the s/and/add/ botch, all of the above are tiny nits which don't actually impact the meaning of the commit message, thus not really important.

Introduce a checkout.defaultRemote setting which can be used to designate a remote to prefer (via checkout.defaultRemote=origin) when running e.g. "git checkout master" to mean origin/master, even though there's other remotes that have the "master" branch.

I want this because it's very handy to use this workflow to checkout a repository and create a topic branch, then get back to a "master" as retrieved from upstream:

    (
        cd /tmp &&
        rm -rf tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git checkout master
    )

That will output:

Branch 'master' set up to track remote branch 'master' from 'origin'. Switched to a new branch 'master'

But as soon as a new remote is added (e.g. just to inspect something from someone else) the DWIMery goes away:

    (
        cd /tmp &&
        rm -rf tbdiff &&
        git clone [email protected]:trast/tbdiff.git &&
        cd tbdiff &&
        git branch -m topic &&
        git remote add avar [email protected]:avar/tbdiff.git &&
        git fetch avar &&
        git checkout master
    )

Will output (without the advice output added earlier in this series):

error: pathspec 'master' did not match any file(s) known to git.

The new checkout.defaultRemote config allows me to say that whenever that ambiguity comes up I'd like to prefer "origin", and it'll still work as though the only remote I had was "origin".

Also adjust the advice.checkoutAmbiguousRemoteBranchName message to mention this new config setting to the user, the full output on my git.git is now (the last paragraph is new):

    $ ./git --exec-path=$PWD checkout master
    error: pathspec 'master' did not match any file(s) known to git.
    hint: The argument 'master' matched more than one remote tracking branch.
    hint: We found 26 remotes with a reference that matched. So we fell back
    hint: on trying to resolve the argument as a path, but failed there too!
    hint:
    hint: If you meant to check out a remote tracking branch on e.g. 'origin'
    hint: you can do so by fully-qualifying the name with the --track option:
    hint:
    hint:     git checkout --track origin/<name>
    hint:
    hint: If you'd like to always have checkouts of an ambiguous <name> prefer
    hint: one remote, e.g. the 'origin' remote, consider setting
    hint: checkout.defaultRemote=origin in your config.

I considered splitting this into checkout.defaultRemote and worktree.defaultRemote, but it's probably less confusing to break our own rules that anything shared between config should live in core.* than have two config settings, and I couldn't come up with a short name under core.* that made sense (core.defaultRemoteForCheckout?).

See also 70c9ac2f19 ("DWIM "git checkout frotz" to "git checkout -b frotz origin/frotz"", 2009-10-18) which introduced this DWIM feature to begin with, and 4e85333197 ("worktree: make add <path> <branch> dwim", 2017-11-26) which added it to git-worktree.

---
 Documentation/config.txt       | 21 ++++++++++++++++++++-
 Documentation/git-checkout.txt |  9 +++++++++
 Documentation/git-worktree.txt |  9 +++++++++
 builtin/checkout.c             | 12 +++++++++---
 checkout.c                     | 26 ++++++++++++++++++++++++--
 t/t2024-checkout-dwim.sh       | 18 +++++++++++++++++-
 t/t2025-worktree-add.sh        | 21 +++++++++++++++++++++
 7 files changed, 109 insertions(+), 7 deletions(-)

index dfc0413a84..aef2769211 100644
@@ -350,7 +350,10 @@ advice.*::
 		remote tracking branch on more than one remote in
 		situations where an unambiguous argument would have
 		otherwise caused a remote-tracking branch to be
-		checked out.
+		checked out. See the `checkout.defaultRemote`
+		configuration variable for how to set a given remote
+		to used by default in some situations where this
+		advice would be printed.
 	amWorkDir::
 		Advice that shows the location of the patch file when
 		linkgit:git-am[1] fails to apply it.

@@ -1105,6 +1108,22 @@ browser.<tool>.path::
 	browse HTML help (see `-w` option in linkgit:git-help[1]) or a
 	working repository in gitweb (see linkgit:git-instaweb[1]).
 
+checkout.defaultRemote::
+	When you run 'git checkout <something>' and only have one
+	remote, it may implicitly fall back on checking out and
+	tracking e.g. 'origin/<something>'. This stops working as soon
+	as you have more than one remote with a '<something>'
+	reference. This setting allows for setting the name of a
+	preferred remote that should always win when it comes to
+	disambiguation. The typical use-case is to set this to
+	`origin`.
++
+Currently this is used by linkgit:git-checkout[1] when 'git checkout
+<something>' will checkout the '<something>' branch on another remote,
+and by linkgit:git-worktree[1] when 'git worktree add' refers to a
+remote branch. This setting might be used for other checkout-like
+commands or functionality in the future.
+
 clean.requireForce::
 	A boolean to make git-clean do nothing unless given -f,
 	-i or -n.   Defaults to true.

index ca5fc9c798..9db02928c4 100644
@@ -38,6 +38,15 @@ equivalent to
 $ git checkout -b <branch> --track <remote>/<branch>
 ------------
 +
+If the branch exists in multiple remotes and one of them is named by
+the `checkout.defaultRemote` configuration variable, we'll use that
+one for the purposes of disambiguation, even if the `<branch>` isn't
+unique across all remotes. Set it to
+e.g. `checkout.defaultRemote=origin` to always checkout remote
+branches from there if `<branch>` is ambiguous but exists on the
+'origin' remote. See also `checkout.defaultRemote` in
+linkgit:git-config[1].
++
 You could omit <branch>, in which case the command degenerates to
 "check out the current branch", which is a glorified no-op with
 rather expensive side-effects to show only the tracking information,

index afc6576a14..9c26be40f4 100644
@@ -60,6 +60,15 @@ with a matching name, treat as equivalent to:
 $ git worktree add --track -b <branch> <path> <remote>/<branch>
 ------------
 +
+If the branch exists in multiple remotes and one of them is named by
+the `checkout.defaultRemote` configuration variable, we'll use that
+one for the purposes of disambiguation, even if the `<branch>` isn't
+unique across all remotes. Set it to
+e.g. `checkout.defaultRemote=origin` to always checkout remote
+branches from there if `<branch>` is ambiguous but exists on the
+'origin' remote. See also `checkout.defaultRemote` in
+linkgit:git-config[1].
++
 If `<commit-ish>` is omitted and neither `-b` nor `-B` nor `--detach` used,
 then, as a convenience, the new worktree is associated with a branch
 (call it `<branch>`) named after `$(basename <path>)`.  If `<branch>`

index 4dfb8f1535..b78481dead 100644
@@ -912,8 +912,10 @@ static int parse_branchname_arg(int argc, const char **argv,
 	 *   (b) If <something> is _not_ a commit, either "--" is present
 	 *       or <something> is not a path, no -t or -b was given, and
 	 *       and there is a tracking branch whose name is <something>
-	 *       in one and only one remote, then this is a short-hand to
-	 *       fork local <something> from that remote-tracking branch.
+	 *       in one and only one remote (or if the branch exists on the
+	 *       remote named in checkout.defaultRemote), then this is a
+	 *       short-hand to fork local <something> from that
+	 *       remote-tracking branch.
 	 *
 	 *   (c) Otherwise, if "--" is present, treat it like case (1).
 	 *

@@ -1277,7 +1279,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 				 "If you meant to check out a remote tracking branch on e.g. 'origin'\n"
 				 "you can do so by fully-qualifying the name with the --track option:\n"
 				 "\n"
-				 "    git checkout --track origin/<name>"),
+				 "    git checkout --track origin/<name>\n"
+				 "\n"
+				 "If you'd like to always have checkouts of an ambiguous <name> prefer\n"
+				 "one remote, e.g. the 'origin' remote, consider setting\n"
+				 "checkout.defaultRemote=origin in your config."),
 			       argv[0],
 			       dwim_remotes_matched);
 		return ret;

index ee3a7e9c05..c72e9f9773 100644
@@ -2,15 +2,19 @@
 #include "remote.h"
 #include "refspec.h"
 #include "checkout.h"
+#include "config.h"
 
 struct tracking_name_data {
 	/* const */ char *src_ref;
 	char *dst_ref;
 	struct object_id *dst_oid;
 	int num_matches;
+	const char *default_remote;
+	char *default_dst_ref;
+	struct object_id *default_dst_oid;
 };
 
-#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0 }
+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0, NULL, NULL, NULL }
 
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {

@@ -24,6 +28,12 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 		return 0;
 	}
 	cb->num_matches++;
+	if (cb->default_remote && !strcmp(remote->name, cb->default_remote)) {
+		struct object_id *dst = xmalloc(sizeof(*cb->default_dst_oid));
+		cb->default_dst_ref = xstrdup(query.dst);
+		oidcpy(dst, cb->dst_oid);
+		cb->default_dst_oid = dst;
+	}
 	if (cb->dst_ref) {
 		free(query.dst);
 		return 0;

@@ -36,14 +46,26 @@ const char *unique_tracking_name(const char *name, struct object_id *oid,
 				 int *dwim_remotes_matched)
 {
 	struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
+	const char *default_remote = NULL;
+	if (!git_config_get_string_const("checkout.defaultremote", &default_remote))
+		cb_data.default_remote = default_remote;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
 	if (dwim_remotes_matched)
 		*dwim_remotes_matched = cb_data.num_matches;
 	free(cb_data.src_ref);
-	if (cb_data.num_matches == 1)
+	free((char *)default_remote);
+	if (cb_data.num_matches == 1) {
+		free(cb_data.default_dst_ref);
+		free(cb_data.default_dst_oid);
 		return cb_data.dst_ref;
+	}
 	free(cb_data.dst_ref);
+	if (cb_data.default_dst_ref) {
+		oidcpy(oid, cb_data.default_dst_oid);
+		free(cb_data.default_dst_oid);
+		return cb_data.default_dst_ref;
+	}
 	return NULL;
 }

index fef263a858..1495c248a7 100755
@@ -87,7 +87,23 @@ test_expect_success 'checkout of branch from multiple remotes fails with advice'
 		checkout foo 2>stderr &&
 	test_branch master &&
 	status_uno_is_clean &&
-	test_i18ngrep ! "^hint: " stderr
+	test_i18ngrep ! "^hint: " stderr &&
+	# Make sure the likes of checkout -p don not print this hint
+	git checkout -p foo 2>stderr &&
+	test_i18ngrep ! "^hint: " stderr &&
+	status_uno_is_clean
+'
+
+test_expect_success 'checkout of branch from multiple remotes succeeds with checkout.defaultRemote #1' '
+	git checkout -B master &&
+	status_uno_is_clean &&
+	test_might_fail git branch -D foo &&
+
+	git -c checkout.defaultRemote=repo_a checkout foo &&
+	status_uno_is_clean &&
+	test_branch foo &&
+	test_cmp_rev remotes/repo_a/foo HEAD &&
+	test_branch_upstream foo repo_a foo
 '
 
 test_expect_success 'checkout of branch from a single remote succeeds #1' '

index d2e49f7632..be6e093142 100755
@@ -402,6 +402,27 @@ test_expect_success '"add" <path> <branch> dwims' '
 	)
 '
 
+test_expect_success '"add" <path> <branch> dwims with checkout.defaultRemote' '
+	test_when_finished rm -rf repo_upstream repo_dwim foo &&
+	setup_remote_repo repo_upstream repo_dwim &&
+	git init repo_dwim &&
+	(
+		cd repo_dwim &&
+		git remote add repo_upstream2 ../repo_upstream &&
+		git fetch repo_upstream2 &&
+		test_must_fail git worktree add ../foo foo &&
+		git -c checkout.defaultRemote=repo_upstream worktree add ../foo foo &&
+		>status.expect &&
+		git status -uno --porcelain >status.actual &&
+		test_cmp status.expect status.actual
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_upstream foo &&
+		test_cmp_rev refs/remotes/repo_upstream/foo refs/heads/foo
+	)
+'
+
 test_expect_success 'git worktree add does not match remote' '
 	test_when_finished rm -rf repo_a repo_b foo &&
 	setup_remote_repo repo_a repo_b &&

2.17.0.290.gded63e768a

Typo & grammar fixes suggested by Eric Sunshine. tbdiff from v5:

1: ab4529d9f5 = 1: ab4529d9f5 checkout tests: index should be clean after dwim checkout 2: c8bbece403 = 2: c8bbece403 checkout.h: wrap the arguments to unique_tracking_name() 3: 4fc5ab27fa ! 3: 881fe63f4f checkout.c: introduce an *_INIT macro
@@ -1,6 +1,6 @@
Author: Ævar Arnfjörð Bjarmason <[email protected]>

- checkout.[ch]: introduce an *_INIT macro
+ checkout.c: introduce an *_INIT macro

Add an *_INIT macro for the tracking_name_data similar to what exists elsewhere in the codebase, e.g. OID_ARRAY_INIT in sha1-array.h. This
4: fbce6df584 ! 4: 72ddaeddd3 checkout.c]: change "unique" member to "num_matches"
@@ -1,6 +1,6 @@
Author: Ævar Arnfjörð Bjarmason <[email protected]>

- checkout.[ch]: change "unique" member to "num_matches"
+ checkout.c]: change "unique" member to "num_matches"

Internally track how many matches we find in the check_tracking_name() callback. Nothing uses this now, but it will be made use of in a later
5: 6e016d43d7 = 5: 5e8c82680b checkout: pass the "num_matches" up to callers 6: 07b11b133d = 6: 07e667f80a builtin/checkout.c: use "ret" variable for return 7: 97e84f6e1c ! 7: 0a148182e6 checkout: add advice for ambiguous "checkout <branch>"
@@ -8,9 +8,9 @@
exactly one remote (call it <remote>) with a matching name, treat as equivalent to [...] <remote>/<branch.

- This is a really useful feature. The problem is that when you and
- another remote (e.g. a fork) git won't find a unique branch name
- anymore, and will instead print this nondescript message:
+ This is a really useful feature. The problem is that when you add
+ another remote (e.g. a fork), git won't find a unique branch name
+ anymore, and will instead print this unhelpful message:

$ git checkout master
error: pathspec 'master' did not match any file(s) known to git
@@ -19,12 +19,12 @@

$ ./git --exec-path=$PWD checkout master
error: pathspec 'master' did not match any file(s) known to git.
- hint: The argument 'master' matched more than one remote tracking branch.
+ hint: 'master' matched more than one remote tracking branch.
hint: We found 26 remotes with a reference that matched. So we fell back
hint: on trying to resolve the argument as a path, but failed there too! hint:
- hint: If you meant to check out a remote tracking branch on e.g. 'origin'
- hint: you can do so by fully-qualifying the name with the --track option:
+ hint: If you meant to check out a remote tracking branch on, e.g. 'origin',
+ hint: you can do so by fully qualifying the name with the --track option:
hint:
hint: git checkout --track origin/<name>

@@ -97,12 +97,12 @@
int ret = checkout_paths(&opts, new_branch_info.name);
+ if (ret && dwim_remotes_matched > 1 &&
+ advice_checkout_ambiguous_remote_branch_name)
-+ advise(_("The argument '%s' matched more than one remote tracking branch.\n"
++ advise(_("'%s' matched more than one remote tracking branch.\n"
+ "We found %d remotes with a reference that matched. So we fell back\n"
+ "on trying to resolve the argument as a path, but failed there too!\n"
+ "\n"
-+ "If you meant to check out a remote tracking branch on e.g. 'origin'\n"
-+ "you can do so by fully-qualifying the name with the --track option:\n"
++ "If you meant to check out a remote tracking branch on, e.g. 'origin',\n"
++ "you can do so by fully qualifying the name with the --track option:\n"
+ "\n"
+ " git checkout --track origin/<name>"),
+ argv[0],
8: a5cc070ebf ! 8: f3a52a26a2 checkout & worktree: introduce checkout.defaultRemote
@@ -175,8 +175,8 @@
* (c) Otherwise, if "--" is present, treat it like case (1).
*
@@
- "If you meant to check out a remote tracking branch on e.g. 'origin'\n"
- "you can do so by fully-qualifying the name with the --track option:\n"
+ "If you meant to check out a remote tracking branch on, e.g. 'origin',\n"
+ "you can do so by fully qualifying the name with the --track option:\n"
"\n"
- " git checkout --track origin/<name>"),
+ " git checkout --track origin/<name>\n"

Ævar Arnfjörð Bjarmason (8):
checkout tests: index should be clean after dwim checkout checkout.h: wrap the arguments to unique_tracking_name() checkout.c: introduce an *_INIT macro checkout.c]: change "unique" member to "num_matches"
checkout: pass the "num_matches" up to callers builtin/checkout.c: use "ret" variable for return
checkout: add advice for ambiguous "checkout <branch>" checkout & worktree: introduce checkout.defaultRemote

 Documentation/config.txt       | 26 +++++++++++++++
 Documentation/git-checkout.txt |  9 ++++++
 Documentation/git-worktree.txt |  9 ++++++
 advice.c                       |  2 ++
 advice.h                       |  1 +
 builtin/checkout.c             | 41 ++++++++++++++++++-----
 builtin/worktree.c             |  4 +--
 checkout.c                     | 37 ++++++++++++++++++---
 checkout.h                     |  4 ++-
 t/t2024-checkout-dwim.sh       | 59 ++++++++++++++++++++++++++++++++++
 t/t2025-worktree-add.sh        | 21 ++++++++++++
 11 files changed, 197 insertions(+), 16 deletions(-)

2.17.0.290.gded63e768a

Assert that whenever there's a DWIM checkout that the index should be clean afterwards, in addition to the correct branch being checked-out.

The way the DWIM checkout code in checkout.[ch] works is by looping over all remotes, and for each remote trying to find if a given reference name only exists on that remote, or if it exists anywhere else.

This is done by starting out with a unique = 1 tracking variable in a struct shared by the entire loop, which will get set to 0 if the data reference is not unique.

Thus if we find a match we know the dst_oid member of tracking_name_data must be correct, since it's associated with the only reference on the only remote that could have matched our query.

But if there was ever a mismatch there for some reason we might end up with the correct branch checked out, but at the wrong oid, which would show whatever the difference between the two staged in the index (checkout branch A, stage changes from the state of branch B).

So let's amend the tests (mostly added in) 399e4a1c56 ("t2024: Add tests verifying current DWIM behavior of 'git checkout <branch>'", 2013-04-21) to always assert that "status" is clean after we run "checkout", that's being done with "-uno" because there's going to be some untracked files related to the test itself which we don't care about.

In all these tests (DWIM or otherwise) we start with a clean index, so these tests are asserting that that's still the case after the "checkout", failed or otherwise.

Then if we ever run into this sort of regression, either in the existing code or with a new feature, we'll know.

---
 t/t2024-checkout-dwim.sh | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

index 3e5ac81bd2..ed32828105 100755
@@ -23,6 +23,12 @@ test_branch_upstream () {
 	test_cmp expect.upstream actual.upstream
 }
 
+status_uno_is_clean() {
+	>status.expect &&
+	git status -uno --porcelain >status.actual &&
+	test_cmp status.expect status.actual
+}
+
 test_expect_success 'setup' '
 	test_commit my_master &&
 	git init repo_a &&

@@ -55,6 +61,7 @@ test_expect_success 'checkout of non-existing branch fails' '
 	test_might_fail git branch -D xyzzy &&
 
 	test_must_fail git checkout xyzzy &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/xyzzy &&
 	test_branch master
 '

@@ -64,6 +71,7 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '
 	test_might_fail git branch -D foo &&
 
 	test_must_fail git checkout foo &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/foo &&
 	test_branch master
 '

@@ -73,6 +81,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #1' '
 	test_might_fail git branch -D bar &&
 
 	git checkout bar &&
+	status_uno_is_clean &&
 	test_branch bar &&
 	test_cmp_rev remotes/repo_a/bar HEAD &&
 	test_branch_upstream bar repo_a bar

@@ -83,6 +92,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '
 	test_might_fail git branch -D baz &&
 
 	git checkout baz &&
+	status_uno_is_clean &&
 	test_branch baz &&
 	test_cmp_rev remotes/other_b/baz HEAD &&
 	test_branch_upstream baz repo_b baz

@@ -90,6 +100,7 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '
 
 test_expect_success '--no-guess suppresses branch auto-vivification' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D bar &&
 
 	test_must_fail git checkout --no-guess bar &&

@@ -99,6 +110,7 @@ test_expect_success '--no-guess suppresses branch auto-vivification' '
 
 test_expect_success 'setup more remotes with unconventional refspecs' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	git init repo_c &&
 	(
 		cd repo_c &&

@@ -128,27 +140,33 @@ test_expect_success 'setup more remotes with unconventional refspecs' '
 
 test_expect_success 'checkout of branch from multiple remotes fails #2' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D bar &&
 
 	test_must_fail git checkout bar &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/bar &&
 	test_branch master
 '
 
 test_expect_success 'checkout of branch from multiple remotes fails #3' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D baz &&
 
 	test_must_fail git checkout baz &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/baz &&
 	test_branch master
 '
 
 test_expect_success 'checkout of branch from a single remote succeeds #3' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	git checkout spam &&
+	status_uno_is_clean &&
 	test_branch spam &&
 	test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD &&
 	test_branch_upstream spam repo_c spam

@@ -156,9 +174,11 @@ test_expect_success 'checkout of branch from a single remote succeeds #3' '
 
 test_expect_success 'checkout of branch from a single remote succeeds #4' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D eggs &&
 
 	git checkout eggs &&
+	status_uno_is_clean &&
 	test_branch eggs &&
 	test_cmp_rev refs/repo_d/eggs HEAD &&
 	test_branch_upstream eggs repo_d eggs

@@ -166,32 +186,38 @@ test_expect_success 'checkout of branch from a single remote succeeds #4' '
 
 test_expect_success 'checkout of branch with a file having the same name fails' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	>spam &&
 	test_must_fail git checkout spam &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/spam &&
 	test_branch master
 '
 
 test_expect_success 'checkout of branch with a file in subdir having the same name fails' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	>spam &&
 	mkdir sub &&
 	mv spam sub/spam &&
 	test_must_fail git -C sub checkout spam &&
+	status_uno_is_clean &&
 	test_must_fail git rev-parse --verify refs/heads/spam &&
 	test_branch master
 '
 
 test_expect_success 'checkout <branch> -- succeeds, even if a file with the same name exists' '
 	git checkout -B master &&
+	status_uno_is_clean &&
 	test_might_fail git branch -D spam &&
 
 	>spam &&
 	git checkout spam -- &&
+	status_uno_is_clean &&
 	test_branch spam &&
 	test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD &&
 	test_branch_upstream spam repo_c spam

@@ -200,6 +226,7 @@ test_expect_success 'checkout <branch> -- succeeds, even if a file with the same
 test_expect_success 'loosely defined local base branch is reported correctly' '
 
 	git checkout master &&
+	status_uno_is_clean &&
 	git branch strict &&
 	git branch loose &&
 	git commit --allow-empty -m "a bit more" &&

@@ -210,7 +237,9 @@ test_expect_success 'loosely defined local base branch is reported correctly' '
 	test_config branch.loose.merge master &&
 
 	git checkout strict | sed -e "s/strict/BRANCHNAME/g" >expect &&
+	status_uno_is_clean &&
 	git checkout loose | sed -e "s/loose/BRANCHNAME/g" >actual &&
+	status_uno_is_clean &&
 
 	test_cmp expect actual
 '

2.17.0.290.gded63e768a

The line was too long already, and will be longer still when a later change adds another argument.

---
 checkout.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 9980711179..4cd4cd1c23 100644
@@ -8,6 +8,7 @@
  * tracking branch.  Return the name of the remote if such a branch
  * exists, NULL otherwise.
  */
-extern const char *unique_tracking_name(const char *name, struct object_id *oid);
+extern const char *unique_tracking_name(const char *name,
+					struct object_id *oid);
 
 #endif /* CHECKOUT_H */

2.17.0.290.gded63e768a

Add an *_INIT macro for the tracking_name_data similar to what exists elsewhere in the codebase, e.g. OID_ARRAY_INIT in sha1-array.h. This will make it more idiomatic in later changes to add more fields to the struct & its initialization macro.

---
 checkout.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index bdefc888ba..80e430cda8 100644
@@ -10,6 +10,8 @@ struct tracking_name_data {
 	int unique;
 };
 
+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 1 }
+
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {
 	struct tracking_name_data *cb = cb_data;

@@ -32,7 +34,7 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 
 const char *unique_tracking_name(const char *name, struct object_id *oid)
 {
-	struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+	struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);

2.17.0.290.gded63e768a

Internally track how many matches we find in the check_tracking_name() callback. Nothing uses this now, but it will be made use of in a later change.

---
 checkout.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

index 80e430cda8..7662a39a62 100644
@@ -7,10 +7,10 @@ struct tracking_name_data {
 	/* const */ char *src_ref;
 	char *dst_ref;
 	struct object_id *dst_oid;
-	int unique;
+	int num_matches;
 };
 
-#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 1 }
+#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0 }
 
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {

@@ -23,9 +23,9 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 		free(query.dst);
 		return 0;
 	}
+	cb->num_matches++;
 	if (cb->dst_ref) {
 		free(query.dst);
-		cb->unique = 0;
 		return 0;
 	}
 	cb->dst_ref = query.dst;

@@ -39,7 +39,7 @@ const char *unique_tracking_name(const char *name, struct object_id *oid)
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
 	free(cb_data.src_ref);
-	if (cb_data.unique)
+	if (cb_data.num_matches == 1)
 		return cb_data.dst_ref;
 	free(cb_data.dst_ref);
 	return NULL;

2.17.0.290.gded63e768a

Pass the previously added "num_matches" struct value up to the callers of unique_tracking_name(). This will allow callers to optionally print better error messages in a later change.

---
 builtin/checkout.c | 10 +++++++---
 builtin/worktree.c |  4 ++--
 checkout.c         |  5 ++++-
 checkout.h         |  3 ++-
 4 files changed, 15 insertions(+), 7 deletions(-)

index 2e1d2376d2..72457fb7d5 100644
@@ -878,7 +878,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 				int dwim_new_local_branch_ok,
 				struct branch_info *new_branch_info,
 				struct checkout_opts *opts,
-				struct object_id *rev)
+				struct object_id *rev,
+				int *dwim_remotes_matched)
 {
 	struct tree **source_tree = &opts->source_tree;
 	const char **new_branch = &opts->new_branch;

@@ -972,7 +973,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 			recover_with_dwim = 0;
 
 		if (recover_with_dwim) {
-			const char *remote = unique_tracking_name(arg, rev);
+			const char *remote = unique_tracking_name(arg, rev,
+								  dwim_remotes_matched);
 			if (remote) {
 				*new_branch = arg;
 				arg = remote;

@@ -1109,6 +1111,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct branch_info new_branch_info;
 	char *conflict_style = NULL;
 	int dwim_new_local_branch = 1;
+	int dwim_remotes_matched = 0;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
 		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),

@@ -1219,7 +1222,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			opts.track == BRANCH_TRACK_UNSPECIFIED &&
 			!opts.new_branch;
 		int n = parse_branchname_arg(argc, argv, dwim_ok,
-					     &new_branch_info, &opts, &rev);
+					     &new_branch_info, &opts, &rev,
+					     &dwim_remotes_matched);
 		argv += n;
 		argc -= n;
 	}

index 5c7d2bb180..a763dbdccb 100644
@@ -412,7 +412,7 @@ static const char *dwim_branch(const char *path, const char **new_branch)
 	if (guess_remote) {
 		struct object_id oid;
 		const char *remote =
-			unique_tracking_name(*new_branch, &oid);
+			unique_tracking_name(*new_branch, &oid, NULL);
 		return remote;
 	}
 	return NULL;

@@ -484,7 +484,7 @@ static int add(int ac, const char **av, const char *prefix)
 
 		commit = lookup_commit_reference_by_name(branch);
 		if (!commit) {
-			remote = unique_tracking_name(branch, &oid);
+			remote = unique_tracking_name(branch, &oid, NULL);
 			if (remote) {
 				new_branch = branch;
 				branch = remote;

index 7662a39a62..ee3a7e9c05 100644
@@ -32,12 +32,15 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
 	return 0;
 }
 
-const char *unique_tracking_name(const char *name, struct object_id *oid)
+const char *unique_tracking_name(const char *name, struct object_id *oid,
+				 int *dwim_remotes_matched)
 {
 	struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
 	cb_data.src_ref = xstrfmt("refs/heads/%s", name);
 	cb_data.dst_oid = oid;
 	for_each_remote(check_tracking_name, &cb_data);
+	if (dwim_remotes_matched)
+		*dwim_remotes_matched = cb_data.num_matches;
 	free(cb_data.src_ref);
 	if (cb_data.num_matches == 1)
 		return cb_data.dst_ref;

index 4cd4cd1c23..6b2073310c 100644
@@ -9,6 +9,7 @@
  * exists, NULL otherwise.
  */
 extern const char *unique_tracking_name(const char *name,
-					struct object_id *oid);
+					struct object_id *oid,
+					int *dwim_remotes_matched);
 
 #endif /* CHECKOUT_H */

2.17.0.290.gded63e768a

There is no point in doing this right now, but in later change the "ret" variable will be inspected. This change makes that meaningful change smaller.

---
 builtin/checkout.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

index 72457fb7d5..8c93c55cbc 100644
@@ -1265,8 +1265,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	}
 
 	UNLEAK(opts);
-	if (opts.patch_mode || opts.pathspec.nr)
-		return checkout_paths(&opts, new_branch_info.name);
-	else
+	if (opts.patch_mode || opts.pathspec.nr) {
+		int ret = checkout_paths(&opts, new_branch_info.name);
+		return ret;
+	} else {
 		return checkout_branch(&opts, &new_branch_info);
+	}
 }