From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got ignore some .{cvs,git}ignore files
To: Sebastien Marie <semarie@online.fr>
Cc: gameoftrees@openbsd.org
Date: Tue, 23 Jun 2020 14:59:21 +0200

On Tue, Jun 23, 2020 at 10:50:42AM +0200, Sebastien Marie wrote:
> ah... so the change I made on regress wasn't the right one to expose my problem :)
> 
> new diff for regress.
> -- 
> Sebastien Marie

This patch extends your regress test and fixes the problem.

I am also changing how the status walk discovers .{git,cvs}ignore files.
Instead of adding patterns in the status_new (diff_new) callback, add a new
callback which gets invoked once per directory traversed by the status walk.

ok?
 
diff 2c67d7592929f77e883ff5f84785c6acaa06fa49 fecd16836cdf07f06816d889b1c02d8f31df4e54
blob - f2f5a3cbeb857a547e2c1bb1e3c13aba9479efa2
blob + 656685591bdfde81e5346507229c0bd87d2fa18c
--- lib/fileindex.c
+++ lib/fileindex.c
@@ -1003,6 +1003,12 @@ diff_fileindex_dir(struct got_fileindex *fileindex,
 	size_t path_len = strlen(path);
 	struct got_pathlist_entry *dle;
 
+	if (cb->diff_traverse) {
+		err = cb->diff_traverse(cb_arg, path, dirfd);
+		if (err)
+			return err;
+	}
+
 	dle = TAILQ_FIRST(dirlist);
 	while ((*ie && got_path_is_child((*ie)->path, path, path_len)) || dle) {
 		if (dle && *ie) {
blob - 88f07eccfa3c94fd5c1e4b81985cf5b7558c9ef2
blob + aff771b0551ee8d209acba24e8ce1cfe8383392d
--- lib/got_lib_fileindex.h
+++ lib/got_lib_fileindex.h
@@ -144,10 +144,13 @@ typedef const struct got_error *(*got_fileindex_diff_d
     struct got_fileindex_entry *, const char *);
 typedef const struct got_error *(*got_fileindex_diff_dir_new_cb)(void *,
     struct dirent *, const char *, int);
+typedef const struct got_error *(*got_fileindex_diff_dir_traverse)(void *,
+    const char *, int);
 struct got_fileindex_diff_dir_cb {
 	got_fileindex_diff_dir_old_new_cb diff_old_new;
 	got_fileindex_diff_dir_old_cb diff_old;
 	got_fileindex_diff_dir_new_cb diff_new;
+	got_fileindex_diff_dir_traverse diff_traverse;
 };
 const struct got_error *got_fileindex_diff_dir(struct got_fileindex *, int,
     const char *, const char *, struct got_repository *,
blob - 1ad69b7dff719dd615ae921d1fc94b3ec3fabf39
blob + a1c9a40aad81a8e24de2e0c06e9c5138b03550b1
--- lib/worktree.c
+++ lib/worktree.c
@@ -2384,6 +2384,7 @@ struct diff_dir_cb_arg {
     /* A pathlist containing per-directory pathlists of ignore patterns. */
     struct got_pathlist_head ignores;
     int report_unchanged;
+    int no_ignores;
 };
 
 static const struct got_error *
@@ -2687,23 +2688,8 @@ status_new(void *arg, struct dirent *de, const char *p
 		path = de->d_name;
 	}
 
-	if (de->d_type == DT_DIR) {
-		int subdirfd = openat(dirfd, de->d_name,
-		    O_RDONLY | O_NOFOLLOW | O_DIRECTORY);
-		if (subdirfd == -1) {
-			if (errno != ENOENT && errno != EACCES)
-				err = got_error_from_errno2("openat", path);
-		} else {
-			err = add_ignores(&a->ignores, a->worktree->root_path,
-			    path, subdirfd, ".cvsignore");
-			if (err == NULL)
-				err = add_ignores(&a->ignores,
-				    a->worktree->root_path, path,
-				    subdirfd, ".gitignore");
-			if (close(subdirfd) == -1 && err == NULL)
-				err = got_error_from_errno2("close", path);
-		}
-	} else if (got_path_is_child(path, a->status_path, a->status_path_len)
+	if (de->d_type != DT_DIR &&
+	    got_path_is_child(path, a->status_path, a->status_path_len)
 	    && !match_ignores(&a->ignores, path))
 		err = (*a->status_cb)(a->status_arg, GOT_STATUS_UNVERSIONED,
 		    GOT_STATUS_NO_CHANGE, path, NULL, NULL, NULL, -1, NULL);
@@ -2713,6 +2699,26 @@ status_new(void *arg, struct dirent *de, const char *p
 }
 
 static const struct got_error *
+status_traverse(void *arg, const char *path, int dirfd)
+{
+	const struct got_error *err = NULL;
+	struct diff_dir_cb_arg *a = arg;
+
+	if (a->no_ignores)
+		return NULL;
+
+	err = add_ignores(&a->ignores, a->worktree->root_path,
+	    path, dirfd, ".cvsignore");
+	if (err)
+		return err;
+
+	err = add_ignores(&a->ignores, a->worktree->root_path, path,
+	    dirfd, ".gitignore");
+
+	return err;
+}
+
+static const struct got_error *
 report_single_file_status(const char *path, const char *ondisk_path,
 struct got_fileindex *fileindex, got_worktree_status_cb status_cb,
 void *status_arg, struct got_repository *repo, int report_unchanged)
@@ -2741,6 +2747,50 @@ void *status_arg, struct got_repository *repo, int rep
 }
 
 static const struct got_error *
+add_ignores_from_parent_paths(struct got_pathlist_head *ignores,
+    const char *root_path, const char *path)
+{
+	const struct got_error *err;
+	char *parent_path, *next_parent_path;
+
+	err = add_ignores(ignores, root_path, "", -1,
+	    ".cvsignore");
+	if (err)
+		return err;
+
+	err = add_ignores(ignores, root_path, "", -1,
+	    ".gitignore");
+	if (err)
+		return err;
+
+	err = got_path_dirname(&parent_path, path);
+	if (err) {
+		if (err->code == GOT_ERR_BAD_PATH)
+			return NULL; /* cannot traverse parent */
+		return err;
+	}
+	for (;;) {
+		err = add_ignores(ignores, root_path, parent_path, -1,
+		    ".cvsignore");
+		if (err)
+			break;
+		err = add_ignores(ignores, root_path, parent_path, -1,
+		    ".gitignore");
+		if (err)
+			break;
+		err = got_path_dirname(&next_parent_path, parent_path);
+		if (err) {
+			if (err->code != GOT_ERR_BAD_PATH)
+				return err;
+			err = NULL; /* traversed everything */
+			break;
+		}
+	}
+
+	return err;
+}
+
+static const struct got_error *
 worktree_status(struct got_worktree *worktree, const char *path,
     struct got_fileindex *fileindex, struct got_repository *repo,
     got_worktree_status_cb status_cb, void *status_arg,
@@ -2753,6 +2803,8 @@ worktree_status(struct got_worktree *worktree, const c
 	struct diff_dir_cb_arg arg;
 	char *ondisk_path = NULL;
 
+	TAILQ_INIT(&arg.ignores);
+
 	if (asprintf(&ondisk_path, "%s%s%s",
 	    worktree->root_path, path[0] ? "/" : "", path) == -1)
 		return got_error_from_errno("asprintf");
@@ -2769,6 +2821,7 @@ worktree_status(struct got_worktree *worktree, const c
 		fdiff_cb.diff_old_new = status_old_new;
 		fdiff_cb.diff_old = status_old;
 		fdiff_cb.diff_new = status_new;
+		fdiff_cb.diff_traverse = status_traverse;
 		arg.fileindex = fileindex;
 		arg.worktree = worktree;
 		arg.status_path = path;
@@ -2779,21 +2832,18 @@ worktree_status(struct got_worktree *worktree, const c
 		arg.cancel_cb = cancel_cb;
 		arg.cancel_arg = cancel_arg;
 		arg.report_unchanged = report_unchanged;
-		TAILQ_INIT(&arg.ignores);
+		arg.no_ignores = no_ignores;
 		if (!no_ignores) {
-			err = add_ignores(&arg.ignores, worktree->root_path,
-			    path, fd, ".cvsignore");
-			if (err == NULL)
-				err = add_ignores(&arg.ignores,
-				    worktree->root_path, path, fd,
-				    ".gitignore");
+			err = add_ignores_from_parent_paths(&arg.ignores,
+			    worktree->root_path, path);
+			if (err)
+				goto done;
 		}
-		if (err == NULL)
-			err = got_fileindex_diff_dir(fileindex, fd,
-			    worktree->root_path, path, repo, &fdiff_cb, &arg);
-		free_ignores(&arg.ignores);
+		err = got_fileindex_diff_dir(fileindex, fd,
+		    worktree->root_path, path, repo, &fdiff_cb, &arg);
 	}
-
+done:
+	free_ignores(&arg.ignores);
 	if (fd != -1 && close(fd) != 0 && err == NULL)
 		err = got_error_from_errno("close");
 	free(ondisk_path);
blob - 5abbce9475bb8bd2f1cbb3e33113883173026f03
blob + bf27874f97a3459375523f37a2f63d9a83168936
--- regress/cmdline/status.sh
+++ regress/cmdline/status.sh
@@ -496,10 +496,13 @@ function test_status_cvsignore {
 
 	echo "unversioned file" > $testroot/wt/foo
 	echo "unversioned file" > $testroot/wt/foop
+	echo "unversioned file" > $testroot/wt/epsilon/foo
 	echo "unversioned file" > $testroot/wt/epsilon/bar
 	echo "unversioned file" > $testroot/wt/epsilon/boo
 	echo "unversioned file" > $testroot/wt/epsilon/moo
-	echo "foo" > $testroot/wt/.cvsignore
+	mkdir -p $testroot/wt/epsilon/new/
+	echo "unversioned file" > $testroot/wt/epsilon/new/foo
+	echo "**/foo" > $testroot/wt/.cvsignore
 	echo "bar" > $testroot/wt/epsilon/.cvsignore
 	echo "moo" >> $testroot/wt/epsilon/.cvsignore
 
@@ -508,6 +511,29 @@ function test_status_cvsignore {
 	echo '?  epsilon/boo' >> $testroot/stdout.expected
 	echo '?  foop' >> $testroot/stdout.expected
 	(cd $testroot/wt && got status > $testroot/stdout)
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo '?  epsilon/.cvsignore' > $testroot/stdout.expected
+	echo '?  epsilon/boo' >> $testroot/stdout.expected
+	(cd $testroot/wt && got status epsilon > $testroot/stdout)
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+		
+	echo -n '' > $testroot/stdout.expected
+	(cd $testroot/wt && got status epsilon/new > $testroot/stdout)
 
 	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret="$?"