diff --git a/drydock/analyzers/codemap.py b/drydock/analyzers/codemap.py index c79d1e0..cd22ac4 100644 --- a/drydock/analyzers/codemap.py +++ b/drydock/analyzers/codemap.py @@ -19,6 +19,7 @@ from ..core.models import Class, Constant, Export, FileId, Func, SourceFile, Symbols, TypeDef from ..core.registry import AnalysisContext, Option +from ..core.toml import load_toml from ..core.output import Reporter __all__ = ["CodemapAnalyzer"] @@ -293,15 +294,14 @@ def _find_entry_points(self, root: Path, files_data: list[dict[str, Any]]) -> li # Check pyproject.toml for [project.scripts] and [project.entry-points] pyproject_path = root / "pyproject.toml" if pyproject_path.exists(): - try: - try: - import tomllib - except ImportError: - import tomli as tomllib # type: ignore - - with open(pyproject_path, "rb") as f: - data = tomllib.load(f) - + # `load_toml` returns None when no TOML parser exists (Python + # 3.10 without the optional backport) instead of raising. The + # guarded import used to live here, and the `except` below listed + # only (OSError, ValueError) -- which does not catch + # ModuleNotFoundError -- so this analyzer crashed outright on a + # clean 3.10 install. + data = load_toml(pyproject_path) + if data is not None: project = data.get("project", {}) if "scripts" in project: for name, value in project["scripts"].items(): @@ -321,21 +321,12 @@ def _find_entry_points(self, root: Path, files_data: list[dict[str, Any]]) -> li {"path": entry_path, "source": value[:60]} ) seen.add(entry_path) - except (OSError, ValueError): - pass # Check Cargo.toml for [[bin]] cargo_path = root / "Cargo.toml" if cargo_path.exists(): - try: - try: - import tomllib - except ImportError: - import tomli as tomllib # type: ignore - - with open(cargo_path, "rb") as f: - data = tomllib.load(f) - + data = load_toml(cargo_path) + if data is not None: if "bin" in data: for bin_entry in data["bin"]: if "path" in bin_entry: @@ -346,8 +337,6 @@ def _find_entry_points(self, root: Path, files_data: list[dict[str, Any]]) -> li {"path": bin_path, "source": f"Cargo.toml [[bin]] {bin_name}"} ) seen.add(bin_path) - except (OSError, ValueError): - pass return entry_points diff --git a/drydock/analyzers/dependencies.py b/drydock/analyzers/dependencies.py index 26b2a7a..df630f1 100644 --- a/drydock/analyzers/dependencies.py +++ b/drydock/analyzers/dependencies.py @@ -18,6 +18,7 @@ from ..core.models import FileId from ..core.output import to_jsonable from ..core.registry import AnalysisContext, Option +from ..core.toml import loads_toml __all__ = ["DependenciesAnalyzer"] @@ -311,25 +312,18 @@ def _parse_pyproject(self, ctx: AnalysisContext) -> dict[str, str]: if not src: continue - # Try tomllib (Python 3.11+) - try: - import tomllib - - data = tomllib.loads(src) - except ImportError: - # Fallback for Python 3.10 - try: - import tomli as tomllib - - data = tomllib.loads(src) - except ImportError: - # Last resort: regex parsing - data = {} - for line in src.split("\n"): - m = re.match(r'^([a-z0-9_\-]+)\s*=', line, re.I) - if m: - pkg = m.group(1) - reqs[pkg.lower()] = "" + # A real parse when any TOML reader exists, otherwise the + # line-regex approximation below. `loads_toml` centralises the + # "which reader, if any" question -- see drydock/core/toml.py. + data = loads_toml(src) + if data is None: + # Last resort: regex parsing + data = {} + for line in src.split("\n"): + m = re.match(r'^([a-z0-9_\-]+)\s*=', line, re.I) + if m: + pkg = m.group(1) + reqs[pkg.lower()] = "" # Extract from [project] dependencies if "project" in data and "dependencies" in data["project"]: diff --git a/drydock/analyzers/platforms.py b/drydock/analyzers/platforms.py index 18bc4bb..c8cf161 100644 --- a/drydock/analyzers/platforms.py +++ b/drydock/analyzers/platforms.py @@ -15,22 +15,15 @@ from __future__ import annotations import json -import sys from dataclasses import dataclass, field from pathlib import Path from typing import Any, Mapping from ..core.registry import AnalysisContext, Option +from ..core.toml import load_toml __all__ = ["PlatformsAnalyzer"] -# Try to import tomllib (Python 3.11+), fall back gracefully for 3.10 -if sys.version_info >= (3, 11): - import tomllib -else: - tomllib = None # type: ignore[assignment] - - @dataclass(frozen=True, slots=True) class _PlatformDetection: """One platform detection result.""" @@ -528,13 +521,15 @@ def _extract_version( if value: return str(value), marker_file elif marker_type == "toml": - if tomllib is not None: - with open(marker_path, "rb") as f: - data = tomllib.load(f) - if json_path: - value = self._traverse_dict(data, json_path) - if value: - return str(value), marker_file + # `load_toml` returns None when no TOML reader is + # available -- on 3.10 the module-level guard used to be + # `sys.version_info >= (3, 11)`, so an installed `tomli` + # went unused and every TOML version marker was skipped. + data = load_toml(marker_path) + if data is not None and json_path: + value = self._traverse_dict(data, json_path) + if value: + return str(value), marker_file except Exception: pass diff --git a/drydock/catalog/cli.py b/drydock/catalog/cli.py index 305e8bd..23f5f63 100644 --- a/drydock/catalog/cli.py +++ b/drydock/catalog/cli.py @@ -370,7 +370,21 @@ def _cmd_ingest(args) -> int: "components_stored": result.components_stored, "facts_recorded": result.facts_recorded, "capabilities_recorded": result.capabilities_recorded, + "failures": result.failures, })) + + # Nothing recorded *and* something failed means the ingest did not do + # its job, so it must not report success -- a scripted run would go + # green over an empty catalog. Found by ingesting under a read-only + # HOME: the catalog could not be created, every phase failed, and this + # returned 0. An empty repo with no failures is still a clean exit. + if result.failures and result.components_recorded == 0: + print( + "error: ingest recorded nothing; " + + "; ".join(f"{phase}: {err}" for phase, err in result.failures.items()), + file=sys.stderr, + ) + return EXIT_ERROR return EXIT_OK except Exception as exc: print(f"error: ingest failed: {exc}", file=sys.stderr) @@ -592,6 +606,21 @@ def _cmd_diagram(args) -> int: catalog.close() +def _unwritable_catalog_message(catalog, exc: Exception) -> str: + """Say what to do, not just which errno. + + The default catalog lives under $HOME, which is not writable in a lot of + real environments -- CI runners, containers run as a non-root user, locked + down workstations. A bare "Permission denied: '/rohome/.drydock'" does not + tell anyone that `--catalog` and `DRYDOCK_CATALOG` exist. + """ + return ( + f"error: cannot use the catalog at {catalog.root}: {exc}\n" + "hint: choose a writable location with --catalog PATH " + "or the DRYDOCK_CATALOG environment variable" + ) + + def _cmd_stats(args) -> int: """Execute the stats subcommand.""" catalog = Catalog(args.catalog or None) @@ -602,6 +631,9 @@ def _cmd_stats(args) -> int: else: _emit(args, _format_stats(stats)) return EXIT_OK + except PermissionError as exc: + print(_unwritable_catalog_message(catalog, exc), file=sys.stderr) + return EXIT_ERROR except Exception as exc: print(f"error: {exc}", file=sys.stderr) return EXIT_ERROR diff --git a/drydock/core/toml.py b/drydock/core/toml.py new file mode 100644 index 0000000..6d0eadb --- /dev/null +++ b/drydock/core/toml.py @@ -0,0 +1,82 @@ +"""Reading TOML without requiring a TOML reader. + +`tomllib` entered the stdlib in 3.11, and this package supports 3.10 with no +dependencies. So on 3.10 there may be no TOML parser at all, and every caller +has to cope with that. + +Each caller coping separately is how this broke: the guarded import was +duplicated at four sites, and at one of them the surrounding `except` clause +listed `(OSError, ValueError)` — which does not catch `ModuleNotFoundError`. +`drydock codemap` therefore crashed outright on a clean Python 3.10 install. +CI never saw it, because CI installs the `dev` extra, which pulls in `tomli`. + +One function, one place to get it right. A missing parser is not an error here: +TOML files are read for enrichment (entry points, platform versions, declared +dependencies), and the correct behaviour without a reader is to return nothing +and carry on. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +__all__ = ["load_toml", "loads_toml", "toml_available"] + +_READER: Any = None +_CHECKED = False + + +def _reader() -> Any: + global _READER, _CHECKED + if not _CHECKED: + _CHECKED = True + try: + import tomllib as _t # Python 3.11+ + except ModuleNotFoundError: + try: + import tomli as _t # 3.10 backport, optional + except ModuleNotFoundError: + _t = None + _READER = _t + return _READER + + +def toml_available() -> bool: + """Whether any TOML parser is importable in this environment.""" + return _reader() is not None + + +def load_toml(path: Path | str) -> dict[str, Any] | None: + """Parse a TOML file, or return None. + + Returns None when there is no parser available, the file is missing, or it + is malformed. Never raises -- callers read TOML to enrich a result, and a + missing enrichment must not sink an analysis. + """ + reader = _reader() + if reader is None: + return None + try: + with open(path, "rb") as handle: + data = reader.load(handle) + except (OSError, ValueError, AttributeError): + return None + return data if isinstance(data, dict) else None + + +def loads_toml(source: str) -> dict[str, Any] | None: + """Parse TOML already held as text, or return None. + + Same contract as `load_toml` for callers that read the file themselves + (the walker hands analyzers decoded source, so re-reading from disk would + be a second read and a second chance to disagree about encoding). + """ + reader = _reader() + if reader is None: + return None + try: + data = reader.loads(source) + except (ValueError, AttributeError, TypeError): + return None + return data if isinstance(data, dict) else None diff --git a/tests/container/README.md b/tests/container/README.md new file mode 100644 index 0000000..f8b0d93 --- /dev/null +++ b/tests/container/README.md @@ -0,0 +1,41 @@ +# Cold-environment smoke tests + +`pytest` runs inside a developer environment that already has everything. These +run the new-user command sequence inside a container that has nothing, which is +where a stranger's first ten minutes actually happen. + +They are not part of the pytest suite — they need Docker, and they install the +package for real rather than importing it from the working tree. + +```sh +for img in python:3.10-slim python:3.13-slim python:3.13-alpine; do + docker run --rm -v "$PWD:/src:ro" -v "$PWD/tests/container:/ctest:ro" \ + -e HOME=/tmp/home -w /tmp "$img" \ + sh -c 'mkdir -p /tmp/home && sh /ctest/smoke.sh' +done +``` + +Non-root, which is how most CI runners and hardened containers execute: + +```sh +docker run --rm -u 1001:1001 -v "$PWD:/src:ro" -v "$PWD/tests/container:/ctest:ro" \ + -e HOME=/tmp/home -w /tmp python:3.13-slim \ + sh -c 'mkdir -p /tmp/home && python -m pip install --user --quiet /src && \ + export PATH=$HOME/.local/bin:$PATH && sh /ctest/smoke.sh' +``` + +## What this has caught + +Both of these were green in CI and green locally. + +- **`codemap` crashed on a clean 3.10.** `tomllib` arrived in 3.11 and the + guarded fallback import was duplicated at several call sites; at one of them + the surrounding `except` listed `(OSError, ValueError)`, which does not catch + `ModuleNotFoundError`. CI missed it because CI installs the `dev` extra, + which pulls in `tomli`. Now behind `drydock/core/toml.py`. +- **`catalog ingest` exited 0 after recording nothing** when `$HOME` was not + writable — a scripted run went green over an empty catalog. + +Note the second one needs a *non-root* container: root ignores directory mode +bits, so a read-only `$HOME` is not read-only for root and the check passes +without testing anything. diff --git a/tests/container/smoke.sh b/tests/container/smoke.sh new file mode 100755 index 0000000..9310a42 --- /dev/null +++ b/tests/container/smoke.sh @@ -0,0 +1,30 @@ +#!/bin/sh +# Runs inside the container. Every check is a stranger's first ten minutes. +set -u +pass=0; fail=0 +chk() { # name, command... + n="$1"; shift + if out=$("$@" 2>&1); then printf ' ok %s\n' "$n"; pass=$((pass+1)) + else printf ' FAIL %s :: %s\n' "$n" "$(printf '%s' "$out" | tail -1 | cut -c1-58)"; fail=$((fail+1)); fi +} +cd /src +python -m pip install --quiet . >/dev/null 2>&1 || { echo " FAIL install"; exit 1; } +echo " (installed; git present: $(command -v git >/dev/null 2>&1 && echo yes || echo NO))" +chk "drydock --version" drydock --version +chk "drydock list" drydock list +chk "structure" drydock structure /src -o /dev/null +chk "codemap" drydock codemap /src -o /dev/null +chk "architecture" drydock architecture /src -o /dev/null +chk "security" drydock security /src -o /dev/null +chk "boundaries" drydock boundaries /src -o /dev/null +chk "interfaces" drydock interfaces /src -o /dev/null +chk "context" drydock context /src -o /dev/null +chk "extract --list-seams" drydock extract /src --list-seams -o /dev/null +chk "catalog stats" drydock catalog stats -o /dev/null +chk "catalog ingest" drydock catalog ingest /src -o /dev/null +chk "catalog search" drydock catalog search parser -o /dev/null +# git analyzer must degrade gracefully when git is absent +if drydock git /src -o /dev/null >/dev/null 2>&1; then printf ' ok git (repo analysed)\n'; pass=$((pass+1)) +else printf ' ok git (declined cleanly, exit %s)\n' "$?"; pass=$((pass+1)); fi +echo " -> $pass passed, $fail failed" +[ "$fail" -eq 0 ] diff --git a/tests/test_catalog_unwritable.py b/tests/test_catalog_unwritable.py new file mode 100644 index 0000000..a9a02af --- /dev/null +++ b/tests/test_catalog_unwritable.py @@ -0,0 +1,79 @@ +"""The catalog lives under $HOME, which is not always writable. + +CI runners, containers run as a non-root user and locked-down workstations all +hit this. Found by running `drydock catalog ingest` inside a container with a +read-only HOME: every phase failed, the command printed warnings, and then +**exited 0** -- a scripted run would have gone green over an empty catalog. +""" + +from __future__ import annotations + +import stat + +import pytest + +from drydock.catalog.cli import main as catalog_main + + +@pytest.fixture +def unwritable_root(tmp_path): + """A directory this process genuinely cannot write to. + + Skips rather than passing vacuously when the permission does not bind -- + root ignores the mode bits, so as root this fixture would hand back a + perfectly writable directory and every assertion below would still pass + while testing nothing. + """ + root = tmp_path / "rohome" + root.mkdir() + root.chmod(stat.S_IRUSR | stat.S_IXUSR) # r-x------ + try: + canary = root / "canary" + canary.write_text("") + except (PermissionError, OSError): + pass + else: + canary.unlink() + root.chmod(stat.S_IRWXU) + pytest.skip("cannot make a directory unwritable for this user (running as root?)") + + yield root / "catalog" + root.chmod(stat.S_IRWXU) # so pytest can clean up + + +def test_ingest_into_an_unwritable_catalog_exits_nonzero(unwritable_root, tmp_path, capsys): + """The shipped bug: warnings printed, exit code 0, catalog empty.""" + project = tmp_path / "proj" + project.mkdir() + (project / "mod.py").write_text("def f():\n return 1\n") + + code = catalog_main(["ingest", str(project), "--catalog", str(unwritable_root)]) + + assert code != 0, "ingest reported success while recording nothing" + assert "recorded nothing" in capsys.readouterr().err + + +def test_stats_on_an_unwritable_catalog_names_the_escape_hatch(unwritable_root, capsys): + """A bare errno does not tell anyone that --catalog exists.""" + code = catalog_main(["stats", "--catalog", str(unwritable_root)]) + + assert code != 0 + err = capsys.readouterr().err + assert "--catalog" in err and "DRYDOCK_CATALOG" in err, err + + +def test_successful_ingest_still_exits_zero(tmp_path): + """The guard must not turn ordinary partial results into failures.""" + project = tmp_path / "proj" + project.mkdir() + (project / "mod.py").write_text("def f():\n return 1\n") + + assert catalog_main(["ingest", str(project), "--catalog", str(tmp_path / "cat")]) == 0 + + +def test_empty_project_ingests_cleanly(tmp_path): + """Zero components with zero failures is a clean exit, not an error.""" + project = tmp_path / "empty" + project.mkdir() + + assert catalog_main(["ingest", str(project), "--catalog", str(tmp_path / "cat")]) == 0 diff --git a/tests/test_toml_optional.py b/tests/test_toml_optional.py new file mode 100644 index 0000000..eba9396 --- /dev/null +++ b/tests/test_toml_optional.py @@ -0,0 +1,138 @@ +"""TOML is optional, and every analyzer that reads it must survive its absence. + +Python gained `tomllib` in 3.11. Drydock supports 3.10 with no required +dependencies, so on 3.10 there may be no TOML parser at all. + +This was a real, shipped crash: `drydock codemap` died with +`ModuleNotFoundError: No module named 'tomli'` on a clean python:3.10-slim, +because the guarded import was duplicated at several call sites and one of +them wrapped it in `except (OSError, ValueError)` -- which does not catch +`ModuleNotFoundError`. CI never saw it because CI installs the `dev` extra, +which pulls in `tomli`. + +These tests run the analyzers with the TOML reader forced to None, which is +what a bare 3.10 looks like from inside the process. +""" + +from __future__ import annotations + +import sys + +import pytest + +from drydock.core import toml as toml_mod +from drydock.core.registry import AnalysisContext +from drydock.analyzers.codemap import CodemapAnalyzer +from drydock.analyzers.dependencies import DependenciesAnalyzer +from drydock.analyzers.platforms import PlatformsAnalyzer + + +class _BlockTomlImports: + """Meta-path hook that makes every TOML module unimportable. + + Patching the helper's cached `_READER` is not enough: it only simulates + absence for code that goes through the helper, so the original crash -- + which came from a direct `import tomli` at the call site -- would still + pass. Blocking at the import system reproduces a bare 3.10 for *any* code + path, which is the only version of this fixture that can catch a + regression back to a local guarded import. + """ + + BLOCKED = ("tomllib", "tomli") + + def find_module(self, fullname, path=None): # pragma: no cover - legacy API + return None + + def find_spec(self, fullname, path=None, target=None): + if fullname in self.BLOCKED: + raise ModuleNotFoundError(f"No module named {fullname!r}", name=fullname) + return None + + +@pytest.fixture +def no_toml_reader(monkeypatch): + """Make the environment look like 3.10 without the `tomli` backport.""" + for name in _BlockTomlImports.BLOCKED: + monkeypatch.delitem(sys.modules, name, raising=False) + monkeypatch.setattr(sys, "meta_path", [_BlockTomlImports(), *sys.meta_path]) + + # Drop the helper's memoised reader so it re-resolves under the block. + monkeypatch.setattr(toml_mod, "_READER", None) + monkeypatch.setattr(toml_mod, "_CHECKED", False) + + with pytest.raises(ModuleNotFoundError): + import tomllib # noqa: F401 + with pytest.raises(ModuleNotFoundError): + import tomli # noqa: F401 + assert toml_mod.toml_available() is False + + +@pytest.fixture +def toml_project(tmp_path): + """A project whose only interesting metadata lives in TOML files.""" + (tmp_path / "pyproject.toml").write_text( + '[project]\n' + 'name = "widget"\n' + 'requires-python = ">=3.10"\n' + 'dependencies = ["requests>=2.0", "click"]\n' + '\n' + '[project.scripts]\n' + 'widget = "widget.cli:main"\n' + ) + (tmp_path / "widget").mkdir() + (tmp_path / "widget" / "__init__.py").write_text("") + (tmp_path / "widget" / "cli.py").write_text( + "import requests\n\n\ndef main() -> None:\n print(requests)\n" + ) + return tmp_path + + +ANALYZERS = [CodemapAnalyzer, DependenciesAnalyzer, PlatformsAnalyzer] + + +@pytest.mark.parametrize("cls", ANALYZERS, ids=lambda c: c.name) +def test_analyzer_runs_without_a_toml_reader(cls, toml_project, no_toml_reader): + """The shipped 3.10 crash: this raised ModuleNotFoundError for codemap.""" + result = cls().run(AnalysisContext(root=toml_project)) + + assert isinstance(result, dict) + assert "summary" in result, f"{cls.name} produced no summary" + + +@pytest.mark.parametrize("cls", ANALYZERS, ids=lambda c: c.name) +def test_analyzer_uses_toml_when_a_reader_exists(cls, toml_project): + """Guard against 'fixing' the crash by never reading TOML at all. + + Each analyzer must extract something from pyproject.toml that it cannot + get any other way, so a regression to a no-op is visible here. + """ + if not toml_mod.toml_available(): # pragma: no cover - 3.10 without tomli + pytest.skip("no TOML reader in this environment") + + result = cls().run(AnalysisContext(root=toml_project)) + blob = repr(result) + + # Each of these facts exists ONLY inside pyproject.toml. + expected = { + "codemap": "widget.cli:main", # [project.scripts] + "dependencies": "requests", # [project] dependencies + "platforms": ">=3.10", # project.requires-python + }[cls.name] + assert expected in blob, f"{cls.name} did not read {expected!r} from pyproject.toml" + + +def test_absent_reader_degrades_rather_than_erroring(toml_project, no_toml_reader): + """Without a reader the TOML-only facts are simply missing, not faked.""" + result = CodemapAnalyzer().run(AnalysisContext(root=toml_project)) + assert "widget.cli:main" not in repr(result) + + +def test_load_toml_never_raises(tmp_path): + """Malformed, missing and non-mapping input all return None.""" + bad = tmp_path / "bad.toml" + bad.write_text("this is = = not toml") + assert toml_mod.load_toml(bad) is None + assert toml_mod.load_toml(tmp_path / "absent.toml") is None + assert toml_mod.load_toml(tmp_path) is None # a directory + assert toml_mod.loads_toml("also = = broken") is None + assert toml_mod.loads_toml("") == {}