Re: [PATCH v5 3/4] worktree add: add --orphan flag

[Jacob Abel <jacobabel@nullpo.dev>]

On 22/12/20 01:19PM, Junio C Hamano wrote:
> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > Adds support for creating an orphan branch when adding a new worktree.
> > This functionality is equivalent to git switch's --orphan flag.
> >
> > The original reason this feature was implemented was to allow a user
> > to initialise a new repository using solely the worktree oriented
> > workflow.
> >
> > Current Behavior:
> >
> > % git init --bare foo.git
> > Initialized empty Git repository in /path/to/foo.git/
> > % git -C foo.git worktree add main/
> > Preparing worktree (new branch 'main')
> > fatal: not a valid object name: 'HEAD'
> > %
> >
> > New Behavior:
> >
> > % git init --bare foo.git
> > Initialized empty Git repository in /path/to/foo.git/
> > % git -C foo.git worktree add --orphan main main/
> > Preparing worktree (new branch 'main')
> > %
>
> Hmph, I suspect that in reviews for the previous rounds someboddy
> must have brought this up, but I wonder if we can detect the case
> automatically and behave as if "--orphan" were given.  In other
> words, shouldn't the desired outcome (i.e. "worktree add" can be run
> to create an orphan branch even when the original were on an unborn
> branch) become the new behaviour WITHOUT having the user learn new
> option?

Yes. Other reviewers have suggested trying to DWYM with the `--orphan` behavior.
I have been hesitant to add that functionality as in my opinion it can lead to
confusing behavior. This is largely because in many cases, where we could want
`--orphan` to DWYM would overlap with a user making a mistake with the more
common uses of `git worktree add`.

I'm not opposed to adding this DWYM behavior in another patch but I think it
might be worth waiting to do so. I'm looking at working on another patchset
after this one which would better illustrate what decisions `git worktree add`
makes when it tries to DWYM. In my opinion, after that patchset would probably
be the best time to try and integrate `--orphan` behavior into DWYM.

>
> If the point of "--orphan" is to create a worktree that checks out a
> yet-to-be-bork branch, whether the original is an empty repository
> or a populated one, then the user may need "--orphan" option, but
> the above illustration is very misleading if that is the intention.
>
> Rather, you should start from a populated repository and explain
> that "worktree add" without "--orphan" (i.e. the current behaviour)
> does not give you an empty tree with empty history, so run an extra
> "switch --orphan" after that.  Then illustrate that you can lose the
> last step with the new option "--orphan".
>
> Or something like that.

Understood. I've updated the commit message.

>
> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> This e-mail coming from you and not from Ævar, with you apparently
> being the author of the patch, makes these two S-o-b lines
> questionable.  What's the chain of custody of the change?  If Ævar
> showed some code changes, and you swallowed that into a larger piece
> of work (i.e. this patch), then the above two lines are swapped.
>

Ah, yes. They provided a fairly substantial fixup patch against this patch
during an earlier revision[1]. I integrated it into 3/4 and added that S-o-b as
requested here[2].

I've swapped the S-o-b lines in the commit message.

The diff on the commit message is below. Apologies about the formatting. I
wasn't sure how to get the commit text diff with `git diff` so I used normal
`diff` instead.

--- 3-of-4-v5   2022-12-20 18:49:43.007548775 -0500
+++ 3-of-4-v6   2022-12-20 19:14:38.296292361 -0500
@@ -1,28 +1,48 @@
 worktree add: add --orphan flag

 Adds support for creating an orphan branch when adding a new worktree.
 This functionality is equivalent to git switch's --orphan flag.

 The original reason this feature was implemented was to allow a user
 to initialise a new repository using solely the worktree oriented
 workflow.

 Current Behavior:
-
-% git init --bare foo.git
-Initialized empty Git repository in /path/to/foo.git/
+% git -C foo.git --no-pager branch -l
++ main
 % git -C foo.git worktree add main/
 Preparing worktree (new branch 'main')
+HEAD is now at 6c93a75 a commit
+%
+
+% git init bar.git
+Initialized empty Git repository in /path/to/bar.git/
+% git -C bar.git --no-pager branch -l
+
+% git -C bar.git worktree add main/
+Preparing worktree (new branch 'main')
 fatal: not a valid object name: 'HEAD'
 %

 New Behavior:

-% git init --bare foo.git
-Initialized empty Git repository in /path/to/foo.git/
-% git -C foo.git worktree add --orphan main main/
+% git -C foo.git --no-pager branch -l
++ main
+% git -C foo.git worktree add main/
+Preparing worktree (new branch 'main')
+HEAD is now at 6c93a75 a commit
+%
+
+% git init --bare bar.git
+Initialized empty Git repository in /path/to/bar.git/
+% git -C bar.git --no-pager branch -l
+
+% git -C bar.git worktree add main/
+Preparing worktree (new branch 'main')
+fatal: invalid reference: HEAD
+% git -C bar.git worktree add --orphan main main/
 Preparing worktree (new branch 'main')
 %

-Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
 Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>

1. https://lore.kernel.org/git/221115.86edu3kfqz.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/221119.861qpzf9ey.gmgdl@evledraar.gmail.com/