Re: Bug? git worktree fails with master on bare repo

[Dennis Kaarsemaker <dennis@kaarsemaker.net>]

On Sat, 2016-10-08 at 19:30 -0500, Michael Tutty wrote:
> Hey all,
> I'm working on some server-side software to do a merge. By using git
> worktree it's possible to check out a given branch for a bare repo and
> merge another branch into it. It's very fast, even with large
> repositories.
> 
> The only exception seems to be merging to master. When I do git
> worktree add /tmp/path/to/worktree master I get an error:
> 
> [fatal: 'master' is already checked out at '/path/to/bare/repo']
> 
> But this is clearly not true, git worktree list gives:
> 
> [/path/to/bare/repo (bare)]
> 
> ...and of course, there is no work tree at that path, just the bare
> repo files you'd expect.

The worktree code treats the base repo as a worktree, even if it's
bare. For the purpose of being able to do a checkout of the main branch
of a bare repo, this patch should do:

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 4bcc335..b618d6b 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -138,6 +138,14 @@ test_expect_success 'checkout from a bare repo without "add"' '
        )
 '
 
+test_expect_success '"add" default branch of a bare repo' '
+       (
+               git clone --bare . bare2 &&
+               cd bare2 &&
+               git worktree add ../there3 master
+       )
+'
+
 test_expect_success 'checkout with grafts' '
        test_when_finished rm .git/info/grafts &&
        test_commit abc &&
diff --git a/worktree.c b/worktree.c
index 5acfe4c..35e95b7 100644
--- a/worktree.c
+++ b/worktree.c
@@ -345,6 +345,8 @@ const struct worktree *find_shared_symref(const char *symref,
 
        for (i = 0; worktrees[i]; i++) {
                struct worktree *wt = worktrees[i];
+               if(wt->is_bare)
+                       continue;
 
                if (wt->is_detached && !strcmp(symref, "HEAD")) {
                        if (is_worktree_being_rebased(wt, target)) {


But I'm wondering why the worktree code does this. A bare repo isn't a
worktree and I think it shouldn't treat it as one. A patch that rips
out this feature and updates the tests to match would look like this:


diff --git a/builtin/worktree.c b/builtin/worktree.c
index 5c4854d..3600530 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -382,15 +382,11 @@ static int add(int ac, const char **av, const char *prefix)
 static void show_worktree_porcelain(struct worktree *wt)
 {
 	printf("worktree %s\n", wt->path);
-	if (wt->is_bare)
-		printf("bare\n");
-	else {
-		printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
-		if (wt->is_detached)
-			printf("detached\n");
-		else
-			printf("branch %s\n", wt->head_ref);
-	}
+	printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
+	if (wt->is_detached)
+		printf("detached\n");
+	else
+		printf("branch %s\n", wt->head_ref);
 	printf("\n");
 }
 
@@ -401,16 +397,12 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
 	int path_adj = cur_path_len - utf8_strwidth(wt->path);
 
 	strbuf_addf(&sb, "%-*s ", 1 + path_maxlen + path_adj, wt->path);
-	if (wt->is_bare)
-		strbuf_addstr(&sb, "(bare)");
-	else {
-		strbuf_addf(&sb, "%-*s ", abbrev_len,
-				find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));
-		if (!wt->is_detached)
-			strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0));
-		else
-			strbuf_addstr(&sb, "(detached HEAD)");
-	}
+	strbuf_addf(&sb, "%-*s ", abbrev_len,
+			find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));
+	if (!wt->is_detached)
+		strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0));
+	else
+		strbuf_addstr(&sb, "(detached HEAD)");
 	printf("%s\n", sb.buf);
 
 	strbuf_release(&sb);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 4bcc335..b618d6b 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -138,6 +138,14 @@ test_expect_success 'checkout from a bare repo without "add"' '
 	)
 '
 
+test_expect_success '"add" default branch of a bare repo' '
+	(
+		git clone --bare . bare2 &&
+		cd bare2 &&
+		git worktree add ../there3 master
+	)
+'
+
 test_expect_success 'checkout with grafts' '
 	test_when_finished rm .git/info/grafts &&
 	test_commit abc &&
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 1b1b65a..842e9d9 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -62,9 +62,8 @@ test_expect_success 'bare repo setup' '
 
 test_expect_success '"list" all worktrees from bare main' '
 	test_when_finished "rm -rf there && git -C bare1 worktree prune" &&
-	git -C bare1 worktree add --detach ../there master &&
-	echo "$(pwd)/bare1 (bare)" >expect &&
-	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
+	git -C bare1 worktree add ../there master &&
+	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) [master]" >expect &&
 	git -C bare1 worktree list | sed "s/  */ /g" >actual &&
 	test_cmp expect actual
 '
@@ -72,10 +71,7 @@ test_expect_success '"list" all worktrees from bare main' '
 test_expect_success '"list" all worktrees --porcelain from bare main' '
 	test_when_finished "rm -rf there && git -C bare1 worktree prune" &&
 	git -C bare1 worktree add --detach ../there master &&
-	echo "worktree $(pwd)/bare1" >expect &&
-	echo "bare" >>expect &&
-	echo >>expect &&
-	echo "worktree $(git -C there rev-parse --show-toplevel)" >>expect &&
+	echo "worktree $(git -C there rev-parse --show-toplevel)" >expect &&
 	echo "HEAD $(git -C there rev-parse HEAD)" >>expect &&
 	echo "detached" >>expect &&
 	echo >>expect &&
@@ -85,9 +81,8 @@ test_expect_success '"list" all worktrees --porcelain from bare main' '
 
 test_expect_success '"list" all worktrees from linked with a bare main' '
 	test_when_finished "rm -rf there && git -C bare1 worktree prune" &&
-	git -C bare1 worktree add --detach ../there master &&
-	echo "$(pwd)/bare1 (bare)" >expect &&
-	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) (detached HEAD)" >>expect &&
+	git -C bare1 worktree add ../there master &&
+	echo "$(git -C there rev-parse --show-toplevel) $(git -C there rev-parse --short HEAD) [master]" >expect &&
 	git -C there worktree list | sed "s/  */ /g" >actual &&
 	test_cmp expect actual
 '
diff --git a/worktree.c b/worktree.c
index 5acfe4c..d4dbaab 100644
--- a/worktree.c
+++ b/worktree.c
@@ -84,7 +84,7 @@ static struct worktree *get_main_worktree(void)
 	strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
 	is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
 	if (is_bare)
-		strbuf_strip_suffix(&worktree_path, "/.");
+		goto done;
 
 	strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
 
@@ -94,7 +94,6 @@ static struct worktree *get_main_worktree(void)
 	worktree = xmalloc(sizeof(struct worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->id = NULL;
-	worktree->is_bare = is_bare;
 	worktree->head_ref = NULL;
 	worktree->is_detached = is_detached;
 	worktree->is_current = 0;
@@ -141,7 +140,6 @@ static struct worktree *get_linked_worktree(const char *id)
 	worktree = xmalloc(sizeof(struct worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->id = xstrdup(id);
-	worktree->is_bare = 0;
 	worktree->head_ref = NULL;
 	worktree->is_detached = is_detached;
 	worktree->is_current = 0;
diff --git a/worktree.h b/worktree.h
index 90e1311..04a75e8 100644
--- a/worktree.h
+++ b/worktree.h
@@ -8,7 +8,6 @@ struct worktree {
 	char *lock_reason;	/* internal use */
 	unsigned char head_sha1[20];
 	int is_detached;
-	int is_bare;
 	int is_current;
 	int lock_reason_valid;
 };