fix(python): stop shipping build artifacts and ship the PEP 561 marker - #4857
Merged
Conversation
Two packaging defects in `src/fable-library-py`, pulling in opposite directions:
the wheel built in the source tree shipped far too much and was uninstallable,
while the wheel published to PyPI shipped too little.
Neither shows up in `git status` -- `.gitignore` has `[Oo]bj/`, so the offending
files are ignored by git but still swept up by maturin.
## The dev wheel was uninstallable
`Fable.Library.Python.fsproj` lives inside the package directory, so `dotnet build`
drops MSBuild output into `fable_library/obj/`. With `python-source = "."` maturin
packaged it wholesale, and one of those paths contains a comma:
fable_library/obj/Debug/net10.0/.NETCoreApp,Version=v10.0.AssemblyAttributes.fs
RECORD is CSV, so that is four fields where three are expected, and the wheel is
rejected:
Caused by: RECORD file is invalid
Caused by: CSV error: record 76: found record with 4 fields, but the previous
record has 3 fields
`pip` is lenient enough to survive this; `uv pip install` is not, which makes the
failure look intermittent. It blocked building a local `fable-library` to benchmark
against a released version.
The same wheel also carried 13 F# sources and 22 `__pycache__` entries -- including
another interpreter's bytecode (`array_.cpython-312.pyc` inside a cp314 wheel), a
stale-cache hazard inside an artifact whose purpose is to be a clean build.
Fixed with an explicit `exclude` in `[tool.maturin]`, which works regardless of
build order. Deleting `obj/` in the build script would not: it regenerates on the
next `dotnet build` and breaks again for the next person.
120 -> 68 entries, 0 build artifacts, installs cleanly. The sdist is covered too.
## The published wheel's type stubs were inert
Under PEP 561 the marker must sit *inside* the package directory. `CopyStage` copied
`GetFiles(SourceDir, "*.py")`, so `fable_library/py.typed` was dropped, while the
distribution-root copy was staged to a location where PEP 561 does not look. Every
released version therefore shipped `core/*.pyi` that mypy would not read.
Verified against an installed wheel, with and without the marker:
with: error: Incompatible types in assignment (expression has type "Int32",
variable has type "str")
without: error: Skipping analyzing "fable_library.core": module is installed,
but missing library stubs or py.typed marker
Note this is narrower than it first appears: pyright reads the stubs either way,
since `useLibraryCodeForTypes` defaults to true. It is mypy that gets nothing, and
`py.typed` is what makes the stubs authoritative rather than inferred.
The inert distribution-root `py.typed` is removed rather than left to suggest the
marker was handled.
## Also
- Dropped the vestigial `[tool.hatch.build.targets.*]` config and `hatchling` from
`build-system.requires`. `build-backend` is maturin, so those settings did
nothing -- while looking exactly like they governed what ships, which is a good
way for a packaging bug to go unnoticed.
- Added a one-line assertion to `build-python` that the marker survives staging.
`CopyStage` filters by extension, so this is easy to drop again, and it costs
nothing in a job that already stages the library.
Deliberately not guarded in CI: wheel contents. A clean checkout has no `obj/` or
`__pycache__`, so a guard would have to dirty the tree first, and the sdist does not
exercise those exclusions at all -- maturin honours gitignore for sdists, so an
sdist built with `exclude = []` still shows 0 obj/ entries. Only a full wheel build
reproduces the failure, which is minutes of CI per PR to protect a static config
block that carries its own explanation, against a break that never reached the
published artifact.
Not addressed: moving the fsproj out of `fable_library/` would remove the cause
rather than filter it, but Fable is invoked on the directory and 12 F# sources plus
8 linked files live there, so it is a restructure rather than a packaging fix.
Verified: 2418 Python tests, 270 fable-library-py tests, wheel and sdist clean and
installable, marker present in the staged layout.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Python Type Checking Results (Pyright)
Excluded files with errors (4 files)These files have known type errors and are excluded from CI. Remove from
|
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
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.
Two packaging defects in
src/fable-library-py, pulling in opposite directions: the wheel built in the source tree shipped far too much and was uninstallable, while the wheel published to PyPI shipped too little.Neither shows up in
git status—.gitignorehas[Oo]bj/, so the offending files are invisible to git but still swept up by maturin.1. The dev wheel was uninstallable
Fable.Library.Python.fsprojlives inside the package directory, sodotnet builddrops MSBuild output intofable_library/obj/. Withpython-source = "."maturin packaged it wholesale — and one of those paths contains a comma:RECORDis CSV, so that is four fields where three are expected:pipis lenient enough to survive this;uv pip installis not — which makes it look intermittent. It blocked building a localfable-libraryto benchmark against a released version.The same wheel also carried 13 F# sources and 22
__pycache__entries, including another interpreter's bytecode (array_.cpython-312.pycinside a cp314 wheel) — a stale-cache hazard inside an artifact whose whole purpose is to be a clean build.Fixed with an explicit
excludein[tool.maturin], which holds regardless of build order. Deletingobj/in the build script would not: it regenerates on the nextdotnet buildand breaks again for the next person.The sdist is covered by the same config (92 entries, 0 junk).
2. The published wheel's type stubs were inert
Under PEP 561 the marker must sit inside the package directory.
CopyStagecopiedGetFiles(SourceDir, "*.py"), sofable_library/py.typedwas dropped, while the distribution-root copy was staged to a location where PEP 561 does not look. Every released version has therefore shippedcore/*.pyistubs that mypy would not read.Verified against an installed wheel, with and without the marker:
Narrower than it first appears: pyright reads the stubs either way, since
useLibraryCodeForTypesdefaults to true. It is mypy that gets nothing;py.typedis what makes the stubs authoritative rather than inferred.The inert distribution-root
py.typedis removed rather than left to suggest the marker was handled.3. Vestigial build config
Dropped
[tool.hatch.build.targets.*]andhatchlingfrombuild-system.requires.build-backendis maturin, so those settings did nothing — while looking exactly like they governed what ships. Two build backends configured in one file is a good way for a packaging bug to go unnoticed.On CI coverage
Added one line to
build-python, which already stages the library: assertpy.typedsurvives. That guards the regression class that actually occurred — aCopyStageextension filter silently dropping a file — and is also the code this PR changes, so it is the most likely thing to break again. It costs nothing in a job that already runs the staging step.Deliberately not guarded: wheel contents. A clean checkout has no
obj/or__pycache__, so any guard must dirty the tree first. I tried to make it cheap viamaturin sdist(0.3s, no Rust compile) and measured that it does not work — maturin honours gitignore for sdists:excludepresentexclude = [].fs, zeroobj/A cheap guard would pass while the actual breakage went through. Only a full wheel build reproduces it: minutes of CI per PR to protect a static config block that now carries its own explanation, against a break that never reached the published artifact.
Not addressed
Moving the
.fsprojout offable_library/would remove the cause rather than filter it. But Fable is invoked on the directory (CmdLine.appendRaw sourceDir), and 12 F# sources plus 8 linked files fromfable-library-tslive there — sosourceDirconflates "where the F# project is" with "where the Python package is". That is a restructure, not a packaging fix, and belongs on its own.Verification
🤖 Generated with Claude Code