On Sat, Jun 25, 2016 at 3:54 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> +static int move_worktree(int ac, const char **av, const char *prefix)
> +{
> + [...]
> + if (file_exists(dst.buf))
> + die(_("target '%s' already exists"), av[1]);
> +
> + worktrees = get_worktrees();
'worktrees' is being leaked at each 'return'. Probably need to call
free_worktrees().
> + wt = find_worktree(worktrees, prefix, av[0]);
> + 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 ((reason = is_worktree_locked(wt))) {
> + if (*reason)
> + die(_("already locked, reason: %s"), reason);
> + die(_("already locked, no reason"));
Saying "already locked" makes sense for the 'worktree locked' command,
however, for 'worktree move', a simple "locked" would be less
confusing.
> + }
> + if (validate_worktree(wt, 0))
> + return -1;
> diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
> @@ -59,4 +59,34 @@ test_expect_success 'unlock worktree twice' '
> +test_expect_success 'move non-worktree' '
> + mkdir abc &&
> + test_must_fail git worktree move abc def
> +'
> +
> +test_expect_success 'move locked worktree' '
> + git worktree lock source &&
> + test_must_fail git worktree move source destination &&
> + git worktree unlock source
> +'
Should the unlock be wrapped in a test_when_finished()?
