[PATCH] checkout: don't check worktrees when not necessary

Subsystems: the rest

STALE3738d

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH] checkout: don't check worktrees when not necessary

From: Spencer Baugh <hidden>
Date: 2016-06-15 23:05:04

When --patch or pathspecs are passed to git checkout, the working tree
will not be switching branch, so there's no need to check if the branch
that we are running checkout on is already checked out.

Signed-off-by: Spencer Baugh <redacted>
---
 builtin/checkout.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2f92328..7039c5c 100644
--- 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);

-- 
2.4.2.339.g77bd3ea

Re: [PATCH] checkout: don't check worktrees when not necessary

From: Duy Nguyen <hidden>
Date: 2016-06-15 23:05:04

On Sun, May 31, 2015 at 07:16:29PM -0400, Spencer Baugh wrote:

quoted 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

Re: [PATCH] checkout: don't check worktrees when not necessary

From: Spencer Baugh <hidden>
Date: 2016-06-15 23:05:15

Duy Nguyen [off-list ref] writes:

On Sun, May 31, 2015 at 07:16:29PM -0400, Spencer Baugh wrote:

quoted

--- 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.

Sorry for late reply, but I think your change is much better than mine
so I'd suggest just using that instead.

[PATCH] checkout: don't check worktrees when not necessary

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 23:05:16

When --patch or pathspecs are passed to git checkout, the working tree
will not be switching branch, so there's no need to check if the branch
that we are running checkout on is already checked out.

Original-patch-by: Spencer Baugh [off-list ref]
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/checkout.c     | 23 +++++++++++------------
 t/t2025-checkout-to.sh |  8 ++++++++
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 9b49f0e..e227f64 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 &&

-- 
2.3.0.rc1.137.g477eb31

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help