From 95d0eaa77d8208e87905e0854ebb03d51871a128 Mon Sep 17 00:00:00 2001 From: Filipe Oliveira <144428646+filipeaaoliveira@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:10:13 +0200 Subject: [PATCH] fix: verify built artifacts and gate release tags against pyproject version The test job installs from the source tree, where diagrams/cli.py and resources/ are always present, so two broken releases went out green: - 0.24.1-0.24.4 declared a `diagrams = diagrams.cli:main` console script while diagrams/cli.py did not exist, so `pipx install diagrams` installed an executable that could only raise ModuleNotFoundError (#1149). - 0.25.0 moved to a PEP 621 [project] table with no [build-system] table, so pip built with setuptools and shipped a wheel with zero icons. It does not raise: the CLI exits 0 and renders an 8 KB image of empty boxes. Separately, the v0.24.4 tag was placed one commit before the version bump, so the tag said 0.24.4 while pyproject.toml said 0.24.3 (#1183). Nothing compared the two. Add tests/test_packaging.py, which asserts against installed metadata and on-disk files rather than the checkout: every declared console-script entry point imports, every Node subclass's icon resolves through the production Node._load_icon path, and diagrams.__version__ matches the distribution metadata. A setUpModule guard skips the module, with an instruction, when `import diagrams` does not resolve to the installed distribution, so an unpacked sdist or a shadowed checkout cannot pass it vacuously. Add .github/workflows/package.yml to build both artifacts, inspect the sdist listing, install the wheel into a clean venv and run those invariants against the install, plus the console script end to end. Add scripts/check_release_version.py and a tag-version job that fails a release whose tag does not name the version in pyproject.toml. The version stays a single literal in pyproject.toml; diagrams.__version__ now reads it back via importlib.metadata, reporting 0.0.0.dev0 when the imported package is not the installed copy rather than echoing the wrong release number. Split [tool.hatch.build] into per-target tables so the sdist also carries tests/ and scripts/check_release_version.py, letting packagers verify a build from the tarball without cloning. Turn diagrams/gis/cplusplus.py into a deprecation shim aliasing diagrams.gis.cli.Mapnik. The new icon test caught it: resources/gis/cplusplus was never committed alongside the generated module in #847, so gis.cplusplus.Mapnik has always rendered a blank node. docs/nodes/gis.md, generated by the same autogen.sh run, already omits the module and documents Mapnik at diagrams.gis.cli.Mapnik, which has a real icon. --- .github/workflows/package.yml | 118 ++++++++++++++++++++++++++++ CHANGELOG.md | 8 ++ diagrams/__init__.py | 23 ++++++ diagrams/gis/cplusplus.py | 25 +++--- pyproject.toml | 22 +++++- scripts/check_release_version.py | 70 +++++++++++++++++ tests/test_check_release_version.py | 52 ++++++++++++ tests/test_packaging.py | 117 +++++++++++++++++++++++++++ 8 files changed, 423 insertions(+), 12 deletions(-) create mode 100644 .github/workflows/package.yml create mode 100644 scripts/check_release_version.py create mode 100644 tests/test_check_release_version.py create mode 100644 tests/test_packaging.py diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml new file mode 100644 index 00000000..a6ede896 --- /dev/null +++ b/.github/workflows/package.yml @@ -0,0 +1,118 @@ +name: Verify built artifacts + +# The regular test job installs from the source tree, where `diagrams/cli.py` +# and `resources/` are always present. That is why #1149 (console script +# pointing at a module that was never shipped) and #1099 (0.25.0 wheel built +# with no resources at all) both reached PyPI green. This workflow tests the +# sdist and the wheel instead. + +on: + push: + branches: + - master + tags: + - "v*" + pull_request: + branches: + - master + paths: + - ".github/workflows/package.yml" + - "pyproject.toml" + - "resources/**" + - "**.py" + +jobs: + artifacts: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Setup Graphviz + uses: ts-graphviz/setup-graphviz@v2 + + - name: Build sdist and wheel + run: | + python -m pip install --upgrade build + python -m build + + - name: The sdist must carry the CLI module and the icon tree + # Only the sdist is inspected by listing: the wheel is covered more + # strongly below, where it is installed and tests/test_packaging.py + # loads every entry point and resolves every icon on disk. + run: | + set -euo pipefail + archive=$(ls dist/*.tar.gz) + # Write the listing to a file rather than piping it into grep: `grep -q` + # exits on the first match and closes the pipe, which kills the writer + # with EPIPE and makes pipefail report a false failure. + tar tzf "$archive" | cut -d/ -f2- > /tmp/sdist-names.txt + grep -qx 'diagrams/cli.py' /tmp/sdist-names.txt \ + || { echo "::error::$archive is missing diagrams/cli.py (issue #1149)"; exit 1; } + icons=$(grep -c '^resources/.*\.png$' /tmp/sdist-names.txt || true) + echo "$archive contains $icons icons" + [ "$icons" -gt 2000 ] \ + || { echo "::error::$archive contains only $icons icons (issue #1099)"; exit 1; } + + - name: Install the wheel into a clean environment + run: | + python -m venv /tmp/wheelenv + /tmp/wheelenv/bin/pip install dist/*.whl + + - name: Packaging invariants must hold for the installed distribution + run: | + set -euo pipefail + # Run from a scratch directory so the checkout cannot shadow the + # installed package - that shadowing is exactly what hid these bugs. + mkdir -p /tmp/pkgcheck + cp tests/test_packaging.py /tmp/pkgcheck/ + cd /tmp/pkgcheck + /tmp/wheelenv/bin/python -m unittest -v test_packaging 2>&1 | tee result.txt + # The suite skips itself when it cannot see an installed distribution. + # Here it must actually run, otherwise this step passes without + # checking the wheel at all. + grep -q 'skipped' result.txt \ + && { echo "::error::packaging invariants were skipped, not run against the wheel"; exit 1; } + true + + - name: Console script must render a diagram with real icons + run: | + set -euo pipefail + mkdir -p /tmp/e2e && cd /tmp/e2e + cat > example.py <<'EOF' + from diagrams import Diagram + from diagrams.aws.compute import EC2 + from diagrams.aws.database import RDS + from diagrams.aws.network import ELB + + with Diagram("web_service", show=False): + ELB("lb") >> EC2("web") >> RDS("userdb") + EOF + /tmp/wheelenv/bin/diagrams example.py + # A diagram whose icons failed to load still exits 0 and still writes a + # PNG - 0.25.0 produced an 8 KB image of empty boxes - so assert on size. + size=$(wc -c < web_service.png) + echo "rendered web_service.png: ${size} bytes" + [ "$size" -gt 20000 ] \ + || { echo "::error::rendered diagram is ${size} bytes; icons did not load (issue #1099)"; exit 1; } + + - name: pipx install of the wheel must provide a working console script + run: | + set -euo pipefail + python -m pip install --upgrade pipx + python -m pipx install dist/*.whl + cd /tmp/e2e && rm -f web_service.png + "$(python -m pipx environment --value PIPX_BIN_DIR)/diagrams" example.py + test -s web_service.png + + tag-version: + if: startsWith(github.ref, 'refs/tags/v') + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Tag must name the version in pyproject.toml + run: python -m scripts.check_release_version "${GITHUB_REF_NAME}" diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a832169..f44d95a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelogs +## Unreleased + +### Deprecated + +- `diagrams.gis.cplusplus.Mapnik` is now an alias of `diagrams.gis.cli.Mapnik` and warns on + import. The `gis.cplusplus` module never had an icon directory, so it always rendered a + blank node; the alias will be removed in the next major release. + ## 0.17.0 ### Added diff --git a/diagrams/__init__.py b/diagrams/__init__.py index 062ecae2..1fa343b0 100644 --- a/diagrams/__init__.py +++ b/diagrams/__init__.py @@ -1,4 +1,5 @@ import contextvars +import importlib.metadata import os import uuid from pathlib import Path @@ -6,6 +7,28 @@ from typing import Dict, List, Optional, Union from graphviz import Digraph + +def _distribution_version() -> str: + # Single source of truth is `version` in pyproject.toml; read it back from the + # installed distribution rather than repeating the literal here. When this + # import is a source checkout shadowing a separately installed release, the + # environment's metadata describes the install, not this code - report the + # dev placeholder rather than the wrong release number. + try: + dist = importlib.metadata.distribution("diagrams") + except importlib.metadata.PackageNotFoundError: + return "0.0.0.dev0" + installed_init = Path(str(dist.locate_file("diagrams/__init__.py"))) + try: + if installed_init.exists() and installed_init.resolve() != Path(__file__).resolve(): + return "0.0.0.dev0" + except OSError: # pragma: no cover - unreadable install location + pass + return dist.version + + +__version__ = _distribution_version() + # Theme definitions for diagram styling # Each theme defines: cluster background colors (by depth), border color, edge color THEMES = { diff --git a/diagrams/gis/cplusplus.py b/diagrams/gis/cplusplus.py index a772d45d..1e4e2e78 100644 --- a/diagrams/gis/cplusplus.py +++ b/diagrams/gis/cplusplus.py @@ -1,15 +1,18 @@ -# This module is automatically generated by autogen.sh. DO NOT EDIT. +"""Deprecated backward-compatibility alias. NOT regenerated by autogen.sh. -from . import _GIS +This module shipped since #847, but its icon directory (resources/gis/cplusplus) +never did, so `diagrams.gis.cplusplus.Mapnik` always rendered a blank node. +The working node is `diagrams.gis.cli.Mapnik`; this alias keeps old imports +running and should be removed in the next major release. +""" +import warnings -class _Cplusplus(_GIS): - _type = "cplusplus" - _icon_dir = "resources/gis/cplusplus" +from diagrams.gis.cli import Mapnik # noqa: F401 - -class Mapnik(_Cplusplus): - _icon = "mapnik.png" - - -# Aliases +warnings.warn( + "diagrams.gis.cplusplus is deprecated and will be removed in a future release; " + "import Mapnik from diagrams.gis.cli instead.", + DeprecationWarning, + stacklevel=2, +) diff --git a/pyproject.toml b/pyproject.toml index 0a43e988..ba4551de 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,8 +31,28 @@ isort = "^6.0" black = "^24.4" pre-commit = "^4.0" -[tool.hatch.build] +# The wheel ships the package and the icon tree it loads at runtime, and nothing +# else. `resources` is a top-level directory because diagrams.Node._load_icon +# resolves icons relative to the *parent* of the diagrams package. +[tool.hatch.build.targets.wheel] only-include = ["diagrams", "resources"] +# The sdist additionally carries the test suite and the release-version check, +# so distribution packagers can verify a build without cloning the repository. +# Not the rest of scripts/ or autogen.sh: those need templates/, website/ and +# external binaries, and shipping them without their inputs advertises a script +# that cannot run. Note tests/test_packaging.py skips itself unless `import +# diagrams` resolves to an installed copy - run it from outside the unpacked +# tree so it exercises the build under test, not the source that ships with it. +[tool.hatch.build.targets.sdist] +only-include = [ + "diagrams", + "resources", + "tests", + "scripts/__init__.py", + "scripts/check_release_version.py", + "CHANGELOG.md", +] + [tool.black] line-length=120 diff --git a/scripts/check_release_version.py b/scripts/check_release_version.py new file mode 100644 index 00000000..d31af6cb --- /dev/null +++ b/scripts/check_release_version.py @@ -0,0 +1,70 @@ +"""Fail a release when the git tag disagrees with the version in pyproject.toml. + +Issue #1183: the `v0.24.4` tag was placed on a commit whose pyproject.toml still +read `version = "0.24.3"`, so the tag, the sdist and the PyPI release all +disagreed and downstream packagers could not tell which was authoritative. +Nothing in the repository checked for that, so it went unnoticed. + +Run as `python -m scripts.check_release_version v0.25.2`, or with no argument to +pick the tag up from `$GITHUB_REF_NAME`. Reading pyproject.toml requires +Python 3.11+ (tomllib); the tag-version workflow runs it on 3.12. +""" + +import os +import sys +from pathlib import Path + +PYPROJECT = Path(__file__).resolve().parent.parent / "pyproject.toml" + + +class ReleaseCheckError(Exception): + """Raised when the release tag and pyproject.toml cannot be reconciled.""" + + +def project_version(pyproject: Path = PYPROJECT) -> str: + """Return the `version` declared in the [project] table of pyproject.toml.""" + try: + import tomllib + except ModuleNotFoundError as exc: # Python < 3.11 + raise ReleaseCheckError( + "reading pyproject.toml requires Python 3.11+ (tomllib); " + "the tag-version workflow runs this check on 3.12" + ) from exc + + try: + with open(pyproject, "rb") as f: + return tomllib.load(f)["project"]["version"] + except (OSError, tomllib.TOMLDecodeError, KeyError) as exc: + raise ReleaseCheckError(f"could not read the [project] version from {pyproject}: {exc!r}") from exc + + +def check_tag(tag: str, version: str) -> None: + """Raise ReleaseCheckError unless `tag` names exactly `version`.""" + normalized = tag[1:] if tag.startswith("v") else tag + if normalized != version: + raise ReleaseCheckError( + f"git tag {tag!r} does not match the project version {version!r} in pyproject.toml. " + f"Bump the version and tag that commit, or move the tag." + ) + + +def main(argv=None) -> int: + argv = sys.argv[1:] if argv is None else argv + tag = argv[0] if argv else os.environ.get("GITHUB_REF_NAME", "") + if not tag: + print("usage: python -m scripts.check_release_version ", file=sys.stderr) + return 2 + + try: + version = project_version() + check_tag(tag, version) + except ReleaseCheckError as exc: + print(f"error: {exc}", file=sys.stderr) + return 1 + + print(f"ok: tag {tag} matches pyproject.toml version {version}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_check_release_version.py b/tests/test_check_release_version.py new file mode 100644 index 00000000..0ef0a616 --- /dev/null +++ b/tests/test_check_release_version.py @@ -0,0 +1,52 @@ +import sys +import unittest + +from scripts.check_release_version import ReleaseCheckError, check_tag, project_version + + +class CheckTagTest(unittest.TestCase): + def test_accepts_tag_matching_the_project_version(self): + check_tag("v0.24.4", "0.24.4") + + def test_accepts_tag_without_the_v_prefix(self): + check_tag("0.24.4", "0.24.4") + + def test_rejects_tag_ahead_of_the_project_version(self): + """Reproduces #1183: tag v0.24.4 landed on a commit whose pyproject.toml still said 0.24.3.""" + with self.assertRaises(ReleaseCheckError) as ctx: + check_tag("v0.24.4", "0.24.3") + self.assertIn("0.24.4", str(ctx.exception)) + self.assertIn("0.24.3", str(ctx.exception)) + + def test_rejects_tag_that_is_not_a_version(self): + with self.assertRaises(ReleaseCheckError): + check_tag("release-candidate", "0.24.3") + + +@unittest.skipIf(sys.version_info < (3, 11), "project_version needs tomllib; the release workflow runs on 3.12") +class ProjectVersionTest(unittest.TestCase): + def test_reads_the_version_out_of_pyproject_toml(self): + version = project_version() + self.assertRegex(version, r"^\d+\.\d+") + + def test_tolerates_toml_the_stdlib_parser_accepts(self): + """A trailing comment on the version line is valid TOML and must not break the gate.""" + import tempfile + from pathlib import Path + + with tempfile.NamedTemporaryFile("w", suffix=".toml", delete=False) as f: + f.write('[project]\nname = "diagrams"\nversion = "0.26.0" # bumped by release bot\n') + self.assertEqual("0.26.0", project_version(Path(f.name))) + + def test_reports_a_missing_version_as_a_check_error_not_a_traceback(self): + import tempfile + from pathlib import Path + + with tempfile.NamedTemporaryFile("w", suffix=".toml", delete=False) as f: + f.write('[project]\nname = "diagrams"\n') + with self.assertRaises(ReleaseCheckError): + project_version(Path(f.name)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_packaging.py b/tests/test_packaging.py new file mode 100644 index 00000000..46449ff2 --- /dev/null +++ b/tests/test_packaging.py @@ -0,0 +1,117 @@ +"""Invariants that must hold for an *installed* diagrams distribution. + +These are deliberately written against installed metadata and on-disk files +rather than against the source tree, because the source tree is not what users +get. Both #1149 (console script pointing at a module that was never shipped) +and #1099 (wheel built without the resources tree) were invisible to a test +suite that only ever imported from a checkout. + +setUpModule skips the whole module unless `import diagrams` actually resolves +to the installed distribution: without that guard, running from a checkout or +an unpacked sdist silently validates the source tree's own resources/ - the +exact shadowing that let #1099 ship - and running without any install at all +errors with PackageNotFoundError instead of explaining itself. +""" + +import importlib +import importlib.metadata +import os +import pkgutil +import unittest +import warnings +from pathlib import Path + +import diagrams + +DIST_NAME = "diagrams" + + +def setUpModule(): + try: + dist = importlib.metadata.distribution(DIST_NAME) + except importlib.metadata.PackageNotFoundError: + raise unittest.SkipTest( + "the 'diagrams' distribution is not installed; these invariants apply to an " + "installed distribution - run `pip install .` (or `poetry install`) first" + ) + # An editable/development install has no diagrams/__init__.py under + # site-packages, so the existence check leaves that flow running. + installed_init = Path(str(dist.locate_file("diagrams/__init__.py"))) + imported_init = Path(diagrams.__file__).resolve() + if installed_init.exists() and installed_init.resolve() != imported_init: + raise unittest.SkipTest( + f"'import diagrams' resolved to {imported_init}, not the installed copy at " + f"{installed_init}; run these tests from a directory that does not contain the " + "diagrams source tree so they exercise the installed distribution" + ) + + +def _iter_node_classes(): + """Yield every Node subclass that declares an icon, across all providers.""" + for module_info in pkgutil.walk_packages(diagrams.__path__, prefix="diagrams."): + with warnings.catch_warnings(): + # diagrams.gis.cplusplus warns on import by design; keep suite output clean. + warnings.simplefilter("ignore", DeprecationWarning) + module = importlib.import_module(module_info.name) + for obj in vars(module).values(): + if not isinstance(obj, type) or not issubclass(obj, diagrams.Node): + continue + # Only look at classes defined in this module, so a class re-exported + # by several providers is not checked (and reported) many times over. + if obj.__module__ != module_info.name: + continue + if obj._icon and obj._icon_dir: + yield obj + + +class ConsoleScriptTest(unittest.TestCase): + def test_declared_console_scripts_are_importable(self): + """A declared entry point whose target module does not ship is a broken install. + + Reproduces #1149: 0.24.1-0.24.4 shipped `diagrams=diagrams.cli:main` + without ever shipping `diagrams/cli.py`, so `diagrams foo.py` raised + ModuleNotFoundError. + """ + entry_points = [ + ep for ep in importlib.metadata.distribution(DIST_NAME).entry_points if ep.group == "console_scripts" + ] + self.assertTrue(entry_points, "distribution declares no console_scripts entry point") + for entry_point in entry_points: + with self.subTest(entry_point=entry_point.name): + entry_point.load() + + +class IconResourceTest(unittest.TestCase): + def test_every_node_icon_file_exists(self): + """Every node's icon must resolve to a real file in the installed tree. + + Reproduces #1099: the 0.25.0 wheel contained no `resources/` at all. + Nothing raised - graphviz silently rendered blank nodes and the CLI + still exited 0 - so only an explicit file-existence check catches it. + """ + missing = [] + checked = 0 + for node_class in _iter_node_classes(): + # `_load_icon` only reads class attributes, so calling it with the + # class as `self` exercises the exact resolution the constructor uses. + icon_path = diagrams.Node._load_icon(node_class) + checked += 1 + if not os.path.isfile(icon_path): + missing.append(f"{node_class.__module__}.{node_class.__name__} -> {icon_path}") + + self.assertGreater(checked, 0, "no icon-bearing Node subclasses were discovered") + self.assertEqual([], missing[:20], f"{len(missing)} of {checked} node icons are missing from the install") + + +class VersionTest(unittest.TestCase): + def test_runtime_version_matches_distribution_metadata(self): + """`diagrams.__version__` must be read back from metadata, not duplicated. + + Related to #1183: every extra place the version is written by hand is + another place it can drift out of sync with the release tag. + """ + self.assertEqual(importlib.metadata.version(DIST_NAME), diagrams.__version__) + + +if __name__ == "__main__": + unittest.main()