diff --git a/packages/python/changelog.d/2026-09-05-config-extension-loader-split.md b/packages/python/changelog.d/2026-09-05-config-extension-loader-split.md new file mode 100644 index 000000000..faa85b546 --- /dev/null +++ b/packages/python/changelog.d/2026-09-05-config-extension-loader-split.md @@ -0,0 +1,3 @@ +**Changed** + +- **The config-extension loader split into four modules.** `_load_toml_module` (with the `_toml` binding it produces) moved to `dirsql/toml_module.py`, `_load_extension_entries` to `dirsql/load_extension_entries.py` and `_has_bare_name` to `dirsql/has_bare_name.py`; `dirsql/resolve_config_extensions.py` keeps the intervene-or-not decision, `resolve_config_extension_specs`. Every function keeps its name and behavior — internal reorganisation with no change to the published API or CLI behavior. (#1079) diff --git a/packages/python/changelog.d/2026-09-05-resolve-extension-split.md b/packages/python/changelog.d/2026-09-05-resolve-extension-split.md new file mode 100644 index 000000000..818da78d6 --- /dev/null +++ b/packages/python/changelog.d/2026-09-05-resolve-extension-split.md @@ -0,0 +1,3 @@ +**Changed** + +- **The extension-path resolver split into four modules.** `_platform_patterns` moved to `dirsql/platform_patterns.py`, `is_bare_name` (with `_LOADABLE_SUFFIXES`) to `dirsql/is_bare_name.py`, and `_resolve_package` to `dirsql/resolve_package.py`; `dirsql/resolve_extension.py` keeps the ordered probe, `resolve_extension_path`. Every function keeps its name and behavior — internal reorganisation with no change to the published API or CLI behavior. (#1066) diff --git a/packages/python/dirsql/has_bare_name.py b/packages/python/dirsql/has_bare_name.py new file mode 100644 index 000000000..dabf6dc4d --- /dev/null +++ b/packages/python/dirsql/has_bare_name.py @@ -0,0 +1,14 @@ +"""Whether any of a config's extension entries names a package.""" + +from __future__ import annotations + +from .is_bare_name import is_bare_name + + +def _has_bare_name(entries): + return any( + isinstance(e, dict) + and isinstance(e.get("path"), str) + and is_bare_name(e["path"]) + for e in entries + ) diff --git a/packages/python/dirsql/has_bare_name_test.py b/packages/python/dirsql/has_bare_name_test.py new file mode 100644 index 000000000..9f3d70716 --- /dev/null +++ b/packages/python/dirsql/has_bare_name_test.py @@ -0,0 +1,41 @@ +"""Unit tests for `_has_bare_name`. + +The one collaborator, the pure `is_bare_name`, is mocked so the probe's own +filtering -- non-dict entries, non-string paths, short-circuiting -- is what +these observe. +""" + +from unittest import mock + +import dirsql.has_bare_name as mod + + +def _patch(): + return mock.patch.object( + mod, "is_bare_name", side_effect=lambda path: not path.endswith(".so") + ) + + +def describe_has_bare_name(): + def it_is_false_for_no_entries(): + with _patch() as is_bare_name: + assert mod._has_bare_name([]) is False + is_bare_name.assert_not_called() + + def it_is_false_when_every_path_is_literal(): + with _patch(): + assert mod._has_bare_name([{"path": "ext/a.so"}]) is False + + def it_is_true_when_some_path_is_a_bare_name(): + with _patch(): + assert mod._has_bare_name([{"path": "ext/a.so"}, {"path": "vec"}]) is True + + def it_skips_an_entry_that_is_not_a_table(): + with _patch() as is_bare_name: + assert mod._has_bare_name([1, "vec"]) is False + is_bare_name.assert_not_called() + + def it_skips_an_entry_whose_path_is_not_a_string(): + with _patch() as is_bare_name: + assert mod._has_bare_name([{"path": 42}, {}]) is False + is_bare_name.assert_not_called() diff --git a/packages/python/dirsql/is_bare_name.py b/packages/python/dirsql/is_bare_name.py new file mode 100644 index 000000000..b0643ed76 --- /dev/null +++ b/packages/python/dirsql/is_bare_name.py @@ -0,0 +1,14 @@ +"""Whether an extension ``path`` names a package rather than a file.""" + +import os + +# Suffixes that mark a value as "already a file path", so package resolution +# is never attempted. +_LOADABLE_SUFFIXES = (".so", ".dylib", ".dll", ".pyd") + + +def is_bare_name(path): + """True when ``path`` is a bare package name rather than a file path.""" + if os.sep in path or (os.altsep and os.altsep in path): + return False + return not path.endswith(_LOADABLE_SUFFIXES) diff --git a/packages/python/dirsql/is_bare_name_test.py b/packages/python/dirsql/is_bare_name_test.py new file mode 100644 index 000000000..bbc0aaf7b --- /dev/null +++ b/packages/python/dirsql/is_bare_name_test.py @@ -0,0 +1,42 @@ +"""Unit tests for `is_bare_name`. + +`os.sep` / `os.altsep` are mocked where a non-posix separator matters, so the +alternate-separator arm runs on every host. +""" + +from unittest import mock + +import dirsql.is_bare_name as mod + + +def describe_loadable_suffixes(): + def it_lists_every_loadable_extension_suffix(): + assert mod._LOADABLE_SUFFIXES == (".so", ".dylib", ".dll", ".pyd") + + +def describe_is_bare_name(): + def it_treats_a_separatorful_value_as_a_path(): + assert mod.is_bare_name("ext/vec0.so") is False + assert mod.is_bare_name("/abs/ext.so") is False + + def it_treats_a_separatorful_value_without_a_loadable_suffix_as_a_path(): + assert mod.is_bare_name("ext/foo") is False + assert mod.is_bare_name("/abs/dir") is False + + def it_treats_an_alternate_separator_as_a_path(): + with ( + mock.patch.object(mod.os, "sep", "\\"), + mock.patch.object(mod.os, "altsep", "/"), + ): + assert mod.is_bare_name("ext/foo") is False + assert mod.is_bare_name("ext\\foo") is False + assert mod.is_bare_name("sqlite_vec") is True + + def it_treats_a_loadable_suffix_as_a_path(): + assert mod.is_bare_name("vec0.so") is False + assert mod.is_bare_name("vec0.dylib") is False + assert mod.is_bare_name("vec0.dll") is False + assert mod.is_bare_name("vec0.pyd") is False + + def it_treats_a_plain_identifier_as_a_bare_name(): + assert mod.is_bare_name("sqlite_vec") is True diff --git a/packages/python/dirsql/load_extension_entries.py b/packages/python/dirsql/load_extension_entries.py new file mode 100644 index 000000000..120d75886 --- /dev/null +++ b/packages/python/dirsql/load_extension_entries.py @@ -0,0 +1,28 @@ +"""Reading one TOML config's ``[[dirsql.extension]]`` array off disk.""" + +from __future__ import annotations + +import os + +from .toml_module import _toml + + +def _load_extension_entries(config_path): + """Return ``(entries, base_dir)`` for a config's ``[[dirsql.extension]]``. + + ``None`` when the config is missing, unreadable/malformed, or declares no + extension array -- the caller should leave such configs to the core. + """ + if not os.path.isfile(config_path): + return None + try: + with open(config_path, "rb") as f: + doc = _toml.load(f) + except (OSError, _toml.TOMLDecodeError): + # Leave a malformed / unreadable config for the core to report. + return None + + entries = (doc.get("dirsql") or {}).get("extension") + if not isinstance(entries, list): + return None + return entries, os.path.dirname(os.path.abspath(config_path)) diff --git a/packages/python/dirsql/load_extension_entries_test.py b/packages/python/dirsql/load_extension_entries_test.py new file mode 100644 index 000000000..25efc6a2e --- /dev/null +++ b/packages/python/dirsql/load_extension_entries_test.py @@ -0,0 +1,92 @@ +"""Unit tests for `_load_extension_entries`. + +Real `.dirsql.toml` files under a temp dir are parsed by the real TOML module: +the unit under test is the shape-checking around the parse, and a stub parser +would only restate it. The unreadable-file arm mocks `open`. +""" + +import os +import tempfile +from unittest import mock + +import pytest + +import dirsql.load_extension_entries as mod + + +@pytest.fixture +def cfg_dir(): + with tempfile.TemporaryDirectory() as d: + yield d + + +def _write(cfg_dir, body): + path = os.path.join(cfg_dir, ".dirsql.toml") + with open(path, "w") as f: + f.write(body) + return path + + +def describe_configs_with_no_usable_extension_array(): + def it_returns_none_when_the_config_is_missing(): + assert mod._load_extension_entries("/nope/.dirsql.toml") is None + + def it_returns_none_on_malformed_toml(cfg_dir): + path = _write(cfg_dir, "this is not = valid = toml") + assert mod._load_extension_entries(path) is None + + def it_returns_none_when_the_config_cannot_be_read(cfg_dir): + path = _write(cfg_dir, '[[dirsql.extension]]\npath = "vec"\n') + with mock.patch("builtins.open", side_effect=OSError("denied")): + assert mod._load_extension_entries(path) is None + + def it_returns_none_when_there_is_no_dirsql_section(cfg_dir): + path = _write( + cfg_dir, + '[[table]]\nname = "t"\nddl = "CREATE TABLE t (x TEXT)"\nglob = "*"\n', + ) + assert mod._load_extension_entries(path) is None + + def it_returns_none_when_no_extensions_are_declared(cfg_dir): + path = _write(cfg_dir, '[dirsql]\nignore = ["x"]\n') + assert mod._load_extension_entries(path) is None + + def it_returns_none_when_extension_is_not_a_list(cfg_dir): + path = _write(cfg_dir, '[dirsql]\nextension = "not-a-list"\n') + assert mod._load_extension_entries(path) is None + + +def describe_configs_declaring_extensions(): + def it_returns_the_entries_and_the_configs_own_directory(cfg_dir): + path = _write( + cfg_dir, + "[[dirsql.extension]]\n" + 'path = "sqlite_vec"\n' + 'entrypoint = "sqlite3_vec_init"\n\n' + "[[dirsql.extension]]\n" + 'path = "ext/local.so"\n', + ) + assert mod._load_extension_entries(path) == ( + [ + {"path": "sqlite_vec", "entrypoint": "sqlite3_vec_init"}, + {"path": "ext/local.so"}, + ], + cfg_dir, + ) + + def it_returns_entries_that_are_not_tables_verbatim(cfg_dir): + # Shape-checking each entry is the caller's job; this only asserts the + # array is a list. + path = _write(cfg_dir, "[dirsql]\nextension = [1, 2]\n") + assert mod._load_extension_entries(path) == ([1, 2], cfg_dir) + + def it_normalizes_the_base_directory(cfg_dir): + # A traversal segment in the config path must not survive into the + # base directory every extension is then resolved against. + sub = os.path.join(cfg_dir, "sub") + os.mkdir(sub) + _write(sub, '[[dirsql.extension]]\npath = "vec"\n') + _, base = mod._load_extension_entries( + os.path.join(cfg_dir, "sub", "..", "sub", ".dirsql.toml") + ) + assert base == sub diff --git a/packages/python/dirsql/platform_patterns.py b/packages/python/dirsql/platform_patterns.py new file mode 100644 index 000000000..256547ad3 --- /dev/null +++ b/packages/python/dirsql/platform_patterns.py @@ -0,0 +1,12 @@ +"""The loadable-file glob pattern(s) the running platform uses.""" + +import sys + + +def _platform_patterns(): + """Loadable-file glob(s) for the current platform.""" + if sys.platform == "darwin": + return ("*.dylib",) + if sys.platform == "win32": + return ("*.dll", "*.pyd") + return ("*.so",) diff --git a/packages/python/dirsql/platform_patterns_test.py b/packages/python/dirsql/platform_patterns_test.py new file mode 100644 index 000000000..2798600dd --- /dev/null +++ b/packages/python/dirsql/platform_patterns_test.py @@ -0,0 +1,33 @@ +"""Unit tests for `_platform_patterns`. + +`sys.platform` is mocked, so the dispatch is exercised on every host. +""" + +from unittest import mock + +import dirsql.platform_patterns as mod + + +def _patterns_on(platform): + with mock.patch.object(mod.sys, "platform", platform): + return mod._platform_patterns() + + +def describe_platform_patterns(): + def it_globs_dylib_on_macos(): + assert _patterns_on("darwin") == ("*.dylib",) + + def it_globs_dll_and_pyd_on_windows(): + assert _patterns_on("win32") == ("*.dll", "*.pyd") + + def it_globs_so_on_any_other_platform(): + # aix sorts below "darwin" and zos above "win32", pinning the + # dispatch to equality rather than any ordering. + for platform in ("aix", "zos", "linux"): + assert _patterns_on(platform) == ("*.so",) + + def it_dispatches_on_platform_string_equality_not_identity(): + # Runtime-built platform strings (not interned literals) must still + # dispatch correctly -- `sys.platform` is compared by value. + assert _patterns_on("".join(["dar", "win"])) == ("*.dylib",) + assert _patterns_on("".join(["win", "32"])) == ("*.dll", "*.pyd") diff --git a/packages/python/dirsql/resolve_config_extensions.py b/packages/python/dirsql/resolve_config_extensions.py index 4470ac52c..35b22d726 100644 --- a/packages/python/dirsql/resolve_config_extensions.py +++ b/packages/python/dirsql/resolve_config_extensions.py @@ -14,59 +14,9 @@ from __future__ import annotations -import os -import sys -from importlib import import_module - +from .has_bare_name import _has_bare_name +from .load_extension_entries import _load_extension_entries from .resolve_entries import _resolve_entries -from .resolve_extension import is_bare_name - - -def _load_toml_module(): - """Return the TOML parser module for the running interpreter. - - ``tomllib`` is stdlib only on 3.11+; on 3.10 the ``tomli`` backport it was - derived from (a version-gated dependency) provides the same surface. - Imported by name so a unit test can exercise both arms on any - interpreter -- a literal ``import tomllib`` is unreachable on 3.10 no - matter what ``sys.version_info`` claims. - """ - if sys.version_info >= (3, 11): - return import_module("tomllib") - return import_module("tomli") - - -_toml = _load_toml_module() - - -def _load_extension_entries(config_path): - """Return ``(entries, base_dir)`` for a config's ``[[dirsql.extension]]``. - - ``None`` when the config is missing, unreadable/malformed, or declares no - extension array -- the caller should leave such configs to the core. - """ - if not os.path.isfile(config_path): - return None - try: - with open(config_path, "rb") as f: - doc = _toml.load(f) - except (OSError, _toml.TOMLDecodeError): - # Leave a malformed / unreadable config for the core to report. - return None - - entries = (doc.get("dirsql") or {}).get("extension") - if not isinstance(entries, list): - return None - return entries, os.path.dirname(os.path.abspath(config_path)) - - -def _has_bare_name(entries): - return any( - isinstance(e, dict) - and isinstance(e.get("path"), str) - and is_bare_name(e["path"]) - for e in entries - ) def resolve_config_extension_specs(config_path): diff --git a/packages/python/dirsql/resolve_config_extensions_test.py b/packages/python/dirsql/resolve_config_extensions_test.py index 70be44c87..ed1fae286 100644 --- a/packages/python/dirsql/resolve_config_extensions_test.py +++ b/packages/python/dirsql/resolve_config_extensions_test.py @@ -1,132 +1,41 @@ """Unit tests for `resolve_config_extension_specs`. -Real `.dirsql.toml` files are parsed; `_resolve_entries` (which owns the -effectful per-entry resolution) is mocked, returning a `base:path` sentinel -per entry. The pure `is_bare_name` stays real. +Every collaborator -- the config reader, the bare-name probe and the effectful +per-entry resolution -- is mocked, so these isolate the intervene-or-not +decision from the filesystem. """ -import os -import sys -import tempfile from unittest import mock -import pytest +import dirsql.resolve_config_extensions as mod -import dirsql.resolve_config_extensions as rce - -@pytest.fixture -def cfg_dir(): - with tempfile.TemporaryDirectory() as d: - yield d - - -def _write(path, body): - with open(path, "w") as f: - f.write(body) - - -def _patch(): - return mock.patch.object( - rce, - "_resolve_entries", - side_effect=lambda entries, base: [f"{base}:{e['path']}" for e in entries], +def _patch(*, loaded, bare, resolved="SPECS"): + return ( + mock.patch.object(mod, "_load_extension_entries", return_value=loaded), + mock.patch.object(mod, "_has_bare_name", return_value=bare), + mock.patch.object(mod, "_resolve_entries", return_value=resolved), ) -def describe_load_toml_module(): - # `import_module` is mocked and `version_info` patched so both arms run on - # every interpreter -- neither `tomllib` (absent on 3.10) nor `tomli` - # (absent on 3.11+) is importable everywhere. - def _requested(version): - with ( - mock.patch.object(sys, "version_info", version), - mock.patch.object(rce, "import_module") as import_module, - ): - assert rce._load_toml_module() is import_module.return_value - (name,) = import_module.call_args.args - return name - - # Exactly `(3, 11)`, not `(3, 11, 0)`: the latter compares greater than the - # bare `(3, 11)` literal, so it cannot tell `>=` from `>`. - def it_uses_the_stdlib_parser_on_the_version_that_gained_it(): - assert _requested((3, 11)) == "tomllib" - - def it_uses_the_stdlib_parser_on_newer_versions(): - assert _requested((3, 12, 4)) == "tomllib" - - def it_uses_the_tomli_backport_below_311(): - assert _requested((3, 10, 17)) == "tomli" - - def describe_resolve_config_extension_specs(): - def it_returns_none_when_the_config_is_missing(): - assert rce.resolve_config_extension_specs("/nope/.dirsql.toml") is None - - def it_returns_none_on_malformed_toml(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, "this is not = valid = toml") - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_there_is_no_dirsql_section(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write( - path, '[[table]]\nname = "t"\nddl = "CREATE TABLE t (x TEXT)"\nglob = "*"\n' - ) - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_no_extensions_are_declared(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, '[dirsql]\nignore = ["x"]\n') - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_extension_is_not_a_list(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, '[dirsql]\nextension = "not-a-list"\n') - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_extension_entries_are_not_tables(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, "[dirsql]\nextension = [1, 2]\n") - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_all_paths_are_literal(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, '[[dirsql.extension]]\npath = "ext/a.so"\n') - with _patch() as resolve_entries: - assert rce.resolve_config_extension_specs(path) is None - resolve_entries.assert_not_called() - - def it_resolves_every_entry_when_a_path_is_a_bare_package_name(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write( - path, - "[[dirsql.extension]]\n" - 'path = "sqlite_vec"\n' - 'entrypoint = "sqlite3_vec_init"\n\n' - "[[dirsql.extension]]\n" - 'path = "ext/local.so"\n', - ) - with _patch() as resolve_entries: - specs = rce.resolve_config_extension_specs(path) - assert specs == [f"{cfg_dir}:sqlite_vec", f"{cfg_dir}:ext/local.so"] - resolve_entries.assert_called_once_with( - [ - {"path": "sqlite_vec", "entrypoint": "sqlite3_vec_init"}, - {"path": "ext/local.so"}, - ], - cfg_dir, - ) - - def it_skips_a_non_string_path_in_the_package_name_probe(cfg_dir): - # A dict entry whose `path` isn't a string is not treated as a package - # name; a sibling bare name still triggers resolution of every entry. - path = os.path.join(cfg_dir, ".dirsql.toml") - _write( - path, - "[[dirsql.extension]]\npath = 42\n\n" - '[[dirsql.extension]]\npath = "sqlite_vec"\n', - ) - with _patch(): - specs = rce.resolve_config_extension_specs(path) - assert specs == [f"{cfg_dir}:42", f"{cfg_dir}:sqlite_vec"] + def it_returns_none_when_the_config_declares_no_extensions(): + load, bare, resolve = _patch(loaded=None, bare=True) + with load as loader, bare, resolve as resolve_entries: + assert mod.resolve_config_extension_specs("/cfg/.dirsql.toml") is None + loader.assert_called_once_with("/cfg/.dirsql.toml") + resolve_entries.assert_not_called() + + def it_returns_none_when_every_path_is_literal(): + load, bare, resolve = _patch(loaded=([{"path": "a.so"}], "/cfg"), bare=False) + with load, bare as has_bare_name, resolve as resolve_entries: + assert mod.resolve_config_extension_specs("/cfg/.dirsql.toml") is None + has_bare_name.assert_called_once_with([{"path": "a.so"}]) + resolve_entries.assert_not_called() + + def it_resolves_every_entry_when_a_path_is_a_bare_package_name(): + entries = [{"path": "sqlite_vec"}, {"path": "ext/local.so"}] + load, bare, resolve = _patch(loaded=(entries, "/cfg"), bare=True) + with load, bare, resolve as resolve_entries: + assert mod.resolve_config_extension_specs("/cfg/.dirsql.toml") == "SPECS" + resolve_entries.assert_called_once_with(entries, "/cfg") diff --git a/packages/python/dirsql/resolve_configs_extension_specs.py b/packages/python/dirsql/resolve_configs_extension_specs.py index ed07dbccb..112e917d9 100644 --- a/packages/python/dirsql/resolve_configs_extension_specs.py +++ b/packages/python/dirsql/resolve_configs_extension_specs.py @@ -7,7 +7,8 @@ from __future__ import annotations -from .resolve_config_extensions import _has_bare_name, _load_extension_entries +from .has_bare_name import _has_bare_name +from .load_extension_entries import _load_extension_entries from .resolve_entries import _resolve_entries diff --git a/packages/python/dirsql/resolve_configs_extension_specs_test.py b/packages/python/dirsql/resolve_configs_extension_specs_test.py index c5d64085e..72c095d57 100644 --- a/packages/python/dirsql/resolve_configs_extension_specs_test.py +++ b/packages/python/dirsql/resolve_configs_extension_specs_test.py @@ -75,3 +75,17 @@ def it_resolves_each_config_against_its_own_directory(cfg_dir): with _patch(): specs = mod.resolve_configs_extension_specs([a, b]) assert specs == [f"{cfg_dir}:sqlite_vec", f"{other}:ext/b.so"] + + +def describe_module_wiring(): + # The helpers live in their own modules; these pin where this module binds + # them from, so a mis-pointed import is a failure rather than a silent + # re-export. + def it_reads_each_config_through_the_shared_loader(): + assert mod._load_extension_entries.__module__ == "dirsql.load_extension_entries" + + def it_probes_for_bare_names_through_the_shared_helper(): + assert mod._has_bare_name.__module__ == "dirsql.has_bare_name" + + def it_resolves_entries_through_the_shared_resolver(): + assert mod._resolve_entries.__module__ == "dirsql.resolve_entries" diff --git a/packages/python/dirsql/resolve_extension.py b/packages/python/dirsql/resolve_extension.py index dc7d10605..c55a217e4 100644 --- a/packages/python/dirsql/resolve_extension.py +++ b/packages/python/dirsql/resolve_extension.py @@ -10,76 +10,13 @@ 2. **Bare name** -- a same-named local file under ``base`` *shadows* the package; otherwise the package dir is located via :func:`importlib.util.find_spec` and the current platform's loadable is - globbed from inside it. Zero matches and multiple matches are both hard - errors -- the caller must disambiguate with a literal path. + globbed from inside it. """ -import glob as _glob -import importlib.util import os -import sys -# Suffixes that mark a value as "already a file path", so package resolution -# is never attempted. -_LOADABLE_SUFFIXES = (".so", ".dylib", ".dll", ".pyd") - - -def _platform_patterns(): - """Loadable-file glob(s) for the current platform.""" - if sys.platform == "darwin": - return ("*.dylib",) - if sys.platform == "win32": - return ("*.dll", "*.pyd") - return ("*.so",) - - -def is_bare_name(path): - """True when ``path`` is a bare package name rather than a file path.""" - if os.sep in path or (os.altsep and os.altsep in path): - return False - return not path.endswith(_LOADABLE_SUFFIXES) - - -def _resolve_package(name): - """Locate ``name``'s package dir and glob its platform loadable file.""" - try: - spec = importlib.util.find_spec(name) - except (ImportError, ValueError) as exc: - raise ValueError( - f"could not resolve extension package {name!r}: {exc}" - ) from exc - if spec is None: - raise ValueError(f"could not resolve extension package {name!r}: not installed") - - dirs = list(spec.submodule_search_locations or []) - if not dirs and spec.origin and spec.origin not in ("built-in", "frozen"): - dirs.append(os.path.dirname(spec.origin)) - if not dirs: - raise ValueError( - f"could not resolve extension package {name!r}: no package directory" - ) - - patterns = _platform_patterns() - matches = set() - for d in dirs: - for pat in patterns: - matches.update(_glob.glob(os.path.join(d, "**", pat), recursive=True)) - found = sorted(matches) - - try: - (single,) = found - except ValueError: - pat_desc = " / ".join(patterns) - if not found: - raise ValueError( - f"no loadable extension file ({pat_desc}) found in package " - f"{name!r} (searched {', '.join(dirs)})" - ) from None - raise ValueError( - f"multiple loadable extension files found in package {name!r}: " - f"{', '.join(found)}; disambiguate with a literal path" - ) from None - return single +from .is_bare_name import is_bare_name +from .resolve_package import _resolve_package def resolve_extension_path(path, base, resolve_relative): diff --git a/packages/python/dirsql/resolve_extension_test.py b/packages/python/dirsql/resolve_extension_test.py index d79bf70e4..8bf42c241 100644 --- a/packages/python/dirsql/resolve_extension_test.py +++ b/packages/python/dirsql/resolve_extension_test.py @@ -1,238 +1,60 @@ -"""Unit tests for `resolve_extension`. +"""Unit tests for `resolve_extension_path`. -Every collaborator -- `importlib.util.find_spec`, the filesystem probes -(`os.path.isfile`), the loadable glob (`glob.glob`), and `sys.platform` -- is -mocked, so these isolate the resolution logic from any real package or disk. +The effectful collaborators -- the filesystem probe (`os.path.isfile`) and the +package resolver -- are mocked, so these isolate the ordered probe from any +real package or disk. The pure `is_bare_name` stays real. """ -import types from unittest import mock -import pytest - -import dirsql.resolve_extension as re - - -def _spec(*, locations=None, origin=None): - return types.SimpleNamespace(submodule_search_locations=locations, origin=origin) - - -def describe_is_bare_name(): - def it_treats_a_separatorful_value_as_a_path(): - assert re.is_bare_name("ext/vec0.so") is False - assert re.is_bare_name("/abs/ext.so") is False - - def it_treats_a_separatorful_value_without_a_loadable_suffix_as_a_path(): - assert re.is_bare_name("ext/foo") is False - assert re.is_bare_name("/abs/dir") is False - - def it_treats_a_loadable_suffix_as_a_path(): - assert re.is_bare_name("vec0.so") is False - assert re.is_bare_name("vec0.dylib") is False - assert re.is_bare_name("vec0.dll") is False - assert re.is_bare_name("vec0.pyd") is False - - def it_treats_a_plain_identifier_as_a_bare_name(): - assert re.is_bare_name("sqlite_vec") is True +import dirsql.resolve_extension as mod def describe_path_looking_values(): - def it_makes_a_relative_path_absolute_when_resolve_relative(tmp_path): - out = re.resolve_extension_path("ext/a.so", base="/cfg", resolve_relative=True) + def it_makes_a_relative_path_absolute_when_resolve_relative(): + out = mod.resolve_extension_path("ext/a.so", base="/cfg", resolve_relative=True) assert out == "/cfg/ext/a.so" def it_preserves_an_absolute_path_when_resolve_relative(): - out = re.resolve_extension_path("/abs/a.so", base="/cfg", resolve_relative=True) + out = mod.resolve_extension_path( + "/abs/a.so", base="/cfg", resolve_relative=True + ) assert out == "/abs/a.so" def it_returns_a_path_verbatim_when_not_resolve_relative(): - out = re.resolve_extension_path( + out = mod.resolve_extension_path( "relative/a.so", base="/cfg", resolve_relative=False ) assert out == "relative/a.so" -def describe_bare_name_shadowing(): +def describe_bare_names(): def it_uses_a_same_named_local_file_when_present(): - with mock.patch.object(re.os.path, "isfile", return_value=True) as isfile: - out = re.resolve_extension_path("vec", base="/cfg", resolve_relative=True) - assert out == "/cfg/vec" - isfile.assert_called_once_with("/cfg/vec") - - -def describe_bare_name_package_resolution(): - def it_globs_the_platform_loadable_inside_the_package_dir(): - with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object(re.sys, "platform", "linux"), - mock.patch.object( - re.importlib.util, - "find_spec", - return_value=_spec(locations=["/site/sqlite_vec"]), - ), - mock.patch.object( - re._glob, "glob", return_value=["/site/sqlite_vec/vec0.so"] - ) as glob, - ): - out = re.resolve_extension_path( - "sqlite_vec", base="/cfg", resolve_relative=True - ) - assert out == "/site/sqlite_vec/vec0.so" - glob.assert_called_once_with("/site/sqlite_vec/**/*.so", recursive=True) - - def it_falls_back_to_origin_dir_for_a_single_file_module(): - with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object(re.sys, "platform", "linux"), - mock.patch.object( - re.importlib.util, - "find_spec", - return_value=_spec(origin="/site/sqlite_vec/__init__.py"), - ), - mock.patch.object( - re._glob, "glob", return_value=["/site/sqlite_vec/vec0.so"] - ), - ): - out = re.resolve_extension_path( - "sqlite_vec", base="/cfg", resolve_relative=True - ) - assert out == "/site/sqlite_vec/vec0.so" - - def it_globs_dylib_on_macos(): - with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object(re.sys, "platform", "darwin"), - mock.patch.object( - re.importlib.util, - "find_spec", - return_value=_spec(locations=["/site/x"]), - ), - mock.patch.object( - re._glob, "glob", return_value=["/site/x/y.dylib"] - ) as glob, - ): - out = re.resolve_extension_path("x", base="/c", resolve_relative=True) - assert out == "/site/x/y.dylib" - glob.assert_called_once_with("/site/x/**/*.dylib", recursive=True) - - def it_globs_dll_and_pyd_on_windows(): with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object(re.sys, "platform", "win32"), - mock.patch.object( - re.importlib.util, - "find_spec", - return_value=_spec(locations=["/site/x"]), - ), - mock.patch.object( - re._glob, "glob", side_effect=[["/site/x/y.dll"], []] - ) as glob, + mock.patch.object(mod.os.path, "isfile", return_value=True) as isfile, + mock.patch.object(mod, "_resolve_package") as resolve_package, ): - out = re.resolve_extension_path("x", base="/c", resolve_relative=True) - assert out == "/site/x/y.dll" - assert glob.call_count == 2 - - def it_globs_so_on_any_other_platform(): - # aix sorts below "darwin" and zos above "win32", pinning the - # dispatch to equality rather than any ordering. - for platform in ("aix", "zos", "linux"): - with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object(re.sys, "platform", platform), - mock.patch.object( - re.importlib.util, - "find_spec", - return_value=_spec(locations=["/site/x"]), - ), - mock.patch.object( - re._glob, "glob", return_value=["/site/x/y.so"] - ) as glob, - ): - out = re.resolve_extension_path("x", base="/c", resolve_relative=True) - assert out == "/site/x/y.so" - glob.assert_called_once_with("/site/x/**/*.so", recursive=True) - - def it_dispatches_on_platform_string_equality_not_identity(): - # Runtime-built platform strings (not interned literals) must still - # dispatch correctly -- `sys.platform` is compared by value. - for platform, pattern, loadable in ( - ("".join(["dar", "win"]), "*.dylib", "/site/x/y.dylib"), - ("".join(["win", "32"]), "*.dll", "/site/x/y.dll"), - ): - with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object(re.sys, "platform", platform), - mock.patch.object( - re.importlib.util, - "find_spec", - return_value=_spec(locations=["/site/x"]), - ), - mock.patch.object( - re._glob, "glob", side_effect=[[loadable], []] - ) as glob, - ): - out = re.resolve_extension_path("x", base="/c", resolve_relative=True) - assert out == loadable - assert glob.call_args_list[0].args[0] == f"/site/x/**/{pattern}", ( - glob.call_args_list - ) - - def it_errors_when_the_package_is_not_installed(): - with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object(re.importlib.util, "find_spec", return_value=None), - pytest.raises(ValueError, match="not installed"), - ): - re.resolve_extension_path("nope", base="/c", resolve_relative=True) - - def it_wraps_a_find_spec_error(): - with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object( - re.importlib.util, "find_spec", side_effect=ImportError("boom") - ), - pytest.raises(ValueError, match="could not resolve extension package"), - ): - re.resolve_extension_path("nope", base="/c", resolve_relative=True) - - def it_errors_when_the_spec_has_no_package_directory(): - with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object( - re.importlib.util, - "find_spec", - return_value=_spec(origin="built-in"), - ), - pytest.raises(ValueError, match="no package directory"), - ): - re.resolve_extension_path("nope", base="/c", resolve_relative=True) + out = mod.resolve_extension_path("vec", base="/cfg", resolve_relative=True) + assert out == "/cfg/vec" + isfile.assert_called_once_with("/cfg/vec") + resolve_package.assert_not_called() - def it_errors_when_no_loadable_file_is_found(): + def it_resolves_the_package_when_no_local_file_shadows_it(): with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object(re.sys, "platform", "linux"), + mock.patch.object(mod.os.path, "isfile", return_value=False), mock.patch.object( - re.importlib.util, - "find_spec", - return_value=_spec(locations=["/site/x"]), - ), - mock.patch.object(re._glob, "glob", return_value=[]), - pytest.raises(ValueError, match="no loadable extension file"), + mod, "_resolve_package", return_value="/site/vec/vec0.so" + ) as resolve_package, ): - re.resolve_extension_path("x", base="/c", resolve_relative=True) + out = mod.resolve_extension_path("vec", base="/cfg", resolve_relative=True) + assert out == "/site/vec/vec0.so" + resolve_package.assert_called_once_with("vec") - def it_errors_when_multiple_loadable_files_are_found(): + def it_probes_the_shadow_file_even_when_not_resolve_relative(): with ( - mock.patch.object(re.os.path, "isfile", return_value=False), - mock.patch.object(re.sys, "platform", "linux"), - mock.patch.object( - re.importlib.util, - "find_spec", - return_value=_spec(locations=["/site/x"]), - ), - mock.patch.object( - re._glob, "glob", return_value=["/site/x/a.so", "/site/x/b.so"] - ), - pytest.raises(ValueError, match="multiple loadable extension files"), + mock.patch.object(mod.os.path, "isfile", return_value=True) as isfile, + mock.patch.object(mod, "_resolve_package"), ): - re.resolve_extension_path("x", base="/c", resolve_relative=True) + out = mod.resolve_extension_path("vec", base="/cfg", resolve_relative=False) + assert out == "/cfg/vec" + isfile.assert_called_once_with("/cfg/vec") diff --git a/packages/python/dirsql/resolve_package.py b/packages/python/dirsql/resolve_package.py new file mode 100644 index 000000000..eb0dfc5e1 --- /dev/null +++ b/packages/python/dirsql/resolve_package.py @@ -0,0 +1,53 @@ +"""Location of an extension package's loadable file for this platform. + +Zero matches and multiple matches are both hard errors -- the caller must +disambiguate with a literal path. +""" + +import glob as _glob +import importlib.util +import os + +from .platform_patterns import _platform_patterns + + +def _resolve_package(name): + """Locate ``name``'s package dir and glob its platform loadable file.""" + try: + spec = importlib.util.find_spec(name) + except (ImportError, ValueError) as exc: + raise ValueError( + f"could not resolve extension package {name!r}: {exc}" + ) from exc + if spec is None: + raise ValueError(f"could not resolve extension package {name!r}: not installed") + + dirs = list(spec.submodule_search_locations or []) + if not dirs and spec.origin and spec.origin not in ("built-in", "frozen"): + dirs.append(os.path.dirname(spec.origin)) + if not dirs: + raise ValueError( + f"could not resolve extension package {name!r}: no package directory" + ) + + patterns = _platform_patterns() + matches = set() + for d in dirs: + for pat in patterns: + matches.update(_glob.glob(os.path.join(d, "**", pat), recursive=True)) + found = sorted(matches) + + try: + (single,) = found + except ValueError: + pat_desc = " / ".join(patterns) + if not found: + raise ValueError( + f"no loadable extension file ({pat_desc}) found in package " + f"{name!r} (searched {', '.join(dirs)})" + ) from None + raise ValueError( + f"multiple loadable extension files found in package {name!r}: " + f"{', '.join(found)}; disambiguate with a literal path" + ) from None + return single diff --git a/packages/python/dirsql/resolve_package_test.py b/packages/python/dirsql/resolve_package_test.py new file mode 100644 index 000000000..6c20e3c2a --- /dev/null +++ b/packages/python/dirsql/resolve_package_test.py @@ -0,0 +1,176 @@ +"""Unit tests for `_resolve_package`. + +Every collaborator -- `importlib.util.find_spec`, the loadable glob +(`glob.glob`) and the platform pattern table -- is mocked, so these isolate the +package-location logic from any installed package or disk. +""" + +import types +from unittest import mock + +import pytest + +import dirsql.resolve_package as mod + + +def _spec(*, locations=None, origin=None): + return types.SimpleNamespace(submodule_search_locations=locations, origin=origin) + + +def _resolving(*, spec=None, find_spec_error=None, patterns=("*.so",), glob=None): + find_spec = ( + mock.patch.object(mod.importlib.util, "find_spec", side_effect=find_spec_error) + if find_spec_error + else mock.patch.object(mod.importlib.util, "find_spec", return_value=spec) + ) + return ( + find_spec, + mock.patch.object(mod, "_platform_patterns", return_value=patterns), + mock.patch.object(mod._glob, "glob", **(glob or {})), + ) + + +def describe_locating_the_package_directory(): + def it_globs_the_platform_loadable_inside_the_package_dir(): + find_spec, patterns, globbing = _resolving( + spec=_spec(locations=["/site/sqlite_vec"]), + glob={"return_value": ["/site/sqlite_vec/vec0.so"]}, + ) + with find_spec, patterns, globbing as glob: + assert mod._resolve_package("sqlite_vec") == "/site/sqlite_vec/vec0.so" + glob.assert_called_once_with("/site/sqlite_vec/**/*.so", recursive=True) + + def it_falls_back_to_origin_dir_for_a_single_file_module(): + find_spec, patterns, globbing = _resolving( + spec=_spec(origin="/site/sqlite_vec/__init__.py"), + glob={"return_value": ["/site/sqlite_vec/vec0.so"]}, + ) + with find_spec, patterns, globbing as glob: + assert mod._resolve_package("sqlite_vec") == "/site/sqlite_vec/vec0.so" + glob.assert_called_once_with("/site/sqlite_vec/**/*.so", recursive=True) + + def it_globs_every_pattern_the_platform_declares(): + find_spec, patterns, globbing = _resolving( + spec=_spec(locations=["/site/x"]), + patterns=("*.dll", "*.pyd"), + glob={"side_effect": [["/site/x/y.dll"], []]}, + ) + with find_spec, patterns, globbing as glob: + assert mod._resolve_package("x") == "/site/x/y.dll" + assert [c.args[0] for c in glob.call_args_list] == [ + "/site/x/**/*.dll", + "/site/x/**/*.pyd", + ] + + def it_globs_every_declared_package_directory(): + find_spec, patterns, globbing = _resolving( + spec=_spec(locations=["/site/a", "/site/b"]), + glob={"side_effect": [[], ["/site/b/y.so"]]}, + ) + with find_spec, patterns, globbing as glob: + assert mod._resolve_package("x") == "/site/b/y.so" + assert [c.args[0] for c in glob.call_args_list] == [ + "/site/a/**/*.so", + "/site/b/**/*.so", + ] + + +def describe_unresolvable_packages(): + def it_errors_when_the_package_is_not_installed(): + find_spec, patterns, globbing = _resolving(spec=None) + with ( + find_spec, + patterns, + globbing, + pytest.raises( + ValueError, + match=r"could not resolve extension package 'nope': not installed", + ), + ): + mod._resolve_package("nope") + + def it_wraps_a_find_spec_error(): + find_spec, patterns, globbing = _resolving(find_spec_error=ImportError("boom")) + with ( + find_spec, + patterns, + globbing, + pytest.raises( + ValueError, match=r"could not resolve extension package 'nope': boom" + ), + ): + mod._resolve_package("nope") + + def it_wraps_a_find_spec_value_error(): + find_spec, patterns, globbing = _resolving(find_spec_error=ValueError("bad")) + with ( + find_spec, + patterns, + globbing, + pytest.raises( + ValueError, match=r"could not resolve extension package 'nope': bad" + ), + ): + mod._resolve_package("nope") + + def it_errors_when_the_spec_has_no_package_directory(): + # A namespace-less builtin/frozen module, and one with no origin at + # all, are equally unresolvable -- neither names a directory to glob. + for origin in ("built-in", "frozen", None): + find_spec, patterns, globbing = _resolving(spec=_spec(origin=origin)) + with ( + find_spec, + patterns, + globbing, + pytest.raises( + ValueError, + match=( + r"could not resolve extension package 'nope': " + r"no package directory" + ), + ), + ): + mod._resolve_package("nope") + + +def describe_ambiguous_matches(): + def it_errors_when_no_loadable_file_is_found(): + find_spec, patterns, globbing = _resolving( + spec=_spec(locations=["/site/x"]), + patterns=("*.dll", "*.pyd"), + glob={"return_value": []}, + ) + with ( + find_spec, + patterns, + globbing, + pytest.raises( + ValueError, + match=( + r"no loadable extension file \(\*\.dll / \*\.pyd\) found in " + r"package 'x' \(searched /site/x\)" + ), + ), + ): + mod._resolve_package("x") + + def it_errors_when_multiple_loadable_files_are_found(): + # The listing is sorted, so the message is stable whatever order the + # globs returned the matches in. + find_spec, patterns, globbing = _resolving( + spec=_spec(locations=["/site/x"]), + glob={"return_value": ["/site/x/b.so", "/site/x/a.so"]}, + ) + with ( + find_spec, + patterns, + globbing, + pytest.raises( + ValueError, + match=( + r"multiple loadable extension files found in package 'x': " + r"/site/x/a\.so, /site/x/b\.so; disambiguate with a literal path" + ), + ), + ): + mod._resolve_package("x") diff --git a/packages/python/dirsql/toml_module.py b/packages/python/dirsql/toml_module.py new file mode 100644 index 000000000..838fddad9 --- /dev/null +++ b/packages/python/dirsql/toml_module.py @@ -0,0 +1,21 @@ +"""The TOML parser this interpreter reads ``.dirsql.toml`` files with.""" + +import sys +from importlib import import_module + + +def _load_toml_module(): + """Return the TOML parser module for the running interpreter. + + ``tomllib`` is stdlib only on 3.11+; on 3.10 the ``tomli`` backport it was + derived from (a version-gated dependency) provides the same surface. + Imported by name so a unit test can exercise both arms on any + interpreter -- a literal ``import tomllib`` is unreachable on 3.10 no + matter what ``sys.version_info`` claims. + """ + if sys.version_info >= (3, 11): + return import_module("tomllib") + return import_module("tomli") + + +_toml = _load_toml_module() diff --git a/packages/python/dirsql/toml_module_test.py b/packages/python/dirsql/toml_module_test.py new file mode 100644 index 000000000..8602c6d83 --- /dev/null +++ b/packages/python/dirsql/toml_module_test.py @@ -0,0 +1,40 @@ +"""Unit tests for `_load_toml_module`. + +`import_module` is mocked and `sys.version_info` patched so both arms run on +every interpreter -- neither `tomllib` (absent on 3.10) nor `tomli` (absent on +3.11+) is importable everywhere. +""" + +from unittest import mock + +import dirsql.toml_module as mod + + +def _requested(version): + with ( + mock.patch.object(mod.sys, "version_info", version), + mock.patch.object(mod, "import_module") as import_module, + ): + assert mod._load_toml_module() is import_module.return_value + (name,) = import_module.call_args.args + return name + + +def describe_load_toml_module(): + # Exactly `(3, 11)`, not `(3, 11, 0)`: the latter compares greater than the + # bare `(3, 11)` literal, so it cannot tell `>=` from `>`. + def it_uses_the_stdlib_parser_on_the_version_that_gained_it(): + assert _requested((3, 11)) == "tomllib" + + def it_uses_the_stdlib_parser_on_newer_versions(): + assert _requested((3, 12, 4)) == "tomllib" + + def it_uses_the_tomli_backport_below_311(): + assert _requested((3, 10, 17)) == "tomli" + + +def describe_module_binding(): + def it_binds_the_parser_the_running_interpreter_asked_for(): + assert mod._toml.__name__ == ( + "tomllib" if mod.sys.version_info >= (3, 11) else "tomli" + ) diff --git a/packages/python/e2e-attestations/claude-1066-p1-resolve-extension.json b/packages/python/e2e-attestations/claude-1066-p1-resolve-extension.json new file mode 100644 index 000000000..975b41d6d --- /dev/null +++ b/packages/python/e2e-attestations/claude-1066-p1-resolve-extension.json @@ -0,0 +1,7 @@ +{ + "command": "just test-e2e", + "ran_at": 1788573322, + "exit_code": 0, + "commit": "4acf00655f8ebdce4297edca6b4d01b999ddfdeb", + "branch": "claude/1066-p1-resolve-extension" +} diff --git a/packages/python/e2e-attestations/claude-1079-p2-resolve-config-extensions.json b/packages/python/e2e-attestations/claude-1079-p2-resolve-config-extensions.json new file mode 100644 index 000000000..8dabe3547 --- /dev/null +++ b/packages/python/e2e-attestations/claude-1079-p2-resolve-config-extensions.json @@ -0,0 +1,7 @@ +{ + "command": "just test-e2e", + "ran_at": 1788573708, + "exit_code": 0, + "commit": "fb7bf9e800f9d682e3b8e4b83f3d69cc8642fe69", + "branch": "claude/1079-p2-resolve-config-extensions" +}