From da5206927d5f0e1bfdb9ab90f355055a5c1292bc Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Singh Date: Sun, 27 Sep 2026 00:31:55 +0530 Subject: [PATCH] ci: the drift job checks an installed wheel, not just the source tree Closes #103. #116 already covered most of what that issue asked for -- a weekly resolve that ignores `uv.lock`, reporting to the issue tracker, not gating pull requests. What it did not cover is the criterion's other half: importing every subpackage from the *built wheel*. The distinction is the one #101 was: `mcp>=1.2` resolved to 2.0.0 for anyone installing fresh, `mcp.server.fastmcp` had gone, and `grapharc[mcp]` was broken on arrival while every locked job stayed green. A suite run from the source tree does not see a packaging break that only shows in an installed wheel -- a subpackage dropped from the build, or an extra whose new major moves a module the package imports at import time. The walk moves out of `ci.yml`'s heredoc into `scripts/wheel_import_walk.py`, because two jobs now need exactly this check and two copies would drift apart. Same logic, two parameters: which checkout to compare against, and the prefix the import must come from. `scripts/` holds no `grapharc` package, so running it by path does not put the checkout on `sys.path` -- the property the old `cd /tmp` was there for, kept and now asserted with a message rather than a bare `assert`. Verified by running it for real rather than reasoning about it, and it earned its keep immediately: against the wheel then sitting in `dist/` it reported `grapharc.examples.plan_research` missing -- correctly, because that wheel was built before #115 merged. Rebuilt, it walks 132 modules clean. Deliberately **not** done: putting a ceiling on the eight unbounded extras. The issue asks for a decision on that and argues a bound should be "a ceiling with a reason attached, not a reflex" -- so adding eight of them at once is the reflex it warns against, and it would refuse users upgrades that are fine. Detection is what this closes; the policy is left open. Verified: 2214 selected, 13 deselected, ruff clean; the sdist's required-files and secret-leak checks still pass with `scripts/` added. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 63 ++++++++++-------------- scripts/wheel_import_walk.py | 95 ++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 38 deletions(-) create mode 100644 scripts/wheel_import_walk.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7ab6ae..0a71de3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -236,6 +236,25 @@ jobs: run: | uv sync --all-extras --group dev uv run pytest + - name: The wheel still imports against the re-resolved versions + # The other half of #103. The suite above runs from the source tree, so a + # packaging break that only shows in an *installed* wheel -- a subpackage + # dropped, or an extra whose new major moves a module the package imports + # at import time -- would not surface there. #101 was exactly that shape: + # `mcp>=1.2` resolved to 2.0.0 for anyone installing fresh, + # `mcp.server.fastmcp` had gone, and `grapharc[mcp]` was broken on + # arrival while every locked job stayed green. + # + # Same script the build job runs, pointed at a clean environment built + # from `pyproject.toml`'s ranges rather than from `uv.lock`. + run: | + uv build + uv venv --python 3.12 /tmp/driftcheck + uv pip install --python /tmp/driftcheck/bin/python "$(echo dist/*.whl)[all]" + cd /tmp + /tmp/driftcheck/bin/python "$GITHUB_WORKSPACE/scripts/wheel_import_walk.py" \ + --source-tree "$GITHUB_WORKSPACE" --expect-prefix /tmp/driftcheck/ + /tmp/driftcheck/bin/grapharc --version - name: Say so where someone will see it # A scheduled run reports to nobody. This repository has already paid # for that: `pages.yml` failed on two consecutive pushes and the @@ -291,48 +310,16 @@ jobs: run: uvx twine check --strict dist/* - name: Wheel installs and imports in a clean environment # Runs from /tmp so an `import grapharc` cannot fall back to the - # checked-out source tree and pass for the wrong reason. + # checked-out source tree and pass for the wrong reason. The check is + # `scripts/wheel_import_walk.py` rather than a heredoc because + # `upstream-drift` needs exactly the same one against re-resolved + # dependencies, and two copies of it would drift apart. run: | uv venv --python 3.12 /tmp/wheelcheck uv pip install --python /tmp/wheelcheck/bin/python "$(echo dist/*.whl)[all]" cd /tmp - SOURCE_TREE="$GITHUB_WORKSPACE" /tmp/wheelcheck/bin/python - <<'PY' - import importlib - import os - import pkgutil - import sys - from pathlib import Path - - import grapharc - - assert "/tmp/wheelcheck/" in grapharc.__file__, grapharc.__file__ - from grapharc import Budget, GraphARC, GraphARCState # noqa: F401 - from grapharc.gateway import get_model # noqa: F401 - from grapharc.harness import Harness # noqa: F401 - - # Compared against the checkout rather than a magic number. A `walked - # > N` check cannot notice a whole subpackage going missing, and one - # did go missing in testing: hatchling treats `.gitignore` as a build - # exclusion unless `ignore-vcs` is set, and the build still succeeds. - source = Path(os.environ["SOURCE_TREE"]) / "grapharc" - expected = { - ".".join(("grapharc", *path.relative_to(source).parts))[: -len(".py")].removesuffix( - ".__init__" - ) - for path in source.rglob("*.py") - if "__pycache__" not in path.parts - } - installed = {m.name for m in pkgutil.walk_packages(grapharc.__path__, "grapharc.")} - installed.add("grapharc") - - missing = sorted(expected - installed) - if missing: - sys.exit(f"in the source tree but not in the wheel: {missing}") - - for name in sorted(installed): - importlib.import_module(name) - print(f"ok: {len(installed)} modules imported from wheel {grapharc.__version__}") - PY + /tmp/wheelcheck/bin/python "$GITHUB_WORKSPACE/scripts/wheel_import_walk.py" \ + --source-tree "$GITHUB_WORKSPACE" --expect-prefix /tmp/wheelcheck/ /tmp/wheelcheck/bin/grapharc --version - name: Sdist installs and imports in a clean environment run: | diff --git a/scripts/wheel_import_walk.py b/scripts/wheel_import_walk.py new file mode 100644 index 0000000..39fd09a --- /dev/null +++ b/scripts/wheel_import_walk.py @@ -0,0 +1,95 @@ +"""Every module in the source tree is in the installed wheel, and imports. + +Run against a *clean* environment that has `grapharc` installed from a built +wheel, from a working directory outside the checkout — otherwise `import +grapharc` can fall back to the source tree and pass for the wrong reason. This +file lives in `scripts/`, which contains no `grapharc` package, so running it by +path does not put the checkout on `sys.path` either. + +Two jobs need exactly this check, which is why it is a file rather than a +heredoc: + +- `build`, against the dependencies `uv.lock` pins — the shipped artifact is + importable; +- `upstream-drift`, against dependencies re-resolved from `pyproject.toml` with + no ceiling — the shipped artifact is *still* importable once upstream moves. + That is the half issue #103 asked for: `uv.lock` hides a new major from every + other job, because keeping development reproducible is the lockfile's whole + job. + +The comparison is against the checkout rather than a magic number. A `walked > +N` check cannot notice a whole subpackage going missing, and one did go missing +in testing: hatchling treats `.gitignore` as a build exclusion unless +`ignore-vcs` is set, and the build still succeeds. +""" + +from __future__ import annotations + +import argparse +import importlib +import pkgutil +import sys +from pathlib import Path + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--source-tree", + required=True, + type=Path, + help="the checkout to compare against (the directory holding grapharc/)", + ) + parser.add_argument( + "--expect-prefix", + required=True, + help="a path fragment the imported package must come from, e.g. /tmp/wheelcheck/", + ) + args = parser.parse_args() + + import grapharc + + if args.expect_prefix not in grapharc.__file__: + return _fail( + f"imported grapharc from {grapharc.__file__}, which is not under " + f"{args.expect_prefix!r} — the installed wheel is not what was imported" + ) + + # The public entry points, named explicitly: these are what a reader of the + # README types first, so a wheel that walks cleanly and cannot do these is + # still broken. + from grapharc import Budget, GraphARC, GraphARCState # noqa: F401 + from grapharc.gateway import get_model # noqa: F401 + from grapharc.harness import Harness # noqa: F401 + + source = args.source_tree / "grapharc" + if not source.is_dir(): + return _fail(f"no grapharc package under {args.source_tree}") + + expected = { + ".".join(("grapharc", *path.relative_to(source).parts))[: -len(".py")].removesuffix( + ".__init__" + ) + for path in source.rglob("*.py") + if "__pycache__" not in path.parts + } + installed = {module.name for module in pkgutil.walk_packages(grapharc.__path__, "grapharc.")} + installed.add("grapharc") + + missing = sorted(expected - installed) + if missing: + return _fail(f"in the source tree but not in the wheel: {missing}") + + for name in sorted(installed): + importlib.import_module(name) + print(f"ok: {len(installed)} modules imported from wheel {grapharc.__version__}") + return 0 + + +def _fail(message: str) -> int: + print(f"error: {message}", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main())