Skip to content

Commit

Permalink
Update emsdk version to 3.1.67 in Bazel configuration
Browse files Browse the repository at this point in the history
This CL mostly follows the instructions in
https://github.com/emscripten-core/emsdk/blob/main/bazel/README.md.
Even so, there are a few things that are worth mentioning:

1. I tested the changes locally and in my tests
   incompatible_enable_cc_toolchain_resolution bazel flag made no
   difference (e.g. everything builds with or without the flag);
   I still keep it though because instructions say that it should
   be there and it does not cause any harm.
2. I don't think that we still need to patch emsdk to exclude npm
   modules from the toolchain, because it appears that upstream
   already removed those as a toolchain dependency in
   emscripten-core/emsdk#1045.

It's worth noting, that even though I don't think that emsdk patch
is still needed, I actually wasn't able to reproduce the problem
reported in proxy-wasm#149
locally without the emsdk.patch even with the current version of
emsdk used by C++ SDK.

Signed-off-by: Mikhail Krinkin <krinkin.m.u@gmail.com>
  • Loading branch information
krinkinmu committed Oct 7, 2024
1 parent 2ffee66 commit 90d8c07
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 54 deletions.
9 changes: 9 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# NOTE: incompatible_enable_cc_toolchain_resolution is set by default on Bazel
# versions 7.0+, see https://github.com/bazelbuild/bazel/issues/7260.
#
# emsdk documentation asks to set this flag (see
# https://github.com/emscripten-core/emsdk/blob/main/bazel/README.md). And even
# though things seem to work even without this flag, given that this flag
# enables improved Bazel C++ toolchain resolution method that became the
# default in newer Bazel version and does not cause problems, we keep it.
build --incompatible_enable_cc_toolchain_resolution
2 changes: 2 additions & 0 deletions bazel/dependencies_extra.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
# limitations under the License.

load("@emsdk//:emscripten_deps.bzl", "emscripten_deps")
load("@emsdk//:toolchains.bzl", "register_emscripten_toolchains")

# Requires proxy_wasm_cpp_sdk_dependencies() to be loaded first.
def proxy_wasm_cpp_sdk_dependencies_extra():
emscripten_deps()
register_emscripten_toolchains()
48 changes: 0 additions & 48 deletions bazel/emsdk.patch

This file was deleted.

9 changes: 3 additions & 6 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,9 @@ def proxy_wasm_cpp_sdk_repositories():
maybe(
http_archive,
name = "emsdk",
sha256 = "1ca0ff918d476c55707bb99bc0452be28ac5fb8f22a9260a8aae8a38d1bc0e27",
# v3.1.7 with Bazel fixes
strip_prefix = "emsdk-0ea8f8a8707070e9a7c83fbb4a3065683bcf1799/bazel",
url = "https://github.com/emscripten-core/emsdk/archive/0ea8f8a8707070e9a7c83fbb4a3065683bcf1799.tar.gz",
patches = ["@proxy_wasm_cpp_sdk//bazel:emsdk.patch"],
patch_args = ["-p2"],
sha256 = "0cb0eabd6e3ceb1a970a2363e67f2b1689c2d83fbeae1e75901213c1f84de2e2",
strip_prefix = "emsdk-3.1.67/bazel",
url = "https://github.com/emscripten-core/emsdk/archive/refs/tags/3.1.67.tar.gz",
)

maybe(
Expand Down

0 comments on commit 90d8c07

Please sign in to comment.