Don't panic if WASI_SDK_PATH not set when detecting compiler

[pmur]

The fedora packaging builds the wasm sysroot outside of the rust build system. Fedora applies a couple of patches related to wasm which I think make this possible. Not panicking seems consistent with the detection logic of other targets when they cannot find cc.

r? @Kobzol

[rustbot]

Kobzol is not on the review rotation at the moment.
They may take a while to respond.

[Kobzol]

Hi, how does it help you if you return None from this function? Won't that just break the wasi build, if cc isn't properly configured?

In any case, if it does help you, I'd appreciate printing a warning containing the WASI_SDK_PATH environment variable name in it, if the path cannot be found. It is quite useful if you try to build a wasi target, as it tells you what you should configure in order to get it working :)

[cuviper]

Won't that just break the wasi build, if cc isn't properly configured?

If not specified, it falls back to the cc crate's default, which is clang for wasm targets -- and that works fine for us.

Also, Fedora uses build.optimized-compiler-builtins = false (pending #143689), and I don't think it needs any cc otherwise.

[Kobzol]

I see. Then use the fallback to the default cc, but if the wask path isn't found, also print a warning, like println!("WARNING: WASI_SDK_PATH environment variable not found, falling back to default cc");. I wonder if we should maybe also keep the panic on CI to make sure we don't fall back by accident 🤔

[pmur]

Would it be ok to restrict panicking to a missing WASI_SDK_PATH envvar and an unset <target>.wasi-root build config?

[Kobzol]

So if I understand it correctly, you are using wasi_root instead of WASI_SDK_PATH? Or you're using neither and you just always depend on the fallback?

There is already a function in bootstrap (

rust/src/bootstrap/src/lib.rs

Line 1359 in 2fbc642

fn wasi_libdir(&self, target: TargetSelection) -> Option<PathBuf> {

) that loads these two things, so it seems that the cc detection logic should also first be accessing the wasi root and only then the WASI_SDK_PATH - assuming that the wasi-root is actually useful for the cc detection (maybe that's not the case).

[Kobzol]

I even see that we already assert this (

rust/src/bootstrap/src/core/build_steps/compile.rs

Line 423 in 205587b

let srcdir = builder.wasi_libdir(target).unwrap_or_else(|| {

).

[pmur]

So if I understand it correctly, you are using wasi_root instead of WASI_SDK_PATH? Or you're using neither and you just always depend on the fallback?

Yes, wasi-root is always set when building a wasm std on fedora.

I don't think wasi-root has meaning for cc detection beyond indicating the default cc/cxx settings might suffice in the absence of WASI_SDK_PATH.

[Kobzol]

Thanks.

@bors r+ rollup

[bors]

📌 Commit c3980ab has been approved by Kobzol

It is now in the queue for this repository.

[Kobzol]

@bors r-

Actually, the panic when on CI hasn't been implemented yet. We should make sure to keep the original behavior there.

[Kobzol]

Thanks!

@bors r+

[bors]

📌 Commit 9bdd3b0 has been approved by Kobzol

It is now in the queue for this repository.