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 <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. - 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 (i.e. can be interpreted as both ref and as path). Signed-off-by: Pierre Habouzit <madcoder@debian.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Pierre Habouzit committed
Jul 23, 2008 at 12:15 UTC
859fdabaede6733c98b1ca8df2fabce000522bf9
2 files changed
+115
-17
builtin-checkout.c
+65
-17
@@ -430,6 +430,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
430
OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
431
OPT_END(),
432
};
433
+ int has_dash_dash;
434
435
memset(&opts, 0, sizeof(opts));
436
memset(&new, 0, sizeof(new));
@@ -440,11 +441,55 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
441
442
argc = parse_options(argc, argv, options, checkout_usage,
443
PARSE_OPT_KEEP_DASHDASH);
444
+
445
+ if (!opts.new_branch && (opts.track != git_branch_track))
446
+ die("git checkout: --track and --no-track require -b");
447
+
448
+ if (opts.force && opts.merge)
449
+ die("git checkout: -f and -m are incompatible");
450
+
451
+ /*
452
+ * case 1: git checkout <ref> -- [<paths>]
453
+ *
454
+ * <ref> must be a valid tree, everything after the '--' must be
455
+ * a path.
456
+ *
457
+ * case 2: git checkout -- [<paths>]
458
+ *
459
+ * everything after the '--' must be paths.
460
+ *
461
+ * case 3: git checkout <something> [<paths>]
462
+ *
463
+ * With no paths, if <something> is a commit, that is to
464
+ * switch to the branch or detach HEAD at it.
465
+ *
466
+ * Otherwise <something> shall not be ambiguous.
467
+ * - If it's *only* a reference, treat it like case (1).
468
+ * - If it's only a path, treat it like case (2).
469
+ * - else: fail.
470
+ *
471
+ */
472
if (argc) {
473
+ if (!strcmp(argv[0], "--")) { /* case (2) */
474
+ argv++;
475
+ argc--;
476
+ goto no_reference;
477
+ }
478
+
479
arg = argv[0];
445
- if (get_sha1(arg, rev))
446
- ;
447
- else if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
480
+ has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
481
+
482
+ if (get_sha1(arg, rev)) {
483
+ if (has_dash_dash) /* case (1) */
484
+ die("invalid reference: %s", arg);
485
+ goto no_reference; /* case (3 -> 2) */
486
+ }
487
+
488
+ /* we can't end up being in (2) anymore, eat the argument */
489
+ argv++;
490
+ argc--;
491
+
492
+ if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
493
new.name = arg;
494
setup_branch_path(&new);
495
if (resolve_ref(new.path, rev, 1, NULL))
@@ -453,25 +498,28 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
498
new.path = NULL;
499
parse_commit(new.commit);
500
source_tree = new.commit->tree;
456
- argv++;
457
- argc--;
458
- } else if ((source_tree = parse_tree_indirect(rev))) {
501
+ } else
502
+ source_tree = parse_tree_indirect(rev);
503
+
504
+ if (!source_tree) /* case (1): want a tree */
505
+ die("reference is not a tree: %s", arg);
506
+ if (!has_dash_dash) {/* case (3 -> 1) */
507
+ /*
508
+ * Do not complain the most common case
509
+ * git checkout branch
510
+ * even if there happen to be a file called 'branch';
511
+ * it would be extremely annoying.
512
+ */
513
+ if (argc)
514
+ verify_non_filename(NULL, arg);
515
+ }
516
+ else {
517
argv++;
518
argc--;
519
}
520
}
521
464
- if (argc && !strcmp(argv[0], "--")) {
465
- argv++;
466
- argc--;
467
- }
468
-
469
- if (!opts.new_branch && (opts.track != git_branch_track))
470
- die("git checkout: --track and --no-track require -b");
471
-
472
- if (opts.force && opts.merge)
473
- die("git checkout: -f and -m are incompatible");
474
-
522
+no_reference:
523
if (argc) {
524
const char **pathspec = get_pathspec(prefix, argv);
525
t/t2010-checkout-ambiguous.sh
new
+50
@@ -0,0 +1,50 @@
1
+#!/bin/sh
2
+
3
+test_description='checkout and pathspecs/refspecs ambiguities'
4
+
5
+. ./test-lib.sh
6
+
7
+test_expect_success 'setup' '
8
+ echo hello >world &&
9
+ echo hello >all &&
10
+ git add all world &&
11
+ git commit -m initial &&
12
+ git branch world
13
+'
14
+
15
+test_expect_success 'reference must be a tree' '
16
+ test_must_fail git checkout $(git hash-object ./all) --
17
+'
18
+
19
+test_expect_success 'branch switching' '
20
+ test "refs/heads/master" = "$(git symbolic-ref HEAD)" &&
21
+ git checkout world -- &&
22
+ test "refs/heads/world" = "$(git symbolic-ref HEAD)"
23
+'
24
+
25
+test_expect_success 'checkout world from the index' '
26
+ echo bye > world &&
27
+ git checkout -- world &&
28
+ git diff --exit-code --quiet
29
+'
30
+
31
+test_expect_success 'non ambiguous call' '
32
+ git checkout all
33
+'
34
+
35
+test_expect_success 'allow the most common case' '
36
+ git checkout world &&
37
+ test "refs/heads/world" = "$(git symbolic-ref HEAD)"
38
+'
39
+
40
+test_expect_success 'check ambiguity' '
41
+ test_must_fail git checkout world all
42
+'
43
+
44
+test_expect_success 'disambiguate checking out from a tree-ish' '
45
+ echo bye > world &&
46
+ git checkout world -- world &&
47
+ git diff --exit-code --quiet
48
+'
49
+
50
+test_done