Re: [PATCH 8/9] worktree: teach 'remove' to override lock when --force given twice

[Eric Sunshine]

On Thu, Aug 30, 2018 at 3:40 AM Jeff King  wrote:
> On Tue, Aug 28, 2018 at 05:20:25PM -0400, Eric Sunshine wrote:
> > -  N_("force removing even if the worktree is dirty"),
> > +  N_("force removal even if worktree is dirty or 
> > locked"),
>
> I wonder if somebody might assume from this that a single "-f" would
> override a lock. Perhaps not the end of the world, and the manpage does
> make it clear. And also I don't really know how to be more specific here
> without an overly long line.

Precisely, on all counts. Plus, if they try a single --force and it
fails, the hint printed when it fails says explicitly to use --force
twice, so the "solution" is easily discoverable.

> I'm guessing all those thoughts went through your head before ending up
> here, too. :)

You guess correctly and you came to the same conclusion as I did.
These -h summary lines are just to short for any real discussion.

Re: [PATCH 8/9] worktree: teach 'remove' to override lock when --force given twice

[Jeff King]

On Tue, Aug 28, 2018 at 05:20:25PM -0400, Eric Sunshine wrote:

> For consistency with "add -f -f" and "move -f -f" which override
> the lock on a worktree, allow "remove -f -f" to do so, as well, as a
> convenience.

This one makes more sense to me than the last, since "remove -f" already
does something useful.

> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 354a6c0eb5..a95fe67da6 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -875,13 +875,13 @@ static int remove_worktree(int ac, const char **av, 
> const char *prefix)
>   int force = 0;
>   struct option options[] = {
>   OPT__FORCE(,
> -  N_("force removing even if the worktree is dirty"),
> +  N_("force removal even if worktree is dirty or 
> locked"),

I wonder if somebody might assume from this that a single "-f" would
override a lock. Perhaps not the end of the world, and the manpage does
make it clear. And also I don't really know how to be more specific here
without an overly long line.

I'm guessing all those thoughts went through your head before ending up
here, too. :)

-Peff

[PATCH 8/9] worktree: teach 'remove' to override lock when --force given twice

[Eric Sunshine]

For consistency with "add -f -f" and "move -f -f" which override
the lock on a worktree, allow "remove -f -f" to do so, as well, as a
convenience.

Signed-off-by: Eric Sunshine 
---
 Documentation/git-worktree.txt |  1 +
 builtin/worktree.c | 11 ++-
 t/t2028-worktree-move.sh   | 10 ++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index d08b8d8e4f..e2ee9fc21b 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -129,6 +129,7 @@ OPTIONS
 twice.
 +
 `remove` refuses to remove an unclean working tree unless `--force` is used.
+To remove a locked working tree, specify `--force` twice.
 
 -b ::
 -B ::
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 354a6c0eb5..a95fe67da6 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -875,13 +875,13 @@ static int remove_worktree(int ac, const char **av, const 
char *prefix)
int force = 0;
struct option options[] = {
OPT__FORCE(,
-N_("force removing even if the worktree is dirty"),
+N_("force removal even if worktree is dirty or 
locked"),
 PARSE_OPT_NOCOMPLETE),
OPT_END()
};
struct worktree **worktrees, *wt;
struct strbuf errmsg = STRBUF_INIT;
-   const char *reason;
+   const char *reason = NULL;
int ret = 0;
 
ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
@@ -894,12 +894,13 @@ static int remove_worktree(int ac, const char **av, const 
char *prefix)
die(_("'%s' is not a working tree"), av[0]);
if (is_main_worktree(wt))
die(_("'%s' is a main working tree"), av[0]);
-   reason = is_worktree_locked(wt);
+   if (force < 2)
+   reason = is_worktree_locked(wt);
if (reason) {
if (*reason)
-   die(_("cannot remove a locked working tree, lock 
reason: %s"),
+   die(_("cannot remove a locked working tree, lock 
reason: %s\nuse 'remove -f -f' to override or unlock first"),
reason);
-   die(_("cannot remove a locked working tree"));
+   die(_("cannot remove a locked working tree;\nuse 'remove -f -f' 
to override or unlock first"));
}
if (validate_worktree(wt, , WT_VALIDATE_WORKTREE_MISSING_OK))
die(_("validation failed, cannot remove working tree: %s"),
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 9756ede8f1..1b5079e8fa 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -163,4 +163,14 @@ test_expect_success 'proper error when worktree not found' 
'
done
 '
 
+test_expect_success 'remove locked worktree (force)' '
+   git worktree add --detach gumby &&
+   test_when_finished "git worktree remove gumby || :" &&
+   git worktree lock gumby &&
+   test_when_finished "git worktree unlock gumby || :" &&
+   test_must_fail git worktree remove gumby &&
+   test_must_fail git worktree remove --force gumby &&
+   git worktree remove --force --force gumby
+'
+
 test_done
-- 
2.19.0.rc1.350.ge57e33dbd1