Merge branch 'ab/checkout-default-remote'
authorJunio C Hamano <gitster@pobox.com>
Thu, 2 Aug 2018 22:30:41 +0000 (15:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 2 Aug 2018 22:30:41 +0000 (15:30 -0700)
"git checkout" and "git worktree add" learned to honor
checkout.defaultRemote when auto-vivifying a local branch out of a
remote tracking branch in a repository with multiple remotes that
have tracking branches that share the same names.

* ab/checkout-default-remote:
checkout & worktree: introduce checkout.defaultRemote
checkout: add advice for ambiguous "checkout <branch>"
builtin/checkout.c: use "ret" variable for return
checkout: pass the "num_matches" up to callers
checkout.c: change "unique" member to "num_matches"
checkout.c: introduce an *_INIT macro
checkout.h: wrap the arguments to unique_tracking_name()
checkout tests: index should be clean after dwim checkout

Documentation/config.txt
Documentation/git-checkout.txt
Documentation/git-worktree.txt
advice.c
advice.h
builtin/checkout.c
builtin/worktree.c
checkout.c
checkout.h
t/t2024-checkout-dwim.sh
t/t2025-worktree-add.sh
index 3fdc6f0327fe8051bbf42c154fe0e56b06bfe88c..44df580a81c3076a89483c34f07b40192894c7fa 100644 (file)
@@ -344,6 +344,16 @@ 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. 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.
@@ -1104,6 +1114,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 ca5fc9c79887652e38e3d11dc86dba657fe585c0..9db02928c4634ea07c3950b1431bdf9981ec1ab9 100644 (file)
@@ -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 afc6576a14d56ea49e37d1251a5665bf77457f89..9c26be40f4412b5f0a9c478236b9d3c38fcb14ec 100644 (file)
@@ -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 52aa85bdfd9e9054c24445f259125bc4b81be038..3561cd64e9dab0a5b0c52d117253f37a5926f9c7 100644 (file)
--- a/advice.c
+++ b/advice.c
@@ -23,6 +23,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] = {
@@ -75,6 +76,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 7e9377864f8fca1051ce3fd27f3def62b8d234eb..ab24df0fd0d0c739f6f58bb2650bb4162ef4c7f2 100644 (file)
--- a/advice.h
+++ b/advice.h
@@ -23,6 +23,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 28627650cd66bb38a432397b2e6398b412cbf073..2de10d28c76e7616748ef37def3b6e4f43a4e7a7 100644 (file)
@@ -23,6 +23,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>"),
@@ -879,7 +880,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;
@@ -911,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).
         *
@@ -973,7 +977,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;
@@ -1110,6 +1115,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"),
@@ -1222,7 +1228,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;
        }
@@ -1264,8 +1271,26 @@ 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);
+               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/<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;
+       } else {
                return checkout_branch(&opts, &new_branch_info);
+       }
 }
index 5c7d2bb1807f942139b3ec41b426320e4b0cfc2a..a763dbdccb490ab6707c247d618203ffd37d4052 100644 (file)
@@ -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 bdefc888bae1516f0ddcf22264807d031af7c991..c72e9f9773e173c0dbcb5de88bde54d5434cf808 100644 (file)
@@ -2,14 +2,20 @@
 #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 unique;
+       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, NULL, NULL, NULL }
+
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {
        struct tracking_name_data *cb = cb_data;
@@ -21,24 +27,45 @@ static int check_tracking_name(struct remote *remote, void *cb_data)
                free(query.dst);
                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);
-               cb->unique = 0;
                return 0;
        }
        cb->dst_ref = query.dst;
        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 = { NULL, NULL, NULL, 1 };
+       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.unique)
+       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 998071117952de27f2ec0c16d7b3d49abaa952bc..6b2073310c4a6a735a557ab0883c7bb045c2c458 100644 (file)
@@ -8,6 +8,8 @@
  * 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,
+                                       int *dwim_remotes_matched);
 
 #endif /* CHECKOUT_H */
index 3e5ac81bd29bf6aded0d0fa643dca448a281d481..26dc3f1fc0c25df1b359b5d56aece9f171344f5a 100755 (executable)
@@ -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,15 +71,47 @@ 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_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 &&
+       # 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' '
        git checkout -B master &&
        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 +122,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 +130,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 +140,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 +170,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 +204,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 +216,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 +256,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 +267,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
 '
index d2e49f763263333e8e269051686d3b8b2cae44ba..be6e093142cdbeb18fa0ff155f3adf1e7a566ca2 100755 (executable)
@@ -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 &&