From 0cb6ad3c3d8e9c738686ef8dc6f173f725e509bc Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 11 Jan 2011 15:00:38 -0500 Subject: [PATCH 1/5] checkout: fix bug with ambiguous refs The usual dwim_ref lookup prefers tags to branches. Because checkout primarily works on branches, though, we switch that behavior to prefer branches. However, there was a bug in the implementation in which we used lookup_commit_reference (which used the regular lookup rules) to get the actual commit to checkout. Checking out an ambiguous ref therefore ended up putting us in an extremely broken state in which we wrote the branch ref into HEAD, but actually checked out the tree for the tag. This patch fixes the bug by always attempting to pull the commit to be checked out from the branch-ified version of the name we were given. Patch by Junio, tests and commit message from Jeff King. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/checkout.c | 23 +++++++------ t/t2019-checkout-amiguous-ref.sh | 59 ++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 10 deletions(-) create mode 100755 t/t2019-checkout-amiguous-ref.sh diff --git a/builtin/checkout.c b/builtin/checkout.c index a54583b3a4..953abdd0fa 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -678,7 +678,7 @@ static const char *unique_tracking_name(const char *name) int cmd_checkout(int argc, const char **argv, const char *prefix) { struct checkout_opts opts; - unsigned char rev[20]; + unsigned char rev[20], branch_rev[20]; const char *arg; struct branch_info new; struct tree *source_tree = NULL; @@ -832,18 +832,21 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) argc--; new.name = arg; - if ((new.commit = lookup_commit_reference_gently(rev, 1))) { - setup_branch_path(&new); + setup_branch_path(&new); - if ((check_ref_format(new.path) == CHECK_REF_FORMAT_OK) && - resolve_ref(new.path, rev, 1, NULL)) - ; - else - new.path = NULL; + if (check_ref_format(new.path) == CHECK_REF_FORMAT_OK && + resolve_ref(new.path, branch_rev, 1, NULL)) + hashcpy(rev, branch_rev); + else + new.path = NULL; /* not an existing branch */ + + if (!(new.commit = lookup_commit_reference_gently(rev, 1))) { + /* not a commit */ + source_tree = parse_tree_indirect(rev); + } else { parse_commit(new.commit); source_tree = new.commit->tree; - } else - source_tree = parse_tree_indirect(rev); + } if (!source_tree) /* case (1): want a tree */ die("reference is not a tree: %s", arg); diff --git a/t/t2019-checkout-amiguous-ref.sh b/t/t2019-checkout-amiguous-ref.sh new file mode 100755 index 0000000000..943541d40d --- /dev/null +++ b/t/t2019-checkout-amiguous-ref.sh @@ -0,0 +1,59 @@ +#!/bin/sh + +test_description='checkout handling of ambiguous (branch/tag) refs' +. ./test-lib.sh + +test_expect_success 'setup ambiguous refs' ' + test_commit branch file && + git branch ambiguity && + git branch vagueness && + test_commit tag file && + git tag ambiguity && + git tag vagueness HEAD:file && + test_commit other file +' + +test_expect_success 'checkout ambiguous ref succeeds' ' + git checkout ambiguity >stdout 2>stderr +' + +test_expect_success 'checkout produces ambiguity warning' ' + grep "warning.*ambiguous" stderr +' + +test_expect_success 'checkout chooses branch over tag' ' + echo refs/heads/ambiguity >expect && + git symbolic-ref HEAD >actual && + test_cmp expect actual && + echo branch >expect && + test_cmp expect file +' + +test_expect_success 'checkout reports switch to branch' ' + grep "Switched to branch" stderr && + ! grep "^HEAD is now at" stderr +' + +test_expect_success 'checkout vague ref succeeds' ' + git checkout vagueness >stdout 2>stderr && + test_set_prereq VAGUENESS_SUCCESS +' + +test_expect_success VAGUENESS_SUCCESS 'checkout produces ambiguity warning' ' + grep "warning.*ambiguous" stderr +' + +test_expect_success VAGUENESS_SUCCESS 'checkout chooses branch over tag' ' + echo refs/heads/vagueness >expect && + git symbolic-ref HEAD >actual && + test_cmp expect actual && + echo branch >expect && + test_cmp expect file +' + +test_expect_success VAGUENESS_SUCCESS 'checkout reports switch to branch' ' + grep "Switched to branch" stderr && + ! grep "^HEAD is now at" stderr +' + +test_done From 09ebad6faeec11c3dbad0bdaf95faed57be5dcba Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 8 Feb 2011 04:29:09 -0600 Subject: [PATCH 2/5] checkout: split off a function to peel away branchname arg The code to parse and consume the tree name and "--" in commands such as "git checkout @{-1} -- '*.c'" is intimidatingly long. Split it out into a separate function and make it easier to skip on first reading by making the data it uses and produces more explicit. No functional change intended. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/checkout.c | 229 ++++++++++++++++++++++++++------------------- 1 file changed, 131 insertions(+), 98 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 953abdd0fa..0e7a6a3952 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -675,11 +675,123 @@ static const char *unique_tracking_name(const char *name) return NULL; } +static int parse_branchname_arg(int argc, const char **argv, + int dwim_new_local_branch_ok, + struct branch_info *new, + struct tree **source_tree, + unsigned char rev[20], + const char **new_branch) +{ + int argcount = 0; + unsigned char branch_rev[20]; + const char *arg; + int has_dash_dash; + + /* + * 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. As a special case, + * if 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 is _not_ a commit, no -t nor -b + * was given, and there is a tracking branch whose name is + * in one and only one remote, then this is a short-hand + * to fork local from that remote tracking branch. + * + * 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) + return 0; + + if (!strcmp(argv[0], "--")) /* case (2) */ + return 1; + + arg = argv[0]; + has_dash_dash = (argc > 1) && !strcmp(argv[1], "--"); + + 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) { + const char *remote = unique_tracking_name(arg); + if (!remote || get_sha1(remote, rev)) + return argcount; + *new_branch = arg; + arg = remote; + /* DWIMmed to create local branch */ + } else { + return argcount; + } + } + + /* we can't end up being in (2) anymore, eat the argument */ + argcount++; + argv++; + argc--; + + new->name = arg; + setup_branch_path(new); + + if (check_ref_format(new->path) == CHECK_REF_FORMAT_OK && + resolve_ref(new->path, branch_rev, 1, NULL)) + hashcpy(rev, branch_rev); + else + new->path = NULL; /* not an existing branch */ + + new->commit = lookup_commit_reference_gently(rev, 1); + if (!new->commit) { + /* not a commit */ + *source_tree = parse_tree_indirect(rev); + } else { + parse_commit(new->commit); + *source_tree = new->commit->tree; + } + + 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 { + argcount++; + argv++; + argc--; + } + + return argcount; +} + int cmd_checkout(int argc, const char **argv, const char *prefix) { struct checkout_opts opts; - unsigned char rev[20], branch_rev[20]; - const char *arg; + unsigned char rev[20]; struct branch_info new; struct tree *source_tree = NULL; char *conflict_style = NULL; @@ -709,7 +821,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) PARSE_OPT_NOARG | PARSE_OPT_HIDDEN }, OPT_END(), }; - int has_dash_dash; memset(&opts, 0, sizeof(opts)); memset(&new, 0, sizeof(new)); @@ -766,108 +877,30 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) die("git checkout: -f and -m are incompatible"); /* - * case 1: git checkout -- [] + * Extract branch name from command line arguments, so + * all that is left is pathspecs. * - * must be a valid tree, everything after the '--' must be - * a path. + * Handle * - * 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. As a special case, - * if 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 is _not_ a commit, no -t nor -b - * was given, and there is a tracking branch whose name is - * in one and only one remote, then this is a short-hand - * to fork local from that remote tracking branch. - * - * 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. + * 1) git checkout -- [] + * 2) git checkout -- [] + * 3) git checkout [] * + * including "last branch" syntax and DWIM-ery for names of + * remote branches, erroring out for invalid or ambiguous cases. */ if (argc) { - if (!strcmp(argv[0], "--")) { /* case (2) */ - argv++; - argc--; - goto no_reference; - } - - arg = argv[0]; - has_dash_dash = (argc > 1) && !strcmp(argv[1], "--"); - - if (!strcmp(arg, "-")) - arg = "@{-1}"; - - if (get_sha1_mb(arg, rev)) { - if (has_dash_dash) /* case (1) */ - die("invalid reference: %s", arg); - if (!patch_mode && - dwim_new_local_branch && - opts.track == BRANCH_TRACK_UNSPECIFIED && - !opts.new_branch && - !check_filename(NULL, arg) && - argc == 1) { - const char *remote = unique_tracking_name(arg); - if (!remote || get_sha1(remote, rev)) - goto no_reference; - opts.new_branch = arg; - arg = remote; - /* DWIMmed to create local branch */ - } - else - goto no_reference; - } - - /* we can't end up being in (2) anymore, eat the argument */ - argv++; - argc--; - - new.name = arg; - setup_branch_path(&new); - - if (check_ref_format(new.path) == CHECK_REF_FORMAT_OK && - resolve_ref(new.path, branch_rev, 1, NULL)) - hashcpy(rev, branch_rev); - else - new.path = NULL; /* not an existing branch */ - - if (!(new.commit = lookup_commit_reference_gently(rev, 1))) { - /* not a commit */ - source_tree = parse_tree_indirect(rev); - } else { - parse_commit(new.commit); - source_tree = new.commit->tree; - } - - 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--; - } + int dwim_ok = + !patch_mode && + dwim_new_local_branch && + opts.track == BRANCH_TRACK_UNSPECIFIED && + !opts.new_branch; + int n = parse_branchname_arg(argc, argv, dwim_ok, + &new, &source_tree, rev, &opts.new_branch); + argv += n; + argc -= n; } -no_reference: - if (opts.track == BRANCH_TRACK_UNSPECIFIED) opts.track = git_branch_track; From 32669671c7746888aa1e3832907deb7fc8405061 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 8 Feb 2011 04:32:49 -0600 Subject: [PATCH 3/5] checkout: introduce --detach synonym for "git checkout foo^{commit}" For example, one might use this when making a temporary merge to test that two topics work well together. Patch by Junio, with tests from Jeff King. [jn: with some extra checks for bogus commandline usage] Suggested-by: Jeff King Signed-off-by: Junio C Hamano Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Documentation/git-checkout.txt | 13 ++++- builtin/checkout.c | 25 +++++++-- t/t2020-checkout-detach.sh | 95 ++++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 7 deletions(-) create mode 100755 t/t2020-checkout-detach.sh diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 22d36114df..d162117e71 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -9,6 +9,7 @@ SYNOPSIS -------- [verse] 'git checkout' [-q] [-f] [-m] [] +'git checkout' [-q] [-f] [-m] [--detach] [] 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] ] [] 'git checkout' [-f|--ours|--theirs|-m|--conflict=