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

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

 



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@xxxxxxxxxx> 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



[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]

  Powered by Linux