Repository navigation
Conversation
d-v-b
marked this pull request as ready for review
August 13, 2026 11:15
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
check_sdist_contents.py derived `testpaths` from the working tree's pyproject.toml while comparing it against the tarball's contents, so the two halves of the comparison could come from different trees -- and the job comment claims the opposite, that these checks inspect the artifact rather than the checkout. Read pyproject.toml out of the tarball instead, which makes the check a property of the artifact alone: the same answer against a release sdist downloaded from PyPI as against a fresh `hatch build`. Also pin pyyaml, matching how hatch is installed in test.yml, and correct the workflow_dispatch note: dispatch only offers branches of the repository it runs in, so for a fork PR that means dispatching in the fork. Assisted-by: ClaudeCode:claude-opus-5
d-v-b
force-pushed
the
claude/zarr-sdist-ci-checks
branch
from
August 13, 2026 14:33
22cf142 to
749ab76
Compare
github-actions
Bot
added
the
needs release notes
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4263 +/- ##
=======================================
Coverage 94.71% 94.71%
=======================================
Files 94 94
Lines 13663 13663
=======================================
Hits 12941 12941
Misses 722 722 🚀 New features to boost your workflow:
|
Assisted-by: Codex:GPT-6
Assisted-by: Codex:GPT-6
maxrjones
reviewed
Oct 17, 2026
maxrjones
left a comment
maxrjones
left a comment
Member
There was a problem hiding this comment.
I like the check_sdist_contents addition. I'd rather not add the conda_recipe_for_sdist.py because I don't think it's worth replicating conda forge's testing infra in our CI. I also would prefer more concise comments. A lot of the comments seem overly verbose and/or repetitive about the motivations for the changes.
Contributor
Author
|
good points, I'll remove the conda recipe thing and tighten up the comments |
Resolves the releases.yml conflict with main's action pin bumps (zarr-developers#4552). Assisted-by: ClaudeCode:claude-opus-5.5
Review asked to keep the contents check and drop the conda-forge recipe job. The check now also requires `examples/`, which `tests/test_examples.py` globs and silently skips when it is missing, and stops listing files hatchling always adds. It runs through a new `just check-sdist` recipe, as the other ci/ scripts do. Assisted-by: ClaudeCode:claude-opus-5.5
d-v-b
changed the title
github-actions
Bot
removed
the
needs release notes
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI text below 🤖
The release workflow now checks that the built sdist contains every
testpathsentry,examples/, the generated_version.pyandpy.typed, and nothing frompackages/. Run it locally withjust check-sdistafterjust build.The
test_dist_pypijob used to runls dist. It now runsci/check_sdist_contents.pyon the built tarball, readingtestpathsfrom thepyproject.tomlinside the tarball.examples/is listed by hand becausetests/test_examples.pyglobs it and skips silently when it is missing.Checked locally against
uv build --sdist: a clean build passes, and removing/docsor/examplesfrom the allowlist, or adding/packages, fails the check.History
An earlier version also built the sdist with the conda-forge recipe; that job was dropped after review. Main was merged in to resolve a conflict with the action pin bumps from #4552.
🤖 Generated with Claude Code