[bazel] Fix emsdk python path issue on Windows.

[kalmard0]

Tested with bazel 8.6.0 with git bash as the main shell and build --action_env=EM_FROZEN_CACHE=0.

Without this change, my build fails with the error:

The system cannot find the path specified.
emcc: error: subprocess 1/1 failed (returned 1)! (cmdline: 'C:\bazel\build_root\pn2yxrdx\execroot\_main\external\emsdk++emscripten_deps+emscripten_bin_win\emscripten\em++.bat' -c -O2 -Wall -fno-unroll-loops -g -sSTRICT -Werror -sMEMORY64=1 '-ffile-prefix-map=C:\bazel\build_root\pn2yxrdx\execroot\_main\external\emsdk++emscripten_deps+emscripten_bin_win\emscripten=/emsdk/emscripten' '-ffile-prefix-map=..\..\..=/emsdk/emscripten' -fdebug-compilation-dir=/emsdk/emscripten -std=c++20 ../../../system/lib/embind/bind.cpp)

[sbc100]

I wonder if we could find a way to instead make EMSDK_PYTHON always absolute? @DoDoENT ?

[DoDoENT]

I wonder if we could find a way to instead make EMSDK_PYTHON always absolute? @DoDoENT ?

The EMSDK_PYTHON is defined here which is literally obtained from rules_python here.

As far as I understand, Bazel provides either path or short_path. The former is as absolute as possible, without breaking hermeticiy, and is usually used when you need a path that includes configuration (e.g., during cross-compiling to WASM, you need a path to the Python interpreter for the host system), and the latter is just relative from the repository root to be used in various custom Bazel rules.

I think that the proposed change in this PR is the best possible solution and consistent with unix version (the EXT_BUILD_ROOT is never set, so bash evaluates pwd -P which results in the current absolute path).

For more details about the reasons why an absolute path can't be resolved within the Starlark, you can see the explanation from the absolute expert on Bazel from whom I learned a lot - Google's Gemini 😅

[sbc100]

The reason I'm not totally sure about this patch is that its in the .bat file, which I think we hope to delete at some point.

Also, why is it needed in the .bat file but not on the equivalent .sh file?

I will defer to others on all things bazel though so LGTM aside from the above questions/concerns.

[DoDoENT]

Also, why is it needed in the .bat file but not on the equivalent .sh file?

I see...

The culprit is --action_env=EM_FROZEN_CACHE=0, which is fundamentally not compatible with how Bazel works. Namely, Bazel expects everything to be hermetic, so lazy cache building is forbidden. This is why Bazel's emscripten toolchain sets EM_FROZEN_CACHE to prevent lazy cache building within Bazel's cache, which should be read-only.

I've never tried building on Mac/Linux with a disabled frozen cache - maybe the Linux/Mac will encounter the same problem. Bazel goes to great lengths to prevent escaping the sandbox and attempting to modify read-only folders.

If you need to use a non-default cache, there is already a mechanism for that using the cache extension. See the README for details. However, even this extension has problems with hermeticity - even though it's technically hermetic, the built cache is different on every machine, so remote caching and remote build do not work. I fixed that in my PR, where you can use embuilder to build the non-default cache on one machine, deploy the cache on some server, and then use the new prebuilt_cache tag class (implemented in the PR) to obtain that cache on all machines that participate in the build, be it local or remote.

@kalmard0, what problem are you trying to solve? Why are you using --action_env=EM_FROZEN_CACHE=0 in your build?

[DoDoENT]

The library generation should always be disabled in Bazel, as it is not hermetic and won't work with remote builds.

[DoDoENT]

I wonder if we could find a way to instead make EMSDK_PYTHON always absolute? @DoDoENT ?

The EMSDK_PYTHON is defined here which is literally obtained from rules_python here.

As far as I understand, Bazel provides either path or short_path. The former is as absolute as possible, without breaking hermeticiy, and is usually used when you need a path that includes configuration (e.g., during cross-compiling to WASM, you need a path to the Python interpreter for the host system), and the latter is just relative from the repository root to be used in various custom Bazel rules.

I think that the proposed change in this PR is the best possible solution and consistent with unix version (the EXT_BUILD_ROOT is never set, so bash evaluates pwd -P which results in the current absolute path).

For more details about the reasons why an absolute path can't be resolved within the Starlark, you can see the explanation from the absolute expert on Bazel from whom I learned a lot - Google's Gemini 😅

I should avoid doing reviews at late hours 😅 - I saw the path being "absolutized" for a totally different variable than it was written in code 😅

[sbc100]

the built cache is different on every machine, so remote caching and remote build do not work.

Do you know the reason for this? I would hope our libraries were hermetic.. perhaps we can fix that in emscripten if they not?

[DoDoENT]

Do you know the reason for this? I would hope our libraries were hermetic.. perhaps we can fix that in emscripten if they not?

I don't know. But you can run embuilder with the same flags twice, even on the same machine, the SHA256 of the resulting binaries will be different... This is enough to invalidate the Bazel cache.

And even if you somehow manage to make embuilder be fully hermetic (each invocation with the same flags every time on any machine produces the same binaries with the same SHA256), I still think there is a benefit to having the ability to simply download a prebuilt cache in Bazel emscripten rules than wait for it to rebuild it again.

[kalmard0]

build --action_env=EM_FROZEN_CACHE=0 was added to enable 64 bit emscripten builds. Unfortunately I don't remember the exact details now.

[DoDoENT]

build --action_env=EM_FROZEN_CACHE=0 was added to enable 64 bit emscripten builds. Unfortunately I don't remember the exact details now.

Yeah, that's what I was afraid of and what explains your use case. The default cache that ships with emscripten does not support 64-bit memory or LTO, so you need to build your own.

A default distribution of emscripten does that lazily, on demand. However, this doesn't work in Bazel as the cache builder tool does not produce identical binaries (the SHA256 hash differs even if you always use the same build flags), so different Bazel workers observe different inputs to their actions, and this results in too many unnecessary rebuilds of code at best, or a broken binary at worst.

Instead of "unfreezing" the cache, you should do this the way which is compatible with Bazel, as explained in the README.

Basically, you need to create your own cache with 64-bit memory enabled and then use it:

emscripten_cache = use_extension(
    "@emsdk//:emscripten_cache.bzl",
    "emscripten_cache",
)
emscripten_cache.configuration(flags = ["--wasm64"])
emscripten_cache.targets(targets = [
    "crtbegin",
    "libprintf_long_double-debug",
    "libstubs-debug",
    "libnoexit",
    "libc-debug",
    "libdlmalloc",
    "libcompiler_rt",
    "libc++-noexcept",
    "libc++abi-debug-noexcept",
    "libsockets"
])

This will then build additional cache compatible with 64-bit, which you can use without needing to unset EM_FROZEN_CACHE.

Note: You may need to tweak the list of targets to rebuild based on your project's needs. This is the list I needed for my projects. You can see what's missing by observing the build error from emscripten telling you that specific target is missing and cannot be lazily rebuilt because EM_FROZEN_CACHE is set (I guess you tried "fixing" this error before by setting this variable, but this then just caused other problems that ended up with this PR).

[kalmard0]

@DoDoENT Thank you, I totally missed that. In that case I don't consider this a valid pull request for my purposes. Feel free to close it.