From 8dc9a3b3603c5d17f26b121d6dc66981bef8c0a0 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Tue, 28 Jul 2026 01:30:57 +0200 Subject: [PATCH] fix(python): stop shipping build artifacts and ship the PEP 561 marker 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) --- .github/workflows/build.yml | 9 ++++++++ src/Fable.Build/FableLibrary/Python.fs | 6 ++++++ src/fable-library-py/py.typed | 1 - src/fable-library-py/pyproject.toml | 30 ++++++++++++++++++-------- 4 files changed, 36 insertions(+), 10 deletions(-) delete mode 100644 src/fable-library-py/py.typed diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 245186ccc7..9586b9cf85 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -179,6 +179,15 @@ jobs: if: matrix.platform == 'ubuntu-latest' run: ./build.sh fable-library --python + # PEP 561 requires the marker inside the package directory. CopyStage filters + # by extension, so it is easy to drop again; without it mypy refuses to read + # the core/*.pyi stubs at all. + - name: PEP 561 marker survives staging + if: matrix.platform == 'ubuntu-latest' + run: | + test -f temp/fable-library-py/fable_library/py.typed \ + || { echo "::error::fable_library/py.typed missing from staged package; stubs would be inert for mypy"; exit 1; } + - name: Fable Library - Python (Windows) if: matrix.platform == 'windows-latest' run: .\build.bat fable-library --python diff --git a/src/Fable.Build/FableLibrary/Python.fs b/src/Fable.Build/FableLibrary/Python.fs index e36f528206..d2e1c7c49c 100644 --- a/src/Fable.Build/FableLibrary/Python.fs +++ b/src/Fable.Build/FableLibrary/Python.fs @@ -31,6 +31,12 @@ type BuildFableLibraryPython(?skipCore: bool, ?postFableBuildStage: unit -> unit Directory.GetFiles(this.LibraryDir, "*") |> Shell.copyFiles this.BuildDir Directory.GetFiles(this.SourceDir, "*.py") |> Shell.copyFiles this.OutDir + // PEP 561 marker. It has to sit *inside* the package directory for type + // checkers to read the stubs at all, and the `*.py` filter above drops it -- + // which is why every published wheel so far has shipped `core/*.pyi` that no + // consumer could see. + [ Path.Combine(this.SourceDir, "py.typed") ] |> Shell.copyFiles this.OutDir + // Python extension modules Directory.GetFiles(Path.Combine(this.SourceDir, "core"), "*") |> Shell.copyFiles (Path.Combine(this.OutDir, "core")) diff --git a/src/fable-library-py/py.typed b/src/fable-library-py/py.typed deleted file mode 100644 index 4882510100..0000000000 --- a/src/fable-library-py/py.typed +++ /dev/null @@ -1 +0,0 @@ -# PEP-0561 -- https://www.python.org/dev/peps/pep-0561/ diff --git a/src/fable-library-py/pyproject.toml b/src/fable-library-py/pyproject.toml index 10c5d2f596..ba3499a1e6 100644 --- a/src/fable-library-py/pyproject.toml +++ b/src/fable-library-py/pyproject.toml @@ -23,20 +23,32 @@ dev = [ "pydantic>=2.13.4", ] -[tool.hatch.build.targets.sdist] -include = ["./fable_library"] - -[tool.hatch.build.targets.wheel] -include = ["./fable_library"] - -[tool.hatch.build.targets.wheel.sources] -"./fable_library" = "fable_library" +# Note: there is deliberately no [tool.hatch.*] config here. `build-backend` below +# is maturin, so hatch settings would be dead config that merely *looks* like it +# governs what ships -- which is a good way for a packaging bug to go unnoticed. [tool.maturin] python-source = "." # Look at project root module-name = "fable_library.core._core" # Full module path features = ["pyo3/extension-module"] +# `Fable.Library.Python.fsproj` lives inside the package directory, so `dotnet build` +# drops MSBuild output into `fable_library/obj/`. With `python-source = "."` maturin +# would package that wholesale, and one of those paths contains a comma +# (`.NETCoreApp,Version=v10.0.AssemblyAttributes.fs`) -- RECORD is CSV, so the +# resulting wheel is rejected by `uv pip install` as malformed. `pip` is lenient +# enough to survive it, which makes the failure look intermittent. +# +# None of this shows up in a clean CI checkout, only for contributors who have run +# `dotnet build`. Excluding here fixes it regardless of build order; deleting `obj/` +# in the build script would not, since it regenerates on the next build. +exclude = [ + "fable_library/obj/**/*", + "**/__pycache__/**/*", + "fable_library/*.fs", + "fable_library/*.fsproj", +] + [tool.ty.environment] python-version = "3.12" @@ -56,5 +68,5 @@ testpaths = ["tests", "tests-benchmark"] addopts = "--benchmark-columns=min,max,mean,stddev,iqr" [build-system] -requires = ["hatchling", "maturin>=1.5,<2.0"] +requires = ["maturin>=1.5,<2.0"] build-backend = "maturin"