Re: [PATCH v5 1/5] worktree remove: refuse a worktree in the middle of an operation

[Junio C Hamano <gitster@pobox.com>]

Jasper Mayone <me@jaspermayone.com> writes:

> +static void check_no_operation_in_progress(struct worktree *wt,
> +					   const char *original_path)
> +{
> +	struct wt_status_state state = { 0 };
> +
> +	if (wt_status_check_rebase(wt, &state)) {
> ...
> +	if (is_directory(worktree_git_path(wt, "sequencer")))
> +		die(_("'%s' is in the middle of a cherry-pick or revert, use --force to delete it"),
> +		    original_path);
> +}

Having a central place like this is a very good design to keep it
extensible.  In addition to "git worktree remove", we have
protection against checking out the same branch in multiple
worktrees.  How should this check interact with the checks done in
that other code path, if any?

>  static int delete_git_work_tree(struct worktree *wt)
>  {
>  	struct strbuf sb = STRBUF_INIT;
> @@ -1461,8 +1494,10 @@ static int remove_worktree(int ac, const char **av, const char *prefix,
>  	strbuf_release(&errmsg);
>  
>  	if (file_exists(wt->path)) {
> -		if (!force)
> +		if (!force) {
> +			check_no_operation_in_progress(wt, av[0]);
>  			check_clean_worktree(wt, av[0]);
> +		}

In the longer term, I think it would make sense to create a single
helper function that checks if the worktree is in discardable state
(without dying.  Just return an error code instead), call it from
here, and have call(s) to die() here.  Then move everything below
the callchain to worktree.c one level up to libify.

Not in a scope for this change, of course.  #leftoverbits.