Re: [PATCH v2 06/20] checkout: check_linked_checkout: improve "already checked out" aesthetic

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

Eric Sunshine <sunshine@sunshineco.com> writes:

> When check_linked_checkout() discovers that the branch is already
> checked out elsewhere, it emits the diagnostic:
>
>     'blorp' is already checked out at '/some/path/.git'
>
> which is mildly misleading and a bit unsightly due to the trailing
> "/.git". For the user, "/some/path" is significant, whereas "/.git" is
> mere noise, so drop it.

More importantly, when a user hears "a checkout", the word means the
working tree location.  My top-level Makefile is at /some/path/Makefile,
not /some/path/.git/Makefile, so having /.git suffix is wrong.

How does this work with manually configured GIT_DIR environment, by
the way?  I think GIT_DIR=/collection/of/repos/foo.git would be OK,
as strbuf_strip_suffix() would hopefully leave it intact, but I am
more interested in the general working of linked checkout feature,
not just this error message.

In the new world order with GIT_DIR and GIT_COMMON_DIR, does
"$GIT_DIR" always have to be the same as "$GIT_WORK_TREE/.git"?  Do
we need some sanity check if that is the case?  Perhaps: if you have
$GIT_DIR set to $somewhere/.git/worktrees/$name, then

 - $GIT_COMMON_DIR must match $somewhere/.git,

 - $somewhere/.git/worktrees/$name/commondir must point at
   $GIT_COMMON_DIR,

 - $GIT_WORK_TREE/.git must match $GIT_DIR

or something like that?

> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>
> New in v2.
>
>  builtin/checkout.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 177ad6a..a331345 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -909,6 +909,7 @@ static void check_linked_checkout(const char *branch, const char *id)
>  	} else
>  		strbuf_addstr(&gitdir, get_git_common_dir());
>  	skip_prefix(branch, "refs/heads/", &branch);
> +	strbuf_strip_suffix(&gitdir, "/.git");

Sick people have '/.git' and run "git add etc/passwd"; do we want to
consider such a use case?

>  	die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
>  done:
>  	strbuf_release(&path);