possible to checkout same branch in different worktree |
|||||||||||||||||||||||||||||||||||||||||
123456
![]() Re: [PATCH 2/7] worktree.c: rewrite mark_current_worktree() to avoid strbuf
![]() Re: [PATCH 5/7] worktree.c: add clear_worktree()
![]() Re: [PATCH 5/7] worktree.c: add clear_worktree()
![]() Re: [PATCH 1/5] worktree.c: add find_worktree_by_path()
![]() Re: [PATCH 2/5] worktree.c: add is_main_worktree()
![]() Re: [PATCH 3/5] worktree.c: add is_worktree_locked()
![]() Re: [PATCH 4/5] worktree: add "lock" command
![]() Re: [PATCH 4/5] worktree: add "lock" command
![]() Re: [PATCH 5/5] worktree: add "unlock" command
![]() Re: [PATCH 5/7] worktree.c: add clear_worktree()
![]()
[PATCH v2 0/6] nd/worktree-cleanup-post-head-
|
In reply to this post by Duy Nguyen
This is mostly commit message update plus
- dropping "--force" from the completion patch (a comment from Szeder Gabor in a previous attempt of worktree completion support). - the removal of clear_worktree() patch. I keep the completion patch anyway even though two versions of it were posted by Eric and Szeder (and didn't get discussed much) because I needed this for new commands and because I didn't aim high. It's meant to provide very basic completion support. Fancy stuff like ref completion can be added later by people who are more experienced in bash completion stuff. Anyway, interdiff -- 8< -- diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index d3ac391..951a186 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2604,7 +2604,7 @@ _git_worktree () else case "$subcommand,$cur" in add,--*) - __gitcomp "--detach --force" + __gitcomp "--detach" ;; list,--*) __gitcomp "--porcelain" diff --git a/worktree.c b/worktree.c index 335c58f..f4a4f38 100644 --- a/worktree.c +++ b/worktree.c @@ -5,22 +5,14 @@ #include "dir.h" #include "wt-status.h" -void clear_worktree(struct worktree *wt) -{ - if (!wt) - return; - free(wt->path); - free(wt->id); - free(wt->head_ref); - memset(wt, 0, sizeof(*wt)); -} - void free_worktrees(struct worktree **worktrees) { int i = 0; for (i = 0; worktrees[i]; i++) { - clear_worktree(worktrees[i]); + free(worktrees[i]->path); + free(worktrees[i]->id); + free(worktrees[i]->head_ref); free(worktrees[i]); } free (worktrees); diff --git a/worktree.h b/worktree.h index 7430a4e..1394909 100644 --- a/worktree.h +++ b/worktree.h @@ -29,11 +29,6 @@ extern struct worktree **get_worktrees(void); */ extern const char *get_worktree_git_dir(const struct worktree *wt); -/* - * Free up the memory for worktree - */ -extern void clear_worktree(struct worktree *); - /* * Free up the memory for worktree(s) */ -- 8< -- Nguyễn Thái Ngọc Duy (6): completion: support git-worktree worktree.c: rewrite mark_current_worktree() to avoid strbuf git-worktree.txt: keep subcommand listing in alphabetical order worktree.c: use is_dot_or_dotdot() worktree: avoid 0{40}, too many zeroes, hard to read worktree: simplify prefixing paths Documentation/git-worktree.txt | 10 +++++----- builtin/worktree.c | 10 ++++++---- contrib/completion/git-completion.bash | 23 +++++++++++++++++++++++ worktree.c | 18 ++++++++---------- 4 files changed, 42 insertions(+), 19 deletions(-) -- 2.8.2.524.g6ff3d78 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
This adds bare-bone completion support for git-worktree. More advanced
completion (e.g. ref completion in git-worktree-add) can be added later. --force completion in "worktree add" is left out because that option should be handled with care. Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]> --- contrib/completion/git-completion.bash | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 3402475..951a186 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2595,6 +2595,29 @@ _git_whatchanged () _git_log } +_git_worktree () +{ + local subcommands="add list prune" + local subcommand="$(__git_find_on_cmdline "$subcommands")" + if [ -z "$subcommand" ]; then + __gitcomp "$subcommands" + else + case "$subcommand,$cur" in + add,--*) + __gitcomp "--detach" + ;; + list,--*) + __gitcomp "--porcelain" + ;; + prune,--*) + __gitcomp "--dry-run --expire --verbose" + ;; + *) + ;; + esac + fi +} + __git_main () { local i c=1 command __git_dir -- 2.8.2.524.g6ff3d78 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
In reply to this post by Duy Nguyen
strbuf is a bit overkill for this function. What we need is to call
absolute_path() twice and make sure the second call does not destroy the result of the first. One buffer allocation is enough. Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]> --- worktree.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/worktree.c b/worktree.c index 4817d60..6a11611 100644 --- a/worktree.c +++ b/worktree.c @@ -153,21 +153,19 @@ done: static void mark_current_worktree(struct worktree **worktrees) { - struct strbuf git_dir = STRBUF_INIT; - struct strbuf path = STRBUF_INIT; + char *git_dir = xstrdup(absolute_path(get_git_dir())); int i; - strbuf_addstr(&git_dir, absolute_path(get_git_dir())); for (i = 0; worktrees[i]; i++) { struct worktree *wt = worktrees[i]; - strbuf_addstr(&path, absolute_path(get_worktree_git_dir(wt))); - wt->is_current = !fspathcmp(git_dir.buf, path.buf); - strbuf_reset(&path); - if (wt->is_current) + const char *wt_git_dir = get_worktree_git_dir(wt); + + if (!fspathcmp(git_dir, absolute_path(wt_git_dir))) { + wt->is_current = 1; break; + } } - strbuf_release(&git_dir); - strbuf_release(&path); + free(git_dir); } struct worktree **get_worktrees(void) -- 2.8.2.524.g6ff3d78 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
In reply to this post by Duy Nguyen
This is probably not the best order. But it makes it no-brainer to know
where to insert new commands. At some point we might want to reorder at least the synopsis part again, grouping commonly use subcommands together. Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]> --- Documentation/git-worktree.txt | 10 +++++----- builtin/worktree.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index c622345..27feff6 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -10,8 +10,8 @@ SYNOPSIS -------- [verse] 'git worktree add' [-f] [--detach] [--checkout] [-b <new-branch>] <path> [<branch>] -'git worktree prune' [-n] [-v] [--expire <expire>] 'git worktree list' [--porcelain] +'git worktree prune' [-n] [-v] [--expire <expire>] DESCRIPTION ----------- @@ -54,10 +54,6 @@ 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:: - -Prune working tree information in $GIT_DIR/worktrees. - list:: List details of each worktree. The main worktree is listed first, followed by @@ -65,6 +61,10 @@ each of the linked worktrees. The output details include if the worktree is bare, the revision currently checked out, and the branch currently checked out (or 'detached HEAD' if none). +prune:: + +Prune working tree information in $GIT_DIR/worktrees. + OPTIONS ------- diff --git a/builtin/worktree.c b/builtin/worktree.c index 12c0af7..bf80111 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -13,8 +13,8 @@ static const char * const worktree_usage[] = { N_("git worktree add [<options>] <path> [<branch>]"), - N_("git worktree prune [<options>]"), N_("git worktree list [<options>]"), + N_("git worktree prune [<options>]"), NULL }; -- 2.8.2.524.g6ff3d78 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
In reply to this post by Duy Nguyen
Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]>
--- builtin/worktree.c | 2 +- worktree.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index bf80111..aaee0e2 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -95,7 +95,7 @@ static void prune_worktrees(void) if (!dir) return; while ((d = readdir(dir)) != NULL) { - if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, "..")) + if (is_dot_or_dotdot(d->d_name)) continue; strbuf_reset(&reason); if (!prune_worktree(d->d_name, &reason)) diff --git a/worktree.c b/worktree.c index 6a11611..f4a4f38 100644 --- a/worktree.c +++ b/worktree.c @@ -187,7 +187,7 @@ struct worktree **get_worktrees(void) if (dir) { while ((d = readdir(dir)) != NULL) { struct worktree *linked = NULL; - if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, "..")) + if (is_dot_or_dotdot(d->d_name)) continue; if ((linked = get_linked_worktree(d->d_name))) { -- 2.8.2.524.g6ff3d78 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
In reply to this post by Duy Nguyen
Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]>
--- builtin/worktree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index aaee0e2..b53f802 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -262,7 +262,7 @@ static int add_worktree(const char *path, const char *refname, */ strbuf_reset(&sb); strbuf_addf(&sb, "%s/HEAD", sb_repo.buf); - write_file(sb.buf, "0000000000000000000000000000000000000000"); + write_file(sb.buf, sha1_to_hex(null_sha1)); strbuf_reset(&sb); strbuf_addf(&sb, "%s/commondir", sb_repo.buf); write_file(sb.buf, "../.."); -- 2.8.2.524.g6ff3d78 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
In reply to this post by Duy Nguyen
This also makes slash conversion always happen on Windows (a side effect
of prefix_filename). Which is a good thing. Signed-off-by: Nguyễn Thái Ngọc Duy <[hidden email]> --- builtin/worktree.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index b53f802..f9dac37 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -337,7 +337,7 @@ static int add(int ac, const char **av, const char *prefix) if (ac < 1 || ac > 2) usage_with_options(worktree_usage, options); - path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0]; + path = prefix_filename(prefix, strlen(prefix), av[0]); branch = ac < 2 ? "HEAD" : av[1]; opts.force_new_branch = !!new_branch_force; @@ -467,6 +467,8 @@ int cmd_worktree(int ac, const char **av, const char *prefix) if (ac < 2) usage_with_options(worktree_usage, options); + if (!prefix) + prefix = ""; if (!strcmp(av[1], "add")) return add(ac - 1, av + 1, prefix); if (!strcmp(av[1], "prune")) -- 2.8.2.524.g6ff3d78 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
In reply to this post by Eric Sunshine
On Fri, May 13, 2016 at 11:52 PM, Eric Sunshine <[hidden email]> wrote:
> Actually, I recall that when I suggested the idea of 'struct worktree' > and get_worktrees() to Mike that it would be natural for the structure > to have a 'locked' (or 'locked_reason') field, in which case the > reason could be stored there instead of in this static strbuf. In > fact, my idea at the time was that get_worktrees() would populate that > field automatically, rather than having to do so via a separate > on-demand function call as in this patch. I'm keeping this as a separate function for now. I don't like get_worktrees() doing extra work unless it has to. We soon will see the complete picture of "git worktree" then we can merge it back to get_worktrees() if it turns out checking "locked" is the common operation. is_worktree_locked() then may become a thin wrapper to access this "locked" field. >> +extern const char *is_worktree_locked(const struct worktree *wt); > > I was wondering if builtin/worktree.c:prune_worktree() should be > updated to invoke this new function instead of consulting > "worktrees/<id>/locked" manually, but I see that the entire "prune > worktrees" functionality in builting/worktree.c first needs to be > updated to the get_worktrees() API for that to happen. I thought about updating prune too. But it is in a bit special situation where it may need to consider invalid (or partly invalid) worktrees as well. So far worktree api is about valid worktrees only if I'm not mistaken and we probably should keep it that way, otherwise all callers may need to check "is this worktree valid" all over the place. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
In reply to this post by Eric Sunshine
(the answer to rest of your questions is "yes you're right, will fix"
or something along that line so I will not quote them here) On Mon, May 16, 2016 at 7:09 AM, Eric Sunshine <[hidden email]> wrote: >> + old_reason = is_worktree_locked(wt); >> + if (old_reason) { >> + if (*old_reason) >> + die(_("already locked, reason: %s"), old_reason); >> + die(_("already locked, no reason")); >> + } > > As a convenience, would it make sense to allow a worktree to be > re-locked with a different reason rather than erroring out? For now I think the user can just unlock then relock, after examining the previous reason and maybe incorporating it to the new reason. Yes there is a race but for these operations I don't think it really matters. If it's done often enough, we can lift this restriction. >> + write_file(git_common_path("worktrees/%s/locked", wt->id), >> + "%s", reason); >> + return 0; >> +} > > This is a tangent, but it would be nice for the "git worktree list" > command to show whether a worktree is locked, and the reason (if > available), both in pretty and porcelain formats. (That was another > reason why I suggested to Mike, back when he was adding the "list" > command, that 'struct worktree' should have a 'locked' field and > get_worktrees() should populate it automatically.) Yes. And a good reason for get_worktrees() to read lock status automatically too. I will not do it right away though or at least until after I'm done with move/remove and the shared config file problem. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
In reply to this post by Duy Nguyen
This should address all of Eric's comments (thanks!). An extra change
I made is free_worktrees() at the end of {,un}lock_worktree() to avoid leaking. This series depends on nd/worktree-cleanup-post-head-protection. Interdiff -- 8< -- diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 9ac1129..e0f2605 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -40,9 +40,8 @@ section "DETAILS" for more information. If a linked working tree is stored on a portable device or network share which is not always mounted, you can prevent its administrative files from -being pruned by creating a file named 'locked' alongside the other -administrative files, optionally containing a plain text reason that -pruning should be suppressed. See section "DETAILS" for more information. +being pruned by issuing the `git worktree lock` command, optionally +specifying `--reason` to explain why the working tree is locked. COMMANDS -------- @@ -65,9 +64,11 @@ bare, the revision currently checked out, and the branch currently checked out lock:: -When a worktree is locked, it cannot be pruned, moved or deleted. For -example, if the worktree is on portable device that is not available -when "git worktree <command>" is executed. +If a working tree is on a portable device or network share which +is not always mounted, lock it to prevent its administrative +files from being pruned automatically. This also prevents it from +being moved or deleted. Optionally, specify a reason for the lock +with `--reason`. prune:: @@ -123,7 +124,7 @@ OPTIONS With `prune`, only expire unused working trees older than <time>. --reason <string>: - An explanation why the worktree is locked. + With `lock`, an explanation why the worktree is locked. DETAILS ------- @@ -165,7 +166,8 @@ instead. 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 +entry's working tree is stored on a portable device), use the +`git worktree lock` comamnd, which adds a file named 'locked' to the entry's directory. The file contains the reason in plain text. For example, if a linked working tree's `.git` file points to `/path/main/.git/worktrees/test-next` then a file named diff --git a/builtin/worktree.c b/builtin/worktree.c index 53e5f5a..da9f771 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -482,6 +482,7 @@ static int lock_worktree(int ac, const char **av, const char *prefix) worktrees = get_worktrees(); wt = find_worktree_by_path(worktrees, dst.buf); + strbuf_release(&dst); if (!wt) die(_("'%s' is not a working directory"), av[0]); if (is_main_worktree(wt)) @@ -491,11 +492,12 @@ static int lock_worktree(int ac, const char **av, const char *prefix) if (old_reason) { if (*old_reason) die(_("already locked, reason: %s"), old_reason); - die(_("already locked, no reason")); + die(_("already locked")); } write_file(git_common_path("worktrees/%s/locked", wt->id), "%s", reason); + free_worktrees(worktrees); return 0; } @@ -506,6 +508,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix) }; struct worktree **worktrees, *wt; struct strbuf dst = STRBUF_INIT; + int ret; ac = parse_options(ac, av, prefix, options, worktree_usage, 0); if (ac != 1) @@ -517,14 +520,16 @@ static int unlock_worktree(int ac, const char **av, const char *prefix) worktrees = get_worktrees(); wt = find_worktree_by_path(worktrees, dst.buf); + strbuf_release(&dst); if (!wt) die(_("'%s' is not a working directory"), av[0]); if (is_main_worktree(wt)) die(_("'%s' is a main working directory"), av[0]); if (!is_worktree_locked(wt)) die(_("not locked")); - - return unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id)); + ret = unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id)); + free_worktrees(worktrees); + return ret; } int cmd_worktree(int ac, const char **av, const char *prefix) diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh index f4b2816..1927537 100755 --- a/t/t2028-worktree-move.sh +++ b/t/t2028-worktree-move.sh @@ -25,12 +25,32 @@ test_expect_success 'lock linked worktree' ' test_cmp expected .git/worktrees/source/locked ' +test_expect_success 'lock linked worktree from another worktree' ' + rm .git/worktrees/source/locked && + git worktree add elsewhere && + ( + cd elsewhere && + git worktree lock --reason hahaha ../source + ) && + echo hahaha >expected && + test_cmp expected .git/worktrees/source/locked +' + test_expect_success 'lock worktree twice' ' test_must_fail git worktree lock source && echo hahaha >expected && test_cmp expected .git/worktrees/source/locked ' +test_expect_success 'lock worktree twice (from the locked worktree)' ' + ( + cd source && + test_must_fail git worktree lock . + ) && + echo hahaha >expected && + test_cmp expected .git/worktrees/source/locked +' + test_expect_success 'unlock main worktree' ' test_must_fail git worktree unlock . ' diff --git a/worktree.c b/worktree.c index a9cfbb3..cb30af3 100644 --- a/worktree.c +++ b/worktree.c @@ -218,38 +218,37 @@ struct worktree *find_worktree_by_path(struct worktree **list, const char *path_) { char *path = xstrdup(real_path(path_)); - struct worktree *wt = NULL; - while (*list) { - wt = *list++; - if (!fspathcmp(path, real_path(wt->path))) + for (; *list; list++) + if (!fspathcmp(path, real_path((*list)->path))) break; - wt = NULL; - } free(path); - return wt; + return *list; } int is_main_worktree(const struct worktree *wt) { - return wt && !wt->id; + return !wt->id; } const char *is_worktree_locked(const struct worktree *wt) { static struct strbuf sb = STRBUF_INIT; + struct strbuf path = STRBUF_INIT; + + strbuf_git_common_path(&path, "worktrees/%s/locked", wt->id); - if (!file_exists(git_common_path("worktrees/%s/locked", wt->id))) + if (!file_exists(path.buf)) { + strbuf_release(&path); return NULL; + } strbuf_reset(&sb); - if (strbuf_read_file(&sb, - git_common_path("worktrees/%s/locked", wt->id), - 0) < 0) - die_errno(_("failed to read '%s'"), - git_common_path("worktrees/%s/locked", wt->id)); + if (strbuf_read_file(&sb, path.buf, 0) < 0) + die_errno(_("failed to read '%s'"), path.buf); + strbuf_release(&path); - strbuf_rtrim(&sb); + strbuf_trim(&sb); return sb.buf; } diff --git a/worktree.h b/worktree.h index 19a6790..3a780c2 100644 --- a/worktree.h +++ b/worktree.h @@ -42,7 +42,7 @@ extern int is_main_worktree(const struct worktree *wt); /* * Return the reason string if the given worktree is locked. Return - * NULL otherwise. + * NULL otherwise. Return value is only valid until the next invocation. */ extern const char *is_worktree_locked(const struct worktree *wt); -- 8< -- Nguyễn Thái Ngọc Duy (5): worktree.c: add find_worktree_by_path() worktree.c: add is_main_worktree() worktree.c: add is_worktree_locked() worktree: add "lock" command worktree: add "unlock" command Documentation/git-worktree.txt | 27 +++++++++--- builtin/worktree.c | 77 ++++++++++++++++++++++++++++++++++ contrib/completion/git-completion.bash | 5 ++- t/t2028-worktree-move.sh (new +x) | 68 ++++++++++++++++++++++++++++++ worktree.c | 38 +++++++++++++++++ worktree.h | 17 ++++++++ 6 files changed, 225 insertions(+), 7 deletions(-) create mode 100755 t/t2028-worktree-move.sh -- 2.8.2.524.g6ff3d78 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [hidden email] More majordomo info at http://vger.kernel.org/majordomo-info.html |
Free forum by Nabble | Edit this page |