Eric Sunshine [off-list ref] 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?
quoted hunk
Signed-off-by: Eric Sunshine <redacted>
---
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);
On Thu, Jul 16, 2015 at 1:55 PM, Junio C Hamano [off-list ref] 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.)
quoted
} 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
quoted
die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
done:
strbuf_release(&path);
On Fri, Jul 17, 2015 at 7:32 AM, Eric Sunshine [off-list ref] wrote:
quoted
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.)
The thing is, we just don't know where the worktree is. All we know is
somewhere there is a .git file sharing this repository. People can
create a linked worktree, then move the actual linked worktree away,
set GIT_DIR/GIT_WORK_TREE to reflect that, and everything must still
work. So, we could say "foo is already checked out at the worktree
that is linked to /some/path/.git" to be technically correct. But
that's not so friendly? We could cache the $GIT_WORK_TREE, when the
user accesses the linked checkout, somewhere in .git/worktrees/foo and
show it instead of /some/path/.git. But that's not always accurate.
--
Duy