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

[Eric Sunshine]

On Thu, Aug 30, 2018 at 3:38 AM Jeff King  wrote:
> On Tue, Aug 28, 2018 at 05:20:24PM -0400, Eric Sunshine wrote:
> > For consistency with "add -f -f", which allows a missing but locked
> > worktree path to be re-used, allow "move -f -f" to override a lock,
> > as well, as a convenience.
>
> I don't have a strong opinion on this one, as I have never used
> "worktree mv" myself. :)

I don't have strong feelings about this either (nor about "remove -f -f").

> But anytime I see "-f -f", I have to wonder what "-f" does. In this
> case, nothing. Is there some future lesser forcing we might use it for?

I had the same concern. A single --force probably ought to be
sufficient (given that there is no other meaning presently for a
single --force), but it somehow seemed wrong to override a lock with a
single --force when the other commands demand specifying it twice.

The strictness could always be downgraded later to require only a
single --force if it becomes obvious that that makes more sense.

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

[Jeff King]

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

> For consistency with "add -f -f", which allows a missing but locked
> worktree path to be re-used, allow "move -f -f" to override a lock,
> as well, as a convenience.

I don't have a strong opinion on this one, as I have never used
"worktree mv" myself. :)

But anytime I see "-f -f", I have to wonder what "-f" does. In this
case, nothing. Is there some future lesser forcing we might use it for?

-Peff

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

[Eric Sunshine]

For consistency with "add -f -f", which allows a missing but locked
worktree path to be re-used, allow "move -f -f" to override a lock,
as well, as a convenience.

Signed-off-by: Eric Sunshine 
---
 Documentation/git-worktree.txt |  3 +++
 builtin/worktree.c | 13 +
 t/t2028-worktree-move.sh   | 14 ++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 8537692f05..d08b8d8e4f 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -125,6 +125,9 @@ OPTIONS
manually). This option overrides these safeguards. To add a missing but
locked working tree path, specify `--force` twice.
 +
+`move` refuses to move a locked working tree unless `--force` is specified
+twice.
++
 `remove` refuses to remove an unclean working tree unless `--force` is used.
 
 -b ::
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 3eb2f89b0f..354a6c0eb5 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -740,13 +740,17 @@ static void validate_no_submodules(const struct worktree 
*wt)
 
 static int move_worktree(int ac, const char **av, const char *prefix)
 {
+   int force = 0;
struct option options[] = {
+   OPT__FORCE(,
+N_("force move even if worktree is dirty or locked"),
+PARSE_OPT_NOCOMPLETE),
OPT_END()
};
struct worktree **worktrees, *wt;
struct strbuf dst = STRBUF_INIT;
struct strbuf errmsg = STRBUF_INIT;
-   const char *reason;
+   const char *reason = NULL;
char *path;
 
ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
@@ -777,12 +781,13 @@ static int move_worktree(int ac, const char **av, const 
char *prefix)
 
validate_no_submodules(wt);
 
-   reason = is_worktree_locked(wt);
+   if (force < 2)
+   reason = is_worktree_locked(wt);
if (reason) {
if (*reason)
-   die(_("cannot move a locked working tree, lock reason: 
%s"),
+   die(_("cannot move a locked working tree, lock reason: 
%s\nuse 'move -f -f' to override or unlock first"),
reason);
-   die(_("cannot move a locked working tree"));
+   die(_("cannot move a locked working tree;\nuse 'move -f -f' to 
override or unlock first"));
}
if (validate_worktree(wt, , 0))
die(_("validation failed, cannot move working tree: %s"),
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 60aba7c41a..9756ede8f1 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -98,6 +98,20 @@ test_expect_success 'move worktree to another dir' '
test_cmp expected2 actual2
 '
 
+test_expect_success 'move locked worktree (force)' '
+   test_when_finished "
+   git worktree unlock flump || :
+   git worktree remove flump || :
+   git worktree unlock ploof || :
+   git worktree remove ploof || :
+   " &&
+   git worktree add --detach flump &&
+   git worktree lock flump &&
+   test_must_fail git worktree move flump ploof" &&
+   test_must_fail git worktree move --force flump ploof" &&
+   git worktree move --force --force flump ploof
+'
+
 test_expect_success 'remove main worktree' '
test_must_fail git worktree remove .
 '
-- 
2.19.0.rc1.350.ge57e33dbd1