This is v2 of [1] which rids git-checkout of the need to have specialized knowledge that it's operating in a newly created linked worktree, and decouples git-worktree from git-checkout. Thanks to Duy and Junio for reviews. A v1 to v2 interdiff is included below. Changes since v1: * patch 03/20: relocate from caller to callee a comment explaining why check_linked_checkout() resolves HEAD manually[2] * patch 06/20 (new): drop "/.git" from "'blorp' already checked out at '/foo/bar/.git'" diagnostic[3] * patch 07/20 & 08/20 (new): teach checked_linked_checkout() about symbolic-link HEAD[2] (to augment its existing symref-style HEAD knowledge) * patch 14/20 (new): take advantage of 'struct child_process.env' to make it obvious that assigned environment variables are intended for child process invocations[2] * patch 16/20: side-step potential problem with future pluggable backends answering ref_exists() incorrectly from cached information after we've invoked git-branch behind their backs[4,5], and add a few in-code comments explaining the logic * reword several commit messages to avoid stressing "git reset --hard" as endgame, and instead emphasize removal of specialized worktree creation knowledge from git-checkout[2], and resolve conflation of new branch creation, setting of worktree HEAD, and new worktree population. [1]: http://thread.gmane.org/gmane.comp.version-control.git/273856 [2]: http://article.gmane.org/gmane.comp.version-control.git/273921 [3]: http://thread.gmane.org/gmane.comp.version-control.git/274001 [4]: http://article.gmane.org/gmane.comp.version-control.git/273885 [5]: http://article.gmane.org/gmane.comp.version-control.git/273901 Eric Sunshine (20): 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: simplify new branch (-b/-B) option checking worktree: introduce options container worktree: make --detach mutually exclusive with -b/-B 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 | 120 ++++++++++++++++++++++++++++++++---------------- t/t2025-worktree-add.sh | 8 ++++ 5 files changed, 169 insertions(+), 115 deletions(-) -- 2.5.0.rc2.378.g0af52e8 ---- 8< ---- diff --git a/branch.c b/branch.c index 7b8b9a3..dfd7698 100644 --- a/branch.c +++ b/branch.c @@ -315,22 +315,28 @@ 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 + * $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_read_file(&sb, path.buf, 0) < 0 || - !skip_prefix(sb.buf, "ref:", &start)) + 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; - 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); @@ -341,6 +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"); die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf); done: strbuf_release(&path); @@ -354,11 +361,6 @@ void die_if_checked_out(const char *branch) DIR *dir; struct dirent *d; - /* - * $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(branch, NULL); strbuf_addf(&path, "%s/worktrees", get_git_common_dir()); diff --git a/builtin/worktree.c b/builtin/worktree.c index 51c57bc..2873064 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -188,6 +188,7 @@ static int add_worktree(const char *path, const char *refname, const char *name; struct stat st; 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; @@ -195,11 +196,14 @@ static int add_worktree(const char *path, const char *refname, if (file_exists(path) && !is_empty_dir(path)) die(_("'%s' already exists"), path); - if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) && - ref_exists(symref.buf)) { + /* 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 { + } else { /* must be a commit */ commit = lookup_commit_reference_by_name(refname); if (!commit) die(_("invalid reference: %s"), refname); @@ -262,8 +266,8 @@ static int add_worktree(const char *path, const char *refname, fprintf_ln(stderr, _("Enter %s (identifier %s)"), path, name); - 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; @@ -273,6 +277,7 @@ static int add_worktree(const char *path, const char *refname, 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; @@ -280,6 +285,7 @@ static int add_worktree(const char *path, const char *refname, cp.argv = NULL; argv_array_clear(&cp.args); argv_array_pushl(&cp.args, "reset", "--hard", NULL); + cp.env = child_env.argv; ret = run_command(&cp); if (!ret) { is_junk = 0; @@ -292,6 +298,7 @@ done: 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(&symref); strbuf_release(&sb_repo); 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 && ---- 8< ---- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
