git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 6/9] worktree: teach 'add' to respect --force for registered but missing path

From
Jeff King <peff@peff.net>
Date
Aug 30, 2018, 07:36 UTC
Message-ID
<20180830073642.GC11944@sigill.intra.peff.net>
In-Reply-To
<20180828212026.21989-7-sunshine@sunshineco.com>
On Tue, Aug 28, 2018 at 05:20:23PM -0400, Eric Sunshine wrote:
Show 18 quoted lines
> For safety, "git worktree add <path>" will refuse to add a new
> worktree at <path> if <path> is already associated with a worktree
> entry, even if <path> is missing (for instance, has been deleted or
> resides on non-mounted removable media or network share). The typical
> way to re-create a worktree at <path> in such a situation is either to
> prune all "broken" entries ("git worktree prune") or to selectively
> remove the worktree entry manually ("git worktree remove <path>").
> 
> However, neither of these approaches ("prune" nor "remove") is
> especially convenient, and they may be unsuitable for scripting when a
> tool merely wants to re-use a worktree if it exists or create it from
> scratch if it doesn't (much as a tool might use "mkdir -p" to re-use
> or create a directory).
> 
> Therefore, teach 'add' to respect --force as a convenient way to
> re-use a path already associated with a worktree entry if the path is
> non-existent. For a locked worktree, require --force to be specified
> twice.
This makes sense to me, and...
Show 6 quoted lines
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  Documentation/git-worktree.txt |  8 ++++++--
>  builtin/worktree.c             | 10 ++++++++--
>  t/t2025-worktree-add.sh        | 13 ++++++++++++-
>  3 files changed, 26 insertions(+), 5 deletions(-)
The patch looks quite good. One minor comment:
Show 13 quoted lines
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 1122f27b5f..3eb2f89b0f 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -241,10 +241,16 @@ static void validate_worktree_add(const char *path, const struct add_opts *opts)
>  		goto done;
>  
>  	locked = !!is_worktree_locked(wt);
> +	if ((!locked && opts->force) || (locked && opts->force > 1)) {
> +		if (delete_git_dir(wt->id))
> +		    die(_("unable to re-add worktree '%s'"), path);
> +		goto done;
> +	}

This "unable to re-add" seemed funny to me at first, since the failure is in deletion. I guess we're relying on delete_git_dir() to already have said "I had trouble deleting $GIT_DIR/worktrees/foo", and this is just the follow-up to tell that the whole operation is cancelled. So that makes sense.

I wonder if we should volunteer the information that we're overwriting an existing worktree. I guess the user would generally know that already, though, since they just specified "-f", so it's probably just being overly chatty to do so.

-Peff
Previous: Eric SunshineNext: Eric Sunshine
Message 9 of 28 in “worktree: fix bugs and broaden --force applicability”
  1. 0/9 worktree: fix bugs and broaden --force applicabilityEric Sunshine, Aug 28, 2018
  2. 1/9 worktree: don't die() in library function find_worktree()Eric Sunshine, Aug 28, 2018
  3. 2/9 worktree: move delete_git_dir() earlier in file for upcoming new callersEric Sunshine, Aug 28, 2018
  4. 4/9 worktree: prepare for more checks of whether path can become worktreeEric Sunshine, Aug 28, 2018
  5. 3/9 worktree: generalize delete_git_dir() to reduce code duplicationEric Sunshine, Aug 28, 2018
  6. Jeff KingAug 30, 2018
  7. Eric SunshineAug 30, 2018
  8. 6/9 worktree: teach 'add' to respect --force for registered but missing pathEric Sunshine, Aug 28, 2018
  9. Jeff KingAug 30, 2018
  10. Eric SunshineAug 30, 2018
  11. 8/9 worktree: teach 'remove' to override lock when --force given twiceEric Sunshine, Aug 28, 2018
  12. Jeff KingAug 30, 2018
  13. Eric SunshineAug 30, 2018
  14. 9/9 worktree: delete .git/worktrees if empty after 'remove'Eric Sunshine, Aug 28, 2018
  15. 7/9 worktree: teach 'move' to override lock when --force given twiceEric Sunshine, Aug 28, 2018
  16. Jeff KingAug 30, 2018
  17. Eric SunshineAug 30, 2018
  18. 5/9 worktree: disallow adding same path multiple timesEric Sunshine, Aug 28, 2018
  19. Jeff KingAug 30, 2018
  20. Eric SunshineAug 30, 2018
  21. Jeff KingAug 30, 2018
  22. Eric SunshineAug 30, 2018
  23. Eric SunshineAug 30, 2018
  24. Jeff KingAug 30, 2018
  25. Junio C HamanoAug 30, 2018
  26. Ramsay JonesAug 30, 2018
  27. Jeff KingAug 31, 2018
  28. Ramsay JonesAug 31, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.