From f2e526c77726d14ecee100e82af438730cc2a0e3 Mon Sep 17 00:00:00 2001 From: Que0x Date: Wed, 24 Jun 2026 22:37:58 +0300 Subject: [PATCH 1/2] fix(packaging): ship bundled skills in the wheel via setup.py data_files pyproject's [tool.setuptools.data-files] table silently overrode setup.py's data_files, so wheels shipped zero skills/ and optional-skills/ and sync_skills() returned total_bundled: 0 on sealed installs. Consolidate all bundled data dirs into one dynamic generator in setup.py (also fixes a Windows path-separator bug, excludes index-cache/__pycache__ cruft, and auto-includes the silently-dropped optional-mcps/unreal-engine). Remove the shadowing pyproject table and add regression tests. --- pyproject.toml | 36 ++++---- setup.py | 91 +++++++++++++++++-- tests/test_packaging_metadata.py | 148 ++++++++++++++++++++++++++++--- 3 files changed, 233 insertions(+), 42 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d269ba840be2..bd696d191fd2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -301,28 +301,22 @@ hermes-acp = "acp_adapter.entry:main" [tool.setuptools] py-modules = ["run_agent", "model_tools", "toolsets", "batch_runner", "trajectory_compressor", "toolset_distributions", "cli", "hermes_bootstrap", "hermes_constants", "hermes_state", "hermes_time", "hermes_logging", "utils", "mcp_serve"] -[tool.setuptools.data-files] -# i18n catalogs. locales/ is a bare data directory (no __init__.py), so it is -# neither a package (packages.find) nor package-data (which attaches to a -# package). data-files ships it in the wheel; MANIFEST.in `graft locales` -# ships it in the sdist. Without this, sealed installs (pip wheel, Nix store -# venv) drop the catalogs and gateway/CLI commands surface raw i18n keys like -# `gateway.reset.header_default` (#27632, #35374, #23943). -locales = ["locales/*.yaml"] -# Shipped MCP catalog (optional-mcps//manifest.yaml). Same bare-data-dir -# case as locales: data-files ships it in the wheel, `graft optional-mcps` in -# MANIFEST.in ships it in the sdist. Without this, `hermes mcp catalog` and the -# dashboard catalog screen come up empty on packaged installs even though the -# manifests exist in the repo (hermes_cli/mcp_catalog.py:_catalog_root resolves -# the packaged dir; list_catalog() returns [] when it's missing). +# NOTE: there is intentionally NO [tool.setuptools.data-files] table here. # -# data-files flattens every glob match into its single target dir, so each -# catalog entry needs its OWN target to preserve the per-entry directory the -# catalog iterates over (a shared `optional-mcps/*/*` glob would collapse all -# manifests into one colliding optional-mcps/manifest.yaml). One target per -# entry; tests/test_packaging_metadata.py enforces an entry per optional-mcps/. -"optional-mcps/linear" = ["optional-mcps/linear/manifest.yaml"] -"optional-mcps/n8n" = ["optional-mcps/n8n/manifest.yaml"] +# Bundled non-package data dirs (skills/, optional-skills/, locales/, +# optional-mcps/) are bare data directories — no __init__.py, so they are +# neither packages (packages.find) nor package-data (which attaches to a +# package). They ship in the wheel via setuptools `data_files`, generated in +# setup.py:bundled_data_files(), and in the sdist via MANIFEST.in `graft`. +# +# data_files can be declared in EITHER this table OR setup.py, never merged: +# whichever pyproject declares wins and silently drops setup.py's list. The +# skills/ and optional-skills/ trees are too deep and change too often to +# enumerate statically here (data_files also flattens each glob into one target +# dir, so a nested tree needs one target per directory), so ALL four dirs are +# generated dynamically in setup.py. Reintroducing this table would shadow +# setup.py and the wheel would ship zero skills again — see +# tests/test_packaging_metadata.py and CONTRIBUTING.md (#27632, #35374, #23943). [tool.setuptools.package-data] hermes_cli = ["web_dist/**/*", "tui_dist/**/*", "scripts/install.sh", "scripts/install.ps1"] diff --git a/setup.py b/setup.py index 6e3e8c4272e8..10763dbe93bd 100644 --- a/setup.py +++ b/setup.py @@ -12,6 +12,37 @@ REPO_ROOT = Path(__file__).parent.resolve() +# Directory names that never carry shippable bundled-data content: VCS, +# virtualenv / dependency trees, byte-code and test caches, and the skills +# ``index-cache`` (a runtime download cache, also filtered by +# nix/hermes-agent.nix). Mirrors agent.skill_utils.EXCLUDED_SKILL_DIRS — kept +# as a literal so setup.py stays import-free at build time — plus +# ``index-cache``. NOTE: skill *support* dirs (references/templates/assets/ +# scripts) are deliberately NOT excluded; they are real skill payload that +# tools/skills_sync.py copies into ~/.hermes/skills/ on seed. +_EXCLUDED_DATA_DIRS = frozenset( + { + ".git", + ".github", + ".hub", + ".archive", + ".venv", + "venv", + "node_modules", + "site-packages", + "__pycache__", + ".tox", + ".nox", + ".pytest_cache", + ".mypy_cache", + ".ruff_cache", + "index-cache", + } +) +_EXCLUDED_DATA_SUFFIXES = frozenset({".pyc", ".pyo"}) +_EXCLUDED_DATA_NAMES = frozenset({".DS_Store", "Thumbs.db"}) + + def _source_tree_is_writable() -> bool: probe = REPO_ROOT / ".setuptools-write-probe" try: @@ -64,24 +95,68 @@ def finalize_options(self) -> None: super().finalize_options() +def _is_shippable(rel_path: Path) -> bool: + """True when *rel_path* (relative to REPO_ROOT) is real bundled-data content.""" + if _EXCLUDED_DATA_DIRS.intersection(rel_path.parts): + return False + if rel_path.suffix.lower() in _EXCLUDED_DATA_SUFFIXES: + return False + if rel_path.name in _EXCLUDED_DATA_NAMES: + return False + return True + + def _data_file_tree(root_name: str) -> list[tuple[str, list[str]]]: + """Map every file under *root_name* to a setuptools ``data_files`` entry. + + Returns ``(target_dir, [source_files])`` tuples with the on-disk directory + layout preserved one-directory-per-target. setuptools ``data_files`` + FLATTENS every glob match into its single target dir, so a nested tree must + be enumerated per directory or the structure collapses — skills are seeded + by category/skill path (tools/skills_sync.py), so a flattened + ``skills/*`` glob would corrupt the install. Paths are emitted as POSIX + (forward slash) so the wheel is correct even when built on Windows, where + ``str(Path(...))`` would otherwise bake in backslash targets. + """ root = REPO_ROOT / root_name + if not root.is_dir(): + return [] grouped: defaultdict[str, list[str]] = defaultdict(list) for path in sorted(root.rglob("*")): if not path.is_file(): continue rel_path = path.relative_to(REPO_ROOT) - grouped[str(rel_path.parent)].append(str(rel_path)) + if not _is_shippable(rel_path): + continue + grouped[rel_path.parent.as_posix()].append(rel_path.as_posix()) return sorted(grouped.items()) -setup( - cmdclass={ - "build": ReadOnlySourceBuild, - "egg_info": ReadOnlySourceEggInfo, - }, - data_files=[ +def bundled_data_files() -> list[tuple[str, list[str]]]: + """All bundled non-package data shipped in the wheel's data scheme. + + setuptools resolves ``data_files`` from EITHER pyproject.toml's + ``[tool.setuptools.data-files]`` table OR setup.py — never a merge of both. + Whichever the pyproject table declares wins and silently drops setup.py's + list (this clobbered the skills shipping for two releases). The skills and + optional-skills trees are too deep and change too often to enumerate + statically in TOML, so ALL bundled-data dirs are generated here in one + place. Do NOT reintroduce ``[tool.setuptools.data-files]`` in pyproject.toml + or it will shadow this and the wheel will ship zero skills again. + """ + return [ *_data_file_tree("skills"), *_data_file_tree("optional-skills"), + *_data_file_tree("locales"), + *_data_file_tree("optional-mcps"), ] -) + + +if __name__ == "__main__": + setup( + cmdclass={ + "build": ReadOnlySourceBuild, + "egg_info": ReadOnlySourceEggInfo, + }, + data_files=bundled_data_files(), + ) diff --git a/tests/test_packaging_metadata.py b/tests/test_packaging_metadata.py index 5499dc47c055..b7f7a8488019 100644 --- a/tests/test_packaging_metadata.py +++ b/tests/test_packaging_metadata.py @@ -1,4 +1,6 @@ +import importlib.util from pathlib import Path +import posixpath import re import tomllib @@ -15,6 +17,28 @@ REPO_ROOT = Path(__file__).resolve().parents[1] +def _load_setup_module(): + """Import the repo-root setup.py without triggering its setup() call. + + The setup() invocation is guarded behind ``if __name__ == "__main__"``, so + loading the file under any other module name exposes + ``bundled_data_files()`` / ``_data_file_tree()`` for inspection without + running a build. + """ + spec = importlib.util.spec_from_file_location( + "hermes_setup_py", REPO_ROOT / "setup.py" + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _shipped_data_sources() -> set[str]: + """All POSIX source paths setup.py ships as wheel data-files.""" + setup_mod = _load_setup_module() + return {src for _, sources in setup_mod.bundled_data_files() for src in sources} + + def _distribution_name(requirement: str) -> str: """Extract the PEP 508 distribution name from a requirement string. @@ -243,17 +267,25 @@ def test_locked_starlette_is_not_vulnerable_to_cve_2026_48710(): def test_locale_catalogs_ship_in_both_wheel_and_sdist(): """Regression test for #27632 / #35374 / #23943. - locales/ is a bare data directory (no __init__.py), so it is invisible to - packages.find and to package-data (which attaches to a package). It must be - declared as setuptools data-files (wheel) AND grafted in MANIFEST.in - (sdist). Without both, sealed installs drop the catalogs and gateway/CLI - commands surface raw i18n keys like `gateway.reset.header_default`. + locales/ is a bare data directory (no __init__.py), so it reaches installs + only as setuptools data-files (wheel) AND a MANIFEST.in graft (sdist). The + wheel data-files are generated in ``setup.py:bundled_data_files()`` — NOT a + ``[tool.setuptools.data-files]`` table in pyproject.toml, which would shadow + setup.py and drop the skills tree (see + ``test_pyproject_does_not_shadow_setup_py_data_files``). Without both + channels, sealed installs drop the catalogs and gateway/CLI commands surface + raw i18n keys like ``gateway.reset.header_default``. """ - data = tomllib.loads((REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8")) - data_files = data["tool"]["setuptools"].get("data-files", {}) - assert data_files.get("locales") == ["locales/*.yaml"], ( - "pyproject [tool.setuptools.data-files] must declare " - 'locales = ["locales/*.yaml"] so the wheel ships i18n catalogs' + shipped = _shipped_data_sources() + on_disk = { + p.relative_to(REPO_ROOT).as_posix() + for p in (REPO_ROOT / "locales").glob("*.yaml") + } + assert on_disk, "expected locales/*.yaml catalogs on disk" + missing = sorted(on_disk - shipped) + assert not missing, ( + f"locale catalogs dropped from wheel data-files: {missing} — " + "setup.py:bundled_data_files() must ship every locales/*.yaml" ) manifest = (REPO_ROOT / "MANIFEST.in").read_text(encoding="utf-8") @@ -261,7 +293,97 @@ def test_locale_catalogs_ship_in_both_wheel_and_sdist(): "MANIFEST.in must `graft locales` so the sdist ships i18n catalogs" ) - # Every on-disk catalog has the .yaml extension the globs above match. - on_disk = list((REPO_ROOT / "locales").glob("*.yaml")) - assert on_disk, "expected locales/*.yaml catalogs on disk" + +def test_bundled_skills_ship_in_wheel_data_files(): + """Regression test: the wheel must ship bundled skills/ and optional-skills/. + + Both are bare data dirs (no __init__.py), so they reach the wheel only via + setuptools ``data_files``. setup.py generates that list dynamically, but a + ``[tool.setuptools.data-files]`` table in pyproject.toml silently OVERRIDES + setup.py's ``data_files`` — which shipped a wheel with ZERO bundled skills + for two releases. On a sealed/wheel install ``_get_bundled_dir()`` then + resolves an absent dir and ``sync_skills()`` returns ``total_bundled: 0``, + so nothing is seeded into ~/.hermes/skills/. + + This drives setup.py's own generator and asserts: + - every on-disk SKILL.md is shipped (completeness), + - the nested category/skill layout is preserved (data_files flattens each + glob into one target dir, so a collapsed tree would corrupt seeding), + - runtime cruft (index-cache, __pycache__, *.pyc) is not shipped. + """ + setup_mod = _load_setup_module() + entries = setup_mod.bundled_data_files() + + shipped: set[str] = set() + for target, sources in entries: + for src in sources: + assert posixpath.dirname(src) == target, ( + f"data_files flatten hazard: {src!r} is not directly under its " + f"declared target {target!r}; setuptools would collapse the tree" + ) + shipped.add(src) + + for root_name in ("skills", "optional-skills"): + on_disk = { + p.relative_to(REPO_ROOT).as_posix() + for p in (REPO_ROOT / root_name).rglob("SKILL.md") + } + assert on_disk, f"expected SKILL.md files under {root_name}/ on disk" + missing = sorted(on_disk - shipped) + assert not missing, ( + f"{root_name}/ skills dropped from wheel data-files: {missing}" + ) + + cruft = sorted( + s + for s in shipped + if "index-cache" in s.split("/") + or "__pycache__" in s.split("/") + or s.endswith((".pyc", ".pyo")) + ) + assert not cruft, f"runtime cruft leaked into wheel data-files: {cruft}" + + +def test_optional_mcps_manifests_ship_in_wheel(): + """Every optional-mcps//manifest.yaml must ship in the wheel and sdist. + + The catalog reader (hermes_cli/mcp_catalog.py) lists whatever manifests are + present in the packaged optional-mcps dir, so a dropped entry silently + removes a server from ``hermes mcp catalog``. Generating data-files from the + tree (vs a hand-maintained pyproject list) keeps this from drifting — the + earlier static list shipped linear + n8n but missed unreal-engine. + """ + shipped = _shipped_data_sources() + on_disk = { + p.relative_to(REPO_ROOT).as_posix() + for p in (REPO_ROOT / "optional-mcps").rglob("manifest.yaml") + } + assert on_disk, "expected optional-mcps//manifest.yaml on disk" + missing = sorted(on_disk - shipped) + assert not missing, ( + f"optional-mcps manifests dropped from wheel data-files: {missing}" + ) + + manifest = (REPO_ROOT / "MANIFEST.in").read_text(encoding="utf-8") + assert "graft optional-mcps" in manifest, ( + "MANIFEST.in must `graft optional-mcps` so the sdist ships the catalog" + ) + + +def test_pyproject_does_not_shadow_setup_py_data_files(): + """pyproject.toml must NOT declare ``[tool.setuptools.data-files]``. + + Bundled data dirs are generated dynamically in + ``setup.py:bundled_data_files()`` because skills/ and optional-skills/ are + too deep to enumerate statically and data_files flattens nested globs. + setuptools resolves ``data_files`` from EITHER pyproject OR setup.py, never + both — a pyproject table wins and silently drops setup.py's list, which + shipped a wheel with zero bundled skills. Keep the table out of pyproject so + setup.py stays the single source of truth. + """ + data = tomllib.loads((REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8")) + assert "data-files" not in data["tool"]["setuptools"], ( + "Remove [tool.setuptools.data-files] from pyproject.toml — it shadows " + "setup.py:bundled_data_files() and drops skills/ from the wheel." + ) From 5a3679aaa952280db40d141c9503a69add203321 Mon Sep 17 00:00:00 2001 From: Que0x Date: Wed, 24 Jun 2026 22:58:46 +0300 Subject: [PATCH 2/2] fix(packaging): don't guard setup() behind __main__ runpy.run_path runs setup.py as "", not "__main__", so the guard added earlier in this branch stopped setuptools.setup() from running and broke test_docker_webui_install_surface. Remove the guard; the packaging tests now capture data_files via runpy + a setuptools.setup patch (the same technique the docker-webui test uses) instead of importing the module. --- setup.py | 15 ++++++------- tests/test_packaging_metadata.py | 38 ++++++++++++++++++-------------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/setup.py b/setup.py index 10763dbe93bd..4f8ca9445673 100644 --- a/setup.py +++ b/setup.py @@ -152,11 +152,10 @@ def bundled_data_files() -> list[tuple[str, list[str]]]: ] -if __name__ == "__main__": - setup( - cmdclass={ - "build": ReadOnlySourceBuild, - "egg_info": ReadOnlySourceEggInfo, - }, - data_files=bundled_data_files(), - ) +setup( + cmdclass={ + "build": ReadOnlySourceBuild, + "egg_info": ReadOnlySourceEggInfo, + }, + data_files=bundled_data_files(), +) diff --git a/tests/test_packaging_metadata.py b/tests/test_packaging_metadata.py index b7f7a8488019..bd88bc7484d0 100644 --- a/tests/test_packaging_metadata.py +++ b/tests/test_packaging_metadata.py @@ -1,8 +1,9 @@ -import importlib.util from pathlib import Path import posixpath import re +import runpy import tomllib +import unittest.mock import pytest @@ -17,26 +18,30 @@ REPO_ROOT = Path(__file__).resolve().parents[1] -def _load_setup_module(): - """Import the repo-root setup.py without triggering its setup() call. +def _setup_data_files() -> list[tuple[str, list[str]]]: + """Return the ``data_files`` list setup.py passes to ``setuptools.setup()``. - The setup() invocation is guarded behind ``if __name__ == "__main__"``, so - loading the file under any other module name exposes - ``bundled_data_files()`` / ``_data_file_tree()`` for inspection without - running a build. + Runs setup.py via runpy with ``setuptools.setup`` patched to a capture shim + (the same technique as tests/test_docker_webui_install_surface.py), so the + exact wheel data-files the build backend receives are read without running a + real build. setup.py generates these dynamically — see + ``setup.py:bundled_data_files()``. """ - spec = importlib.util.spec_from_file_location( - "hermes_setup_py", REPO_ROOT / "setup.py" - ) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module + import setuptools + + captured: dict = {} + + def _capture(**kwargs): + captured.update(kwargs) + + with unittest.mock.patch.object(setuptools, "setup", _capture): + runpy.run_path(str(REPO_ROOT / "setup.py")) + return captured.get("data_files", []) def _shipped_data_sources() -> set[str]: """All POSIX source paths setup.py ships as wheel data-files.""" - setup_mod = _load_setup_module() - return {src for _, sources in setup_mod.bundled_data_files() for src in sources} + return {src for _, sources in _setup_data_files() for src in sources} def _distribution_name(requirement: str) -> str: @@ -311,8 +316,7 @@ def test_bundled_skills_ship_in_wheel_data_files(): glob into one target dir, so a collapsed tree would corrupt seeding), - runtime cruft (index-cache, __pycache__, *.pyc) is not shipped. """ - setup_mod = _load_setup_module() - entries = setup_mod.bundled_data_files() + entries = _setup_data_files() shipped: set[str] = set() for target, sources in entries: