gh-145219: Cache Emscripten libffi and mpdec builds, add install-emscripten cmd#145664
gh-145219: Cache Emscripten libffi and mpdec builds, add install-emscripten cmd#145664freakboy3742 merged 4 commits intopython:mainfrom
Conversation
…l-emscripten cmd
This moves the emsdk install from `{emsdk_cache}/{emscripten_version}` to
`{emsdk_cache}/{emscripten_version}/emsdk` so that we can put the prefix at
`emsdk_cache_dir/{emscripten_version}/prefix.
I moved the data about mpdec & libffi version, url, and shasum into config.toml along
with the emscripten version. Then as a cache key I write the library config section to
disk in the prefix dir like `{libname}.json`. I write it as a json file since tomllib
can't write toml. On subsequent builds, if prefix/{libname}.json contains the same data
then we can skip a rebuild.
I also added an install-emscripten command to make it easy to get this directory
structure correct.
|
!buildbot emscripten |
|
🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit ccad6f4 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145664%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.
This all looks good; I've added one minor bugfix (another path .absolute() call), plus added a wrapper around "configure + make" targets that I've found helpful in the iOS case.
I'm running buildbot CI now; assuming that passes, I'll merge in the morning - but that also gives you a chance to check that I haven't missed anything in the modifications I've made.
Tools/wasm/emscripten/__main__.py
Outdated
| @functools.cache | ||
| def emsdk_cache_root(emsdk_cache): | ||
| required_version = required_emscripten_version() | ||
| return Path(emsdk_cache) / required_version |
There was a problem hiding this comment.
| return Path(emsdk_cache) / required_version | |
| return Path(emsdk_cache).absolute() / required_version |
| "host_build_dir": host_triple_dir / "build", | ||
| "host_dir": host_triple_dir / "build" / "python", |
There was a problem hiding this comment.
This is the only part that I'd like to clean up - dropping the build part of the path. With this is place, we end up with cross-build/wasm32-emscripten/build/python and cross-build/wasm32-emscripten/build/mpdec etc - but nothing else at the same level as build.
However, we can't remove that level without also changing the buildbot config, so we might want to include that as part of the "move to Platforms" change.
There was a problem hiding this comment.
Does the buildbot config hard code build as part of the path somewhere? I suppose when it goes to run tests?
There was a problem hiding this comment.
Yes - it's hardcoded in the buildbot config.
One option to consider would be to add a python3 Tools/wasm/emscripten test target (and/or python3 Tools/wasm/emscripten run target). That would break the dependency on the internals of the build location at least.
|
Thanks @freakboy3742! |
Tools/wasm/emscripten/__main__.py
Outdated
| make_emscripten_libffi, | ||
| make_mpdec, |
There was a problem hiding this comment.
Surely these should be in host?
There was a problem hiding this comment.
So - they're things that need to be compiled once, and then never built again. I put them in host on the basis that they need to be done once, and then never again - which to me makes them more like build than host.
However, the fact that the target has a shortcut if they're already there means there's no real overhead to putting them in build.
|
!buildbot emscripten |
|
🤖 New build scheduled with the buildbot fleet by @hoodmane for commit 6b90ac2 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145664%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.
👍 Thanks for those updates!
|
Thanks @hoodmane for the PR, and @freakboy3742 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…cripten (pythonGH-145664) Modifies the Emscripten build script to allow for caching of dependencies, and for automated installation of new EMSDK versions. (cherry picked from commit ebb150e76ab4988fdcd5e8caa36b9014497573a5) Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com> Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
|
GH-145790 is a backport of this pull request to the 3.14 branch. |
…scripten (GH-145664) (#145790) Modifies the Emscripten build script to allow for caching of dependencies, and for automated installation of new EMSDK versions. (cherry picked from commit ebb150e) Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com> Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
|
3 participants
This moves the emsdk install from
{emsdk_cache}/{emscripten_version}to{emsdk_cache}/{emscripten_version}/emsdkso that we can put the prefix at `emsdk_cache_dir/{emscripten_version}/prefix.I moved the data about mpdec & libffi version, url, and shasum into config.toml along with the emscripten version. Then as a cache key I write the library config section to disk in the prefix dir like
{libname}.json. I write it as a json file since tomllib can't write toml. On subsequent builds, if prefix/{libname}.json contains the same data then we can skip a rebuild.I also added an install-emscripten command to make it easy to get this directory structure correct.