emscripten-core / emsdk

Emscripten SDK
http://emscripten.org
Other
2.92k stars 660 forks source link

[Bazel] Main bootstrap script for Wasm Audio Worklets not generated #1399

Closed WilliamIzzo83 closed 3 weeks ago

WilliamIzzo83 commented 4 weeks ago

Trying to build the basic noise generator example with Bazel results in the audio worklet js file not generated.

Suppose we have a target defined like so

load("@emsdk//emscripten_toolchain:wasm_rules.bzl", "wasm_cc_binary")

cc_binary(
    name = "audio_worklet",
    srcs = ["audio-worklet.c"],
    copts = [
        "-sWASM_WORKERS=1",
    ],
    linkopts = [
        "-sAUDIO_WORKLET",
        "-sWASM_WORKERS",
        "--oformat=html",
    ],
)

wasm_cc_binary(
    name = "audio_worklet_wasm",
    cc_target = "audio_worklet",
    outputs = [
        "index.html",
        "audio_worklet.js",
        "audio_worklet.wasm",
        "audio_worklet.aw.js"
    ],
)

Results in the following build error

Starting local Bazel server and connecting to it...
INFO: Analyzed target //:audio_worklet_wasm (80 packages loaded, 7258 targets configured).
INFO: From Action index.html:
[ERROR] Archive does not contain file with extname: .aw.js
ERROR: <redacted>/bazel_audio_worklet/BUILD:16:15: output 'audio_worklet.aw.js' was not created
ERROR: <redacted>/bazel_audio_worklet/BUILD:16:15: Action index.html failed: not all outputs were created or valid
Target //:audio_worklet_wasm failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 23.288s, Critical Path: 3.69s
INFO: 10 processes: 7 internal, 3 darwin-sandbox.

Here's a minimal example to reproduce the issue: bazel_audio_worklet_ko.zip

The problem seems to be that in the allowed extension files list in link_wrapper.py here, the .aw.js extension isn't listed. By adding the extension into the list the file is properly generated and the build ends successfully as shown here: bazel_audio_worklet_ok.zip

Since I plan to use Emscripten, Audio Worklets, etc. within a Bazel workspace I'm willing to propose a patch myself of course, but I need some guidance because there's no CONTRIBUTING file in the repo.

Thank you

kripken commented 3 weeks ago

A PR would definitely be welcome!

What information do you need, that you expected to see in a CONTRIBUTING file? (we can add that too if it would be generally useful)

WilliamIzzo83 commented 3 weeks ago

What information do you need, that you expected to see in a CONTRIBUTING file? (we can add that too if it would be generally useful)

Just wanted to know if there's anything a newcomer needs to know about the whole process or if there are any requirements to submit a PR (i.e. some repos requires a contributor licence agreement).

Anyways I guess I'll find out as we go 😄

About the issue at hand before attempting a fix I'd like to know more about the rationale of the implementation of this part of the linker_wrapper.py: why is there a set of valid file extensions in the first place? IIUC the outdir should contain the files generated by the compiler, so it seems to me that the implementation could be simplified by putting all files contained in the outdir in the list of files to be added to the files array. Am I missing something here?

Of course the easy way to fix the issue is to just add the .aw.js extension to the set mention before, but on the other hand I think simplifying the logic of the wrapper, if possible, can be more valuable in the long run. Maybe the original author of the rule, @walkingeyerobot, can give me more context.

Thanks!

walkingeyerobot commented 3 weeks ago

Of course PRs are welcome :)

The toolchain is much older than github history would lead you to believe. Originally, we wanted to build existing cc_binary targets under emscripten, so wasm_cc_binary was written as a wrapper rule that simply transitioned the cc toolchain. This means that the cc_binary is still the thing that does the actual building. cc_binary only has exactly one output, whereas emscripten outputs many files. To work around this, we put all emscripten's output files into a tarball, and then after everything is built wasm_cc_binary extracts this tarball and exposes all possible output files. Because bazel isn't capable of optional outputs (or at least wasn't at the time wasm_cc_binary was written), we needed to make sure all the files were created even if they were empty.

I suspect (but I'm not sure) that simplifying or eliminating this tarball process is possible but a huge amount of work. I would recommend just adding the .aw.js output file to the relevant scripts / rules.