From 859fdabaede6733c98b1ca8df2fabce000522bf9 Mon Sep 17 00:00:00 2001 From: Pierre Habouzit Date: Wed, 23 Jul 2008 12:15:33 +0200 Subject: [PATCH] git-checkout: improve error messages, detect ambiguities. 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 -- now complains about the reference not pointing to a tree, instead of things like: error: pathspec did not match any file(s) known to git. error: pathspec '--' did not match any file(s) known to git. - a better error message when doing: git checkout -- It now complains about 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 (i.e. can be interpreted as both ref and as path). Signed-off-by: Pierre Habouzit Signed-off-by: Junio C Hamano --- builtin-checkout.c | 82 +++++++++++++++++++++++++++-------- t/t2010-checkout-ambiguous.sh | 50 +++++++++++++++++++++ 2 files changed, 115 insertions(+), 17 deletions(-) create mode 100755 t/t2010-checkout-ambiguous.sh diff --git a/builtin-checkout.c b/builtin-checkout.c index 9cadf9c299..411cc513c6 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,55 @@ 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 -- [] + * + * must be a valid tree, everything after the '--' must be + * a path. + * + * case 2: git checkout -- [] + * + * everything after the '--' must be paths. + * + * case 3: git checkout [] + * + * With no paths, if is a commit, that is to + * switch to the branch or detach HEAD at it. + * + * Otherwise 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 +498,28 @@ 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) */ + /* + * Do not complain the most common case + * git checkout branch + * even if there happen to be a file called 'branch'; + * it would be extremely annoying. + */ + if (argc) + 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 0000000000..7cc0a3582e --- /dev/null +++ b/t/t2010-checkout-ambiguous.sh @@ -0,0 +1,50 @@ +#!/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 'allow the most common case' ' + git checkout world && + test "refs/heads/world" = "$(git symbolic-ref HEAD)" +' + +test_expect_success 'check ambiguity' ' + test_must_fail git checkout world all +' + +test_expect_success 'disambiguate checking out from a tree-ish' ' + echo bye > world && + git checkout world -- world && + git diff --exit-code --quiet +' + +test_done