On Sat, Aug 02, 2025 at 12:13:25PM -0400, D. Ben Knoble wrote:

quoted

Also, I guess this function ought to be respecting the literal_pathspecs
global? The actual pathspec parser does.

If we can, we probably ought to be feeding the paths to a function like
pathspec.c:parse_element_magic() and then checking the resulting flags
(and skipping past the prefix as it indicates).

Thanks for pointing me at this; maybe I'll find some time for patches
unless someone beats me to it.

So here's a simple-ish way to use the full pathspec parsing code:

diff --git a/setup.c b/setup.c
index 6f52dab64c..ad27a65d6b 100644
--- a/setup.c
+++ b/setup.c

@@ -176,28 +176,27 @@ int path_inside_repo(const char *prefix, const char *path)
 
 int check_filename(const char *prefix, const char *arg)
 {
-	char *to_free = NULL;
+	const char *args[] = { arg, NULL };
+	struct pathspec ps;
 	struct stat st;
 
-	if (skip_prefix(arg, ":/", &arg)) {
-		if (!*arg) /* ":/" is root dir, always exists */
-			return 1;
-		prefix = NULL;
-	} else if (skip_prefix(arg, ":!", &arg) ||
-		   skip_prefix(arg, ":^", &arg)) {
-		if (!*arg) /* excluding everything is silly, but allowed */
-			return 1;
-	}
+	parse_pathspec(&ps, 0, 0, prefix, args);
+	if (ps.nr < 1)
+		BUG("pathspec ended up with no items?");
+	arg = ps.items[0].match;
 
-	if (prefix)
-		arg = to_free = prefix_filename(prefix, arg);
+	/* empty paths (after parsing magic) are OK */
+	if (!*arg) {
+		clear_pathspec(&ps);
+		return 1;
+	}
 
 	if (!lstat(arg, &st)) {
-		free(to_free);
+		clear_pathspec(&ps);
 		return 1; /* file exists */
 	}
 	if (is_missing_file_error(errno)) {
-		free(to_free);
+		clear_pathspec(&ps);
 		return 0; /* file does not exist */
 	}
 	die_errno(_("failed to stat '%s'"), arg);

It does make :^:Documentation work as you'd expect. Curiously, testing
:/Documentation is hard because that is _also_ a valid rev (the ":/"
searches for a commit with that string in its message). So it will
almost always fail anyway as "hey, this is both a rev and a file". But
you can do ":^/Documentation" to see how it behaves. ;)

But here's the interesting part: it breaks a bunch of tests. They all
seem to be doing things like ":file.txt". In check_filename() right now
we treat that literally. But as a pathspec, it is technically "colon
followed by zero or more magic signature letters", and it is eaten.

So I wonder if we have painted ourselves into a compatibility corner a
bit, if we have two conflicting expectations. We might be better off
just teaching check_filename() to parse multiple of [^/!] and the
trailing colon. It's horrible and not great for maintainability, but
this syntax is not something that changes often.

-Peff