diff mbox series

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

Message ID 20221220023637.29042-4-jacobabel@nullpo.dev (mailing list archive)
State Superseded
Headers show
Series worktree: Support `--orphan` when creating new worktrees | expand

Commit Message

Jacob Abel Dec. 20, 2022, 2:38 a.m. UTC
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')
%

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-worktree.txt | 15 +++++++++
 builtin/worktree.c             | 59 ++++++++++++++++++++++++++++++----
 t/t2400-worktree-add.sh        | 53 ++++++++++++++++++++++++++++++
 3 files changed, 120 insertions(+), 7 deletions(-)

--
2.38.2

Comments

Junio C Hamano Dec. 20, 2022, 4:19 a.m. UTC | #1
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?

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.

> 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.
Jacob Abel Dec. 21, 2022, 12:17 a.m. UTC | #2
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/
diff mbox series

Patch

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 4dd658012b..c6e6899d8b 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,8 @@  SYNOPSIS
 [verse]
 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
 		   [[-b | -B] <new-branch>] <path> [<commit-ish>]
+'git worktree add' [-f] [--lock [--reason <string>]]
+		   --orphan <new-branch> <path>
 'git worktree list' [-v | --porcelain [-z]]
 'git worktree lock' [--reason <string>] <worktree>
 'git worktree move' <worktree> <new-path>
@@ -95,6 +97,15 @@  exist, a new branch based on `HEAD` is automatically created as if
 `-b <branch>` was given.  If `<branch>` does exist, it will be checked out
 in the new worktree, if it's not checked out anywhere else, otherwise the
 command will refuse to create the worktree (unless `--force` is used).
++
+------------
+$ git worktree add --orphan <branch> <path>
+------------
++
+Create a worktree containing no files, with an empty index, and associated
+with a new orphan branch named `<branch>`. The first commit made on this new
+branch will have no parents and will be the root of a new history disconnected
+from any other branches.

 list::

@@ -222,6 +233,10 @@  This can also be set up as the default behaviour by using the
 	With `prune`, do not remove anything; just report what it would
 	remove.

+--orphan <new-branch>::
+	With `add`, make the new worktree and index empty, associating
+	the worktree with a new orphan branch named `<new-branch>`.
+
 --porcelain::
 	With `list`, output in an easy-to-parse format for scripts.
 	This format will remain stable across Git versions and regardless of user
diff --git a/builtin/worktree.c b/builtin/worktree.c
index fccb17f070..194c3ccabf 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -17,7 +17,10 @@ 

 #define BUILTIN_WORKTREE_ADD_USAGE \
 	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
-	   "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]")
+	   "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]"), \
+	N_("git worktree add [-f] [--lock [--reason <string>]]\n" \
+	   "                 --orphan <new-branch> <path>")
+
 #define BUILTIN_WORKTREE_LIST_USAGE \
 	N_("git worktree list [-v | --porcelain [-z]]")
 #define BUILTIN_WORKTREE_LOCK_USAGE \
@@ -90,6 +93,7 @@  struct add_opts {
 	int detach;
 	int quiet;
 	int checkout;
+	int orphan;
 	const char *keep_locked;
 };

@@ -364,6 +368,22 @@  static int checkout_worktree(const struct add_opts *opts,
 	return run_command(&cp);
 }

+static int make_worktree_orphan(const char * ref, const struct add_opts *opts,
+				struct strvec *child_env)
+{
+	struct strbuf symref = STRBUF_INIT;
+	struct child_process cp = CHILD_PROCESS_INIT;
+
+	validate_new_branchname(ref, &symref, 0);
+	strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
+	if (opts->quiet)
+		strvec_push(&cp.args, "--quiet");
+	strvec_pushv(&cp.env, child_env->v);
+	strbuf_release(&symref);
+	cp.git_cmd = 1;
+	return run_command(&cp);
+}
+
 static int add_worktree(const char *path, const char *refname,
 			const struct add_opts *opts)
 {
@@ -393,7 +413,7 @@  static int add_worktree(const char *path, const char *refname,
 			die_if_checked_out(symref.buf, 0);
 	}
 	commit = lookup_commit_reference_by_name(refname);
-	if (!commit)
+	if (!commit && !opts->orphan)
 		die(_("invalid reference: %s"), refname);

 	name = worktree_basename(path, &len);
@@ -482,10 +502,10 @@  static int add_worktree(const char *path, const char *refname,
 	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
 	cp.git_cmd = 1;

-	if (!is_branch)
+	if (!is_branch && commit) {
 		strvec_pushl(&cp.args, "update-ref", "HEAD",
 			     oid_to_hex(&commit->object.oid), NULL);
-	else {
+	} else {
 		strvec_pushl(&cp.args, "symbolic-ref", "HEAD",
 			     symref.buf, NULL);
 		if (opts->quiet)
@@ -497,6 +517,10 @@  static int add_worktree(const char *path, const char *refname,
 	if (ret)
 		goto done;

+	if (opts->orphan &&
+	    (ret = make_worktree_orphan(refname, opts, &child_env)))
+		goto done;
+
 	if (opts->checkout &&
 	    (ret = checkout_worktree(opts, &child_env)))
 		goto done;
@@ -516,7 +540,7 @@  static int add_worktree(const char *path, const char *refname,
 	 * Hook failure does not warrant worktree deletion, so run hook after
 	 * is_junk is cleared, but do return appropriate code when hook fails.
 	 */
-	if (!ret && opts->checkout) {
+	if (!ret && opts->checkout && !opts->orphan) {
 		struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;

 		strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL);
@@ -605,6 +629,7 @@  static int add(int ac, const char **av, const char *prefix)
 	char *path;
 	const char *branch;
 	const char *new_branch = NULL;
+	const char *orphan_branch = NULL;
 	const char *opt_track = NULL;
 	const char *lock_reason = NULL;
 	int keep_locked = 0;
@@ -616,6 +641,8 @@  static int add(int ac, const char **av, const char *prefix)
 			   N_("create a new branch")),
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
 			   N_("create or reset a branch")),
+		OPT_STRING(0, "orphan", &orphan_branch, N_("branch"),
+			   N_("new unparented branch")),
 		OPT_BOOL('d', "detach", &opts.detach, N_("detach HEAD at named commit")),
 		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
 		OPT_BOOL(0, "lock", &keep_locked, N_("keep the new working tree locked")),
@@ -633,8 +660,20 @@  static int add(int ac, const char **av, const char *prefix)
 	memset(&opts, 0, sizeof(opts));
 	opts.checkout = 1;
 	ac = parse_options(ac, av, prefix, options, git_worktree_add_usage, 0);
+	opts.orphan = !!orphan_branch;
 	if (!!opts.detach + !!new_branch + !!new_branch_force > 1)
 		die(_("options '%s', '%s', and '%s' cannot be used together"), "-b", "-B", "--detach");
+	if (!!opts.detach + !!opts.orphan + !!new_branch + !!new_branch_force > 1)
+		die(_("options '%s', '%s', '%s', and '%s' cannot be used together"),
+		    "-b", "-B", "--orphan", "--detach");
+	if (opts.orphan && opt_track)
+		die(_("'%s' and '%s' cannot be used together"), "--orphan", "--track");
+	if (opts.orphan && !opts.checkout)
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    "--no-checkout");
+	if (opts.orphan && ac == 2)
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    _("<commit-ish>"));
 	if (lock_reason && !keep_locked)
 		die(_("the option '%s' requires '%s'"), "--reason", "--lock");
 	if (lock_reason)
@@ -663,7 +702,9 @@  static int add(int ac, const char **av, const char *prefix)
 		strbuf_release(&symref);
 	}

-	if (ac < 2 && !new_branch && !opts.detach) {
+	if (opts.orphan) {
+		new_branch = orphan_branch;
+	} else if (ac < 2 && !new_branch && !opts.detach) {
 		const char *s = dwim_branch(path, &new_branch);
 		if (s)
 			branch = s;
@@ -686,7 +727,11 @@  static int add(int ac, const char **av, const char *prefix)
 	if (!opts.quiet)
 		print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);

-	if (new_branch) {
+	if (opts.orphan) {
+		branch = new_branch;
+	} else if (!lookup_commit_reference_by_name(branch)) {
+		die(_("invalid reference: %s"), branch);
+	} else if (new_branch) {
 		struct child_process cp = CHILD_PROCESS_INIT;
 		cp.git_cmd = 1;
 		strvec_push(&cp.args, "branch");
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index f05e2483c2..73ad26651e 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -309,6 +309,11 @@  test_wt_add_excl() {
 test_wt_add_excl -b poodle -B poodle bamboo main
 test_wt_add_excl -b poodle --detach bamboo main
 test_wt_add_excl -B poodle --detach bamboo main
+test_wt_add_excl -B poodle --orphan poodle bamboo
+test_wt_add_excl -b poodle --orphan poodle bamboo
+test_wt_add_excl --orphan poodle --detach bamboo
+test_wt_add_excl --orphan poodle --no-checkout bamboo
+test_wt_add_excl --orphan poodle bamboo main

 test_expect_success '"add -B" fails if the branch is checked out' '
 	git rev-parse newmain >before &&
@@ -330,6 +335,46 @@  test_expect_success 'add --quiet' '
 	test_must_be_empty actual
 '

+test_expect_success '"add --orphan"' '
+	test_when_finished "git worktree remove -f -f orphandir" &&
+	git worktree add --orphan neworphan orphandir &&
+	echo refs/heads/neworphan >expected &&
+	git -C orphandir symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add --orphan" fails if the branch already exists' '
+	test_when_finished "git branch -D existingbranch" &&
+	test_when_finished "git worktree remove -f -f orphandir" &&
+	git worktree add -b existingbranch orphandir main &&
+	test_must_fail git worktree add --orphan existingbranch orphandir2 &&
+	test_path_is_missing orphandir2
+'
+
+test_expect_success '"add --orphan" with empty repository' '
+	test_when_finished "rm -rf empty_repo" &&
+	echo refs/heads/newbranch >expected &&
+	GIT_DIR="empty_repo" git init --bare &&
+	git -C empty_repo  worktree add --orphan newbranch worktreedir &&
+	git -C empty_repo/worktreedir symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add" worktree with orphan branch and lock' '
+	git worktree add --lock --orphan orphanbr orphan-with-lock &&
+	test_when_finished "git worktree unlock orphan-with-lock || :" &&
+	test -f .git/worktrees/orphan-with-lock/locked
+'
+
+test_expect_success '"add" worktree with orphan branch, lock, and reason' '
+	lock_reason="why not" &&
+	git worktree add --detach --lock --reason "$lock_reason" orphan-with-lock-reason main &&
+	test_when_finished "git worktree unlock orphan-with-lock-reason || :" &&
+	test -f .git/worktrees/orphan-with-lock-reason/locked &&
+	echo "$lock_reason" >expect &&
+	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
+'
+
 test_expect_success 'local clone from linked checkout' '
 	git clone --local here here-clone &&
 	( cd here-clone && git fsck )
@@ -446,6 +491,14 @@  setup_remote_repo () {
 	)
 }

+test_expect_success '"add" <path> <remote/branch> w/ no HEAD' '
+	test_when_finished rm -rf repo_upstream repo_local foo &&
+	setup_remote_repo repo_upstream repo_local &&
+	git -C repo_local config --bool core.bare true &&
+	git -C repo_local branch -D main &&
+	git -C repo_local worktree add ./foo repo_upstream/foo
+'
+
 test_expect_success '--no-track avoids setting up tracking' '
 	test_when_finished rm -rf repo_upstream repo_local foo &&
 	setup_remote_repo repo_upstream repo_local &&