[PATCH 00/11] git worktree (re)move

2016-11-11 Thread Nguyễn Thái Ngọc Duy
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



Re: [PATCH 00/11] git worktree (re)move

2016-11-16 Thread Duy Nguyen
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< --


Re: [PATCH 00/11] git worktree (re)move

2016-11-16 Thread Duy Nguyen
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

2016-11-16 Thread Junio C Hamano
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

2016-11-16 Thread Junio C Hamano
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.