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

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Jul 16, 2015 at 1:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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.

I may be misunderstanding your question, but my answer is that
strbuf_strip_suffix() is not applied to $GIT_DIR, but rather to the
content of file $GIT_COMMON_DIR/worktrees/<tag>/gitdir, which is the
path to the .git file in the linked worktree. That is, given:

    git worktree add ../foo HEAD

then the content of 'gitdir' is the absolute path of "../foo/.git",
and strbuf_strip_suffix() operates on that value.

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

Duy is probably better suited to answer this, as he would likely have
taken these issues into consideration when implementing the feature.
(I've been poking through documentation and code for quite a while
trying to answer this email but don't yet have a sufficient grasp to
do it justice. I'm not even sure where such a sanity check would be
placed.)

>>       } 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?

I originally implemented this as stripping only ".git" since it felt
natural for the result to have a trailing slash, however, when I
looked back at your report[1], I saw that you suggested stripping
"/.git", so I changed it to strip the slash as well. Given the above
"sick" use-case, we may indeed want to strip only ".git".

[1]: http://article.gmane.org/gmane.comp.version-control.git/274001

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