Add `LINKER_LANGUAGE` option to `corrosion_import_crate`

[amunra]

Background and Linker Problem

I am developing a library that is intended to be consumed by both C and C++. The library's core logic is written in Rust, exporting a .h C header and an optional wrapper .hpp C++ header, with tests in C++.

I've started using corrosion to integrate calling cargo with CMake, but noticed (investigating with ldd on Linux and otool -L on MacOS) that the corrosion-cargo-built binary depended on the C++ standard library.

The C++ dependency is absent when invoking cargo on the command line directly and is introduced by corrosion when it sets the CORROSION_LINKER_PREFERENCE variable cmake/Corrosion.cmake which is eventually passed to cargo as CARGO_TARGET_...._LINKER=path/to/cpp_compiler. Reading the code, this logic is skipped when building on Windows with MSVC.

It is a hard requirement for my project that the rust-built library does not depend on C++, as such I've started adding a feature to corrosion to specify which language's linker to pass into the cargo invocation.

Approach

The approach I'm taking is to add an optional LINKER_LANGUAGE parameter to corrosion_import_crate. For my needs, this ends up looking like so:

corrosion_import_crate(
    MANIFEST_PATH Cargo.toml
    FEATURES ffi
    LINKER_LANGUAGE C)

Does this make sense?

P.S.: I've also noticed there's a corrosion_set_linker_language CMake function, but it didn't seem to do anything relevant to my problem.

State of this pull request

I'm raising a draft pull request because, frankly, I'm still learning CMake and I expect more changes are needed before this can be merged in.

Testing

So far I've re-run a subset of the existing tests locally on Linux as so:

# Prepare for testing
rm -fR build
cmake \
  -S . \
  -B build \
  -DCORROSION_VERBOSE_OUTPUT=ON \
  -DCMAKE_C_COMPILER=gcc \
  -DCMAKE_CXX_COMPILER=g++ \
  -GNinja

# Run Config Tests (x86_64 Only)
(cd build &&
cmake \
  --build . --verbose \
  --target test-install
ctest --verbose --build-config Debug)

# Build C++ program linking Rust
(cmake \
  --build build --verbose \
  --target cpp-exe)

# Build Rust program linking C++
(cmake \
  --build build --verbose \
  --target cargo-build_rust-exe)

# Build Rust cdylib and link against C++
(cmake \
  --build build --verbose \
  --target my_program)

This is not exhausitve, but I think it's unlikely I've broken anything.

I certainly haven't tested the new feature though, aside from the low bar of "it works for me".

I would appreciate some guidance on how to progress, especially on the testing front:

• Would this be a new directory under test/?
• Should I create different tests to with and without the native tools or is this somehow handled by the github actions workflow matrix?
• Any idea on how to actually assert if a cargo built library doesn't have a C++ dependency from CMake -- pretty clueless here.

Docs

There's also a matter of documentation: The "LINKER_LANGUAGE" feature would be de facto Mac/Linux only. Is it OK to just document (and maybe log) that the named argument is ignored on Windows/MSVC?

Naming

Should the parameter name here be changed? Would LINK_AS be a better name to avoid creating confusion with the already existing corrosion_set_linker_language cmake function (that I don't really understand the intended purpose of)?

Thanks!

[jschwe]

The C++ dependency is absent when invoking cargo on the command line directly and is introduced by corrosion when it sets the CORROSION_LINKER_PREFERENCE variable cmake/Corrosion.cmake which is eventually passed to cargo as CARGO_TARGET_...._LINKER=path/to/cpp_compiler.

Corrosion currently will select the linker with the highest priority (as determined by CMake). So if you have C++ enabled as a language in your project, The C++ compiler will be selected. (I don't know why it's different for MSVC).

The approach I'm taking is to add an optional LINKER_LANGUAGE parameter to corrosion_import_crate

This should not be a parameter for corrosion_import_crate but rather a target based property.

P.S.: I've also noticed there's a corrosion_set_linker_language CMake function, but it didn't seem to do anything relevant to my problem.

Yes, this function does not work as intended. I wrote up how I view the problem in issue https://github.com/corrosion-rs/corrosion/issues/133. But I got busy with other stuff, so I never ended up fixing it.

As described in the issue I would prefer if we had a new corrosion_set_linker function, which sets a target property (the name should not be LINKER_LANGUAGE, since that is an existing CMake target property). Then when setting CARGO_TARGET_...._LINKER we should set it to the value of the new target property if set, and otherwise set it to the same value as before (i.e. the linker with the highest priority).

I think this approach would also fix your problem. Would you be interested in doing this?

[jschwe]

As for testing:

Would this be a new directory under test/?

Yes

Should I create different tests to with and without the native tools or is this somehow handled by the github actions workflow matrix?

This is handled by our github actions matrix.

Any idea on how to actually assert if a cargo built library doesn't have a C++ dependency from CMake -- pretty clueless here

You could try to call a C++ standard library function. This should cause a failure at the linking step if the C++ stdlib is not linked. Then you just have to tell ctest that you expected a build-failure for this test - I think this stack overflow answer might point you in the right direction: https://stackoverflow.com/a/30191576

[amunra]

The C++ dependency is absent when invoking cargo on the command line directly and is introduced by corrosion when it sets the CORROSION_LINKER_PREFERENCE variable cmake/Corrosion.cmake which is eventually passed to cargo as CARGO_TARGET_...._LINKER=path/to/cpp_compiler.

Corrosion currently will select the linker with the highest priority (as determined by CMake). So if you have C++ enabled as a language in your project, The C++ compiler will be selected. (I don't know why it's different for MSVC).

Yes, that's what I noticed. I'm assuming there's no way in CMake to disable C++ for just part of the project. It's also quite surprising that a corrosion uses a different linker than the default one. I'm not sure why it goes through the effort rather than just letting cargo select it: I'm sure there's a good reason for it though :-).

The approach I'm taking is to add an optional LINKER_LANGUAGE parameter to corrosion_import_crate

This should not be a parameter for corrosion_import_crate but rather a target based property.

P.S.: I've also noticed there's a corrosion_set_linker_language CMake function, but it didn't seem to do anything relevant to my problem.

Yes, this function does not work as intended. I wrote up how I view the problem in issue #133. But I got busy with other stuff, so I never ended up fixing it.

As described in the issue I would prefer if we had a new corrosion_set_linker function, which sets a target property (the name should not be LINKER_LANGUAGE, since that is an existing CMake target property). Then when setting CARGO_TARGET_...._LINKER we should set it to the value of the new target property if set, and otherwise set it to the same value as before (i.e. the linker with the highest priority).

I think this approach would also fix your problem. Would you be interested in doing this?

I'd be happy to give it a try, but as I mentioned this is my first time using CMake: I would need some pointers and assistance. In the meantime I need the feature urgently and I'll be using my forked repo.

I'm also unclear on what the benefit is of having this be a per-target function: My understanding is that a rust crate may have exactly 0 or 1 libraries defined. It can however have multiple binaries, though these would all be built as part of the same invocation of cargo with the same linker. I'm unclear how a per-target approach would work in practice.

[jschwe]

https://github.com/corrosion-rs/corrosion/pull/208 added an option to explicitly set the linker for a target, which should also fit your usecase.