From 757fa51c0dad6f8d2914f1cfba51772df45fb9d2 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 2 Oct 2026 17:05:47 -0400 Subject: [PATCH 1/8] refactor(core): fail the build on an on-file command failure; drop the parsed-row cache and exit 23 An on-file command runs once per table, so a run that fails is the table's error and the build's. The partial-scan exit code, the capped `dirsql: skipping` stderr report, and the per-table entry in `scan_failures` go with it; `OnFileFailure` is a file again. The persisted row cache for a parsed path-table (`_dirsql_parsed_rows`, the vtab's cache argument, and the db/persist plumbing that threaded it) is removed: there is no per-file result to cache. --- PARITY.md | 19 +- docs/howto/define-tables.md | 4 +- docs/howto/persist.md | 18 +- docs/reference/cli.md | 6 +- docs/reference/config.md | 6 +- docs/reference/hooks.md | 21 +- docs/reference/path-tables.md | 16 +- .../2026-10-02-on-file-no-partial-scan.md | 8 + .../2026-10-02-on-file-no-partial-scan.md | 50 ++ packages/rust/src/cli/run.rs | 63 +- packages/rust/src/command.rs | 28 +- packages/rust/src/config.rs | 67 +- packages/rust/src/db.rs | 94 +-- packages/rust/src/lib.rs | 40 +- packages/rust/src/parsed_cache.rs | 454 ------------- packages/rust/src/parsed_vtab.rs | 613 +----------------- packages/rust/src/persist.rs | 7 +- packages/rust/tests/command_runner.rs | 12 +- packages/rust/tests/extensions.rs | 6 +- packages/rust/tests/multi_config.rs | 28 +- .../rust/tests/no_implicit_files_table.rs | 2 +- packages/rust/tests/on_file.rs | 98 ++- packages/rust/tests/on_file_e2e.rs | 12 +- packages/rust/tests/on_file_failure_e2e.rs | 118 ++++ .../rust/tests/persist_parsed_path_table.rs | 356 ---------- packages/rust/tests/scan_failures.rs | 54 +- packages/rust/tests/scan_failures_e2e.rs | 218 ------- 27 files changed, 394 insertions(+), 2024 deletions(-) create mode 100644 packages/rust/changelog.d/2026-10-02-on-file-no-partial-scan.md create mode 100644 packages/rust/migrations.d/2026-10-02-on-file-no-partial-scan.md delete mode 100644 packages/rust/src/parsed_cache.rs create mode 100644 packages/rust/tests/on_file_failure_e2e.rs delete mode 100644 packages/rust/tests/persist_parsed_path_table.rs delete mode 100644 packages/rust/tests/scan_failures_e2e.rs diff --git a/PARITY.md b/PARITY.md index 1d5a1019b..bcc52c58c 100644 --- a/PARITY.md +++ b/PARITY.md @@ -399,12 +399,13 @@ to the Rust-core-public `functions::DEFAULT_FUNCTION_TIMEOUT` (30s, #820) — config-schema surface shared by every install, with **no** Python/TypeScript public-API binding, so no drift. -- **`on-file` (B2 #327).** A **required** `[[table]]` key naming a per-file - command whose JSON-array stdout becomes the table's rows (interpolation-only - placeholders `{path}` (the file's **absolute** path, #542) / `{root}` — a - template that omits one receives no value, no append-if-absent, #538/#539; - per-file error isolation; unbounded since #820 — bound a hook by wrapping - its command in `timeout(1)`). A `[[table]]` without +- **`on-file` (B2 #327).** A **required** `[[table]]` key naming a per-table + command whose JSON-array stdout becomes the table's rows (every matched + file's **absolute** path appended as a trailing argument; the + interpolation-only placeholder `{root}` — a template that omits it receives + no value, no append-if-absent, #538/#539; a failed command fails the build; + unbounded since #820 — bound a hook by wrapping its command in + `timeout(1)`). A `[[table]]` without it is a load error since #634 (after fact-injection removal a hook-less table would emit only all-NULL rows), so `config::TableConfig::on_file` is `String`, not `Option`. Parsed and executed in the shared Rust core @@ -423,8 +424,8 @@ public-API binding, so no drift. - **`--on-file` (path-table parser, #631).** A `dirsql query` flag naming a command that supplies every path-table's rows and schema (a JSON array of row - objects; the `on-file` hook contract — argv splitting, `{path}`/`{root}`, - per-file failure isolation). The stat columns are not + objects; the `on-file` hook contract — argv splitting, `{root}`, trailing + paths, a failed command fails the query). The stat columns are not reachable on a parsed path-table; parsed scans honor the same skip rules stat scans do. Threaded through the shared Rust core (`db::Db::set_path_table_parser` → the `dirsql_parsed` module) via a **doc-hidden** `DirSQLBuilder::path_table_parser` @@ -451,7 +452,7 @@ public-API binding, so no drift. - `AsyncDirSQL` uses tokio and `OnceCell` internally. - Watch returns `futures_channel::mpsc::UnboundedReceiver` implementing `Stream`. - All fallible operations return `Result`. Statements classified as writes by SQLite's `sqlite3_stmt_readonly` surface as the unit variant `DirSqlError::WriteForbidden`; in the Python/TS bindings the same condition is a `RuntimeError` / `Error` with a "read-only" message. -- **Parity restored (dirsql#715).** All three SDKs expose the files a scan could not index: Rust `DirSQL::scan_failures() -> &[OnFileFailure]`, Python `await db.scan_failures()` (a list of `ScanFailure` with `.path` / `.message`, importable as `from dirsql import ScanFailure`), TypeScript `await db.scanFailures()` (`ScanFailure[]` with `path` / `message`, exported from the package barrel). A hook that fails, or a row the table rejects, skips that file rather than the scan; the list is how a caller learns the index is incomplete and which files are missing. Empty after a clean scan. The CLI additionally reports skips on stderr (capped at ten, then `... and N more`) and exits `23` — CLI-only by nature, since a library has no exit code. `--allow-skipped` remains unbuilt and is tracked in #715. +- **Parity restored (dirsql#715).** All three SDKs expose the files a scan could not index: Rust `DirSQL::scan_failures() -> &[OnFileFailure]`, Python `await db.scan_failures()` (a list of `ScanFailure` with `.path` / `.message`, importable as `from dirsql import ScanFailure`), TypeScript `await db.scanFailures()` (`ScanFailure[]` with `path` / `message`, exported from the package barrel). A programmatic per-file hook that fails, or a row the table rejects, skips that file rather than the scan; the list is how a caller learns the index is incomplete and which files are missing. Empty after a clean scan. A config-declared `on-file` command runs once per table, so its failure is a build error, not a skip; the CLI prints it and exits `1`. `--allow-skipped` remains unbuilt and is tracked in #715. ### TypeScript - Uses `camelCase` for method names. diff --git a/docs/howto/define-tables.md b/docs/howto/define-tables.md index f9d6d3742..0a849bbf2 100644 --- a/docs/howto/define-tables.md +++ b/docs/howto/define-tables.md @@ -116,8 +116,8 @@ templates, with the trigger mistakes that fail silently, are in ## Going further -- The parser mechanics — placeholders, stdout protocol, per-file failure - isolation — are the [`on-file` hook contract](../reference/hooks.md#on-file); +- The parser mechanics — placeholders, stdout protocol, failure semantics — + are the [`on-file` hook contract](../reference/hooks.md#on-file); [Extract rows from file contents](./extract-from-contents.md) is the fuller recipe. - Your directory layout encodes data (authors, dates, IDs)? Split the path in diff --git a/docs/howto/persist.md b/docs/howto/persist.md index 514a3d183..f06e5d802 100644 --- a/docs/howto/persist.md +++ b/docs/howto/persist.md @@ -4,8 +4,8 @@ By default the database is ephemeral: rebuilt from your files on every startup and discarded on exit. The [`--persist [PATH]`](../reference/cli.md#dirsql-server) flag keeps the SQLite index on disk instead, so a restart only re-parses files that actually changed — the difference between seconds and -milliseconds on large trees, and between re-running and skipping expensive -[`on-file`](./extract-from-contents.md) commands. +milliseconds on large trees, and between re-running and skipping an expensive +declared-table [`on-file`](./extract-from-contents.md) command. Whether and where to cache is a machine-local operational choice — it belongs to the command you run, not to the shared `.dirsql.toml`. That is @@ -58,15 +58,11 @@ exactly what a non-persistent startup does. A run that changes nothing writes nothing: the cache file is read and left exactly as it was. -This covers both kinds of table. Declared tables keep their indexed rows, and -a [path-table](../reference/path-tables.md) parsed with `--on-file` keeps each -file's parser output — so the second run over an unchanged tree spawns no -parser process at all. A path-table's cached rows are keyed by the tree, the -glob, the parser command and the `dirsql` version together, so changing any of -them parses afresh rather than serving rows the current command never -produced. A *stat* path-table caches nothing, because there is nothing to -save: its columns are the metadata the scan already collected, and `content` -is read live by design. +This covers declared tables, which keep their indexed rows. A +[path-table](../reference/path-tables.md) is not cached: a *stat* path-table +has nothing to save, since its columns are the metadata the scan already +collected and `content` is read live by design, and a path-table parsed with +`--on-file` runs its parser on every start. Persistence is a startup-time optimization, not a change in meaning: the database remains a derived view of your files, and queries return the same diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 668495c13..1ed560f0b 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -223,11 +223,7 @@ every statement, so it is reported once and exits `1` before the first prompt. | Code | Meaning | |---|---| | `0` | Clean EOF (Ctrl-D, `exit`, `quit`, or the end of a piped script) — **including when statements failed**. Matches interactive `sqlite3`; use [`dirsql query`](#dirsql-query) when a script needs a statement's exit status. | -| `1` | The index could not be built (a bad `-c`, an unresolvable `--on-file`), or stdin could not be read. Nothing was executed. | - -`23` (partial scan) is not produced here: skipped files are reported before the -first prompt, and a session's exit code describes the session rather than one -scan. +| `1` | The index could not be built (a bad `-c`, an unresolvable `--on-file`, a failed `on-file` command), or stdin could not be read. Nothing was executed. | ## `dirsql server` diff --git a/docs/reference/config.md b/docs/reference/config.md index 8543f0516..435bffccd 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -330,9 +330,9 @@ integral number → `INTEGER`, any other number → `REAL`; a string → `TEXT`; nested array or object → its JSON text as `TEXT`. A row's columns are exactly the keys the command emits, narrowed to the DDL; -dirsql merges nothing else in. Output that is not a JSON array of objects is a -per-file failure: the file is skipped with a stderr warning and the scan -continues (see [failure semantics](./hooks.md#failure-semantics)). +dirsql merges nothing else in. Output that is not a JSON array of objects fails +the table, and a failed table fails the build (see +[failure semantics](./hooks.md#failure-semantics)). ## Composing multiple configs diff --git a/docs/reference/hooks.md b/docs/reference/hooks.md index d11ae533d..883c6c45e 100644 --- a/docs/reference/hooks.md +++ b/docs/reference/hooks.md @@ -69,8 +69,8 @@ on-file = "timeout 30 my-extractor" ``` When the wrapper kills an overrunning command, the run exits non-zero and -the ordinary [failure semantics](#failure-semantics) apply — the file is -skipped, the scan continues. +the ordinary [failure semantics](#failure-semantics) apply — the table's +command failed, so the build fails. ::: warning Windows Windows's built-in `timeout` command is a *sleep*, not a bound — it cannot @@ -94,18 +94,17 @@ A hook run fails when the command: - exits zero but prints no non-empty stdout line, - or prints output that does not parse as a JSON array of row objects. -What a failure *means*: **per-file isolation.** The file contributes no rows -and is reported as skipped; the scan indexes every other file and commits. -The CLI names up to ten skipped files on stderr, then `... and N more`, and -exits `23` — distinct from `0` (clean) and `1` (the run failed), so a caller -can tell a partial index from a complete one. A row the table rejects under -`strict` counts as the same kind of failure. +What a failure *means*: the command ran once for the whole table, so its +failure is the table's, and a table that cannot be filled fails the build. +The error names the table and carries the command's exit status and stderr +tail; the CLI prints it and exits `1`. A row the table rejects under `strict` +counts as the same kind of failure. ## `on-file` contract -Runs once per file matched by the table's `glob`, at initial scan and on -every watched change. The command reads the file itself and prints a JSON -array of row objects; see [`[[table]]`](./config.md#table) for the +Runs once per table, over every file matched by the table's `glob`, at +initial scan and on every watched change. The command reads the files itself +and prints a JSON array of row objects; see [`[[table]]`](./config.md#table) for the row-mapping rules. The same command is attachable two ways, over the same contract: the diff --git a/docs/reference/path-tables.md b/docs/reference/path-tables.md index 5054f49ff..5a366bcab 100644 --- a/docs/reference/path-tables.md +++ b/docs/reference/path-tables.md @@ -156,18 +156,14 @@ placeholder, same timeout. Its output *is* the table: across the emitted rows. The stat columns (`path`, `size`, …) are **not** reachable on a parsed path-table — a parser that wants the path emits it (it has the paths). The two modes stay cleanly separate. -- **Failures are isolated per file.** A file whose parser fails (spawn, non-zero - exit, timeout, or no output) or whose output is not a JSON array of rows - contributes no rows; a one-line warning naming the file goes to stderr and the - scan continues. The schema is inferred from the files that did parse. +- **A parser failure is the table's failure.** A run that fails (spawn, + non-zero exit, a `timeout(1)` kill, or no output) or whose output is not a + JSON array of rows fails the query with an error naming the table; there is + no partial table. - **The skip rules still apply.** A parsed scan honors the same `node_modules` /`.git`/`ignore` rules a stat scan does (see below). -- **`--persist` skips the parser for unchanged files.** With a - [persistent cache](/howto/persist), each file's parser output is stored - against its stat metadata, so a later run over an unchanged tree serves the - rows from the cache and spawns no process. Change the file, the glob, the - parser command, or the dirsql version and that file (or that whole table) is - parsed again. +- **The parser runs on every start.** A parsed path-table is not kept in the + [persistent cache](/howto/persist); declared tables are. `--on-file` applies to **every** path-table in the query and may be given **at most once**. For different parsers per file set, define named tables in a diff --git a/packages/rust/changelog.d/2026-10-02-on-file-no-partial-scan.md b/packages/rust/changelog.d/2026-10-02-on-file-no-partial-scan.md new file mode 100644 index 000000000..10f77606f --- /dev/null +++ b/packages/rust/changelog.d/2026-10-02-on-file-no-partial-scan.md @@ -0,0 +1,8 @@ +**Removed** The partial-scan exit code `23` and the `dirsql: skipping ...` +diagnostics. An `on-file` command runs once per table, so a run that fails, +prints no output, or prints something other than a JSON array of row objects +is a build error naming the table: the CLI prints it and exits `1`, and the +SDK constructors return it. `scan_failures()` now records only programmatic +per-file hooks. Also removed is the persisted row cache for a path-table +parsed with `--on-file`: the parser runs on every start, and the cache file's +`_dirsql_parsed_rows` table is no longer created or read. diff --git a/packages/rust/migrations.d/2026-10-02-on-file-no-partial-scan.md b/packages/rust/migrations.d/2026-10-02-on-file-no-partial-scan.md new file mode 100644 index 000000000..c65f18950 --- /dev/null +++ b/packages/rust/migrations.d/2026-10-02-on-file-no-partial-scan.md @@ -0,0 +1,50 @@ +### Core: a failed `on-file` command fails the build; no exit code 23, no parsed-row cache + +#### Summary + +An `on-file` command that fails is no longer reported as a skipped table +while the run continues. Its table cannot be filled, so the build fails with +an error naming the table and carrying the command's stderr tail. The CLI's +partial-scan exit code `23` and the `dirsql: skipping ...` stderr lines are +gone, and a path-table parsed with `--on-file` is no longer cached under +`--persist`. + +#### Required changes + +| Surface | Before | After | +| ------- | ------ | ----- | +| Shell script checking `dirsql query`'s exit code | treats `23` as "rows on stdout, some tables empty" | a failed command exits `1` with nothing on stdout; `0` means every table filled | +| SDK caller reading `scan_failures()` / `scanFailures()` for a config-declared table | the failed table appears under its name | the constructor returns `DirSqlError::TableCommand` (Python `RuntimeError`, TypeScript `Error`) instead | +| `--persist` with `--on-file` | second run over an unchanged tree served the parsed rows from the cache | the parser runs on every start | + +#### Deprecations removed + +_None._ + +#### Behavior changes without code changes + +- A `[[table]]` whose command fails now fails the whole build, including every + other table; previously the other tables were queryable and the failed one + was empty. +- `OnFileFailure.path` is always a file path again; it never carries a table + name. +- An existing persist cache keeps its `_dirsql_parsed_rows` table, now unread. + Delete the cache file to reclaim the space; the next start rebuilds it. + +#### Verification + +```bash +cd "$(mktemp -d)" +printf 'alpha\n' > a.md +cat > .dirsql.toml <<'TOML' +[[table]] +name = "notes" +ddl = "CREATE TABLE notes (name TEXT)" +glob = "*.md" +on-file = "sh -c 'echo nope >&2; exit 1'" +TOML +dirsql query "SELECT name FROM notes" -c .dirsql.toml +echo "exit $?" +# expected on stderr: dirsql query: ... table `notes`: ... nope +# expected: exit 1 +``` diff --git a/packages/rust/src/cli/run.rs b/packages/rust/src/cli/run.rs index e96ee529f..e7a70faa8 100644 --- a/packages/rust/src/cli/run.rs +++ b/packages/rust/src/cli/run.rs @@ -367,10 +367,6 @@ async fn run_default(cli: Cli) -> u8 { } }; let state = load_state(&cli.common, parser); - // Skipped files are named once here rather than per statement, and - // `PARTIAL_SCAN_EXIT` has no REPL meaning: the session's exit code - // describes the session, not one scan. - report_scan_failures(&state); run_repl( &state, cli.format.resolve(std::io::stdout().is_terminal()), @@ -401,7 +397,6 @@ async fn run_query(args: QueryArgs) -> u8 { } }; let state = load_state(&args.common, parser); - let skipped = report_scan_failures(&state); // Unbounded: the process IS the query, so `timeout(1)` expresses any cap // natively; only the long-lived server enforces `query_timeout` (408). @@ -410,11 +405,7 @@ async fn run_query(args: QueryArgs) -> u8 { match execute_query(&state, query_body(&args.sql), None).await { Ok(value) => { print!("{}", render_rows(&value, format)); - // The query ran and its rows are on stdout, so this is not a - // failure -- but the index behind them is missing files, and a - // caller piping into `jq` under `set -e` has no other way to find - // that out. - if skipped { PARTIAL_SCAN_EXIT } else { 0 } + 0 } Err(failure) => { eprintln!("dirsql query: {}", failure.message()); @@ -423,38 +414,6 @@ async fn run_query(args: QueryArgs) -> u8 { } } -/// Exit code for "the scan completed, but some files were skipped" -- distinct -/// from `1` so a script can tell a partial index from a run that failed. -/// Follows rsync's `23` ("partial transfer due to error"). -const PARTIAL_SCAN_EXIT: u8 = 23; - -/// How many skipped files to name before collapsing the rest into a count. A -/// directory of unreadable files should not bury the terminal. -const MAX_REPORTED_FAILURES: usize = 10; - -/// Print the scan's skipped files to stderr, capped, and report whether there -/// were any. stdout is left for the query result alone. -fn report_scan_failures(state: &AppState) -> bool { - let AppState::Ready(db) = state else { - return false; - }; - let failures = db.scan_failures(); - if failures.is_empty() { - return false; - } - for failure in failures.iter().take(MAX_REPORTED_FAILURES) { - eprintln!("dirsql: skipping `{}`: {}", failure.path, failure.message); - } - if let Some(rest) = failures - .len() - .checked_sub(MAX_REPORTED_FAILURES) - .filter(|n| *n > 0) - { - eprintln!("dirsql: ... and {rest} more"); - } - true -} - /// Synthesize the exact `POST /query` body for a positional SQL argument, /// so the shared pipeline's intake validation sees byte-for-byte what an /// HTTP client would send. Shared with the REPL, whose statements must reach @@ -747,9 +706,9 @@ mod tests { #[test] fn bare_sql_carries_on_file_for_the_default_query() { // #662: `--on-file` works in the default mode exactly as under `query`. - let cli = Cli::parse_from(["dirsql", "SELECT 1", "--on-file", "cat {path}"]); + let cli = Cli::parse_from(["dirsql", "SELECT 1", "--on-file", "cat"]); assert!(cli.command.is_none()); - assert_eq!(cli.on_file, vec!["cat {path}".to_string()]); + assert_eq!(cli.on_file, vec!["cat".to_string()]); } #[test] @@ -1001,8 +960,8 @@ mod tests { #[test] fn on_file_parses_after_the_query_subcommand() { assert_eq!( - query_on_file(&["dirsql", "query", "SELECT 1", "--on-file", "cat {path}"]), - vec!["cat {path}".to_string()] + query_on_file(&["dirsql", "query", "SELECT 1", "--on-file", "cat"]), + vec!["cat".to_string()] ); } @@ -1039,8 +998,8 @@ mod tests { #[test] fn resolve_on_file_returns_the_single_command() { assert_eq!( - resolve_on_file(&["cat {path}".to_string()]), - Ok(Some("cat {path}".to_string())) + resolve_on_file(&["cat".to_string()]), + Ok(Some("cat".to_string())) ); } @@ -1141,14 +1100,6 @@ mod tests { assert_eq!(code, 2); } - #[test] - fn report_scan_failures_is_false_when_the_index_is_degraded() { - // The `Ready` arm needs a real scanned directory, so it lives in the - // integration tier; the degraded arm short-circuits before any I/O. - let state = AppState::Unavailable("config failed to load".to_string()); - assert!(!report_scan_failures(&state)); - } - #[test] fn default_records_table_is_built_from_the_shipped_starter_config() { // `--include-default` seeds this table, so a starter config that diff --git a/packages/rust/src/command.rs b/packages/rust/src/command.rs index 8e2fc2a88..6a26412f2 100644 --- a/packages/rust/src/command.rs +++ b/packages/rust/src/command.rs @@ -8,8 +8,8 @@ //! single argument — but no shell is ever invoked: there is no globbing, //! piping, or variable expansion. To get a real shell, ask for one //! explicitly (`sh -c '…'`). -//! - **Placeholders.** `{path}`, `{args}`, `{root}` (and any -//! others a caller supplies) are substituted into whole argv tokens, every +//! - **Placeholders.** `{args}`, `{root}` (and any others a caller +//! supplies) are substituted into whole argv tokens, every //! occurrence. Substitution is single-pass and left-to-right per token, so a //! value that itself contains `{…}` is never re-scanned — a substituted value //! is always exactly one argv element, keeping values with spaces (and @@ -31,8 +31,8 @@ use std::process::{Command, Stdio}; /// A named placeholder substituted into a command's argv. /// -/// `name` is the bare identifier (no braces): a `name` of `path` matches the -/// template token `{path}`. Substitution only — a placeholder whose `{name}` +/// `name` is the bare identifier (no braces): a `name` of `root` matches the +/// template token `{root}`. Substitution only — a placeholder whose `{name}` /// never appears in the template is a no-op. #[derive(Debug, Clone)] pub struct Placeholder { @@ -573,7 +573,7 @@ mod tests { #[test] fn substitutes_a_single_placeholder() { assert_eq!( - argv("cat {path}", &[Placeholder::new("path", "a/b.json")]), + argv("cat {file}", &[Placeholder::new("file", "a/b.json")]), ["cat", "a/b.json"] ); } @@ -582,8 +582,8 @@ mod tests { fn substitutes_all_occurrences_across_and_within_tokens() { assert_eq!( argv( - "cp {path} {path}.bak --label={path}", - &[Placeholder::new("path", "x")] + "cp {file} {file}.bak --label={file}", + &[Placeholder::new("file", "x")] ), ["cp", "x", "x.bak", "--label=x"] ); @@ -592,7 +592,7 @@ mod tests { #[test] fn a_substituted_value_with_spaces_stays_a_single_arg() { assert_eq!( - argv("read {path}", &[Placeholder::new("path", "my file.json")]), + argv("read {file}", &[Placeholder::new("file", "my file.json")]), ["read", "my file.json"] ); } @@ -601,27 +601,27 @@ mod tests { fn substitution_is_single_pass_so_injected_braces_are_not_rescanned() { assert_eq!( argv( - "run {args} {path}", + "run {args} {file}", &[ - Placeholder::new("args", "hello {path}"), - Placeholder::new("path", "REAL"), + Placeholder::new("args", "hello {file}"), + Placeholder::new("file", "REAL"), ] ), - ["run", "hello {path}", "REAL"] + ["run", "hello {file}", "REAL"] ); } #[test] fn unknown_placeholder_is_left_literal() { assert_eq!( - argv("echo {unknown} {path}", &[Placeholder::new("path", "p")]), + argv("echo {unknown} {file}", &[Placeholder::new("file", "p")]), ["echo", "{unknown}", "p"] ); } #[test] fn a_placeholder_the_template_omits_is_dropped() { - assert_eq!(argv("run", &[Placeholder::new("path", "/tmp/x")]), ["run"]); + assert_eq!(argv("run", &[Placeholder::new("file", "/tmp/x")]), ["run"]); } #[test] diff --git a/packages/rust/src/config.rs b/packages/rust/src/config.rs index aa1c7da30..bd5512268 100644 --- a/packages/rust/src/config.rs +++ b/packages/rust/src/config.rs @@ -507,13 +507,13 @@ ignore = ["node_modules/**", ".git/**"] name = "comments" ddl = "CREATE TABLE comments (thread_id TEXT, path TEXT)" glob = "_comments/{thread_id}/index.jsonl" -on-file = "cat {path}" +on-file = "cat" [[table]] name = "items" ddl = "CREATE TABLE items (path TEXT, size INTEGER)" glob = "catalog/*.json" -on-file = "cat {path}" +on-file = "cat" strict = true "#; let config = load_config_str(toml).unwrap(); @@ -535,7 +535,7 @@ strict = true [[table]] ddl = "CREATE TABLE t (x TEXT)" glob = "*.json" -on-file = "cat {path}" +on-file = "cat" "#; let err = load_config_str(toml).unwrap_err(); assert!( @@ -552,7 +552,7 @@ on-file = "cat {path}" name = " " ddl = "CREATE TABLE t (x TEXT)" glob = "*.json" -on-file = "cat {path}" +on-file = "cat" "#; let err = load_config_str(toml).unwrap_err(); assert!( @@ -570,7 +570,7 @@ on-file = "cat {path}" name = "messages" ddl = "CREATE TABLE notes (x TEXT)" glob = "*.json" -on-file = "cat {path}" +on-file = "cat" "#; let config = load_config_str(toml).unwrap(); assert_eq!(config.tables[0].name, "messages"); @@ -582,7 +582,7 @@ on-file = "cat {path}" [[table]] name = "t" glob = "*.json" -on-file = "cat {path}" +on-file = "cat" "#; let err = load_config_str(toml).unwrap_err(); assert!(err.to_string().contains("ddl")); @@ -632,7 +632,7 @@ ignore = ["*.tmp"] name = "t" ddl = "CREATE TABLE t (x TEXT)" glob = "*.json" -on-file = "cat {path}" +on-file = "cat" "#; let config = load_config_str(toml).unwrap(); assert!(config.ignore.is_empty()); @@ -674,19 +674,19 @@ persist_path = "/var/cache/dirsql.db" name = "a" ddl = "CREATE TABLE a (path TEXT)" glob = "a/*.json" -on-file = "cat {path}" +on-file = "cat" [[table]] name = "b" ddl = "CREATE TABLE b (path TEXT)" glob = "b/*.csv" -on-file = "cat {path}" +on-file = "cat" [[table]] name = "c" ddl = "CREATE TABLE c (path TEXT)" glob = "c/*.yaml" -on-file = "cat {path}" +on-file = "cat" "#; let config = load_config_str(toml).unwrap(); assert_eq!(config.tables.len(), 3); @@ -704,7 +704,7 @@ on-file = "cat {path}" name = "t" ddl = "CREATE TABLE t (path TEXT)" glob = "*.json" -on-file = "cat {path}" +on-file = "cat" format = "json" "#; let err = load_config_str(toml).unwrap_err(); @@ -772,7 +772,7 @@ entrypoint = "sqlite3_vec_init" name = "t" ddl = "CREATE TABLE t (path TEXT)" glob = "*.json" -on-file = "cat {path}" +on-file = "cat" "#; let config = load_config_str(toml).unwrap(); assert_eq!(config.extensions.len(), 1); @@ -814,7 +814,7 @@ entrypoint = "sqlite3_x_init" name = "t" ddl = "CREATE TABLE t (path TEXT)" glob = "*.json" -on-file = "cat {path}" +on-file = "cat" "#; let config = load_config_str(toml).unwrap(); assert!(config.extensions.is_empty()); @@ -842,14 +842,11 @@ path = "b.so" name = "papers" ddl = "CREATE TABLE papers (paper_id TEXT, title TEXT)" glob = "**/meta.json" -on-file = "uv run python extract_papers.py {path}" +on-file = "uv run python extract_papers.py" "#; let config = load_config_str(toml).unwrap(); assert_eq!(config.tables.len(), 1); - assert_eq!( - config.tables[0].on_file, - "uv run python extract_papers.py {path}" - ); + assert_eq!(config.tables[0].on_file, "uv run python extract_papers.py"); } #[test] @@ -939,7 +936,7 @@ entrypoint = "sqlite3_vec_init" name = "t" ddl = "CREATE TABLE t (path TEXT)" glob = "*.json" -on-file = "cat {path}" +on-file = "cat" "#); let merged = combine_configs(&[(src("/proj/.dirsql.toml"), config.clone())]).unwrap(); assert_eq!(merged.ignore, config.ignore); @@ -956,20 +953,20 @@ on-file = "cat {path}" name = "a" ddl = "CREATE TABLE a (path TEXT)" glob = "a/*.json" -on-file = "cat {path}" +on-file = "cat" [[table]] name = "b" ddl = "CREATE TABLE b (path TEXT)" glob = "b/*.json" -on-file = "cat {path}" +on-file = "cat" "#); let b = cfg(r#" [[table]] name = "c" ddl = "CREATE TABLE c (path TEXT)" glob = "c/*.json" -on-file = "cat {path}" +on-file = "cat" "#); let merged = combine_configs(&[(src("/a"), a), (src("/b"), b)]).unwrap(); let ddls: Vec<&str> = merged.tables.iter().map(|t| t.ddl.as_str()).collect(); @@ -1015,10 +1012,10 @@ on-file = "cat {path}" #[test] fn combine_duplicate_table_name_errors_naming_both_sources() { let a = cfg( - "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (x TEXT)\"\nglob = \"a/*.json\"\non-file = \"cat {path}\"\n", + "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (x TEXT)\"\nglob = \"a/*.json\"\non-file = \"cat\"\n", ); let b = cfg( - "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat {path}\"\n", + "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat\"\n", ); let err = combine_configs(&[ (src("/proj/.dirsql.toml"), a), @@ -1048,8 +1045,8 @@ on-file = "cat {path}" // The single-entry identity path runs no collision check, exactly like // a plain single-config load. let config = cfg(concat!( - "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (x TEXT)\"\nglob = \"a/*.json\"\non-file = \"cat {path}\"\n", - "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat {path}\"\n", + "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (x TEXT)\"\nglob = \"a/*.json\"\non-file = \"cat\"\n", + "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat\"\n", )); let merged = combine_configs(&[(src("/a"), config)]).unwrap(); assert_eq!(merged.tables.len(), 2); @@ -1058,8 +1055,8 @@ on-file = "cat {path}" #[test] fn combine_intra_config_duplicate_in_multi_merge_errors() { let a = cfg(concat!( - "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (x TEXT)\"\nglob = \"a/*.json\"\non-file = \"cat {path}\"\n", - "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat {path}\"\n", + "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (x TEXT)\"\nglob = \"a/*.json\"\non-file = \"cat\"\n", + "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat\"\n", )); let b = cfg("[dirsql]\nignore = [\"c/**\"]\n"); let err = combine_configs(&[(src("/a"), a), (src("/b"), b)]).unwrap_err(); @@ -1080,10 +1077,10 @@ on-file = "cat {path}" #[test] fn combine_duplicate_table_name_detected_through_quoting() { let a = cfg( - "[[table]]\nname = 't'\nddl = 'CREATE TABLE \"t\" (x TEXT)'\nglob = \"a/*.json\"\non-file = \"cat {path}\"\n", + "[[table]]\nname = 't'\nddl = 'CREATE TABLE \"t\" (x TEXT)'\nglob = \"a/*.json\"\non-file = \"cat\"\n", ); let b = cfg( - "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat {path}\"\n", + "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat\"\n", ); let err = combine_configs(&[(src("/a"), a), (src("/b"), b)]).unwrap_err(); assert!( @@ -1095,10 +1092,10 @@ on-file = "cat {path}" #[test] fn combine_error_display_includes_package_source_verbatim() { let a = cfg( - "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (x TEXT)\"\nglob = \"a/*.json\"\non-file = \"cat {path}\"\n", + "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (x TEXT)\"\nglob = \"a/*.json\"\non-file = \"cat\"\n", ); let b = cfg( - "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat {path}\"\n", + "[[table]]\nname = \"t\"\nddl = \"CREATE TABLE t (y TEXT)\"\nglob = \"b/*.json\"\non-file = \"cat\"\n", ); let err = combine_configs(&[ (src("/proj/.dirsql.toml"), a), @@ -1113,13 +1110,13 @@ on-file = "cat {path}" #[test] fn combine_three_configs_concatenates_across_all() { let a = cfg( - "[dirsql]\nignore = [\"a/**\"]\n\n[[table]]\nname = \"a\"\nddl = \"CREATE TABLE a (x TEXT)\"\nglob = \"a/*\"\non-file = \"cat {path}\"\n", + "[dirsql]\nignore = [\"a/**\"]\n\n[[table]]\nname = \"a\"\nddl = \"CREATE TABLE a (x TEXT)\"\nglob = \"a/*\"\non-file = \"cat\"\n", ); let b = cfg( - "[[table]]\nname = \"b\"\nddl = \"CREATE TABLE b (x TEXT)\"\nglob = \"b/*\"\non-file = \"cat {path}\"\n", + "[[table]]\nname = \"b\"\nddl = \"CREATE TABLE b (x TEXT)\"\nglob = \"b/*\"\non-file = \"cat\"\n", ); let c = cfg( - "[dirsql]\nignore = [\"c/**\"]\n\n[[table]]\nname = \"c\"\nddl = \"CREATE TABLE c (x TEXT)\"\nglob = \"c/*\"\non-file = \"cat {path}\"\n", + "[dirsql]\nignore = [\"c/**\"]\n\n[[table]]\nname = \"c\"\nddl = \"CREATE TABLE c (x TEXT)\"\nglob = \"c/*\"\non-file = \"cat\"\n", ); let merged = combine_configs(&[(src("/a"), a), (src("/b"), b), (src("/c"), c)]).unwrap(); assert_eq!(merged.ignore, vec!["a/**", "c/**"]); diff --git a/packages/rust/src/db.rs b/packages/rust/src/db.rs index 328e4e34c..b1270dc75 100644 --- a/packages/rust/src/db.rs +++ b/packages/rust/src/db.rs @@ -144,18 +144,14 @@ fn no_home_path_table(name: &str) -> String { /// the stat columns, and the `path_prefix` is irrelevant (a parser wanting a /// path emits it). Both forms carry the same ignore rules. /// -/// 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, 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. +/// A parsed table also carries `index_root`, where the parser is spawned. The +/// stat module takes no such argument: it spawns nothing. fn path_table_ddl( name: &str, table: &PathTable, ignore: &[String], gitignore: bool, parser: Option<&str>, - cache: Option<&Path>, index_root: &Path, ) -> String { let (module, mut args) = match parser { @@ -182,11 +178,6 @@ fn path_table_ddl( scanner::NO_GITIGNORE_ARG })); if parser.is_some() { - args.push(quote_literal( - &cache - .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))); @@ -332,9 +323,6 @@ pub struct Db { /// parser instead of the stat columns: its rows and schema come from the /// command's output. `None` keeps the stat path-table behavior. path_table_parser: Option, - /// The persistent cache a parsed path-table reuses across runs. `None` for - /// an ephemeral index, which has nowhere to cache to. - path_table_cache: Option, /// Counts the worker round trips a query pays for. Shared with every /// declared function registered on `conn`; inert until a function is both /// declared and called. @@ -396,7 +384,6 @@ impl Db { path_table_ignore: default_path_table_ignore(), path_table_gitignore: true, path_table_parser: None, - path_table_cache: None, calls: CallReporter::new(), batched: Vec::new(), }) @@ -422,7 +409,6 @@ impl Db { path_table_ignore: default_path_table_ignore(), path_table_gitignore: true, path_table_parser: None, - path_table_cache: None, calls: CallReporter::new(), batched: Vec::new(), }) @@ -461,14 +447,6 @@ impl Db { self.path_table_parser = Some(command); } - /// Point every parsed path-table minted on this connection at the - /// persistent cache, so an unchanged file's rows are served from it - /// instead of re-running the parser. Set only when the index persists; - /// an ephemeral index has nowhere to cache to. - pub fn set_path_table_cache(&mut self, path: PathBuf) { - self.path_table_cache = Some(path); - } - /// Register the declared functions on the connection. Inert: no worker /// is spawned until a query calls one. pub(crate) fn register_functions( @@ -1197,7 +1175,6 @@ impl Db { &self.path_table_ignore, self.path_table_gitignore, self.path_table_parser.as_deref(), - self.path_table_cache.as_deref(), index_root, ))?; Ok(()) @@ -3125,7 +3102,6 @@ mod tests { &[], true, None, - None, Path::new("/root"), ); assert_eq!( @@ -3149,7 +3125,6 @@ mod tests { &[], true, None, - None, Path::new("/root"), ) .contains("'/var/log', '*.log', '/var/log', 'gitignore')"), @@ -3160,7 +3135,6 @@ mod tests { &[], true, None, - None, Path::new("/root"), ) ); @@ -3174,7 +3148,6 @@ mod tests { &[], false, None, - None, Path::new("/root"), ); assert!( @@ -3191,7 +3164,6 @@ mod tests { &["node_modules/**".to_string(), "*.tmp".to_string()], true, None, - None, Path::new("/root"), ); assert!( @@ -3207,48 +3179,13 @@ mod tests { &docs_path_table(), &[], true, - Some("cat {path}"), - None, + Some("cat"), Path::new("/root"), ); assert_eq!( ddl, "CREATE VIRTUAL TABLE IF NOT EXISTS temp.\"./docs/*.md\" \ - USING dirsql_parsed('/root', 'docs/*.md', 'cat {path}', 'gitignore', '', '/root')" - ); - } - - #[test] - fn path_table_ddl_parser_form_carries_the_cache_path() { - let ddl = path_table_ddl( - "./docs/*.md", - &docs_path_table(), - &[], - true, - Some("cat {path}"), - Some(Path::new("/cache/dirsql.db")), - Path::new("/root"), - ); - assert!( - ddl.ends_with("'cat {path}', 'gitignore', '/cache/dirsql.db', '/root')"), - "a persisted index points the parsed module at its cache, got: {ddl}" - ); - } - - #[test] - fn path_table_ddl_stat_form_takes_no_cache_path() { - let ddl = path_table_ddl( - "./docs/*.md", - &docs_path_table(), - &[], - true, - None, - Some(Path::new("/cache/dirsql.db")), - Path::new("/root"), - ); - assert!( - ddl.ends_with("'', 'gitignore')"), - "the stat form has nothing to cache, got: {ddl}" + USING dirsql_parsed('/root', 'docs/*.md', 'cat', 'gitignore', '/root')" ); } @@ -3264,14 +3201,13 @@ mod tests { &table, &["node_modules/**".to_string()], true, - Some("parse.py {path}"), - None, + Some("parse.py"), Path::new("/index"), ); assert!( ddl.ends_with( - "USING dirsql_parsed('/var/log', '*.log', 'parse.py {path}', 'gitignore', \ - '', '/index', 'node_modules/**')" + "USING dirsql_parsed('/var/log', '*.log', 'parse.py', 'gitignore', \ + '/index', 'node_modules/**')" ), "the parser form drops the path prefix, names the index root and keeps \ ignore rules, got: {ddl}" @@ -3286,7 +3222,6 @@ mod tests { &[], true, Some("sh -c 'echo hi'"), - None, Path::new("/root"), ); assert!( @@ -3298,19 +3233,8 @@ mod tests { #[test] fn set_path_table_parser_arms_the_parsed_module() { let mut db = Db::new().unwrap(); - db.set_path_table_parser("cat {path}".to_string()); - assert_eq!(db.path_table_parser.as_deref(), Some("cat {path}")); - } - - #[test] - fn set_path_table_cache_points_parsed_tables_at_the_cache() { - let mut db = Db::new().unwrap(); - assert_eq!(db.path_table_cache, None, "an ephemeral index has no cache"); - db.set_path_table_cache(PathBuf::from("/cache/dirsql.db")); - assert_eq!( - db.path_table_cache.as_deref(), - Some(Path::new("/cache/dirsql.db")) - ); + db.set_path_table_parser("cat".to_string()); + assert_eq!(db.path_table_parser.as_deref(), Some("cat")); } #[test] diff --git a/packages/rust/src/lib.rs b/packages/rust/src/lib.rs index 489ff2f9c..15ad54e91 100644 --- a/packages/rust/src/lib.rs +++ b/packages/rust/src/lib.rs @@ -27,7 +27,6 @@ pub mod launcher; pub mod matcher; mod on_file; #[doc(hidden)] -pub mod parsed_cache; #[doc(hidden)] pub mod parsed_vtab; #[doc(hidden)] @@ -120,8 +119,7 @@ const BATCH_OWNER: &str = ""; /// failure rather than only whichever came first. #[derive(Debug, Clone, PartialEq, Eq)] pub struct OnFileFailure { - /// Path relative to the scan root, or the table's name when its command - /// failed over the whole table. + /// Path relative to the scan root. pub path: String, /// The hook's error, as rendered by its `Display`. pub message: String, @@ -211,6 +209,9 @@ pub enum DirSqlError { #[error("{0}")] PathPlaceholder(String), + + #[error("table `{name}`: {message}")] + TableCommand { name: String, message: String }, } impl DirSqlError { @@ -997,7 +998,6 @@ impl DirSQL { hint_legacy_files_table, persist: persist_ctx.map(|ctx| PreparedPersist { db: ctx.db, - path: ctx.path, deleted, meta: ctx.expected_meta, meta_current: ctx.meta_current, @@ -1059,11 +1059,11 @@ impl DirSQL { } = prepared; let (mut db, persist_ready, needs_sweep) = match persist { - Some(p) => { - let mut db = p.db; - db.set_path_table_cache(p.path); - (db, Some((p.deleted, p.meta, p.meta_current)), p.needs_sweep) - } + Some(p) => ( + p.db, + Some((p.deleted, p.meta, p.meta_current)), + p.needs_sweep, + ), None => (Db::new()?, None, false), }; db.set_path_table_root(root.clone()); @@ -1316,10 +1316,8 @@ impl DirSQL { } } Err(message) => { - db.delete_rows_by_file(&table_name, BATCH_OWNER) - .map_err(map_db_error)?; - on_file_failures.push(OnFileFailure { - path: table_name.clone(), + return Err(DirSqlError::TableCommand { + name: table_name, message, }); } @@ -1739,7 +1737,6 @@ pub struct PreparedBuild { #[doc(hidden)] pub struct PreparedPersist { db: Db, - path: PathBuf, deleted: Vec<(String, String)>, meta: HashMap, /// Whether the cache already holds exactly `meta`. When it does the meta @@ -1754,9 +1751,6 @@ pub struct PreparedPersist { struct PersistContext { db: Db, - /// Where the cache lives, carried through so parsed path-tables can reuse - /// their rows from the same file. - path: PathBuf, /// Cached file bookkeeping keyed by `(rel_path, table_name)` — a file may /// be cached under several tables under fan-out. cached: HashMap<(String, String), CachedFile>, @@ -1832,7 +1826,6 @@ fn prepare_persist( Ok(PersistContext { db, - path, cached, // Compatible means every expected key matches; equal length makes it // equality, which is what lets the write be skipped outright. @@ -2169,8 +2162,8 @@ fn build_tables_from_config( /// Run a table's `on-file` command once over every matched file and parse its /// output into rows. The absolute paths are appended to the command's argv in -/// scan order; `{root}` is the index root. Any failure is the table's: the -/// scan records it and the table stays empty. +/// scan order; `{root}` is the index root. Any failure is the table's and +/// fails the build. fn run_on_files( command: &str, paths: &[PathBuf], @@ -3185,7 +3178,6 @@ mod internal_tests { ); let ctx = PersistContext { db: Db::new().unwrap(), - path: PathBuf::from("/unused/cache.db"), cached, expected_meta: HashMap::new(), meta_current: false, @@ -3222,7 +3214,6 @@ mod internal_tests { ); let ctx = PersistContext { db: Db::new().unwrap(), - path: PathBuf::from("/unused/cache.db"), cached, expected_meta: HashMap::new(), meta_current: false, @@ -3239,7 +3230,6 @@ mod internal_tests { let dir = TempDir::new().unwrap(); let ctx = PersistContext { db: Db::new().unwrap(), - path: PathBuf::from("/unused/cache.db"), cached: HashMap::new(), expected_meta: HashMap::new(), meta_current: false, @@ -4046,7 +4036,6 @@ mod internal_tests { ); let ctx = PersistContext { db: Db::new().unwrap(), - path: PathBuf::from("/unused/cache.db"), cached, expected_meta: HashMap::new(), meta_current: false, @@ -4080,7 +4069,6 @@ mod internal_tests { ); let ctx = PersistContext { db: Db::new().unwrap(), - path: PathBuf::from("/unused/cache.db"), cached, expected_meta: HashMap::new(), meta_current: false, @@ -4108,7 +4096,6 @@ mod internal_tests { ); let ctx = PersistContext { db: Db::new().unwrap(), - path: PathBuf::from("/unused/cache.db"), cached, expected_meta: HashMap::new(), meta_current: false, @@ -4654,7 +4641,6 @@ mod internal_tests { create_sidecar_tables(db.conn()).unwrap(); PreparedPersist { db, - path: PathBuf::from("/unused/cache.db"), deleted: deleted .iter() .map(|(path, table)| (path.to_string(), table.to_string())) diff --git a/packages/rust/src/parsed_cache.rs b/packages/rust/src/parsed_cache.rs deleted file mode 100644 index 4509bc9aa..000000000 --- a/packages/rust/src/parsed_cache.rs +++ /dev/null @@ -1,454 +0,0 @@ -//! The persistent row cache behind a parsed path-table. -//! -//! A parsed path-table ([`crate::parsed_vtab`]) materializes its rows by -//! running the `on-file` parser once per matched file. That is the expensive -//! step `--persist` exists to skip: with a cache, a file whose stat tuple has -//! not moved serves the payload the parser produced last time and the process -//! is never spawned. -//! -//! The cache lives in the same SQLite file the declared tables use -//! (`_dirsql_parsed_rows`) and is keyed by [`table_key`] — the identity of the -//! *table*, not of the query, so two path-tables over the same tree with -//! different parsers never read each other's rows. -//! -//! Staleness is decided by [`crate::persist::is_trusted`], the same racy-stat -//! rule the declared-table reconcile uses. - -use std::collections::HashMap; -use std::path::Path; - -use rusqlite::{Connection, params}; - -use crate::persist::{CachedFile, FILE_COLUMNS, FileStat, is_trusted, read_cached_file_row}; - -/// A cached parse: the file identity the payload was produced from, and the -/// payload itself (the parser's stdout, verbatim). -#[derive(Debug, Clone)] -pub struct CachedParse { - pub rel_path: String, - pub stat: FileStat, - pub content_hash: Option<[u8; 32]>, - pub snapshot_ns: i64, - pub payload: String, -} - -impl From<(CachedFile, String)> for CachedParse { - fn from((file, payload): (CachedFile, String)) -> Self { - Self { - rel_path: file.rel_path, - stat: file.stat, - content_hash: file.content_hash, - snapshot_ns: file.snapshot_ns, - payload, - } - } -} - -/// One file's parse, as handed to [`RowCache::commit`]. -pub struct Entry<'a> { - pub rel_path: &'a str, - pub stat: &'a FileStat, - pub content_hash: Option<[u8; 32]>, - pub snapshot_ns: i64, - pub payload: &'a str, -} - -/// The cache as the vtab sees it: read what a prior run left, record what this -/// one learned. A trait so the reuse logic can be unit-tested against a double -/// instead of a database. -pub trait RowCache { - /// Every cached parse for this table, keyed by root-relative path. - fn read(&self) -> rusqlite::Result>; - - /// Record `writes` and forget `deletes`, together. Callers skip this - /// entirely when both are empty, which is what leaves an unchanged run's - /// cache file byte-for-byte alone. - fn commit(&self, writes: &[Entry<'_>], deletes: &[&str]) -> rusqlite::Result<()>; -} - -/// The identity of a parsed path-table: everything that decides what its rows -/// are. A change to any of it (a different tree, glob, parser, or dirsql build) -/// yields a different key, so stale rows are never served — no invalidation -/// pass required. -pub fn table_key(root: &Path, glob: &str, command: &str) -> String { - let mut hasher = blake3::Hasher::new(); - for part in [ - crate::persist::SCHEMA_VERSION, - env!("CARGO_PKG_VERSION"), - &root.to_string_lossy(), - glob, - command, - ] { - hasher.update(part.as_bytes()); - hasher.update(b"\0"); - } - hasher.finalize().to_hex().to_string() -} - -/// Create the `_dirsql_parsed_rows` table if it is not already there. Called by -/// [`crate::persist::create_sidecar_tables`], which owns the cache's schema as -/// a whole. -pub fn create_table(conn: &Connection) -> rusqlite::Result<()> { - conn.execute_batch( - "CREATE TABLE IF NOT EXISTS _dirsql_parsed_rows ( - table_key TEXT NOT NULL, - rel_path TEXT NOT NULL, - size INTEGER NOT NULL, - mtime_ns INTEGER NOT NULL, - ctime_ns INTEGER NOT NULL, - inode INTEGER NOT NULL, - dev INTEGER NOT NULL, - content_hash BLOB, - snapshot_ns INTEGER NOT NULL, - payload TEXT NOT NULL, - PRIMARY KEY (table_key, rel_path) - );", - ) -} - -/// Forget every cached parse. Used when the whole cache is being discarded. -pub fn clear(conn: &Connection) -> rusqlite::Result<()> { - conn.execute("DELETE FROM _dirsql_parsed_rows", [])?; - Ok(()) -} - -/// Whether a cached parse can be served for the file `live` describes. -pub fn is_fresh( - cached: Option<&CachedParse>, - live: &FileStat, - hash: impl FnOnce() -> Option<[u8; 32]>, -) -> bool { - cached.is_some_and(|c| is_trusted(&c.stat, c.content_hash.as_ref(), c.snapshot_ns, live, hash)) -} - -/// One parsed path-table's slice of the cache, over a connection of its own. -/// The owning connection holds the same file open in WAL mode, which is what -/// makes the concurrent read-and-write safe; the busy timeout covers the moment -/// a checkpoint holds the write lock. -pub struct SqliteRowCache { - conn: Connection, - table_key: String, -} - -impl SqliteRowCache { - pub fn open(path: &Path, table_key: String) -> rusqlite::Result { - let conn = Connection::open(path)?; - conn.busy_timeout(std::time::Duration::from_secs(10))?; - Ok(Self { conn, table_key }) - } -} - -impl RowCache for SqliteRowCache { - fn read(&self) -> rusqlite::Result> { - let mut stmt = self.conn.prepare(&format!( - "SELECT {FILE_COLUMNS}, payload FROM _dirsql_parsed_rows WHERE table_key = ?1" - ))?; - let rows = stmt.query_map(params![self.table_key], |row| { - let file = read_cached_file_row(row, &self.table_key)?; - let payload: String = row.get(8)?; - Ok(CachedParse::from((file, payload))) - })?; - let mut out = HashMap::new(); - for row in rows { - let parse = row?; - out.insert(parse.rel_path.clone(), parse); - } - Ok(out) - } - - fn commit(&self, writes: &[Entry<'_>], deletes: &[&str]) -> rusqlite::Result<()> { - let tx = self.conn.unchecked_transaction()?; - for entry in writes { - let hash_blob: Option<&[u8]> = entry.content_hash.as_ref().map(|h| h.as_slice()); - self.conn.execute( - "INSERT INTO _dirsql_parsed_rows - (table_key, rel_path, size, mtime_ns, ctime_ns, inode, dev, - content_hash, snapshot_ns, payload) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10) - ON CONFLICT(table_key, rel_path) DO UPDATE SET - size = excluded.size, - mtime_ns = excluded.mtime_ns, - ctime_ns = excluded.ctime_ns, - inode = excluded.inode, - dev = excluded.dev, - content_hash = excluded.content_hash, - snapshot_ns = excluded.snapshot_ns, - payload = excluded.payload", - params![ - self.table_key, - entry.rel_path, - entry.stat.size, - entry.stat.mtime_ns, - entry.stat.ctime_ns, - entry.stat.inode, - entry.stat.dev, - hash_blob, - entry.snapshot_ns, - entry.payload, - ], - )?; - } - for rel_path in deletes { - self.conn.execute( - "DELETE FROM _dirsql_parsed_rows WHERE table_key = ?1 AND rel_path = ?2", - params![self.table_key, rel_path], - )?; - } - tx.commit() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - fn stat(size: i64, mtime_ns: i64) -> FileStat { - FileStat { - size, - mtime_ns, - ctime_ns: 5, - inode: 9, - dev: 3, - } - } - - /// A cache over an in-memory database with this module's table in it. - fn cache(table_key: &str) -> SqliteRowCache { - let cache = empty(table_key); - create_table(&cache.conn).unwrap(); - cache - } - - /// A cache over a database with no tables at all, for the error paths. - fn empty(table_key: &str) -> SqliteRowCache { - SqliteRowCache { - conn: Connection::open_in_memory().unwrap(), - table_key: table_key.to_string(), - } - } - - fn entry<'a>(rel_path: &'a str, stat: &'a FileStat, payload: &'a str) -> Entry<'a> { - Entry { - rel_path, - stat, - content_hash: None, - snapshot_ns: 1, - payload, - } - } - - #[test] - fn table_key_is_stable_for_the_same_table() { - let a = table_key(Path::new("/data"), "**/*.json", "cat {path}"); - let b = table_key(Path::new("/data"), "**/*.json", "cat {path}"); - assert_eq!(a, b); - } - - #[test] - fn table_key_separates_root_glob_and_parser() { - let base = table_key(Path::new("/data"), "**/*.json", "cat {path}"); - assert_ne!( - base, - table_key(Path::new("/other"), "**/*.json", "cat {path}") - ); - assert_ne!(base, table_key(Path::new("/data"), "**/*.md", "cat {path}")); - assert_ne!( - base, - table_key(Path::new("/data"), "**/*.json", "jq . {path}") - ); - } - - #[test] - fn create_table_is_idempotent() { - let conn = Connection::open_in_memory().unwrap(); - create_table(&conn).unwrap(); - create_table(&conn).unwrap(); - } - - #[test] - fn commit_and_read_round_trip() { - let cache = cache("k"); - let stat = stat(10, 100); - cache - .commit( - &[Entry { - rel_path: "a.json", - stat: &stat, - content_hash: Some([4u8; 32]), - snapshot_ns: 200, - payload: "[{}]", - }], - &[], - ) - .unwrap(); - - let cached = cache.read().unwrap(); - let parse = cached.get("a.json").unwrap(); - assert_eq!(parse.payload, "[{}]"); - assert_eq!(parse.stat, stat); - assert_eq!(parse.content_hash, Some([4u8; 32])); - assert_eq!(parse.snapshot_ns, 200); - assert_eq!(parse.rel_path, "a.json"); - } - - #[test] - fn commit_replaces_an_earlier_parse_of_the_same_file() { - let cache = cache("k"); - let (old, new) = (stat(1, 1), stat(2, 2)); - cache.commit(&[entry("a.json", &old, "[1]")], &[]).unwrap(); - cache.commit(&[entry("a.json", &new, "[2]")], &[]).unwrap(); - - let cached = cache.read().unwrap(); - assert_eq!(cached.len(), 1); - assert_eq!(cached["a.json"].payload, "[2]"); - assert_eq!(cached["a.json"].stat, new); - } - - #[test] - fn commit_deletes_the_named_files_only() { - let cache = cache("k"); - let stat = stat(1, 1); - cache - .commit( - &[entry("a.json", &stat, "[1]"), entry("b.json", &stat, "[2]")], - &[], - ) - .unwrap(); - - cache.commit(&[], &["a.json"]).unwrap(); - - let cached = cache.read().unwrap(); - assert_eq!(cached.len(), 1); - assert!(cached.contains_key("b.json")); - } - - #[test] - fn read_is_scoped_to_one_table_key() { - let mine = cache("k1"); - let stat = stat(1, 1); - mine.commit(&[entry("a.json", &stat, "[1]")], &[]).unwrap(); - - // A second table over the same database and the same file path. - let theirs = SqliteRowCache { - conn: Connection::open_in_memory().unwrap(), - table_key: "k2".to_string(), - }; - create_table(&theirs.conn).unwrap(); - theirs - .commit(&[entry("a.json", &stat, "[2]")], &[]) - .unwrap(); - - assert_eq!(mine.read().unwrap()["a.json"].payload, "[1]"); - assert_eq!(theirs.read().unwrap()["a.json"].payload, "[2]"); - } - - #[test] - fn read_drops_a_malformed_content_hash() { - let cache = cache("k"); - cache - .conn - .execute( - "INSERT INTO _dirsql_parsed_rows - (table_key, rel_path, size, mtime_ns, ctime_ns, inode, dev, - content_hash, snapshot_ns, payload) - VALUES ('k', 'a.json', 0, 0, 0, 0, 0, ?1, 0, '[]')", - params![&[1u8, 2][..]], - ) - .unwrap(); - assert_eq!(cache.read().unwrap()["a.json"].content_hash, None); - } - - #[test] - fn clear_forgets_every_cached_parse() { - let cache = cache("k"); - let stat = stat(1, 1); - cache.commit(&[entry("a.json", &stat, "[1]")], &[]).unwrap(); - - clear(&cache.conn).unwrap(); - - assert!(cache.read().unwrap().is_empty()); - } - - #[test] - fn read_propagates_a_missing_table_error() { - let err = empty("k").read().unwrap_err(); - assert!(err.to_string().contains("no such table"), "got: {err}"); - } - - #[test] - fn commit_propagates_a_write_error() { - let stat = stat(1, 1); - let err = empty("k") - .commit(&[entry("a.json", &stat, "[]")], &[]) - .unwrap_err(); - assert!(err.to_string().contains("no such table"), "got: {err}"); - } - - #[test] - fn commit_propagates_a_delete_error() { - let err = empty("k").commit(&[], &["a.json"]).unwrap_err(); - assert!(err.to_string().contains("no such table"), "got: {err}"); - } - - #[test] - fn clear_propagates_a_missing_table_error() { - let conn = Connection::open_in_memory().unwrap(); - let err = clear(&conn).unwrap_err(); - assert!(err.to_string().contains("no such table"), "got: {err}"); - } - - fn parse(stat: FileStat, hash: Option<[u8; 32]>, snapshot_ns: i64) -> CachedParse { - CachedParse { - rel_path: "a.json".to_string(), - stat, - content_hash: hash, - snapshot_ns, - payload: "[]".to_string(), - } - } - - #[test] - fn is_fresh_is_false_without_a_cached_parse() { - assert!(!is_fresh(None, &stat(1, 1), || None)); - } - - #[test] - fn is_fresh_trusts_an_unchanged_stat_outside_the_racy_window() { - let cached = parse(stat(1, 100), None, 200); - assert!(is_fresh(Some(&cached), &stat(1, 100), || { - panic!("must not hash outside the racy window") - })); - } - - #[test] - fn is_fresh_is_false_for_a_changed_stat() { - let cached = parse(stat(1, 100), None, 200); - assert!(!is_fresh(Some(&cached), &stat(2, 100), || None)); - } - - #[test] - fn is_fresh_hash_confirms_inside_the_racy_window() { - let hash = [7u8; 32]; - let cached = parse(stat(1, 100), Some(hash), 50); - assert!(is_fresh(Some(&cached), &stat(1, 100), || Some(hash))); - assert!(!is_fresh(Some(&cached), &stat(1, 100), || Some([8u8; 32]))); - } - - #[test] - fn open_reports_an_unopenable_path() { - let dir = tempfile::TempDir::new().unwrap(); - let missing = dir.path().join("no-such-dir").join("cache.db"); - assert!(SqliteRowCache::open(&missing, "k".to_string()).is_err()); - } - - #[test] - fn open_yields_a_usable_cache() { - let dir = tempfile::TempDir::new().unwrap(); - let path = dir.path().join("cache.db"); - let cache = SqliteRowCache::open(&path, "k".to_string()).unwrap(); - create_table(&cache.conn).unwrap(); - let stat = stat(1, 1); - cache.commit(&[entry("a.json", &stat, "[1]")], &[]).unwrap(); - assert_eq!(cache.read().unwrap().len(), 1); - } -} diff --git a/packages/rust/src/parsed_vtab.rs b/packages/rust/src/parsed_vtab.rs index 367951579..8fb136722 100644 --- a/packages/rust/src/parsed_vtab.rs +++ b/packages/rust/src/parsed_vtab.rs @@ -16,11 +16,7 @@ //! join over a parsed table quadratic in parser invocations, and re-inferring //! could change the schema out from under a prepared statement. //! -//! Under `--persist` that materialization is cached across *runs* too: a file -//! whose stat tuple has not moved serves the payload the parser produced last -//! time and the process is never spawned. See [`crate::parsed_cache`]. -use std::collections::{HashMap, HashSet}; use std::ffi::c_int; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -30,12 +26,10 @@ use rusqlite::vtab::Context; use rusqlite::{Connection, Error, Result}; use crate::Value; -use crate::infer::{JsonRow, cell, declared_schema, infer_schema, parse_rows}; +use crate::infer::{JsonRow, cell, declared_schema, infer_schema}; use crate::matcher::TableMatcher; use crate::on_file; -use crate::parsed_cache::{self, CachedParse, Entry, RowCache, SqliteRowCache}; -use crate::persist::{FileStat, hash_file, now_ns}; -use crate::scanner::{scan_glob, to_slash}; +use crate::scanner::scan_glob; use crate::vtab_scaffold::{self, StatementScope, TableSource}; /// SQL module name a parsed path-table is created with. @@ -47,44 +41,34 @@ pub fn load_module(conn: &Connection, scope: Arc) -> Result<()> } /// Number of module arguments that are not ignore patterns. -const FIXED_ARGS: usize = 6; +const FIXED_ARGS: usize = 5; /// The module's own arguments: scan root, glob pattern, parser command, the -/// gitignore switch, the persistent cache path (empty when the index is -/// ephemeral), the index root the parser is spawned in, and the skip rules -/// the scan applies (mirroring the stat path-table's ignore args). +/// gitignore switch, 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, glob: GlobSet, 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, the index root, then -/// any ignore patterns. +/// glob, parser, the gitignore switch, 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, - index_root, - ignore @ .., - ] = user_args.as_slice() + let [root, pattern, command, gitignore, 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, index root", + "root, glob, parser, gitignore switch, index root", user_args.len(), )); }; @@ -95,9 +79,6 @@ fn parse_module_args(args: &[&[u8]]) -> Result { glob: vtab_scaffold::compile_glob(pattern)?, command: command.clone(), gitignore: vtab_scaffold::parse_gitignore(gitignore)?, - // 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)?, }) @@ -106,8 +87,7 @@ fn parse_module_args(args: &[&[u8]]) -> Result { /// Run the parser over every matched file and concatenate the rows. /// /// `run` is injected so the fan-out and the ordering can be unit-tested without -/// spawning a process; `warn` is injected so the skip warnings can be captured. -/// Production passes a closure over [`run_parser`] and `|m| eprintln!("{m}")`. +/// spawning a process. Production passes a closure over [`run_parser`]. /// /// One run over every file, matching the `on-file` hook contract: the /// parser's output is the table's rows, so a run that fails or does not parse @@ -116,141 +96,10 @@ fn collect_rows(rel_paths: &[PathBuf], run: &RunParser<'_>) -> Result = dyn Fn(&[PathBuf]) -> std::result::Result, String> + 'a; -/// [`collect_rows`] with the persistent cache in front of the parser: when -/// every matched file's stat tuple matches its cached entry and no cached file -/// has gone, the table's cached rows are served and the parser never runs. -/// Any one change re-runs it over every file, since that is what it saw. -/// -/// Everything effectful is injected — `stat`, `hash`, `run`, `warn` — so the -/// reuse decision can be exercised without a filesystem or a child process. -/// `hash` is only ever called for a file the stat tuple alone cannot settle, -/// which is what keeps an unchanged tree free of file reads. -/// -/// The cache is only written when the scan actually changed something, which is -/// what lets an unchanged tree leave the cache file byte-for-byte alone. A -/// failed parse is the table's error and leaves the cache untouched. -/// -/// A failed *write* is warned about and swallowed. The rows are already correct -/// and the next run simply re-parses; failing the user's query over a lost -/// optimization would be the worse outcome. It is reachable: the cache file is -/// normally WAL, where this connection's write and the owning connection's read -/// coexist, but WAL is unavailable on some filesystems and there the two can -/// genuinely contend. -fn collect_rows_cached( - cache: &dyn RowCache, - rel_paths: &[PathBuf], - fs: &dyn ParsedFs, - run: &RunParser<'_>, - warn: &dyn Fn(&str), -) -> Result> { - let cached: HashMap = cache.read()?; - let snapshot_ns = now_ns(); - - let mut present: Vec = Vec::with_capacity(rel_paths.len()); - let mut changed: Vec<(String, FileStat)> = Vec::new(); - for rel_path in rel_paths { - let key = to_slash(rel_path); - let Some(live) = fs.stat(rel_path) else { - // The file vanished between the scan and the stat. Nothing to - // parse and nothing to cache; the next run decides afresh. - continue; - }; - if !parsed_cache::is_fresh(cached.get(&key), &live, || fs.hash(rel_path)) { - changed.push((key.clone(), live)); - } - present.push(PathBuf::from(key)); - } - - let live: HashSet = present.iter().map(|path| to_slash(path)).collect(); - let stale: Vec<&str> = cached - .keys() - .map(String::as_str) - .filter(|key| *key != TABLE_ENTRY && !live.contains(*key)) - .collect(); - - if changed.is_empty() - && stale.is_empty() - && let Some(table) = cached.get(TABLE_ENTRY) - { - return parse_rows(&table.payload).map_err(Error::ModuleError); - } - - let rows = collect_rows(&present, run)?; - let payload = serde_json::to_string(&rows).map_err(|e| Error::ModuleError(e.to_string()))?; - let mut writes: Vec> = changed - .iter() - .map(|(key, stat)| Entry { - rel_path: key, - stat, - content_hash: fs.hash(Path::new(key)), - snapshot_ns, - payload: "", - }) - .collect(); - let table_stat = FileStat { - size: 0, - mtime_ns: 0, - ctime_ns: 0, - inode: 0, - dev: 0, - }; - writes.push(Entry { - rel_path: TABLE_ENTRY, - stat: &table_stat, - content_hash: None, - snapshot_ns, - payload: &payload, - }); - if let Err(error) = cache.commit(&writes, &stale) { - warn(&cache_write_skip_message(&error)); - } - - Ok(rows) -} - -/// Warning for a cache the run could not update. Says what was lost (the reuse, -/// not the rows) so the reader knows this is a slow next run, not a wrong one. -fn cache_write_skip_message(error: &Error) -> String { - format!( - "dirsql: could not update the persist cache: {error}; rows are unaffected, the next run re-parses" - ) -} - -/// The filesystem questions the cached collection asks, injected so the reuse -/// decision is testable without a real tree. -trait ParsedFs { - /// The file's stat tuple, or `None` when it is gone. - fn stat(&self, rel_path: &Path) -> Option; - /// The file's content hash. Best-effort: `None` only costs a re-parse. - fn hash(&self, rel_path: &Path) -> Option<[u8; 32]>; -} - -/// The production [`ParsedFs`]: paths resolved against the scan root. -struct RootedFs<'a> { - root: &'a Path, -} - -impl ParsedFs for RootedFs<'_> { - fn stat(&self, rel_path: &Path) -> Option { - std::fs::metadata(self.root.join(rel_path)) - .ok() - .map(|meta| FileStat::from_metadata(&meta)) - } - - fn hash(&self, rel_path: &Path) -> Option<[u8; 32]> { - hash_file(&self.root.join(rel_path)).ok() - } -} - /// The error raised when the sample yields nothing to infer from. SQLite has /// no zero-column table, and inventing a placeholder column would make /// `SELECT *` mean something the parser never said. @@ -298,7 +147,6 @@ impl TableSource for ParsedTable { glob, command, gitignore, - cache, index_root, ignore, } = parse_module_args(args)?; @@ -308,15 +156,7 @@ impl TableSource for ParsedTable { // parsed `SELECT * FROM './'` doesn't drown in dependency trees. 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)?, - Some(path) => { - let key = parsed_cache::table_key(&root, &pattern, &command); - let cache = SqliteRowCache::open(path, key)?; - collect_rows_cached(&cache, &rel_paths, &RootedFs { root: &root }, &run, &warn)? - } - }; + let rows = collect_rows(&rel_paths, &run)?; let columns = infer_schema(&rows); if columns.is_empty() { @@ -348,6 +188,7 @@ impl TableSource for ParsedTable { #[cfg(test)] mod tests { use super::*; + use crate::infer::parse_rows; // Real vtab behavior over a real directory and a real parser process is // covered by `tests/schema_inference.rs` (unit-lint isolation); only the @@ -374,16 +215,15 @@ mod tests { let args = args_with(&[ b"'/tmp/notes/docs'", b"'**/*.json'", - b"'cat {path}'", + b"'cat'", b"'gitignore'", - b"''", b"'/tmp/notes'", ]); let parsed = parse_module_args(&args).unwrap(); assert_eq!(parsed.root, PathBuf::from("/tmp/notes/docs")); assert_eq!(parsed.pattern, "**/*.json"); - assert_eq!(parsed.command, "cat {path}"); + assert_eq!(parsed.command, "cat"); 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"))); @@ -391,7 +231,7 @@ mod tests { #[test] fn parse_module_args_accepts_unquoted_arguments() { - let args = args_with(&[b"/tmp/notes", b"**/*", b"cat", b"gitignore", b"", b"/tmp"]); + let args = args_with(&[b"/tmp/notes", b"**/*", b"cat", b"gitignore", b"/tmp"]); assert_eq!( parse_module_args(&args).unwrap().root, PathBuf::from("/tmp/notes") @@ -400,37 +240,16 @@ mod tests { #[test] fn parse_module_args_reads_the_gitignore_switch() { - let on = args_with(&[ - b"'/tmp'", - b"'**/*'", - b"'cat'", - b"'gitignore'", - b"''", - b"'/tmp'", - ]); + let on = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'gitignore'", b"'/tmp'"]); assert!(parse_module_args(&on).unwrap().gitignore); - let off = args_with(&[ - b"'/tmp'", - b"'**/*'", - b"'cat'", - b"'no-gitignore'", - b"''", - b"'/tmp'", - ]); + let off = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'no-gitignore'", 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"''", - b"'/tmp'", - ]); + let args = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'sometimes'", b"'/tmp'"]); let err = match parse_module_args(&args) { Err(err) => err, Ok(_) => panic!("an unknown switch must be rejected"), @@ -445,7 +264,6 @@ mod tests { b"'**/*'", b"'cat'", b"'gitignore'", - b"''", b"'/tmp'", b"'node_modules/**'", ]); @@ -456,24 +274,17 @@ 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"''", - b"'/tmp'", - ]); + let args = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'gitignore'", 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'", b"''"]); + let args = args_with(&[b"'/tmp'", b"'**/*'", b"'cat'", b"'gitignore'"]); let err = match parse_module_args(&args) { Err(err) => err, - Ok(_) => panic!("five arguments must be rejected"), + Ok(_) => panic!("four arguments must be rejected"), }; assert!( err.to_string().contains("at least"), @@ -483,46 +294,10 @@ mod tests { #[test] fn parse_module_args_rejects_an_invalid_glob() { - let args = args_with(&[ - b"'/tmp'", - b"'['", - b"'cat'", - b"'gitignore'", - b"''", - b"'/tmp'", - ]); + let args = args_with(&[b"'/tmp'", b"'['", b"'cat'", b"'gitignore'", b"'/tmp'"]); assert!(parse_module_args(&args).is_err()); } - #[test] - fn parse_module_args_reads_the_cache_path() { - let args = args_with(&[ - b"'/tmp'", - b"'**/*'", - b"'cat'", - b"'gitignore'", - b"'/cache/dirsql.db'", - b"'/tmp'", - ]); - assert_eq!( - parse_module_args(&args).unwrap().cache, - Some(PathBuf::from("/cache/dirsql.db")) - ); - } - - #[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"''", - b"'/tmp'", - ]); - assert_eq!(parse_module_args(&args).unwrap().cache, None); - } - #[test] fn parse_module_args_rejects_an_invalid_ignore_pattern() { let args = args_with(&[ @@ -530,19 +305,12 @@ mod tests { b"'**/*'", b"'cat'", b"'gitignore'", - b"''", b"'/tmp'", b"'['", ]); assert!(parse_module_args(&args).is_err()); } - fn ok( - payload: &'static str, - ) -> impl Fn(&[PathBuf]) -> std::result::Result, String> { - move |_| parse_rows(payload) - } - #[test] fn collect_rows_hands_every_file_to_one_run_in_scan_order() { let paths = vec![PathBuf::from("a.json"), PathBuf::from("b.json")]; @@ -612,6 +380,12 @@ mod tests { // references only `super::` items (plus pure std), matching the pattern // `command.rs` uses for the runner underneath. + fn ids(rows: &[JsonRow]) -> Vec { + rows.iter() + .map(|r| r.get("id").unwrap().as_i64().unwrap()) + .collect() + } + #[test] fn run_parser_returns_the_commands_rows() { let rows = run_parser( @@ -665,335 +439,4 @@ mod tests { .unwrap_err(); assert!(err.contains('7'), "the exit code is reported, got: {err}"); } - - /// A [`ParsedFs`] over a fixed table of stats and hashes, so the reuse - /// decision is exercised without a filesystem. `hashed` records every hash - /// asked for, which is how "an unchanged tree reads no files" is asserted. - #[derive(Default)] - struct FakeFs { - stats: HashMap, - hashes: HashMap, - hashed: std::cell::RefCell>, - } - - impl FakeFs { - fn with(paths: &[(&str, i64)]) -> Self { - let mut fs = Self::default(); - for (path, mtime_ns) in paths { - fs.stats.insert((*path).to_string(), stat(*mtime_ns)); - } - fs - } - } - - impl ParsedFs for FakeFs { - fn stat(&self, rel_path: &Path) -> Option { - self.stats - .get(&rel_path.to_string_lossy().to_string()) - .cloned() - } - - fn hash(&self, rel_path: &Path) -> Option<[u8; 32]> { - let key = rel_path.to_string_lossy().to_string(); - self.hashed.borrow_mut().push(key.clone()); - self.hashes.get(&key).copied() - } - } - - /// An in-memory [`RowCache`] that records what it was asked to commit, so - /// the unit under test is exercised without a database. - #[derive(Default)] - struct FakeCache { - entries: std::cell::RefCell>, - commits: std::cell::RefCell, - fail_read: bool, - fail_commit: bool, - } - - impl FakeCache { - /// Seed the cache as a prior run would have, with a snapshot far enough - /// ahead of the file's mtime to be outside the racy window. - fn seeded(files: &[(&str, i64)], rows: &str) -> Self { - let cache = Self::default(); - for (rel_path, mtime_ns) in files { - cache.put(rel_path, stat(*mtime_ns), None, mtime_ns + 1, ""); - } - cache.put(TABLE_ENTRY, stat(0), None, 1, rows); - cache - } - - fn put( - &self, - rel_path: &str, - stat: FileStat, - content_hash: Option<[u8; 32]>, - snapshot_ns: i64, - payload: &str, - ) { - self.entries.borrow_mut().insert( - rel_path.to_string(), - CachedParse { - rel_path: rel_path.to_string(), - stat, - content_hash, - snapshot_ns, - payload: payload.to_string(), - }, - ); - } - - fn payloads(&self) -> std::collections::BTreeMap { - self.entries - .borrow() - .iter() - .map(|(k, v)| (k.clone(), v.payload.clone())) - .collect() - } - } - - impl RowCache for FakeCache { - fn read(&self) -> rusqlite::Result> { - if self.fail_read { - return Err(rusqlite::Error::InvalidQuery); - } - Ok(self.entries.borrow().clone()) - } - - fn commit(&self, writes: &[Entry<'_>], deletes: &[&str]) -> rusqlite::Result<()> { - *self.commits.borrow_mut() += 1; - if self.fail_commit { - return Err(rusqlite::Error::InvalidQuery); - } - for entry in writes { - self.put( - entry.rel_path, - entry.stat.clone(), - entry.content_hash, - entry.snapshot_ns, - entry.payload, - ); - } - for rel_path in deletes { - self.entries.borrow_mut().remove(*rel_path); - } - Ok(()) - } - } - - fn stat(mtime_ns: i64) -> FileStat { - FileStat { - size: 3, - mtime_ns, - ctime_ns: 1, - inode: 2, - dev: 4, - } - } - - fn paths(names: &[&str]) -> Vec { - names.iter().map(PathBuf::from).collect() - } - - fn collect_cached( - cache: &FakeCache, - fs: &FakeFs, - rel_paths: &[PathBuf], - run: &RunParser<'_>, - ) -> (Vec, Vec) { - let warnings = std::cell::RefCell::new(Vec::new()); - let rows = collect_rows_cached(cache, rel_paths, fs, run, &|m| { - warnings.borrow_mut().push(m.to_string()) - }) - .unwrap(); - (rows, warnings.into_inner()) - } - - fn ids(rows: &[JsonRow]) -> Vec { - rows.iter() - .map(|r| r.get("id").unwrap().as_i64().unwrap()) - .collect() - } - - #[test] - fn collect_rows_cached_parses_a_file_the_cache_does_not_know() { - let cache = FakeCache::default(); - let fs = FakeFs::with(&[("a.json", 10)]); - - let (rows, warnings) = - collect_cached(&cache, &fs, &paths(&["a.json"]), &ok(r#"[{"id":1}]"#)); - - assert_eq!(ids(&rows), vec![1]); - assert!(warnings.is_empty(), "{warnings:?}"); - assert_eq!(cache.payloads()[TABLE_ENTRY], r#"[{"id":1}]"#); - assert!(cache.payloads().contains_key("a.json")); - } - - #[test] - fn collect_rows_cached_serves_an_unchanged_tree_without_parsing_or_reading_it() { - let cache = FakeCache::seeded(&[("a.json", 10)], r#"[{"id":9}]"#); - let fs = FakeFs::with(&[("a.json", 10)]); - - let (rows, _) = collect_cached(&cache, &fs, &paths(&["a.json"]), &|_| { - panic!("an unchanged tree must not reach the parser") - }); - - assert_eq!(ids(&rows), vec![9], "the cached rows are served"); - assert!( - fs.hashed.borrow().is_empty(), - "a file outside the racy window is not read: {:?}", - fs.hashed.borrow(), - ); - } - - #[test] - fn collect_rows_cached_leaves_the_cache_alone_when_nothing_changed() { - let cache = FakeCache::seeded(&[("a.json", 10)], r#"[{"id":9}]"#); - let fs = FakeFs::with(&[("a.json", 10)]); - - collect_cached(&cache, &fs, &paths(&["a.json"]), &ok("[]")); - - assert_eq!( - *cache.commits.borrow(), - 0, - "an unchanged scan writes nothing at all", - ); - } - - #[test] - fn collect_rows_cached_reruns_over_every_file_when_one_stat_moved() { - let cache = FakeCache::seeded(&[("a.json", 10), ("b.json", 10)], r#"[{"id":9}]"#); - let fs = FakeFs::with(&[("a.json", 20), ("b.json", 10)]); - - let seen = std::cell::RefCell::new(Vec::new()); - let (rows, _) = collect_cached(&cache, &fs, &paths(&["a.json", "b.json"]), &|rel| { - seen.borrow_mut().push(rel.to_vec()); - parse_rows(r#"[{"id":2}]"#) - }); - - assert_eq!(ids(&rows), vec![2]); - assert_eq!( - *seen.borrow(), - vec![paths(&["a.json", "b.json"])], - "the parser sees the whole table again", - ); - assert_eq!( - cache.payloads()[TABLE_ENTRY], - r#"[{"id":2}]"#, - "the cache learns the new rows", - ); - } - - #[test] - fn collect_rows_cached_reruns_when_a_cached_file_no_longer_matches() { - let cache = FakeCache::seeded(&[("a.json", 10), ("gone.json", 10)], r#"[{"id":9}]"#); - let fs = FakeFs::with(&[("a.json", 10)]); - - let (rows, _) = collect_cached(&cache, &fs, &paths(&["a.json"]), &ok(r#"[{"id":1}]"#)); - - assert_eq!(ids(&rows), vec![1]); - assert_eq!( - cache.payloads().keys().collect::>(), - vec![TABLE_ENTRY, "a.json"], - "the vanished file's entry is dropped", - ); - } - - #[test] - fn collect_rows_cached_leaves_out_a_file_that_vanished_before_the_stat() { - let cache = FakeCache::default(); - let fs = FakeFs::with(&[("a.json", 10)]); - - let (rows, warnings) = - collect_cached(&cache, &fs, &paths(&["ghost.json", "a.json"]), &|rel| { - assert_eq!( - rel, - paths(&["a.json"]), - "the vanished file is not handed over" - ); - parse_rows(r#"[{"id":1}]"#) - }); - - assert_eq!(ids(&rows), vec![1]); - assert!(warnings.is_empty(), "a race is not a parse failure"); - } - - #[test] - fn collect_rows_cached_propagates_a_parser_failure_and_leaves_the_cache_alone() { - let cache = FakeCache::default(); - let fs = FakeFs::with(&[("a.json", 10)]); - - let err = collect_rows_cached( - &cache, - &paths(&["a.json"]), - &fs, - &|_| Err("exit 7".to_string()), - &|_| {}, - ) - .unwrap_err(); - - assert!( - matches!(err, Error::ModuleError(ref m) if m == "exit 7"), - "got: {err}" - ); - assert_eq!(*cache.commits.borrow(), 0, "a failed run is not cached"); - } - - #[test] - fn collect_rows_cached_hash_confirms_a_file_inside_the_racy_window() { - // snapshot_ns == mtime_ns puts the entry inside the racy window, where - // the stat tuple alone cannot settle it. - let cache = FakeCache::seeded(&[], "[]"); - cache.put("a.json", stat(10), Some([3u8; 32]), 10, ""); - let mut fs = FakeFs::with(&[("a.json", 10)]); - fs.hashes.insert("a.json".to_string(), [3u8; 32]); - - collect_cached(&cache, &fs, &paths(&["a.json"]), &|_| { - panic!("a hash-confirmed file must not reach the parser") - }); - - assert_eq!(fs.hashed.borrow().len(), 1, "the file is hashed once"); - } - - #[test] - fn collect_rows_cached_warns_but_returns_rows_when_the_cache_cannot_be_written() { - let cache = FakeCache { - fail_commit: true, - ..FakeCache::default() - }; - let fs = FakeFs::with(&[("a.json", 10)]); - - let (rows, warnings) = - collect_cached(&cache, &fs, &paths(&["a.json"]), &ok(r#"[{"id":1}]"#)); - - assert_eq!(ids(&rows), vec![1], "the rows are correct regardless"); - assert_eq!(warnings.len(), 1, "{warnings:?}"); - assert!( - warnings[0].contains("next run re-parses"), - "the warning says what was lost, got: {}", - warnings[0], - ); - } - - #[test] - fn cache_write_skip_message_names_the_error_and_the_consequence() { - let message = cache_write_skip_message(&Error::SqliteSingleThreadedMode); - assert!(message.contains("persist cache"), "got: {message}"); - assert!(message.contains("rows are unaffected"), "got: {message}"); - } - - #[test] - fn collect_rows_cached_propagates_a_cache_read_failure() { - let cache = FakeCache { - fail_read: true, - ..FakeCache::default() - }; - let fs = FakeFs::with(&[("a.json", 10)]); - let err = - collect_rows_cached(&cache, &paths(&["a.json"]), &fs, &ok("[]"), &|_| {}).unwrap_err(); - assert!(matches!(err, Error::InvalidQuery), "got: {err}"); - } - - // RootedFs answers real filesystem questions, so the isolation rule keeps - // it out of the unit tier: `tests/persist_parsed_path_table.rs` exercises - // it against a real tree, the way `vtab.rs`'s `read_text` is covered. } diff --git a/packages/rust/src/persist.rs b/packages/rust/src/persist.rs index a450c511b..8bb73cd4e 100644 --- a/packages/rust/src/persist.rs +++ b/packages/rust/src/persist.rs @@ -229,8 +229,7 @@ pub fn create_sidecar_tables(conn: &Connection) -> rusqlite::Result<()> { snapshot_ns INTEGER NOT NULL, PRIMARY KEY (rel_path, table_name) );", - )?; - crate::parsed_cache::create_table(conn) + ) } /// Read all `_dirsql_meta` key/value pairs. @@ -429,10 +428,6 @@ pub fn drop_user_tables(conn: &Connection) -> rusqlite::Result<()> { // repopulates it; stale rows would duplicate/orphan state. conn.execute("DELETE FROM _dirsql_internal_rows", [])?; conn.execute("DELETE FROM _dirsql_internal_ranges", [])?; - // The parsed path-table rows are keyed by their own identity hash, not by - // the meta the caller just rejected, but they are cached data in a cache - // being discarded: keep the file one thing, wholly stale or wholly warm. - crate::parsed_cache::clear(conn)?; Ok(()) } diff --git a/packages/rust/tests/command_runner.rs b/packages/rust/tests/command_runner.rs index 1b75387f7..b34de59f4 100644 --- a/packages/rust/tests/command_runner.rs +++ b/packages/rust/tests/command_runner.rs @@ -27,8 +27,8 @@ fn substitutes_a_placeholder_and_reads_the_named_file() { let dir = TempDir::new().unwrap(); std::fs::write(dir.path().join("data.txt"), "row-a\nrow-b\n").unwrap(); let out = run_command( - "cat {path}", - &[Placeholder::new("path", "data.txt")], + "cat {file}", + &[Placeholder::new("file", "data.txt")], dir.path(), None, ) @@ -40,11 +40,11 @@ fn substitutes_a_placeholder_and_reads_the_named_file() { fn a_placeholder_the_template_omits_is_not_appended() { let dir = TempDir::new().unwrap(); std::fs::write(dir.path().join("data.txt"), "only-line\n").unwrap(); - // `cat` with no argument reads its (null) stdin: an omitted `{path}` is not + // `cat` with no argument reads its (null) stdin: an omitted `{file}` is not // appended, so there is no file to read and the run produces no payload. let err = run_command( "cat", - &[Placeholder::new("path", "data.txt")], + &[Placeholder::new("file", "data.txt")], dir.path(), None, ) @@ -155,7 +155,7 @@ fn a_missing_program_path_is_a_spawn_error_naming_the_program() { fn a_bare_name_missing_from_path_names_the_program_and_the_path_search() { let dir = TempDir::new().unwrap(); let err = run_command( - "dirsql-no-such-program-xyzzy {path}", + "dirsql-no-such-program-xyzzy {file}", &[Placeholder::new("path", "a.md")], dir.path(), None, @@ -181,7 +181,7 @@ fn a_bare_name_present_in_the_cwd_suggests_the_dot_slash_form() { std::fs::write(&script, "#!/bin/sh\necho '[]'\n").unwrap(); std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap(); - let err = run_command("dirsql-local-extract.sh {path}", &[], dir.path(), None) + let err = run_command("dirsql-local-extract.sh {file}", &[], dir.path(), None) .expect_err("a bare name is never resolved against the cwd"); let message = err.to_string(); assert!( diff --git a/packages/rust/tests/extensions.rs b/packages/rust/tests/extensions.rs index 369ac5168..9d2bcd2a9 100644 --- a/packages/rust/tests/extensions.rs +++ b/packages/rust/tests/extensions.rs @@ -33,7 +33,7 @@ on-file = "cat" "#, ) .unwrap(); - fs::write(root.path().join("a.txt"), "x").unwrap(); + fs::write(root.path().join("a.txt"), r#"[{"path":"a.txt"}]"#).unwrap(); let result = DirSQL::builder() .root(root.path()) @@ -126,7 +126,7 @@ on-file = "cat" ), ) .unwrap(); - fs::write(root.path().join("a.txt"), "x").unwrap(); + fs::write(root.path().join("a.txt"), r#"[{"path":"a.txt"}]"#).unwrap(); let db = DirSQL::builder() .root(root.path()) @@ -175,7 +175,7 @@ on-file = "cat" "#, ) .unwrap(); - fs::write(root.path().join("a.txt"), "x").unwrap(); + fs::write(root.path().join("a.txt"), r#"[{"path":"a.txt"}]"#).unwrap(); let db = DirSQL::builder() .root(root.path()) diff --git a/packages/rust/tests/multi_config.rs b/packages/rust/tests/multi_config.rs index c8593a6be..cdbf5bb3c 100644 --- a/packages/rust/tests/multi_config.rs +++ b/packages/rust/tests/multi_config.rs @@ -16,7 +16,7 @@ use std::fs; use std::path::Path; -use dirsql::{DirSQL, Value}; +use dirsql::{DirSQL, DirSqlError, Value}; use tempfile::TempDir; /// Write `.dirsql.toml` with `contents` into `dir` and return its path. @@ -138,10 +138,10 @@ on-file = "sh ./emit.sh" } #[test] -fn a_timeout_wrapped_hook_in_one_config_leaves_the_other_untouched() { +fn a_timeout_wrapped_hook_in_one_config_fails_the_build_under_its_table() { // Distinct globs (one-file-one-table). Config A wraps ITS slow hook in - // timeout(1): its table fails and stays empty. Config B's unwrapped fast - // hook is unaffected. + // timeout(1): the kill fails the build, and the error names A's table + // rather than config B's fast one. let data = TempDir::new().unwrap(); fs::write(data.path().join("a.json"), "{}").unwrap(); fs::write(data.path().join("b.json"), "{}").unwrap(); @@ -180,26 +180,18 @@ on-file = "sh ./fast.sh" "#, ); - let db = DirSQL::builder() + let err = DirSQL::builder() .root(data.path()) .config(&cfg_a_path) .config(&cfg_b_path) .build() - .expect("two config entries must both load"); + .err() + .expect("a hook killed by its timeout(1) wrapper fails the build"); - let slow = db - .query("SELECT v FROM slow") - .expect("the timed-out table must still exist"); - assert_eq!( - slow.len(), - 0, - "a hook killed by its timeout(1) wrapper leaves its table empty" + assert!( + matches!(&err, DirSqlError::TableCommand { name, .. } if name == "slow"), + "got: {err}" ); - - let fast = db - .query("SELECT v FROM fast") - .expect("the second config's table must be queryable"); - assert_eq!(fast[0]["v"], Value::Text("in-time".into())); } #[test] diff --git a/packages/rust/tests/no_implicit_files_table.rs b/packages/rust/tests/no_implicit_files_table.rs index 64c8e232c..f92f80665 100644 --- a/packages/rust/tests/no_implicit_files_table.rs +++ b/packages/rust/tests/no_implicit_files_table.rs @@ -91,7 +91,7 @@ fn a_configured_table_set_that_omits_files_gets_the_plain_error() { let config = root.path().join("dirsql.toml"); fs::write( &config, - "[[table]]\nname = \"posts\"\nddl = \"CREATE TABLE posts (path TEXT)\"\nglob = \"**/*.md\"\non-file = \"cat\"\n", + "[[table]]\nname = \"posts\"\nddl = \"CREATE TABLE posts (path TEXT)\"\nglob = \"**/*.md\"\non-file = \"printf '[]'\"\n", ) .unwrap(); diff --git a/packages/rust/tests/on_file.rs b/packages/rust/tests/on_file.rs index 87ed21673..9abc3a602 100644 --- a/packages/rust/tests/on_file.rs +++ b/packages/rust/tests/on_file.rs @@ -12,7 +12,7 @@ use std::fs; use std::sync::{Arc, Mutex}; -use dirsql::{DirSQL, Table, Value}; +use dirsql::{DirSQL, DirSqlError, Table, Value}; use tempfile::TempDir; /// An `on-file` command that reads the matched file (a JSON array of row @@ -242,12 +242,11 @@ on-file = "sh abscheck.sh" } /// The command runs once for the whole table, so its exit status is the -/// table's: a non-zero exit leaves the table empty and is reported under the -/// table's name with the command's stderr, while the scan itself succeeds. The -/// helper (kept out of the TOML to sidestep nested-quote parsing) fails when -/// any file it is handed contains `BOOM`. +/// table's: a non-zero exit fails the build, naming the table and carrying +/// the command's stderr. The helper (kept out of the TOML to sidestep +/// nested-quote parsing) fails when any file it is handed contains `BOOM`. #[test] -fn a_failing_command_fails_the_whole_table() { +fn a_failing_command_fails_the_build() { let root = TempDir::new().unwrap(); fs::write( root.path().join("extract.sh"), @@ -268,27 +267,30 @@ on-file = "sh extract.sh" fs::write(root.path().join("good.txt"), "fine\n").unwrap(); fs::write(root.path().join("bad.txt"), "BOOM\n").unwrap(); - let db = DirSQL::builder() - .root(root.path()) - .config(root.path().join(".dirsql.toml")) - .build() - .unwrap(); - let rows = db.query("SELECT name FROM items").unwrap(); - assert!(rows.is_empty(), "a failed table has no rows: {rows:?}"); + let err = build_err(&root); - let failures = db.scan_failures(); - assert_eq!(failures.len(), 1, "{failures:?}"); - assert_eq!(failures[0].path, "items"); + let DirSqlError::TableCommand { name, message } = err else { + panic!("a failed command is the table's error, got: {err}"); + }; + assert_eq!(name, "items"); assert!( - failures[0].message.contains("boom seen"), - "the failure carries the command's stderr: {}", - failures[0].message + message.contains("boom seen"), + "the failure carries the command's stderr: {message}" ); } -/// Output that is not a JSON array of objects fails the table the same way. +fn build_err(root: &TempDir) -> DirSqlError { + DirSQL::builder() + .root(root.path()) + .config(root.path().join(".dirsql.toml")) + .build() + .err() + .expect("a failed command must fail the build") +} + +/// Output that is not a JSON array of objects fails the build the same way. #[test] -fn malformed_output_fails_the_whole_table() { +fn malformed_output_fails_the_build() { let root = TempDir::new().unwrap(); fs::write( root.path().join("extract.sh"), @@ -308,29 +310,20 @@ on-file = "sh extract.sh" .unwrap(); fs::write(root.path().join("good.txt"), "GOOD\n").unwrap(); - let db = DirSQL::builder() - .root(root.path()) - .config(root.path().join(".dirsql.toml")) - .build() - .unwrap(); - let rows = db.query("SELECT name FROM items").unwrap(); - assert!(rows.is_empty(), "{rows:?}"); + let err = build_err(&root); - let failures = db.scan_failures(); - assert_eq!(failures.len(), 1, "{failures:?}"); - assert_eq!(failures[0].path, "items"); assert!( - failures[0].message.contains("not a JSON array of rows"), - "got: {}", - failures[0].message + matches!(&err, DirSqlError::TableCommand { name, message } + if name == "items" && message.contains("not a JSON array of rows")), + "got: {err}" ); } /// Bounding a hook is the command's job now: a `timeout(1)`-wrapped hook that -/// overruns its bound exits non-zero, which fails the table like any other +/// overruns its bound exits non-zero, which fails the build like any other /// hook failure. #[test] -fn a_timeout_wrapped_hook_that_overruns_fails_the_table() { +fn a_timeout_wrapped_hook_that_overruns_fails_the_build() { let root = TempDir::new().unwrap(); fs::write( root.path().join("slow.sh"), @@ -350,16 +343,11 @@ on-file = "timeout 0.5 sh slow.sh" .unwrap(); fs::write(root.path().join("a.txt"), "x\n").unwrap(); - // The scan must succeed; the killed hook's table has no rows. - let db = DirSQL::builder() - .root(root.path()) - .config(root.path().join(".dirsql.toml")) - .build() - .unwrap(); - let rows = db.query("SELECT name FROM items").unwrap(); + let err = build_err(&root); + assert!( - rows.is_empty(), - "a table whose timeout(1)-wrapped hook overruns must be empty, got {rows:?}" + matches!(&err, DirSqlError::TableCommand { name, .. } if name == "items"), + "got: {err}" ); } @@ -396,11 +384,11 @@ on-file = "sh slowish.sh" assert_eq!(rows[0]["name"], Value::Text("ok".into())); } -/// A row that fails strict normalization is the hook's mistake, not the -/// database's: the table it belongs to is reported and left empty, and the -/// build itself still succeeds. +/// A row that fails strict normalization is the hook's mistake, and the +/// command ran once for the whole table, so it fails the build under the +/// table's name. #[test] -fn a_strict_violation_fails_the_whole_table() { +fn a_strict_violation_fails_the_build() { let root = TempDir::new().unwrap(); fs::write( root.path().join("gen.sh"), @@ -421,18 +409,12 @@ on-file = "sh gen.sh" .unwrap(); fs::write(root.path().join("a.txt"), "fine\n").unwrap(); - let db = DirSQL::builder() - .root(root.path()) - .config(root.path().join(".dirsql.toml")) - .build() - .expect("one bad row must not fail the build"); - let rows = db.query("SELECT name FROM items").unwrap(); + let err = build_err(&root); assert!( - rows.is_empty(), - "no row lands when one is rejected: {rows:?}" + matches!(&err, DirSqlError::TableCommand { name, .. } if name == "items"), + "got: {err}" ); - assert_eq!(db.scan_failures()[0].path, "items"); } /// A scan attempts every matched file. One hook failure is that file's diff --git a/packages/rust/tests/on_file_e2e.rs b/packages/rust/tests/on_file_e2e.rs index 85f6d6abf..ffb9d84e1 100644 --- a/packages/rust/tests/on_file_e2e.rs +++ b/packages/rust/tests/on_file_e2e.rs @@ -286,7 +286,7 @@ on-file = "sh count.sh" } #[test] -fn a_table_whose_command_errors_is_empty_while_the_server_stays_up() { +fn a_table_whose_command_errors_leaves_the_server_unavailable() { let root = TempDir::new().unwrap(); fs::write( root.path().join("extract.sh"), @@ -316,9 +316,13 @@ on-file = "sh extract.sh" .json(&json!({"sql": "SELECT name FROM items"})) .send() .unwrap(); - assert_eq!(resp.status(), StatusCode::OK); - let body: Vec = resp.json().unwrap(); - assert!(body.is_empty(), "a failed table has no rows: {body:?}"); + assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); + let body: Value = resp.json().unwrap(); + let error = body["error"].as_str().unwrap_or_default(); + assert!( + error.contains("`items`"), + "the failed table is named in the error: {body}" + ); kill_and_wait(child); } diff --git a/packages/rust/tests/on_file_failure_e2e.rs b/packages/rust/tests/on_file_failure_e2e.rs new file mode 100644 index 000000000..9e3c09580 --- /dev/null +++ b/packages/rust/tests/on_file_failure_e2e.rs @@ -0,0 +1,118 @@ +//! End-to-end tests for what a run does when a table's command fails. +//! +//! These spawn the real compiled `dirsql` binary over a temp directory and +//! assert the three things only the process boundary can show: the exit code, +//! what reaches stdout, and what reaches stderr. Nothing is mocked (real +//! process, real filesystem, real SQLite, real command spawn). +//! +//! The contract under test: an `on-file` command runs once for its whole +//! table, so its failure is the table's and the table's failure is the run's. +//! The failed table is named on stderr with the command's own stderr, no rows +//! reach stdout, and the run exits non-zero like any other failed build. +//! +//! Gated behind `--features cli` (the `dirsql` bin needs it) and Unix (the +//! fixtures shell out to `sh`); the Rust CI test job runs on Linux. + +#![cfg(all(feature = "cli", unix))] + +use std::fs; +use std::process::Output; + +use assert_cmd::prelude::*; +use serde_json::json; +use tempfile::TempDir; + +/// A hook that exits non-zero when any file it is handed contains `BOOM`, and +/// otherwise emits one row. Kept in a script rather than inline TOML to +/// sidestep nested-quote parsing. +const EXTRACT: &str = "#!/bin/sh\nif grep -q BOOM \"$@\"; then echo \"cannot read $1\" >&2; exit 1; fi\nprintf '[{\"name\":\"ok\"}]'\n"; + +/// A hook that emits a row with an unexpected column alongside a good one. +/// Under `strict = true` that row fails normalization. +const STRICTGEN: &str = "#!/bin/sh\nprintf '[{\"name\":\"ok\"},{\"nope\":1}]'\n"; + +fn fixture(script: &str, config: &str) -> TempDir { + let root = TempDir::new().unwrap(); + fs::write(root.path().join("hook.sh"), script).unwrap(); + fs::write(root.path().join(".dirsql.toml"), config).unwrap(); + root +} + +const LENIENT_CONFIG: &str = r#" +[[table]] +name = "items" +ddl = "CREATE TABLE items (name TEXT)" +glob = "*.txt" +on-file = "sh hook.sh" +"#; + +const STRICT_CONFIG: &str = r#" +[[table]] +name = "items" +ddl = "CREATE TABLE items (name TEXT)" +glob = "*.txt" +strict = true +on-file = "sh hook.sh" +"#; + +fn query(root: &TempDir) -> Output { + std::process::Command::cargo_bin("dirsql") + .expect("binary must exist") + .arg("SELECT name FROM items ORDER BY name") + .arg("-c") + .arg(".dirsql.toml") + .current_dir(root.path()) + .output() + .expect("spawning `dirsql` failed") +} + +#[test] +fn a_clean_scan_exits_zero() { + let root = fixture(EXTRACT, LENIENT_CONFIG); + fs::write(root.path().join("a.txt"), "fine\n").unwrap(); + + let out = query(&root); + + assert_eq!(out.status.code(), Some(0), "{out:?}"); + assert_eq!( + serde_json::from_slice::(&out.stdout).unwrap(), + json!([{"name": "ok"}]) + ); +} + +#[test] +fn a_failed_command_fails_the_run() { + let root = fixture(EXTRACT, LENIENT_CONFIG); + fs::write(root.path().join("good.txt"), "fine\n").unwrap(); + fs::write(root.path().join("bad.txt"), "BOOM\n").unwrap(); + + let out = query(&root); + + assert_eq!(out.status.code(), Some(1), "{out:?}"); + assert!(out.stdout.is_empty(), "no rows reach stdout: {out:?}"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("`items`"), + "the failed table must be named on stderr: {stderr}" + ); + assert!( + stderr.contains("cannot read"), + "the command's stderr must be carried: {stderr}" + ); +} + +#[test] +fn a_strict_violation_fails_the_run() { + let root = fixture(STRICTGEN, STRICT_CONFIG); + fs::write(root.path().join("a.txt"), "fine\n").unwrap(); + + let out = query(&root); + + assert_eq!(out.status.code(), Some(1), "{out:?}"); + assert!(out.stdout.is_empty(), "no rows reach stdout: {out:?}"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("`items`"), + "the failed table must be named on stderr: {stderr}" + ); +} diff --git a/packages/rust/tests/persist_parsed_path_table.rs b/packages/rust/tests/persist_parsed_path_table.rs deleted file mode 100644 index 2cf1fdf6e..000000000 --- a/packages/rust/tests/persist_parsed_path_table.rs +++ /dev/null @@ -1,356 +0,0 @@ -//! Integration tests for the persistent cache behind a *parsed* path-table — -//! the `--on-file` form of `SELECT ... FROM './**/*.json'`. -//! -//! `docs/howto/persist.md` promises that `--persist` turns a restart into -//! "the difference between re-running and skipping expensive `on-file` -//! commands". These tests hold that promise to a path-table, the surface a -//! configless `dirsql --persist` actually queries: a second run over an -//! unchanged tree must re-run the parser for no file, must leave the cache -//! byte-for-byte alone, and must cost a fraction of the first. -//! -//! Real core, real SQLite, real temp-dir filesystem, real parser processes. - -use dirsql::DirSQL; -use std::fs; -use std::path::{Path, PathBuf}; -use std::time::{Duration, Instant, SystemTime}; -use tempfile::TempDir; - -/// Files in the synthetic corpus for the timing test. The parser forks once per -/// file it is handed, so this is sized to make parsing, not the fixed cost of a -/// run, the bulk of the cold run's work. -const CORPUS: usize = 4_000; - -/// The second run must cost no more than this share of the first. -const WARM_RATIO: f64 = 0.10; - -const SQL: &str = "SELECT id, tag FROM './**/*.json'"; - -/// The default parser body: hand every file's rows back as one array. -const ECHO_FILES: &str = "printf '['\nsep=''\nfor f; do\n body=$(cat \"$f\")\n body=${body#'['}\n body=${body%']'}\n printf '%s%s' \"$sep\" \"$body\"\n sep=','\ndone\nprintf ']'"; - -/// A temp tree, a cache path outside it, and a parser that counts its own runs. -struct Fixture { - root: TempDir, - side: TempDir, - cache: PathBuf, - counter: PathBuf, -} - -impl Fixture { - /// Write `count` deterministic JSON files, each a single-row payload the - /// parser can hand straight back. Seeded by index, never random, so two - /// runs are comparable. - fn with_corpus(count: usize) -> Self { - let root = TempDir::new().unwrap(); - let side = TempDir::new().unwrap(); - let fixture = Self { - cache: side.path().join("cache.db"), - counter: side.path().join("parses"), - root, - side, - }; - for i in 0..count { - let path = fixture.file(i); - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, payload(i, "v1")).unwrap(); - } - fixture - } - - fn file(&self, i: usize) -> PathBuf { - self.root - .path() - .join(format!("d{:02}", i % 50)) - .join(format!("f{i}.json")) - } - - /// Install a parser that records one byte per invocation before running - /// `body` over the files it was handed. Counting the parser's own runs is - /// the read instrumentation: a table served from the cache never reaches - /// it. - fn parser(&self, name: &str, body: &str) -> String { - let script = self.side.path().join(name); - fs::write(&script, format!("printf x >> \"$1\"\nshift\n{body}\n")).unwrap(); - format!("sh {} {}", shell_word(&script), shell_word(&self.counter)) - } - - /// Parser invocations since the last [`Self::reset_counter`]. - fn parses(&self) -> usize { - fs::metadata(&self.counter) - .map(|m| usize::try_from(m.len()).unwrap()) - .unwrap_or(0) - } - - fn reset_counter(&self) { - fs::write(&self.counter, b"").unwrap(); - } - - /// Run one query against a parsed path-table, timing the whole - /// build-and-query the way a `dirsql` invocation does. - fn run(&self, parser: &str, persist: bool) -> (Vec, Duration) { - let started = Instant::now(); - let mut builder = DirSQL::builder() - .root(self.root.path()) - .path_table_parser(parser); - if persist { - builder = builder.persist(Some(&self.cache)); - } - let db = builder.build().unwrap(); - let rows = db.query(SQL).unwrap(); - let elapsed = started.elapsed(); - - let mut out: Vec = rows - .iter() - .map(|r| format!("{:?}|{:?}", r["id"], r["tag"])) - .collect(); - out.sort(); - (out, elapsed) - } - - fn cache_size(&self) -> u64 { - fs::metadata(&self.cache).unwrap().len() - } - - fn cache_digest(&self) -> [u8; 32] { - *blake3::hash(&fs::read(&self.cache).unwrap()).as_bytes() - } -} - -fn shell_word(path: &Path) -> String { - shlex::try_quote(&path.to_string_lossy()) - .unwrap() - .into_owned() -} - -fn payload(i: usize, tag: &str) -> String { - format!( - "[{{\"id\":{i},\"tag\":\"{tag}\",\"body\":\"{}\"}}]", - "x".repeat(120 + i % 17) - ) -} - -/// Windows refuses `set_modified` on a handle opened read-only. -fn writable(path: &Path) -> fs::File { - fs::OpenOptions::new().write(true).open(path).unwrap() -} - -/// Push a file's mtime clear of the racy window so the stat tuple alone -/// decides whether it changed. -fn touch_into_the_future(path: &Path) { - let future = SystemTime::now() + Duration::from_secs(5); - writable(path).set_modified(future).unwrap(); -} - -/// Pin a file's mtime far enough ahead that every cache written during the -/// test predates it. That is the racy window: the cache write does not postdate -/// the file's mtime, so the stat tuple alone cannot settle whether the file -/// changed and the reuse decision falls through to the content hash. -fn pin_inside_the_racy_window(path: &Path) -> SystemTime { - let pinned = SystemTime::now() + Duration::from_secs(3600); - writable(path).set_modified(pinned).unwrap(); - pinned -} - -#[test] -fn unchanged_second_run_reuses_the_cache_instead_of_reparsing() { - let fx = Fixture::with_corpus(CORPUS); - let parser = fx.parser("parse.sh", ECHO_FILES); - eprintln!("corpus: {CORPUS} files"); - - let (cold_rows, cold) = fx.run(&parser, true); - assert!(fx.parses() >= 1, "the cold run reaches the parser"); - assert_eq!(cold_rows.len(), CORPUS); - - let size_before = fx.cache_size(); - let digest_before = fx.cache_digest(); - fx.reset_counter(); - - let (warm_rows, warm) = fx.run(&parser, true); - - assert_eq!( - fx.parses(), - 0, - "an unchanged tree must not re-run the parser", - ); - assert_eq!(warm_rows, cold_rows, "the warm run returns the same rows"); - assert_eq!( - fx.cache_size(), - size_before, - "an unchanged tree must not grow the cache", - ); - assert_eq!( - fx.cache_digest(), - digest_before, - "an unchanged tree must not rewrite the cache", - ); - let ratio = warm.as_secs_f64() / cold.as_secs_f64(); - assert!( - ratio <= WARM_RATIO, - "the warm run must cost at most {:.0}% of the cold one; \ - cold {cold:?}, warm {warm:?} ({:.1}%)", - WARM_RATIO * 100.0, - ratio * 100.0, - ); -} - -#[test] -fn a_single_changed_file_reparses_the_tree_once() { - let fx = Fixture::with_corpus(40); - let parser = fx.parser("parse.sh", ECHO_FILES); - let _ = fx.run(&parser, true); - fx.reset_counter(); - - let changed = fx.file(7); - fs::write(&changed, payload(7, "v2")).unwrap(); - touch_into_the_future(&changed); - - let (rows, _) = fx.run(&parser, true); - - assert_eq!(fx.parses(), 1, "the parser runs once over the whole tree"); - assert!( - rows.contains(&"Integer(7)|Text(\"v2\")".to_string()), - "the changed file's new rows are returned: {rows:?}", - ); - assert_eq!(rows.len(), 40, "every other file is still present"); -} - -#[test] -fn a_persisted_run_returns_what_a_non_persisted_run_does() { - let fx = Fixture::with_corpus(40); - let parser = fx.parser("parse.sh", ECHO_FILES); - - let (fresh, _) = fx.run(&parser, false); - let (cold, _) = fx.run(&parser, true); - let (warm, _) = fx.run(&parser, true); - - assert_eq!(cold, fresh, "a cold persisted run matches an ephemeral one"); - assert_eq!(warm, fresh, "a warm persisted run matches an ephemeral one"); -} - -#[test] -fn a_deleted_file_drops_out_of_the_cached_rows() { - let fx = Fixture::with_corpus(40); - let parser = fx.parser("parse.sh", ECHO_FILES); - let _ = fx.run(&parser, true); - fs::remove_file(fx.file(7)).unwrap(); - fx.reset_counter(); - - let (rows, _) = fx.run(&parser, true); - - assert_eq!(rows.len(), 39, "the deleted file's row is gone"); - assert_eq!(fx.parses(), 1, "the survivors are re-parsed in one run"); -} - -#[test] -fn a_new_file_reparses_the_tree_once() { - let fx = Fixture::with_corpus(40); - let parser = fx.parser("parse.sh", ECHO_FILES); - let _ = fx.run(&parser, true); - fx.reset_counter(); - - let added = fx.file(40); - fs::create_dir_all(added.parent().unwrap()).unwrap(); - fs::write(added, payload(40, "v1")).unwrap(); - let (rows, _) = fx.run(&parser, true); - - assert_eq!(fx.parses(), 1, "the parser runs once over the whole tree"); - assert_eq!(rows.len(), 41, "its rows join the others"); -} - -#[test] -fn a_changed_parser_command_invalidates_the_cached_rows() { - let fx = Fixture::with_corpus(20); - let _ = fx.run(&fx.parser("parse.sh", ECHO_FILES), true); - fx.reset_counter(); - - // Same tree, different parser: the cached rows describe the old command's - // output and must not be served for the new one. - let other = fx.parser("other.sh", "printf '[{\"id\":-1,\"tag\":\"other\"}]'"); - let (rows, _) = fx.run(&other, true); - - assert_eq!(fx.parses(), 1, "the new parser runs over the tree"); - assert!( - rows.iter().all(|r| r.ends_with("Text(\"other\")")), - "the new parser's rows are served, not the cached ones: {rows:?}", - ); -} - -#[test] -fn files_inside_the_racy_window_are_hash_confirmed_rather_than_reparsed() { - // Every file is pinned into the window, so no reuse decision here can be - // settled by the stat tuple: the table is reused only if every content - // hash confirms its file. A hash that cannot be computed cannot confirm, - // and the whole corpus goes back to the parser. - let fx = Fixture::with_corpus(8); - let parser = fx.parser("parse.sh", ECHO_FILES); - let pinned: Vec = (0..8) - .map(|i| pin_inside_the_racy_window(&fx.file(i))) - .collect(); - - let (cold_rows, _) = fx.run(&parser, true); - - assert_eq!(fx.parses(), 1, "the cold run parses the tree once"); - let now = SystemTime::now(); - assert!( - pinned.iter().all(|mtime| *mtime > now), - "the cold run must finish before the pinned mtimes, or the corpus is \ - not in the racy window and this test proves nothing", - ); - fx.reset_counter(); - - let (warm_rows, _) = fx.run(&parser, true); - - assert_eq!( - fx.parses(), - 0, - "a hash-confirmed file must not reach the parser", - ); - assert_eq!(warm_rows, cold_rows, "the warm run returns the same rows"); -} - -#[test] -fn content_that_leaves_the_stat_tuple_untouched_is_caught_by_the_hash() { - // The one edit the stat tuple cannot see: same size, same mtime, same - // inode, different bytes. Inside the racy window the content hash is the - // only thing standing between the query and a stale cached payload. - let fx = Fixture::with_corpus(8); - let parser = fx.parser("parse.sh", ECHO_FILES); - let edited = fx.file(3); - let pinned = pin_inside_the_racy_window(&edited); - - let (cold_rows, _) = fx.run(&parser, true); - assert!(cold_rows.contains(&"Integer(3)|Text(\"v1\")".to_string())); - let before = fs::metadata(&edited).unwrap(); - fx.reset_counter(); - - // `payload` is the same length for either tag, so rewriting in place leaves - // size, inode and device alone; restoring the mtime leaves the rest. - fs::write(&edited, payload(3, "v2")).unwrap(); - writable(&edited).set_modified(pinned).unwrap(); - - let after = fs::metadata(&edited).unwrap(); - assert_eq!( - after.len(), - before.len(), - "the edit must not change the size" - ); - assert_eq!( - after.modified().unwrap(), - before.modified().unwrap(), - "the edit must not change the mtime", - ); - assert_eq!( - after.created().ok(), - before.created().ok(), - "the edit must not change the creation time", - ); - - let (rows, _) = fx.run(&parser, true); - - assert_eq!(fx.parses(), 1, "the edit sends the tree back to the parser"); - assert!( - rows.contains(&"Integer(3)|Text(\"v2\")".to_string()), - "the edited file's new rows are returned, not its cached ones: {rows:?}", - ); -} diff --git a/packages/rust/tests/scan_failures.rs b/packages/rust/tests/scan_failures.rs index f9ba5cf28..731f0ce83 100644 --- a/packages/rust/tests/scan_failures.rs +++ b/packages/rust/tests/scan_failures.rs @@ -1,11 +1,12 @@ -//! Integration tests for the scan's record of the tables it could not fill. +//! Integration test for the scan's record of the files it could not index. //! -//! A hook failure is per-table, so a caller needs to know *which* tables are -//! empty because their command failed rather than inferring it from absent -//! rows. These build a real `DirSQL` over real temp files (the effectful -//! spawn path, kept out of colocated unit tests by the Rust isolation rule). +//! A clean scan over a config-declared table must report nothing, so a caller +//! can use emptiness as the signal. This builds a real `DirSQL` over real temp +//! files (the effectful spawn path, kept out of colocated unit tests by the +//! Rust isolation rule). //! -//! Unix-only: the fixtures shell out to `sh`. The Rust CI test job runs on Linux. +//! Unix-only: the fixture shells out to `printf`. The Rust CI test job runs on +//! Linux. #![cfg(unix)] use std::fs; @@ -13,47 +14,6 @@ use std::fs; use dirsql::DirSQL; use tempfile::TempDir; -/// A failed table is reachable from the built database under its name, with -/// the command's stderr, so a caller can report it rather than inferring an -/// empty table from missing rows. -#[test] -fn failed_tables_are_reported_on_the_built_database() { - let root = TempDir::new().unwrap(); - fs::write( - root.path().join("extract.sh"), - "#!/bin/sh\nif grep -q BOOM \"$@\"; then echo 'cannot read' >&2; exit 1; fi\nprintf '[{\"name\":\"ok\"}]'\n", - ) - .unwrap(); - fs::write( - root.path().join(".dirsql.toml"), - r#" -[[table]] -name = "items" -ddl = "CREATE TABLE items (name TEXT)" -glob = "*.txt" -on-file = "sh extract.sh" -"#, - ) - .unwrap(); - fs::write(root.path().join("good.txt"), "fine\n").unwrap(); - fs::write(root.path().join("bad.txt"), "BOOM\n").unwrap(); - - let db = DirSQL::builder() - .root(root.path()) - .config(root.path().join(".dirsql.toml")) - .build() - .unwrap(); - - let failed = db.scan_failures(); - assert_eq!(failed.len(), 1, "expected one failed table: {failed:?}"); - assert_eq!(failed[0].path, "items", "the failure names the table"); - assert!( - failed[0].message.contains("cannot read"), - "the failure carries the command's stderr: {failed:?}" - ); -} - -/// A clean scan reports nothing, so a caller can use emptiness as the signal. #[test] fn a_clean_scan_reports_no_skipped_files() { let root = TempDir::new().unwrap(); diff --git a/packages/rust/tests/scan_failures_e2e.rs b/packages/rust/tests/scan_failures_e2e.rs deleted file mode 100644 index b4c32c71d..000000000 --- a/packages/rust/tests/scan_failures_e2e.rs +++ /dev/null @@ -1,218 +0,0 @@ -//! End-to-end tests for what a scan does when a table's command fails. -//! -//! These spawn the real compiled `dirsql` binary over a temp directory and -//! assert the three things only the process boundary can show: the exit code, -//! what reaches stdout, and what reaches stderr. Nothing is mocked (real -//! process, real filesystem, real SQLite, real command spawn). -//! -//! The contract under test: a table whose `on-file` command fails is left -//! empty rather than fatal, the scan commits every other table, the failures -//! are named on stderr, and the run exits with a code that says "completed, -//! some tables failed" — distinct from both success and failure, so -//! `dirsql "SELECT …" | jq` under `set -e` can tell a partial index from a -//! broken run. -//! -//! Gated behind `--features cli` (the `dirsql` bin needs it) and Unix (the -//! fixtures shell out to `sh`); the Rust CI test job runs on Linux. - -#![cfg(all(feature = "cli", unix))] - -use std::fs; -use std::process::Output; - -use assert_cmd::prelude::*; -use serde_json::{Value, json}; -use tempfile::TempDir; - -/// The exit code for "the scan completed, but some tables failed". Distinct -/// from `1` so a caller can separate a partial index from a failed run; `23` -/// follows rsync's "partial transfer due to error". -const PARTIAL: i32 = 23; - -/// A hook that exits non-zero when any file it is handed contains `BOOM`, and -/// otherwise emits one row. Kept in a script rather than inline TOML to -/// sidestep nested-quote parsing. -const EXTRACT: &str = "#!/bin/sh\nif grep -q BOOM \"$@\"; then echo \"cannot read $1\" >&2; exit 1; fi\nprintf '[{\"name\":\"ok\"}]'\n"; - -/// A hook that emits a row with an unexpected column alongside a good one. -/// Under `strict = true` that row fails normalization. -const STRICTGEN: &str = "#!/bin/sh\nprintf '[{\"name\":\"ok\"},{\"nope\":1}]'\n"; - -fn fixture(script: &str, config: &str) -> TempDir { - let root = TempDir::new().unwrap(); - fs::write(root.path().join("hook.sh"), script).unwrap(); - fs::write(root.path().join(".dirsql.toml"), config).unwrap(); - root -} - -const LENIENT_CONFIG: &str = r#" -[[table]] -name = "items" -ddl = "CREATE TABLE items (name TEXT)" -glob = "*.txt" -on-file = "sh hook.sh" -"#; - -const STRICT_CONFIG: &str = r#" -[[table]] -name = "items" -ddl = "CREATE TABLE items (name TEXT)" -glob = "*.txt" -strict = true -on-file = "sh hook.sh" -"#; - -fn query(root: &TempDir) -> Output { - query_sql(root, "SELECT name FROM items ORDER BY name") -} - -fn query_sql(root: &TempDir, sql: &str) -> Output { - std::process::Command::cargo_bin("dirsql") - .expect("binary must exist") - .arg(sql) - .arg("-c") - .arg(".dirsql.toml") - .current_dir(root.path()) - .output() - .expect("spawning `dirsql` failed") -} - -fn stdout_rows(out: &Output) -> Value { - serde_json::from_slice(&out.stdout).unwrap_or_else(|e| { - panic!( - "stdout was not JSON ({e}): {:?}", - String::from_utf8_lossy(&out.stdout) - ) - }) -} - -#[test] -fn a_scan_with_no_failures_still_exits_zero() { - // The floor: introducing a partial code must not make ordinary runs - // non-zero. - let root = fixture(EXTRACT, LENIENT_CONFIG); - fs::write(root.path().join("a.txt"), "fine\n").unwrap(); - - let out = query(&root); - - assert_eq!( - out.status.code(), - Some(0), - "a clean scan exits 0, got {out:?}" - ); - assert_eq!(stdout_rows(&out), json!([{"name": "ok"}])); -} - -#[test] -fn a_failed_table_exits_with_the_partial_code() { - // Without a distinct code, `dirsql "SELECT …" | jq` cannot tell a complete - // index from one missing a whole table. - let root = fixture(EXTRACT, LENIENT_CONFIG); - fs::write(root.path().join("good.txt"), "fine\n").unwrap(); - fs::write(root.path().join("bad.txt"), "BOOM\n").unwrap(); - - let out = query(&root); - - assert_eq!( - out.status.code(), - Some(PARTIAL), - "a scan with a failed table must exit {PARTIAL}, got {out:?}" - ); - // stdout stays parseable: the table exists and is empty. - assert_eq!(stdout_rows(&out), json!([])); - let stderr = String::from_utf8_lossy(&out.stderr); - assert!( - stderr.contains("items"), - "the failed table must be named on stderr: {stderr}" - ); - assert!( - stderr.contains("cannot read"), - "the command's stderr must be carried: {stderr}" - ); -} - -#[test] -fn a_strict_violation_fails_the_table() { - // A rejected row is the hook's mistake, so it costs that table alone -- - // aborting here would lose every other table to one bad column. - let root = fixture(STRICTGEN, STRICT_CONFIG); - fs::write(root.path().join("a.txt"), "fine\n").unwrap(); - - let out = query(&root); - - assert_eq!( - out.status.code(), - Some(PARTIAL), - "a strict violation is one table's problem, not the scan's: {out:?}" - ); - assert_eq!( - stdout_rows(&out), - json!([]), - "no row of a rejected batch lands" - ); - let stderr = String::from_utf8_lossy(&out.stderr); - assert!( - stderr.contains("items"), - "the failed table must be named on stderr: {stderr}" - ); -} - -#[test] -fn many_failures_are_capped_with_a_count_of_the_rest() { - // One line per failing table does not scale: a config full of broken - // hooks should not bury the shell in output. - let config: String = (0..15) - .map(|index| { - format!( - "[[table]]\nname = \"bad{index:02}\"\nddl = \"CREATE TABLE bad{index:02} (name TEXT)\"\nglob = \"bad{index:02}.txt\"\non-file = \"sh hook.sh\"\n\n" - ) - }) - .collect(); - let root = fixture(EXTRACT, &config); - for index in 0..15 { - fs::write(root.path().join(format!("bad{index:02}.txt")), "BOOM\n").unwrap(); - } - - let out = query_sql(&root, "SELECT name FROM bad00"); - - assert_eq!(out.status.code(), Some(PARTIAL), "{out:?}"); - let stderr = String::from_utf8_lossy(&out.stderr); - let named = (0..15) - .filter(|index| stderr.contains(&format!("`bad{index:02}`"))) - .count(); - assert!( - named <= 10, - "at most 10 tables should be named individually, {named} were: {stderr}" - ); - assert!( - stderr.contains("and 5 more"), - "the remainder must be counted, not dropped: {stderr}" - ); -} - -#[test] -fn a_sqlite_error_still_fails_the_whole_run() { - // The split must be real: a hook's failure is per-table, but a broken table - // definition is not something a partial index can paper over. - let root = fixture( - EXTRACT, - r#" -[[table]] -name = "items" -ddl = "CREATE TABLE items (name TEXT" -glob = "*.txt" -on-file = "sh hook.sh" -"#, - ); - fs::write(root.path().join("a.txt"), "fine\n").unwrap(); - - let out = query(&root); - - let code = out.status.code(); - assert_ne!(code, Some(0), "a malformed DDL must not pass: {out:?}"); - assert_ne!( - code, - Some(PARTIAL), - "a DDL error is a failed run, not a partial one: {out:?}" - ); -} From 2a81faea5d62e6a29a3be5eb0c7d506c911e5980 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 2 Oct 2026 17:12:42 -0400 Subject: [PATCH 2/8] test(core): build the parsed-row fixture in place; lift the CLI entry points from the mutation gate --- packages/rust/src/parsed_vtab.rs | 6 ++++-- testing-conventions.toml | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/rust/src/parsed_vtab.rs b/packages/rust/src/parsed_vtab.rs index 8fb136722..64153c4b7 100644 --- a/packages/rust/src/parsed_vtab.rs +++ b/packages/rust/src/parsed_vtab.rs @@ -188,7 +188,6 @@ impl TableSource for ParsedTable { #[cfg(test)] mod tests { use super::*; - use crate::infer::parse_rows; // Real vtab behavior over a real directory and a real parser process is // covered by `tests/schema_inference.rs` (unit-lint isolation); only the @@ -317,7 +316,10 @@ mod tests { let seen = std::cell::RefCell::new(Vec::new()); let rows = collect_rows(&paths, &|rel| { seen.borrow_mut().push(rel.to_vec()); - parse_rows(r#"[{"i":1},{"i":2}]"#) + Ok(vec![ + JsonRow(vec![("i".into(), serde_json::json!(1))]), + JsonRow(vec![("i".into(), serde_json::json!(2))]), + ]) }) .unwrap(); diff --git a/testing-conventions.toml b/testing-conventions.toml index bbdfadbe1..79376aa46 100644 --- a/testing-conventions.toml +++ b/testing-conventions.toml @@ -128,6 +128,12 @@ rules = ["mutation"] 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" +[[rust.exempt]] +path = "src/cli/run.rs" +rules = ["mutation"] +lines = [351, 392] +reason = "run_default and run_query build a real index and drive stdout, stderr and the process exit; tests/on_file_failure_e2e.rs (a_clean_scan_exits_zero, a_failed_command_fails_the_run) and tests/repl_pipe_e2e.rs assert their codes through the built binary" + # Unit-only coverage floor: measured by `cargo llvm-cov --lib --features cli` # (the `--lib` scoping keeps the integration tier under packages/rust/tests/ out # of the number -- testing-conventions #269). `branch` runs under `--branch`, From 09af520c3c7f28bdbc00464de45098a97819368d Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 2 Oct 2026 17:15:19 -0400 Subject: [PATCH 3/8] test(sdk): drop the parsed-row cache e2e test and the exit-23 headers on the scan-failure bindings --- .../python/tests/e2e/persist_query_test.py | 133 ------------------ .../integration/binding/scan_failures_test.py | 10 +- .../integration/binding/scan-failures.test.ts | 11 +- 3 files changed, 8 insertions(+), 146 deletions(-) delete mode 100644 packages/python/tests/e2e/persist_query_test.py diff --git a/packages/python/tests/e2e/persist_query_test.py b/packages/python/tests/e2e/persist_query_test.py deleted file mode 100644 index 67c7c7430..000000000 --- a/packages/python/tests/e2e/persist_query_test.py +++ /dev/null @@ -1,133 +0,0 @@ -"""CLI e2e: `dirsql query ... --persist` over a parsed path-table -(docs/howto/persist.md). - -Runs the Python launcher (`dirsql.cli.main:main`) as a subprocess twice over the -same unchanged temp tree, with a real parser script that records every file it -is handed. The second run must serve the rows from the cache: the parser runs -for no file and the cache file is not rewritten. No mocks: real launcher, real -binary, real process, real filesystem. -""" - -import json -import os -import shutil -import subprocess -import sys - -_REPO_ROOT = os.path.abspath( - os.path.join(os.path.dirname(__file__), "..", "..", "..", "..") -) -_RELEASE = os.path.join(_REPO_ROOT, "target", "release", "dirsql") -_DEBUG = os.path.join(_REPO_ROOT, "target", "debug", "dirsql") -_BINARY = _RELEASE if os.path.exists(_RELEASE) else _DEBUG - -import dirsql as _dirsql_pkg - -_BINARY_STAGE_DIR = os.path.join(os.path.dirname(_dirsql_pkg.__file__), "_binary") - -_PARSER = """#!/bin/sh -counter=$1 -shift -printf '[' -sep="" -for f; do - printf x >> "$counter" - body=$(cat "$f") - body=${body#"["} - body=${body%"]"} - printf '%s%s' "$sep" "$body" - sep="," -done -printf ']' -""" - - -def describe_persist_query(): - import pytest - - @pytest.fixture - def tree(tmp_path): - assert os.path.exists(_BINARY), ( - f"dirsql binary not built at {_BINARY}; " - "run `cargo build -p dirsql --features cli` first" - ) - os.makedirs(_BINARY_STAGE_DIR, exist_ok=True) - staged = os.path.join(_BINARY_STAGE_DIR, "dirsql") - shutil.copy(_BINARY, staged) - os.chmod(staged, 0o755) - - root = tmp_path / "data" - (root / "docs").mkdir(parents=True) - for i in range(5): - (root / "docs" / f"a{i}.json").write_text( - json.dumps([{"id": i, "tag": "v1"}]) - ) - parser = tmp_path / "parse.sh" - parser.write_text(_PARSER) - os.chmod(parser, 0o755) - try: - yield root - finally: - shutil.rmtree(_BINARY_STAGE_DIR, ignore_errors=True) - - def _run(root, *args): - return subprocess.run( - [ - sys.executable, - "-c", - "import sys; from dirsql.cli.main import main; sys.exit(main())", - *args, - ], - cwd=str(root), - capture_output=True, - text=True, - ) - - def _query(root, counter): - return _run( - root, - "query", - "SELECT id, tag FROM './docs/*.json'", - "--on-file", - f"sh {root.parent / 'parse.sh'} {counter}", - "--persist", - ) - - def _parses(counter): - return os.path.getsize(counter) if os.path.exists(counter) else 0 - - def it_serves_the_second_run_from_the_cache(tree): - counter = tree.parent / "parses" - - cold = _query(tree, counter) - assert cold.returncode == 0, cold.stderr - assert _parses(counter) == 5, "the cold run parses every file once" - cold_rows = sorted(json.loads(cold.stdout), key=lambda r: r["id"]) - - cache = tree / ".dirsql" / "cache.db" - before = cache.read_bytes() - counter.write_bytes(b"") - - warm = _query(tree, counter) - assert warm.returncode == 0, warm.stderr - assert _parses(counter) == 0, "an unchanged tree must not re-run the parser" - assert sorted(json.loads(warm.stdout), key=lambda r: r["id"]) == cold_rows - assert cache.read_bytes() == before, ( - "an unchanged tree must not rewrite the cache" - ) - - def it_reparses_every_file_after_one_change(tree): - counter = tree.parent / "parses" - assert _query(tree, counter).returncode == 0 - counter.write_bytes(b"") - - changed = tree / "docs" / "a3.json" - changed.write_text(json.dumps([{"id": 3, "tag": "v2"}])) - future = os.path.getmtime(changed) + 5 - os.utime(changed, (future, future)) - - warm = _query(tree, counter) - assert warm.returncode == 0, warm.stderr - assert _parses(counter) == 5, "one changed file re-runs the parser over all" - rows = {r["id"]: r["tag"] for r in json.loads(warm.stdout)} - assert rows == {0: "v1", 1: "v1", 2: "v1", 3: "v2", 4: "v1"} diff --git a/packages/python/tests/integration/binding/scan_failures_test.py b/packages/python/tests/integration/binding/scan_failures_test.py index 08cb75764..f783a7b3d 100644 --- a/packages/python/tests/integration/binding/scan_failures_test.py +++ b/packages/python/tests/integration/binding/scan_failures_test.py @@ -1,9 +1,7 @@ -"""Binding-tier tests for the scan's record of files it skipped (#715). - -Since #714 a file whose ``on_file`` hook raises, or whose row the table -rejects, is skipped rather than failing the scan. The CLI reports those skips -on stderr and exits 23; a Python caller had no equivalent, so an incomplete -index was indistinguishable from a complete one -- the regression this closes. +"""Binding-tier tests for the scan's record of files a per-file ``on_file`` +hook skipped: one whose hook raises, or whose row the table rejects, is skipped +rather than failing the scan, and the caller can tell an incomplete index from +a complete one. """ import os diff --git a/packages/ts/tests/integration/binding/scan-failures.test.ts b/packages/ts/tests/integration/binding/scan-failures.test.ts index d6d56f663..71f9ee5a7 100644 --- a/packages/ts/tests/integration/binding/scan-failures.test.ts +++ b/packages/ts/tests/integration/binding/scan-failures.test.ts @@ -1,10 +1,7 @@ -// Binding-tier tests for the scan's record of files it skipped (#715). -// -// Since #714 a file whose `onFile` hook throws, or whose row the table -// rejects, is skipped rather than failing the scan. The CLI reports those -// skips on stderr and exits 23; a TypeScript caller had no equivalent, so an -// incomplete index was indistinguishable from a complete one -- the -// regression this closes. +// Binding-tier tests for the scan's record of files a per-file `onFile` hook +// skipped: one whose hook throws, or whose row the table rejects, is skipped +// rather than failing the scan, and the caller can tell an incomplete index +// from a complete one. import { writeFileSync } from "node:fs"; import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; From f3529be9e3597688d2c2da6cf08f46d70232b92c Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 2 Oct 2026 17:17:43 -0400 Subject: [PATCH 4/8] e2e attestation for claude/1216-on-file-cleanup --- .../e2e-attestations/claude-1216-on-file-cleanup.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 packages/python/e2e-attestations/claude-1216-on-file-cleanup.json diff --git a/packages/python/e2e-attestations/claude-1216-on-file-cleanup.json b/packages/python/e2e-attestations/claude-1216-on-file-cleanup.json new file mode 100644 index 000000000..e4426b951 --- /dev/null +++ b/packages/python/e2e-attestations/claude-1216-on-file-cleanup.json @@ -0,0 +1,7 @@ +{ + "command": "just test-e2e", + "ran_at": 1790975863, + "exit_code": 0, + "commit": "2aa0df818cd7140d9567930544cd2e5ad9346317", + "branch": "claude/1216-on-file-cleanup" +} From ee597952633c9c2a0fe88be3e2493b65686ad083 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 2 Oct 2026 17:17:54 -0400 Subject: [PATCH 5/8] e2e attestation for claude/1216-on-file-cleanup --- .../ts/e2e-attestations/claude-1216-on-file-cleanup.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 packages/ts/e2e-attestations/claude-1216-on-file-cleanup.json diff --git a/packages/ts/e2e-attestations/claude-1216-on-file-cleanup.json b/packages/ts/e2e-attestations/claude-1216-on-file-cleanup.json new file mode 100644 index 000000000..c03f19165 --- /dev/null +++ b/packages/ts/e2e-attestations/claude-1216-on-file-cleanup.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1790975873, + "exit_code": 0, + "commit": "a991a190f83623bbc0e6d35d511a1b4347d3026e", + "branch": "claude/1216-on-file-cleanup" +} From aca220b1da45e03e5ff612678b4cdca085dbdedf Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Sun, 4 Oct 2026 20:24:27 -0400 Subject: [PATCH 6/8] refactor(core): drop the cache argument from the parsed module's DDL after the rebase The index-root argument main added stays; the empty cache slot before it goes, so the schema_inference fixture and the persist doc comments stop describing the removed row cache. --- PARITY.md | 3 +-- packages/rust/src/parsed_vtab.rs | 3 +-- packages/rust/src/persist.rs | 22 ++++++++-------------- packages/rust/tests/schema_inference.rs | 5 ++--- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/PARITY.md b/PARITY.md index bcc52c58c..3dc95b44a 100644 --- a/PARITY.md +++ b/PARITY.md @@ -487,7 +487,7 @@ Real-core file map: | Async / ready | `async_dirsql_test.py` | `async_sdk.rs` | `index.test.ts` | | Watch events | `async_dirsql_test.py`, `docs_gaps_test.py` | `sdk.rs`, `watcher.rs`, `watch_relative_root.rs` | `watch.test.ts`, `index.test.ts` | | Config file | `from_config_test.py` | `from_config.rs`, `config.rs` | `from-config.test.ts` | -| Persistence | `persist_test.py` | `persist.rs`, `persist_parsed_path_table.rs` | `persist.test.ts` | +| Persistence | `persist_test.py` | `persist.rs` | `persist.test.ts` | | Extensions | `extensions_test.py`, `extension_package_test.py`, `config_extension_package_test.py` | `extensions.rs` | `extensions.test.ts`, `extension-package.test.ts`, `config-extension-package.test.ts` | | Quoted-identifier DDL (#204) | `table_name_resolution_test.py` | `table_name_resolution.rs` | (covered by `declared-table-name.test.ts`) | | Declared table `name` (#962) | `declared_table_name_test.py` | `declared_table_name.rs`, `declared_table_name_e2e.rs` | `declared-table-name.test.ts` | @@ -604,7 +604,6 @@ incl. #313). | Persist: `.dirsql/` excluded from walk | Y | Y | Y | | Persist: custom persist_path honored | Y | Y | Y | | Persist: unchanged run leaves cache byte-identical | Y | Y | Y | -| Persist: parsed path-table reuses cached rows (#825) | N/A — `--on-file` is a CLI flag, not an SDK surface | Y (`persist_parsed_path_table.rs`) | N/A — same | ### Extensions diff --git a/packages/rust/src/parsed_vtab.rs b/packages/rust/src/parsed_vtab.rs index 64153c4b7..573414544 100644 --- a/packages/rust/src/parsed_vtab.rs +++ b/packages/rust/src/parsed_vtab.rs @@ -63,8 +63,7 @@ struct ModuleArgs { fn parse_module_args(args: &[&[u8]]) -> Result { let user_args = vtab_scaffold::user_args(args); - let [root, pattern, command, gitignore, index_root, ignore @ ..] = user_args.as_slice() - else { + let [root, pattern, command, gitignore, index_root, ignore @ ..] = user_args.as_slice() else { return Err(vtab_scaffold::arity_error( MODULE_NAME, FIXED_ARGS, diff --git a/packages/rust/src/persist.rs b/packages/rust/src/persist.rs index 8bb73cd4e..ffb1bb861 100644 --- a/packages/rust/src/persist.rs +++ b/packages/rust/src/persist.rs @@ -117,12 +117,10 @@ pub fn hash_file(path: &Path) -> io::Result<[u8; 32]> { /// Whether a cached entry still describes the live file. /// -/// The racy-stat rule, in one place because both cache tiers (the declared -/// tables' `_dirsql_files` and the parsed path-tables' row cache) must decide -/// it identically: an unchanged stat tuple is trusted outright once the cache -/// write is known to postdate the file's mtime, and hash-confirmed when it is -/// not (the file could have changed within the same timestamp tick). A cached -/// entry with no hash cannot be confirmed, so it is not trusted. +/// The racy-stat rule: an unchanged stat tuple is trusted outright once the +/// cache write is known to postdate the file's mtime, and hash-confirmed when +/// it is not (the file could have changed within the same timestamp tick). A +/// cached entry with no hash cannot be confirmed, so it is not trusted. /// /// `hash` is lazy: it is only called for entries inside the racy window, which /// is what keeps the common path free of file reads. @@ -208,9 +206,8 @@ pub fn ensure_parent_dir(path: &Path) -> io::Result<()> { Ok(()) } -/// Create the `_dirsql_meta`, `_dirsql_files` and `_dirsql_parsed_rows` -/// sidecar tables (if they don't already exist). The parsed path-table's own -/// table is created by the module that owns its shape. +/// Create the `_dirsql_meta` and `_dirsql_files` sidecar tables (if they +/// don't already exist). pub fn create_sidecar_tables(conn: &Connection) -> rusqlite::Result<()> { conn.execute_batch( "CREATE TABLE IF NOT EXISTS _dirsql_meta ( @@ -307,16 +304,13 @@ pub fn meta_is_compatible( true } -/// The bookkeeping columns every cache tier stores per file, in the order +/// The bookkeeping columns `_dirsql_files` stores per file, in the order /// [`read_cached_file_row`] expects them selected. pub const FILE_COLUMNS: &str = "rel_path, size, mtime_ns, ctime_ns, inode, dev, content_hash, snapshot_ns"; /// Read one file's bookkeeping from a row whose first eight columns are -/// [`FILE_COLUMNS`]. Shared so the declared-table index (`_dirsql_files`) and -/// the parsed path-table row cache (`_dirsql_parsed_rows`) store and interpret -/// a file's identity identically; each supplies its own `table_name` (the -/// declared table, or the path-table's identity key). +/// [`FILE_COLUMNS`]. pub fn read_cached_file_row( row: &rusqlite::Row<'_>, table_name: &str, diff --git a/packages/rust/tests/schema_inference.rs b/packages/rust/tests/schema_inference.rs index f7ffc5a44..1a26206a0 100644 --- a/packages/rust/tests/schema_inference.rs +++ b/packages/rust/tests/schema_inference.rs @@ -21,12 +21,11 @@ const COMBINE_SCRIPT: &str = "#!/bin/sh\nprintf '['\nsep=''\nfor f; do\n body=$ /// The SQL that declares a vtab named `t` over `glob` under `dir`, parsed by /// 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. +/// named relative to it. 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('{0}', '{1}', 'sh combine.sh', 'gitignore', '', '{0}')", + "CREATE VIRTUAL TABLE t USING dirsql_parsed('{0}', '{1}', 'sh combine.sh', 'gitignore', '{0}')", dir.path().display(), glob, ) From 925026bf05b32b5f99961089cde13c354a8b10f3 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Sun, 4 Oct 2026 20:29:05 -0400 Subject: [PATCH 7/8] e2e attestation for claude/1216-on-file-cleanup --- .../python/e2e-attestations/claude-1216-on-file-cleanup.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/python/e2e-attestations/claude-1216-on-file-cleanup.json b/packages/python/e2e-attestations/claude-1216-on-file-cleanup.json index e4426b951..de6e0f0eb 100644 --- a/packages/python/e2e-attestations/claude-1216-on-file-cleanup.json +++ b/packages/python/e2e-attestations/claude-1216-on-file-cleanup.json @@ -1,7 +1,7 @@ { "command": "just test-e2e", - "ran_at": 1790975863, + "ran_at": 1791160145, "exit_code": 0, - "commit": "2aa0df818cd7140d9567930544cd2e5ad9346317", + "commit": "aca220b1da45e03e5ff612678b4cdca085dbdedf", "branch": "claude/1216-on-file-cleanup" } From 7f73adabaa67275534d874d0ba8b0cdd2ad2ce1e Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Sun, 4 Oct 2026 20:29:22 -0400 Subject: [PATCH 8/8] e2e attestation for claude/1216-on-file-cleanup --- packages/ts/e2e-attestations/claude-1216-on-file-cleanup.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ts/e2e-attestations/claude-1216-on-file-cleanup.json b/packages/ts/e2e-attestations/claude-1216-on-file-cleanup.json index c03f19165..1d5a0638f 100644 --- a/packages/ts/e2e-attestations/claude-1216-on-file-cleanup.json +++ b/packages/ts/e2e-attestations/claude-1216-on-file-cleanup.json @@ -1,7 +1,7 @@ { "command": "pnpm test:e2e", - "ran_at": 1790975873, + "ran_at": 1791160162, "exit_code": 0, - "commit": "a991a190f83623bbc0e6d35d511a1b4347d3026e", + "commit": "925026bf05b32b5f99961089cde13c354a8b10f3", "branch": "claude/1216-on-file-cleanup" }