diff --git a/docs/reference/path-tables.md b/docs/reference/path-tables.md index b7be587a0..3a72b9d99 100644 --- a/docs/reference/path-tables.md +++ b/docs/reference/path-tables.md @@ -47,8 +47,13 @@ beneath it; the non-recursive form is spelled explicitly with `*`. | `'./docs/**/*.md'` | markdown files at any depth under `docs/` | | `'./notes/today.md'` | exactly that one file — one file is one row | -A path containing `*`, `?` or `[` is a glob and is used exactly as written: `*` -matches within a single directory, `**` crosses directories. +A path containing `*`, `?`, `[` or `{` is a glob and is used exactly as +written: `*` matches within a single directory, `**` crosses directories. + +The scan starts at the last directory named outright before the first glob +component -- `'./small/*.md'` walks `small/` and nothing else -- so a query +over one directory costs what `find ./small` costs, however large the +directories beside it. A path naming a single file yields exactly one row. dirsql never splits a file into rows on its own — that is what a table's `on_file` hook is for. @@ -186,8 +191,9 @@ bug to design around. The table itself is per-connection: it lives in `temp`, so it cannot leak into `sqlite_master` or survive a restart. Under `--persist` a *parsed* table's rows outlive the connection in the cache (above), but the table is still minted -fresh each run and the scan still decides what exists. The reserved top-level -`.dirsql/` directory is excluded from the scan, as everywhere else. +fresh each run and the scan still decides what exists. A `.dirsql/` directory +at the top of the directory the scan starts in is reserved and excluded, as +everywhere else. ### When to promote to a declared table @@ -234,8 +240,8 @@ defaults and configured `ignore` patterns still apply. ### Naming a skipped directory -Skip rules are judged on the part of the path *below* what you named outright, -so pointing at a skipped directory — built-in or gitignored — still scans it: +Skip rules are judged from the directory the scan starts in, so pointing at a +skipped directory — built-in or gitignored — still scans it: ```sql SELECT path FROM './'; -- no node_modules rows @@ -243,8 +249,8 @@ SELECT path FROM './node_modules/*/package.json'; -- scans it anyway SELECT path FROM './dist'; -- scans dist/ even when gitignored ``` -A `.gitignore` at or below the directory you named still filters beneath it; -only rules inherited from above it are set aside. +A `.gitignore` at or below the directory the scan starts in still filters +beneath it; one above it is never read. ### Hidden files diff --git a/packages/python/e2e-attestations/claude-1199-glob-prefix.json b/packages/python/e2e-attestations/claude-1199-glob-prefix.json new file mode 100644 index 000000000..150c07523 --- /dev/null +++ b/packages/python/e2e-attestations/claude-1199-glob-prefix.json @@ -0,0 +1,7 @@ +{ + "command": "just test-e2e", + "ran_at": 1790974761, + "exit_code": 0, + "commit": "80332ac7891d42541409c3544f1bf1b41644b17c", + "branch": "claude/1199-glob-prefix" +} diff --git a/packages/rust/changelog.d/2026-10-02-glob-prefix-walk.md b/packages/rust/changelog.d/2026-10-02-glob-prefix-walk.md new file mode 100644 index 000000000..6d02a8125 --- /dev/null +++ b/packages/rust/changelog.d/2026-10-02-glob-prefix-walk.md @@ -0,0 +1 @@ +**Fixed** A `./` path-table that names a directory before its first wildcard -- `'./small/*.md'`, `'./docs'`, `'./src/**/*.rs'` -- now starts its walk at that directory instead of at the index root, so it costs what `find ./small` costs rather than what `find .` costs, however large the siblings. `path` is reported exactly as before. Two consequences follow from where the walk starts: a `.dirsql/` directly under the named directory is now the reserved one (`'./.dirsql'` scans it outright, `'./x/.dirsql'` is pruned), and a symlinked directory named outright is now entered, both as absolute tables already behave. A parser (`--on-file`) over any path-table now runs in the index root with `{root}` naming it, which absolute tables previously got wrong. The scanner's `scan_glob` drops its `ignore_base` argument, `path_table::ignore_base` is gone, and the `dirsql_parsed` module takes the index root as an argument; see the migration note. diff --git a/packages/rust/migrations.d/2026-10-02-glob-prefix-walk.md b/packages/rust/migrations.d/2026-10-02-glob-prefix-walk.md new file mode 100644 index 000000000..63baa4f22 --- /dev/null +++ b/packages/rust/migrations.d/2026-10-02-glob-prefix-walk.md @@ -0,0 +1,47 @@ +### Path-tables walk from their literal prefix + +**Summary** + +A `./` path-table is now resolved like an absolute one: the literal directory +chain ahead of the first wildcard becomes the scan root, and the glob is the +remainder. The walker therefore no longer needs to know which part of a glob +was named outright, so `dirsql::scanner::scan_glob` drops its `ignore_base` +parameter and `dirsql::path_table::ignore_base` is removed. Because the scan +root can now sit below the index root, the `dirsql_parsed` module takes the +index root as a sixth fixed argument and spawns the parser there, with +`{root}` naming it. Only Rust callers of those functions and hand-written +`CREATE VIRTUAL TABLE ... USING dirsql_parsed(...)` statements are affected; +the CLI, the Python and TypeScript SDKs, and every query are unchanged in what +they return. + +**Required changes** + +| Surface | Before | After | +|---|---|---| +| `scanner::scan_glob` | `scan_glob(&root, &glob, &ignore, &ignore_base, gitignore)` | `scan_glob(&root, &glob, &ignore, gitignore)` | +| `path_table::ignore_base(glob)` | returned the glob's leading literal directories | removed; `path_table::resolve` now puts them in `PathTable::root` and `PathTable::path_prefix` | +| `dirsql_parsed` module arguments | `(root, glob, parser, gitignore, cache, ignores...)` | `(root, glob, parser, gitignore, cache, index root, ignores...)` | + +**Deprecations removed** + +_None._ + +**Behavior changes without code changes** + +| Surface | Before | After | +|---|---|---| +| `'./small/*.md'` beside a large sibling tree | walked the whole index root | walks `small/` only | +| `'./.dirsql'` | zero rows | scans the reserved directory, as naming it outright does for every other skipped directory | +| `'./x/.dirsql/**'` | scanned | pruned, as the reserved directory is at any scan root | +| a symlinked directory named ahead of the first wildcard | never entered | entered, as for an absolute table | +| `--on-file` over an absolute path-table (`'/var/log/*.log'`) | the parser ran in `/var/log`, `{root}` = `/var/log` | the parser runs in the index root, `{root}` = the index root, as the `on-file` hook documents | + +**Verification** + +```bash +mkdir -p /tmp/t/small /tmp/t/big +touch /tmp/t/small/a.md /tmp/t/big/b.md +cd /tmp/t +dirsql "SELECT path FROM './small/*.md'" +# [{"path":"small/a.md"}] +``` diff --git a/packages/rust/src/db.rs b/packages/rust/src/db.rs index 2d9c2f2cb..44cf3566b 100644 --- a/packages/rust/src/db.rs +++ b/packages/rust/src/db.rs @@ -145,8 +145,9 @@ fn no_home_path_table(name: &str) -> String { /// /// A parsed table also carries `cache` — the persistent cache path when the /// index has one — so it can serve an unchanged file's rows without re-running -/// the parser. The stat module takes no such argument: its columns are the -/// stat tuple the scan already has, so there is nothing to save. +/// the parser, and `index_root`, where the parser is spawned. The stat module +/// takes neither: its columns are the stat tuple the scan already has, so +/// there is nothing to save and no process to place. fn path_table_ddl( name: &str, table: &PathTable, @@ -154,6 +155,7 @@ fn path_table_ddl( gitignore: bool, parser: Option<&str>, cache: Option<&Path>, + index_root: &Path, ) -> String { let (module, mut args) = match parser { Some(command) => ( @@ -184,6 +186,7 @@ fn path_table_ddl( .map(|p| p.to_string_lossy().into_owned()) .unwrap_or_default(), )); + args.push(quote_literal(&index_root.to_string_lossy())); } args.extend(ignore.iter().map(|p| quote_literal(p))); @@ -1027,6 +1030,10 @@ impl Db { /// straight on the connection rather than through [`query`](Self::query), /// which classifies `CREATE VIRTUAL TABLE` as a write and would reject it. fn create_path_table(&self, name: &str, table: &PathTable) -> Result<()> { + let index_root = self + .path_table_root + .as_deref() + .expect("a path-table resolves only under an index root"); self.conn.execute_batch(&path_table_ddl( name, table, @@ -1034,6 +1041,7 @@ impl Db { self.path_table_gitignore, self.path_table_parser.as_deref(), self.path_table_cache.as_deref(), + index_root, ))?; Ok(()) } @@ -2639,7 +2647,15 @@ mod tests { #[test] fn path_table_ddl_creates_in_temp_if_not_exists() { - let ddl = path_table_ddl("./docs/*.md", &docs_path_table(), &[], true, None, None); + let ddl = path_table_ddl( + "./docs/*.md", + &docs_path_table(), + &[], + true, + None, + None, + Path::new("/root"), + ); assert_eq!( ddl, "CREATE VIRTUAL TABLE IF NOT EXISTS temp.\"./docs/*.md\" \ @@ -2655,16 +2671,40 @@ mod tests { path_prefix: "/var/log".to_string(), }; assert!( - path_table_ddl("/var/log/*.log", &table, &[], true, None, None) - .contains("'/var/log', '*.log', '/var/log', 'gitignore')"), + path_table_ddl( + "/var/log/*.log", + &table, + &[], + true, + None, + None, + Path::new("/root"), + ) + .contains("'/var/log', '*.log', '/var/log', 'gitignore')"), "got: {}", - path_table_ddl("/var/log/*.log", &table, &[], true, None, None) + path_table_ddl( + "/var/log/*.log", + &table, + &[], + true, + None, + None, + Path::new("/root"), + ) ); } #[test] fn path_table_ddl_carries_the_no_gitignore_switch() { - let ddl = path_table_ddl("./", &docs_path_table(), &[], false, None, None); + let ddl = path_table_ddl( + "./", + &docs_path_table(), + &[], + false, + None, + None, + Path::new("/root"), + ); assert!( ddl.ends_with("'', 'no-gitignore')"), "gitignore off must emit the no-gitignore switch, got: {ddl}" @@ -2680,6 +2720,7 @@ mod tests { true, None, None, + Path::new("/root"), ); assert!( ddl.ends_with("'', 'gitignore', 'node_modules/**', '*.tmp')"), @@ -2696,11 +2737,12 @@ mod tests { true, Some("cat {path}"), None, + Path::new("/root"), ); assert_eq!( ddl, "CREATE VIRTUAL TABLE IF NOT EXISTS temp.\"./docs/*.md\" \ - USING dirsql_parsed('/root', 'docs/*.md', 'cat {path}', 'gitignore', '')" + USING dirsql_parsed('/root', 'docs/*.md', 'cat {path}', 'gitignore', '', '/root')" ); } @@ -2713,9 +2755,10 @@ mod tests { true, Some("cat {path}"), Some(Path::new("/cache/dirsql.db")), + Path::new("/root"), ); assert!( - ddl.ends_with("'cat {path}', 'gitignore', '/cache/dirsql.db')"), + ddl.ends_with("'cat {path}', 'gitignore', '/cache/dirsql.db', '/root')"), "a persisted index points the parsed module at its cache, got: {ddl}" ); } @@ -2729,6 +2772,7 @@ mod tests { true, None, Some(Path::new("/cache/dirsql.db")), + Path::new("/root"), ); assert!( ddl.ends_with("'', 'gitignore')"), @@ -2750,13 +2794,15 @@ mod tests { true, Some("parse.py {path}"), None, + Path::new("/index"), ); assert!( ddl.ends_with( "USING dirsql_parsed('/var/log', '*.log', 'parse.py {path}', 'gitignore', \ - '', 'node_modules/**')" + '', '/index', 'node_modules/**')" ), - "the parser form drops the path prefix and keeps ignore rules, got: {ddl}" + "the parser form drops the path prefix, names the index root and keeps \ + ignore rules, got: {ddl}" ); } @@ -2769,6 +2815,7 @@ mod tests { true, Some("sh -c 'echo hi'"), None, + Path::new("/root"), ); assert!( ddl.contains("'sh -c ''echo hi'''"), diff --git a/packages/rust/src/parsed_vtab.rs b/packages/rust/src/parsed_vtab.rs index abd089d0a..367951579 100644 --- a/packages/rust/src/parsed_vtab.rs +++ b/packages/rust/src/parsed_vtab.rs @@ -34,7 +34,6 @@ use crate::infer::{JsonRow, cell, declared_schema, infer_schema, parse_rows}; use crate::matcher::TableMatcher; use crate::on_file; use crate::parsed_cache::{self, CachedParse, Entry, RowCache, SqliteRowCache}; -use crate::path_table; use crate::persist::{FileStat, hash_file, now_ns}; use crate::scanner::{scan_glob, to_slash}; use crate::vtab_scaffold::{self, StatementScope, TableSource}; @@ -48,12 +47,12 @@ pub fn load_module(conn: &Connection, scope: Arc) -> Result<()> } /// Number of module arguments that are not ignore patterns. -const FIXED_ARGS: usize = 5; +const FIXED_ARGS: usize = 6; -/// The module's own arguments: root, glob pattern, parser command, the +/// The module's own arguments: scan root, glob pattern, parser command, the /// gitignore switch, the persistent cache path (empty when the index is -/// ephemeral), and the skip rules the scan applies (mirroring the stat -/// path-table's ignore args). +/// ephemeral), the index root the parser is spawned in, and the skip rules +/// the scan applies (mirroring the stat path-table's ignore args). struct ModuleArgs { root: PathBuf, pattern: String, @@ -61,21 +60,31 @@ struct ModuleArgs { command: String, gitignore: bool, cache: Option, + index_root: PathBuf, ignore: TableMatcher, } /// Parse a parsed path-table's `CREATE VIRTUAL TABLE` arguments. `args[0..3]` /// are the module, database and table names; the module's own follow — root, -/// glob, parser, the gitignore switch, the cache path, then any ignore -/// patterns. +/// glob, parser, the gitignore switch, the cache path, the index root, then +/// any ignore patterns. fn parse_module_args(args: &[&[u8]]) -> Result { let user_args = vtab_scaffold::user_args(args); - let [root, pattern, command, gitignore, cache, ignore @ ..] = user_args.as_slice() else { + let [ + root, + pattern, + command, + gitignore, + cache, + index_root, + ignore @ .., + ] = user_args.as_slice() + else { return Err(vtab_scaffold::arity_error( MODULE_NAME, FIXED_ARGS, - "root, glob, parser, gitignore switch, cache path", + "root, glob, parser, gitignore switch, cache path, index root", user_args.len(), )); }; @@ -89,6 +98,7 @@ fn parse_module_args(args: &[&[u8]]) -> Result { // The empty string is how "no cache" is spelled: a module argument // cannot be absent without shifting every argument after it. cache: Some(PathBuf::from(cache)).filter(|p| !p.as_os_str().is_empty()), + index_root: PathBuf::from(index_root), ignore: vtab_scaffold::compile_ignore(ignore)?, }) } @@ -249,14 +259,16 @@ fn no_rows_message(pattern: &str) -> String { } /// Run the parser once over `rel_paths`, handing it their absolute paths as -/// trailing arguments, from the scan root as its working directory. +/// trailing arguments, from the index root as its working directory, with +/// `{root}` naming it, matching the `on-file` contract. fn run_parser( command: &str, + index_root: &Path, root: &Path, rel_paths: &[PathBuf], ) -> std::result::Result, String> { let abs_paths: Vec = rel_paths.iter().map(|rel| root.join(rel)).collect(); - on_file::run(command, root, root, &abs_paths) + on_file::run(command, index_root, index_root, &abs_paths) } /// Column name for an index, or `None` when SQLite asks for one out of range. @@ -287,15 +299,15 @@ impl TableSource for ParsedTable { command, gitignore, cache, + index_root, ignore, } = parse_module_args(args)?; // A parsed path-table honors the same skip rules a stat path-table does // (node_modules/.git, gitignore, plus any configured ignore), so a // parsed `SELECT * FROM './'` doesn't drown in dependency trees. - let ignore_base = path_table::ignore_base(&pattern); - let rel_paths = scan_glob(&root, &glob, &ignore, &ignore_base, gitignore); - let run = |rel: &[PathBuf]| run_parser(&command, &root, rel); + let rel_paths = scan_glob(&root, &glob, &ignore, gitignore); + let run = |rel: &[PathBuf]| run_parser(&command, &index_root, &root, rel); let warn = |message: &str| eprintln!("{message}"); let rows = match &cache { None => collect_rows(&rel_paths, &run)?, @@ -358,26 +370,28 @@ mod tests { } #[test] - fn parse_module_args_extracts_root_glob_and_command() { + fn parse_module_args_extracts_root_glob_command_and_index_root() { let args = args_with(&[ - b"'/tmp/notes'", + b"'/tmp/notes/docs'", b"'**/*.json'", b"'cat {path}'", b"'gitignore'", b"''", + b"'/tmp/notes'", ]); let parsed = parse_module_args(&args).unwrap(); - assert_eq!(parsed.root, PathBuf::from("/tmp/notes")); + assert_eq!(parsed.root, PathBuf::from("/tmp/notes/docs")); assert_eq!(parsed.pattern, "**/*.json"); assert_eq!(parsed.command, "cat {path}"); + assert_eq!(parsed.index_root, PathBuf::from("/tmp/notes")); assert!(parsed.glob.is_match(Path::new("a.json"))); assert!(!parsed.glob.is_match(Path::new("a.md"))); } #[test] fn parse_module_args_accepts_unquoted_arguments() { - let args = args_with(&[b"/tmp/notes", b"**/*", b"cat", b"gitignore", b""]); + let args = args_with(&[b"/tmp/notes", b"**/*", b"cat", b"gitignore", b"", b"/tmp"]); assert_eq!( parse_module_args(&args).unwrap().root, PathBuf::from("/tmp/notes") @@ -386,16 +400,37 @@ mod tests { #[test] fn parse_module_args_reads_the_gitignore_switch() { - let on = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'gitignore'", b"''"]); + let on = args_with(&[ + b"'/tmp'", + b"'**/*'", + b"'cat'", + b"'gitignore'", + b"''", + b"'/tmp'", + ]); assert!(parse_module_args(&on).unwrap().gitignore); - let off = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'no-gitignore'", b"''"]); + let off = args_with(&[ + b"'/tmp'", + b"'**/*'", + b"'cat'", + b"'no-gitignore'", + b"''", + b"'/tmp'", + ]); assert!(!parse_module_args(&off).unwrap().gitignore); } #[test] fn parse_module_args_rejects_an_unknown_gitignore_switch() { - let args = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'sometimes'", b"''"]); + let args = args_with(&[ + b"'/tmp'", + b"'**/*'", + b"'cat'", + b"'sometimes'", + b"''", + b"'/tmp'", + ]); let err = match parse_module_args(&args) { Err(err) => err, Ok(_) => panic!("an unknown switch must be rejected"), @@ -411,6 +446,7 @@ mod tests { b"'cat'", b"'gitignore'", b"''", + b"'/tmp'", b"'node_modules/**'", ]); let parsed = parse_module_args(&args).unwrap(); @@ -420,17 +456,24 @@ mod tests { #[test] fn parse_module_args_with_no_ignore_patterns_ignores_nothing() { - let args = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'gitignore'", b"''"]); + let args = args_with(&[ + b"'/tmp'", + b"'**/*'", + b"'cat'", + b"'gitignore'", + b"''", + b"'/tmp'", + ]); let parsed = parse_module_args(&args).unwrap(); assert!(!parsed.ignore.is_ignored(Path::new("node_modules/pkg/a.js"))); } #[test] fn parse_module_args_rejects_too_few_arguments() { - let args = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'gitignore'"]); + let args = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'gitignore'", b"''"]); let err = match parse_module_args(&args) { Err(err) => err, - Ok(_) => panic!("four arguments must be rejected"), + Ok(_) => panic!("five arguments must be rejected"), }; assert!( err.to_string().contains("at least"), @@ -440,7 +483,14 @@ mod tests { #[test] fn parse_module_args_rejects_an_invalid_glob() { - let args = args_with(&[b"'/tmp'", b"'['", b"'cat'", b"'gitignore'", b"''"]); + let args = args_with(&[ + b"'/tmp'", + b"'['", + b"'cat'", + b"'gitignore'", + b"''", + b"'/tmp'", + ]); assert!(parse_module_args(&args).is_err()); } @@ -452,6 +502,7 @@ mod tests { b"'cat'", b"'gitignore'", b"'/cache/dirsql.db'", + b"'/tmp'", ]); assert_eq!( parse_module_args(&args).unwrap().cache, @@ -461,7 +512,14 @@ mod tests { #[test] fn parse_module_args_reads_an_empty_cache_path_as_no_cache() { - let args = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'gitignore'", b"''"]); + let args = args_with(&[ + b"'/tmp'", + b"'**/*'", + b"'cat'", + b"'gitignore'", + b"''", + b"'/tmp'", + ]); assert_eq!(parse_module_args(&args).unwrap().cache, None); } @@ -473,6 +531,7 @@ mod tests { b"'cat'", b"'gitignore'", b"''", + b"'/tmp'", b"'['", ]); assert!(parse_module_args(&args).is_err()); @@ -558,6 +617,7 @@ mod tests { let rows = run_parser( r#"sh -c "echo chatter; printf '[{\"id\":1}]'""#, Path::new("."), + Path::new("."), &[PathBuf::from("a.json")], ) .unwrap(); @@ -568,32 +628,41 @@ mod tests { /// order. The command writes them to a side file rather than into the /// JSON payload, where a Windows path's `\` would need escaping. #[test] - fn run_parser_appends_every_absolute_path_after_the_root() { - let root = std::env::temp_dir().join(format!("dirsql-run-parser-{}", std::process::id())); + fn run_parser_appends_every_absolute_path_after_the_index_root() { + let index_root = + std::env::temp_dir().join(format!("dirsql-run-parser-{}", std::process::id())); + let root = index_root.join("docs"); std::fs::create_dir_all(&root).unwrap(); run_parser( r#"sh -c 'printf "%s\n%s\n%s\n" "$1" "$2" "$3" > seen; echo "[]"' sh {root}"#, + &index_root, &root, &[PathBuf::from("a.json"), PathBuf::from("b.json")], ) .unwrap(); - let seen = std::fs::read_to_string(root.join("seen")).unwrap(); - std::fs::remove_dir_all(&root).unwrap(); + let seen = std::fs::read_to_string(index_root.join("seen")).unwrap(); + std::fs::remove_dir_all(&index_root).unwrap(); assert_eq!( seen, format!( "{}\n{}\n{}\n", - root.display(), + index_root.display(), root.join("a.json").display(), root.join("b.json").display() - ) + ), + "the side file lands in the index root, which is the working directory" ); } #[test] fn run_parser_surfaces_a_command_failure_as_a_message() { - let err = - run_parser("sh -c 'exit 7'", Path::new("."), &[PathBuf::from("a.json")]).unwrap_err(); + let err = run_parser( + "sh -c 'exit 7'", + Path::new("."), + Path::new("."), + &[PathBuf::from("a.json")], + ) + .unwrap_err(); assert!(err.contains('7'), "the exit code is reported, got: {err}"); } diff --git a/packages/rust/src/path_table.rs b/packages/rust/src/path_table.rs index 46a20863e..7bd7bae9b 100644 --- a/packages/rust/src/path_table.rs +++ b/packages/rust/src/path_table.rs @@ -31,8 +31,8 @@ pub struct PathTable { /// Glob matched against paths relative to [`root`](Self::root). pub glob: String, /// Prepended to each matched relative path before the stat columns are - /// computed. Empty for index-root-relative tables, which report relative - /// paths; the scan root for the rest, which report absolute ones. + /// computed: the directories a `./` table named ahead of its glob, so its + /// paths read as index-root-relative; the absolute scan root for the rest. pub path_prefix: String, } @@ -96,7 +96,7 @@ impl Syntax for Utf8WindowsEncoding { /// Whether `name` contains a character that makes it a glob rather than a /// literal path. fn has_glob_metacharacter(name: &str) -> bool { - name.contains(['*', '?', '[']) + name.contains(['*', '?', '[', '{']) } /// Resolve `name` against the index root, reporting what kind of thing it is. @@ -119,11 +119,7 @@ fn resolve_as( is_dir: &dyn Fn(&Path) -> bool, ) -> Resolution { if let Some(rest) = name.strip_prefix("./") { - return Resolution::Table(PathTable { - root: index_root.to_path_buf(), - glob: relative_glob(rest, &|rel| is_dir(&index_root.join(rel))), - path_prefix: String::new(), - }); + return Resolution::Table(split_relative(index_root, rest, is_dir)); } match absolute_target::(name, index_root, home) { @@ -134,20 +130,23 @@ fn resolve_as( } } -/// The glob a `./`-relative path expands to, relative to the index root. -fn relative_glob(rest: &str, is_dir: &dyn Fn(&Path) -> bool) -> String { - let trimmed = rest.trim_end_matches('/'); - - if trimmed.is_empty() { - return RECURSIVE_GLOB.to_string(); - } - if has_glob_metacharacter(trimmed) { - return trimmed.to_string(); - } - if is_dir(Path::new(trimmed)) { - return format!("{trimmed}/{RECURSIVE_GLOB}"); +/// Split a `./`-relative target into the directory to walk beneath the index +/// root and the glob to match there. The walk starts at the literal prefix +/// and `path` is reported under it, so the rows read as if the index root +/// had been walked whole. +fn split_relative(index_root: &Path, rest: &str, is_dir: &dyn Fn(&Path) -> bool) -> PathTable { + let target = Utf8Path::::new(rest.trim_end_matches('/')); + let (literal, glob) = split_target(target, &|rel| is_dir(&index_root.join(rel))); + let root = if literal.as_str().is_empty() { + index_root.to_path_buf() + } else { + index_root.join(literal.as_str()) + }; + PathTable { + root, + glob, + path_prefix: literal.into_string(), } - trimmed.to_string() } fn typed(path: &Path) -> Utf8PathBuf { @@ -214,24 +213,34 @@ fn normalize(path: &Utf8Path) -> Utf8PathBuf { } /// Split an absolute path-table target into the directory to walk and the glob -/// to match beneath it. A wholly literal target is a directory (scan it -/// recursively) or a single file (match exactly that name). +/// to match beneath it. fn split_absolute(target: &Utf8Path, is_dir: &dyn Fn(&Path) -> bool) -> PathTable { + let (literal, glob) = split_target(target, is_dir); + table_at(&literal, glob) +} + +/// The directory to walk and the glob to match beneath it. A wholly literal +/// target is a directory (scan it recursively) or a single file (match exactly +/// that name beneath its parent). +fn split_target( + target: &Utf8Path, + is_dir: &dyn Fn(&Path) -> bool, +) -> (Utf8PathBuf, String) { let (literal, rest) = split_at_first_glob(target); if !rest.is_empty() { - return table_at(&literal, rest); + return (literal, rest); } if is_dir(Path::new(literal.as_str())) { - return table_at(&literal, RECURSIVE_GLOB.to_string()); + return (literal, RECURSIVE_GLOB.to_string()); } let name = literal .file_name() .map(str::to_string) .unwrap_or_else(|| RECURSIVE_GLOB.to_string()); - let parent = literal.parent().unwrap_or(&literal); - table_at(parent, name) + let parent = literal.parent().unwrap_or(&literal).to_path_buf(); + (parent, name) } /// A path-table rooted at `root`, reporting absolute paths. @@ -262,22 +271,6 @@ fn split_at_first_glob(target: &Utf8Path) -> (Utf8PathBuf, Stri (literal, rest.join("/")) } -/// The leading literal directories of `glob` — the part the user named -/// outright. Skip rules are evaluated below this, so writing -/// `'./node_modules'` scans a directory the defaults would otherwise skip. -pub fn ignore_base(glob: &str) -> PathBuf { - let mut base = PathBuf::new(); - for segment in glob.split('/') { - if has_glob_metacharacter(segment) { - break; - } - base.push(segment); - } - // A wholly literal glob names one file; its own directory chain is the - // named part, so nothing is left to check. - base -} - #[cfg(test)] mod tests { use super::*; @@ -313,6 +306,7 @@ mod tests { assert!(has_glob_metacharacter("a*")); assert!(has_glob_metacharacter("a?")); assert!(has_glob_metacharacter("a[bc]")); + assert!(has_glob_metacharacter("{a,b}")); } #[test] @@ -330,22 +324,59 @@ mod tests { #[test] fn a_relative_directory_expands_recursively() { - assert_eq!(table("./docs", &everything_is_a_dir).glob, "docs/**/*"); + let t = table("./docs", &everything_is_a_dir); + assert_eq!(t.root, Path::new("/index/docs")); + assert_eq!(t.glob, "**/*"); + assert_eq!(t.path_prefix, "docs"); } #[test] fn a_relative_directory_with_a_trailing_slash_expands_recursively() { - assert_eq!(table("./docs/", &everything_is_a_dir).glob, "docs/**/*"); + let t = table("./docs/", &everything_is_a_dir); + assert_eq!(t.root, Path::new("/index/docs")); + assert_eq!(t.glob, "**/*"); } #[test] - fn a_relative_single_file_matches_only_itself() { - assert_eq!(table("./docs/a.md", ¬hing_is_a_dir).glob, "docs/a.md"); + fn a_relative_single_file_roots_at_its_parent() { + let t = table("./docs/a.md", ¬hing_is_a_dir); + assert_eq!(t.root, Path::new("/index/docs")); + assert_eq!(t.glob, "a.md"); + assert_eq!(t.path_prefix, "docs"); } #[test] - fn a_relative_glob_is_used_as_written() { - assert_eq!(table("./docs/*.md", &everything_is_a_dir).glob, "docs/*.md"); + fn a_relative_top_level_file_roots_at_the_index_root() { + let t = table("./a.md", ¬hing_is_a_dir); + assert_eq!(t.root, Path::new(ROOT)); + assert_eq!(t.glob, "a.md"); + assert_eq!(t.path_prefix, ""); + } + + #[test] + fn a_relative_glob_keeps_every_component_after_the_first() { + let t = table("./src/*/tests/*.rs", ¬hing_is_a_dir); + assert_eq!(t.root, Path::new("/index/src")); + assert_eq!(t.glob, "*/tests/*.rs"); + assert_eq!(t.path_prefix, "src"); + } + + #[test] + fn a_relative_alternation_is_a_glob_not_a_directory() { + let t = table("./{docs,notes}/*.md", ¬hing_is_a_dir); + assert_eq!(t.root, Path::new(ROOT)); + assert_eq!(t.glob, "{docs,notes}/*.md"); + } + + #[test] + fn the_directory_question_is_asked_of_the_absolute_path() { + let asked = std::cell::RefCell::new(Vec::new()); + let is_dir = |p: &Path| { + asked.borrow_mut().push(p.to_path_buf()); + true + }; + table("./docs/nested", &is_dir); + assert_eq!(asked.into_inner(), [PathBuf::from("/index/docs/nested")]); } #[test] @@ -354,8 +385,19 @@ mod tests { } #[test] - fn a_relative_table_reports_relative_paths() { - assert_eq!(table("./docs/*.md", ¬hing_is_a_dir).path_prefix, ""); + fn a_relative_table_reports_under_its_literal_prefix() { + assert_eq!( + table("./docs/nested/*.md", ¬hing_is_a_dir).path_prefix, + "docs/nested" + ); + } + + #[test] + fn a_relative_glob_roots_at_its_literal_prefix() { + let t = table("./docs/*.md", &everything_is_a_dir); + assert_eq!(t.root, Path::new("/index/docs")); + assert_eq!(t.glob, "*.md"); + assert_eq!(t.path_prefix, "docs"); } #[test] @@ -491,24 +533,6 @@ mod tests { .to_string() } - #[test] - fn ignore_base_is_empty_for_a_leading_glob() { - assert_eq!(ignore_base("**/*"), PathBuf::new()); - } - - #[test] - fn ignore_base_is_the_literal_prefix_of_a_glob() { - assert_eq!( - ignore_base("node_modules/**/*"), - PathBuf::from("node_modules") - ); - } - - #[test] - fn ignore_base_is_the_whole_of_a_literal_glob() { - assert_eq!(ignore_base("docs/a.md"), PathBuf::from("docs/a.md")); - } - mod windows { use super::*; diff --git a/packages/rust/src/scanner.rs b/packages/rust/src/scanner.rs index 436e126d6..002f3d8da 100644 --- a/packages/rust/src/scanner.rs +++ b/packages/rust/src/scanner.rs @@ -74,27 +74,20 @@ fn scan_below( // Match against relative path so globs like "comments/**/*.jsonl" work // regardless of the absolute root directory. - walk( - root, - start, - matcher, - Path::new(""), - false, - &mut |rel_path, entry| { - if matcher.is_ignored(&rel_path) { - return; - } + walk(root, start, matcher, false, &mut |rel_path, entry| { + if matcher.is_ignored(&rel_path) { + return; + } - seen += 1; - on_file(seen); + seen += 1; + on_file(seen); - // Fan-out: a file matching N tables' globs yields N (path, table) - // pairs, one per matching table, in declaration order. - for m in matcher.match_all(&rel_path) { - results.push((entry.path(), m.table_name)); - } - }, - ); + // Fan-out: a file matching N tables' globs yields N (path, table) + // pairs, one per matching table, in declaration order. + for m in matcher.match_all(&rel_path) { + results.push((entry.path(), m.table_name)); + } + }); results } @@ -107,42 +100,32 @@ fn scan_below( /// the walker, and with it the reserved-directory rule, and the same /// [`TableMatcher`] ignore handling declared tables get. /// -/// Skip rules are evaluated against the path *below* `ignore_base` — the -/// literal directories the pattern named outright — so a path that reaches -/// into an ignored directory on purpose still scans it. +/// Skip rules are judged on paths relative to `root`, so a table rooted at a +/// directory the rules would otherwise skip still scans it. /// /// With `gitignore` set, `.gitignore` files apply hierarchically (each one /// below its own directory) and prune traversal, like fd/ripgrep — except /// that hidden files are still scanned, and no `.git` directory is required. -/// Rules from `.gitignore` files *above* `ignore_base` are exempt beneath it, -/// mirroring the skip-rule exemption. +/// Only files at or below `root` are read; one above it has no say. pub fn scan_glob( root: &Path, glob: &GlobSet, ignore: &TableMatcher, - ignore_base: &Path, gitignore: bool, ) -> Vec { let mut results = Vec::new(); - walk( - root, - root, - ignore, - ignore_base, - gitignore, - &mut |rel_path, _| { - if is_wanted(glob, ignore, ignore_base, &rel_path) { - results.push(rel_path); - } - }, - ); + walk(root, root, ignore, gitignore, &mut |rel_path, _| { + if is_wanted(glob, ignore, &rel_path) { + results.push(rel_path); + } + }); results } /// Whether a walked file is a row of the table: it matches the glob and no -/// skip rule ignores it below the base. -fn is_wanted(glob: &GlobSet, ignore: &TableMatcher, ignore_base: &Path, rel_path: &Path) -> bool { - is_glob_match(glob, rel_path) && !is_ignored_below(ignore, ignore_base, rel_path) +/// skip rule ignores it. +fn is_wanted(glob: &GlobSet, ignore: &TableMatcher, rel_path: &Path) -> bool { + is_glob_match(glob, rel_path) && !ignore.is_ignored(rel_path) } /// Compile a single glob pattern into the set [`scan_glob`] expects. @@ -184,7 +167,6 @@ fn walk( root: &Path, start: &Path, ignore: &TableMatcher, - ignore_base: &Path, gitignore: bool, visit: &mut dyn FnMut(PathBuf, &DirEntry), ) { @@ -194,7 +176,6 @@ fn walk( let depth = rel.components().count(); let mut walk = Walk { ignore, - ignore_base, gitignore, frames: Vec::new(), }; @@ -203,11 +184,10 @@ fn walk( struct Walk<'a> { ignore: &'a TableMatcher, - ignore_base: &'a Path, gitignore: bool, /// The `.gitignore` files in force at the walk's current position, root /// first; a directory's own file is pushed on entry and popped on exit. - frames: Vec, + frames: Vec, } impl Walk<'_> { @@ -222,10 +202,7 @@ impl Walk<'_> { if self.gitignore && let Some(matcher) = load_gitignore(dir) { - self.frames.push(GitignoreFrame { - dir: rel.to_path_buf(), - matcher, - }); + self.frames.push(matcher); pushed = true; } let below = depth + 1; @@ -251,9 +228,8 @@ impl Walk<'_> { /// Whether the walk takes an entry at `depth`: the skip rules and the /// reserved-directory rule first, then the `.gitignore` files in force. fn admits(&self, depth: usize, is_dir: bool, name: &OsStr, path: &Path, rel: &Path) -> bool { - should_descend(depth, is_dir, name, rel, self.ignore, self.ignore_base) - && (self.frames.is_empty() - || !is_gitignored(&self.frames, path, rel, is_dir, self.ignore_base)) + should_descend(depth, is_dir, name, rel, self.ignore) + && (self.frames.is_empty() || !is_gitignored(&self.frames, path, is_dir)) } } @@ -269,13 +245,6 @@ fn sorted_entries(dir: &Path) -> Vec<(OsString, DirEntry)> { entries } -/// One `.gitignore` in force over the walk: its compiled matcher and the -/// root-relative directory it sits in. -struct GitignoreFrame { - dir: PathBuf, - matcher: Gitignore, -} - /// Compile the `.gitignore` in `dir`, if one exists. An unparsable file /// degrades to no matcher rather than failing the scan. fn load_gitignore(dir: &Path) -> Option { @@ -290,24 +259,9 @@ fn load_gitignore(dir: &Path) -> Option { /// Whether the `.gitignore` files in force mark `path` ignored. Deeper files /// take precedence (git's rule), and a whitelisting `!pattern` un-ignores. -/// The literal chain the scan's pattern named outright is exempt, as are -/// rules from files *above* [`ignore_base`] for entries beneath it — naming a -/// gitignored directory on purpose still scans it. -fn is_gitignored( - frames: &[GitignoreFrame], - path: &Path, - rel_path: &Path, - is_dir: bool, - ignore_base: &Path, -) -> bool { - if ignore_base.starts_with(rel_path) { - return false; - } +fn is_gitignored(frames: &[Gitignore], path: &Path, is_dir: bool) -> bool { for frame in frames.iter().rev() { - if !frame_applies(&frame.dir, rel_path, ignore_base) { - continue; - } - match frame.matcher.matched(path, is_dir) { + match frame.matched(path, is_dir) { Match::Ignore(_) => return true, Match::Whitelist(_) => return false, Match::None => {} @@ -316,37 +270,20 @@ fn is_gitignored( false } -/// Whether a `.gitignore` living at `frame_dir` gets a say over `rel_path`. -/// False exactly when the entry sits inside `ignore_base` and the file sits -/// strictly above it: the base was named outright, so ancestors' rules do not -/// reach past it, while a `.gitignore` at or below the base still applies. -fn frame_applies(frame_dir: &Path, rel_path: &Path, ignore_base: &Path) -> bool { - let entry_inside_base = - !ignore_base.as_os_str().is_empty() && rel_path.starts_with(ignore_base); - let frame_above_base = frame_dir != ignore_base && ignore_base.starts_with(frame_dir); - !(entry_inside_base && frame_above_base) -} - /// Whether the walk keeps `rel_path`. False prunes the reserved top-level /// `.dirsql/` directory and any directory whose whole subtree the skip rules -/// ignore — unless the literal base the pattern named runs through it, which -/// keeps a scan pointed *into* an ignored directory working. +/// ignore. fn should_descend( depth: usize, is_dir: bool, file_name: &std::ffi::OsStr, rel_path: &Path, ignore: &TableMatcher, - ignore_base: &Path, ) -> bool { if is_reserved_dir(depth, is_dir, file_name) { return false; } - if !is_dir || ignore_base.starts_with(rel_path) { - return true; - } - let below = rel_path.strip_prefix(ignore_base).unwrap_or(rel_path); - !ignore.is_ignored_dir(below) + !is_dir || !ignore.is_ignored_dir(rel_path) } /// Whether `rel_path` matches `glob`. @@ -354,12 +291,6 @@ fn is_glob_match(glob: &GlobSet, rel_path: &Path) -> bool { glob.is_match(rel_path) } -/// Whether `rel_path` is ignored, judged on the part of it beneath `base`. -fn is_ignored_below(ignore: &TableMatcher, base: &Path, rel_path: &Path) -> bool { - let below = rel_path.strip_prefix(base).unwrap_or(rel_path); - ignore.is_ignored(below) -} - /// True for the reserved top-level `.dirsql/` directory (`depth == 1`), which /// the scan unconditionally excludes. fn is_reserved_dir(depth: usize, is_dir: bool, file_name: &std::ffi::OsStr) -> bool { @@ -450,46 +381,6 @@ mod tests { assert!(is_glob_match(&set, Path::new("docs/deep/a.md"))); } - #[test] - fn is_ignored_below_matches_an_ignored_path_at_the_top() { - let ignore = TableMatcher::new(&[], &["node_modules/**"]).unwrap(); - assert!(is_ignored_below( - &ignore, - Path::new(""), - Path::new("node_modules/pkg/index.js") - )); - } - - #[test] - fn is_ignored_below_exempts_the_base_the_pattern_named() { - let ignore = TableMatcher::new(&[], &["node_modules/**"]).unwrap(); - assert!(!is_ignored_below( - &ignore, - Path::new("node_modules"), - Path::new("node_modules/pkg/index.js") - )); - } - - #[test] - fn is_ignored_below_judges_the_whole_path_when_the_base_does_not_apply() { - let ignore = TableMatcher::new(&[], &["other/**"]).unwrap(); - assert!(is_ignored_below( - &ignore, - Path::new("docs"), - Path::new("other/a.tmp") - )); - } - - #[test] - fn is_ignored_below_passes_an_unignored_path() { - let ignore = TableMatcher::new(&[], &["node_modules/**"]).unwrap(); - assert!(!is_ignored_below( - &ignore, - Path::new(""), - Path::new("docs/a.md") - )); - } - #[test] fn should_descend_prunes_a_directory_the_skip_rules_fully_ignore() { let ignore = TableMatcher::new(&[], &["**/node_modules/**"]).unwrap(); @@ -498,8 +389,7 @@ mod tests { true, OsStr::new("node_modules"), Path::new("apps/node_modules"), - &ignore, - Path::new("") + &ignore )); } @@ -511,8 +401,7 @@ mod tests { true, OsStr::new("docs"), Path::new("docs"), - &ignore, - Path::new("") + &ignore )); } @@ -524,47 +413,7 @@ mod tests { false, OsStr::new("node_modules"), Path::new("apps/node_modules"), - &ignore, - Path::new("") - )); - } - - #[test] - fn should_descend_keeps_the_directory_the_base_names() { - let ignore = TableMatcher::new(&[], &["**/node_modules/**"]).unwrap(); - assert!(should_descend( - 1, - true, - OsStr::new("node_modules"), - Path::new("node_modules"), - &ignore, - Path::new("node_modules") - )); - } - - #[test] - fn should_descend_keeps_an_ancestor_of_the_named_base() { - let ignore = TableMatcher::new(&[], &["**/node_modules/**"]).unwrap(); - assert!(should_descend( - 2, - true, - OsStr::new("node_modules"), - Path::new("apps/node_modules"), - &ignore, - Path::new("apps/node_modules/pkg") - )); - } - - #[test] - fn should_descend_judges_the_part_below_the_named_base() { - let ignore = TableMatcher::new(&[], &["**/node_modules/**"]).unwrap(); - assert!(!should_descend( - 3, - true, - OsStr::new("node_modules"), - Path::new("apps/pkg/node_modules"), - &ignore, - Path::new("apps") + &ignore )); } @@ -576,8 +425,7 @@ mod tests { true, OsStr::new(RESERVED_DIR), Path::new(RESERVED_DIR), - &ignore, - Path::new("") + &ignore )); } @@ -596,51 +444,32 @@ mod tests { } /// A gitignore frame compiled from in-memory lines; no filesystem. - fn frame(dir: &str, lines: &[&str]) -> GitignoreFrame { + fn frame(dir: &str, lines: &[&str]) -> Gitignore { let mut builder = GitignoreBuilder::new(Path::new(dir)); for line in lines { builder.add_line(None, line).unwrap(); } - GitignoreFrame { - dir: PathBuf::from(dir), - matcher: builder.build().unwrap(), - } + builder.build().unwrap() } #[test] fn is_wanted_takes_a_glob_match_no_skip_rule_ignores() { let glob = compile_glob("**/*.md").unwrap(); let ignore = TableMatcher::new(&[], &["drafts/**"]).unwrap(); - assert!(is_wanted( - &glob, - &ignore, - Path::new(""), - Path::new("docs/a.md") - )); + assert!(is_wanted(&glob, &ignore, Path::new("docs/a.md"))); } #[test] fn is_wanted_rejects_a_glob_miss_and_an_ignored_match() { let glob = compile_glob("**/*.md").unwrap(); let ignore = TableMatcher::new(&[], &["drafts/**"]).unwrap(); - assert!(!is_wanted( - &glob, - &ignore, - Path::new(""), - Path::new("docs/a.csv") - )); - assert!(!is_wanted( - &glob, - &ignore, - Path::new(""), - Path::new("drafts/a.md") - )); + assert!(!is_wanted(&glob, &ignore, Path::new("docs/a.csv"))); + assert!(!is_wanted(&glob, &ignore, Path::new("drafts/a.md"))); } - fn walk_with<'a>(ignore: &'a TableMatcher, frames: Vec) -> Walk<'a> { + fn walk_with<'a>(ignore: &'a TableMatcher, frames: Vec) -> Walk<'a> { Walk { ignore, - ignore_base: Path::new(""), gitignore: !frames.is_empty(), frames, } @@ -695,124 +524,25 @@ mod tests { #[test] fn is_gitignored_matches_a_rule_from_the_root_gitignore() { let frames = [frame("", &["*.log"])]; - assert!(is_gitignored( - &frames, - Path::new("debug.log"), - Path::new("debug.log"), - false, - Path::new("") - )); - assert!(!is_gitignored( - &frames, - Path::new("app.js"), - Path::new("app.js"), - false, - Path::new("") - )); + assert!(is_gitignored(&frames, Path::new("debug.log"), false)); + assert!(!is_gitignored(&frames, Path::new("app.js"), false)); } #[test] fn is_gitignored_marks_a_directory_rule_for_pruning() { let frames = [frame("", &["dist/"])]; - assert!(is_gitignored( - &frames, - Path::new("dist"), - Path::new("dist"), - true, - Path::new("") - )); + assert!(is_gitignored(&frames, Path::new("dist"), true)); } #[test] fn is_gitignored_lets_a_deeper_whitelist_override_a_shallower_rule() { let frames = [frame("", &["*.log"]), frame("sub", &["!keep.log"])]; - assert!(!is_gitignored( - &frames, - Path::new("sub/keep.log"), - Path::new("sub/keep.log"), - false, - Path::new("") - )); - } - - #[test] - fn is_gitignored_exempts_the_literal_chain_the_pattern_named() { - let frames = [frame("", &["dist/"])]; - assert!(!is_gitignored( - &frames, - Path::new("dist"), - Path::new("dist"), - true, - Path::new("dist") - )); + assert!(!is_gitignored(&frames, Path::new("sub/keep.log"), false)); } #[test] - fn is_gitignored_exempts_ancestor_rules_beneath_the_named_base() { - let frames = [frame("", &["dist/pkg/"])]; - assert!(!is_gitignored( - &frames, - Path::new("dist/pkg"), - Path::new("dist/pkg"), - true, - Path::new("dist") - )); - } - - #[test] - fn is_gitignored_honors_a_rule_at_the_named_base_itself() { - let frames = [frame("dist", &["*.map"])]; - assert!(is_gitignored( - &frames, - Path::new("dist/a.map"), - Path::new("dist/a.map"), - false, - Path::new("dist") - )); - } - - #[test] - fn frame_applies_everywhere_with_no_named_base() { - assert!(frame_applies( - Path::new(""), - Path::new("a.md"), - Path::new("") - )); - } - - #[test] - fn frame_applies_to_entries_outside_the_named_base() { - assert!(frame_applies( - Path::new(""), - Path::new("other/a.md"), - Path::new("dist") - )); - } - - #[test] - fn frame_above_the_base_does_not_apply_beneath_it() { - assert!(!frame_applies( - Path::new(""), - Path::new("dist/a.js"), - Path::new("dist") - )); - } - - #[test] - fn frame_at_the_base_still_applies_beneath_it() { - assert!(frame_applies( - Path::new("dist"), - Path::new("dist/a.js"), - Path::new("dist") - )); - } - - #[test] - fn frame_below_the_base_still_applies_beneath_it() { - assert!(frame_applies( - Path::new("dist/sub"), - Path::new("dist/sub/a.js"), - Path::new("dist") - )); + fn is_gitignored_lets_a_deeper_rule_override_a_shallower_whitelist() { + let frames = [frame("", &["!keep.log"]), frame("sub", &["*.log"])]; + assert!(is_gitignored(&frames, Path::new("sub/keep.log"), false)); } } diff --git a/packages/rust/src/vtab.rs b/packages/rust/src/vtab.rs index 38456844e..a135bac95 100644 --- a/packages/rust/src/vtab.rs +++ b/packages/rust/src/vtab.rs @@ -11,7 +11,6 @@ use rusqlite::vtab::Context; use rusqlite::{Connection, Result}; use crate::matcher::TableMatcher; -use crate::path_table; use crate::scanner::{scan_glob, to_slash}; use crate::vtab_scaffold::{self, TableSource}; @@ -84,9 +83,6 @@ struct ScanSpec { /// Empty for index-root-relative tables. path_prefix: PathBuf, ignore: TableMatcher, - /// Literal directories the pattern named outright; skip rules are judged - /// below this. - ignore_base: PathBuf, /// Whether the scan respects `.gitignore` files (off under `--no-ignore`). gitignore: bool, } @@ -258,7 +254,6 @@ fn parse_module_args(args: &[&[u8]]) -> Result { glob: vtab_scaffold::compile_glob(pattern)?, path_prefix: PathBuf::from(path_prefix), ignore: vtab_scaffold::compile_ignore(ignore)?, - ignore_base: path_table::ignore_base(pattern), gitignore: vtab_scaffold::parse_gitignore(gitignore)?, }) } @@ -291,13 +286,7 @@ impl TableSource for ScanSpec { fn rows(&self) -> Arc> { // The scan runs per statement rather than at CREATE, which is what // makes reads live: each statement sees the filesystem as it is now. - let rel_paths = scan_glob( - &self.root, - &self.glob, - &self.ignore, - &self.ignore_base, - self.gitignore, - ); + let rel_paths = scan_glob(&self.root, &self.glob, &self.ignore, self.gitignore); let stat = |rel: &Path| { fs::metadata(self.root.join(rel)).map_or_else( |_| StatFacts::default(), @@ -443,15 +432,6 @@ mod tests { ); } - #[test] - fn parse_module_args_derives_the_ignore_base_from_the_glob() { - let args = args_with(&[b"'/tmp'", b"'docs/**/*'", b"''", b"'gitignore'"]); - assert_eq!( - parse_module_args(&args).unwrap().ignore_base, - PathBuf::from("docs") - ); - } - #[test] fn parse_module_args_reads_the_gitignore_switch() { let on = args_with(&[b"'/tmp'", b"'**/*'", b"''", b"'gitignore'"]); diff --git a/packages/rust/tests/path_table_gitignore.rs b/packages/rust/tests/path_table_gitignore.rs index 64af67148..6e56388f3 100644 --- a/packages/rust/tests/path_table_gitignore.rs +++ b/packages/rust/tests/path_table_gitignore.rs @@ -194,3 +194,20 @@ fn a_scoped_glob_still_honors_gitignore_rules_beneath_its_base() { "a gitignore at the named base still filters below it" ); } + +#[test] +fn a_gitignore_above_the_named_base_does_not_reach_beneath_it() { + let root = fixture(); + fs::create_dir_all(root.path().join("docs")).unwrap(); + fs::write(root.path().join("docs/a.md"), "a").unwrap(); + fs::write(root.path().join("docs/trace.log"), "log").unwrap(); + let db = open(&root); + + let scanned = paths(&db.query("SELECT path FROM './docs/*'").unwrap()); + + assert_eq!( + scanned, + vec!["docs/a.md", "docs/trace.log"], + "the root `*.log` rule is above the named base, so it does not apply there" + ); +} diff --git a/packages/rust/tests/path_table_globs.rs b/packages/rust/tests/path_table_globs.rs index 588bc1e75..a8cbf2a06 100644 --- a/packages/rust/tests/path_table_globs.rs +++ b/packages/rust/tests/path_table_globs.rs @@ -319,3 +319,37 @@ fn a_missing_absolute_path_table_returns_no_rows() { assert!(rows.is_empty(), "expected no rows, got {rows:?}"); } + +/// Make `dir` traversable but not listable. Returns false when the process +/// bypasses permission checks (root), since the test's premise cannot hold. +#[cfg(unix)] +fn make_unlistable(dir: &Path) -> bool { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(dir, fs::Permissions::from_mode(0o311)).unwrap(); + fs::read_dir(dir).is_err() +} + +#[cfg(unix)] +fn make_listable(dir: &Path) { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(dir, fs::Permissions::from_mode(0o755)).unwrap(); +} + +#[cfg(unix)] +#[test] +fn a_prefixed_glob_starts_its_walk_at_the_named_directory() { + let root = fixture(); + let db = open(&root); + if !make_unlistable(root.path()) { + return; + } + + let found = db.query("SELECT path FROM './docs/*.md'"); + make_listable(root.path()); + + assert_eq!( + texts(&found.unwrap(), "path"), + vec!["docs/a.md", "docs/b.md"], + "the walk must begin at docs/, not list the index root" + ); +} diff --git a/packages/rust/tests/path_table_parser.rs b/packages/rust/tests/path_table_parser.rs index 2d0dc6ebf..10565838a 100644 --- a/packages/rust/tests/path_table_parser.rs +++ b/packages/rust/tests/path_table_parser.rs @@ -124,3 +124,27 @@ fn a_file_the_parser_cannot_handle_fails_the_whole_table() { "the parser's output is the table's output, so one bad file fails it: {err}" ); } + +#[cfg(unix)] +#[test] +fn a_parser_is_spawned_in_the_index_root_and_root_names_it() { + let root = fixture(); + fs::write( + root.path().join("parse.sh"), + "printf '[{\"root\":\"%s\"}]' \"$1\"\n", + ) + .unwrap(); + let db = DirSQL::builder() + .root(root.path()) + .path_table_parser("sh parse.sh {root}") + .build() + .unwrap(); + + let rows = db.query("SELECT root FROM './docs/*.md'").unwrap(); + + assert_eq!( + texts(&rows, "root"), + vec![root.path().to_string_lossy().into_owned()], + "a relative parser resolves against the index root, which {{root}} names" + ); +} diff --git a/packages/rust/tests/schema_inference.rs b/packages/rust/tests/schema_inference.rs index 26e34038a..f7ffc5a44 100644 --- a/packages/rust/tests/schema_inference.rs +++ b/packages/rust/tests/schema_inference.rs @@ -19,13 +19,14 @@ use tempfile::TempDir; const COMBINE_SCRIPT: &str = "#!/bin/sh\nprintf '['\nsep=''\nfor f; do\n body=$(cat \"$f\")\n body=${body#'['}\n body=${body%']'}\n if [ -n \"$body\" ]; then printf '%s%s' \"$sep\" \"$body\"; sep=','; fi\ndone\nprintf ']'\n"; /// The SQL that declares a vtab named `t` over `glob` under `dir`, parsed by -/// the combining script written into `dir`. The parser runs from `dir`, so -/// the script is named relative to it. The trailing empty argument is the -/// cache path: this vtab is ephemeral, so there is nowhere to reuse rows from. +/// the combining script written into `dir`. The parser runs from the index +/// root, the last argument, which is `dir` itself here, so the script is +/// named relative to it. The empty argument before it is the cache path: +/// this vtab is ephemeral, so there is nowhere to reuse rows from. fn declare(dir: &TempDir, glob: &str) -> String { fs::write(dir.path().join("combine.sh"), COMBINE_SCRIPT).unwrap(); format!( - "CREATE VIRTUAL TABLE t USING dirsql_parsed('{}', '{}', 'sh combine.sh', 'gitignore', '')", + "CREATE VIRTUAL TABLE t USING dirsql_parsed('{0}', '{1}', 'sh combine.sh', 'gitignore', '', '{0}')", dir.path().display(), glob, ) diff --git a/packages/rust/tests/vtab.rs b/packages/rust/tests/vtab.rs index fee41d405..72ad2c87d 100644 --- a/packages/rust/tests/vtab.rs +++ b/packages/rust/tests/vtab.rs @@ -359,16 +359,30 @@ fn ignore_patterns_skip_matching_files() { } #[test] -fn a_pattern_naming_an_ignored_directory_still_scans_it() { +fn a_table_rooted_in_an_ignored_directory_still_scans_it() { let dir = TempDir::new().unwrap(); fs::create_dir_all(dir.path().join("node_modules")).unwrap(); fs::write(dir.path().join("node_modules/x.js"), "x").unwrap(); - let conn = open_over_with(&dir, "node_modules/**/*", "", &["node_modules/**"]); + let conn = Connection::open_in_memory().unwrap(); + load_module(&conn, StatementScope::new()).unwrap(); + conn.execute_batch(&format!( + "CREATE VIRTUAL TABLE t USING dirsql_path('{}', '**/*', 'node_modules', 'gitignore', '**/node_modules/**')", + dir.path().join("node_modules").display() + )) + .unwrap(); - let count: i64 = conn - .query_row("SELECT count(*) FROM t", [], |r| r.get(0)) - .unwrap(); - assert_eq!(count, 1, "skip rules apply below the path you name"); + let paths: Vec = conn + .prepare("SELECT path FROM t") + .unwrap() + .query_map([], |r| r.get(0)) + .unwrap() + .map(Result::unwrap) + .collect(); + assert_eq!( + paths, + vec!["node_modules/x.js"], + "skip rules are judged relative to the scan root, not the reported path" + ); } #[test] diff --git a/packages/ts/e2e-attestations/claude-1199-glob-prefix.json b/packages/ts/e2e-attestations/claude-1199-glob-prefix.json new file mode 100644 index 000000000..7cd6d91c6 --- /dev/null +++ b/packages/ts/e2e-attestations/claude-1199-glob-prefix.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1790974768, + "exit_code": 0, + "commit": "f8ccb07e9a7c1aec92c043eb7b3ed49d3d23f41e", + "branch": "claude/1199-glob-prefix" +} diff --git a/testing-conventions.toml b/testing-conventions.toml index f4172e9b0..bbdfadbe1 100644 --- a/testing-conventions.toml +++ b/testing-conventions.toml @@ -107,25 +107,25 @@ features = ["cli"] [[rust.exempt]] path = "src/scanner.rs" rules = ["mutation"] -lines = [72, 126, 191, 221, 261] +lines = [72, 116, 173, 201, 237] reason = "scan_below, scan_glob, walk, descend and sorted_entries read the filesystem; tests/scanner.rs (scan_finds_matching_files, scan_recurses_into_subdirectories) and tests/vtab.rs (glob_scopes_the_walk, rowids_count_up_from_zero_in_scan_order) assert what they return" [[rust.exempt]] path = "src/scanner.rs" rules = ["mutation"] -lines = [88] +lines = [82] reason = "the walk's running file count is observable only through the CLI's progress line; tests/scan_progress_e2e.rs forced_progress_reports_the_walk_phase_on_stderr asserts the number" [[rust.exempt]] path = "src/scanner.rs" rules = ["mutation"] -lines = [231] +lines = [208] reason = "a child's depth decides the reserved top-level .dirsql rule for a real tree; tests/scanner.rs scan_excludes_top_level_dirsql_directory and tests/vtab.rs reserved_dirsql_directory_is_skipped assert it" [[rust.exempt]] path = "src/vtab.rs" rules = ["mutation"] -lines = [294, 311] +lines = [289, 302] reason = "rows walks and stats a real tree and column hands a cell to SQLite; the cell choice is unit-tested in `cell`, and tests/vtab.rs stat_columns_carry_real_values and content_is_excluded_from_star_but_selectable_by_name assert both through a real connection" # Unit-only coverage floor: measured by `cargo llvm-cov --lib --features cli`