[bazel] Fix and update emscripten_toolchain #1060
[bazel] Fix and update emscripten_toolchain #1060jrandolf wants to merge 7 commits intoemscripten-core:mainfrom mathematic-inc:main
Conversation
This allows consumption of this script for non-Bazel usage. For example, for `rules_foreign_cc`.
|
when making changes to the bazel toolchain would you mind prefixing the PR title with |
jrandolf
changed the title
* Removes `asm` CPU target. * [feat] Add `wasm32` and `wasm64` CPU targets. * Fix static library linker flags. * Use the new `platform` API for toolchains * Add a `emsdk_register_toolchains` for Platform toolchains.
This reverts commit 03cec6f.
walkingeyerobot
left a comment
walkingeyerobot
left a comment
There was a problem hiding this comment.
Thanks very much for the PR! I'm excited to get this in. Apologies for the slowness in getting to this review; it's a busy holiday season right now where I am.
| "//command_line_option:linkopt": linkopts, | ||
| "//command_line_option:platforms": [], | ||
| "//command_line_option:incompatible_enable_cc_toolchain_resolution": True, | ||
| "//command_line_option:platforms": ["@emsdk//:cpu_wasm32"], |
There was a problem hiding this comment.
does this need to be set to cpu_wasm64 if we're targeting wasm64?
| f.write(param) | ||
| f.write('\n') | ||
| sys.argv[1] = '@' + new_param_filename | ||
| else: |
There was a problem hiding this comment.
are there cases where we wouldn't only get the single @ file?
There was a problem hiding this comment.
This occurs in, e.g., foreign_cc builds.
|
|
||
| See `test_external/` for an example using [embind](https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html). | ||
|
|
||
| ### Using --config=wasm |
There was a problem hiding this comment.
does --config=wasm no longer work?
There was a problem hiding this comment.
In an ideal world, --config=wasm32 and --config=wasm64 are both allowed, and --config=wasm just points to --config=wasm32 for backwards compatibility.
There was a problem hiding this comment.
I'm not sure what you mean by "backwards-compatibility". If we update the config here, it doesn't imply it propagates to the user. In particular, this only affects developers of this repo, not external users.
|
I'll reopen this with my internal account. |
3 participants
This PR makes several changes. See the commit descriptions.