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.

Sign up for GitHub

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

Jump to bottom

msvc: handle flags that come from native-static-libs #511

Merged
merged 3 commits into from
Apr 26, 2024
Merged

msvc: handle flags that come from native-static-libs #511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
26d5e43
msvc: handle flags that come from native-static-libs
russelltg Apr 26, 2024
91232d9
only detect flags as starting with / on *msvc targets
russelltg Apr 26, 2024
d4ad661
add line in RELEASES.md
russelltg Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions RELEASES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@
- Set the `AR_<triple>` variable for `cc-rs` (except for msvc targets) [#456]
- `corrosion_experimental_cbindgen()` now forwards the Rust target-triple (e.g. `aarch64-unknown-linux-gnu`)
to cbindgen via the `TARGET` environment variable. The `hostbuild` property is considered. [#507]
- Detect msvc linker flags coming from `--print=native-static-libs` and put them into `INTERFACE_LINK_OPTIONS` instead of `INTERFACE_LINK_LIBRARIES` [#511]

[#459]: https://github.com/corrosion-rs/corrosion/pull/459
[#456]: https://github.com/corrosion-rs/corrosion/pull/456
[#455]: https://github.com/corrosion-rs/corrosion/pull/455
[#506]: https://github.com/corrosion-rs/corrosion/pull/506
[#507]: https://github.com/corrosion-rs/corrosion/pull/507
[#511]: https://github.com/corrosion-rs/corrosion/pull/511

# v0.4.7 (2024-01-19)

Expand Down
4 changes: 4 additions & 0 deletions cmake/Corrosion.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,10 @@ function(_corrosion_add_library_target)
TARGET ${target_name}-static
PROPERTY INTERFACE_LINK_LIBRARIES ${Rust_CARGO_TARGET_LINK_NATIVE_LIBS}
)
set_property(
TARGET ${target_name}-static
PROPERTY INTERFACE_LINK_OPTIONS ${Rust_CARGO_TARGET_LINK_OPTIONS}
)
if(is_macos)
set_property(TARGET ${target_name}-static
PROPERTY INTERFACE_LINK_DIRECTORIES "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib"
Expand Down
29 changes: 22 additions & 7 deletions cmake/FindRust.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ function(_corrosion_parse_target_triple target_triple out_arch out_vendor out_os
set("${out_env}" "${CMAKE_MATCH_6}" PARENT_SCOPE)
endfunction()

function(_corrosion_determine_libs_new target_triple out_libs)
function(_corrosion_determine_libs_new target_triple out_libs out_flags)
set(package_dir "${CMAKE_BINARY_DIR}/corrosion/required_libs")
# Cleanup on reconfigure to get a cleans state (in case we change something in the future)
file(REMOVE_RECURSE "${package_dir}")
Expand Down Expand Up @@ -162,6 +162,7 @@ function(_corrosion_determine_libs_new target_triple out_libs)
if(cargo_build_error_message MATCHES "native-static-libs: ([^\r\n]+)\r?\n")
string(REPLACE " " ";" "libs_list" "${CMAKE_MATCH_1}")
set(stripped_lib_list "")
set(flag_list "")

set(was_last_framework OFF)
foreach(lib ${libs_list})
Expand All @@ -175,10 +176,16 @@ function(_corrosion_determine_libs_new target_triple out_libs)
set(was_last_framework OFF)
continue()
endif()
# Strip leading `-l` (unix) and potential .lib suffix (windows)
string(REGEX REPLACE "^-l" "" "stripped_lib" "${lib}")
string(REGEX REPLACE "\.lib$" "" "stripped_lib" "${stripped_lib}")
list(APPEND stripped_lib_list "${stripped_lib}")

# Flags start with / for MSVC
if (lib MATCHES "^/" AND ${target_triple} MATCHES "msvc$")
list(APPEND flag_list "${lib}")
else()
# Strip leading `-l` (unix) and potential .lib suffix (windows)
string(REGEX REPLACE "^-l" "" "stripped_lib" "${lib}")
string(REGEX REPLACE "\.lib$" "" "stripped_lib" "${stripped_lib}")
list(APPEND stripped_lib_list "${stripped_lib}")
endif()
endforeach()
set(libs_list "${stripped_lib_list}")
# Special case `msvcrt` to link with the debug version in Debug mode.
Expand All @@ -190,6 +197,7 @@ function(_corrosion_determine_libs_new target_triple out_libs)
endif()
endif()
set("${out_libs}" "${libs_list}" PARENT_SCOPE)
set("${out_flags}" "${flag_list}" PARENT_SCOPE)
endfunction()

if (NOT "${Rust_TOOLCHAIN}" STREQUAL "$CACHE{Rust_TOOLCHAIN}")
Expand Down Expand Up @@ -762,28 +770,35 @@ set(Rust_CARGO_HOST_ENV "${rust_host_env}" CACHE INTERNAL "Host environment")
if(NOT DEFINED CACHE{Rust_CARGO_TARGET_LINK_NATIVE_LIBS})
message(STATUS "Determining required link libraries for target ${Rust_CARGO_TARGET_CACHED}")
unset(required_native_libs)
_corrosion_determine_libs_new("${Rust_CARGO_TARGET_CACHED}" required_native_libs)
_corrosion_determine_libs_new("${Rust_CARGO_TARGET_CACHED}" required_native_libs required_link_flags)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add unset(required_link_flags) before this line?

if(DEFINED required_native_libs)
message(STATUS "Required static libs for target ${Rust_CARGO_TARGET_CACHED}: ${required_native_libs}" )
endif()
if(DEFINED required_link_flags)
message(STATUS "Required link flags for target ${Rust_CARGO_TARGET_CACHED}: ${required_link_flags}" )
endif()
# In very recent corrosion versions it is possible to override the rust compiler version
# per target, so to be totally correct we would need to determine the libraries for
# every installed Rust version, that the user could choose from.
# In practice there aren't likely going to be any major differences, so we just do it once
# for the target and once for the host target (if cross-compiling).
set(Rust_CARGO_TARGET_LINK_NATIVE_LIBS "${required_native_libs}" CACHE INTERNAL
"Required native libraries when linking Rust static libraries")
set(Rust_CARGO_TARGET_LINK_OPTIONS "${required_link_flags}" CACHE INTERNAL
"Required link flags when linking Rust static libraries")
endif()

if(Rust_CROSSCOMPILING AND NOT DEFINED CACHE{Rust_CARGO_HOST_TARGET_LINK_NATIVE_LIBS})
message(STATUS "Determining required link libraries for target ${Rust_CARGO_HOST_TARGET_CACHED}")
unset(host_libs)
_corrosion_determine_libs_new("${Rust_CARGO_HOST_TARGET_CACHED}" host_libs)
_corrosion_determine_libs_new("${Rust_CARGO_HOST_TARGET_CACHED}" host_libs host_flags)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we should add the following message after line 797, similar to lines 777-779 above:

    if(DEFINED host_flags)
        message(STATUS "Required link flags for host target ${Rust_CARGO_HOST_TARGET_CACHED}: ${host_flags}" )
    endif()

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add unset(ost_flags) before this line?

if(DEFINED host_libs)
message(STATUS "Required static libs for host target ${Rust_CARGO_HOST_TARGET_CACHED}: ${host_libs}" )
endif()
set(Rust_CARGO_HOST_TARGET_LINK_NATIVE_LIBS "${host_libs}" CACHE INTERNAL
"Required native libraries when linking Rust static libraries for the host target")
set(Rust_CARGO_HOST_TARGET_LINK_OPTIONS "${host_flags}" CACHE INTERNAL
"Required linker flags when linking Rust static libraries for the host target")
endif()

# Set the input variables as non-cache variables so that the variables are available after
Expand Down