From f054996d8370c3213f8c8cbde908545ba6c0f854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Tue, 22 Nov 2016 17:00:44 +0700 Subject: [PATCH 1/5] worktree.c: zero new 'struct worktree' on allocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This keeps things a bit simpler when we add more fields, knowing that default values are always zero. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- worktree.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/worktree.c b/worktree.c index f7869f8d60..f7c1b5e24d 100644 --- a/worktree.c +++ b/worktree.c @@ -91,16 +91,11 @@ static struct worktree *get_main_worktree(void) if (parse_ref(path.buf, &head_ref, &is_detached) < 0) goto done; - worktree = xmalloc(sizeof(struct worktree)); + worktree = xcalloc(1, sizeof(*worktree)); worktree->path = strbuf_detach(&worktree_path, NULL); - worktree->id = NULL; worktree->is_bare = is_bare; - worktree->head_ref = NULL; worktree->is_detached = is_detached; - worktree->is_current = 0; add_head_info(&head_ref, worktree); - worktree->lock_reason = NULL; - worktree->lock_reason_valid = 0; done: strbuf_release(&path); @@ -138,16 +133,11 @@ static struct worktree *get_linked_worktree(const char *id) if (parse_ref(path.buf, &head_ref, &is_detached) < 0) goto done; - worktree = xmalloc(sizeof(struct worktree)); + worktree = xcalloc(1, sizeof(*worktree)); worktree->path = strbuf_detach(&worktree_path, NULL); worktree->id = xstrdup(id); - worktree->is_bare = 0; - worktree->head_ref = NULL; worktree->is_detached = is_detached; - worktree->is_current = 0; add_head_info(&head_ref, worktree); - worktree->lock_reason = NULL; - worktree->lock_reason_valid = 0; done: strbuf_release(&path); From 96f09e2a1100d78aacad80a741c3046031fda6b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 28 Nov 2016 16:36:53 +0700 Subject: [PATCH 2/5] worktree: reorder an if statement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is no-op. But it helps reduce diff noise in the next patch. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/worktree.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 5c4854d3e4..8a654e4ad3 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -406,10 +406,10 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len) else { strbuf_addf(&sb, "%-*s ", abbrev_len, find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV)); - if (!wt->is_detached) - strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0)); - else + if (wt->is_detached) strbuf_addstr(&sb, "(detached HEAD)"); + else + strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0)); } printf("%s\n", sb.buf); From a234563a3b329c8513fabbe9d8f9435987f10eb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 28 Nov 2016 16:36:54 +0700 Subject: [PATCH 3/5] get_worktrees() must return main worktree as first item even on error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is required by git-worktree.txt, stating that the main worktree is the first line (especially in --porcelain mode when we can't just change behavior at will). There's only one case when get_worktrees() may skip main worktree, when parse_ref() fails. Update the code so that we keep first item as main worktree and return something sensible in this case: - In user-friendly mode, since we're not constraint by anything, returning "(error)" should do the job (we already show "(detached HEAD)" which is not machine-friendly). Actually errors should be printed on stderr by parse_ref() (*) - In plumbing mode, we do not show neither 'bare', 'detached' or 'branch ...', which is possible by the format description if I read it right. Careful readers may realize that when the local variable "head_ref" in get_main_worktree() is emptied, add_head_info() will do nothing to wt->head_sha1. But that's ok because head_sha1 is zero-ized in the previous patch. (*) Well, it does not. But it's supposed to be a stop gap implementation until we can reuse refs code to parse "ref: " stuff in HEAD, from resolve_refs_unsafe(). Now may be the time since refs refactoring is mostly done. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/worktree.c | 6 ++++-- t/t2027-worktree-list.sh | 21 +++++++++++++++++++++ worktree.c | 10 +++------- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 8a654e4ad3..b835b91f63 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -388,7 +388,7 @@ static void show_worktree_porcelain(struct worktree *wt) printf("HEAD %s\n", sha1_to_hex(wt->head_sha1)); if (wt->is_detached) printf("detached\n"); - else + else if (wt->head_ref) printf("branch %s\n", wt->head_ref); } printf("\n"); @@ -408,8 +408,10 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len) find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV)); if (wt->is_detached) strbuf_addstr(&sb, "(detached HEAD)"); - else + else if (wt->head_ref) strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0)); + else + strbuf_addstr(&sb, "(error)"); } printf("%s\n", sb.buf); diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh index 1b1b65a6b0..98b5f340e5 100755 --- a/t/t2027-worktree-list.sh +++ b/t/t2027-worktree-list.sh @@ -96,4 +96,25 @@ test_expect_success 'bare repo cleanup' ' rm -rf bare1 ' +test_expect_success 'broken main worktree still at the top' ' + git init broken-main && + ( + cd broken-main && + test_commit new && + git worktree add linked && + cat >expected <<-EOF && + worktree $(pwd) + HEAD $_z40 + + EOF + cd linked && + echo "worktree $(pwd)" >expected && + echo "ref: .broken" >../.git/HEAD && + git worktree list --porcelain | head -n 3 >actual && + test_cmp ../expected actual && + git worktree list | head -n 1 >actual.2 && + grep -F "(error)" actual.2 + ) +' + test_done diff --git a/worktree.c b/worktree.c index f7c1b5e24d..3145522536 100644 --- a/worktree.c +++ b/worktree.c @@ -88,16 +88,13 @@ static struct worktree *get_main_worktree(void) strbuf_addf(&path, "%s/HEAD", get_git_common_dir()); - if (parse_ref(path.buf, &head_ref, &is_detached) < 0) - goto done; - worktree = xcalloc(1, sizeof(*worktree)); worktree->path = strbuf_detach(&worktree_path, NULL); worktree->is_bare = is_bare; worktree->is_detached = is_detached; - add_head_info(&head_ref, worktree); + if (!parse_ref(path.buf, &head_ref, &is_detached)) + add_head_info(&head_ref, worktree); -done: strbuf_release(&path); strbuf_release(&worktree_path); strbuf_release(&head_ref); @@ -173,8 +170,7 @@ struct worktree **get_worktrees(void) list = xmalloc(alloc * sizeof(struct worktree *)); - if ((list[counter] = get_main_worktree())) - counter++; + list[counter++] = get_main_worktree(); strbuf_addf(&path, "%s/worktrees", get_git_common_dir()); dir = opendir(path.buf); From 4fff1ef7ffe0e370459242cf08c51177eeb4181f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 28 Nov 2016 16:36:55 +0700 Subject: [PATCH 4/5] worktree.c: get_worktrees() takes a new flag argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is another no-op patch, in preparation for get_worktrees() to do optional things, like sorting. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- branch.c | 2 +- builtin/branch.c | 2 +- builtin/worktree.c | 6 +++--- worktree.c | 4 ++-- worktree.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/branch.c b/branch.c index 0d459b3cfe..c431cbf6a9 100644 --- a/branch.c +++ b/branch.c @@ -348,7 +348,7 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree) int replace_each_worktree_head_symref(const char *oldref, const char *newref) { int ret = 0; - struct worktree **worktrees = get_worktrees(); + struct worktree **worktrees = get_worktrees(0); int i; for (i = 0; worktrees[i]; i++) { diff --git a/builtin/branch.c b/builtin/branch.c index 60cc5c8e8d..475707528a 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -531,7 +531,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin static void reject_rebase_or_bisect_branch(const char *target) { - struct worktree **worktrees = get_worktrees(); + struct worktree **worktrees = get_worktrees(0); int i; for (i = 0; worktrees[i]; i++) { diff --git a/builtin/worktree.c b/builtin/worktree.c index b835b91f63..d7d195cd95 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix) if (ac) usage_with_options(worktree_usage, options); else { - struct worktree **worktrees = get_worktrees(); + struct worktree **worktrees = get_worktrees(0); int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i; if (!porcelain) @@ -478,7 +478,7 @@ static int lock_worktree(int ac, const char **av, const char *prefix) if (ac != 1) usage_with_options(worktree_usage, options); - worktrees = get_worktrees(); + worktrees = get_worktrees(0); wt = find_worktree(worktrees, prefix, av[0]); if (!wt) die(_("'%s' is not a working tree"), av[0]); @@ -511,7 +511,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix) if (ac != 1) usage_with_options(worktree_usage, options); - worktrees = get_worktrees(); + worktrees = get_worktrees(0); wt = find_worktree(worktrees, prefix, av[0]); if (!wt) die(_("'%s' is not a working tree"), av[0]); diff --git a/worktree.c b/worktree.c index 3145522536..ead088e43c 100644 --- a/worktree.c +++ b/worktree.c @@ -160,7 +160,7 @@ static void mark_current_worktree(struct worktree **worktrees) free(git_dir); } -struct worktree **get_worktrees(void) +struct worktree **get_worktrees(unsigned flags) { struct worktree **list = NULL; struct strbuf path = STRBUF_INIT; @@ -327,7 +327,7 @@ const struct worktree *find_shared_symref(const char *symref, if (worktrees) free_worktrees(worktrees); - worktrees = get_worktrees(); + worktrees = get_worktrees(0); for (i = 0; worktrees[i]; i++) { struct worktree *wt = worktrees[i]; diff --git a/worktree.h b/worktree.h index 90e1311fa7..2e68d4ad86 100644 --- a/worktree.h +++ b/worktree.h @@ -23,7 +23,7 @@ struct worktree { * The caller is responsible for freeing the memory from the returned * worktree(s). */ -extern struct worktree **get_worktrees(void); +extern struct worktree **get_worktrees(unsigned flags); /* * Return git dir of the worktree. Note that the path may be relative. From 4df1d4d4666eb26b420d5b386010470729846b8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 28 Nov 2016 16:36:56 +0700 Subject: [PATCH 5/5] worktree list: keep the list sorted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It makes it easier to write tests for. But it should also be good for the user since locating a worktree by eye would be easier once they notice this. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/worktree.c | 2 +- t/t2027-worktree-list.sh | 19 +++++++++++++++++++ worktree.c | 14 ++++++++++++++ worktree.h | 2 ++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index d7d195cd95..9a97e37a3f 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -447,7 +447,7 @@ static int list(int ac, const char **av, const char *prefix) if (ac) usage_with_options(worktree_usage, options); else { - struct worktree **worktrees = get_worktrees(0); + struct worktree **worktrees = get_worktrees(GWT_SORT_LINKED); int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i; if (!porcelain) diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh index 98b5f340e5..465eeeacd3 100755 --- a/t/t2027-worktree-list.sh +++ b/t/t2027-worktree-list.sh @@ -117,4 +117,23 @@ test_expect_success 'broken main worktree still at the top' ' ) ' +test_expect_success 'linked worktrees are sorted' ' + mkdir sorted && + git init sorted/main && + ( + cd sorted/main && + test_tick && + test_commit new && + git worktree add ../first && + git worktree add ../second && + git worktree list --porcelain | grep ^worktree >actual + ) && + cat >expected <<-EOF && + worktree $(pwd)/sorted/main + worktree $(pwd)/sorted/first + worktree $(pwd)/sorted/second + EOF + test_cmp expected sorted/main/actual +' + test_done diff --git a/worktree.c b/worktree.c index ead088e43c..eb6121263b 100644 --- a/worktree.c +++ b/worktree.c @@ -160,6 +160,13 @@ static void mark_current_worktree(struct worktree **worktrees) free(git_dir); } +static int compare_worktree(const void *a_, const void *b_) +{ + const struct worktree *const *a = a_; + const struct worktree *const *b = b_; + return fspathcmp((*a)->path, (*b)->path); +} + struct worktree **get_worktrees(unsigned flags) { struct worktree **list = NULL; @@ -191,6 +198,13 @@ struct worktree **get_worktrees(unsigned flags) ALLOC_GROW(list, counter + 1, alloc); list[counter] = NULL; + if (flags & GWT_SORT_LINKED) + /* + * don't sort the first item (main worktree), which will + * always be the first + */ + QSORT(list + 1, counter - 1, compare_worktree); + mark_current_worktree(list); return list; } diff --git a/worktree.h b/worktree.h index 2e68d4ad86..d59ce1fee8 100644 --- a/worktree.h +++ b/worktree.h @@ -15,6 +15,8 @@ struct worktree { /* Functions for acting on the information about worktrees. */ +#define GWT_SORT_LINKED (1 << 0) /* keeps linked worktrees sorted */ + /* * Get the worktrees. The primary worktree will always be the first returned, * and linked worktrees will be pointed to by 'next' in each subsequent