Merge branch 'mm/checkout-auto-track-fix'
authorJunio C Hamano <gitster@pobox.com>
Wed, 30 Oct 2013 19:10:16 +0000 (12:10 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 30 Oct 2013 19:10:16 +0000 (12:10 -0700)
"git checkout topic", when there is not yet a local "topic" branch
but there is a unique remote-tracking branch for a remote "topic"
branch, pretended as if "git checkout -t -b topic remote/$r/topic"
(for that unique remote $r) was run. This hack however was not
implemented for "git checkout topic --".

* mm/checkout-auto-track-fix:
checkout: proper error message on 'git checkout foo bar --'
checkout: allow dwim for branch creation for "git checkout $branch --"

1  2 
builtin/checkout.c
t/t2024-checkout-dwim.sh
diff --combined builtin/checkout.c
index 0f573970372664a16024e83ed9733c94eeee3b50,e5103b9c9558d227a0a21c973fdbef456744e2a5..54f80bd38a7413390ed2093a626b129a1a3e8e7f
@@@ -46,7 -46,7 +46,7 @@@ struct checkout_opts 
  
        int branch_exists;
        const char *prefix;
 -      const char **pathspec;
 +      struct pathspec pathspec;
        struct tree *source_tree;
  };
  
@@@ -83,9 -83,12 +83,9 @@@ static int update_some(const unsigned c
        return 0;
  }
  
 -static int read_tree_some(struct tree *tree, const char **pathspec)
 +static int read_tree_some(struct tree *tree, const struct pathspec *pathspec)
  {
 -      struct pathspec ps;
 -      init_pathspec(&ps, pathspec);
 -      read_tree_recursive(tree, "", 0, 0, &ps, update_some, NULL);
 -      free_pathspec(&ps);
 +      read_tree_recursive(tree, "", 0, 0, pathspec, update_some, NULL);
  
        /* update the index with the given tree's info
         * for all args, expanding wildcards, and exit
@@@ -94,7 -97,7 +94,7 @@@
        return 0;
  }
  
 -static int skip_same_name(struct cache_entry *ce, int pos)
 +static int skip_same_name(const struct cache_entry *ce, int pos)
  {
        while (++pos < active_nr &&
               !strcmp(active_cache[pos]->name, ce->name))
        return pos;
  }
  
 -static int check_stage(int stage, struct cache_entry *ce, int pos)
 +static int check_stage(int stage, const struct cache_entry *ce, int pos)
  {
        while (pos < active_nr &&
               !strcmp(active_cache[pos]->name, ce->name)) {
                return error(_("path '%s' does not have their version"), ce->name);
  }
  
 -static int check_stages(unsigned stages, struct cache_entry *ce, int pos)
 +static int check_stages(unsigned stages, const struct cache_entry *ce, int pos)
  {
        unsigned seen = 0;
        const char *name = ce->name;
@@@ -225,6 -228,8 +225,6 @@@ static int checkout_paths(const struct 
        int flag;
        struct commit *head;
        int errs = 0;
 -      int stage = opts->writeout_stage;
 -      int merge = opts->merge;
        int newfd;
        struct lock_file *lock_file;
  
  
        if (opts->patch_mode)
                return run_add_interactive(revision, "--patch=checkout",
 -                                         opts->pathspec);
 +                                         &opts->pathspec);
  
        lock_file = xcalloc(1, sizeof(struct lock_file));
  
        newfd = hold_locked_index(lock_file, 1);
 -      if (read_cache_preload(opts->pathspec) < 0)
 +      if (read_cache_preload(&opts->pathspec) < 0)
                return error(_("corrupt index file"));
  
        if (opts->source_tree)
 -              read_tree_some(opts->source_tree, opts->pathspec);
 +              read_tree_some(opts->source_tree, &opts->pathspec);
  
 -      for (pos = 0; opts->pathspec[pos]; pos++)
 -              ;
 -      ps_matched = xcalloc(1, pos);
 +      ps_matched = xcalloc(1, opts->pathspec.nr);
  
        /*
         * Make sure all pathspecs participated in locating the paths
                 * match_pathspec() for _all_ entries when
                 * opts->source_tree != NULL.
                 */
 -              if (match_pathspec(opts->pathspec, ce->name, ce_namelen(ce),
 +              if (match_pathspec_depth(&opts->pathspec, ce->name, ce_namelen(ce),
                                   0, ps_matched))
                        ce->ce_flags |= CE_MATCHED;
        }
  
 -      if (report_path_error(ps_matched, opts->pathspec, opts->prefix)) {
 +      if (report_path_error(ps_matched, &opts->pathspec, opts->prefix)) {
                free(ps_matched);
                return 1;
        }
  
        /* Any unmerged paths? */
        for (pos = 0; pos < active_nr; pos++) {
 -              struct cache_entry *ce = active_cache[pos];
 +              const struct cache_entry *ce = active_cache[pos];
                if (ce->ce_flags & CE_MATCHED) {
                        if (!ce_stage(ce))
                                continue;
                        if (opts->force) {
                                warning(_("path '%s' is unmerged"), ce->name);
 -                      } else if (stage) {
 -                              errs |= check_stage(stage, ce, pos);
 +                      } else if (opts->writeout_stage) {
 +                              errs |= check_stage(opts->writeout_stage, ce, pos);
                        } else if (opts->merge) {
                                errs |= check_stages((1<<2) | (1<<3), ce, pos);
                        } else {
                                errs |= checkout_entry(ce, &state, NULL);
                                continue;
                        }
 -                      if (stage)
 -                              errs |= checkout_stage(stage, ce, pos, &state);
 -                      else if (merge)
 +                      if (opts->writeout_stage)
 +                              errs |= checkout_stage(opts->writeout_stage, ce, pos, &state);
 +                      else if (opts->merge)
                                errs |= checkout_merged(pos, &state);
                        pos = skip_same_name(ce, pos) - 1;
                }
@@@ -580,7 -587,7 +580,7 @@@ static void update_refs_for_switch(cons
                                   struct branch_info *new)
  {
        struct strbuf msg = STRBUF_INIT;
 -      const char *old_desc;
 +      const char *old_desc, *reflog_msg;
        if (opts->new_branch) {
                if (opts->new_orphan_branch) {
                        if (opts->new_branch_log && !log_all_ref_updates) {
        old_desc = old->name;
        if (!old_desc && old->commit)
                old_desc = sha1_to_hex(old->commit->object.sha1);
 -      strbuf_addf(&msg, "checkout: moving from %s to %s",
 -                  old_desc ? old_desc : "(invalid)", new->name);
 +
 +      reflog_msg = getenv("GIT_REFLOG_ACTION");
 +      if (!reflog_msg)
 +              strbuf_addf(&msg, "checkout: moving from %s to %s",
 +                      old_desc ? old_desc : "(invalid)", new->name);
 +      else
 +              strbuf_insert(&msg, 0, reflog_msg, strlen(reflog_msg));
  
        if (!strcmp(new->name, "HEAD") && !new->path && !opts->force_detach) {
                /* Nothing to do. */
@@@ -836,16 -838,13 +836,16 @@@ static int check_tracking_name(struct r
        memset(&query, 0, sizeof(struct refspec));
        query.src = cb->src_ref;
        if (remote_find_tracking(remote, &query) ||
 -          get_sha1(query.dst, cb->dst_sha1))
 +          get_sha1(query.dst, cb->dst_sha1)) {
 +              free(query.dst);
                return 0;
 +      }
        if (cb->dst_ref) {
 +              free(query.dst);
                cb->unique = 0;
                return 0;
        }
 -      cb->dst_ref = xstrdup(query.dst);
 +      cb->dst_ref = query.dst;
        return 0;
  }
  
@@@ -873,7 -872,9 +873,9 @@@ static int parse_branchname_arg(int arg
        int argcount = 0;
        unsigned char branch_rev[20];
        const char *arg;
-       int has_dash_dash;
+       int dash_dash_pos;
+       int has_dash_dash = 0;
+       int i;
  
        /*
         * case 1: git checkout <ref> -- [<paths>]
         *
         *   everything after the '--' must be paths.
         *
-        * case 3: git checkout <something> [<paths>]
+        * case 3: git checkout <something> [--]
         *
-        *   With no paths, if <something> is a commit, that is to
-        *   switch to the branch or detach HEAD at it.  As a special case,
-        *   if <something> is A...B (missing A or B means HEAD but you can
-        *   omit at most one side), and if there is a unique merge base
-        *   between A and B, A...B names that merge base.
+        *   (a) If <something> is a commit, that is to
+        *       switch to the branch or detach HEAD at it.  As a special case,
+        *       if <something> is A...B (missing A or B means HEAD but you can
+        *       omit at most one side), and if there is a unique merge base
+        *       between A and B, A...B names that merge base.
         *
-        *   With no paths, if <something> is _not_ a commit, no -t nor -b
-        *   was given, 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.
+        *   (b) If <something> is _not_ a commit, either "--" is present
+        *       or <something> is not a path, no -t nor -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.
         *
-        *   Otherwise <something> shall not be ambiguous.
+        *   (c) Otherwise, if "--" is present, treat it like case (1).
+        *
+        *   (d) Otherwise :
+        *       - if it's a reference, treat it like case (1)
+        *       - else if it's a path, treat it like case (2)
+        *       - else: fail.
+        *
+        * case 4: git checkout <something> <paths>
+        *
+        *   The first argument must not be ambiguous.
         *   - If it's *only* a reference, treat it like case (1).
         *   - If it's only a path, treat it like case (2).
         *   - else: fail.
        if (!argc)
                return 0;
  
-       if (!strcmp(argv[0], "--"))     /* case (2) */
-               return 1;
        arg = argv[0];
-       has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
+       dash_dash_pos = -1;
+       for (i = 0; i < argc; i++) {
+               if (!strcmp(argv[i], "--")) {
+                       dash_dash_pos = i;
+                       break;
+               }
+       }
+       if (dash_dash_pos == 0)
+               return 1; /* case (2) */
+       else if (dash_dash_pos == 1)
+               has_dash_dash = 1; /* case (3) or (1) */
+       else if (dash_dash_pos >= 2)
+               die(_("only one reference expected, %d given."), dash_dash_pos);
  
        if (!strcmp(arg, "-"))
                arg = "@{-1}";
  
        if (get_sha1_mb(arg, rev)) {
-               if (has_dash_dash)          /* case (1) */
-                       die(_("invalid reference: %s"), arg);
-               if (dwim_new_local_branch_ok &&
-                   !check_filename(NULL, arg) &&
-                   argc == 1) {
+               /*
+                * Either case (3) or (4), with <something> not being
+                * a commit, or an attempt to use case (1) with an
+                * invalid ref.
+                *
+                * It's likely an error, but we need to find out if
+                * we should auto-create the branch, case (3).(b).
+                */
+               int recover_with_dwim = dwim_new_local_branch_ok;
+               if (check_filename(NULL, arg) && !has_dash_dash)
+                       recover_with_dwim = 0;
+               /*
+                * Accept "git checkout foo" and "git checkout foo --"
+                * as candidates for dwim.
+                */
+               if (!(argc == 1 && !has_dash_dash) &&
+                   !(argc == 2 && has_dash_dash))
+                       recover_with_dwim = 0;
+               if (recover_with_dwim) {
                        const char *remote = unique_tracking_name(arg, rev);
-                       if (!remote)
-                               return argcount;
-                       *new_branch = arg;
-                       arg = remote;
-                       /* DWIMmed to create local branch */
-               } else {
+                       if (remote) {
+                               *new_branch = arg;
+                               arg = remote;
+                               /* DWIMmed to create local branch, case (3).(b) */
+                       } else {
+                               recover_with_dwim = 0;
+                       }
+               }
+               if (!recover_with_dwim) {
+                       if (has_dash_dash)
+                               die(_("invalid reference: %s"), arg);
                        return argcount;
                }
        }
  
        if (!*source_tree)                   /* case (1): want a tree */
                die(_("reference is not a tree: %s"), arg);
-       if (!has_dash_dash) {/* case (3 -> 1) */
+       if (!has_dash_dash) {/* case (3).(d) -> (1) */
                /*
                 * Do not complain the most common case
                 *      git checkout branch
@@@ -995,7 -1037,7 +1038,7 @@@ static int switch_unborn_to_new_branch(
  static int checkout_branch(struct checkout_opts *opts,
                           struct branch_info *new)
  {
 -      if (opts->pathspec)
 +      if (opts->pathspec.nr)
                die(_("paths cannot be used with switching branches"));
  
        if (opts->patch_mode)
@@@ -1049,8 -1091,8 +1092,8 @@@ int cmd_checkout(int argc, const char *
                           N_("create and checkout a new branch")),
                OPT_STRING('B', NULL, &opts.new_branch_force, N_("branch"),
                           N_("create/reset and checkout a branch")),
 -              OPT_BOOLEAN('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
 -              OPT_BOOLEAN(0, "detach", &opts.force_detach, N_("detach the HEAD at named commit")),
 +              OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
 +              OPT_BOOL(0, "detach", &opts.force_detach, N_("detach the HEAD at named commit")),
                OPT_SET_INT('t', "track",  &opts.track, N_("set upstream info for new branch"),
                        BRANCH_TRACK_EXPLICIT),
                OPT_STRING(0, "orphan", &opts.new_orphan_branch, N_("new branch"), N_("new unparented branch")),
                OPT_SET_INT('3', "theirs", &opts.writeout_stage, N_("checkout their version for unmerged files"),
                            3),
                OPT__FORCE(&opts.force, N_("force checkout (throw away local modifications)")),
 -              OPT_BOOLEAN('m', "merge", &opts.merge, N_("perform a 3-way merge with the new branch")),
 -              OPT_BOOLEAN(0, "overwrite-ignore", &opts.overwrite_ignore, N_("update ignored files (default)")),
 +              OPT_BOOL('m', "merge", &opts.merge, N_("perform a 3-way merge with the new branch")),
 +              OPT_BOOL(0, "overwrite-ignore", &opts.overwrite_ignore, N_("update ignored files (default)")),
                OPT_STRING(0, "conflict", &conflict_style, N_("style"),
                           N_("conflict style (merge or diff3)")),
 -              OPT_BOOLEAN('p', "patch", &opts.patch_mode, N_("select hunks interactively")),
 +              OPT_BOOL('p', "patch", &opts.patch_mode, N_("select hunks interactively")),
                OPT_BOOL(0, "ignore-skip-worktree-bits", &opts.ignore_skipworktree,
                         N_("do not limit pathspecs to sparse entries only")),
 -              { OPTION_BOOLEAN, 0, "guess", &dwim_new_local_branch, NULL,
 -                N_("second guess 'git checkout no-such-branch'"),
 -                PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
 +              OPT_HIDDEN_BOOL(0, "guess", &dwim_new_local_branch,
 +                              N_("second guess 'git checkout no-such-branch'")),
                OPT_END(),
        };
  
        }
  
        if (argc) {
 -              opts.pathspec = get_pathspec(prefix, argv);
 +              parse_pathspec(&opts.pathspec, 0,
 +                             opts.patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
 +                             prefix, argv);
  
 -              if (!opts.pathspec)
 +              if (!opts.pathspec.nr)
                        die(_("invalid path specification"));
  
                /*
                strbuf_release(&buf);
        }
  
 -      if (opts.patch_mode || opts.pathspec)
 +      if (opts.patch_mode || opts.pathspec.nr)
                return checkout_paths(&opts, new.name);
        else
                return checkout_branch(&opts, &new);
diff --combined t/t2024-checkout-dwim.sh
index 094b92ef489156980dc1429136d5dad25d01ac7d,36be80fd41a7301fad657b33f028af844add60f8..6ecb559465b43e528a5cb1511b758cc48d940e92
@@@ -104,7 -104,7 +104,7 @@@ test_expect_success 'setup more remote
                cd repo_c &&
                test_commit c_master &&
                git checkout -b bar &&
 -              test_commit c_bar
 +              test_commit c_bar &&
                git checkout -b spam &&
                test_commit c_spam
        ) &&
                cd repo_d &&
                test_commit d_master &&
                git checkout -b baz &&
 -              test_commit f_baz
 +              test_commit d_baz &&
                git checkout -b eggs &&
 -              test_commit c_eggs
 +              test_commit d_eggs
        ) &&
        git remote add repo_c repo_c &&
        git config remote.repo_c.fetch \
@@@ -164,4 -164,25 +164,25 @@@ test_expect_success 'checkout of branc
        test_branch_upstream eggs repo_d eggs
  '
  
+ test_expect_success 'checkout of branch with a file having the same name fails' '
+       git checkout -B master &&
+       test_might_fail git branch -D spam &&
+       >spam &&
+       test_must_fail git checkout spam &&
+       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 &&
+       test_might_fail git branch -D spam &&
+       >spam &&
+       git checkout spam -- &&
+       test_branch spam &&
+       test_cmp_rev refs/remotes/extra_dir/repo_c/extra_dir/spam HEAD &&
+       test_branch_upstream spam repo_c spam
+ '
  test_done