rust-lang / cc-rs

Rust library for build scripts to compile C/C++ code into a Rust library
https://docs.rs/cc
Apache License 2.0
1.82k stars 440 forks source link

v1.0.102 breaks wasm builds with trunk #1113

Closed Kuuuube closed 3 months ago

Kuuuube commented 3 months ago

I depend on cc through zstd-safe (zstd-safe -> zstd-sys -> cc). v1.0.102 causes wasm builds through trunk (ex: trunk serve) to display the following error when accessing the wasm webpage.

Uncaught TypeError: The specifier “env” was a bare specifier, but was not remapped to anything. Relative module specifiers must start with “./”, “../” or “/”.

I've tested manually reverting cc through Cargo.lock and a clean build. In my testing, cc v1.0.101 does not have this issue.

Possibly related to #1105 and building with wasm32-unknown-unknown target.

Repo and build steps for reference (I've committed the cargo lock with v1.0.101 to mitigate this issue currently): https://github.com/Kuuuube/hentaigana_input https://github.com/Kuuuube/hentaigana_input/blob/main/.github/workflows/pages.yml

NobodyXu commented 3 months ago

Thank you for reporting!

cc @antaalt It seems that for wasm-unknown-unknown and wasmp1, mentioned in #1109 , we don't have to explicitly pass the wasi root and the compiler (clang) can infer it

NobodyXu commented 3 months ago

Both use ubuntu, so I guess clang on Linux can find the wasi root itself?

Or maybe clang on Linux already has the wasi libraries required.

Or perhaps it's just that Ubuntu-latest runner already has it installed and configured.

antaalt commented 3 months ago

Thank you for reporting!

cc @antaalt It seems that for wasm-unknown-unknown and wasmp1, mentioned in #1109 , we don't have to explicitly pass the wasi root and the compiler (clang) can infer it

The issue here is probably that wasm32-u known-unknown is web assembly and not WASI, so we don't need WASI sysroot at all I think, not sure how to compile webassembly correctly though, it does not support exception either so these changes should be working I think

NobodyXu commented 3 months ago

The issue here is probably that wasm32-u known-unknown is web assembly and not WASI, so we don't need WASI sysroot at all I think

Yeah but it also happened for wasip1, so I think we should make the WASI_SYSROOT optional.

antaalt commented 3 months ago

Yes, that's probably best

dhedey commented 3 months ago

This might be totally unrelated, and I don't know anything about the motivation for these changes, but in #1105 the tool match statement changed the match logic for wasm32-unknown-unknown. Was this intentional @antaalt ?

Specifically:

                } else if target == "wasm32-wasi"
                    || target == "wasm32-unknown-wasi"
                    || target == "wasm32-unknown-unknown"
                {

was replaced by Build::is_wasi_target(target) which doesn't pass for wasm32-unknown-unknown anymore

antaalt commented 3 months ago

This was an issue on my side, but it is now fixed with #1115

NobodyXu commented 3 months ago

cc @Kuuuube Can you try 1.0.103 to see if it fixes your issue?

Kuuuube commented 3 months ago

It works! Thanks for getting to this so fast.