Re: [PATCH 00/11] git worktree (re)move
Junio C Hamano writes: > Duy Nguyen writes: > >> The following patch should fix it if that's the same thing you saw. I >> could pile it on worktree-move series, or you can make it a separate >> one-patch series. What's your preference? > > Giving a stable output to the users is probably a good preparatory > fix to what is already in the released versions, so it would make > the most sense to make it a separate patch to be applied to maint > then build the remainder on top. > > I do not think "always show the primary first" is necessarily a good > idea (I would have expected an output more like "git branch --list"). Yikes, "worktree list" is documented like so: List details of each worktree. The main worktree is listed first, followed by each of the linked worktrees. If the primary workree is somehow missing, it still should be listed first as missing---otherwise the readers of --porcelain readers will have hard time telling what is going on.
Re: [PATCH 00/11] git worktree (re)move
Duy Nguyen writes: > The following patch should fix it if that's the same thing you saw. I > could pile it on worktree-move series, or you can make it a separate > one-patch series. What's your preference? Giving a stable output to the users is probably a good preparatory fix to what is already in the released versions, so it would make the most sense to make it a separate patch to be applied to maint then build the remainder on top. I do not think "always show the primary first" is necessarily a good idea (I would have expected an output more like "git branch --list").
Re: [PATCH 00/11] git worktree (re)move
On Wed, Nov 16, 2016 at 8:05 PM, Duy Nguyen wrote: > diff --git a/worktree.c b/worktree.c > index f7869f8..fe92d6f 100644 > --- a/worktree.c > +++ b/worktree.c > @@ -173,6 +173,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(void) > { > struct worktree **list = NULL; > @@ -205,6 +212,11 @@ struct worktree **get_worktrees(void) > ALLOC_GROW(list, counter + 1, alloc); > list[counter] = NULL; > > + /* > +* don't sort the first item (main worktree), which will > +* always be the first > +*/ Urgh.. I should review my patches more carefully before sending out :( The main worktree could be missing (failing to parse HEAD) so I need a better trick than simply assuming the first item is the main worktree here. Tests did not catch this, naturally.. > + qsort(list + 1, counter - 1, sizeof(*list), compare_worktree); > mark_current_worktree(list); > return list; > } -- Duy
Re: [PATCH 00/11] git worktree (re)move
On Sat, Nov 12, 2016 at 06:53:44PM -0800, Junio C Hamano wrote: > not ok 12 - move worktree > # > # git worktree move source destination && > # test_path_is_missing source && > # git worktree list --porcelain | grep "^worktree" >actual && > # cat <<-EOF >expected && > # worktree $TRASH_DIRECTORY > # worktree $TRASH_DIRECTORY/destination > # worktree $TRASH_DIRECTORY/elsewhere > # EOF > # test_cmp expected actual && > # git -C destination log --format=%s >actual2 && > # echo init >expected2 && > # test_cmp expected2 actual2 I think I've seen this (i.e. 'expected' and 'actual' differ only in the order of items) once after a rebase and ignored it, assuming something was changed during the rebase that caused this. The following patch should fix it if that's the same thing you saw. I could pile it on worktree-move series, or you can make it a separate one-patch series. What's your preference? -- 8< -- Subject: [PATCH] worktree list: keep the list sorted 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 --- worktree.c | 12 1 file changed, 12 insertions(+) diff --git a/worktree.c b/worktree.c index f7869f8..fe92d6f 100644 --- a/worktree.c +++ b/worktree.c @@ -173,6 +173,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(void) { struct worktree **list = NULL; @@ -205,6 +212,11 @@ struct worktree **get_worktrees(void) ALLOC_GROW(list, counter + 1, alloc); list[counter] = NULL; + /* +* don't sort the first item (main worktree), which will +* always be the first +*/ + qsort(list + 1, counter - 1, sizeof(*list), compare_worktree); mark_current_worktree(list); return list; } -- 2.8.2.524.g6ff3d78 -- 8< --
[PATCH 00/11] git worktree (re)move
This is mostly a resend from last time [1]. The main difference is patch 10/11 to prevent moving a worktree that contains submodules because these .git files may need rewritten to point to the right place. I'm leaving the rewriting .git files for future (preferably done by "submodules guys"). [1] https://public-inbox.org/git/20160625075433.4608-1-pclo...@gmail.com/ Nguyễn Thái Ngọc Duy (11): copy.c: import copy_file() from busybox copy.c: delete unused code in copy_file() copy.c: convert bb_(p)error_msg to error(_errno) copy.c: style fix copy.c: convert copy_file() to copy_dir_recursively() worktree.c: add validate_worktree() worktree.c: add update_worktree_location() worktree move: new command worktree move: accept destination as directory worktree move: refuse to move worktrees with submodules worktree remove: new command Documentation/git-worktree.txt | 28 ++- builtin/worktree.c | 179 cache.h| 1 + contrib/completion/git-completion.bash | 5 +- copy.c | 369 + t/t2028-worktree-move.sh | 56 + worktree.c | 84 worktree.h | 11 + 8 files changed, 722 insertions(+), 11 deletions(-) -- 2.8.2.524.g6ff3d78