From: Mikhail Pchelin Subject: Re: fix .{cvs,git}ignore handling To: gameoftrees@openbsd.org Date: Sat, 15 Aug 2026 10:27:24 +0300 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 < $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