Fix out-of-tree PyPI dependency resolution correctness bugs - #385
Closed
henryiii wants to merge 2 commits into
Closed
Fix out-of-tree PyPI dependency resolution correctness bugs#385henryiii wants to merge 2 commits into
henryiii wants to merge 2 commits into
Conversation
Fixes four correctness bugs in the out-of-tree PyPI resolver: - Forward isolation / skip_dependency_check flags from _resolve_and_build into download_or_build_wheel and into the sdist builds used for metadata resolution (via new PyPIProvider.BUILD_ISOLATION / BUILD_SKIP_DEPENDENCY_CHECK class attributes), using keyword arguments to prevent positional-argument mistakes. - Union extras across all requirements in PyPIProvider.find_matches so a package required both with and without extras resolves all dependencies, and evaluate dependency markers once per requested extra so extra == "a" matches when any requested extra satisfies it. - Support non-gzip sdists (.zip, .tar.bz2, ...) when building from PyPI and raise a clear error for unsupported archive types instead of UnboundLocalError. - Parse PyPI source locations with packaging.requirements.Requirement in _extract_extras so name, extras (including comma-separated) and the version specifier are all preserved. Part of pyodide#376. Assisted-by: ClaudeCode:claude-opus-4-8
Assisted-by: ClaudeCode:claude-opus-4-8
Member
|
We are actually planning to remove this feature soon in #362. So let me close this. Thanks anyways! |
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.
🤖 AI text below 🤖
Part of #376.
This fixes four verified correctness bugs in the out-of-tree PyPI dependency resolution (
pyodide_build/out_of_tree/pypi.pyandpyodide_build/cli/build.py).Bug 1:
isolation/skip_dependency_checksilently dropped for dependency builds_resolve_and_buildcalleddownload_or_build_wheel(x.url, target_folder, compression_level), where the third positional argument landed incompression_leveland theisolation/skip_dependency_checkflags were never forwarded. The same happened during metadata resolution, whereget_built_wheel(url)was called with no flags. As a resultpyodide build -r reqs.txt --no-isolation -xstill built dependency sdists with default isolation.Fix: forward the flags using keyword arguments throughout the call chain. Because metadata resolution flows through
Candidate->get_metadata_for_wheel->get_built_wheel(which has no direct access to the flags), the flags are also stored on newPyPIProvider.BUILD_ISOLATION/BUILD_SKIP_DEPENDENCY_CHECKclass attributes (mirroring the existingBUILD_FLAGS/BUILD_EXPORTSpattern) and honored there.Bug 2:
PyPIProvider.find_matchesonly honored the last extras setThe loop over
extra_requirementsrebound thecandidatesgenerator each iteration, so only the last extras set was consumed. When a package was required both with and without extras (fooandfoo[bar]), dependencies pulled in by the lost extras were never resolved.Fix: union the extras across all requirements and create the candidates once.
Candidate._get_dependenciesevaluates eachextra == "..."marker against every extra in the set, so a single candidate carrying the union is the correct choice.Related: marker evaluation in
_resolve_and_buildused{"extra": ",".join(extras)}, which can never match a marker likeextra == "a"when there are multiple extras. Now the marker is evaluated once per requested extra (plus the empty extra) and the requirement is included if it matches any of them.Bug 3:
UnboundLocalErrorfor non-gz, non-whl sdist URLsdownload_or_build_wheelonly handled URLs ending ingzor.whl. For a.zipor.tar.bz2sdist (still present on PyPI for older packages),wheel_pathwas never assigned and the laterrepack_zip_archive(wheel_path, ...)raisedUnboundLocalError.Fix: route any supported sdist archive format (
.tar.gz,.tgz,.tar.bz2,.tbz2,.tar.xz,.txz,.tar,.zip) to the builder (the build path is format-agnostic viashutil.unpack_archive), and raise a clear error for unsupported types. The temp file used during the build now preserves the archive suffix so the format is inferred correctly.get_metadata_for_wheelwas made consistent.Bug 4:
_extract_extrasdropped version specifiers and mishandled multiple extrasFor
"pkg[extra]==1.0"the hand-rolled regex discarded==1.0(so the latest version was fetched), and for"pkg[a,b]"the regex\[(\w+)\]did not match commas, so no extras were extracted.Fix: parse with
packaging.requirements.Requirementso name, extras (including comma-separated) and the version specifier are all preserved. The specifier is folded back into the source-location string that flows tofetch_pypi_package. Non-requirement source locations (URLs, paths) are returned unchanged.Tests
Added unit tests in
pyodide_build/tests/test_pypi.py:_resolve_and_buildintodownload_or_build_wheel(monkeypatched) and onto thePyPIProviderclass attributes;find_matcheswithfoo+foo[bar]honors the extras union; a candidate with multiple extras resolves dependencies whose markers match any single extra;UnboundLocalError), and a.zipsdist is routed to the builder with flags forwarded;_extract_extras("pkg[a,b]==1.0")yields namepkg==1.0and extras["a", "b"], with specifiers preserved and URLs passed through.🤖 Generated with Claude Code