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

[Junio C Hamano <gitster@pobox.com>]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

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

Yeah, I agree that having this check in parse_branchname_arg() does
not make any sense, as that function is still trying to decide if we
are switching to a different branch or not.  The new location looks
much more sensible.

Will queue this on nd/multiple-work-trees (which has been in
'master' for about a month).  Good thing that we caught it before it
got in any tagged release.

Thanks.

>
> Original-patch-by: Spencer Baugh <sbaugh@catern.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  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 &&