Re: [PATCH] checkout: don't check worktrees when not necessary
From: Duy Nguyen <hidden>
Date: 2016-06-15 23:05:04
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Sun, May 31, 2015 at 07:16:29PM -0400, Spencer Baugh wrote:
quoted hunk ↗ jump to hunk
--- a/builtin/checkout.c +++ b/builtin/checkout.c@@ -1237,6 +1237,7 @@ static int parse_branchname_arg(int argc, const char **argv, char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); if (head_ref && (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && + !(opts->patch_mode || opts->pathspec.nr) && !opts->ignore_other_worktrees) check_linked_checkouts(new); free(head_ref);
Simple and effective. But if in future we add more options for non-switching-branch checkout, we need to update both places, here and near the end of cmd_checkout(). Perhaps we can move all this block inside checkout_branch() so we only need to test "opts->patch_mode || opts->pathspec.nr" once, at the end of cmd_checkout(). Something like below? I'm not opposed to your change, but if you go with it, you should cherry pick my test in the below patch. Or create a similar test. -- 8< --
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2f92328..e9aee58 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c@@ -1110,7 +1110,6 @@ static int parse_branchname_arg(int argc, const char **argv, { struct tree **source_tree = &opts->source_tree; const char **new_branch = &opts->new_branch; - int force_detach = opts->force_detach; int argcount = 0; unsigned char branch_rev[20]; const char *arg;
@@ -1231,17 +1230,6 @@ static int parse_branchname_arg(int argc, const char **argv, else new->path = NULL; /* not an existing branch */ - if (new->path && !force_detach && !*new_branch) { - unsigned char sha1[20]; - int flag; - char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); - if (head_ref && - (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && - !opts->ignore_other_worktrees) - check_linked_checkouts(new); - free(head_ref); - } - new->commit = lookup_commit_reference_gently(rev, 1); if (!new->commit) { /* not a commit */
@@ -1321,6 +1309,17 @@ static int checkout_branch(struct checkout_opts *opts, die(_("Cannot switch branch to a non-commit '%s'"), new->name); + if (new->path && !opts->force_detach && !opts->new_branch) { + unsigned char sha1[20]; + int flag; + char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); + if (head_ref && + (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && + !opts->ignore_other_worktrees) + check_linked_checkouts(new); + free(head_ref); + } + if (opts->new_worktree) return prepare_linked_checkout(opts, new);
diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh
index f8e4df4..a8d9336 100755
--- a/t/t2025-checkout-to.sh
+++ b/t/t2025-checkout-to.sh@@ -28,6 +28,14 @@ test_expect_success 'checkout --to refuses to checkout locked branch' ' ! test -d .git/worktrees/zere ' +test_expect_success 'checking out paths not complaining about linked checkouts' ' + ( + cd existing_empty && + echo dirty >>init.t && + git checkout master -- init.t + ) +' + test_expect_success 'checkout --to a new worktree' ' git rev-parse HEAD >expect && git checkout --detach --to here master && -- 8< -- --
Duy