From c8cbf20cc2a54e07705f14ddef901d676b5410ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 14:40:42 +0000 Subject: checkout tests: index should be clean after dwim checkout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 '", 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. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh index 3e5ac81..d2fb4e2 100755 --- a/t/t2024-checkout-dwim.sh +++ b/t/t2024-checkout-dwim.sh @@ -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 -- 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 -- 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 ' -- cgit v0.10.2-6-g49f6 From 17b44aebb59379efd30e5c7c81b66d254d48066b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 14:40:43 +0000 Subject: checkout.h: wrap the arguments to unique_tracking_name() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The line was too long already, and will be longer still when a later change adds another argument. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/checkout.h b/checkout.h index 9980711..4cd4cd1 100644 --- a/checkout.h +++ b/checkout.h @@ -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 */ -- cgit v0.10.2-6-g49f6 From e417151b24bcf1277a48fc6e9a5c184998402428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 14:40:44 +0000 Subject: checkout.c: introduce an *_INIT macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/checkout.c b/checkout.c index bdefc88..80e430c 100644 --- a/checkout.c +++ b/checkout.c @@ -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); -- cgit v0.10.2-6-g49f6 From e4d2d55ae46216872c9eaa3b9d0072aa8361d5d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 14:40:45 +0000 Subject: checkout.c: change "unique" member to "num_matches" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/checkout.c b/checkout.c index 80e430c..7662a39 100644 --- a/checkout.c +++ b/checkout.c @@ -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; -- cgit v0.10.2-6-g49f6 From 3c87aa946a9ffc31cf1355b11e63df7c3315a2f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 14:40:46 +0000 Subject: checkout: pass the "num_matches" up to callers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/builtin/checkout.c b/builtin/checkout.c index 2e1d237..72457fb 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -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; } diff --git a/builtin/worktree.c b/builtin/worktree.c index 5c7d2bb..a763dbd 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -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; diff --git a/checkout.c b/checkout.c index 7662a39..ee3a7e9 100644 --- a/checkout.c +++ b/checkout.c @@ -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; diff --git a/checkout.h b/checkout.h index 4cd4cd1..6b20733 100644 --- a/checkout.h +++ b/checkout.h @@ -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 */ -- cgit v0.10.2-6-g49f6 From 1c550553c589b1bbc6f55dd074f9db55952d3431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 14:40:47 +0000 Subject: builtin/checkout.c: use "ret" variable for return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/builtin/checkout.c b/builtin/checkout.c index 72457fb..8c93c55 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -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); + } } -- cgit v0.10.2-6-g49f6 From ad8d5104b42108851b082d895018655ad5f9e4f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 14:40:48 +0000 Subject: checkout: add advice for ambiguous "checkout " MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the "checkout" documentation describes: If is not found but there does exist a tracking branch in exactly one remote (call it ) with a matching name, treat as equivalent to [...] / Note that the "error: pathspec[...]" message is still printed. This is because whatever else checkout may have tried earlier, its final fallback is to try to resolve the argument as a path. E.g. in this case: $ ./git --exec-path=$PWD checkout master pu error: pathspec 'master' did not match any file(s) known to git. error: pathspec 'pu' did not match any file(s) known to git. There we don't print the "hint:" implicitly due to earlier logic around the DWIM fallback. That fallback is only used if it looks like we have one argument that might be a branch. I can't think of an intrinsic reason for why we couldn't in some future change skip printing the "error: pathspec[...]" error. However, to do so we'd need to pass something down to checkout_paths() to make it suppress printing an error on its own, and for us to be confident that we're not silencing cases where those errors are meaningful. I don't think that's worth it since determining whether that's the case could easily change due to future changes in the checkout logic. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/Documentation/config.txt b/Documentation/config.txt index ab641bf..dfc0413 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -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. diff --git a/advice.c b/advice.c index 370a56d..75e7ded 100644 --- a/advice.c +++ b/advice.c @@ -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 } diff --git a/advice.h b/advice.h index 9f5064e..4d11d51 100644 --- a/advice.h +++ b/advice.h @@ -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))) diff --git a/builtin/checkout.c b/builtin/checkout.c index 8c93c55..baa0274 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -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 [] "), @@ -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(_("'%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/"), + argv[0], + dwim_remotes_matched); return ret; } else { return checkout_branch(&opts, &new_branch_info); diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh index d2fb4e2..082147a 100755 --- a/t/t2024-checkout-dwim.sh +++ b/t/t2024-checkout-dwim.sh @@ -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 && -- cgit v0.10.2-6-g49f6 From 8d7b558baebe3abbbad4973ce1e1f87a7da17f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 5 Jun 2018 14:40:49 +0000 Subject: checkout & worktree: introduce checkout.defaultRemote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 git@github.com: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 git@github.com:trast/tbdiff.git && cd tbdiff && git branch -m topic && git remote add avar git@github.com: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: '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/ hint: hint: If you'd like to always have checkouts of an ambiguous 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 dwim", 2017-11-26) which added it to git-worktree. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano diff --git a/Documentation/config.txt b/Documentation/config.txt index dfc0413..aef2769 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -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..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 ' and only have one + remote, it may implicitly fall back on checking out and + tracking e.g. 'origin/'. This stops working as soon + as you have more than one remote with a '' + 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 +' will checkout the '' 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. diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index ca5fc9c..9db0292 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -38,6 +38,15 @@ equivalent to $ git checkout -b --track / ------------ + +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 `` isn't +unique across all remotes. Set it to +e.g. `checkout.defaultRemote=origin` to always checkout remote +branches from there if `` is ambiguous but exists on the +'origin' remote. See also `checkout.defaultRemote` in +linkgit:git-config[1]. ++ You could omit , 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, diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index afc6576..9c26be4 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -60,6 +60,15 @@ with a matching name, treat as equivalent to: $ git worktree add --track -b / ------------ + +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 `` isn't +unique across all remotes. Set it to +e.g. `checkout.defaultRemote=origin` to always checkout remote +branches from there if `` is ambiguous but exists on the +'origin' remote. See also `checkout.defaultRemote` in +linkgit:git-config[1]. ++ If `` is omitted and neither `-b` nor `-B` nor `--detach` used, then, as a convenience, the new worktree is associated with a branch (call it ``) named after `$(basename )`. If `` diff --git a/builtin/checkout.c b/builtin/checkout.c index baa0274..5b357e9 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -912,8 +912,10 @@ static int parse_branchname_arg(int argc, const char **argv, * (b) If is _not_ a commit, either "--" is present * or is not a path, no -t or -b was given, and * and there is a tracking branch whose name is - * in one and only one remote, then this is a short-hand to - * fork local 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 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/"), + " git checkout --track origin/\n" + "\n" + "If you'd like to always have checkouts of an ambiguous 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; diff --git a/checkout.c b/checkout.c index ee3a7e9..c72e9f9 100644 --- a/checkout.c +++ b/checkout.c @@ -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; } diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh index 082147a..26dc3f1 100755 --- a/t/t2024-checkout-dwim.sh +++ b/t/t2024-checkout-dwim.sh @@ -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 do 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' ' diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index d2e49f7..be6e093 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -402,6 +402,27 @@ test_expect_success '"add" dwims' ' ) ' +test_expect_success '"add" 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 && -- cgit v0.10.2-6-g49f6