Allow explicitly selecting the linker for a rust target #208
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There is no point in selecting the CXX compiler as the linker driver for the corrosion-generator. We might as well let cargo choose the appropriate linker based on the target.
Let CMake deal with the correct quoting. This change makes it easier to e.g. pass --cfg options via Rustflags.
Allow explicitly defining a linker for when linking is done on the Rust side. Previously Corrosion offered an corrosion_set_linker_language(), which effectively enabled a language, but still determined the linker by choosing the compiler with the highest priority as determined by CMake. This is probably not what most users actually want. Explicitly setting a linker should be less surprising and more in line with what a user would actually want to do. Since setting a linker may happen multiple times for different targets and potentially with different linkers, we don't use find_program to verify if the linker can actually be found (for performance reasons). However, we do not allow explicitely setting the linker for MSVC toolchains. Setting the linker causes strange problems with MSVC, where build-script-build fails with a file not found os-error, and I'm not exactly sure why. Since with MSVC the build should anyway be run from a visual studio command prompt, the correct `cl.exe` will be in PATH, so we can just let cargo find it in this case.
It's not possible for corrosion to automatically add this if the linker is manually set. Since cargo does actually automatically add the `--target` linker option when appropriate, there should also be no need for corrosion to set it.
jschwe
changed the title
ogoffart
approved these changes
Sep 1, 2022
This was referenced Sep 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
VERBATIMoption when invokingcargo buildso that CMake quotes as necessary. This should improve the experience for passing some parameters, but is a breaking change.Closes #133
Closes #88