A few "linked checkout" niggles

A few "linked checkout" niggles

[Junio C Hamano]

I've tried to set up a non-bare clone of git.git at ~/w/src and
attached one of its branches to a separate work tree at ~/w/rerere

 ~/w/src$ git worktree add ../rerere jc/rerere

Then I tried the "multiple checkout not allowed".

 ~/w/src$ git checkout jc/rerere
 fatal: 'jc/rerere' is already checked out at '/home/gitster/w/rerere/.git'

You see the first "niggle" above.  Do we need "/.git" at the end?
That is hopefully merely cosmetic and I am guessing that we can
unconditionally drop "/.git" from the end, but maybe there are
subtle reasons why it may not be a good idea (e.g. introduces some
ambiguities?)

There is another, a more serious one.

 ~/w/src$ git fetch ../git.git jc/rerere:jc/rerere

This succeeds, without checking if there is a work tree that has it
checked out.  I think the safety in "git fetch" that can be disabled
by "--update-head-ok" option should be updated to reuse the same
protection used by the updated "git checkout".

Something to work on for 2.5.x maintenance track, aiming for 2.5.1.

Thanks.

Re: A few "linked checkout" niggles

[Duy Nguyen]

On Thu, Jul 16, 2015 at 6:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I've tried to set up a non-bare clone of git.git at ~/w/src and
> attached one of its branches to a separate work tree at ~/w/rerere
>
>  ~/w/src$ git worktree add ../rerere jc/rerere
>
> Then I tried the "multiple checkout not allowed".
>
>  ~/w/src$ git checkout jc/rerere
>  fatal: 'jc/rerere' is already checked out at '/home/gitster/w/rerere/.git'
>
> You see the first "niggle" above.  Do we need "/.git" at the end?
> That is hopefully merely cosmetic and I am guessing that we can
> unconditionally drop "/.git" from the end, but maybe there are
> subtle reasons why it may not be a good idea (e.g. introduces some
> ambiguities?)
>
> There is another, a more serious one.
>
>  ~/w/src$ git fetch ../git.git jc/rerere:jc/rerere
>
> This succeeds, without checking if there is a work tree that has it
> checked out.  I think the safety in "git fetch" that can be disabled
> by "--update-head-ok" option should be updated to reuse the same
> protection used by the updated "git checkout".
>
> Something to work on for 2.5.x maintenance track, aiming for 2.5.1.

To keep things clear, Eric if you want these they are yours :-) I'll
focus on the submodule support after i'm done with all my bug fix
patches.
-- 
Duy

Re: A few "linked checkout" niggles

[Eric Sunshine]

On Thu, Jul 16, 2015 at 1:24 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Thu, Jul 16, 2015 at 6:13 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> I've tried to set up a non-bare clone of git.git at ~/w/src and
>> attached one of its branches to a separate work tree at ~/w/rerere
>>
>>  ~/w/src$ git worktree add ../rerere jc/rerere
>>
>> Then I tried the "multiple checkout not allowed".
>>
>>  ~/w/src$ git checkout jc/rerere
>>  fatal: 'jc/rerere' is already checked out at '/home/gitster/w/rerere/.git'
>>
>> You see the first "niggle" above.  Do we need "/.git" at the end?
>> That is hopefully merely cosmetic and I am guessing that we can
>> unconditionally drop "/.git" from the end, but maybe there are
>> subtle reasons why it may not be a good idea (e.g. introduces some
>> ambiguities?)

I can't think of any ambiguities which might arise from stripping the
.git suffix. Even when playing with GIT_COMMON_DIR and GIT_WORK_TREE,
I don't notice any ambiguities.

Since I'm already touching this code and am wrapping up v2 of [1], I
can easily plop in a fix for this cosmetic issue.

>> There is another, a more serious one.
>>
>>  ~/w/src$ git fetch ../git.git jc/rerere:jc/rerere
>>
>> This succeeds, without checking if there is a work tree that has it
>> checked out.  I think the safety in "git fetch" that can be disabled
>> by "--update-head-ok" option should be updated to reuse the same
>> protection used by the updated "git checkout".
>>
>> Something to work on for 2.5.x maintenance track, aiming for 2.5.1.
>
> To keep things clear, Eric if you want these they are yours :-) I'll
> focus on the submodule support after i'm done with all my bug fix
> patches.

I want to focus on getting v2 of [1] out and finalized. After that, I
might be able to take a look at the git-fetch issue (but I won't
complain if you get to it first).

[1]: http://thread.gmane.org/gmane.comp.version-control.git/273856

Re: A few "linked checkout" niggles

[Junio C Hamano]

Also in a linked checkout of git.git itself, t5601.21 seems to fail
with:

fatal: Not a git repository: /home/gitster/w/src/.git/worktrees/rerere
not ok 21 - clone respects global branch.autosetuprebase
#
#               (
#                       test_config="$HOME/.gitconfig" &&
#                       git config -f "$test_config" branch.autosetuprebase remote &&
#                       rm -fr dst &&
#                       git clone src dst &&
#                       cd dst &&
#                       actual="z$(git config branch.master.rebase)" &&
#                       test ztrue = $actual
#               )
#

This test is running in /home/gitster/w/rerere, whose .git points at
that directory the fatail: message complains about.

Re: A few "linked checkout" niggles

[Duy Nguyen]

On Fri, Jul 17, 2015 at 3:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Also in a linked checkout of git.git itself, t5601.21 seems to fail
> with:
>
> fatal: Not a git repository: /home/gitster/w/src/.git/worktrees/rerere
> not ok 21 - clone respects global branch.autosetuprebase
> #
> #               (
> #                       test_config="$HOME/.gitconfig" &&
> #                       git config -f "$test_config" branch.autosetuprebase remote &&
> #                       rm -fr dst &&
> #                       git clone src dst &&
> #                       cd dst &&
> #                       actual="z$(git config branch.master.rebase)" &&
> #                       test ztrue = $actual
> #               )
> #
>
> This test is running in /home/gitster/w/rerere, whose .git points at
> that directory the fatail: message complains about.

I can't reproduce with either master, next or pu. Seems strange that
git.git info is leaking into this test..
-- 
Duy

Re: A few "linked checkout" niggles

[Eric Sunshine]

On Thu, Jul 16, 2015 at 7:13 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, Jul 17, 2015 at 3:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Also in a linked checkout of git.git itself, t5601.21 seems to fail
>> with:
>>
>> fatal: Not a git repository: /home/gitster/w/src/.git/worktrees/rerere
>> not ok 21 - clone respects global branch.autosetuprebase
>> #
>> #               (
>> #                       test_config="$HOME/.gitconfig" &&
>> #                       git config -f "$test_config" branch.autosetuprebase remote &&
>> #                       rm -fr dst &&
>> #                       git clone src dst &&
>> #                       cd dst &&
>> #                       actual="z$(git config branch.master.rebase)" &&
>> #                       test ztrue = $actual
>> #               )
>> #
>>
>> This test is running in /home/gitster/w/rerere, whose .git points at
>> that directory the fatail: message complains about.
>
> I can't reproduce with either master, next or pu. Seems strange that
> git.git info is leaking into this test..

I also am unable to reproduce the failure...

Re: A few "linked checkout" niggles

[Michael J Gruber]

Two more observations:

$ git worktree add /tmp/gitwt
Enter /tmp/gitwt (identifier gitwt)
Switched to a new branch 'gitwt'

Now I'm in /tmp/gitwt at branch gitwt. Right? No. I'm in the original wd
at the original branch.

So either we cd to the new location or quelch these messages or add a
message that we're actually back.

The other:

linked worktrees are prefect for short term tests on detached heads.
Even the manual page advertises detach. But:

$ git worktree add  --detach /tmp/gitwtt
Enter /tmp/gitwtt (identifier gitwtt)
fatal: '--detach' cannot be used with '-b/-B/--orphan'

Maybe it's the auto-naming logic stepping on detached toes ;)

Michael

Re: A few "linked checkout" niggles

[Duy Nguyen]

On Fri, Jul 17, 2015 at 7:27 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Two more observations:
>
> $ git worktree add /tmp/gitwt
> Enter /tmp/gitwt (identifier gitwt)
> Switched to a new branch 'gitwt'
>
> Now I'm in /tmp/gitwt at branch gitwt. Right? No. I'm in the original wd
> at the original branch.
>
> So either we cd to the new location or quelch these messages or add a
> message that we're actually back.

s/Enter/Preparing/ ?

> The other:
>
> linked worktrees are prefect for short term tests on detached heads.
> Even the manual page advertises detach. But:
>
> $ git worktree add  --detach /tmp/gitwtt
> Enter /tmp/gitwtt (identifier gitwtt)
> fatal: '--detach' cannot be used with '-b/-B/--orphan'
>
> Maybe it's the auto-naming logic stepping on detached toes ;)

Problem with master (but not with pu) because "-b gitwtt" is
automatically added. I think this is 1eb07d8 (worktree: add:
auto-vivify new branch when <branch> is omitted - 2015-07-06)
forgetting about --detach. Actually it's still a problem with 'pu'
because the branch 'gitwtt' is created unnecessarily.
-- 
Duy

Re: A few "linked checkout" niggles

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> On Fri, Jul 17, 2015 at 7:27 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Two more observations:
>>
>> $ git worktree add /tmp/gitwt
>> Enter /tmp/gitwt (identifier gitwt)
>> Switched to a new branch 'gitwt'
>>
>> Now I'm in /tmp/gitwt at branch gitwt. Right? No. I'm in the original wd
>> at the original branch.
>>
>> So either we cd to the new location or quelch these messages or add a
>> message that we're actually back.
>
> s/Enter/Preparing/ ?

Hmm, do we even need that message?  I assumed it was a leftover
debugging stuff ;-)

Re: A few "linked checkout" niggles

[Eric Sunshine]

On Fri, Jul 17, 2015 at 8:49 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, Jul 17, 2015 at 7:27 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Two more observations:
>>
>> $ git worktree add /tmp/gitwt
>> Enter /tmp/gitwt (identifier gitwt)
>> Switched to a new branch 'gitwt'
>>
>> Now I'm in /tmp/gitwt at branch gitwt. Right? No. I'm in the original wd
>> at the original branch.
>>
>> So either we cd to the new location or quelch these messages or add a
>> message that we're actually back.
>
> s/Enter/Preparing/ ?

Presumably you're using 'master'. The "Switched to a new branch"
message is an artifact of using git-checkout for the back-end
operation, however, what is queued in 'pu' no long uses git-checkout
at all, so that message is gone.

I agree with Duy that s/Enter/Prepare/ would be a good idea. I don't
think we want to drop the message altogether as Junio suggested since
other messages such as "Checking out files:..." and "HEAD is now
at...." follow it, thus "Prepare..." provides context indicating that
those additional messages apply to the new worktree rather than the
current one.

>> The other:
>>
>> linked worktrees are prefect for short term tests on detached heads.
>> Even the manual page advertises detach. But:
>>
>> $ git worktree add  --detach /tmp/gitwtt
>> Enter /tmp/gitwtt (identifier gitwtt)
>> fatal: '--detach' cannot be used with '-b/-B/--orphan'
>>
>> Maybe it's the auto-naming logic stepping on detached toes ;)
>
> Problem with master (but not with pu) because "-b gitwtt" is
> automatically added. I think this is 1eb07d8 (worktree: add:
> auto-vivify new branch when <branch> is omitted - 2015-07-06)
> forgetting about --detach. Actually it's still a problem with 'pu'
> because the branch 'gitwtt' is created unnecessarily.

Yep. Again, that fatal error is an artifact from git-checkout being
used as the back-end on 'master'. What is queued on 'pu' doesn't fail,
however, as Duy notes, the auto-vivication incorrectly kicks in and
creates a new branch unnecessarily. That'll have to be fixed in v3.

Re: A few "linked checkout" niggles

[Eric Sunshine]

On Fri, Jul 17, 2015 at 11:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Fri, Jul 17, 2015 at 7:27 PM, Michael J Gruber
>> <git@drmicha.warpmail.net> wrote:
>>> Two more observations:
>>>
>>> $ git worktree add /tmp/gitwt
>>> Enter /tmp/gitwt (identifier gitwt)
>>> Switched to a new branch 'gitwt'
>>>
>>> Now I'm in /tmp/gitwt at branch gitwt. Right? No. I'm in the original wd
>>> at the original branch.
>>>
>>> So either we cd to the new location or quelch these messages or add a
>>> message that we're actually back.
>>
>> s/Enter/Preparing/ ?
>
> Hmm, do we even need that message?  I assumed it was a leftover
> debugging stuff ;-)

It helps give context to messages which follow it, such as "Checking
out files:..." and "HEAD is now at...", letting the user know that
those messages apply to the new worktree rather than the current one.
In this case, s/Enter/Preparing/ makes sense.

("Switched to a new branch..." is gone in 'pu'.)