"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mikhail Pchelin <misha@objdump.ru>
Subject:
Re: fix .{cvs,git}ignore handling
To:
gameoftrees@openbsd.org
Date:
Sat, 15 Aug 2026 10:27:24 +0300

Download raw body.

Thread
On Thu, Aug 13, 2026 at 01:28:53PM +0200, Stefan Sperling wrote:
> On Sat, Aug 08, 2026 at 06:02:01PM +0300, Mikhail Pchelin wrote:
> > If both .cvsignore and .gitignore files exist in a directory, only
> > .cvsignore will be respected and ignores from the .gitignore will be
> > dropped, inlined patch fixes that, both files are respected now, the
> > test case is included.
>
> The intention was to merge patterns from both files into a single
> ignores list. Your regression test is good, but the fix isn't quite
> right. Instead of using two lists, I think read_ignores() needs to be
> changed such that it will append to the existing list if a list already
> exists for the given path. Do you agree?

Works for me, the diff becomes less invasive.

diff refs/heads/main 65e9c2597e3c067ceba3ab59d3fa4873f0758bf5
commit - ae25db3f82d89c28505e5fc1f58b8ba693282f5b
commit + 65e9c2597e3c067ceba3ab59d3fa4873f0758bf5
blob - fca04b3b62b7ecd7035e827c334fa35ad977b7a3
blob + b7787f14aef6a7ac488372fc5c6a224e2755c382
--- lib/worktree.c
+++ lib/worktree.c
@@ -3796,14 +3796,28 @@ read_ignores(struct got_pathlist_head *ignores, const 
 	const struct got_error *err = NULL;
 	struct got_pathlist_entry *pe = NULL;
 	struct got_pathlist_head *ignorelist;
+	struct got_pathlist_entry find;
 	char *line = NULL, *pattern, *dirpath = NULL;
 	size_t linesize = 0;
 	ssize_t linelen;
+	int new_list = 0;
 
-	ignorelist = calloc(1, sizeof(*ignorelist));
-	if (ignorelist == NULL)
-		return got_error_from_errno("calloc");
-	RB_INIT(ignorelist);
+	/*
+	 * Check whether an ignorelist for this path already exists; this
+	 * happens when both .cvsignore and .gitignore exist in the directory.
+	 */
+	find.path = path;
+	find.path_len = strlen(path);
+	pe = RB_FIND(got_pathlist_head, ignores, &find);
+	if (pe != NULL) {
+		ignorelist = pe->data;
+	} else {
+		ignorelist = calloc(1, sizeof(*ignorelist));
+		if (ignorelist == NULL)
+			return got_error_from_errno("calloc");
+		RB_INIT(ignorelist);
+		new_list = 1;
+	}
 
 	while ((linelen = getline(&line, &linesize, f)) != -1) {
 		if (linelen > 0 && line[linelen - 1] == '\n')
@@ -3835,15 +3849,18 @@ read_ignores(struct got_pathlist_head *ignores, const 
 		goto done;
 	}
 
-	dirpath = strdup(path);
-	if (dirpath == NULL) {
-		err = got_error_from_errno("strdup");
-		goto done;
+	if (new_list) {
+		dirpath = strdup(path);
+		if (dirpath == NULL) {
+			err = got_error_from_errno("strdup");
+			goto done;
+		}
+		err = got_pathlist_insert(&pe, ignores, dirpath, ignorelist);
 	}
-	err = got_pathlist_insert(&pe, ignores, dirpath, ignorelist);
 done:
 	free(line);
-	if (err || pe == NULL) {
+	/* Free a freshly allocated list if it was not inserted. */
+	if (new_list && (err || pe == NULL)) {
 		free(dirpath);
 		got_pathlist_free(ignorelist, GOT_PATHLIST_FREE_PATH);
 		free(ignorelist);
blob - 0874b88152ef2725e3b0e312b01cda5d76c37b91
blob + e8ac4403e2d35fac28fe1b01bfe6242b5ede36c4
--- regress/cmdline/status.sh
+++ regress/cmdline/status.sh
@@ -875,6 +875,88 @@ test_status_multiple_gitignore_files() {
 	test_done "$testroot" "$ret"
 }
 
+test_status_cvsignore_and_gitignore_together() {
+	local testroot=`test_init status_cvsignore_and_gitignore_together`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "unversioned file" > $testroot/wt/foo
+	echo "unversioned file" > $testroot/wt/bar
+	echo "unversioned file" > $testroot/wt/baz
+	echo "unversioned file" > $testroot/wt/epsilon/foo
+	echo "unversioned file" > $testroot/wt/epsilon/bar
+	echo "unversioned file" > $testroot/wt/epsilon/baz
+
+	# .{cvs,git}ignore together
+	echo "foo" > $testroot/wt/.cvsignore
+	echo "bar" > $testroot/wt/.gitignore
+	echo "foo" > $testroot/wt/epsilon/.cvsignore
+	echo "bar" > $testroot/wt/epsilon/.gitignore
+
+	echo '?  .cvsignore' > $testroot/stdout.expected
+	echo '?  .gitignore' >> $testroot/stdout.expected
+	echo '?  baz' >> $testroot/stdout.expected
+	echo '?  epsilon/.cvsignore' >> $testroot/stdout.expected
+	echo '?  epsilon/.gitignore' >> $testroot/stdout.expected
+	echo '?  epsilon/baz' >> $testroot/stdout.expected
+	(cd $testroot/wt && got status > $testroot/stdout)
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# .{cvs,git}ignore in subdir
+	echo '?  epsilon/.cvsignore' > $testroot/stdout.expected
+	echo '?  epsilon/.gitignore' >> $testroot/stdout.expected
+	echo '?  epsilon/baz' >> $testroot/stdout.expected
+	(cd $testroot/wt && got status epsilon > $testroot/stdout)
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# -I must still reveal ignored files from both ignore mechanisms.
+	cat > $testroot/stdout.expected <<EOF
+?  .cvsignore
+?  .gitignore
+?  bar
+?  baz
+?  epsilon/.cvsignore
+?  epsilon/.gitignore
+?  epsilon/bar
+?  epsilon/baz
+?  epsilon/foo
+?  foo
+EOF
+	(cd $testroot/wt && got status -I > $testroot/stdout)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "got status failed unexpectedly" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+}
+
 test_status_status_code() {
 	local testroot=`test_init status_status_code`
 
@@ -1247,6 +1329,7 @@ run_test test_status_gitignore_leading_slashes
 run_test test_status_gitignore_trailing_slashes
 run_test test_status_gitignore_comments
 run_test test_status_multiple_gitignore_files
+run_test test_status_cvsignore_and_gitignore_together
 run_test test_status_status_code
 run_test test_status_suppress
 run_test test_status_empty_file