diff --git a/Cargo.lock b/Cargo.lock index b62d59c84..762e6601d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -499,7 +499,7 @@ dependencies = [ [[package]] name = "dirsql-py-ext" -version = "0.2.7" +version = "0.0.1" dependencies = [ "dirsql", "pyo3", diff --git a/PARITY.md b/PARITY.md index 8f27a49b8..fde57d18a 100644 --- a/PARITY.md +++ b/PARITY.md @@ -516,12 +516,22 @@ those land. | CLI reached by spawning a bundled binary | N — retired (#738) | Y (`cargo install`) | N — retired (#739) | | Per-platform artifacts shipping the core | **1** (`.so` only) | 1 | **1** (addon only) | | `dirsql server` + Ctrl-C exit code | 0 | 0 | 0 | +| `--version` reports the installed artifact's version (#958) | Y — the wheel's, via the pyo3 crate | Y — the core crate's | Y — `package.json`'s, read by the launcher | **Parity restored.** All three languages now front the same `run_cli`: Rust is the core, and both bindings call it in-process rather than spawning a copy. Neither published package ships a standalone binary — `cargo install dirsql --features cli` remains the way to get one, and it is the same code. +**#958 restored parity on the version string**, which had rendered the core +crate's `CARGO_PKG_VERSION` — a literal only the crates.io lane rewrites, so +both bindings reported `0.2.7` forever. The core gained +`cli::run_cli_with_version(argv, version)` and each binding passes the version +of the artifact it ships inside. The plumbing is asymmetric because the +manifest a publisher rewrites differs per lane: pyo3's `run_cli(argv)` bakes in +its own crate version (the constant it also exports as `__version__`), napi's +`runCli(argv, version?)` takes one. Both are launcher-facing, not SDK API. + #819 removed the per-query timeout from one-shot `dirsql query` in the core (server mode's `query_timeout` / `408` pipeline is unchanged). This is a CLI-lane behavior change, not an SDK-surface one: all three channels front the @@ -556,7 +566,7 @@ both bindings against a real wheel / npm install. | Test Scenario | Python (`tests/e2e/`) | Rust (`tests/`) | TypeScript (`tests/e2e/`) | |----------------------------|--------|------|------------| -| `--version` exits 0 and prints the version | Y (`cli_version_test.py`) | Y (`cli_e2e.rs`) | Y (`internals/distcheck` `dirsql-distcheck node`, against the packed npm install) | +| `--version` exits 0 and prints the *installed artifact's* version (#958) | Y (`cli_version_test.py`; `dirsql-distcheck python` against the pip-installed wheel) | Y (`cli_e2e.rs`) | Y (`internals/distcheck` `dirsql-distcheck node`, against the packed npm install) | | Launcher starts server; `POST /query` over HTTP | Y (`extension_package_test.py`) | Y | Y (`extension-package.test.ts`) | | `[[dirsql.extension]]` package name resolved by the launcher (#227) | Y | N/A | Y | | `interpret` subcommand removed; argv forwarded to clap (#321) | Y | core (clap dispatch) | Y | diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 942ac21d6..fccec0085 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -71,7 +71,7 @@ Config flags are subcommand-local: pass them after `server` | `--persist []` | off | Keep the SQLite index on disk between runs so a restart only re-parses files that actually changed. Bare `--persist` caches at `/.dirsql/cache.db`; `--persist ` caches at ``. Off by default (the index is ephemeral). Also available on [`dirsql query`](#dirsql-query). See [Keep the index across restarts](../howto/persist.md). | | `--no-ignore` | off | Scan files a `.gitignore` would hide. [Path-tables](./path-tables.md#skip-rules) respect `.gitignore` files by default; this flag restores the full walk. The built-in skips (`node_modules`/`.git`) and configured `ignore` patterns still apply. Also available on [`dirsql query`](#dirsql-query). | | `--extension ` | none | Load a SQLite extension by literal path, overriding the config's `[[dirsql.extension]]` entries. Repeatable. Format: `` or `::`. Internal plumbing for the pip/npm launchers, which resolve package-name extensions and pass the resolved paths here — not intended for direct use. When any `--extension` is present, the config file's own extension entries are not loaded. | -| `--version` | | Print the version and exit. | +| `--version` | | Print the version and exit. The version printed is that of the package you installed — the PyPI wheel, the npm package, or the crates.io binary. | | `--help` | | Print usage and exit. | ### Defaults diff --git a/internals/distcheck/src/distcheck/node_flow/gate.py b/internals/distcheck/src/distcheck/node_flow/gate.py index cfc3e06ea..32dcd2b34 100644 --- a/internals/distcheck/src/distcheck/node_flow/gate.py +++ b/internals/distcheck/src/distcheck/node_flow/gate.py @@ -145,8 +145,22 @@ def run( f"stdout={version.stdout!r}, stderr={version.stderr!r}" ) _require_zero(version, version_err) - if "dirsql" not in version.stdout: - raise DistcheckError(version_err) + + # Matching on the word "dirsql" alone let dirsql#958 ship: packages + # published at 0.4.x printed the embedded core crate's literal (0.2.7). + installed_main = json.loads( + fs.read_text( + os.path.join(install_root, "node_modules", "dirsql", "package.json") + ) + ) + installed_version = installed_main.get("version") + if version.stdout.strip() != f"dirsql {installed_version}": + raise DistcheckError( + f"`dirsql --version` printed {version.stdout.strip()!r}, expected " + f"'dirsql {installed_version}' -- the version npm installed. The " + "launcher (packages/ts/src/cli/main.ts) must read it from its own " + "package.json and pass it to the addon's `runCli`." + ) # 5. Cross-check the published layout: the launcher resolves to a real # addon inside the installed `@dirsql/lib-` sub-package, and no diff --git a/internals/distcheck/src/distcheck/node_flow/gate_test.py b/internals/distcheck/src/distcheck/node_flow/gate_test.py index 9cf3e45c1..0d69fd614 100644 --- a/internals/distcheck/src/distcheck/node_flow/gate_test.py +++ b/internals/distcheck/src/distcheck/node_flow/gate_test.py @@ -34,13 +34,17 @@ class _Host: _STAGED = "/ts/build/linux-x64-gnu/dirsql.linux-x64-gnu.node" _BIN = "/inst/node_modules/.bin/dirsql" _CLI_PKG = "/inst/node_modules/@dirsql/lib-linux-x64-gnu" +_MAIN_PKG_JSON = "/inst/node_modules/dirsql/package.json" +# The version npm installed -- `_MAIN_TGZ`'s, since that is the tarball +# installed above. +_INSTALLED = "0.0.1" def _res(rc=0, stdout="", stderr=""): return mock.Mock(returncode=rc, stdout=stdout, stderr=stderr) -def _fs(exists=True, read_name="@dirsql/lib-linux-x64-gnu"): +def _fs(exists=True, read_name="@dirsql/lib-linux-x64-gnu", installed=_INSTALLED): fs = mock.Mock() fs.mkdtemp.side_effect = ["/stg", "/inst"] fs.listdir.side_effect = [[_CLI_TGZ], [_CLI_TGZ, _MAIN_TGZ]] @@ -51,12 +55,18 @@ def _fs(exists=True, read_name="@dirsql/lib-linux-x64-gnu"): # package is installed, which the gate now treats as a failure; the # happy path is "everything present EXCEPT that". fs.exists.side_effect = lambda p: exists and "@dirsql/cli-" not in p - fs.read_text.return_value = json.dumps({"name": read_name}) + # Two installed manifests are read: the main package (for the version the + # CLI must report) and the addon sub-package (for its name). + fs.read_text.side_effect = lambda p: json.dumps( + {"version": installed} if p == _MAIN_PKG_JSON else {"name": read_name} + ) return fs def _ok_sequence(): - return [_res(), _res(), _res(), _res(stdout="dirsql 1.0")] + # The version stdout carries the trailing newline a real process emits, so + # the gate's `.strip()` is load-bearing rather than incidental. + return [_res(), _res(), _res(), _res(stdout=f"dirsql {_INSTALLED}\n")] def test_select_tarball_finds_the_cli_tarball(): @@ -185,10 +195,33 @@ def test_run_version_nonzero_exit(): run("/ts", _HOST, runner=runner, fs=fs) +def test_run_version_disagrees_with_the_installed_package(): + # dirsql#958: the CLI printed the core crate's literal, not the version + # npm installed. A "does it say dirsql" check passes that; this must not. + fs = _fs() + runner = mock.Mock( + side_effect=[_res(), _res(), _res(), _res(stdout="dirsql 0.2.7\n")] + ) + with pytest.raises(DistcheckError, match="expected 'dirsql 0.0.1'"): + run("/ts", _HOST, runner=runner, fs=fs) + + +def test_run_version_older_than_the_installed_package(): + # A stale version can sort either side of the installed one; both are a + # mismatch, and only testing one direction leaves an ordering comparison + # indistinguishable from the equality this asserts. + fs = _fs() + runner = mock.Mock( + side_effect=[_res(), _res(), _res(), _res(stdout="dirsql 0.0.0\n")] + ) + with pytest.raises(DistcheckError, match="expected 'dirsql 0.0.1'"): + run("/ts", _HOST, runner=runner, fs=fs) + + def test_run_version_missing_marker(): fs = _fs() runner = mock.Mock(side_effect=[_res(), _res(), _res(), _res(stdout="nope")]) - with pytest.raises(DistcheckError, match="--version` failed"): + with pytest.raises(DistcheckError, match="expected 'dirsql 0.0.1'"): run("/ts", _HOST, runner=runner, fs=fs) diff --git a/internals/distcheck/src/distcheck/python_flow/gate.py b/internals/distcheck/src/distcheck/python_flow/gate.py index 4cd7a0390..b8fd48d1a 100644 --- a/internals/distcheck/src/distcheck/python_flow/gate.py +++ b/internals/distcheck/src/distcheck/python_flow/gate.py @@ -50,6 +50,15 @@ def sole_wheel(names) -> str: return wheel +def wheel_version(wheel: str) -> str: + """The version segment of a wheel filename (`dirsql---...`). + + What pip actually installed -- the anchor the CLI and the SDK must both + agree with. + """ + return wheel.split("-")[1] + + def check_wheel_tag(wheel: str) -> None: """Assert the stable-ABI (abi3) tag (#487): one `cp3x-abi3` wheel per platform, not a version-locked `cpXY-cpXY` that re-inflates the release @@ -101,6 +110,8 @@ def run( ) _require_zero(install, f"pip install failed:\n{install.stderr}") + installed = wheel_version(wheel_name) + cli = os.path.join(venv_bin, "dirsql") if not fs.exists(cli): raise DistcheckError(f"console script missing at {cli}") @@ -115,8 +126,15 @@ def run( f"stdout={version.stdout!r}, stderr={version.stderr!r}" ) _require_zero(version, version_err) - if "dirsql" not in version.stdout: - raise DistcheckError(version_err) + # Matching on the word "dirsql" alone let dirsql#958 ship: wheels + # published at 0.4.x printed the embedded core crate's literal (0.2.7). + if version.stdout.strip() != f"dirsql {installed}": + raise DistcheckError( + f"`dirsql --version` printed {version.stdout.strip()!r}, expected " + f"'dirsql {installed}' -- the version pip installed. Have " + "`run_cli` in packages/python/src/lib.rs pass this crate's " + "`CARGO_PKG_VERSION` to `dirsql::cli::run_cli_with_version`." + ) # cwd is the scratch staging dir so `import dirsql` resolves the # installed wheel, never the source tree. diff --git a/internals/distcheck/src/distcheck/python_flow/gate_test.py b/internals/distcheck/src/distcheck/python_flow/gate_test.py index 950458d9e..372a7c60e 100644 --- a/internals/distcheck/src/distcheck/python_flow/gate_test.py +++ b/internals/distcheck/src/distcheck/python_flow/gate_test.py @@ -16,6 +16,7 @@ check_wheel_tag, run, sole_wheel, + wheel_version, ) _WHEEL = "dirsql-1.0-cp311-abi3-linux_x86_64.whl" @@ -37,11 +38,13 @@ def _fs(staging="/stg", listdir=None, exists=True): def _ok_sequence(): + # Both version stdouts carry the trailing newline a real process emits, so + # the gate's `.strip()` is load-bearing rather than incidental. return [ _res(), # maturin build _res(), # venv _res(), # pip install - _res(stdout="dirsql 1.0"), # --version + _res(stdout="dirsql 1.0\n"), # --version, matching _WHEEL's version _res(stdout="1.0\n"), # import dirsql ] @@ -79,6 +82,10 @@ def test_sole_wheel_rejects_many(): sole_wheel(["a.whl", "b.whl"]) +def test_wheel_version_reads_the_version_segment(): + assert wheel_version(_WHEEL) == "1.0" + + def test_check_wheel_tag_accepts_abi3_cpython(): check_wheel_tag(_WHEEL) # no raise @@ -172,17 +179,30 @@ def test_run_version_nonzero_exit(): run("/pkg", "/repo", runner=runner, fs=fs) +def test_run_version_disagrees_with_the_installed_wheel(): + # dirsql#958: the CLI printed the core crate's literal, not the version pip + # installed. A "does it say dirsql" check passes that; this must not. Both + # orderings, so an ordering comparison cannot pass for the equality. + fs = _fs() + for printed in ("dirsql 0.2.7\n", "dirsql 2.0\n"): + runner = mock.Mock( + side_effect=[_res(), _res(), _res(), _res(stdout=printed)] + ) + with pytest.raises(DistcheckError, match="expected 'dirsql 1.0'"): + run("/pkg", "/repo", runner=runner, fs=_fs()) + + def test_run_version_missing_marker(): fs = _fs() runner = mock.Mock(side_effect=[_res(), _res(), _res(), _res(stdout="nope")]) - with pytest.raises(DistcheckError, match="--version` failed"): + with pytest.raises(DistcheckError, match="expected 'dirsql 1.0'"): run("/pkg", "/repo", runner=runner, fs=fs) def test_run_import_nonzero_exit(): fs = _fs() runner = mock.Mock( - side_effect=[_res(), _res(), _res(), _res(stdout="dirsql 1.0"), _res(rc=1)] + side_effect=[_res(), _res(), _res(), _res(stdout="dirsql 1.0\n"), _res(rc=1)] ) with pytest.raises(DistcheckError, match="import dirsql` failed"): run("/pkg", "/repo", runner=runner, fs=fs) @@ -195,7 +215,7 @@ def test_run_import_empty_stdout(): _res(), _res(), _res(), - _res(stdout="dirsql 1.0"), + _res(stdout="dirsql 1.0\n"), _res(stdout=" \n"), ] ) diff --git a/packages/python/Cargo.toml b/packages/python/Cargo.toml index 784260053..44ef8f227 100644 --- a/packages/python/Cargo.toml +++ b/packages/python/Cargo.toml @@ -2,9 +2,12 @@ name = "dirsql-py-ext" # Explicit literal (not `version.workspace = true`) so putitoutthere's # pypi/maturin handler can rewrite it via `write-version` before -# `maturin build`. `pyproject.toml` declares `dynamic = ["version"]` -# and maturin reads this field. Mirrors `packages/rust/Cargo.toml`. -version = "0.2.7" +# `maturin build`. `pyproject.toml` declares `dynamic = ["version"]` and +# maturin reads this field, so it is the wheel's version -- and, since #958, +# the version the CLI prints. It must NOT equal `packages/rust/Cargo.toml`'s +# literal: with the two equal, `dirsql-distcheck python` passes even when the +# CLI is reading the core crate's version, which is how #958 shipped. +version = "0.0.1" # abi3 note: the `abi3-py310` pyo3 feature (below) builds ONE stable-ABI # `cp310-abi3` wheel per platform that loads on every CPython >= 3.10 # (matching `pyproject.toml`'s `requires-python = ">=3.10"`), instead of diff --git a/packages/python/changelog.d/2026-08-20-cli-version-host-package.md b/packages/python/changelog.d/2026-08-20-cli-version-host-package.md new file mode 100644 index 000000000..3322b98bc --- /dev/null +++ b/packages/python/changelog.d/2026-08-20-cli-version-host-package.md @@ -0,0 +1,4 @@ +**Fixed** — `dirsql --version` reports the installed wheel's version instead of +a frozen `0.2.7`: it read the embedded core crate's literal, which only the +crates.io release lane rewrites, so `uvx dirsql@0.4.20 --version` answered +`dirsql 0.2.7` (#958). diff --git a/packages/python/e2e-attestations/claude-issue-958-q2loh3.json b/packages/python/e2e-attestations/claude-issue-958-q2loh3.json new file mode 100644 index 000000000..bdf48eb65 --- /dev/null +++ b/packages/python/e2e-attestations/claude-issue-958-q2loh3.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e/ -x -q", + "ran_at": 1787249780, + "exit_code": 0, + "commit": "b1bdc43417b1b170009b720482bd36623c6743e3", + "branch": "claude/issue-958-q2loh3" +} diff --git a/packages/python/src/lib.rs b/packages/python/src/lib.rs index 686ea8408..23f257061 100644 --- a/packages/python/src/lib.rs +++ b/packages/python/src/lib.rs @@ -446,6 +446,15 @@ mod python { } } + /// This wheel's version: `packages/python/Cargo.toml`'s `[package].version`, + /// which maturin writes into the wheel metadata and the release tooling + /// rewrites to the published version at build time. + /// + /// The CLI and `__version__` both read it, so they cannot disagree — and + /// neither reports the embedded core crate's literal, which no release + /// lane but crates.io rewrites (#958). + const VERSION: &str = env!("CARGO_PKG_VERSION"); + /// Run the `dirsql` CLI in this process and return its exit code. /// /// The console script calls this instead of exec'ing a bundled binary, so @@ -463,13 +472,13 @@ mod python { let mut full = Vec::with_capacity(argv.len() + 1); full.push("dirsql".to_string()); full.extend(argv); - py.detach(|| dirsql::cli::run_cli(full)) + py.detach(|| dirsql::cli::run_cli_with_version(full, VERSION)) } #[pymodule] #[pyo3(name = "_dirsql")] fn py_dirsql_module(m: &Bound<'_, PyModule>) -> PyResult<()> { - m.add("__version__", env!("CARGO_PKG_VERSION"))?; + m.add("__version__", VERSION)?; m.add_class::()?; m.add_class::()?; m.add_class::()?; diff --git a/packages/python/tests/e2e/cli_version_test.py b/packages/python/tests/e2e/cli_version_test.py index b556b8d6a..b04b9ebda 100644 --- a/packages/python/tests/e2e/cli_version_test.py +++ b/packages/python/tests/e2e/cli_version_test.py @@ -1,58 +1,31 @@ """E2E test for `dirsql --version` through the Python launcher. -The Python console script must find the bundled Rust binary, forward argv to -it, and surface its output and exit code unchanged. No mocking of any kind: -spawns the real ``dirsql`` console script as a subprocess against the real -built binary (staged where the launcher's ``binary_path()`` looks). +Spawns the real console script (which runs the CLI in-process through the +compiled extension, #738) and asserts it prints the version of the package it +was installed from. The embedded core crate's version is not that -- only the +crates.io lane rewrites that literal, so wheels published at 0.4.x printed +`dirsql 0.2.7` (#958). """ from __future__ import annotations -import os import shutil import subprocess -import pytest - -import dirsql as _dirsql_pkg - -_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 - -# Where the launcher's `binary_path()` looks: `/_binary/dirsql`. -_BINARY_STAGE_DIR = os.path.join(os.path.dirname(_dirsql_pkg.__file__), "_binary") +import dirsql def _cli() -> str: """Resolve the `dirsql` console script for this test env.""" - dirsql = shutil.which("dirsql") - assert dirsql is not None, ( + dirsql_bin = shutil.which("dirsql") + assert dirsql_bin is not None, ( "`dirsql` console script not on PATH -- run `uv run maturin develop`" ) - return dirsql + return dirsql_bin def describe_dirsql_version_flag(): - @pytest.fixture - def staged_binary(): - 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) - try: - yield staged - finally: - shutil.rmtree(_BINARY_STAGE_DIR, ignore_errors=True) - - def it_prints_the_version_and_exits_zero(staged_binary): + def it_prints_the_installed_packages_version_and_exits_zero(): proc = subprocess.run( [_cli(), "--version"], stdin=subprocess.DEVNULL, @@ -63,4 +36,4 @@ def it_prints_the_version_and_exits_zero(staged_binary): assert proc.returncode == 0, ( f"expected exit 0; stdout={proc.stdout!r}, stderr={proc.stderr!r}" ) - assert "dirsql" in proc.stdout + assert proc.stdout.strip() == f"dirsql {dirsql.__version__}" diff --git a/packages/rust/Cargo.toml b/packages/rust/Cargo.toml index 91b439190..e8dcdbbd1 100644 --- a/packages/rust/Cargo.toml +++ b/packages/rust/Cargo.toml @@ -50,7 +50,9 @@ cli = [ [dependencies] axum = { version = "0.8", optional = true } blake3 = "1" -clap = { version = "4", features = ["derive"], optional = true } +# `string`: `--version` is set at runtime from an owned `String` (the host +# package's version, #958), and clap only accepts one behind this feature. +clap = { version = "4", features = ["derive", "string"], optional = true } futures = { version = "0.3", optional = true } futures-channel.workspace = true globset = "0.4" diff --git a/packages/rust/changelog.d/2026-08-20-cli-version-host-package.md b/packages/rust/changelog.d/2026-08-20-cli-version-host-package.md new file mode 100644 index 000000000..4cfa69dfc --- /dev/null +++ b/packages/rust/changelog.d/2026-08-20-cli-version-host-package.md @@ -0,0 +1,4 @@ +**Added** — `dirsql::cli::run_cli_with_version(argv, version)`: the CLI entry +point with the reported version supplied by the caller, for embedders that ship +this crate inside an artifact published on its own version line (#958). +`run_cli` is unchanged. diff --git a/packages/rust/src/cli/mod.rs b/packages/rust/src/cli/mod.rs index 84255abc8..2d9b46114 100644 --- a/packages/rust/src/cli/mod.rs +++ b/packages/rust/src/cli/mod.rs @@ -36,7 +36,7 @@ pub mod run; pub mod serialize; pub mod server; -pub use run::run_cli; +pub use run::{run_cli, run_cli_with_version}; pub use server::{serve, serve_with_state}; /// Re-export of the crate-level baked-in default config diff --git a/packages/rust/src/cli/run.rs b/packages/rust/src/cli/run.rs index 7ad898b8e..64351a7fe 100644 --- a/packages/rust/src/cli/run.rs +++ b/packages/rust/src/cli/run.rs @@ -17,7 +17,7 @@ use std::path::PathBuf; use super::{AppState, ServerConfig, execute::execute_query, init::InitOptions, serve_with_state}; use crate::{DirSQL, Extension, Row, Table}; -use clap::{Args, Parser, Subcommand}; +use clap::{Args, CommandFactory, FromArgMatches, Parser, Subcommand}; #[derive(Debug, Parser)] #[command( @@ -230,6 +230,14 @@ struct InitArgs { force: bool, } +/// `Cli::try_parse_from`, with `--version` reporting `version` instead of the +/// version the derive baked in at compile time. +fn parse_cli(argv: Vec, version: &str) -> Result { + let mut command = Cli::command().version(version.to_string()); + let mut matches = command.try_get_matches_from_mut(argv)?; + Cli::from_arg_matches_mut(&mut matches).map_err(|err| err.format(&mut command)) +} + /// Run the CLI over an explicit `argv` and return its exit code. /// /// The single entry point every packaging path shares: the `dirsql` binary is @@ -246,8 +254,22 @@ struct InitArgs { /// Clap's own `parse()` exits the process on `--help`, `--version` and usage /// errors, so this parses fallibly and renders the result itself — same text, /// same streams, same codes, minus the exit. +/// +/// `--version` reports **this crate's** version, right for the `dirsql` binary +/// and wrong for an artifact published on its own version line; those call +/// [`run_cli_with_version`]. pub fn run_cli(argv: Vec) -> i32 { - let mut cli = match Cli::try_parse_from(argv) { + run_cli_with_version(argv, env!("CARGO_PKG_VERSION")) +} + +/// [`run_cli`], with the version `--version` reports supplied by the caller. +/// +/// The PyPI wheel and the npm addon embed this crate but publish on their own +/// version line, and the release tooling rewrites their manifests rather than +/// this crate's literal — so reporting `CARGO_PKG_VERSION` there names a +/// version nobody installed: `dirsql@0.4.20` answered `0.2.7` (#958). +pub fn run_cli_with_version(argv: Vec, version: &str) -> i32 { + let mut cli = match parse_cli(argv, version) { Ok(cli) => cli, Err(err) => { let code = err.exit_code(); @@ -936,6 +958,31 @@ mod tests { assert_eq!(exts[0].entrypoint.as_deref(), Some("init::extra")); } + #[test] + fn version_flag_renders_the_version_the_caller_supplied() { + // The #958 contract: what `--version` prints is the caller's version, + // not this crate's. clap reports `--version` as an "error", and that + // error carries the exact text `print()` would emit. + let err = parse_cli(vec!["dirsql".into(), "--version".into()], "9.9.9").unwrap_err(); + assert_eq!(err.render().to_string().trim(), "dirsql 9.9.9"); + } + + #[test] + fn run_cli_with_version_returns_zero_for_version_instead_of_exiting() { + assert_eq!( + run_cli_with_version(vec!["dirsql".into(), "--version".into()], "9.9.9"), + 0 + ); + } + + #[test] + fn run_cli_with_version_returns_clap_usage_code_for_an_unknown_flag() { + assert_eq!( + run_cli_with_version(vec!["dirsql".into(), "--nope".into()], "9.9.9"), + 2 + ); + } + #[test] fn run_cli_returns_zero_for_version_instead_of_exiting() { // The contract this slice exists to establish: clap's own `parse()` diff --git a/packages/rust/tests/cli_e2e.rs b/packages/rust/tests/cli_e2e.rs index 79ffe16d5..18f270840 100644 --- a/packages/rust/tests/cli_e2e.rs +++ b/packages/rust/tests/cli_e2e.rs @@ -163,13 +163,18 @@ fn kill_and_wait(mut child: Child) { } #[test] -fn version_flag_prints_and_exits_zero() { +fn version_flag_prints_this_crates_version_and_exits_zero() { + // The binary is this crate, so it reports this crate's version -- the one + // putitoutthere rewrites for the crates.io release. The bindings ship the + // core inside an artifact versioned separately and pass their own instead + // (#958); pinning the exact string here is what keeps `run_cli`'s default + // honest. std::process::Command::cargo_bin("dirsql") .expect("binary must exist (cargo install --features cli / `cargo test --features cli`)") .arg("--version") .assert() .success() - .stdout(predicates::str::is_match(r"^dirsql \d+\.\d+\.\d+").unwrap()); + .stdout(format!("dirsql {}\n", env!("CARGO_PKG_VERSION"))); } #[test] diff --git a/packages/ts/changelog.d/2026-08-20-cli-version-host-package.md b/packages/ts/changelog.d/2026-08-20-cli-version-host-package.md new file mode 100644 index 000000000..934355205 --- /dev/null +++ b/packages/ts/changelog.d/2026-08-20-cli-version-host-package.md @@ -0,0 +1,4 @@ +**Fixed** — `dirsql --version` reports the installed package's version instead +of a frozen `0.2.7`: it read the embedded core crate's literal, which only the +crates.io release lane rewrites (#958). The launcher now passes its own +`package.json` version to the addon. diff --git a/packages/ts/e2e-attestations/claude-issue-958-q2loh3.json b/packages/ts/e2e-attestations/claude-issue-958-q2loh3.json new file mode 100644 index 000000000..780f2522e --- /dev/null +++ b/packages/ts/e2e-attestations/claude-issue-958-q2loh3.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1787250219, + "exit_code": 0, + "commit": "8529e98dc9e36066fa8ba473f916d6662c385557", + "branch": "claude/issue-958-q2loh3" +} diff --git a/packages/ts/napi/src/lib.rs b/packages/ts/napi/src/lib.rs index 284410f03..dd44f10b9 100644 --- a/packages/ts/napi/src/lib.rs +++ b/packages/ts/napi/src/lib.rs @@ -45,14 +45,23 @@ pub fn parse_table_name(ddl: String) -> Option { /// addon is the only copy of the core a package ships (#739). `argv` excludes /// the program name; the core prepends it for clap. /// +/// `version` is what `--version` reports. The launcher passes the `dirsql` +/// package's own, since that is the version npm installed; this crate's +/// `CARGO_PKG_VERSION` is the workspace placeholder and the core's is rewritten +/// only in the crates.io lane, so neither names the published addon (#958). +/// Omitted, the core's version stands. +/// /// Returns rather than exiting — the caller (bin-shim's `mainInProcess`) owns /// the process exit. Codes are ordinary status codes, never 130/143 (#737). #[napi(js_name = "runCli")] -pub fn run_cli(argv: Vec) -> i32 { +pub fn run_cli(argv: Vec, version: Option) -> i32 { let mut full = Vec::with_capacity(argv.len() + 1); full.push("dirsql".to_string()); full.extend(argv); - dirsql::cli::run_cli(full) + match version { + Some(version) => dirsql::cli::run_cli_with_version(full, &version), + None => dirsql::cli::run_cli(full), + } } /// A row-level event emitted by the file watcher. diff --git a/packages/ts/src/cli/main.test.ts b/packages/ts/src/cli/main.test.ts index 40e7b439b..559c3f345 100644 --- a/packages/ts/src/cli/main.test.ts +++ b/packages/ts/src/cli/main.test.ts @@ -2,7 +2,7 @@ import { mainInProcess } from "bin-shim"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { loadNativeCore } from "../load-native-core.js"; import { die } from "./die.js"; -import { main } from "./main.js"; +import { main, packageVersion } from "./main.js"; import { withResolvedExtensions } from "./resolve-config-extensions.js"; vi.mock("bin-shim"); @@ -48,11 +48,48 @@ describe("main", () => { expect.objectContaining({ argv: ["query", "SELECT 1"], binaryName: "dirsql", - runCli, + runCli: expect.any(Function), }), ); }); + it("hands the addon this package's version, so `--version` reports it", async () => { + // #958: the addon's own copy of the core carries a literal that only the + // crates.io release lane rewrites, so left to itself it reports a version + // nobody installed. Let the shim invoke the callable it was handed, as the + // real one does, rather than inspecting it. + vi.mocked(mainInProcess).mockImplementation( + async ({ argv = [], runCli: forwarded }) => forwarded?.(argv) ?? 0, + ); + + await expect(main(["--version"])).rejects.toThrow("EXIT_0"); + expect(runCli).toHaveBeenCalledExactlyOnceWith( + ["--version"], + // The real manifest, read through the default requirer: a specifier that + // missed it would come back undefined. + expect.stringMatching(/^\d+\.\d+\.\d+/), + ); + }); + + describe("packageVersion", () => { + it("reads `version` from the manifest two levels up", () => { + const requirer = vi.fn(() => ({ version: "4.2.0" })); + + expect(packageVersion(requirer)).toBe("4.2.0"); + expect(requirer).toHaveBeenCalledExactlyOnceWith("../../package.json"); + }); + + it("falls back to undefined rather than failing the CLI", () => { + // Either way the addon's own version stands: wrong, but it starts. + expect( + packageVersion(() => { + throw new Error("Cannot find module '../../package.json'"); + }), + ).toBeUndefined(); + expect(packageVersion(() => ({ version: 42 }))).toBeUndefined(); + }); + }); + it("passes the resolver's augmented argv, not the raw argv", async () => { vi.mocked(withResolvedExtensions).mockResolvedValue([ "query", diff --git a/packages/ts/src/cli/main.ts b/packages/ts/src/cli/main.ts index 601b009ec..7334e3ec4 100644 --- a/packages/ts/src/cli/main.ts +++ b/packages/ts/src/cli/main.ts @@ -6,13 +6,35 @@ // `@dirsql/lib-*` addon the SDK loads carries the CLI, so a package ships one // copy of the core instead of two (#739). +import { createRequire } from "node:module"; import { mainInProcess } from "bin-shim"; import { loadNativeCore } from "../load-native-core.js"; import { die } from "./die.js"; import { withResolvedExtensions } from "./resolve-config-extensions.js"; /** The addon's CLI entry point, as napi exports it. */ -type RunCli = (argv: readonly string[]) => number; +type RunCli = (argv: readonly string[], version?: string) => number; + +/** + * This package's version, or `undefined` if its manifest cannot be read. + * + * `package.json` sits two levels above this module both in `src/` and in + * `dist/`. Falling back rather than throwing keeps a `--version` the addon can + * still answer (with its own, wrong number) preferable to a CLI that refuses to + * start. + */ +export function packageVersion( + requirer: (specifier: string) => unknown = createRequire(import.meta.url), +): string | undefined { + let manifest: unknown; + try { + manifest = requirer("../../package.json"); + } catch { + return undefined; + } + const { version } = manifest as { version?: unknown }; + return typeof version === "string" ? version : undefined; +} /** * Resolve `runCli` off the same addon the SDK loads. @@ -23,8 +45,11 @@ type RunCli = (argv: readonly string[]) => number; * `loadNativeCore` keeps one resolution rule for the SDK and the CLI — * including its dev fallback to a locally built `dirsql.node` — so a * monorepo checkout and a published install take the same path. + * + * The addon is handed this package's version, because the core crate compiled + * into it carries a literal no npm release rewrites (#958). */ -function resolveRunCli(): RunCli { +function resolveRunCli(): (argv: readonly string[]) => number { const core = loadNativeCore() as { runCli?: unknown }; if (typeof core.runCli !== "function") { throw new Error( @@ -32,7 +57,9 @@ function resolveRunCli(): RunCli { "it was built without the `cli` feature.", ); } - return core.runCli as RunCli; + const runCli = core.runCli as RunCli; + const version = packageVersion(); + return (argv) => runCli(argv, version); } /**