corrosion-rs / corrosion

Marrying Rust and CMake - Easy Rust and C/C++ Integration!
https://corrosion-rs.github.io/corrosion/
MIT License
1.02k stars 97 forks source link

[Bug]: "multiple rules generate" with Ninja Multiconfig in CROSS_CONFIG mode #491

Open xTachyon opened 4 months ago

xTachyon commented 4 months ago

Current Behavior

Building a simple project which imports a rust static lib fails to build when invoking ninja.

Expected Behavior

The build works.

Steps To Reproduce

1.

> cmake .. -G "Ninja Multi-Config" -DCMAKE_CROSS_CONFIGS="Debug;Release" -DCMAKE_DEFAULT_CONFIGS="Debug;Release"

2.

> ninja
ninja: error: CMakeFiles/impl-Debug.ninja:122: multiple rules generate libthe_rust_lib.a

Environment

- OS: Ubuntu 23.10
- CMake: 3.27.4
- CMake Generator: Ninja Multi-Config

CMake configure log with Debug log-level

-- The C compiler identification is GNU 13.2.0 -- The CXX compiler identification is GNU 13.2.0 -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Check for working C compiler: /usr/bin/cc - skipped -- Detecting C compile features -- Detecting C compile features - done -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Check for working CXX compiler: /usr/bin/c++ - skipped -- Detecting CXX compile features -- Detecting CXX compile features - done -- Using Corrosion 0.4.99.99 with CMake 3.27.4 and the Ninja Multi-Config Generator -- Rust Toolchain: nightly-x86_64-unknown-linux-gnu -- Rust toolchain nightly-x86_64-unknown-linux-gnu -- Rust toolchain path /home/x/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu -- Rust Target: x86_64-unknown-linux-gnu -- Parsed Target triple: arch: x86_64, vendor: unknown, OS: linux, env: gnu -- Parsed Target triple: arch: x86_64, vendor: unknown, OS: linux, env: gnu -- Determining required link libraries for target x86_64-unknown-linux-gnu -- Required static libs for target x86_64-unknown-linux-gnu: gcc_s;util;rt;pthread;m;dl;c -- Found Rust: /home/x/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc (found version "1.74.0") -- Cargo target x86_64-unknown-linux-gnu is an official target-triple -- Installed targets: aarch64-apple-darwin;aarch64-unknown-linux-gnu;arm-unknown-linux-gnueabi;armv7-unknown-linux-gnueabihf;i586-unknown-linux-gnu;i686-pc-windows-gnu;i686-pc-windows-msvc;i686-unknown-linux-gnu;powerpc64-unknown-linux-gnu;riscv64gc-unknown-linux-gnu;wasm32-unknown-unknown;x86_64-pc-windows-msvc;x86_64-unknown-linux-gnu -- Note: the following keywords passed to corrosion_import_crate had no associated value(s): FLAGS -- Found 1 targets in package the_rust_lib -- TARGET the_rust_lib produces byproducts libthe_rust_lib.a;; -- Corrosion created the following CMake targets: the_rust_lib -- Output directory property (target the_rust_lib): ARCHIVE_OUTPUT_DIRECTORY dir: output_directory-NOTFOUND -- Setting IMPORTED_LOCATION_DEBUG for target the_rust_lib-static to /home/x/y/repos/mytest/build/libthe_rust_lib.a. -- Setting IMPORTED_LOCATION_RELEASE for target the_rust_lib-static to /home/x/y/repos/mytest/build/libthe_rust_lib.a. -- Setting IMPORTED_LOCATION_RELWITHDEBINFO for target the_rust_lib-static to /home/x/y/repos/mytest/build/libthe_rust_lib.a. -- Setting IMPORTED_LOCATION for target the_rust_lib-static to /home/x/y/repos/mytest/build/libthe_rust_lib.a. -- Adding command to copy byproducts libthe_rust_lib.a to $<$:/home/x/y/repos/mytest/build>$<$:/home/x/y/repos/mytest/build>$<$:/home/x/y/repos/mytest/build>/libthe_rust_lib.a -- Configuring done (0.5s) -- Generating done (0.0s) -- Build files have been written to: /home/x/y/repos/mytest/build

CMake Build step log

Change Dir: '/home/x/y/repos/mytest/build'

Run Build Command(s): /usr/bin/ninja -v ninja: error: CMakeFiles/impl-Debug.ninja:128: multiple rules generate libthe_rust_lib.a

jschwe commented 4 months ago

Are you setting any of the OUTPUT_DIRECTORY variables in your project?

xTachyon commented 4 months ago

In the original project I found this problem, yes. In this test project I just created, no.

jschwe commented 4 months ago

Thanks for the report! I think know where the issue is and hope I can find a bit of time and fix it before the end of the week.

jschwe commented 4 months ago

@xTachyon Could you test if https://github.com/corrosion-rs/corrosion/pull/492 fixes your main issue? It's not a complete fix, since if the user sets OUTPUT_DIRECTORY to a path that does not differ per config, you would still run into the issue, while for normal CMake C/C++ executables CMake would implicitly add a $<CONFIG>. I'll probably fix that too, but if #492 is sufficient, then I would open a seperate issue for it and fix it later.

xTachyon commented 4 months ago

It's another error now!

➜  build cmake --build . --verbose
Change Dir: '/home/x/y/repos/mytest/build'

Run Build Command(s): /usr/bin/ninja -v
[0/7] cd /home/x/y/repos/mytest/the_rust_lib && /usr/bin/cmake -E env CC_x86_64_unknown_linux_gnu=/usr/bin/cc CXX_x86_64_unknown_linux_gnu=/usr/bin/c++ AR_x86_64_unknown_linux_gnu=/usr/bin/ar CORROSION_BUILD_DIR=/home/x/y/repos/mytest/build CARGO_BUILD_RUSTC=/home/x/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc /home/x/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo rustc --lib --target=x86_64-unknown-linux-gnu --package the_rust_lib --manifest-path /home/x/y/repos/mytest/the_rust_lib/Cargo.toml --target-dir /home/x/y/repos/mytest/build/Debug/cargo/build -- -Cdefault-linker-libraries=yes
   Compiling the_rust_lib v0.1.0 (/home/x/y/repos/mytest/the_rust_lib)
    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
[2/7] cd /home/x/y/repos/mytest/build && /usr/bin/cmake -E make_directory /home/x/y/repos/mytest/build/Release && /usr/bin/cmake -E copy_if_different /home/x/y/repos/mytest/build/Release/cargo/build/x86_64-unknown-linux-gnu/release/libthe_rust_lib.a /home/x/y/repos/mytest/build/Release
FAILED: CMakeFiles/Release/_cargo-build_the_rust_lib.util Release/libthe_rust_lib.a /home/x/y/repos/mytest/build/Release/libthe_rust_lib.a 
cd /home/x/y/repos/mytest/build && /usr/bin/cmake -E make_directory /home/x/y/repos/mytest/build/Release && /usr/bin/cmake -E copy_if_different /home/x/y/repos/mytest/build/Release/cargo/build/x86_64-unknown-linux-gnu/release/libthe_rust_lib.a /home/x/y/repos/mytest/build/Release
Error copying file (if different) from "/home/x/y/repos/mytest/build/Release/cargo/build/x86_64-unknown-linux-gnu/release/libthe_rust_lib.a" to "/home/x/y/repos/mytest/build/Release".
[3/7] cd /home/x/y/repos/mytest/build && /usr/bin/cmake -E make_directory /home/x/y/repos/mytest/build/Debug && /usr/bin/cmake -E copy_if_different /home/x/y/repos/mytest/build/Debug/cargo/build/x86_64-unknown-linux-gnu/debug/libthe_rust_lib.a /home/x/y/repos/mytest/build/Debug
ninja: build stopped: subcommand failed.

Edit: it seems only the Debug folder is populated, the Release folder is empty when that command executes.

jschwe commented 4 months ago

Could you share the test project you created?

xTachyon commented 4 months ago

I believe the only relevant part of the project is the cmake:

cmake_minimum_required(VERSION 3.27)

project(mytest)

add_executable(mytest main.cpp)

add_subdirectory(corrosion)
corrosion_import_crate(MANIFEST_PATH
    the_rust_lib/Cargo.toml
    NO_LINKER_OVERRIDE
    FLAGS
)
target_link_libraries(mytest the_rust_lib)

The rust lib is just a staticlib which exports a symbol, which main.cpp uses. I could share a zip with all the files if you want.

jschwe commented 4 months ago

Hmm, that doesn't sound very different from our rust2cpp test case. If you don't mind to share it, I would like to test it with your zip. I'm travelling this week though (thus also the slow response), so I probably won't have time before next week to investigate.

xTachyon commented 3 months ago

https://drive.google.com/file/d/1A1I2xiC5nqR5u8MJM6ZlQDEieGqn7OOX/view?usp=drive_link

jschwe commented 3 months ago

Edit: it seems only the Debug folder is populated, the Release folder is empty when that command executes.

I've done a bit of investigating and tried out multiple different things, but it turns out this one is actually quite hard to fix. The "proper way" of adding the built outputs as BYPRODUCTS of the cargo build custom target is not an option, since the output location of the artifacts depend on target properties of the imported target, such as hostbuild or the used cargo profile (dev/release/...). Sadly, BYPRODUCTS are not allowed to use target specific generator expressions.

I think I could perhaps work around the issue by creating per-config dummy output files, and using those files to specify the appropriate dependencies, but it requires a bit of refactoring.