From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got: rm * removes current directory
To: Mikhail <mp39590@gmail.com>
Cc: Omar Polo <op@omarpolo.com>, gameoftrees@openbsd.org
Date: Sun, 28 May 2023 12:59:39 +0200

On Sat, May 27, 2023 at 11:19:56AM +0300, Mikhail wrote:
> On Sat, May 27, 2023 at 10:15:53AM +0200, Omar Polo wrote:
> > On 2023/05/27 10:58:41 +0300, Mikhail <mp39590@gmail.com> wrote:
> > > core:~/work/got$ cd got
> > > core:~/work/got/got$ ls
> > > Makefile          git-repository.5  got-worktree.5    got.1
> > > got.c             got.conf.5
> > > core:~/work/got/got$ got rm *
> > > D  got/Makefile
> > > D  got/git-repository.5
> > > D  got/got-worktree.5
> > > D  got/got.1
> > > D  got/got.c
> > > D  got/got.conf.5
> > > core:~/work/got/got$ ls
> > > colorls: .: No such file or directory
> > > core:~/work/got/got$ cd ..
> > > core:~/work/got$ ls got
> > > colorls: got: No such file or directory
> > > 
> > > git rm * removes only files, not current directory
> > > 
> > > in case this is a bug here is a test case:
> > 
> > Since git doesn't track directories, having 'got rm' removing empty
> > directories seems a useful feature to me, even though having $PWD
> > disappearing from under your feets is not nice.
> > 
> > However, what would be the value in keeping empty directories on-disk
> > around?
> 
> I faced this when I was working on port - I needed to "got rm *" inside
> patches/ and then I wanted to add my own patch files there.

I've been thinking about this for a while and did consider whether we
should allow user-control over $PWD removal via options such as -f.
But since the existing options already control other aspects of behaviour
we'd need to add a new option for this, and I don't think that is worth it.

It is easy enough to remove an empty directory manually if needed so
I suppose we should just stop deleting $PWD. This patch makes your new
test pass and doesn't make any of the existing tests fail.

diff /home/stsp/src/got
commit - 77fc0a255189c6c18e1ea0d6ea82ae1d0ddb4ea7
path + /home/stsp/src/got
blob - 622c0ce1ca669ed14ca0d26259483e9beea95863
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -2147,7 +2147,11 @@ remove_ondisk_file(const char *root_path, const char *
 {
 	const struct got_error *err = NULL;
 	char *ondisk_path = NULL, *parent = NULL;
+	char cwd[PATH_MAX];
 
+	if (getcwd(cwd, sizeof(cwd)) == NULL)
+		return got_error_from_errno("getcwd");
+
 	if (asprintf(&ondisk_path, "%s/%s", root_path, path) == -1)
 		return got_error_from_errno("asprintf");
 
@@ -2160,7 +2164,9 @@ remove_ondisk_file(const char *root_path, const char *
 		if (err)
 			goto done;
 		while (got_path_cmp(parent, root_path,
-		    strlen(parent), root_len) != 0) {
+		    strlen(parent), root_len) != 0 &&
+		    got_path_cmp(parent, cwd,
+		    strlen(parent), strlen(cwd)) != 0) {
 			free(ondisk_path);
 			ondisk_path = parent;
 			parent = NULL;
@@ -4394,7 +4400,8 @@ schedule_for_deletion(void *arg, unsigned char status,
 	}
 
 	if (!a->keep_on_disk && status != GOT_STATUS_MISSING) {
-		size_t root_len;
+		char cwd[PATH_MAX];
+		size_t cwd_len, root_len;
 
 		if (dirfd != -1) {
 			if (unlinkat(dirfd, de_name, 0) == -1) {
@@ -4407,12 +4414,21 @@ schedule_for_deletion(void *arg, unsigned char status,
 			goto done;
 		}
 
+		if (getcwd(cwd, sizeof(cwd)) == NULL) {
+			err = got_error_from_errno("getcwd");
+			goto done;
+		}
+		cwd_len = strlen(cwd);
+
 		root_len = strlen(a->worktree->root_path);
 		do {
 			char *parent;
 			err = got_path_dirname(&parent, ondisk_path);
 			if (err)
 				goto done;
+			if (got_path_cmp(parent, cwd, strlen(parent),
+			    cwd_len) == 0)
+				break;
 			free(ondisk_path);
 			ondisk_path = parent;
 			if (rmdir(ondisk_path) == -1) {