Do not require downloading xbuildenv when updating package recipes #30
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
Co-Authored-By: Gyeongjae Choi <def6488@gmail.com>
Closed
ryanking13
approved these changes
Sep 18, 2024
pyodide_build/xbuildenv.py
Outdated
| @@ -250,7 +250,7 @@ def _download(self, url: str, path: Path) -> None: | |||
| Path to extract the cross-build environment to. | |||
| If the path already exists, raise an error. | |||
| """ | |||
| logger.info("Downloading Pyodide cross-build environment from %s", url) | |||
| logger.info(f"Downloading Pyodide cross-build environment from {url}") | |||
There was a problem hiding this comment.
I recently learned that it is not recommended not to use f-strings in log calls (pyodide/micropip#131).
There was a problem hiding this comment.
Thanks for the pointer! I'll keep that in mind and add a Ruff rule for this in another PR.
There was a problem hiding this comment.
Addressed in de4e3d7.
Co-Authored-By: Gyeongjae Choi <def6488@gmail.com>
Co-Authored-By: Gyeongjae Choi <def6488@gmail.com>
agriyakhetarpal
added a commit
that referenced
this pull request
Sep 19, 2024
## Description Thanks to the tip in #30 (comment), I enabled a Ruff rule to avoid using f-strings in future logging statements we add: https://docs.astral.sh/ruff/rules/logging-f-string/, and converted the previous ones to comply with the rule. It's not necessarily required, but it's good to enable best practices. > [!IMPORTANT] > This PR should be merged with a merge commit, not squashed.
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.
Description
Closes #25. This PR improves the xbuildenv search logic to not require downloads of the xbuildenv if a package update fails (for any reason). This means that the Pyodide root is searched in the current directory, and if it doesn't exist, we simply assume that the current working directory is the Pyodide root (and the
packages/directory one level down hosts the recipes).I confirmed that this does not currently cause issues with the existing setup in the Pyodide repository.