From: Omar Polo <op@omarpolo.com>
Subject: Re: got: rm * removes current directory
To: Stefan Sperling <stsp@stsp.name>
Cc: Mark Jamsek <mark@jamsek.com>, Mikhail <mp39590@gmail.com>, gameoftrees@openbsd.org
Date: Mon, 29 May 2023 16:43:32 +0200

On 2023/05/29 13:37:57 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> On Mon, May 29, 2023 at 08:06:12PM +1000, Mark Jamsek wrote:
> > On 23-05-29 11:46AM, Omar Polo wrote:
> > > just to be clear, assuming a repository and a worktree with the
> > > following entries:
> > > 
> > > 	README
> > > 	foo/bar.c
> > > 	foo/x.y
> > > 
> > > what do you expect after doing "got rm foo/bar.c foo/x.y" ?
> > > 
> > > With the proposed diff "foo" will be removed as it only affect what
> > > "cd foo && got rm bar.c x.y" does, leaving foo around as a special
> > > case to avoid removing the current working directory.  This is the
> > > quirk I don't like, but it's personal preference.
> > > 
> > > If everything, I'd prefer if we settle on "got rm foo/bar.c foo/x.y"
> > > leaving "foo" around (which is what I assume jamsek intended), it
> > > would be coherent with rm and don't have any special case.  It could
> > > be tricky though, as with "got rm -R foo" I'd expect foo to be deleted
> > > as well.
> > 
> > Yes, you are right. I would expect what you describe: 'foo' remains on
> > disk.
> > 
> > So irrespective of whether `got rm`ing all the files of a subdir from
> > within that directory or from above that directory, the directory will
> > remain.
> > 
> > In that case, I agree with your aversion to the quirk.
> 
> How about this diff, then?

shorter than what I'd expected! :)

ok op@, thank you a lot for this.

One minor style nit below, and I agree with jamsek re asprintf vs
strlen fwiw.

> --- lib/worktree.c
> +++ lib/worktree.c
> [...]
> @@ -4468,8 +4476,19 @@ got_worktree_schedule_delete(struct got_worktree *work
>  	sda.status_codes = status_codes;
>  
>  	TAILQ_FOREACH(pe, paths, entry) {
> +		char *ondisk_status_path;
> +
> +		if (asprintf(&ondisk_status_path, "%s%s%s",
> +		    got_worktree_get_root_path(worktree),
> +			pe->path[0] == '\0' ? "" : "/", pe->path) == -1) {

can you please fix the indentation here before committing?

> +			err = got_error_from_errno("asprintf");
> +			goto done;
> +		}