Skip to content

build(zarr-metadata): give the sdist an explicit include allowlist - #4248

Merged
d-v-b merged 1 commit into
zarr-developers:mainfrom
d-v-b:claude/zen-elgamal-331b96
Aug 12, 2026
Merged

build(zarr-metadata): give the sdist an explicit include allowlist#4248
d-v-b merged 1 commit into
zarr-developers:mainfrom
d-v-b:claude/zen-elgamal-331b96

Conversation

@d-v-b

@d-v-b d-v-b commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

claude wrote this, original PR against my fork is here: d-v-b#284

this change ensures that we have an allowlist for the contents of the source distribution (sdist) when we publish. without this configuration, anything in the package root could get sucked into the sdist and then published, which opens a lot of bad outcomes.

Summary

build(zarr-metadata): the sdist ships an allowlist, not whatever is lying around

hatchling had no sdist configuration here either, so a source distribution carried everything in the package directory. Building from a working tree with scratch files in it put .env.local, a notebook and a __scratch/ dump in the tarball. A tagged release builds from a fresh CI checkout and so was never actually at risk, but nothing made that a property of the package rather than of the runner.

The list is derived from this package rather than copied from zarr-indexing, which needed docs/snippets and examples/ because its suite executes them. Nothing here does: every fixture is a JSON file next to the test module that reads it, so /tests is the whole test dependency. /docs and /mkdocs.yml ride along because they are self-contained — mkdocstrings reads src and the config reaches nowhere outside the package — so the sdist documents itself as well as tests itself. changes/ and .readthedocs.yaml are left out: towncrier fragments are repo bookkeeping, and the RTD config addresses paths from the repo root, where an unpacked sdist is not.

Verified by unpacking the built sdist into a bare venv and working from there: 595 tests pass and mkdocs build --strict succeeds.

Assisted-by: ClaudeCode:claude-opus-5

For reviewers

[What would you most value a second look at? What are you already confident in? For a refactor, say whether behavior is meant to be unchanged.]

Author attestation

  • I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/user-guide/*.md
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

…ying around

hatchling had no sdist configuration here either, so a source distribution
carried everything in the package directory. Building from a working tree with
scratch files in it put `.env.local`, a notebook and a `__scratch/` dump in the
tarball. A tagged release builds from a fresh CI checkout and so was never
actually at risk, but nothing made that a property of the package rather than
of the runner.

The list is derived from this package rather than copied from zarr-indexing,
which needed `docs/snippets` and `examples/` because its suite executes them.
Nothing here does: every fixture is a JSON file next to the test module that
reads it, so `/tests` is the whole test dependency. `/docs` and `/mkdocs.yml`
ride along because they are self-contained — mkdocstrings reads `src` and the
config reaches nowhere outside the package — so the sdist documents itself as
well as tests itself. `changes/` and `.readthedocs.yaml` are left out: towncrier
fragments are repo bookkeeping, and the RTD config addresses paths from the
repo root, where an unpacked sdist is not.

Verified by unpacking the built sdist into a bare venv and working from there:
595 tests pass and `mkdocs build --strict` succeeds.

Assisted-by: ClaudeCode:claude-opus-5
@github-actions github-actions Bot added the needs release notes Automatically applied to PRs which haven't added release notes label Aug 12, 2026
@d-v-b
d-v-b force-pushed the claude/zen-elgamal-331b96 branch 2 times, most recently from 89e35fe to 13a1f9b Compare August 12, 2026 09:18
@d-v-b
d-v-b marked this pull request as ready for review August 12, 2026 09:19
@d-v-b

d-v-b commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

I will self-merge this when CI passes.

@d-v-b
d-v-b merged commit 5abdee2 into zarr-developers:main Aug 12, 2026
56 of 72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs release notes Automatically applied to PRs which haven't added release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant