From c265c533cfbced87d56ad6d3d4ae3d62475c29ce Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 18:59:56 -0400 Subject: [PATCH 01/25] checkout: avoid resolving HEAD unnecessarily 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 Signed-off-by: Junio C Hamano --- builtin/checkout.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 57545543eb..75f90a9d1e 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); } From e13d37094e3b2ceb51dab362b8e2e1ad9e4bb758 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 18:59:57 -0400 Subject: [PATCH 02/25] checkout: name check_linked_checkouts() more meaningfully 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 Signed-off-by: Junio C Hamano --- builtin/checkout.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 75f90a9d1e..e75fb5e50f 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); } From aaad2c948f11cd57348135fb1e62b012db716182 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 18:59:58 -0400 Subject: [PATCH 03/25] checkout: improve die_if_checked_out() robustness 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 Signed-off-by: Junio C Hamano --- builtin/checkout.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index e75fb5e50f..1992c41d48 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; From 4e07815dba5ccad0cfcb672b99e2000bd3dc3543 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 18:59:59 -0400 Subject: [PATCH 04/25] checkout: die_if_checked_out: simplify strbuf management 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 Signed-off-by: Junio C Hamano --- builtin/checkout.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 1992c41d48..c36bf8acfb 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); } From 4341460d92c2857193954158f35aaf7518aa7a58 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:00 -0400 Subject: [PATCH 05/25] checkout: generalize die_if_checked_out() branch name argument 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 Signed-off-by: Junio C Hamano --- builtin/checkout.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index c36bf8acfb..177ad6a2d7 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); } From 39e69e151951040b952b3c81d2ee40b827fd9abf Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:01 -0400 Subject: [PATCH 06/25] checkout: check_linked_checkout: improve "already checked out" aesthetic 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 Signed-off-by: Junio C Hamano --- builtin/checkout.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/checkout.c b/builtin/checkout.c index 177ad6a2d7..de6619f4c5 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); From 33aef83666900df03c39bcf7e391e2f8195dd13c Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:02 -0400 Subject: [PATCH 07/25] checkout: check_linked_checkout: simplify symref parsing 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 Signed-off-by: Junio C Hamano --- builtin/checkout.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index de6619f4c5..6f4e49232a 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); From 746bbdc64f1e1b2618cd71e5da81e38aa772e8ba Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:03 -0400 Subject: [PATCH 08/25] checkout: teach check_linked_checkout() about symbolic link HEAD 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 Signed-off-by: Junio C Hamano --- 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 6f4e49232a..f04dcaaf1f 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 ead8aa2a9d..9e30690692 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 && From ed89f84b3c05d6359aba842e245910c996d91859 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:04 -0400 Subject: [PATCH 09/25] branch: publish die_if_checked_out() 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 Signed-off-by: Junio C Hamano --- 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 4bab55a9a8..af9480f25a 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 64173abf4d..58aa45fe72 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 f04dcaaf1f..4ae895ca94 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, From cd2f4713112119e18af8b5b580c5661a52320632 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:05 -0400 Subject: [PATCH 10/25] worktree: improve worktree setup message When git-worktree creates a new worktree, it reports: Enter "" (identifier ) which misleadingly implies that it is setting as the working directory (as if "cd " 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 "" (identifier ) Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/worktree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 69248ba0a3..5f0e3c28e8 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); From eef005dcb3e039ccc3da65e9347cf4357bdeea6a Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:06 -0400 Subject: [PATCH 11/25] worktree: simplify new branch (-b/-B) option checking 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 Signed-off-by: Junio C Hamano --- builtin/worktree.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 5f0e3c28e8..bf4db9fffb 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); From 5dd6e234a75bb96e159c4e7d30f6acb28c3283a0 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:07 -0400 Subject: [PATCH 12/25] worktree: introduce options container 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 Signed-off-by: Junio C Hamano --- builtin/worktree.c | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index bf4db9fffb..7d70eb682c 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 even if already checked out in other worktree")), - OPT_STRING('b', NULL, &new_branch, N_("branch"), + OPT__FORCE(&opts.force, N_("checkout 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) From ab0b2c53ed853e34def18e7b84acd7da7e2ddd49 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:08 -0400 Subject: [PATCH 13/25] worktree: make --detach mutually exclusive with -b/-B Be consistent with git-checkout which disallows this (not particularly meaningful) combination. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- 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 7d70eb682c..813e016130 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 9e30690692..249e4540dc 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 From 5c942570fe2a48d8fde348e89392c2e9e23aa483 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:09 -0400 Subject: [PATCH 14/25] worktree: add: suppress auto-vivication with --detach and no Fix oversight where branch auto-vivication incorrectly kicks in when --detach is specified and omitted. Instead, treat: git worktree add --detach as shorthand for: git worktree add --detach HEAD Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/git-worktree.txt | 6 +++--- builtin/worktree.c | 2 +- t/t2025-worktree-add.sh | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 32344598c3..332dd7734d 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -51,9 +51,9 @@ Create `` and checkout `` 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 `` 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 )` was specified. +If `` 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 )` was specified. prune:: diff --git a/builtin/worktree.c b/builtin/worktree.c index 813e016130..83484ad502 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 249e4540dc..8267411a0e 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -153,6 +153,14 @@ test_expect_success '"add -b" with omitted' ' test_cmp_rev HEAD burble ' +test_expect_success '"add --detach" with 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 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 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 ' From c2842439a3ab9bcab122105f3f457afb37fbc7c1 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:10 -0400 Subject: [PATCH 15/25] worktree: make branch creation distinct from worktree population 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 Signed-off-by: Junio C Hamano --- builtin/worktree.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 83484ad502..8225468f11 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); From ae2a38271f778522ceb9182b94e0024a816e3338 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:11 -0400 Subject: [PATCH 16/25] worktree: elucidate environment variables intended for child processes 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 Signed-off-by: Junio C Hamano --- builtin/worktree.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 8225468f11..7bd6f1793e 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); From 80a0548f6c12f43e9bd62e13eacb033f05e2b001 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:12 -0400 Subject: [PATCH 17/25] worktree: add_worktree: construct worktree-population command locally 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 Signed-off-by: Junio C Hamano --- builtin/worktree.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 7bd6f1793e..da76eb7d34 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 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) From f7c9dac1b037e453e934c272d77cc648d56d5477 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:13 -0400 Subject: [PATCH 18/25] worktree: detect branch-name/detached and error conditions locally 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 Signed-off-by: Junio C Hamano --- builtin/worktree.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/builtin/worktree.c b/builtin/worktree.c index da76eb7d34..cf35b2aa7e 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; From 7f44e3d1de08bf99c8dbd69d6437b712df369692 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:14 -0400 Subject: [PATCH 19/25] worktree: make setup of new HEAD distinct from worktree population 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 Signed-off-by: Junio C Hamano --- builtin/worktree.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index cf35b2aa7e..79d088c4a8 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); From ed197a6ab983783550bd0b0c71cb97d734f4cfaa Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:15 -0400 Subject: [PATCH 20/25] worktree: avoid resolving HEAD unnecessarily 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 Signed-off-by: Junio C Hamano --- builtin/worktree.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 79d088c4a8..461930858b 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"); From 1c56190aec502d946eb3db7365d6e54e2ffc7bd2 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:16 -0400 Subject: [PATCH 21/25] worktree: populate via "git reset --hard" rather than "git checkout" 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 Signed-off-by: Junio C Hamano --- builtin/worktree.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 461930858b..5d9371cf95 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) { From 272be14a85d3a8dca2ca842a3f35dfcf5895d2a6 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 17 Jul 2015 19:00:17 -0400 Subject: [PATCH 22/25] checkout: drop intimate knowledge of newly created worktree 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 Signed-off-by: Junio C Hamano --- builtin/checkout.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 4ae895ca94..02d78baae0 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); From 5f5f553fd571732f6ff78107fb6223c5cf7b441f Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 24 Jul 2015 00:00:52 -0400 Subject: [PATCH 23/25] Documentation/git-worktree: fix broken 'linkgit' invocation Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/git-worktree.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 332dd7734d..400b313538 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -27,7 +27,7 @@ bare repository) and zero or more linked working trees. When you are done with a linked working tree you can simply delete it. The working tree's administrative files in the repository (see "DETAILS" below) will eventually be removed automatically (see -`gc.worktreePruneExpire` in linkgit::git-config[1]), or you can run +`gc.worktreePruneExpire` in linkgit:git-config[1]), or you can run `git worktree prune` in the main or any linked working tree to clean up any stale administrative files. From 8cc88166c00e555f1bf5375017ed91b7e2cc904e Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 28 Jul 2015 16:06:10 -0400 Subject: [PATCH 24/25] Documentation/config: mention "now" and "never" for 'expire' settings In addition to approxidate-style values ("2.months.ago", "yesterday"), consumers of 'gc.*expire*' configuration variables also accept and respect 'now' ("do it immediately") and 'never' ("suppress entirely"). Suggested-by: Michael Haggerty Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/config.txt | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 583d24fda4..e09ee02df8 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1226,20 +1226,24 @@ gc.packrefs:: gc.pruneexpire:: When 'git gc' is run, it will call 'prune --expire 2.weeks.ago'. Override the grace period with this config variable. The value - "now" may be used to disable this grace period and always prune - unreachable objects immediately. + "now" may be used to disable this grace period and always prune + unreachable objects immediately, or "never" may be used to + suppress pruning. gc.worktreePruneExpire:: When 'git gc' is run, it calls 'git worktree prune --expire 3.months.ago'. This config variable can be used to set a different grace period. The value "now" may be used to disable the grace - period and prune $GIT_DIR/worktrees immediately. + period and prune $GIT_DIR/worktrees immediately, or "never" + may be used to suppress pruning. gc.reflogexpire:: gc..reflogexpire:: 'git reflog expire' removes reflog entries older than - this time; defaults to 90 days. With "" (e.g. + this time; defaults to 90 days. The value "now" expires all + entries immediately, and "never" suppresses expiration + altogether. With "" (e.g. "refs/stash") in the middle the setting applies only to the refs that match the . @@ -1247,7 +1251,9 @@ gc.reflogexpireunreachable:: gc..reflogexpireunreachable:: 'git reflog expire' removes reflog entries older than this time and are not reachable from the current tip; - defaults to 30 days. With "" (e.g. "refs/stash") + defaults to 30 days. The value "now" expires all entries + immediately, and "never" suppresses expiration altogether. + With "" (e.g. "refs/stash") in the middle, the setting applies only to the refs that match the . From 65f9b75dfe34a67cf4c6a8507f999c5710d36ba9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 4 Aug 2015 14:27:57 +0200 Subject: [PATCH 25/25] Documentation/git-worktree: fix duplicated 'from' Signed-off-by: Patrick Steinhardt Acked-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/git-worktree.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 400b313538..fb68156cf8 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -124,7 +124,7 @@ thumb is do not make any assumption about whether a path belongs to $GIT_DIR or $GIT_COMMON_DIR when you need to directly access something inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. -To prevent a $GIT_DIR/worktrees entry from from being pruned (which +To prevent a $GIT_DIR/worktrees entry from being pruned (which can be useful in some situations, such as when the entry's working tree is stored on a portable device), add a file named 'locked' to the entry's directory. The file contains the reason in