gh-145177: Support multiple Emscripten versions for Emscripten buildbot#145180
gh-145177: Support multiple Emscripten versions for Emscripten buildbot#145180freakboy3742 merged 7 commits intopython:mainfrom
Conversation
…buildbot This adds an `--emsdk-cache` argument to the Emscripten build script and an emscripten_version.txt file. If the `--emsdk-cache` argument is passed, the build script will look in emscripten_version.txt to get the expected emsdk version is installed in a folder called e.g., 4.0.12 in the directory indicated by the `--emsdk-cache` argument. Otherwise, it will exit with an error.
|
Would appreciate review on this one next @freakboy3742 |
Ack - I've been tied up with some other things at work. Will take a look as soon as I get a moment - probably early next week. |
freakboy3742
left a comment
freakboy3742
left a comment
There was a problem hiding this comment.
The basic structure for this looks good; the only question is about whether it... actually works. The environment persistence stuff doesn't seem to work like this code suggests it will.
I know you've made the change to put the Emscripten version in code; however, Brett's comment about making it easy to backport is a fair one - maybe we want to revert to the older approach?
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
This reverts commit f1a5336.
Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
freakboy3742
left a comment
freakboy3742
left a comment
There was a problem hiding this comment.
This looks good, and worked in my testing; one clarifying question inline that doesn't seem to impact on how the script works, but I want to make sure I understand the reasoning for the choice.
|
!buildbot emscripten |
|
🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit 90f65ea 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145180%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
|
Looks like #145439 has broken CI for Emscripten; we'll need to fix that before landing this PR. |
|
On the subject of which - add "adding Emscripten to GitHub CI" to the TODO list, as that will help avoid this sort of problem, and CI is otherwise stable. |
|
!buildbot emscripten |
|
🤖 New build scheduled with the buildbot fleet by @hoodmane for commit 3b80a0b 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145180%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
|
!buildbot emscripten |
|
🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit b66e2fa 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145180%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
freakboy3742
left a comment
freakboy3742
left a comment
There was a problem hiding this comment.
👍 Looks great - thanks for those updates.
|
@hoodmane I guess the question is whether we backport this to 3.14. That would make updating the buildbot script easier... |
3 participants
This adds an
--emsdk-cacheargument to the Emscripten build script and an emscripten_version.txt file. If the--emsdk-cacheargument is passed, the build script will look in emscripten_version.txt to get the expected emsdk version is installed in a folder called e.g., 4.0.12 in the directory indicated by the--emsdk-cacheargument. Otherwise, it will exit with an error.