Conversation
|
Would you mind prefixing the PR title with |
| source $(dirname $0)/env.sh | ||
|
|
||
| exec python3 $EMSCRIPTEN/emar.py "$@" | ||
| exec $EMSDK_PYTHON $EMSCRIPTEN/emar.py "$@" |
There was a problem hiding this comment.
I wonder if could now maybe remove these custom launcher scripts and just use the standard emcc launcher shell script (as a followup, no in this PR)
I guess it depends what else in env.sh is needed?
There was a problem hiding this comment.
I actually tried that in one of my unsuccessful iterations. I replaced all those scripts with pure Python, presented them to Bazel via py_binary, and then gave those binaries to the cc_toolchain.
The first problem was that binaries were resolved in the "target context (WASM)" instead of the "exec" context, which then tried to run the "WASM version of Python", which doesn't exist. I circumvented that by creating a transitioned binary in exec context, and that almost worked, except final compilation attempts failed because execv("/path/to/python/wrapper") failed. I didn't investigate deeply enough to find out what was wrong there, and instead backtracked to pursuing adding Python interpreter runfiles to the toolchain context, so they end up in the sandbox.
My first working solution required a patch to rules_python, which I created a PR for, but the rules_python author then told me this wasn't how it is supposed to be used and instructed me to research the Python exec toolchain. I did that, and this solution now works, so I created this PR.
In the future, I'll probably revisit this approach that uses pure Python without any shell wrappers, but I still need to get better at Bazel before that (I'm still learning it).
sbc100
left a comment
There was a problem hiding this comment.
Not sure I understand the bzl stuff .. but if the tests pass then lgtm.
|
Can you add bazel 9 (macOs) testing in CircleCI with that change ? |
Added. The windows failure on Bazel 8 is some transient network issue. I don't have permissions to rerun it. |
Thanks to @rickeylev for the help with figuring out the correct way to ensure the Python interpreter with all its dependencies is correctly added to the toolchain files.
Note: this does not touch Windows support, which may also have a similar issue. I see windows bat files using
py -3, which may also not be hermetic, but I have no access to any Windows machines to test and fix that, so I'll leave that untouched for now.Fixes: #1675