From: Stefan Sperling <stsp@stsp.name>
Subject: Re: ignored files can't handle trailing slashes
To: Lucas <lucas@sexy.is>
Cc: gameoftrees@openbsd.org
Date: Sun, 5 Feb 2023 18:04:33 +0100

On Sun, Feb 05, 2023 at 02:05:30PM +0000, Lucas wrote:
> Hello list,
> 
> As I shared on IRC, when `got import -I dir/` is used, `dir/` is *not*
> ignored: fnmatch is called with the name in dirent's d_name, which
> doesn't include a trailing slash if the entry is of type directory.
> Do note that calling got_path_strip_trailing_slashes on `-I` argument
> is not an option: 'dir*/' can have some nasty side effects. The patch
> for regress/cmdline/import.sh below makes this explicit.
> 
> Same can be observed for .{cvs,git}ignore.
> 
> As for the fix, I believe it's outside of my reach for the time being.
> The snippet below fixes it for import's case in a quite rough way: I
> don't like malloc'ing on every iteration, but it seems the only way to
> do it without screwing -portable (POSIX only mandates for d_name in
> dirent, and doesn't specify a max size).
> 
> The best way to actually fix it should be as stsp@ suggested: unify the
> ignore matching in both got import and the commands that deal with
> .{cvs,git}ignore, and then fix the matching.

The patch below is what I would suggest.

This extends the existing test case to cover this issue and also avoids
the extra allocation. We must pay the cost of an extra strlcpy in case
the user passes a slash because we should not modify pe->path in-place.

diff 27749ea2ddbc482ad434ed865e0f855313db0a27 d489b0b043cb277f2a76f0fb76b1603f5bb02fca
commit - 27749ea2ddbc482ad434ed865e0f855313db0a27
commit + d489b0b043cb277f2a76f0fb76b1603f5bb02fca
blob - 9b6bfeb03479b844dcb5ea253742cfce88a0b45f
blob + f6751ef796e43adee8a439b80804fde1f72ca199
--- lib/repository.c
+++ lib/repository.c
@@ -2154,7 +2154,20 @@ write_tree(struct got_object_id **new_tree_id, const c
 			continue;
 
 		TAILQ_FOREACH(pe, ignores, entry) {
-			if (fnmatch(pe->path, de->d_name, 0) == 0) {
+			if (pe->path_len > 0 &&
+			    pe->path[pe->path_len - 1] == '/') {
+				char stripped[PATH_MAX];
+				if (strlcpy(stripped, pe->path,
+				    sizeof(stripped)) >= sizeof(stripped)) {
+					err = got_error(GOT_ERR_NO_SPACE);
+					goto done;
+				}
+				got_path_strip_trailing_slashes(stripped);
+				if (fnmatch(stripped, de->d_name, 0) == 0) {
+					ignore = 1;
+					break;
+				}
+			} else if (fnmatch(pe->path, de->d_name, 0) == 0) {
 				ignore = 1;
 				break;
 			}
blob - 9ce21cacfc1fea14fa9289ed4180e0640cfdffdf
blob + fe27dceddf1f2be371eba833bd864eb7a2ca636e
--- regress/cmdline/import.sh
+++ regress/cmdline/import.sh
@@ -374,7 +374,7 @@ test_import_ignores() {
 	mkdir $testroot/tree
 	make_test_tree $testroot/tree
 
-	got import -I alpha -I '*lta*' -I '*silon' \
+	got import -I alpha -I '*lta*' -I '*silon/' \
 		-m 'init' -r $testroot/repo $testroot/tree > $testroot/stdout
 	ret=$?
 	if [ $ret -ne 0 ]; then