-
Notifications
You must be signed in to change notification settings - Fork 694
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
Add linux/arm64 support to bazel rules #1156
Add linux/arm64 support to bazel rules #1156
Conversation
@@ -9,7 +9,7 @@ FROZEN_CACHE = True | |||
|
|||
system = platform.system() | |||
|
|||
machine = "arm64" if platform.machine() == "arm64" else "amd64" | |||
machine = "arm64" if (platform.machine() == "arm64" or platform.machine() == "aarch64") else "amd64" |
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.
It seems like mac/linux python report this differently:
$ python3 # on a arm mac
Python 3.10.8 (main, Oct 13 2022, 09:48:40) [Clang 14.0.0 (clang-1400.0.29.102)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import platform; platform.machine()
'arm64'
$ docker run -it --platform=linux/arm64 python
Python 3.11.1 (main, Dec 21 2022, 09:36:17) [GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import platform; platform.machine()
'aarch64'
Without this, during linking I'd be hit with a:
emcc: warning: cannot check node version: [Errno 2] No such file or directory: '/root/.cache/bazel/.../external/nodejs_linux_amd64/bin/node' [-Wversion-check]
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 see now this may not be necessary after #1151
@@ -54,6 +54,7 @@ EMSCRIPTEN_TAGS = { | |||
"3.1.21": struct( | |||
hash = "a16a8bca2466eb144f7c93fa899c0272c8815dc3", | |||
sha_linux = "7045ddb3b37a2cc63cb1cf976019a6a3b7f8dbdc71254db0eee5b0452f94e9e7", | |||
sha_linux_arm64 = "2852c8b108ec748d52d31dab3f4854bc6022df008019daff1c7e31ac00363b3f", |
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.
3.1.21
is the only linux arm64 binaries I could find the sha for. If there's a way to find/list shas for more I'd be happy to add them.
For this one:
emsdk/emscripten-releases-tags.json
Line 27 in 5b80c10
"3.1.21": "a16a8bca2466eb144f7c93fa899c0272c8815dc3", |
$ curl -sSL https://storage.googleapis.com/webassembly/emscripten-releases-builds/linux/a16a8bca2466eb144f7c93fa899c0272c8815dc3/wasm-binaries-arm64.tbz2 | shasum -a 256
2852c8b108ec748d52d31dab3f4854bc6022df008019daff1c7e31ac00363b3f
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.
Seeing the ci failures are
ERROR: Traceback (most recent call last):
File "/root/project/bazel/WORKSPACE", line 9, column 16, in <toplevel>
emscripten_deps()
File "/root/project/bazel/emscripten_deps.bzl", line 131, column 30, in emscripten_deps
sha256 = revision.sha_linux_arm64,
Error: 'struct' value has no field or method 'sha_linux_arm64'
Available attributes: hash, sha_linux, sha_mac, sha_mac_arm64, sha_win
this PR may need more sha_linux_arm64
, but these don't always exist (maybe that's also the reason this wasn't in the bazel rules :)). I think there is a starlark attr check and we can skip defining the dep if there's no sha, that may be a way forward.
@@ -9,7 +9,7 @@ FROZEN_CACHE = True | |||
|
|||
system = platform.system() | |||
|
|||
machine = "arm64" if platform.machine() == "arm64" else "amd64" | |||
machine = "arm64" if (platform.machine() == "arm64" or platform.machine() == "aarch64") else "amd64" |
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 see now this may not be necessary after #1151
Not all releases have linux shas. Use getattr to default to no sha if none was present. This way non linux arm64 builds should not fail.
I think the remaining build failure is the same as #1153 |
Small friendly nudge here, anything I can do to help this get merged? |
lgtm, but I'd rather wait for someone how knows more about the bazel rules.. I think @walkingeyerobot is OOO for a while, but maybe you can find someone else who as contributed a few times to this file to review? |
Sounds good, happy to wait till @walkingeyerobot is available again (but would also like to get rid of my patch/fork :)) @mattsoulanille @zaucy you both respectively added macos/arm support and windows support to the bazel rules before in these files. Any chance either of you is available to weigh in on the bazel specifics here to for linux/arm64 support? |
This looks great, thanks! Apologies for being offline for so long, and thank you for your patience. We just new a few hashes updated for the more recent emscripten versions and this will be good to merge. |
This is the latest arm64 release according to emscripten-core#1204. There do not seem to be a any releases between .21 and .33.
@@ -12,6 +12,7 @@ EMSCRIPTEN_TAGS = { | |||
"3.1.33": struct( | |||
hash = "49b960bd03b3a9da478a08541ce6eafe792a58a8", | |||
sha_linux = "eab02b3f4b7c076974452ba602f908a36adf597afa15b16095b441f191ede1bb", | |||
sha_linux_arm64 = "5e15af6affcf37c9ce6c304b4aeccb87a2758e1ef029dbc996f9d77d7444378e", |
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.
emsdk/emscripten-releases-tags.json
Line 15 in a2ccccb
"3.1.33": "49b960bd03b3a9da478a08541ce6eafe792a58a8", |
$ curl -sSL https://storage.googleapis.com/webassembly/emscripten-releases-builds/linux/49b960bd03b3a9da478a08541ce6eafe792a58a8/wasm-binaries-arm64.tbz2 | shasum -a 256
5e15af6affcf37c9ce6c304b4aeccb87a2758e1ef029dbc996f9d77d7444378e -
This is the only newer release which seems to have linux arm64 binaries released.
* Add linux/arm64 support to bazel rules * Make linux arm64 sha optional and PR feedback Not all releases have linux shas. Use getattr to default to no sha if none was present. This way non linux arm64 builds should not fail. * Update linux arm64 sha for 3.1.33 This is the latest arm64 release according to emscripten-core#1204. There do not seem to be a any releases between .21 and .33.
I could find that there are some prebuilt Linux/arm64 binaries out there, but that their version trails a bit behind and not supported in the bazel rules. This pr aims to change the latter bit.
My usecase is a bazel+emsdk build in a Docker build (unfortunatly can't just build the container directly from bazel). While this works fine in our CI system, locally I'd have to build with docker's
--platform=linux/amd64
which is unbearably slow with bazel in it. Since there are arm64 binaries for linux, I'd like to be able to use those!