-
Notifications
You must be signed in to change notification settings - Fork 239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updates for Pyodide builds after pyodide-build
was unvendored
#2090
Merged
henryiii
merged 9 commits into
pypa:main
from
agriyakhetarpal:pyodide-build-has-been-unvendored-from-pyodide
Nov 22, 2024
Merged
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
aa3bf38
Fix typos and capitalise
agriyakhetarpal f2bdd7f
Add docstring for `install_xbuildenv` function
agriyakhetarpal d54ab6f
Include Emscripten, Pyodide version in logger messages
agriyakhetarpal 4b97ee8
Mark separation of pyodide-build and Pyodide
agriyakhetarpal 8037155
Bump to Pyodide 0.26.4 (py3.12.7) + pyodide-build 0.29.0
agriyakhetarpal 5ce4951
Use pyodide-build version as the source of truth
agriyakhetarpal f9b789b
Update Pyodide constraints
agriyakhetarpal 15f5784
Use 3.12 as the Python version instead of 3.12.7
agriyakhetarpal 35467fc
chore: appease the formatter
henryiii File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not respect overrides (like
latest
or pip installing a new version), but will always be the one hard-coded. Was that intentional?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is intentional, since we don't have any ways to override here (no environment variables or the like, etc.). We should be fine with the hardcoded version. We know that users can still override
pyodide-build
using their constraint files if they like, but that won't be of much use (unless there's a bug inpyodide-build
or if they want to use a fork or for any other reasons to use a constraint). I plan to rebase #2002 after this gets reviewed/merged.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, just saw your edit. How do we ensure that we use the version of
pyodide-build
that users install themselves, e.g., inbefore-build
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Constraint flags apply when it is setting up the environment. If a user installs it themselves, that will happen after the constrained version has already set up a new environment. Do we need the pyodide-build version in the env path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if I understood that correctly 😅 If I got it right,
Is "environment" here the cibuildwheel venv that will be used for the build? If these changes constrain the
pyodide-build
version in this environment and constraints files will have no effect, I'd definitely like to try out a different method. Could you give me a pointer on how I could proceed?Also, a side note – after thinking a bit more about this, I think it would be great not to hardcode the Pyodide version, too (not in this PR, though). In case we release a new version of
pyodide-build
, say, X, that drops compatibility with the Pyodide version Y listed inbuild-platforms.toml
, overriding the version here with constraints to use X would make it unable to work with Pyodide version Y.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the xbuildenv gets set up with the constrained versions, but before-build runs after the xbuildenv is setup, so it can't affect the version used to setup the xbuildenv. After that I believe you can change it, so it's only the pyodide-build version used for the xbuildenv setup that's a little harder to control.