git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Bug? git worktree fails with master on bare repo
@ 2016-10-09  0:30 Michael Tutty
  2016-10-09  6:50 ` Kevin Daudt
  2016-10-09  7:51 ` Dennis Kaarsemaker
  0 siblings, 2 replies; 14+ messages in thread
From: Michael Tutty @ 2016-10-09  0:30 UTC (permalink / raw)
  To: git

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Bug? git worktree fails with master on bare repo
  2016-10-09  0:30 Bug? git worktree fails with master on bare repo Michael Tutty
@ 2016-10-09  6:50 ` Kevin Daudt
  2016-10-09  7:51 ` Dennis Kaarsemaker
  1 sibling, 0 replies; 14+ messages in thread
From: Kevin Daudt @ 2016-10-09  6:50 UTC (permalink / raw)
  To: Michael Tutty; +Cc: git

On Sat, Oct 08, 2016 at 07:30:36PM -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.

A bare repo still has a HEAD, which by default points to
refs/heads/master. That's what's it complaining about.

So the question is, should there be an exception for the branch 'checked
out' on a bare reposity.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Bug? git worktree fails with master on bare repo
  2016-10-09  0:30 Bug? git worktree fails with master on bare repo Michael Tutty
  2016-10-09  6:50 ` Kevin Daudt
@ 2016-10-09  7:51 ` Dennis Kaarsemaker
  2016-10-09 10:52   ` Duy Nguyen
  1 sibling, 1 reply; 14+ messages in thread
From: Dennis Kaarsemaker @ 2016-10-09  7:51 UTC (permalink / raw)
  To: Michael Tutty, git, pclouds

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: Bug? git worktree fails with master on bare repo
  2016-10-09  7:51 ` Dennis Kaarsemaker
@ 2016-10-09 10:52   ` Duy Nguyen
  2016-10-09 13:42     ` Michael Tutty
  2016-10-12 16:41     ` [PATCH] worktree: allow the main brach of a bare repository to be checked out Dennis Kaarsemaker
  0 siblings, 2 replies; 14+ messages in thread
From: Duy Nguyen @ 2016-10-09 10:52 UTC (permalink / raw)
  To: Dennis Kaarsemaker; +Cc: Michael Tutty, Git Mailing List, Michael Rappazzo

On Sun, Oct 9, 2016 at 2:51 PM, Dennis Kaarsemaker
<dennis@kaarsemaker.net> wrote:
> 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)) {
>
>

You're fast :) I'm still studying  8d9fdd7 (worktree.c: check whether
branch is rebased in another worktree - 2016-04-22). But yeah that
should fix it.

> 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");
>  }

This goes back to the first very first commit of "git worktree list":
bb9c03b (worktree: add 'list' command - 2015-10-08) and was sort of
pointed out during review [1] but nobody answered it.

A bare repo does not have an associated worktree. However only main
worktree can be bare. If we take this out, "git worktree list"'s first
line will no longer be about the main worktree (because it does not
exist). That may cause trouble since we promised in "git-worktree.txt"
that the main worktree is listed first. I don't think we have any way
else to determine if the main worktree exists. Showing "bare" may be
the way to see if we have a main worktree or not. So we probably want
to keep this function unchanged.

[1] https://public-inbox.org/git/%3CCANoM8SWeqxD2vWLQmEfxxxn8Dz4yPfjGOoOH=Azn1A3So+wz2Q@mail.gmail.com%3E/
-- 
Duy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Bug? git worktree fails with master on bare repo
  2016-10-09 10:52   ` Duy Nguyen
@ 2016-10-09 13:42     ` Michael Tutty
  2016-10-10  9:45       ` Duy Nguyen
  2016-10-12 16:41     ` [PATCH] worktree: allow the main brach of a bare repository to be checked out Dennis Kaarsemaker
  1 sibling, 1 reply; 14+ messages in thread
From: Michael Tutty @ 2016-10-09 13:42 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Dennis Kaarsemaker, Git Mailing List, Michael Rappazzo

Dennis,
Thanks for the great response, and for spending time on my issue.
I'll try that first patch and see what happens.

In the meantime, it got weirder...

I created a brand-new (bare) repo and was able to git add worktree
/path master.  I was able to do this repeatedly, even using the
worktree to merge other branches to master.  I didn't find any
condition or step that caused some kind of orphan master work tree,
which was what I thought the underlying problem might be.

So, on the one hand, you found code validating my initial experience.
But on the other hand, I found a test case that didn't appear to have
that problem.

WAT.
      M.

On Sun, Oct 9, 2016 at 5:52 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Oct 9, 2016 at 2:51 PM, Dennis Kaarsemaker
> <dennis@kaarsemaker.net> wrote:
>> 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)) {
>>
>>
>
> You're fast :) I'm still studying  8d9fdd7 (worktree.c: check whether
> branch is rebased in another worktree - 2016-04-22). But yeah that
> should fix it.
>
>> 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");
>>  }
>
> This goes back to the first very first commit of "git worktree list":
> bb9c03b (worktree: add 'list' command - 2015-10-08) and was sort of
> pointed out during review [1] but nobody answered it.
>
> A bare repo does not have an associated worktree. However only main
> worktree can be bare. If we take this out, "git worktree list"'s first
> line will no longer be about the main worktree (because it does not
> exist). That may cause trouble since we promised in "git-worktree.txt"
> that the main worktree is listed first. I don't think we have any way
> else to determine if the main worktree exists. Showing "bare" may be
> the way to see if we have a main worktree or not. So we probably want
> to keep this function unchanged.
>
> [1] https://public-inbox.org/git/%3CCANoM8SWeqxD2vWLQmEfxxxn8Dz4yPfjGOoOH=Azn1A3So+wz2Q@mail.gmail.com%3E/
> --
> Duy



-- 
Michael Tutty, CTO

e: mtutty@gforgegroup.com
t: @mtutty, @gforgegroup
v: 515-789-0772
w: http://gforgegroup.com, http://gforge.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Bug? git worktree fails with master on bare repo
  2016-10-09 13:42     ` Michael Tutty
@ 2016-10-10  9:45       ` Duy Nguyen
  2016-10-10 13:06         ` Michael Tutty
  0 siblings, 1 reply; 14+ messages in thread
From: Duy Nguyen @ 2016-10-10  9:45 UTC (permalink / raw)
  To: Michael Tutty; +Cc: Dennis Kaarsemaker, Git Mailing List, Michael Rappazzo

On Sun, Oct 9, 2016 at 8:42 PM, Michael Tutty <mtutty@gforgegroup.com> wrote:
> Dennis,
> Thanks for the great response, and for spending time on my issue.
> I'll try that first patch and see what happens.
>
> In the meantime, it got weirder...
>
> I created a brand-new (bare) repo

Elaboration needed here. If I create a bare _clone_, then "HEAD" could
be detached, or point to some branch, depending on where "HEAD" is in
the source repo. If source repo's HEAD is "master", I got the same
behavior (worktree add fails). If it's detached or points to some
other branch, it's ok. If this is "git init --bare" then I got "fatal:
invalid reference: master".

> and was able to git add worktree
> /path master.  I was able to do this repeatedly, even using the
> worktree to merge other branches to master.  I didn't find any
> condition or step that caused some kind of orphan master work tree,
> which was what I thought the underlying problem might be.
-- 
Duy

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Bug? git worktree fails with master on bare repo
  2016-10-10  9:45       ` Duy Nguyen
@ 2016-10-10 13:06         ` Michael Tutty
  2016-10-11 15:41           ` Kevin Daudt
  0 siblings, 1 reply; 14+ messages in thread
From: Michael Tutty @ 2016-10-10 13:06 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Dennis Kaarsemaker, Git Mailing List, Michael Rappazzo

> If I create a bare _clone_, then "HEAD" could be detached, or point to some branch, depending on where "HEAD" is in the source repo

I didn't mean a clone, I meant a brand-new (bare) repo.  Then I would
clone it somewhere, add commits and branches, and push them to the
bare repo.


> If source repo's HEAD is "master", I got the same behavior (worktree add fails)

So if it's possible for a bare repo to have HEAD pointing at master,
is there a safe way for me to change this (e.g., as a cleanup step
before doing my actual merge process)?

On Mon, Oct 10, 2016 at 4:45 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Oct 9, 2016 at 8:42 PM, Michael Tutty <mtutty@gforgegroup.com> wrote:
>> Dennis,
>> Thanks for the great response, and for spending time on my issue.
>> I'll try that first patch and see what happens.
>>
>> In the meantime, it got weirder...
>>
>> I created a brand-new (bare) repo
>
> Elaboration needed here. If I create a bare _clone_, then "HEAD" could
> be detached, or point to some branch, depending on where "HEAD" is in
> the source repo. If source repo's HEAD is "master", I got the same
> behavior (worktree add fails). If it's detached or points to some
> other branch, it's ok. If this is "git init --bare" then I got "fatal:
> invalid reference: master".
>
>> and was able to git add worktree
>> /path master.  I was able to do this repeatedly, even using the
>> worktree to merge other branches to master.  I didn't find any
>> condition or step that caused some kind of orphan master work tree,
>> which was what I thought the underlying problem might be.
> --
> Duy



-- 
Michael Tutty, CTO

e: mtutty@gforgegroup.com
t: @mtutty, @gforgegroup
v: 515-789-0772
w: http://gforgegroup.com, http://gforge.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Bug? git worktree fails with master on bare repo
  2016-10-10 13:06         ` Michael Tutty
@ 2016-10-11 15:41           ` Kevin Daudt
  0 siblings, 0 replies; 14+ messages in thread
From: Kevin Daudt @ 2016-10-11 15:41 UTC (permalink / raw)
  To: Michael Tutty
  Cc: Duy Nguyen, Dennis Kaarsemaker, Git Mailing List,
	Michael Rappazzo

On Mon, Oct 10, 2016 at 08:06:47AM -0500, Michael Tutty wrote:
> 
> > If source repo's HEAD is "master", I got the same behavior (worktree add fails)
> 
> So if it's possible for a bare repo to have HEAD pointing at master,
> is there a safe way for me to change this (e.g., as a cleanup step
> before doing my actual merge process)?

You can change where HEAD points to in a bare repositor with `git
symbolic-ref HEAD <branch>`, but note that this changes what branch gets
checked out when cloning a repository. 

Where users would normally check out master by default, after changing
what HEAD points to, it would be that branch (or detached when HEAD
doesn't even point at a branch).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] worktree: allow the main brach of a bare repository to be checked out
  2016-10-09 10:52   ` Duy Nguyen
  2016-10-09 13:42     ` Michael Tutty
@ 2016-10-12 16:41     ` Dennis Kaarsemaker
  2016-10-12 17:35       ` Michael Tutty
  2016-10-12 18:37       ` Junio C Hamano
  1 sibling, 2 replies; 14+ messages in thread
From: Dennis Kaarsemaker @ 2016-10-12 16:41 UTC (permalink / raw)
  To: git; +Cc: pclouds, mtutty, rappazzo

In bare repositories, get_worktrees() still returns the main repository,
so git worktree list can show it. ignore it in find_shared_symref so we
can still check out the main branch.

Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
---
 t/t2025-worktree-add.sh | 8 ++++++++
 worktree.c              | 2 ++
 2 files changed, 10 insertions(+)

On Sun, 2016-10-09 at 17:52 +0700, Duy Nguyen wrote:
> On Sun, Oct 9, 2016 at 2:51 PM, Dennis Kaarsemaker > <dennis@kaarsemaker.net> wrote:
> > On Sat, 2016-10-08 at 19:30 -0500, Michael Tutty wrote:
> > > 
> > > 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']
> > > 
> > 
> > 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:
> > 
> --snip--
> 
> You're fast :) I'm still studying  8d9fdd7 (worktree.c: check whether
> branch is rebased in another worktree - 2016-04-22). But yeah that
> should fix it.

OK, so here it is as a proper patch.

D.

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 4bcc335..2996c38 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)) {
-- 
2.10.1-356-g947a599


-- 
Dennis Kaarsemaker <dennis@kaarsemaker.net>
http://twitter.com/seveas

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] worktree: allow the main brach of a bare repository to be checked out
  2016-10-12 16:41     ` [PATCH] worktree: allow the main brach of a bare repository to be checked out Dennis Kaarsemaker
@ 2016-10-12 17:35       ` Michael Tutty
  2016-10-12 18:37       ` Junio C Hamano
  1 sibling, 0 replies; 14+ messages in thread
From: Michael Tutty @ 2016-10-12 17:35 UTC (permalink / raw)
  To: Dennis Kaarsemaker; +Cc: Git Mailing List, Duy Nguyen, Michael Rappazzo

I have tested this successfully with git 2.10.1 built from source
(failing), then patched as above (passing).
Thanks!
     M.


On Wed, Oct 12, 2016 at 11:41 AM, Dennis Kaarsemaker
<dennis@kaarsemaker.net> wrote:
> In bare repositories, get_worktrees() still returns the main repository,
> so git worktree list can show it. ignore it in find_shared_symref so we
> can still check out the main branch.
>
> Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
> ---
>  t/t2025-worktree-add.sh | 8 ++++++++
>  worktree.c              | 2 ++
>  2 files changed, 10 insertions(+)
>
> On Sun, 2016-10-09 at 17:52 +0700, Duy Nguyen wrote:
>> On Sun, Oct 9, 2016 at 2:51 PM, Dennis Kaarsemaker > <dennis@kaarsemaker.net> wrote:
>> > On Sat, 2016-10-08 at 19:30 -0500, Michael Tutty wrote:
>> > >
>> > > 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']
>> > >
>> >
>> > 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:
>> >
>> --snip--
>>
>> You're fast :) I'm still studying  8d9fdd7 (worktree.c: check whether
>> branch is rebased in another worktree - 2016-04-22). But yeah that
>> should fix it.
>
> OK, so here it is as a proper patch.
>
> D.
>
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> index 4bcc335..2996c38 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)) {
> --
> 2.10.1-356-g947a599
>
>
> --
> Dennis Kaarsemaker <dennis@kaarsemaker.net>
> http://twitter.com/seveas



-- 
Michael Tutty, CTO

e: mtutty@gforgegroup.com
t: @mtutty, @gforgegroup
v: 515-789-0772
w: http://gforgegroup.com, http://gforge.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] worktree: allow the main brach of a bare repository to be checked out
  2016-10-12 16:41     ` [PATCH] worktree: allow the main brach of a bare repository to be checked out Dennis Kaarsemaker
  2016-10-12 17:35       ` Michael Tutty
@ 2016-10-12 18:37       ` Junio C Hamano
  2016-10-12 18:50         ` Junio C Hamano
  2016-10-12 19:23         ` Dennis Kaarsemaker
  1 sibling, 2 replies; 14+ messages in thread
From: Junio C Hamano @ 2016-10-12 18:37 UTC (permalink / raw)
  To: Dennis Kaarsemaker; +Cc: git, pclouds, mtutty, rappazzo

Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:

> OK, so here it is as a proper patch.
>
> D.
>
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> index 4bcc335..2996c38 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' '

Huh?

> +	(
> +		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)) {
> -- 
> 2.10.1-356-g947a599

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] worktree: allow the main brach of a bare repository to be checked out
  2016-10-12 18:37       ` Junio C Hamano
@ 2016-10-12 18:50         ` Junio C Hamano
  2016-10-13 10:31           ` Duy Nguyen
  2016-10-12 19:23         ` Dennis Kaarsemaker
  1 sibling, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2016-10-12 18:50 UTC (permalink / raw)
  To: Dennis Kaarsemaker; +Cc: git, pclouds, mtutty, rappazzo

Junio C Hamano <gitster@pobox.com> writes:

> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>
>> OK, so here it is as a proper patch.

Here is what I queued.  Duy, what do you think?  It seems OK to me.

Thanks.

-- >8 --
From: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Date: Wed, 12 Oct 2016 18:41:07 +0200
Subject: [PATCH] worktree: allow the main brach of a bare repository to be
 checked out

In bare repositories, get_worktrees() still returns the main repository,
so git worktree list can show it. ignore it in find_shared_symref so we
can still check out the main branch.

Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t2025-worktree-add.sh | 8 ++++++++
 worktree.c              | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 4bcc335a19..b618d6be21 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 5acfe4cd64..f7869f8d60 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)) {
-- 
2.10.1-591-g271c03b70f


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] worktree: allow the main brach of a bare repository to be checked out
  2016-10-12 18:37       ` Junio C Hamano
  2016-10-12 18:50         ` Junio C Hamano
@ 2016-10-12 19:23         ` Dennis Kaarsemaker
  1 sibling, 0 replies; 14+ messages in thread
From: Dennis Kaarsemaker @ 2016-10-12 19:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, pclouds, mtutty, rappazzo

On Wed, 2016-10-12 at 11:37 -0700, Junio C Hamano wrote:
> > ++test_expect_success '"add" default branch of a bare repo' '
> 
> Huh?

Copy paste error. And I missed

ok 17 - checkout from a bare repo without "add"
./t2025-worktree-add.sh: 141: ./t2025-worktree-add.sh: +test_expect_success: not found

in the output of 'make test'. Thanks for fixing up!

D.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] worktree: allow the main brach of a bare repository to be checked out
  2016-10-12 18:50         ` Junio C Hamano
@ 2016-10-13 10:31           ` Duy Nguyen
  0 siblings, 0 replies; 14+ messages in thread
From: Duy Nguyen @ 2016-10-13 10:31 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Dennis Kaarsemaker, Git Mailing List, Michael Tutty,
	Michael Rappazzo

On Thu, Oct 13, 2016 at 1:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>>
>>> OK, so here it is as a proper patch.
>
> Here is what I queued.  Duy, what do you think?  It seems OK to me.

Ack. Thanks both.
-- 
Duy

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-10-13 10:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-09  0:30 Bug? git worktree fails with master on bare repo Michael Tutty
2016-10-09  6:50 ` Kevin Daudt
2016-10-09  7:51 ` Dennis Kaarsemaker
2016-10-09 10:52   ` Duy Nguyen
2016-10-09 13:42     ` Michael Tutty
2016-10-10  9:45       ` Duy Nguyen
2016-10-10 13:06         ` Michael Tutty
2016-10-11 15:41           ` Kevin Daudt
2016-10-12 16:41     ` [PATCH] worktree: allow the main brach of a bare repository to be checked out Dennis Kaarsemaker
2016-10-12 17:35       ` Michael Tutty
2016-10-12 18:37       ` Junio C Hamano
2016-10-12 18:50         ` Junio C Hamano
2016-10-13 10:31           ` Duy Nguyen
2016-10-12 19:23         ` Dennis Kaarsemaker

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).