-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
Add LINKER_LANGUAGE
option to corrosion_import_crate
#188
Conversation
…s to allow a crate to not have dependencies on the C++ standard lib when compiled from corrosion.
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).
This should not be a parameter for
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 I think this approach would also fix your problem. Would you be interested in doing this? |
As for testing:
Yes
This is handled by our github actions matrix.
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 |
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 :-).
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 |
…th CMake's variable.
#208 added an option to explicitly set the linker for a target, which should also fit your usecase. |
Hi @jschwe, I know it's an old PR, but I just now finally have the cycles to move off my fork and onto your most recent changes. I've tried using the new feature as suggested, but I couldn't quite figure out how This is what I tried: # Imports `questdb_client` target.
add_subdirectory(corrosion)
corrosion_import_crate(
MANIFEST_PATH questdb-rs-ffi/Cargo.toml)
corrosion_set_linker(questdb_client C) The error I get is:
If I compile with
I guess what I really need to pass is the path to the actual linker to use.
More broadly, why is this even a thing? Why even pass a linker in the first place and not just let |
Correct, you should pass a linker or linker-driver (e.g.
|
Sorry. I'm still a little confused. I have Rust code that I want to build both static and dynamic libs from. The Rust code itself does not link C++ code. The Rust code exposes a C API and needs to be callable from both C and C++. The C API does not take callbacks or anything fancy (so no worry about trapped exceptions). If I call I'm struggling to determine the right incantation of CMake functions I need to call for this. For example, this: corrosion_set_linker(questdb_client ${CMAKE_C_LINK_EXECUTABLE}) also fails with:
The library I'm shipping can be built by multiple compilers on multiple OSs, so I don't know how to determine the right linker. Would you be able to help? |
Are you sure you don't call |
Nice one! You guys fixed it! It seems that the default behaviour I was worried about which I noticed when I initally forked I don't call Sorry for not re-verifying with the default options 😅! Made my day! Thanks! |
Yeah, previously simply having C++ enabled as a language would cause corrosion to use the C++ compiler as the linker (which caused the behaviour you saw). |
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 withldd
on Linux andotool -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 theCORROSION_LINKER_PREFERENCE
variablecmake/Corrosion.cmake
which is eventually passed tocargo
asCARGO_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 tocorrosion_import_crate
. For my needs, this ends up looking like so: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:
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:
test/
?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 existingcorrosion_set_linker_language
cmake function (that I don't really understand the intended purpose of)?Thanks!