Skip to content

Commit 22cf142

Browse files
committed
ci: check the sdist can actually be consumed
The sdist contents are defined by an allowlist in pyproject.toml. That is the safe default -- a new subpackage under packages/ stays out of the zarr sdist unless someone opts it in -- but it inverts the failure mode. A blocklist ships too much silently; an allowlist can ship too little, by adding a file consumers need without a matching allowlist entry. The 'Test distribution artifacts' job previously ran 'ls dist' and asserted nothing. Two checks now replace it, split by what they cost, both inspecting the built tarball rather than the working tree since a git checkout has files the sdist does not. test_dist_pypi runs ci/check_sdist_contents.py: every 'testpaths' entry from pyproject.toml ships, packages/ does not, and the handful of paths packagers read but no test opens (LICENSE.txt, PKG-INFO, py.typed, the hatch-vcs version file) are present. The testpaths check is derived from config maintained for other reasons rather than restated, so it cannot drift into being a second allowlist to forget. Seconds and hermetic, so it runs on pull requests too. Against the pre-allowlist sdist it reports both defects that were live: docs/user-guide missing, and 370 packages/ paths. test_dist_conda builds the sdist with the real conda-forge recipe, fetched from the feedstock at run time and repointed at the tarball we just built by ci/conda_recipe_for_sdist.py. conda-forge is the consumer most exposed to a missing file: it builds from the sdist rather than the wheel and runs our suite from a directory holding only what its recipe copies out of the tarball. Fetching beats reproducing -- a copy of that recipe would go stale silently, which is the same class of bug this job exists to catch. Note the recipe's requirements are already regenerated from our pyproject.toml by grayskull on each version bump (bot: inspection: update-grayskull in the feedstock's conda-forge.yml); the tests block is hand-maintained there. That job is skipped on pull_request. It runs the whole suite from an unpacked sdist, measured at 591.95s of a 613s run, so conda's build and solve are about 22 seconds of it and there is no cheaper variant of the same coverage. On push to main it still catches a bad merge within minutes and attributes it to one commit, and it gates upload_pypi so no release can publish an sdist conda cannot consume. workflow_dispatch runs it against a branch on demand. Keeping it off pull requests also stops a recipe change in the feedstock from reddening unrelated PRs. A url source rather than a path one, deliberately: path sources honour .gitignore, and src/zarr/_version.py is gitignored. It is written into the sdist by the hatch-vcs hook precisely because an unpacked sdist has no git history, so a path source would drop it and break the build. Verified on CI before the trigger was narrowed: imports test passed, pip check passed, 6884 passed / 475 skipped / 4 xfailed. Assisted-by: ClaudeCode:claude-opus-5
1 parent 8881b25 commit 22cf142

4 files changed

Lines changed: 303 additions & 4 deletions

File tree

‎.github/workflows/releases.yml‎

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,24 +48,98 @@ jobs:
4848
name: releases
4949
path: dist
5050

51+
# The sdist contents are defined by an allowlist in pyproject.toml, so the
52+
# failure mode to guard against is shipping too little: a new file that
53+
# consumers need, added without a matching allowlist entry. This job and
54+
# `test_dist_conda` below inspect the built artifact rather than the working
55+
# tree, because a git checkout has files the tarball does not.
56+
#
57+
# Seconds, no network beyond the artifact download, so it runs everywhere the
58+
# workflow does -- including pull requests.
5159
test_dist_pypi:
5260
name: Test distribution artifacts
5361
needs: [build_artifacts]
5462
runs-on: ubuntu-latest
5563
steps:
64+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
65+
with:
66+
persist-credentials: false
67+
5668
- uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
5769
with:
5870
name: releases
5971
path: dist
6072

61-
- name: test
73+
- uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
74+
name: Install Python
75+
with:
76+
python-version: '3.12'
77+
78+
# Catches what the conda build cannot: paths that no test opens and no
79+
# build step reads, and paths that must never ship (packages/, which is
80+
# three separately released distributions -- conda would not object to
81+
# those, but we do).
82+
- name: Check sdist contents
83+
run: python ci/check_sdist_contents.py dist
84+
85+
# conda-forge is the consumer most exposed to a missing file: it builds from
86+
# the sdist rather than the wheel, and runs our suite from a directory holding
87+
# only what its recipe copies out of the tarball. Run the real recipe rather
88+
# than a copy of it -- a copy would go stale the moment the feedstock changed,
89+
# and silently, which is the same class of bug this job exists to catch.
90+
#
91+
# Not on pull requests. It runs the whole suite from an unpacked sdist, which
92+
# is ~10 minutes whether or not conda is involved, and it depends on a fetch
93+
# from conda-forge/zarr-feedstock, so a recipe change there would redden
94+
# unrelated PRs. On `push` to main it still catches a bad merge within
95+
# minutes, attributed to one commit, and it gates `upload_pypi` so no release
96+
# can publish an sdist conda cannot consume. Use `workflow_dispatch` to run it
97+
# against a PR branch on demand.
98+
test_dist_conda:
99+
name: Test conda-forge can consume the sdist
100+
needs: [build_artifacts]
101+
if: github.event_name != 'pull_request'
102+
runs-on: ubuntu-latest
103+
steps:
104+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
105+
with:
106+
persist-credentials: false
107+
108+
- uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
109+
with:
110+
name: releases
111+
path: dist
112+
113+
- uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
114+
name: Install Python
115+
with:
116+
python-version: '3.12'
117+
118+
- name: Fetch the conda-forge recipe
62119
run: |
63-
ls
64-
ls dist
120+
curl -fsSL --retry 3 -o "${RUNNER_TEMP}/recipe-upstream.yaml" \
121+
https://raw.githubusercontent.com/conda-forge/zarr-feedstock/main/recipe/recipe.yaml
122+
123+
- name: Point the recipe at the sdist we just built
124+
run: |
125+
python -m pip install pyyaml
126+
python ci/conda_recipe_for_sdist.py \
127+
"${RUNNER_TEMP}/recipe-upstream.yaml" \
128+
"$(ls dist/*.tar.gz)" \
129+
conda.recipe/recipe.yaml
130+
131+
# Runs the recipe's own `tests:` block: an import check, `pip check`, and
132+
# our suite from a tree containing only the recipe's `files.source`.
133+
- uses: prefix-dev/rattler-build-action@1ca5f45832f419a46d1326ccc5861d7e14d67c44 # v0.2.39
134+
with:
135+
rattler-build-version: v0.73.0
136+
recipe-path: conda.recipe/recipe.yaml
137+
upload-artifact: false
138+
build-args: -c conda-forge --output-dir ${{ runner.temp }}/conda-build
65139

66140
upload_pypi:
67141
name: Upload to PyPI
68-
needs: [build_artifacts, test_dist_pypi]
142+
needs: [build_artifacts, test_dist_pypi, test_dist_conda]
69143
runs-on: ubuntu-latest
70144
if: github.event_name == 'release'
71145
environment:

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ __pycache__/
77

88
# Distribution / packaging
99
.Python
10+
# Written by ci/conda_recipe_for_sdist.py when checking the sdist against the
11+
# conda-forge recipe; generated per run, never committed.
12+
conda.recipe/
1013
env/
1114
.venv/
1215
build/

‎ci/check_sdist_contents.py‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
"""
2+
Check a built sdist ships everything its consumers need, and nothing they don't.
3+
4+
Usage:
5+
python check_sdist_contents.py SDIST
6+
7+
SDIST is a path to a `.tar.gz`, or a directory containing exactly one.
8+
9+
`[tool.hatch.build.targets.sdist]` in pyproject.toml is an allowlist. That is
10+
the safe default -- a new subpackage under `packages/` stays out of the `zarr`
11+
sdist unless someone opts it in -- but it inverts the failure mode: instead of
12+
shipping too much silently, an allowlist can ship too little. This script
13+
guards that direction.
14+
15+
Wherever possible the expectations are *derived* from configuration that is
16+
maintained for other reasons, rather than restated here. A hand-written list of
17+
"files the sdist must contain" would just be a second allowlist to forget to
18+
update, which is the problem it is meant to solve.
19+
"""
20+
21+
import sys
22+
import tarfile
23+
import tomllib
24+
from pathlib import Path
25+
26+
REPO_ROOT = Path(__file__).parent.parent.resolve()
27+
28+
# Paths no test opens, so the sdist test run in `releases.yml` cannot vouch for
29+
# them, but that packagers do need. conda-forge's recipe installs the sdist
30+
# (pyproject.toml, README.md, src/) and reads `license_file: LICENSE.txt`; the
31+
# version file is written into the sdist by the hatch-vcs hook, because an
32+
# unpacked sdist has no git history to derive a version from.
33+
# https://github.com/conda-forge/zarr-feedstock/blob/main/recipe/recipe.yaml
34+
REQUIRED_PATHS = [
35+
"pyproject.toml",
36+
"README.md",
37+
"LICENSE.txt",
38+
"PKG-INFO",
39+
"src/zarr/__init__.py",
40+
"src/zarr/py.typed",
41+
"src/zarr/_version.py",
42+
]
43+
44+
# Each subpackage under `packages/` is its own distribution with its own PyPI
45+
# release, its own version tags and its own sdist. Before the allowlist they
46+
# were bundled into every `zarr` sdist -- 2.9M of unrelated sources -- which is
47+
# the regression this check exists to prevent recurring.
48+
FORBIDDEN_PREFIXES = ["packages/"]
49+
50+
51+
def sdist_members(sdist: Path) -> set[str]:
52+
"""Every path inside the tarball, relative to its top-level directory."""
53+
with tarfile.open(sdist) as tar:
54+
names = tar.getnames()
55+
# Members are `zarr-<version>/<path>`; strip the leading component so the
56+
# expectations below don't have to know the version.
57+
return {name.split("/", 1)[1] for name in names if "/" in name}
58+
59+
60+
def testpaths() -> list[str]:
61+
"""`testpaths` from pyproject.toml.
62+
63+
Derived rather than duplicated: adding a directory to `testpaths` without
64+
adding it to the sdist allowlist is exactly the mistake this catches. It is
65+
also the mistake that was already live -- `docs/user-guide` has been a
66+
testpath while `docs/` was excluded, so `pytest` on an unpacked sdist died
67+
at collection.
68+
"""
69+
with (REPO_ROOT / "pyproject.toml").open("rb") as f:
70+
config = tomllib.load(f)
71+
return config["tool"]["pytest"]["ini_options"]["testpaths"]
72+
73+
74+
def check(sdist: Path) -> int:
75+
print(f"Checking {sdist.name}")
76+
members = sdist_members(sdist)
77+
print(f"Found {len(members)} paths")
78+
print()
79+
80+
missing_required = [p for p in REQUIRED_PATHS if p not in members]
81+
# A testpath is a directory; it is present if anything ships beneath it.
82+
missing_testpaths = [p for p in testpaths() if not any(m.startswith(f"{p}/") for m in members)]
83+
forbidden = sorted(m for m in members if any(m.startswith(p) for p in FORBIDDEN_PREFIXES))
84+
85+
if not (missing_required or missing_testpaths or forbidden):
86+
print("OK")
87+
return 0
88+
89+
if missing_required:
90+
print("Required paths missing from the sdist")
91+
print("-------------------------------------")
92+
print("\n".join(missing_required))
93+
print()
94+
if missing_testpaths:
95+
print("testpaths entries missing from the sdist")
96+
print("----------------------------------------")
97+
print("\n".join(missing_testpaths))
98+
print("`pytest` on an unpacked sdist will fail to collect these.")
99+
print()
100+
if forbidden:
101+
print("Paths that must not ship in the zarr sdist")
102+
print("------------------------------------------")
103+
print("\n".join(forbidden[:20]))
104+
if len(forbidden) > 20:
105+
print(f"... and {len(forbidden) - 20} more")
106+
print()
107+
108+
print(
109+
"Fix by editing `[tool.hatch.build.targets.sdist]` in pyproject.toml. "
110+
"It is an allowlist: a path ships only if it is listed."
111+
)
112+
return 1
113+
114+
115+
if __name__ == "__main__":
116+
if len(sys.argv) != 2:
117+
print(__doc__)
118+
sys.exit(2)
119+
target = Path(sys.argv[1]).resolve()
120+
if target.is_dir():
121+
tarballs = sorted(target.glob("*.tar.gz"))
122+
if len(tarballs) != 1:
123+
print(f"Expected exactly one .tar.gz in {target}, found {len(tarballs)}")
124+
sys.exit(2)
125+
target = tarballs[0]
126+
sys.exit(check(target))

‎ci/conda_recipe_for_sdist.py‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
"""
2+
Point the conda-forge recipe at a locally built sdist.
3+
4+
Usage:
5+
python conda_recipe_for_sdist.py RECIPE SDIST OUTPUT
6+
7+
The `zarr` sdist contents are an allowlist (see `[tool.hatch.build.targets.sdist]`
8+
in pyproject.toml), so the failure mode to guard against is shipping too little.
9+
conda-forge is the consumer most exposed to that: it builds from the sdist rather
10+
than the wheel, and runs our test suite from a directory holding only what its
11+
recipe copies out of the tarball.
12+
13+
Rather than restate that recipe here -- a copy would drift the moment the
14+
feedstock changed -- CI fetches the real one and this script repoints it at the
15+
sdist we just built. Everything that makes the check meaningful (the test files,
16+
the pytest invocation, `pip_check`, `license_file`) comes from the feedstock
17+
unmodified.
18+
19+
Note that only the parts of the recipe that identify *which* tarball to build are
20+
touched. The feedstock's `requirements:` are regenerated from our pyproject.toml
21+
by grayskull on each version bump (`bot: inspection: update-grayskull` in its
22+
conda-forge.yml), so they already track this repo; the `tests:` block is
23+
hand-maintained there and is exactly what we want to run verbatim.
24+
"""
25+
26+
import hashlib
27+
import sys
28+
import tarfile
29+
from pathlib import Path
30+
from typing import Any
31+
32+
import yaml
33+
34+
35+
def sdist_root(sdist: Path) -> str:
36+
"""The tarball's single top-level directory, e.g. `zarr-3.3.1`."""
37+
with tarfile.open(sdist) as tar:
38+
roots = {name.split("/", 1)[0] for name in tar.getnames()}
39+
if len(roots) != 1:
40+
raise SystemExit(f"Expected one top-level directory in {sdist}, got {roots}")
41+
return roots.pop()
42+
43+
44+
def conda_version(root: str) -> str:
45+
"""Derive a conda-acceptable version from the sdist directory name.
46+
47+
A build off a tag gives a clean `3.3.1`, but any other commit gives a
48+
setuptools-scm local version like `3.3.1.dev23+g52a63801`. Conda versions
49+
cannot contain `+`, and the local segment identifies the checkout rather
50+
than the release, so drop it. The value only labels the throwaway package
51+
this check builds; nothing is published from it.
52+
"""
53+
version = root.split("-", 1)[1]
54+
return version.split("+", 1)[0]
55+
56+
57+
def rewrite(recipe: dict[str, Any], sdist: Path) -> dict[str, Any]:
58+
root = sdist_root(sdist)
59+
context = recipe.setdefault("context", {})
60+
context["version"] = conda_version(root)
61+
# The recipe interpolates `${{ sha256 }}` into `source`, which is replaced
62+
# wholesale below; keep the key consistent anyway so a partially templated
63+
# recipe cannot silently reference a stale digest.
64+
context["sha256"] = hashlib.sha256(sdist.read_bytes()).hexdigest()
65+
66+
# A `url` source rather than a `path` one, so this mirrors what conda-forge
67+
# actually does: fetch an archive and unpack it. It also sidesteps a trap --
68+
# `path` sources honour .gitignore by default, and `src/zarr/_version.py` is
69+
# gitignored. It is written into the sdist by the hatch-vcs build hook
70+
# precisely because an unpacked sdist has no git history to derive a version
71+
# from, so silently dropping it would break the build.
72+
recipe["source"] = {
73+
"url": sdist.resolve().as_uri(),
74+
"sha256": context["sha256"],
75+
}
76+
return recipe
77+
78+
79+
if __name__ == "__main__":
80+
if len(sys.argv) != 4:
81+
print(__doc__)
82+
sys.exit(2)
83+
recipe_path, sdist_path, output_path = (Path(p) for p in sys.argv[1:])
84+
85+
with recipe_path.open("rb") as f:
86+
recipe = yaml.safe_load(f)
87+
88+
recipe = rewrite(recipe, sdist_path)
89+
90+
output_path.parent.mkdir(parents=True, exist_ok=True)
91+
with output_path.open("w") as f:
92+
yaml.safe_dump(recipe, f, sort_keys=False)
93+
94+
print(f"Wrote {output_path}")
95+
print(f" version: {recipe['context']['version']}")
96+
print(f" source: {recipe['source']['url']}")

0 commit comments

Comments
 (0)