fix: reset the cached compiler when build_ext is reinitialized - #150
Open
DivyamTalwar wants to merge 1 commit into
Open
fix: reset the cached compiler when build_ext is reinitialized#150DivyamTalwar wants to merge 1 commit into
DivyamTalwar wants to merge 1 commit into
Conversation
`httptools_build_ext.initialize_options()` returns early once `_initialized` is set, deliberately preserving the Cython and extension mutations applied on the first pass. Setuptools reinitializes the `build_ext` command between phases, so a chained invocation such as `setup.py build bdist_wheel` reaches the wheel phase holding the compiler instance the earlier phase already configured, and the build fails instead of selecting a compiler for the current phase. Clear only the transient `compiler` attribute in that early-return path. Delegating to the base initializer instead would discard exactly the cached command options and extension state the early return exists to protect, so the reset is kept as narrow as the problem. `tests/test_build.py` loads the real `setup.py`, finalizes the command, attaches a compiler, reinitializes, and asserts the run proceeds — it fails without this change. Verified: - Focused regression — 1 passed - `python setup.py build bdist_wheel` — succeeds - `make test` — 42 passed - `make typecheck` — 0 errors, 0 warnings - `git diff --check` — clean Fixes MagicStack#126
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.
Fixes #126
The problem
httptools_build_ext.initialize_options()short-circuits after the first pass, on purpose:That early return is load-bearing — it protects the Cython settings and extension mutations applied the first time through. But setuptools calls
reinitialize_command('build_ext')between phases, and the baseinitialize_options()it is bypassing is what clearsself.compiler. So a chained invocation reaches the second phase still holding the compiler instance the first phase configured:The wheel phase then runs against a stale compiler rather than selecting one for the phase it is actually in, and the build fails. Anything that drives both phases through one command — which is the normal shape for a build backend — hits it.
The fix
Clear only the transient attribute, inside the early-return path:
The obvious alternative — dropping the early return and calling the base initializer — was rejected: it discards exactly the cached command options and extension state the early return exists to protect.
compileris the one piece of per-phase state that must not survive reinitialization, so the reset is scoped to it and nothing else.Test
tests/test_build.pyis new. It loads the realsetup.pyviarunpy(withsetuptools.setupmocked out) to get the actualbuild_extsubclass rather than a reimplementation of it, finalizes the command, attaches a compiler, reinitializes, and asserts the run proceeds tobuild_extensions().It fails on
masterand passes with this change.Verification
python setup.py build bdist_wheelmake testmake typecheckgit diff --checkNot covered locally: other Python versions and non-macOS platforms — left to CI.