The patch is twofold: it moves the option consistency checks just under
the parse_options call so that it doesn't get in the way of the tree
reference vs. pathspecs desambiguation.
The other part rewrites the way to understand arguments so that when
git-checkout fails it does with an understandable message. Compared to the
previous behavior we now have:
- a better error message when doing:
git checkout <blob reference> --
now complains about the reference not pointing to a tree, instead of
things like:
error: pathspec <blob reference> did not match any file(s) known to git.
error: pathspec '--' did not match any file(s) known to git.
you what it spitted before.
- a better error message when doing:
git checkout <path> --
It now complains about <path> not being a reference instead of the
completely obscure:
error: pathspec '--' did not match any file(s) known to git.
- an error when -- wasn't used and the first argument is ambiguous.
Signed-off-by: Pierre Habouzit <madcoder@xxxxxxxxxx>
---
builtin-checkout.c | 71 +++++++++++++++++++++++++++++++----------
t/t2010-checkout-ambiguous.sh | 39 ++++++++++++++++++++++
2 files changed, 93 insertions(+), 17 deletions(-)
create mode 100755 t/t2010-checkout-ambiguous.sh
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 9cadf9c..e9ae834 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -430,6 +430,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
OPT_END(),
};
+ int has_dash_dash;
memset(&opts, 0, sizeof(opts));
memset(&new, 0, sizeof(new));
@@ -440,11 +441,52 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+
+ if (!opts.new_branch && (opts.track != git_branch_track))
+ die("git checkout: --track and --no-track require -b");
+
+ if (opts.force && opts.merge)
+ die("git checkout: -f and -m are incompatible");
+
+ /*
+ * case 1: git checkout <ref> -- [<paths>]
+ *
+ * <ref> must be a valid tree, everything after the '--' must be
+ * a path.
+ *
+ * case 2: git checkout -- [<paths>]
+ *
+ * everything after the '--' must be paths.
+ *
+ * case 3: git checkout <something> [<paths>]
+ *
+ * <something> shall 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) {
+ if (!strcmp(argv[0], "--")) { /* case (2) */
+ argv++;
+ argc--;
+ goto no_reference;
+ }
+
arg = argv[0];
- if (get_sha1(arg, rev))
- ;
- else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
+ has_dash_dash = argc > 1 && !strcmp(argv[1], "--");
+
+ if (get_sha1(arg, rev)) {
+ if (has_dash_dash) /* case (1) */
+ die("invalid reference: %s", arg);
+ goto no_reference; /* case (3 -> 2) */
+ }
+
+ /* we can't end up being in (2) anymore, eat the argument */
+ argv++;
+ argc--;
+
+ if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
new.name = arg;
setup_branch_path(&new);
if (resolve_ref(new.path, rev, 1, NULL))
@@ -453,25 +495,20 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
new.path = NULL;
parse_commit(new.commit);
source_tree = new.commit->tree;
- argv++;
- argc--;
- } else if ((source_tree = parse_tree_indirect(rev))) {
+ } else
+ source_tree = parse_tree_indirect(rev);
+
+ if (!source_tree) /* case (1): want a tree */
+ die("reference is not a tree: %s", arg);
+ if (!has_dash_dash) /* case (3 -> 1) */
+ verify_non_filename(NULL, arg);
+ else {
argv++;
argc--;
}
}
- if (argc && !strcmp(argv[0], "--")) {
- argv++;
- argc--;
- }
-
- if (!opts.new_branch && (opts.track != git_branch_track))
- die("git checkout: --track and --no-track require -b");
-
- if (opts.force && opts.merge)
- die("git checkout: -f and -m are incompatible");
-
+no_reference:
if (argc) {
const char **pathspec = get_pathspec(prefix, argv);
diff --git a/t/t2010-checkout-ambiguous.sh b/t/t2010-checkout-ambiguous.sh
new file mode 100755
index 0000000..50d1f43
--- /dev/null
+++ b/t/t2010-checkout-ambiguous.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+test_description='checkout and pathspecs/refspecs ambiguities'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo hello >world &&
+ echo hello >all &&
+ git add all world &&
+ git commit -m initial &&
+ git branch world
+'
+
+test_expect_success 'reference must be a tree' '
+ test_must_fail git checkout $(git hash-object ./all) --
+'
+
+test_expect_success 'branch switching' '
+ test "refs/heads/master" = "$(git symbolic-ref HEAD)" &&
+ git checkout world -- &&
+ test "refs/heads/world" = "$(git symbolic-ref HEAD)"
+'
+
+test_expect_success 'checkout world from the index' '
+ echo bye > world &&
+ git checkout -- world &&
+ git diff --exit-code --quiet
+'
+
+test_expect_success 'non ambiguous call' '
+ git checkout all
+'
+
+test_expect_success 'ambiguous call' '
+ test_must_fail git checkout world
+'
+
+test_done
--
1.6.0.rc0.155.g8e50b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
