Re: [PATCH v3 0/2] worktree: Support `--orphan` when creating new worktrees

[Phillip Wood <phillip.wood123@gmail.com>]

On 19/11/2022 03:47, Jacob Abel wrote:
> On 22/11/17 11:00AM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Tue, Nov 15 2022, Eric Sunshine wrote:
>>
>>> On Thu, Nov 10, 2022 at 6:32 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
>>>> While working with the worktree based git workflow, I realised that setting
>>>> up a new git repository required switching between the traditional and
>>>> worktree based workflows. Searching online I found a SO answer [1] which
>>>> seemed to support this and which indicated that adding support for this should
>>>> not be technically difficult.
>>>>
>>>>    * adding orphan branch functionality (as is present in `git-switch`)
>>>>      to `git-worktree-add`
>>>
>>> I haven't had a chance yet to read v3, but can we take a step back for
>>> a moment and look at this topic from a slightly different angle?
>>> Setting aside the value of adding --orphan to `git worktree add`
>>> (which, I'm perfectly fine with, as mentioned earlier), I have a
>>> question about whether the solution proposed by this series is the
>>> best we can do.
>>>
>>> As I understand it, the actual problem this series wants to solve is
>>> that it's not possible to create a new worktree from an empty bare
>>> repository; for instance:
>>>
>>>      % git init --bare foo.git
>>>      % git -C foo.git worktree add -b main bar
>>>      Preparing worktree (new branch 'main')
>>>      fatal: not a valid object name: 'HEAD'
>>>      %
>>>
>>> This series addresses that shortcoming by adding --orphan, so that the
>>> following works:
>>>
>>>      % git init --bare foo.git
>>>      % git -C foo.git worktree add --orphan main bar
>>>      Preparing worktree (new branch 'main')
>>>      %
>>>
>>> However, is this really the best and most user-friendly and most
>>> discoverable solution? Is it likely that users are somehow going to
>>> instinctively use --orphan when they see the "fatal: not a valid
>>> object name: 'HEAD'" error message?
>>>
>>> Wouldn't a better solution be to somehow fix `git worktree add -b
>>> <branch>` so that it just works rather than erroring out? I haven't
>>> delved into the implementation to determine if this is possible, but
>>> if it is, it seems a far superior "fix" for the problem shown above
>>> since it requires no extra effort on the user's part, and doesn't
>>> raise any discoverability red-flags (since nothing needs to be
>>> "discovered" if `-b <branch>` works as expected in the first place).
>>>
>>> If fixing `-b <branch>` to "just work" is possible, then --orphan is
>>> no longer a needed workaround but becomes "icing on the cake".
>>
>> That's a really good point, and we *could* "fix" that.
>>
>> But I don't see how to do it without overloading "-b" even further, in a
>> way that some users either might not mean, or at least would be
>> confusing.
>>
>> E.g. one script "manually clones" a repo because it does "git init",
>> "git remote set-url", "git fetch" etc. Another one makes worktrees from
>> those fresh checkouts once set up.
>>
>> If we "DWYM" here that second step will carry forward the bad state
>> instead of erroring early.

Wouldn't the first script error out if there was a problem?

>> I haven't fully thought this throuh, so maybe it's fine, just
>> wondering...
>>
>> ...an alternate way to perhaps to do this would be to detect this
>> situation in add(), and emit an advise() telling the user that maybe
>> they want to use "--orphan" for this?
>>
> 
> Prior to writing this patch, I tried to determine if there was a succinct way
> to make `-b` "just work" however I wasn't able to find one that wouldn't
> introduce unintuitive behavior.

Can you say a bit more about what the unintuitive behavior was? As I 
understand it the problem is that "git branch" errors out when HEAD is a 
symbolic ref pointing to a ref that does not exist. I think we can use 
read_ref() to check for that before running "git branch" and act 
accordingly. We might want to check if HEAD matches init.defaultBranch 
and only do an orphan checkout in the new worktree in that case.

> My conclusion was that it was probably best
> to break it out into a separate command as the other tools had.
> 
> I'd support adding an `advise()` for at least the basic case where you try to
> create a worktree and no branches currently exist in the repository.
> i.e. something like this:
> 
>      % git init --bare foo.git
>      % git -C foo.git branch --list
> 
>      % git -C foo.git worktree add foobar/
>      hint: If you meant to create a new initial branch for this repository,
>      hint: e.g. 'main', you can do so using the --orphan option:
>      hint:
>      hint:   git worktree add --orphan main main/
>      hint:
>      fatal: invalid reference: 'foobar'
> 
> and
> 
>      % git init --bare foo.git
>      % git -C foo.git --no-pager branch --list
> 
>      % git -C foo.git worktree add -b foobar foobardir/
>      hint: If you meant to create a new initial branch for this repository,
>      hint: e.g. 'main', you can do so using the --orphan option:
>      hint:
>      hint:   git worktree add --orphan main main/
>      hint:
>      fatal: invalid reference: 'foobar'
> 
> but not in the following circumstances:
> 
>      % git init --bare foo.git
>      % ...
>      % git -C foo.git --no-pager branch --list
>      + foo
>        bar
>      % git -C foo.git worktree add foobar/
>      Preparing worktree (new branch 'foobar')
>      HEAD is now at 319605f8f0 This is a commit message
> 
> or
> 
>      % git init --bare foo.git
>      % ...
>      % git -C foo.git --no-pager branch --list
>      + foo
>        bar
>      % git -C foo.git worktree add -b foobar foobardir/
>      Preparing worktree (new branch 'foobar)
>      HEAD is now at 319605f8f0 This is a commit message
> 
> Would there be any other circumstances where we'd definitely want an `advise()`?
> Generally I'd assume that outside of those two circumstances, most users will
> rarely intend to make an orphan without already knowing they absolutely need to
> make an orphan.

I don't think it matters if the repository is bare so I think it would 
be good to advise() on

	% git init foo
	% git -C foo worktree add bar

Best Wishes

Phillip