fix(packaging): ship locales/ i18n catalogs in wheel, sdist, and Nix (#38383)
* fix(packaging): ship locales/ i18n catalogs in wheel, sdist, and Nix locales/ is a bare data dir (no __init__.py), invisible to packages.find and package-data. Sealed installs (pip wheel, Nix store venv) dropped it, so gateway/CLI commands rendered raw i18n keys like gateway.reset.header_default. - pyproject: [tool.setuptools.data-files] locales = ["locales/*.yaml"] (wheel) - MANIFEST.in: graft locales (sdist) - agent/i18n._locales_dir: env override -> source -> sysconfig data scheme - nix/hermes-agent.nix: copy locales into the store + set HERMES_BUNDLED_LOCALES as defense-in-depth. The wheel's data-files already materialize into the uv2nix venv, so resolution works with no env var; the override pins the store path against a future uv2nix change that could drop data-files. - tests: metadata regression, wheel + sdist build-install smoke tests, and a bundled-locales flake check that verifies BOTH the wrapper override and the env-var-less data-files path. Smoke test wired into CI. Closes #23943, #27632, #35374. Supersedes #23966, #27716, #30261, #33841, #35429, #35494, #35735, #36697. * test: cap locale e2e timeout, tighten catalog count guard The two wheel/sdist e2e tests inherit the global --timeout=30 from addopts; a cold-CI run (isolated build env + venv create + network pip install) can plausibly exceed it. Add @pytest.mark.timeout(300) so they don't ride the unit-test budget and flake intermittently. Also assert the shipped catalog count equals len(SUPPORTED_LANGUAGES) instead of a hardcoded >=16 floor, so the guard self-updates and trips on a single dropped catalog (not just a fully-empty graft).
This commit is contained in:
Siddharth Balyan
@@ -167,3 +167,63 @@ def test_t_missing_key_in_non_english_falls_back_to_english(tmp_path, monkeypatc
|
||||
def test_t_unknown_language_uses_english():
|
||||
"""Unknown lang codes normalize to English, not to a key-path fallback."""
|
||||
assert i18n.t("approval.denied", lang="klingon") == i18n.t("approval.denied", lang="en")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _locales_dir resolution ladder -- regression for #23943 / #27632 / #35374.
|
||||
# Sealed installs (Nix store venv, pip wheel) have no source tree next to
|
||||
# agent/, so _locales_dir must resolve via env override or the data scheme.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_locales_dir_env_override_used_when_dir_exists(tmp_path, monkeypatch):
|
||||
"""HERMES_BUNDLED_LOCALES wins when it points at a real directory."""
|
||||
bundled = tmp_path / "bundled-locales"
|
||||
bundled.mkdir()
|
||||
monkeypatch.setenv("HERMES_BUNDLED_LOCALES", str(bundled))
|
||||
assert i18n._locales_dir() == bundled
|
||||
|
||||
|
||||
def test_locales_dir_env_override_ignored_when_missing(tmp_path, monkeypatch):
|
||||
"""A bogus HERMES_BUNDLED_LOCALES falls through to source/wheel resolution
|
||||
instead of returning a path that doesn't exist."""
|
||||
monkeypatch.setenv("HERMES_BUNDLED_LOCALES", str(tmp_path / "does-not-exist"))
|
||||
result = i18n._locales_dir()
|
||||
assert result != tmp_path / "does-not-exist"
|
||||
# In a source checkout this is the repo-root locales dir.
|
||||
assert result.name == "locales"
|
||||
|
||||
|
||||
def test_locales_dir_falls_back_to_data_scheme(tmp_path, monkeypatch):
|
||||
"""When neither the env override nor a source-adjacent locales/ exists,
|
||||
_locales_dir uses sysconfig's data scheme (the pip-wheel layout)."""
|
||||
import sysconfig
|
||||
|
||||
# No env override.
|
||||
monkeypatch.delenv("HERMES_BUNDLED_LOCALES", raising=False)
|
||||
|
||||
# Force the source-adjacent path to a location with no locales/ dir.
|
||||
fake_pkg = tmp_path / "site-packages" / "agent"
|
||||
fake_pkg.mkdir(parents=True)
|
||||
monkeypatch.setattr(i18n, "__file__", str(fake_pkg / "i18n.py"))
|
||||
|
||||
# Stand up a fake data scheme containing locales/.
|
||||
data_root = tmp_path / "data-scheme"
|
||||
(data_root / "locales").mkdir(parents=True)
|
||||
real_get_path = sysconfig.get_path
|
||||
|
||||
def fake_get_path(name, *args, **kwargs):
|
||||
if name == "data":
|
||||
return str(data_root)
|
||||
return real_get_path(name, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(i18n.sysconfig, "get_path", fake_get_path)
|
||||
|
||||
assert i18n._locales_dir() == data_root / "locales"
|
||||
|
||||
|
||||
def test_t_resolves_real_string_in_source_checkout():
|
||||
"""Sanity: in the test environment (a source checkout) t() must return a
|
||||
human string, never the bare key path. Guards against catalog-load
|
||||
regressions independent of packaging."""
|
||||
assert i18n.t("gateway.reset.header_default", lang="en") != "gateway.reset.header_default"
|
||||
assert i18n.t("gateway.status.header", lang="en") != "gateway.status.header"
|
||||
|
||||
@@ -200,3 +200,29 @@ def test_locked_starlette_is_not_vulnerable_to_cve_2026_48710():
|
||||
f"floor {'.'.join(map(str, _STARLETTE_CVE_FLOOR))} — regenerate the "
|
||||
f"lockfile after bumping the pin"
|
||||
)
|
||||
|
||||
|
||||
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`.
|
||||
"""
|
||||
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'
|
||||
)
|
||||
|
||||
manifest = (REPO_ROOT / "MANIFEST.in").read_text(encoding="utf-8")
|
||||
assert "graft locales" in manifest, (
|
||||
"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"
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
"""End-to-end: a built wheel, installed without a source tree, must resolve
|
||||
i18n catalogs and render human strings — not raw key paths.
|
||||
|
||||
This is the test that would have caught #27632 / #35374 / #23943. Metadata
|
||||
unit tests (test_packaging_metadata.py) prove the glob is declared; this proves
|
||||
the runtime actually finds the catalogs after a real pip install.
|
||||
|
||||
This lives in tests/ (NOT tests/e2e/) so it is collected by the dedicated CI
|
||||
step in Task 9, not by the existing `python -m pytest tests/e2e/` runner.
|
||||
|
||||
Assumption: `from agent import i18n` must import with only stdlib + pyyaml
|
||||
available (the test installs the wheel --no-deps + pyyaml). agent/__init__.py's
|
||||
jiter preload swallows ImportError, and i18n.py imports yaml lazily inside
|
||||
_load_catalog, so this holds today. If i18n.py ever gains a top-level non-stdlib
|
||||
import, add it to the pip install line below.
|
||||
|
||||
Marked `integration` because it shells out to `uv build` + `venv` + `pip` and
|
||||
takes ~15-30s. Run with: pytest -m integration tests/test_wheel_locales_e2e.py
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import glob
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
import tarfile
|
||||
import venv
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.timeout(300) # overrides the global --timeout=30; cold-CI wheel build + venv + pip can exceed it
|
||||
def test_installed_wheel_renders_i18n_strings(tmp_path):
|
||||
# 1. Build the wheel from the current tree.
|
||||
wheel_dir = tmp_path / "wheel"
|
||||
build = subprocess.run(
|
||||
["uv", "build", "--wheel", "--out-dir", str(wheel_dir), "."],
|
||||
cwd=REPO_ROOT,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=600,
|
||||
)
|
||||
assert build.returncode == 0, f"uv build failed:\n{build.stderr}"
|
||||
wheels = glob.glob(str(wheel_dir / "*.whl"))
|
||||
assert wheels, "no wheel produced"
|
||||
wheel = wheels[0]
|
||||
|
||||
# 2. Fresh venv, install the wheel WITHOUT deps (we only exercise i18n,
|
||||
# which needs pyyaml). --force-reinstall guards against pip's
|
||||
# same-version no-op.
|
||||
venv_dir = tmp_path / "venv"
|
||||
venv.create(venv_dir, with_pip=True)
|
||||
vpy = venv_dir / "bin" / "python"
|
||||
subprocess.run([str(vpy), "-m", "pip", "install", "-q", "pyyaml"], check=True, timeout=300)
|
||||
subprocess.run(
|
||||
[str(vpy), "-m", "pip", "install", "-q", "--no-deps", "--force-reinstall", wheel],
|
||||
check=True,
|
||||
timeout=300,
|
||||
)
|
||||
|
||||
# 3. Run from a directory that is NOT the source tree, with a clean env
|
||||
# (no PYTHONPATH leaking the repo, no HERMES_BUNDLED_LOCALES).
|
||||
probe = (
|
||||
"from agent import i18n;"
|
||||
"import sys;"
|
||||
"r = i18n.t('gateway.reset.header_default', lang='en');"
|
||||
"s = i18n.t('gateway.status.header', lang='en');"
|
||||
"print(repr(r)); print(repr(s));"
|
||||
"sys.exit(0 if (r != 'gateway.reset.header_default' "
|
||||
"and s != 'gateway.status.header') else 1)"
|
||||
)
|
||||
env = {k: v for k, v in os.environ.items() if k not in ("PYTHONPATH", "HERMES_BUNDLED_LOCALES")}
|
||||
env["PATH"] = f"{venv_dir / 'bin'}:{env['PATH']}"
|
||||
env["VIRTUAL_ENV"] = str(venv_dir)
|
||||
run = subprocess.run(
|
||||
[str(vpy), "-c", probe],
|
||||
cwd=str(tmp_path), # NOT the repo root
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
timeout=120,
|
||||
)
|
||||
assert run.returncode == 0, (
|
||||
"installed wheel returned raw i18n keys instead of human strings:\n"
|
||||
f"stdout: {run.stdout}\nstderr: {run.stderr}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.timeout(300) # overrides the global --timeout=30; cold-CI sdist build can exceed it
|
||||
def test_built_sdist_ships_locale_catalogs(tmp_path):
|
||||
"""The sdist must carry locales/ too.
|
||||
|
||||
The wheel is covered above; the sdist is a separately shipped artifact
|
||||
(PyPI, and the form distro/Homebrew packagers build from). MANIFEST.in
|
||||
`graft locales` is what puts the catalogs in the tarball — a stale graft or
|
||||
a setuptools change would pass the metadata unit test (which only inspects
|
||||
the declaration) while the actual artifact regresses. This inspects the
|
||||
real tarball so that path can't rot silently. Closes the sdist half of
|
||||
#27632 / #35374 / #23943.
|
||||
"""
|
||||
sdist_dir = tmp_path / "sdist"
|
||||
build = subprocess.run(
|
||||
["uv", "build", "--sdist", "--out-dir", str(sdist_dir), "."],
|
||||
cwd=REPO_ROOT,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=600,
|
||||
)
|
||||
assert build.returncode == 0, f"uv build --sdist failed:\n{build.stderr}"
|
||||
tarballs = glob.glob(str(sdist_dir / "*.tar.gz"))
|
||||
assert tarballs, "no sdist produced"
|
||||
|
||||
with tarfile.open(tarballs[0]) as tf:
|
||||
# Members are prefixed with the sdist root dir, e.g.
|
||||
# hermes_agent-0.15.1/locales/en.yaml — match on the suffix.
|
||||
catalogs = [m for m in tf.getnames() if "/locales/" in m and m.endswith(".yaml")]
|
||||
|
||||
# Compare against the canonical language list rather than a hardcoded floor
|
||||
# so adding/removing a catalog updates the guard automatically and a dropped
|
||||
# catalog (not just a fully-empty graft) trips it.
|
||||
from agent.i18n import SUPPORTED_LANGUAGES
|
||||
|
||||
expected = len(SUPPORTED_LANGUAGES)
|
||||
assert len(catalogs) == expected, (
|
||||
f"sdist shipped {len(catalogs)} locale catalogs, expected {expected} "
|
||||
f"({len(SUPPORTED_LANGUAGES)} supported languages) — check `graft "
|
||||
"locales` in MANIFEST.in"
|
||||
)
|
||||
assert any(m.endswith("/locales/en.yaml") for m in catalogs), (
|
||||
f"sdist missing locales/en.yaml; shipped: {catalogs[:5]}"
|
||||
)
|
||||
Reference in New Issue
Block a user