[PATCH v3 00/22] rid git-checkout of too-intimate knowledge of new worktree

[PATCH v3 00/22] rid git-checkout of too-intimate knowledge of new worktree

[Eric Sunshine]

This is v3 of [1] which rids git-checkout of specialized knowledge that
it's operating in a newly created linked worktree. Thanks to Junio for
his review, and Duy and Michael J Gruber for additional observations.

A v2 to v3 interdiff is included below.

Changes since v2:

* patch 06/22: strip only trailing ".git" rather than "/.git" from
  "'branch' is already checked out at '/some/patch/.git'" message[2];
  also reword commit message slightly to better emphasize that the
  trailing ".git" is more than a mere cosmetic issue[2]

* patch 10/22 (new): change worktree setup message from "Enter
  <path>..." to "Preparing <path>..." to avoid possibly confusing the
  user into thinking that 'cd <path>' occurred[3]

* patch 13/22: add tests of -b/-B/--detach exclusivity

* patch 14/22 (new): suppress accidental triggering of branch
  auto-vivication with "git worktree add --detach <path>"; instead treat
  it as shorthand for "git worktree add --detach <path> HEAD"[3]

[1]: http://thread.gmane.org/gmane.comp.version-control.git/274024
[2]: http://article.gmane.org/gmane.comp.version-control.git/274035
[3]: http://article.gmane.org/gmane.comp.version-control.git/274083

Eric Sunshine (22):
  checkout: avoid resolving HEAD unnecessarily
  checkout: name check_linked_checkouts() more meaningfully
  checkout: improve die_if_checked_out() robustness
  checkout: die_if_checked_out: simplify strbuf management
  checkout: generalize die_if_checked_out() branch name argument
  checkout: check_linked_checkout: improve "already checked out"
    aesthetic
  checkout: check_linked_checkout: simplify symref parsing
  checkout: teach check_linked_checkout() about symbolic link HEAD
  branch: publish die_if_checked_out()
  worktree: improve worktree setup message
  worktree: simplify new branch (-b/-B) option checking
  worktree: introduce options container
  worktree: make --detach mutually exclusive with -b/-B
  worktree: add: suppress auto-vivication with --detach and no <branch>
  worktree: make branch creation distinct from worktree population
  worktree: elucidate environment variables intended for child processes
  worktree: add_worktree: construct worktree-population command locally
  worktree: detect branch-name/detached and error conditions locally
  worktree: make setup of new HEAD distinct from worktree population
  worktree: avoid resolving HEAD unnecessarily
  worktree: populate via "git reset --hard" rather than "git checkout"
  checkout: drop intimate knowledge of newly created worktree

 branch.c                |  67 ++++++++++++++++++++++++++
 branch.h                |   7 +++
 builtin/checkout.c      |  82 +++-----------------------------
 builtin/worktree.c      | 123 ++++++++++++++++++++++++++++++++----------------
 t/t2025-worktree-add.sh |  34 +++++++++++++
 5 files changed, 197 insertions(+), 116 deletions(-)

-- 
2.5.0.rc2.378.g0af52e8

---- 8< ----
diff --git a/branch.c b/branch.c
index dfd7698..af9480f 100644
--- a/branch.c
+++ b/branch.c
@@ -347,7 +347,7 @@ static void check_linked_checkout(const char *branch, const char *id)
 	} else
 		strbuf_addstr(&gitdir, get_git_common_dir());
 	skip_prefix(branch, "refs/heads/", &branch);
-	strbuf_strip_suffix(&gitdir, "/.git");
+	strbuf_strip_suffix(&gitdir, ".git");
 	die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
 done:
 	strbuf_release(&path);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 2873064..5d9371c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -264,7 +264,7 @@ static int add_worktree(const char *path, const char *refname,
 	strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
 	write_file(sb.buf, 1, "../..\n");
 
-	fprintf_ln(stderr, _("Enter %s (identifier %s)"), path, name);
+	fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
 
 	argv_array_pushf(&child_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
 	argv_array_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
@@ -335,7 +335,7 @@ static int add(int ac, const char **av, const char *prefix)
 	if (opts.force_new_branch)
 		opts.new_branch = new_branch_force;
 
-	if (ac < 2 && !opts.new_branch) {
+	if (ac < 2 && !opts.new_branch && !opts.detach) {
 		int n;
 		const char *s = worktree_basename(path, &n);
 		opts.new_branch = xstrndup(s, n);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 9e30690..8267411 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -153,6 +153,14 @@ test_expect_success '"add -b" with <branch> omitted' '
 	test_cmp_rev HEAD burble
 '
 
+test_expect_success '"add --detach" with <branch> omitted' '
+	git worktree add --detach fishhook &&
+	git rev-parse HEAD >expected &&
+	git -C fishhook rev-parse HEAD >actual &&
+	test_cmp expected actual &&
+	test_must_fail git -C fishhook symbolic-ref HEAD
+'
+
 test_expect_success '"add" with <branch> omitted' '
 	git worktree add wiffle/bat &&
 	test_cmp_rev HEAD bat
@@ -167,4 +175,22 @@ test_expect_success '"add" auto-vivify does not clobber existing branch' '
 	test_path_is_missing precious
 '
 
+test_expect_success '"add" no auto-vivify with --detach and <branch> omitted' '
+	git worktree add --detach mish/mash &&
+	test_must_fail git rev-parse mash -- &&
+	test_must_fail git -C mish/mash symbolic-ref HEAD
+'
+
+test_expect_success '"add" -b/-B mutually exclusive' '
+	test_must_fail git worktree add -b poodle -B poodle bamboo master
+'
+
+test_expect_success '"add" -b/--detach mutually exclusive' '
+	test_must_fail git worktree add -b poodle --detach bamboo master
+'
+
+test_expect_success '"add" -B/--detach mutually exclusive' '
+	test_must_fail git worktree add -B poodle --detach bamboo master
+'
+
 test_done
---- 8< ----

[PATCH v3 01/22] checkout: avoid resolving HEAD unnecessarily

[Eric Sunshine]

When --ignore-other-worktree is specified, we unconditionally skip the
check to see if the requested branch is already checked out in a linked
worktree. Since we know that we will be skipping that check, there is no
need to resolve HEAD in order to detect other conditions under which we
may skip the check.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/checkout.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5754554..75f90a9 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1145,13 +1145,13 @@ static int checkout_branch(struct checkout_opts *opts,
 		die(_("Cannot switch branch to a non-commit '%s'"),
 		    new->name);
 
-	if (new->path && !opts->force_detach && !opts->new_branch) {
+	if (new->path && !opts->force_detach && !opts->new_branch &&
+	    !opts->ignore_other_worktrees) {
 		unsigned char sha1[20];
 		int flag;
 		char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
 		if (head_ref &&
-		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) &&
-		    !opts->ignore_other_worktrees)
+		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
 			check_linked_checkouts(new);
 		free(head_ref);
 	}
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 02/22] checkout: name check_linked_checkouts() more meaningfully

[Eric Sunshine]

check_linked_checkouts() doesn't just "check" linked checkouts for
"something"; specifically, it aborts the operation if the branch about
to be checked out is already checked out elsewhere. Therefore, rename it
to die_if_checked_out() to give a better indication of its function.
The more meaningful name will be particularly important when this
function is later published for use by other callers.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/checkout.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 75f90a9..e75fb5e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -910,7 +910,7 @@ done:
 	strbuf_release(&gitdir);
 }
 
-static void check_linked_checkouts(struct branch_info *new)
+static void die_if_checked_out(struct branch_info *new)
 {
 	struct strbuf path = STRBUF_INIT;
 	DIR *dir;
@@ -1152,7 +1152,7 @@ static int checkout_branch(struct checkout_opts *opts,
 		char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
 		if (head_ref &&
 		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
-			check_linked_checkouts(new);
+			die_if_checked_out(new);
 		free(head_ref);
 	}
 
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 03/22] checkout: improve die_if_checked_out() robustness

[Eric Sunshine]

die_if_checked_out() is intended to check if the branch about to be
checked out is already checked out either in the main worktree or in a
linked worktree. However, if .git/worktrees directory does not exist,
then it never bothers checking the main worktree, even though the
specified branch might indeed be checked out there, which is fragile
behavior.

This hasn't been a problem in practice since the current implementation
of "git worktree add" (and, earlier, "git checkout --to") always creates
.git/worktrees before die_if_checked_out() is called by the child "git
checkout" invocation which populates the new worktree.

However, git-worktree will eventually want to call die_if_checked_out()
itself rather than only doing so indirectly as a side-effect of invoking
git-checkout, and reliance upon order of operations (creating
.git/worktrees before checking if a branch is already checked out) is
fragile. As a general function, callers should not be expected to abide
by this undocumented and unwarranted restriction. Therefore, make
die_if_checked_out() more robust by checking the main worktree whether
.git/worktrees exists or not.

While here, also move a comment explaining why die_if_checked_out()'s
helper parses HEAD manually. Such information resides more naturally
with the helper itself rather than at its first point of call.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/checkout.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e75fb5e..1992c41 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -880,6 +880,11 @@ static void check_linked_checkout(struct branch_info *new, const char *id)
 	struct strbuf gitdir = STRBUF_INIT;
 	const char *start, *end;
 
+	/*
+	 * $GIT_COMMON_DIR/HEAD is practically outside
+	 * $GIT_DIR so resolve_ref_unsafe() won't work (it
+	 * uses git_path). Parse the ref ourselves.
+	 */
 	if (id)
 		strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
 	else
@@ -916,19 +921,14 @@ static void die_if_checked_out(struct branch_info *new)
 	DIR *dir;
 	struct dirent *d;
 
+	check_linked_checkout(new, NULL);
+
 	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
 	if ((dir = opendir(path.buf)) == NULL) {
 		strbuf_release(&path);
 		return;
 	}
 
-	/*
-	 * $GIT_COMMON_DIR/HEAD is practically outside
-	 * $GIT_DIR so resolve_ref_unsafe() won't work (it
-	 * uses git_path). Parse the ref ourselves.
-	 */
-	check_linked_checkout(new, NULL);
-
 	while ((d = readdir(dir)) != NULL) {
 		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
 			continue;
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 04/22] checkout: die_if_checked_out: simplify strbuf management

[Eric Sunshine]

There is no reason to keep the strbuf active long after its last use.
By releasing it as early as possible, resource management is simplified
and there is less worry about future changes resulting in a leak.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/checkout.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 1992c41..c36bf8a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -924,17 +924,16 @@ static void die_if_checked_out(struct branch_info *new)
 	check_linked_checkout(new, NULL);
 
 	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
-	if ((dir = opendir(path.buf)) == NULL) {
-		strbuf_release(&path);
+	dir = opendir(path.buf);
+	strbuf_release(&path);
+	if (!dir)
 		return;
-	}
 
 	while ((d = readdir(dir)) != NULL) {
 		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
 			continue;
 		check_linked_checkout(new, d->d_name);
 	}
-	strbuf_release(&path);
 	closedir(dir);
 }
 
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 05/22] checkout: generalize die_if_checked_out() branch name argument

[Eric Sunshine]

The plan is to publish die_if_checked_out() so that callers other than
git-checkout can take advantage of it, however, those callers won't have
access to git-checkout's "struct branch_info". Therefore, change it to
accept the full name of the branch as a simple string instead.

While here, also give the argument a more meaningful name ("branch"
instead of "new").

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/checkout.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index c36bf8a..177ad6a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -873,7 +873,7 @@ static const char *unique_tracking_name(const char *name, unsigned char *sha1)
 	return NULL;
 }
 
-static void check_linked_checkout(struct branch_info *new, const char *id)
+static void check_linked_checkout(const char *branch, const char *id)
 {
 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf path = STRBUF_INIT;
@@ -898,7 +898,7 @@ static void check_linked_checkout(struct branch_info *new, const char *id)
 	end = start;
 	while (*end && !isspace(*end))
 		end++;
-	if (strncmp(start, new->path, end - start) || new->path[end - start] != '\0')
+	if (strncmp(start, branch, end - start) || branch[end - start] != '\0')
 		goto done;
 	if (id) {
 		strbuf_reset(&path);
@@ -908,20 +908,21 @@ static void check_linked_checkout(struct branch_info *new, const char *id)
 		strbuf_rtrim(&gitdir);
 	} else
 		strbuf_addstr(&gitdir, get_git_common_dir());
-	die(_("'%s' is already checked out at '%s'"), new->name, gitdir.buf);
+	skip_prefix(branch, "refs/heads/", &branch);
+	die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
 done:
 	strbuf_release(&path);
 	strbuf_release(&sb);
 	strbuf_release(&gitdir);
 }
 
-static void die_if_checked_out(struct branch_info *new)
+static void die_if_checked_out(const char *branch)
 {
 	struct strbuf path = STRBUF_INIT;
 	DIR *dir;
 	struct dirent *d;
 
-	check_linked_checkout(new, NULL);
+	check_linked_checkout(branch, NULL);
 
 	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
 	dir = opendir(path.buf);
@@ -932,7 +933,7 @@ static void die_if_checked_out(struct branch_info *new)
 	while ((d = readdir(dir)) != NULL) {
 		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
 			continue;
-		check_linked_checkout(new, d->d_name);
+		check_linked_checkout(branch, d->d_name);
 	}
 	closedir(dir);
 }
@@ -1151,7 +1152,7 @@ static int checkout_branch(struct checkout_opts *opts,
 		char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
 		if (head_ref &&
 		    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
-			die_if_checked_out(new);
+			die_if_checked_out(new->path);
 		free(head_ref);
 	}
 
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 06/22] checkout: check_linked_checkout: improve "already checked out" aesthetic

[Eric Sunshine]

When check_linked_checkout() discovers that the branch is already
checked out elsewhere, it emits the diagnostic:

    'blorp' is already checked out at '/some/path/.git'

which is misleading since "checked out at" implies the working tree, but
".git" is the location of the repository administrative files. Fix by
dropping ".git" from the message.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

Changes since v2: strip only trailing ".git" rather than "/.git" from
"already checked" diagnostic[1]; reword commit message to better
emphasize that trailing ".git" is more than a mere cosmetic issue[1]

[1]: http://article.gmane.org/gmane.comp.version-control.git/274035

 builtin/checkout.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 177ad6a..de6619f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -909,6 +909,7 @@ static void check_linked_checkout(const char *branch, const char *id)
 	} else
 		strbuf_addstr(&gitdir, get_git_common_dir());
 	skip_prefix(branch, "refs/heads/", &branch);
+	strbuf_strip_suffix(&gitdir, ".git");
 	die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
 done:
 	strbuf_release(&path);
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 07/22] checkout: check_linked_checkout: simplify symref parsing

[Eric Sunshine]

check_linked_checkout() only understands symref-style HEAD (i.e. "ref:
refs/heads/master"), however, HEAD may also be a an actual symbolic link
(on platforms which support it), thus it will need to check that style
HEAD, as well (via readlink()). As a preparatory step, simplify parsing
of symref-style HEAD so the actual branch check can be re-used easily
for symbolic links (in an upcoming patch).

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/checkout.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index de6619f..6f4e492 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -878,7 +878,6 @@ static void check_linked_checkout(const char *branch, const char *id)
 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf path = STRBUF_INIT;
 	struct strbuf gitdir = STRBUF_INIT;
-	const char *start, *end;
 
 	/*
 	 * $GIT_COMMON_DIR/HEAD is practically outside
@@ -890,15 +889,13 @@ static void check_linked_checkout(const char *branch, const char *id)
 	else
 		strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
 
-	if (strbuf_read_file(&sb, path.buf, 0) < 0 ||
-	    !skip_prefix(sb.buf, "ref:", &start))
+	if (strbuf_read_file(&sb, path.buf, 0) >= 0 &&
+	    starts_with(sb.buf, "ref:")) {
+		strbuf_remove(&sb, 0, strlen("ref:"));
+		strbuf_trim(&sb);
+	} else
 		goto done;
-	while (isspace(*start))
-		start++;
-	end = start;
-	while (*end && !isspace(*end))
-		end++;
-	if (strncmp(start, branch, end - start) || branch[end - start] != '\0')
+	if (strcmp(sb.buf, branch))
 		goto done;
 	if (id) {
 		strbuf_reset(&path);
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 08/22] checkout: teach check_linked_checkout() about symbolic link HEAD

[Eric Sunshine]

check_linked_checkout() only understands symref-style HEAD (i.e. "ref:
refs/heads/master"), however, HEAD may also be a an actual symbolic link
(on platforms which support it). To accurately detect if a branch is
checked out elsewhere, it needs to handle symbolic link HEAD, as well.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/checkout.c      | 6 +++++-
 t/t2025-worktree-add.sh | 8 ++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 6f4e492..f04dcaa 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -889,7 +889,11 @@ static void check_linked_checkout(const char *branch, const char *id)
 	else
 		strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
 
-	if (strbuf_read_file(&sb, path.buf, 0) >= 0 &&
+	if (!strbuf_readlink(&sb, path.buf, 0)) {
+		if (!starts_with(sb.buf, "refs/") ||
+		    check_refname_format(sb.buf, 0))
+			goto done;
+	} else if (strbuf_read_file(&sb, path.buf, 0) >= 0 &&
 	    starts_with(sb.buf, "ref:")) {
 		strbuf_remove(&sb, 0, strlen("ref:"));
 		strbuf_trim(&sb);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index ead8aa2..9e30690 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -83,6 +83,14 @@ test_expect_success 'die the same branch is already checked out' '
 	)
 '
 
+test_expect_success SYMLINKS 'die the same branch is already checked out (symlink)' '
+	head=$(git -C there rev-parse --git-path HEAD) &&
+	ref=$(git -C there symbolic-ref HEAD) &&
+	rm "$head" &&
+	ln -s "$ref" "$head" &&
+	test_must_fail git -C here checkout newmaster
+'
+
 test_expect_success 'not die the same branch is already checked out' '
 	(
 		cd here &&
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 09/22] branch: publish die_if_checked_out()

[Eric Sunshine]

git-worktree currently conflates new branch creation, setting of HEAD in
the new wortkree, and worktree population into a single sub-invocation
of git-checkout. However, these operations will eventually be separated,
and git-worktree itself will need to be able to detect if the branch is
already checked out elsewhere, rather than relying upon git-branch to
make this determination, so publish die_if_checked_out().

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 branch.c           | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 branch.h           |  7 ++++++
 builtin/checkout.c | 67 ------------------------------------------------------
 3 files changed, 74 insertions(+), 67 deletions(-)

diff --git a/branch.c b/branch.c
index 4bab55a..af9480f 100644
--- a/branch.c
+++ b/branch.c
@@ -309,3 +309,70 @@ void remove_branch_state(void)
 	unlink(git_path("MERGE_MODE"));
 	unlink(git_path("SQUASH_MSG"));
 }
+
+static void check_linked_checkout(const char *branch, const char *id)
+{
+	struct strbuf sb = STRBUF_INIT;
+	struct strbuf path = STRBUF_INIT;
+	struct strbuf gitdir = STRBUF_INIT;
+
+	/*
+	 * $GIT_COMMON_DIR/HEAD is practically outside
+	 * $GIT_DIR so resolve_ref_unsafe() won't work (it
+	 * uses git_path). Parse the ref ourselves.
+	 */
+	if (id)
+		strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
+	else
+		strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+
+	if (!strbuf_readlink(&sb, path.buf, 0)) {
+		if (!starts_with(sb.buf, "refs/") ||
+		    check_refname_format(sb.buf, 0))
+			goto done;
+	} else if (strbuf_read_file(&sb, path.buf, 0) >= 0 &&
+	    starts_with(sb.buf, "ref:")) {
+		strbuf_remove(&sb, 0, strlen("ref:"));
+		strbuf_trim(&sb);
+	} else
+		goto done;
+	if (strcmp(sb.buf, branch))
+		goto done;
+	if (id) {
+		strbuf_reset(&path);
+		strbuf_addf(&path, "%s/worktrees/%s/gitdir", get_git_common_dir(), id);
+		if (strbuf_read_file(&gitdir, path.buf, 0) <= 0)
+			goto done;
+		strbuf_rtrim(&gitdir);
+	} else
+		strbuf_addstr(&gitdir, get_git_common_dir());
+	skip_prefix(branch, "refs/heads/", &branch);
+	strbuf_strip_suffix(&gitdir, ".git");
+	die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
+done:
+	strbuf_release(&path);
+	strbuf_release(&sb);
+	strbuf_release(&gitdir);
+}
+
+void die_if_checked_out(const char *branch)
+{
+	struct strbuf path = STRBUF_INIT;
+	DIR *dir;
+	struct dirent *d;
+
+	check_linked_checkout(branch, NULL);
+
+	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+	dir = opendir(path.buf);
+	strbuf_release(&path);
+	if (!dir)
+		return;
+
+	while ((d = readdir(dir)) != NULL) {
+		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
+			continue;
+		check_linked_checkout(branch, d->d_name);
+	}
+	closedir(dir);
+}
diff --git a/branch.h b/branch.h
index 64173ab..58aa45f 100644
--- a/branch.h
+++ b/branch.h
@@ -52,4 +52,11 @@ extern void install_branch_config(int flag, const char *local, const char *origi
  */
 extern int read_branch_desc(struct strbuf *, const char *branch_name);
 
+/*
+ * Check if a branch is checked out in the main worktree or any linked
+ * worktree and die (with a message describing its checkout location) if
+ * it is.
+ */
+extern void die_if_checked_out(const char *branch);
+
 #endif
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f04dcaa..4ae895c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -873,73 +873,6 @@ static const char *unique_tracking_name(const char *name, unsigned char *sha1)
 	return NULL;
 }
 
-static void check_linked_checkout(const char *branch, const char *id)
-{
-	struct strbuf sb = STRBUF_INIT;
-	struct strbuf path = STRBUF_INIT;
-	struct strbuf gitdir = STRBUF_INIT;
-
-	/*
-	 * $GIT_COMMON_DIR/HEAD is practically outside
-	 * $GIT_DIR so resolve_ref_unsafe() won't work (it
-	 * uses git_path). Parse the ref ourselves.
-	 */
-	if (id)
-		strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
-	else
-		strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
-
-	if (!strbuf_readlink(&sb, path.buf, 0)) {
-		if (!starts_with(sb.buf, "refs/") ||
-		    check_refname_format(sb.buf, 0))
-			goto done;
-	} else if (strbuf_read_file(&sb, path.buf, 0) >= 0 &&
-	    starts_with(sb.buf, "ref:")) {
-		strbuf_remove(&sb, 0, strlen("ref:"));
-		strbuf_trim(&sb);
-	} else
-		goto done;
-	if (strcmp(sb.buf, branch))
-		goto done;
-	if (id) {
-		strbuf_reset(&path);
-		strbuf_addf(&path, "%s/worktrees/%s/gitdir", get_git_common_dir(), id);
-		if (strbuf_read_file(&gitdir, path.buf, 0) <= 0)
-			goto done;
-		strbuf_rtrim(&gitdir);
-	} else
-		strbuf_addstr(&gitdir, get_git_common_dir());
-	skip_prefix(branch, "refs/heads/", &branch);
-	strbuf_strip_suffix(&gitdir, ".git");
-	die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
-done:
-	strbuf_release(&path);
-	strbuf_release(&sb);
-	strbuf_release(&gitdir);
-}
-
-static void die_if_checked_out(const char *branch)
-{
-	struct strbuf path = STRBUF_INIT;
-	DIR *dir;
-	struct dirent *d;
-
-	check_linked_checkout(branch, NULL);
-
-	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
-	dir = opendir(path.buf);
-	strbuf_release(&path);
-	if (!dir)
-		return;
-
-	while ((d = readdir(dir)) != NULL) {
-		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
-			continue;
-		check_linked_checkout(branch, d->d_name);
-	}
-	closedir(dir);
-}
-
 static int parse_branchname_arg(int argc, const char **argv,
 				int dwim_new_local_branch_ok,
 				struct branch_info *new,
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 10/22] worktree: improve worktree setup message

[Eric Sunshine]

When git-worktree creates a new worktree, it reports:

    Enter "<path>" (identifier <tag>)

which misleadingly implies that it is setting <path> as the working
directory (as if "cd <path>" had been invoked), whereas it's actually
preparing the new worktree by creating its administrative files, setting
HEAD, and populating it. Make this more clear by instead saying:

    Preparing "<path>" (identifier <tag>)

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

New in v3.

 builtin/worktree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 69248ba..5f0e3c2 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -245,7 +245,7 @@ static int add_worktree(const char *path, const char **child_argv)
 	strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
 	write_file(sb.buf, 1, "../..\n");
 
-	fprintf_ln(stderr, _("Enter %s (identifier %s)"), path, name);
+	fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
 
 	setenv("GIT_CHECKOUT_NEW_WORKTREE", "1", 1);
 	setenv(GIT_DIR_ENVIRONMENT, sb_git.buf, 1);
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 11/22] worktree: simplify new branch (-b/-B) option checking

[Eric Sunshine]

Make 'new_branch' be the name of the new branch for both forced and
non-forced cases; and add boolean 'force_new_branch' to indicate forced
branch creation. This will simplify logic later on when git-worktree
handles branch creation locally rather than delegating it to
git-checkout as part of the worktree population phase.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/worktree.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 5f0e3c2..bf4db9f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -272,7 +272,7 @@ static int add_worktree(const char *path, const char **child_argv)
 
 static int add(int ac, const char **av, const char *prefix)
 {
-	int force = 0, detach = 0;
+	int force = 0, detach = 0, force_new_branch;
 	const char *new_branch = NULL, *new_branch_force = NULL;
 	const char *path, *branch;
 	struct argv_array cmd = ARGV_ARRAY_INIT;
@@ -295,7 +295,11 @@ static int add(int ac, const char **av, const char *prefix)
 	path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];
 	branch = ac < 2 ? "HEAD" : av[1];
 
-	if (ac < 2 && !new_branch && !new_branch_force) {
+	force_new_branch = !!new_branch_force;
+	if (force_new_branch)
+		new_branch = new_branch_force;
+
+	if (ac < 2 && !new_branch) {
 		int n;
 		const char *s = worktree_basename(path, &n);
 		new_branch = xstrndup(s, n);
@@ -305,9 +309,8 @@ static int add(int ac, const char **av, const char *prefix)
 	if (force)
 		argv_array_push(&cmd, "--ignore-other-worktrees");
 	if (new_branch)
-		argv_array_pushl(&cmd, "-b", new_branch, NULL);
-	if (new_branch_force)
-		argv_array_pushl(&cmd, "-B", new_branch_force, NULL);
+		argv_array_pushl(&cmd, force_new_branch ? "-B" : "-b",
+				 new_branch, NULL);
 	if (detach)
 		argv_array_push(&cmd, "--detach");
 	argv_array_push(&cmd, branch);
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 12/22] worktree: introduce options container

[Eric Sunshine]

add_worktree() will eventually need to deal with some options itself, so
introduce a structure into which options can be conveniently bundled,
and pass it along to add_worktree().

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/worktree.c | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index bf4db9f..7d70eb6 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -12,6 +12,13 @@ static const char * const worktree_usage[] = {
 	NULL
 };
 
+struct add_opts {
+	int force;
+	int detach;
+	const char *new_branch;
+	int force_new_branch;
+};
+
 static int show_only;
 static int verbose;
 static unsigned long expire;
@@ -171,7 +178,8 @@ static const char *worktree_basename(const char *path, int *olen)
 	return name;
 }
 
-static int add_worktree(const char *path, const char **child_argv)
+static int add_worktree(const char *path, const char **child_argv,
+			const struct add_opts *opts)
 {
 	struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;
 	struct strbuf sb = STRBUF_INIT;
@@ -272,22 +280,23 @@ static int add_worktree(const char *path, const char **child_argv)
 
 static int add(int ac, const char **av, const char *prefix)
 {
-	int force = 0, detach = 0, force_new_branch;
-	const char *new_branch = NULL, *new_branch_force = NULL;
+	struct add_opts opts;
+	const char *new_branch_force = NULL;
 	const char *path, *branch;
 	struct argv_array cmd = ARGV_ARRAY_INIT;
 	struct option options[] = {
-		OPT__FORCE(&force, N_("checkout <branch> even if already checked out in other worktree")),
-		OPT_STRING('b', NULL, &new_branch, N_("branch"),
+		OPT__FORCE(&opts.force, N_("checkout <branch> even if already checked out in other worktree")),
+		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
 			   N_("create a new branch")),
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
 			   N_("create or reset a branch")),
-		OPT_BOOL(0, "detach", &detach, N_("detach HEAD at named commit")),
+		OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")),
 		OPT_END()
 	};
 
+	memset(&opts, 0, sizeof(opts));
 	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
-	if (new_branch && new_branch_force)
+	if (opts.new_branch && new_branch_force)
 		die(_("-b and -B are mutually exclusive"));
 	if (ac < 1 || ac > 2)
 		usage_with_options(worktree_usage, options);
@@ -295,27 +304,27 @@ static int add(int ac, const char **av, const char *prefix)
 	path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];
 	branch = ac < 2 ? "HEAD" : av[1];
 
-	force_new_branch = !!new_branch_force;
-	if (force_new_branch)
-		new_branch = new_branch_force;
+	opts.force_new_branch = !!new_branch_force;
+	if (opts.force_new_branch)
+		opts.new_branch = new_branch_force;
 
-	if (ac < 2 && !new_branch) {
+	if (ac < 2 && !opts.new_branch) {
 		int n;
 		const char *s = worktree_basename(path, &n);
-		new_branch = xstrndup(s, n);
+		opts.new_branch = xstrndup(s, n);
 	}
 
 	argv_array_push(&cmd, "checkout");
-	if (force)
+	if (opts.force)
 		argv_array_push(&cmd, "--ignore-other-worktrees");
-	if (new_branch)
-		argv_array_pushl(&cmd, force_new_branch ? "-B" : "-b",
-				 new_branch, NULL);
-	if (detach)
+	if (opts.new_branch)
+		argv_array_pushl(&cmd, opts.force_new_branch ? "-B" : "-b",
+				 opts.new_branch, NULL);
+	if (opts.detach)
 		argv_array_push(&cmd, "--detach");
 	argv_array_push(&cmd, branch);
 
-	return add_worktree(path, cmd.argv);
+	return add_worktree(path, cmd.argv, &opts);
 }
 
 int cmd_worktree(int ac, const char **av, const char *prefix)
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 13/22] worktree: make --detach mutually exclusive with -b/-B

[Eric Sunshine]

Be consistent with git-checkout which disallows this (not particularly
meaningful) combination.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

Changes since v2: add tests

 builtin/worktree.c      |  4 ++--
 t/t2025-worktree-add.sh | 12 ++++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7d70eb6..813e016 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -296,8 +296,8 @@ static int add(int ac, const char **av, const char *prefix)
 
 	memset(&opts, 0, sizeof(opts));
 	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
-	if (opts.new_branch && new_branch_force)
-		die(_("-b and -B are mutually exclusive"));
+	if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
+		die(_("-b, -B, and --detach are mutually exclusive"));
 	if (ac < 1 || ac > 2)
 		usage_with_options(worktree_usage, options);
 
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 9e30690..249e454 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -167,4 +167,16 @@ test_expect_success '"add" auto-vivify does not clobber existing branch' '
 	test_path_is_missing precious
 '
 
+test_expect_success '"add" -b/-B mutually exclusive' '
+	test_must_fail git worktree add -b poodle -B poodle bamboo master
+'
+
+test_expect_success '"add" -b/--detach mutually exclusive' '
+	test_must_fail git worktree add -b poodle --detach bamboo master
+'
+
+test_expect_success '"add" -B/--detach mutually exclusive' '
+	test_must_fail git worktree add -B poodle --detach bamboo master
+'
+
 test_done
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 14/22] worktree: add: suppress auto-vivication with --detach and no <branch>

[Eric Sunshine]

Fix oversight where branch auto-vivication incorrectly kicks in when
--detach is specified and <branch> omitted. Instead, treat:

    git worktree add --detach <path>

as shorthand for:

    git worktree add --detach <path> HEAD

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

New in v3.

 builtin/worktree.c      |  2 +-
 t/t2025-worktree-add.sh | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 813e016..83484ad 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -308,7 +308,7 @@ static int add(int ac, const char **av, const char *prefix)
 	if (opts.force_new_branch)
 		opts.new_branch = new_branch_force;
 
-	if (ac < 2 && !opts.new_branch) {
+	if (ac < 2 && !opts.new_branch && !opts.detach) {
 		int n;
 		const char *s = worktree_basename(path, &n);
 		opts.new_branch = xstrndup(s, n);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 249e454..8267411 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -153,6 +153,14 @@ test_expect_success '"add -b" with <branch> omitted' '
 	test_cmp_rev HEAD burble
 '
 
+test_expect_success '"add --detach" with <branch> omitted' '
+	git worktree add --detach fishhook &&
+	git rev-parse HEAD >expected &&
+	git -C fishhook rev-parse HEAD >actual &&
+	test_cmp expected actual &&
+	test_must_fail git -C fishhook symbolic-ref HEAD
+'
+
 test_expect_success '"add" with <branch> omitted' '
 	git worktree add wiffle/bat &&
 	test_cmp_rev HEAD bat
@@ -167,6 +175,12 @@ test_expect_success '"add" auto-vivify does not clobber existing branch' '
 	test_path_is_missing precious
 '
 
+test_expect_success '"add" no auto-vivify with --detach and <branch> omitted' '
+	git worktree add --detach mish/mash &&
+	test_must_fail git rev-parse mash -- &&
+	test_must_fail git -C mish/mash symbolic-ref HEAD
+'
+
 test_expect_success '"add" -b/-B mutually exclusive' '
 	test_must_fail git worktree add -b poodle -B poodle bamboo master
 '
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 15/22] worktree: make branch creation distinct from worktree population

[Eric Sunshine]

git-worktree currently conflates branch creation, setting of HEAD in the
new worktree, and worktree population into a single sub-invocation of
git-checkout, which requires git-checkout to be specially aware that it
is operating in a newly-created worktree. The goal is to free
git-checkout of that special knowledge, and to do so, git-worktree will
eventually perform those operations separately. Thus, as a first step,
rather than piggybacking on git-checkout's -b/-B ability to create a new
branch at checkout time, make git-worktree responsible for branch
creation itself.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/worktree.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 83484ad..8225468 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -314,12 +314,23 @@ static int add(int ac, const char **av, const char *prefix)
 		opts.new_branch = xstrndup(s, n);
 	}
 
+	if (opts.new_branch) {
+		struct child_process cp;
+		memset(&cp, 0, sizeof(cp));
+		cp.git_cmd = 1;
+		argv_array_push(&cp.args, "branch");
+		if (opts.force_new_branch)
+			argv_array_push(&cp.args, "--force");
+		argv_array_push(&cp.args, opts.new_branch);
+		argv_array_push(&cp.args, branch);
+		if (run_command(&cp))
+			return -1;
+		branch = opts.new_branch;
+	}
+
 	argv_array_push(&cmd, "checkout");
 	if (opts.force)
 		argv_array_push(&cmd, "--ignore-other-worktrees");
-	if (opts.new_branch)
-		argv_array_pushl(&cmd, opts.force_new_branch ? "-B" : "-b",
-				 opts.new_branch, NULL);
 	if (opts.detach)
 		argv_array_push(&cmd, "--detach");
 	argv_array_push(&cmd, branch);
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 16/22] worktree: elucidate environment variables intended for child processes

[Eric Sunshine]

Take advantage of 'struct child_process.env' to make it obvious that
environment variables set by add_worktree() are intended specifically
for sub-commands it invokes to operate in the new worktree.

We assign a local 'struct argv_array' to child_process.env, rather than
utilizing the child_process.env_array 'struct argv_array', because
future patches will make add_worktree() invoke additional sub-commands,
and it's simpler to populate the environment array just once, whereas
child_process.env_array gets cleared after each invocation, thus would
require re-population for each sub-command.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/worktree.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 8225468..7bd6f17 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -186,6 +186,7 @@ static int add_worktree(const char *path, const char **child_argv,
 	const char *name;
 	struct stat st;
 	struct child_process cp;
+	struct argv_array child_env = ARGV_ARRAY_INIT;
 	int counter = 0, len, ret;
 	unsigned char rev[20];
 
@@ -256,11 +257,12 @@ static int add_worktree(const char *path, const char **child_argv,
 	fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
 
 	setenv("GIT_CHECKOUT_NEW_WORKTREE", "1", 1);
-	setenv(GIT_DIR_ENVIRONMENT, sb_git.buf, 1);
-	setenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);
+	argv_array_pushf(&child_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
+	argv_array_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
 	memset(&cp, 0, sizeof(cp));
 	cp.git_cmd = 1;
 	cp.argv = child_argv;
+	cp.env = child_env.argv;
 	ret = run_command(&cp);
 	if (!ret) {
 		is_junk = 0;
@@ -272,6 +274,7 @@ static int add_worktree(const char *path, const char **child_argv,
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/locked", sb_repo.buf);
 	unlink_or_warn(sb.buf);
+	argv_array_clear(&child_env);
 	strbuf_release(&sb);
 	strbuf_release(&sb_repo);
 	strbuf_release(&sb_git);
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 17/22] worktree: add_worktree: construct worktree-population command locally

[Eric Sunshine]

The caller of add_worktree() provides it with a command to invoke to
populate the new worktree. This was a useful abstraction during the
conversion of "git checkout --to" functionality to "git worktree add"
since git-checkout and git-worktree constructed the population command
differently. However, now that "git checkout --to" has been retired, and
add_worktree() has access to the options given to "worktree add", this
extra indirection is no longer useful and makes the code a bit
convoluted.

Moreover, the eventual goal is for git-worktree to make setting of HEAD
and worktree population distinct operations, whereas they are currently
conflated into a single git-checkout invocation. As such, add_worktree()
will eventually invoke other commands in addition to the worktree
population command, so it will be doing command construction itself
anyhow.

Therefore, relocate construction of the worktree population command from
add() to add_worktree().

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/worktree.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7bd6f17..da76eb7 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -178,7 +178,7 @@ static const char *worktree_basename(const char *path, int *olen)
 	return name;
 }
 
-static int add_worktree(const char *path, const char **child_argv,
+static int add_worktree(const char *path, const char *refname,
 			const struct add_opts *opts)
 {
 	struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;
@@ -261,7 +261,12 @@ static int add_worktree(const char *path, const char **child_argv,
 	argv_array_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
 	memset(&cp, 0, sizeof(cp));
 	cp.git_cmd = 1;
-	cp.argv = child_argv;
+	argv_array_push(&cp.args, "checkout");
+	if (opts->force)
+		argv_array_push(&cp.args, "--ignore-other-worktrees");
+	if (opts->detach)
+		argv_array_push(&cp.args, "--detach");
+	argv_array_push(&cp.args, refname);
 	cp.env = child_env.argv;
 	ret = run_command(&cp);
 	if (!ret) {
@@ -286,7 +291,6 @@ static int add(int ac, const char **av, const char *prefix)
 	struct add_opts opts;
 	const char *new_branch_force = NULL;
 	const char *path, *branch;
-	struct argv_array cmd = ARGV_ARRAY_INIT;
 	struct option options[] = {
 		OPT__FORCE(&opts.force, N_("checkout <branch> even if already checked out in other worktree")),
 		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
@@ -331,14 +335,7 @@ static int add(int ac, const char **av, const char *prefix)
 		branch = opts.new_branch;
 	}
 
-	argv_array_push(&cmd, "checkout");
-	if (opts.force)
-		argv_array_push(&cmd, "--ignore-other-worktrees");
-	if (opts.detach)
-		argv_array_push(&cmd, "--detach");
-	argv_array_push(&cmd, branch);
-
-	return add_worktree(path, cmd.argv, &opts);
+	return add_worktree(path, branch, &opts);
 }
 
 int cmd_worktree(int ac, const char **av, const char *prefix)
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 18/22] worktree: detect branch-name/detached and error conditions locally

[Eric Sunshine]

git-worktree currently conflates setting of HEAD in the new worktree
with initial worktree population via a single git-checkout invocation,
which requires git-checkout to have special knowledge that it is
operating in a newly created worktree. The eventual goal is to separate
these operations and rid git-checkout of that overly-intimate knowledge.

Once these operations are separate, git-worktree will no longer be able
to rely upon git-branch to determine the state of the worktree (branch
name or detached), or to check for error conditions, such as the
requested branch already checked out elsewhere, or an invalid reference.
Therefore, imbue git-worktree with the intelligence to determine a
branch name or detached state locally, and to perform error checking on
its own.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/worktree.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index da76eb7..cf35b2a 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -3,6 +3,8 @@
 #include "dir.h"
 #include "parse-options.h"
 #include "argv-array.h"
+#include "branch.h"
+#include "refs.h"
 #include "run-command.h"
 #include "sigchain.h"
 
@@ -188,11 +190,26 @@ static int add_worktree(const char *path, const char *refname,
 	struct child_process cp;
 	struct argv_array child_env = ARGV_ARRAY_INIT;
 	int counter = 0, len, ret;
+	struct strbuf symref = STRBUF_INIT;
+	struct commit *commit = NULL;
 	unsigned char rev[20];
 
 	if (file_exists(path) && !is_empty_dir(path))
 		die(_("'%s' already exists"), path);
 
+	/* is 'refname' a branch or commit? */
+	if (opts->force_new_branch) /* definitely a branch */
+		;
+	else if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
+		 ref_exists(symref.buf)) { /* it's a branch */
+		if (!opts->force)
+			die_if_checked_out(symref.buf);
+	} else { /* must be a commit */
+		commit = lookup_commit_reference_by_name(refname);
+		if (!commit)
+			die(_("invalid reference: %s"), refname);
+	}
+
 	name = worktree_basename(path, &len);
 	strbuf_addstr(&sb_repo,
 		      git_path("worktrees/%.*s", (int)(path + len - name), name));
@@ -281,6 +298,7 @@ static int add_worktree(const char *path, const char *refname,
 	unlink_or_warn(sb.buf);
 	argv_array_clear(&child_env);
 	strbuf_release(&sb);
+	strbuf_release(&symref);
 	strbuf_release(&sb_repo);
 	strbuf_release(&sb_git);
 	return ret;
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 19/22] worktree: make setup of new HEAD distinct from worktree population

[Eric Sunshine]

git-worktree currently conflates setting of HEAD in the new worktree and
initial worktree population into a single git-checkout invocation which
requires git-checkout to have special knowledge that it is operating on
a newly created worktree. The eventual goal is to rid git-checkout of
that overly-intimate knowledge.

Once these operations are separate, git-worktree will no longer be able
to delegate to git-branch the setting of the new worktree's HEAD to the
desired branch (or commit, if detached). Therefore, make git-worktree
itself responsible for setting up HEAD as either a symbolic reference,
if associated with a branch, or detached, if not.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/worktree.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index cf35b2a..79d088c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -278,12 +278,21 @@ static int add_worktree(const char *path, const char *refname,
 	argv_array_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
 	memset(&cp, 0, sizeof(cp));
 	cp.git_cmd = 1;
+
+	if (commit)
+		argv_array_pushl(&cp.args, "update-ref", "HEAD",
+				 sha1_to_hex(commit->object.sha1), NULL);
+	else
+		argv_array_pushl(&cp.args, "symbolic-ref", "HEAD",
+				 symref.buf, NULL);
+	cp.env = child_env.argv;
+	ret = run_command(&cp);
+	if (ret)
+		goto done;
+
+	cp.argv = NULL;
+	argv_array_clear(&cp.args);
 	argv_array_push(&cp.args, "checkout");
-	if (opts->force)
-		argv_array_push(&cp.args, "--ignore-other-worktrees");
-	if (opts->detach)
-		argv_array_push(&cp.args, "--detach");
-	argv_array_push(&cp.args, refname);
 	cp.env = child_env.argv;
 	ret = run_command(&cp);
 	if (!ret) {
@@ -293,6 +302,7 @@ static int add_worktree(const char *path, const char *refname,
 		junk_work_tree = NULL;
 		junk_git_dir = NULL;
 	}
+done:
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/locked", sb_repo.buf);
 	unlink_or_warn(sb.buf);
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 20/22] worktree: avoid resolving HEAD unnecessarily

[Eric Sunshine]

Now that git-worktree sets HEAD explicitly to its final value via either
git-symbolic-ref or git-update-ref, rather than relying upon
git-checkout to do so, the "hack" for pacifying is_git_directory() with
a temporary HEAD, though still necessary, can be simplified.

Since the real HEAD is now populated with its proper final value, the
value of the temporary HEAD truly no longer matters, and any value which
looks like an object ID is good enough to satisfy is_git_directory().
Therefore, just set the temporary HEAD to a literal value rather than
going through the effort of resolving the current branch's HEAD.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/worktree.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 79d088c..4619308 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -192,7 +192,6 @@ static int add_worktree(const char *path, const char *refname,
 	int counter = 0, len, ret;
 	struct strbuf symref = STRBUF_INIT;
 	struct commit *commit = NULL;
-	unsigned char rev[20];
 
 	if (file_exists(path) && !is_empty_dir(path))
 		die(_("'%s' already exists"), path);
@@ -253,20 +252,14 @@ static int add_worktree(const char *path, const char *refname,
 		   real_path(get_git_common_dir()), name);
 	/*
 	 * This is to keep resolve_ref() happy. We need a valid HEAD
-	 * or is_git_directory() will reject the directory. Moreover, HEAD
-	 * in the new worktree must resolve to the same value as HEAD in
-	 * the current tree since the command invoked to populate the new
-	 * worktree will be handed the branch/ref specified by the user.
-	 * For instance, if the user asks for the new worktree to be based
-	 * at HEAD~5, then the resolved HEAD~5 in the new worktree must
-	 * match the resolved HEAD~5 in the current tree in order to match
-	 * the user's expectation.
+	 * or is_git_directory() will reject the directory. Any value which
+	 * looks like an object ID will do since it will be immediately
+	 * replaced by the symbolic-ref or update-ref invocation in the new
+	 * worktree.
 	 */
-	if (!resolve_ref_unsafe("HEAD", 0, rev, NULL))
-		die(_("unable to resolve HEAD"));
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/HEAD", sb_repo.buf);
-	write_file(sb.buf, 1, "%s\n", sha1_to_hex(rev));
+	write_file(sb.buf, 1, "0000000000000000000000000000000000000000\n");
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
 	write_file(sb.buf, 1, "../..\n");
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 21/22] worktree: populate via "git reset --hard" rather than "git checkout"

[Eric Sunshine]

Now that git-worktree handles all functionality (--force, --detach,
-b/-B) previously delegated to git-checkout, actual population of the
new worktree can be accomplished more directly and lightweight with
"git reset --hard" in place of "git checkout".

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/worktree.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 4619308..5d9371c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -266,7 +266,6 @@ static int add_worktree(const char *path, const char *refname,
 
 	fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
 
-	setenv("GIT_CHECKOUT_NEW_WORKTREE", "1", 1);
 	argv_array_pushf(&child_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
 	argv_array_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
 	memset(&cp, 0, sizeof(cp));
@@ -285,7 +284,7 @@ static int add_worktree(const char *path, const char *refname,
 
 	cp.argv = NULL;
 	argv_array_clear(&cp.args);
-	argv_array_push(&cp.args, "checkout");
+	argv_array_pushl(&cp.args, "reset", "--hard", NULL);
 	cp.env = child_env.argv;
 	ret = run_command(&cp);
 	if (!ret) {
-- 
2.5.0.rc2.378.g0af52e8

[PATCH v3 22/22] checkout: drop intimate knowledge of newly created worktree

[Eric Sunshine]

Now that git-worktree no longer relies upon git-checkout for new branch
creation, new worktree HEAD set up, or initial worktree population,
git-checkout no longer needs intimate knowledge that it may be operating
in a newly created worktree. Therefore, drop 'new_worktree_mode' and the
private GIT_CHECKOUT_NEW_WORKTREE environment variable by which
git-worktree communicated to git-checkout that it was being invoked to
manipulate a new worktree.

This reverts the remaining changes to checkout.c by 529fef2 (checkout:
support checking out into a new working directory, 2014-11-30).

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

No changes since v2.

 builtin/checkout.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4ae895c..02d78ba 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -48,8 +48,6 @@ struct checkout_opts {
 	const char *prefix;
 	struct pathspec pathspec;
 	struct tree *source_tree;
-
-	int new_worktree_mode;
 };
 
 static int post_checkout_hook(struct commit *old, struct commit *new,
@@ -491,7 +489,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			topts.dir->flags |= DIR_SHOW_IGNORED;
 			setup_standard_excludes(topts.dir);
 		}
-		tree = parse_tree_indirect(old->commit && !opts->new_worktree_mode ?
+		tree = parse_tree_indirect(old->commit ?
 					   old->commit->object.sha1 :
 					   EMPTY_TREE_SHA1_BIN);
 		init_tree_desc(&trees[0], tree->buffer, tree->size);
@@ -807,8 +805,7 @@ static int switch_branches(const struct checkout_opts *opts,
 		return ret;
 	}
 
-	if (!opts->quiet && !old.path && old.commit &&
-	    new->commit != old.commit && !opts->new_worktree_mode)
+	if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
 		orphaned_commit_warning(old.commit, new->commit);
 
 	update_refs_for_switch(opts, &old, new);
@@ -1151,8 +1148,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, checkout_usage,
 			     PARSE_OPT_KEEP_DASHDASH);
 
-	opts.new_worktree_mode = getenv("GIT_CHECKOUT_NEW_WORKTREE") != NULL;
-
 	if (conflict_style) {
 		opts.merge = 1; /* implied */
 		git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
-- 
2.5.0.rc2.378.g0af52e8

Re: [PATCH v3 14/22] worktree: add: suppress auto-vivication with --detach and no <branch>

[Eric Sunshine]

On Fri, Jul 17, 2015 at 07:00:09PM -0400, Eric Sunshine wrote:
> Fix oversight where branch auto-vivication incorrectly kicks in when
> --detach is specified and <branch> omitted. Instead, treat:
> 
>     git worktree add --detach <path>
> 
> as shorthand for:
> 
>     git worktree add --detach <path> HEAD
> 
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  builtin/worktree.c      |  2 +-
>  t/t2025-worktree-add.sh | 14 ++++++++++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)

I meant, but forgot, to include a documentation update with this patch.
Here it is as a squash-in:

---- 8< ----
From: Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH v3 23/22] fixup! worktree: add: suppress auto-vivication with --detach and no <branch>

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/git-worktree.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index da71f50..834a61c 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -51,9 +51,9 @@ Create `<path>` and checkout `<branch>` into it. The new working directory
 is linked to the current repository, sharing everything except working
 directory specific files such as HEAD, index, etc.
 +
-If `<branch>` is omitted and neither `-b` nor `-B` is used, then, as a
-convenience, a new branch based at HEAD is created automatically, as if
-`-b $(basename <path>)` was specified.
+If `<branch>` is omitted and neither `-b` nor `-B` nor `--detached` used,
+then, as a convenience, a new branch based at HEAD is created automatically,
+as if `-b $(basename <path>)` was specified.
 
 prune::
 
-- 
2.5.0.rc2.378.g0af52e8