Skip to content
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

Issues with emscripten support #223

Open
MoAlyousef opened this issue Oct 3, 2024 · 1 comment
Open

Issues with emscripten support #223

MoAlyousef opened this issue Oct 3, 2024 · 1 comment

Comments

@MoAlyousef
Copy link
Contributor

MoAlyousef commented Oct 3, 2024

Hello

I've noticed some issues when using cmake-rs to build a project for the wasm32-unknown-emscripten target:
1- Build failure when the host is windows:

thread 'main' panicked at C:\Users\runneradmin/.cargo\registry\src\index.crates.io-6f17d22bba15001f\cmake-0.1.51\src/lib.rs:1100:5:

  failed to execute command: program not found
  is `cmake` not installed?

cmake-rs looks for emcmake as the CMake program, on posix it finds it, however on windows it's called emcmake.bat, so it doesn't find it:
https://github.com/MoAlyousef/cmake-rs-emscripten-bug/actions/runs/11159453280/job/31017869199

The repo also shows a minimal repro: https://github.com/MoAlyousef/cmake-rs-emscripten-bug

2- Reconfiguring issues. This I noticed on Linux. When the first cmake configure pass runs it effectively finds emcmake and if the configure succeeds, all is well. If the configure doesn't succeed due to any issues, reconfiguring fails to use emcmake with the following message:

You have changed variables that require your cache to be deleted.
  Configure will be re-run and you may have to reset some variables.
  The following variables have changed:
  CMAKE_CXX_COMPILER= /usr/bin/g++

The CMAKE_INSTALL_PREFIX variable is also reset, so the build either fails due to not finding the emscripten headers or due to failure to install to a system path (permission denied).

For 1, it might just be sufficient to add the .bat extension on windows.

But I propose that building for emscripten uses cmake proper and either searches for the Emscripten.cmake toolchain file which is installed by default with emscripten, or requires explicit passing of the toolchain file in build.rs (where the emscripten toolchain file has a known location relative to the EMSDK env variable).

For example, replacing cmake::Config::new("src/mylib").build(); with:

    let emsdk = std::path::PathBuf::from(std::env::var("EMSDK").unwrap());
    std::process::Command::new("cmake").current_dir("src/mylib").args([
        &format!("-B{out_dir}/bin"),
        &format!("-DCMAKE_TOOLCHAIN_FILE={}", emsdk.join("upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake").display()),
        "-DCMAKE_BUILD_TYPE=Release",
        &format!("-DCMAKE_INSTALL_PREFIX={out_dir}"),
    ]).status().ok();
    std::process::Command::new("cmake")
        .current_dir("src/mylib")
        .args(["--build", &format!("{out_dir}/bin"), "--target", "install"])
        .status()
        .ok();

causes the build to succeed on both systems.

@tgross35
Copy link
Contributor

tgross35 commented Oct 3, 2024

The proposal sounds pretty reasonable to me. If you are willing to put up a PR, we ask some of the target maintainers to take a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants