From: Omar Polo <op@omarpolo.com>
Subject: Re: fix revert and remove with missing directories
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Wed, 12 Apr 2023 20:22:09 +0200

On 2023/04/11 14:12:49 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> When directories in a work tree are missing from disk it is currently
> impossible to remove or revert files within the directory.
> With the patch below, 'got remove' will remove the missing files from
> version control and 'got revert' will restore missing the files.
> 
> robert@ noticed the 'got revert' issue. The rm.sh tests made the same
> issue in 'got remove' visible once I had fixed the revert command.
> 
> Ok?

ok op@

two minor style nit below

> -----------------------------------------------
>  make 'got revert' and 'got rm' work on non-existent directories
>  
>  problem found by robert@
>  
> diff cc88020e952af813c1e01b91ab6516969562e972 0ef3ee5b4259299d2bef17091d336aed268c746f
> commit - cc88020e952af813c1e01b91ab6516969562e972
> commit + 0ef3ee5b4259299d2bef17091d336aed268c746f
> blob - 9e87414b038cb6aa0ef965f162999f1108c5d82a
> blob + c6f46c807bd40c15c0cf2c5ba8b5fc399d5d6d88
> --- lib/worktree.c
> +++ lib/worktree.c
> @@ -3879,7 +3879,74 @@ static const struct got_error *
>  	return err;
>  }
>  
> +struct find_missing_children_args {
> +	const char *parent_path;
> +	size_t parent_len;
> +	struct got_pathlist_head *children;
> +	got_cancel_cb cancel_cb;
> +	void *cancel_arg;
> +

extra empty line

> +};
> +
> [...]
> @@ -3920,9 +3999,21 @@ worktree_status(struct got_worktree *worktree, const c
>  				if (err)
>  					goto done;
>  			}
> -			err = report_single_file_status(path, ondisk_path,
> -			    fileindex, status_cb, status_arg, repo,
> -			    report_unchanged, &ignores, no_ignores);
> +			if (TAILQ_EMPTY(&missing_children)) {
> +				err = report_single_file_status(path,
> +				    ondisk_path, fileindex,
> +				    status_cb, status_arg, repo,
> +				    report_unchanged, &ignores, no_ignores);
> +				if (err)
> +					goto done;

While it is fine as-is, I'd remove this check, or replicate in the
else branch below too, or move it after this if-else, because having
it only in one branch tricked me for a second.

> +			} else {
> +				err = report_children(&missing_children,
> +				    worktree, fileindex, repo,
> +				    (path[0] == '\0'), report_unchanged,
> +				    &ignores, no_ignores,
> +				    status_cb, status_arg,
> +				    cancel_cb, cancel_arg);
> +			}
>  		}
>  	} else {
>  		fdiff_cb.diff_old_new = status_old_new;