-
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
[Bazel] Use py_binary for emcc #1422
base: main
Are you sure you want to change the base?
Conversation
Referring to the CI results, on Linux and Mac this currently fails as described in the SO question and as follows:
Interestingly enough, it gets further on Windows and actually starts emcc.py but then fails since I haven't added the required
|
|
Well, I've dug pretty deep into Bazel over the past weeks and while I could be mistaking, I am under the impression that what we are trying to do with |
Fair enough. Sounds like you know way more than me at this point about this stuff :) I though that |
You are not wrong, it is for both. Ironically it seems, however, that people actually run into more problems using |
This would also resolve #1263 |
I think this is a more complex problem to solve than it appears. I don't think it's worth it to spend time on this, but if you can get it working I'd be inclined to accept it. |
This PR was an investigation into clean up the Emscripten toolchain in Bazel using
py_binary
to wrap and runemcc
andemar
etc. This would have been nice since I think it would have allowed us to drop all the sh/bat scripts (and OS detection logic around it) that just call the Python interpreter anyway. The motivation for this is #1420 and I was hoping that by wrappingemcc
withpy_binary
that the paths/modules/libraries would have been a bit more robust to whatever is going wrong in this issue.Unfortunately I hit the roadblock described in this SO question that cannot be worked around without a hack. The main blocker is bazelbuild/bazel#17629, however, if there is interest in this approach, we could conditionally apply a genrule on Linux and Mac to add in the missing shebang as suggested in bazelbuild/bazel#17629 (comment).
What do you think @sbc100 and @walkingeyerobot? Do you think this is worth pursuing further? Incidentally, this PR would make #1333 redundant in my opinion.