Merge pull request #1236 from mingrammer/filipeaaoliveira/02-packaging

fix: test built artifacts and gate release tags against pyproject version

.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}"

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

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 = {

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):
+from diagrams.gis.cli import Mapnik  # noqa: F401
-    _type = "cplusplus"
-    _icon_dir = "resources/gis/cplusplus"
 
-
+warnings.warn(
-class Mapnik(_Cplusplus):
+    "diagrams.gis.cplusplus is deprecated and will be removed in a future release; "
-    _icon = "mapnik.png"
+    "import Mapnik from diagrams.gis.cli instead.",
-
+    DeprecationWarning,
-
+    stacklevel=2,
-# Aliases
+)

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

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 <tag>", 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())

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()

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()