ci: check the sdist can actually be consumed

.github/workflows/releases.yml

@@ -48,24 +48,102 @@ jobs:
           name: releases
           path: dist
 
+  # The sdist contents are defined by an allowlist in pyproject.toml, so the
+  # failure mode to guard against is shipping too little: a new file that
+  # consumers need, added without a matching allowlist entry. This job and
+  # `test_dist_conda` below inspect the built artifact rather than the working
+  # tree, because a git checkout has files the tarball does not.
+  #
+  # Seconds, no network beyond the artifact download, so it runs everywhere the
+  # workflow does -- including pull requests.
   test_dist_pypi:
     name: Test distribution artifacts
     needs: [build_artifacts]
     runs-on: ubuntu-latest
     steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+
       - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
         with:
           name: releases
           path: dist
 
-      - name: test
+      - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
+        name: Install Python
+        with:
+          python-version: '3.12'
+
+      # Catches what the conda build cannot: paths that no test opens and no
+      # build step reads, and paths that must never ship (packages/, which is
+      # three separately released distributions -- conda would not object to
+      # those, but we do).
+      - name: Check sdist contents
+        run: python ci/check_sdist_contents.py dist
+
+  # 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. Run the real recipe rather
+  # than a copy of it -- a copy would go stale the moment the feedstock changed,
+  # and silently, which is the same class of bug this job exists to catch.
+  #
+  # Not on pull requests. It runs the whole suite from an unpacked sdist, which
+  # is ~10 minutes whether or not conda is involved, and it depends on a fetch
+  # from conda-forge/zarr-feedstock, so a recipe change there would redden
+  # unrelated PRs. On `push` to main it still catches a bad merge within
+  # minutes, attributed to one commit, and it gates `upload_pypi` so no release
+  # can publish an sdist conda cannot consume.
+  #
+  # To run it against a branch before merging, use `workflow_dispatch`. Note
+  # that dispatch only offers branches of the repository it runs in, so for a
+  # pull request from a fork that means dispatching this workflow in the fork,
+  # not here.
+  test_dist_conda:
+    name: Test conda-forge can consume the sdist
+    needs: [build_artifacts]
+    if: github.event_name != 'pull_request'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+
+      - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
+        with:
+          name: releases
+          path: dist
+
+      - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
+        name: Install Python
+        with:
+          python-version: '3.12'
+
+      - name: Fetch the conda-forge recipe
         run: |
-          ls
-          ls dist
+          curl -fsSL --retry 3 -o "${RUNNER_TEMP}/recipe-upstream.yaml" \
+            https://raw.githubusercontent.com/conda-forge/zarr-feedstock/main/recipe/recipe.yaml
+
+      - name: Point the recipe at the sdist we just built
+        run: |
+          python -m pip install pyyaml==6.0.3
+          python ci/conda_recipe_for_sdist.py \
+            "${RUNNER_TEMP}/recipe-upstream.yaml" \
+            "$(ls dist/*.tar.gz)" \
+            conda.recipe/recipe.yaml
+
+      # Runs the recipe's own `tests:` block: an import check, `pip check`, and
+      # our suite from a tree containing only the recipe's `files.source`.
+      - uses: prefix-dev/rattler-build-action@1ca5f45832f419a46d1326ccc5861d7e14d67c44 # v0.2.39
+        with:
+          rattler-build-version: v0.73.0
+          recipe-path: conda.recipe/recipe.yaml
+          upload-artifact: false
+          build-args: -c conda-forge --output-dir ${{ runner.temp }}/conda-build
 
   upload_pypi:
     name: Upload to PyPI
-    needs: [build_artifacts, test_dist_pypi]
+    needs: [build_artifacts, test_dist_pypi, test_dist_conda]
     runs-on: ubuntu-latest
     if: github.event_name == 'release'
     environment:

.gitignore

@@ -7,6 +7,9 @@ __pycache__/
 
 # Distribution / packaging
 .Python
+# Written by ci/conda_recipe_for_sdist.py when checking the sdist against the
+# conda-forge recipe; generated per run, never committed.
+conda.recipe/
 env/
 .venv/
 build/

ci/check_sdist_contents.py

@@ -0,0 +1,149 @@
+"""
+Check a built sdist ships everything its consumers need, and nothing they don't.
+
+Usage:
+    python check_sdist_contents.py SDIST
+
+SDIST is a path to a `.tar.gz`, or a directory containing exactly one.
+
+`[tool.hatch.build.targets.sdist]` in pyproject.toml is an allowlist. 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: instead of
+shipping too much silently, an allowlist can ship too little. This script
+guards that direction.
+
+Wherever possible the expectations are *derived* from configuration that is
+maintained for other reasons, rather than restated here. A hand-written list of
+"files the sdist must contain" would just be a second allowlist to forget to
+update, which is the problem it is meant to solve.
+
+Everything is read out of the tarball, including the configuration the
+expectations are derived from. The check is therefore a property of the
+artifact alone: it gives the same answer against a release sdist downloaded
+from PyPI as it does against a fresh `hatch build`, with no working tree in
+the picture -- which matters, because a git checkout has files the tarball
+does not.
+"""
+
+import sys
+import tarfile
+import tomllib
+from pathlib import Path
+
+# Paths no test opens, so the sdist test run in `releases.yml` cannot vouch for
+# them, but that packagers do need. conda-forge's recipe installs the sdist
+# (pyproject.toml, README.md, src/) and reads `license_file: LICENSE.txt`; the
+# version file is written into the sdist by the hatch-vcs hook, because an
+# unpacked sdist has no git history to derive a version from.
+# https://github.com/conda-forge/zarr-feedstock/blob/main/recipe/recipe.yaml
+REQUIRED_PATHS = [
+    "pyproject.toml",
+    "README.md",
+    "LICENSE.txt",
+    "PKG-INFO",
+    "src/zarr/__init__.py",
+    "src/zarr/py.typed",
+    "src/zarr/_version.py",
+]
+
+# Each subpackage under `packages/` is its own distribution with its own PyPI
+# release, its own version tags and its own sdist. Before the allowlist they
+# were bundled into every `zarr` sdist -- 2.9M of unrelated sources -- which is
+# the regression this check exists to prevent recurring.
+FORBIDDEN_PREFIXES = ["packages/"]
+
+
+def sdist_members(sdist: Path) -> set[str]:
+    """Every path inside the tarball, relative to its top-level directory."""
+    with tarfile.open(sdist) as tar:
+        names = tar.getnames()
+    # Members are `zarr-<version>/<path>`; strip the leading component so the
+    # expectations below don't have to know the version.
+    return {name.split("/", 1)[1] for name in names if "/" in name}
+
+
+def read_member(sdist: Path, path: str) -> bytes:
+    """Read one member of the tarball, addressed relative to its top-level directory."""
+    with tarfile.open(sdist) as tar:
+        roots = {name.split("/", 1)[0] for name in tar.getnames()}
+        if len(roots) != 1:
+            raise SystemExit(f"Expected one top-level directory in {sdist}, got {sorted(roots)}")
+        member = tar.extractfile(f"{roots.pop()}/{path}")
+        if member is None:
+            raise SystemExit(f"{path} is missing from {sdist.name}")
+        return member.read()
+
+
+def testpaths(sdist: Path) -> list[str]:
+    """`testpaths` from the pyproject.toml the sdist ships.
+
+    Derived rather than duplicated: adding a directory to `testpaths` without
+    adding it to the sdist allowlist is exactly the mistake this catches. It is
+    also the mistake that was already live -- `docs/user-guide` has been a
+    testpath while `docs/` was excluded, so `pytest` on an unpacked sdist died
+    at collection.
+
+    Read from the tarball rather than the working tree so the two halves of the
+    comparison always come from the same artifact.
+    """
+    config = tomllib.loads(read_member(sdist, "pyproject.toml").decode())
+    return config["tool"]["pytest"]["ini_options"]["testpaths"]
+
+
+def check(sdist: Path) -> int:
+    print(f"Checking {sdist.name}")
+    members = sdist_members(sdist)
+    print(f"Found {len(members)} paths")
+    print()
+
+    missing_required = [p for p in REQUIRED_PATHS if p not in members]
+    # A testpath is a directory; it is present if anything ships beneath it.
+    # Skipped when pyproject.toml itself did not ship: it is already reported as
+    # a missing required path, and there is nothing left to read `testpaths` out
+    # of.
+    expected = [] if "pyproject.toml" in missing_required else testpaths(sdist)
+    missing_testpaths = [p for p in expected if not any(m.startswith(f"{p}/") for m in members)]
+    forbidden = sorted(m for m in members if any(m.startswith(p) for p in FORBIDDEN_PREFIXES))
+
+    if not (missing_required or missing_testpaths or forbidden):
+        print("OK")
+        return 0
+
+    if missing_required:
+        print("Required paths missing from the sdist")
+        print("-------------------------------------")
+        print("\n".join(missing_required))
+        print()
+    if missing_testpaths:
+        print("testpaths entries missing from the sdist")
+        print("----------------------------------------")
+        print("\n".join(missing_testpaths))
+        print("`pytest` on an unpacked sdist will fail to collect these.")
+        print()
+    if forbidden:
+        print("Paths that must not ship in the zarr sdist")
+        print("------------------------------------------")
+        print("\n".join(forbidden[:20]))
+        if len(forbidden) > 20:
+            print(f"... and {len(forbidden) - 20} more")
+        print()
+
+    print(
+        "Fix by editing `[tool.hatch.build.targets.sdist]` in pyproject.toml. "
+        "It is an allowlist: a path ships only if it is listed."
+    )
+    return 1
+
+
+if __name__ == "__main__":
+    if len(sys.argv) != 2:
+        print(__doc__)
+        sys.exit(2)
+    target = Path(sys.argv[1]).resolve()
+    if target.is_dir():
+        tarballs = sorted(target.glob("*.tar.gz"))
+        if len(tarballs) != 1:
+            print(f"Expected exactly one .tar.gz in {target}, found {len(tarballs)}")
+            sys.exit(2)
+        target = tarballs[0]
+    sys.exit(check(target))

ci/conda_recipe_for_sdist.py

@@ -0,0 +1,96 @@
+"""
+Point the conda-forge recipe at a locally built sdist.
+
+Usage:
+    python conda_recipe_for_sdist.py RECIPE SDIST OUTPUT
+
+The `zarr` sdist contents are an allowlist (see `[tool.hatch.build.targets.sdist]`
+in pyproject.toml), so the failure mode to guard against is shipping too little.
+conda-forge is the consumer most exposed to that: it builds from the sdist rather
+than the wheel, and runs our test suite from a directory holding only what its
+recipe copies out of the tarball.
+
+Rather than restate that recipe here -- a copy would drift the moment the
+feedstock changed -- CI fetches the real one and this script repoints it at the
+sdist we just built. Everything that makes the check meaningful (the test files,
+the pytest invocation, `pip_check`, `license_file`) comes from the feedstock
+unmodified.
+
+Note that only the parts of the recipe that identify *which* tarball to build are
+touched. The feedstock's `requirements:` are regenerated from our pyproject.toml
+by grayskull on each version bump (`bot: inspection: update-grayskull` in its
+conda-forge.yml), so they already track this repo; the `tests:` block is
+hand-maintained there and is exactly what we want to run verbatim.
+"""
+
+import hashlib
+import sys
+import tarfile
+from pathlib import Path
+from typing import Any
+
+import yaml
+
+
+def sdist_root(sdist: Path) -> str:
+    """The tarball's single top-level directory, e.g. `zarr-3.3.1`."""
+    with tarfile.open(sdist) as tar:
+        roots = {name.split("/", 1)[0] for name in tar.getnames()}
+    if len(roots) != 1:
+        raise SystemExit(f"Expected one top-level directory in {sdist}, got {roots}")
+    return roots.pop()
+
+
+def conda_version(root: str) -> str:
+    """Derive a conda-acceptable version from the sdist directory name.
+
+    A build off a tag gives a clean `3.3.1`, but any other commit gives a
+    setuptools-scm local version like `3.3.1.dev23+g52a63801`. Conda versions
+    cannot contain `+`, and the local segment identifies the checkout rather
+    than the release, so drop it. The value only labels the throwaway package
+    this check builds; nothing is published from it.
+    """
+    version = root.split("-", 1)[1]
+    return version.split("+", 1)[0]
+
+
+def rewrite(recipe: dict[str, Any], sdist: Path) -> dict[str, Any]:
+    root = sdist_root(sdist)
+    context = recipe.setdefault("context", {})
+    context["version"] = conda_version(root)
+    # The recipe interpolates `${{ sha256 }}` into `source`, which is replaced
+    # wholesale below; keep the key consistent anyway so a partially templated
+    # recipe cannot silently reference a stale digest.
+    context["sha256"] = hashlib.sha256(sdist.read_bytes()).hexdigest()
+
+    # A `url` source rather than a `path` one, so this mirrors what conda-forge
+    # actually does: fetch an archive and unpack it. It also sidesteps a trap --
+    # `path` sources honour .gitignore by default, and `src/zarr/_version.py` is
+    # gitignored. It is written into the sdist by the hatch-vcs build hook
+    # precisely because an unpacked sdist has no git history to derive a version
+    # from, so silently dropping it would break the build.
+    recipe["source"] = {
+        "url": sdist.resolve().as_uri(),
+        "sha256": context["sha256"],
+    }
+    return recipe
+
+
+if __name__ == "__main__":
+    if len(sys.argv) != 4:
+        print(__doc__)
+        sys.exit(2)
+    recipe_path, sdist_path, output_path = (Path(p) for p in sys.argv[1:])
+
+    with recipe_path.open("rb") as f:
+        recipe = yaml.safe_load(f)
+
+    recipe = rewrite(recipe, sdist_path)
+
+    output_path.parent.mkdir(parents=True, exist_ok=True)
+    with output_path.open("w") as f:
+        yaml.safe_dump(recipe, f, sort_keys=False)
+
+    print(f"Wrote {output_path}")
+    print(f"  version: {recipe['context']['version']}")
+    print(f"  source:  {recipe['source']['url']}")