Add a new configuration variable for setting default xbuildenv url #85
Conversation
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
ryanking13
commented
Jan 15, 2025
pyodide_build/config.py
Outdated
Comment on lines
16
to
20
| class ConfigManager: | ||
| """ | ||
| Configuration manager for pyodide-build. | ||
| This class works "before" installing the cross build environment. | ||
| So it does not have access to the variables that are retrieved from the cross build environment. |
There was a problem hiding this comment.
The new config variable needs to be accessed before downloading the cross-build env, so I subclassed the ConfigManager into ConfigManager and CrossBuildEnvConfigManager. The former can be used even before installing the cross build env.
There was a problem hiding this comment.
Can we separate splitting these into its own PR?
There was a problem hiding this comment.
Thanks. Sounds reasonable. Let me split the PR
hoodmane
approved these changes
Jan 23, 2025
There was a problem hiding this comment.
Thanks @ryanking13. I think everything makes sense and it's a nice feature. I would be nice to split the config as a separate PR since that part of the diff is quite noisy.
|
Thanks for the review! |
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.
This PR adds a new config variable:
default_cross_build_env_url.This variable is used to set the default xbuildenv URL, which will be downloaded when a user calls
pyodide xbuildenv installwithout passing the version, or when a user callspyodide buildwithout explicitly installing the xbuildenv.When is this variable useful?
For now, building packages using the nightly cross-build env, we need to explicitly install it from URL:
For instance, in pyodide-recipes repository, this is set in CI script.
However, I would like to put this value in the
pyproject.toml, which, I think is more appropriate place to put build-related variables.