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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> wrote:
> On Sun, Oct 9, 2016 at 2:51 PM, Dennis Kaarsemaker
> <dennis@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx%3E/
> --
> Duy



-- 
Michael Tutty, CTO

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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]