msvc: handle flags that come from native-static-libs #511
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
As of https://github.com/rust-lang/rust/pull/122268/files, rust emits /defaultlib:msvcrt in native-static-libs This ends up in INTERFACE_LINK_LIBRARIES, and then ninja thinks it's a file: ninja: error: '/defaultlib:msvcrt', needed by 'cpp-exe.exe', missing and no known rule to make it This check to see if libraries start with a slash, and then assuming that they are a flag, and putting those in INTERFACE_LINK_OPTIONS instead. If there is some better way to detect flags, I'm all for it. We could also have it have anything that starts with `-` but not start with `-l` be detected as a flag, but that seems a bit more likely to be disruptive. I checked commandlines and it does indeed get propaged properly to the commandline.
jschwe
reviewed
Apr 26, 2024
|
Could you also add an entry to the changelog in |
|
thanks! |
jschwe
pushed a commit
to jschwe/corrosion
that referenced
this pull request
May 10, 2024
* msvc: handle flags that come from native-static-libs As of https://github.com/rust-lang/rust/pull/122268/files, rust emits /defaultlib:msvcrt in native-static-libs This ends up in INTERFACE_LINK_LIBRARIES, and then ninja thinks it's a file: ninja: error: '/defaultlib:msvcrt', needed by 'cpp-exe.exe', missing and no known rule to make it This check to see if libraries start with a slash, and then assuming that they are a flag, and putting those in INTERFACE_LINK_OPTIONS instead. (cherry picked from commit fce4fe5)
jschwe
pushed a commit
to jschwe/corrosion
that referenced
this pull request
May 10, 2024
* msvc: handle flags that come from native-static-libs As of https://github.com/rust-lang/rust/pull/122268/files, rust emits /defaultlib:msvcrt in native-static-libs This ends up in INTERFACE_LINK_LIBRARIES, and then ninja thinks it's a file: ninja: error: '/defaultlib:msvcrt', needed by 'cpp-exe.exe', missing and no known rule to make it This check to see if libraries start with a slash, and then assuming that they are a flag, and putting those in INTERFACE_LINK_OPTIONS instead. (cherry picked from commit fce4fe5)
jschwe
pushed a commit
to jschwe/corrosion
that referenced
this pull request
May 11, 2024
* msvc: handle flags that come from native-static-libs As of https://github.com/rust-lang/rust/pull/122268/files, rust emits /defaultlib:msvcrt in native-static-libs This ends up in INTERFACE_LINK_LIBRARIES, and then ninja thinks it's a file: ninja: error: '/defaultlib:msvcrt', needed by 'cpp-exe.exe', missing and no known rule to make it This check to see if libraries start with a slash, and then assuming that they are a flag, and putting those in INTERFACE_LINK_OPTIONS instead. (cherry picked from commit fce4fe5)
jschwe
pushed a commit
that referenced
this pull request
May 11, 2024
* msvc: handle flags that come from native-static-libs As of https://github.com/rust-lang/rust/pull/122268/files, rust emits /defaultlib:msvcrt in native-static-libs This ends up in INTERFACE_LINK_LIBRARIES, and then ninja thinks it's a file: ninja: error: '/defaultlib:msvcrt', needed by 'cpp-exe.exe', missing and no known rule to make it This check to see if libraries start with a slash, and then assuming that they are a flag, and putting those in INTERFACE_LINK_OPTIONS instead. (cherry picked from commit fce4fe5)
wantehchang
added a commit
to wantehchang/libavif
that referenced
this pull request
Jun 21, 2024
We need the bug fix corrosion-rs/corrosion#511.
wantehchang
reviewed
Jun 21, 2024
| 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) |
There was a problem hiding this comment.
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()
wantehchang
reviewed
Jun 21, 2024
| @@ -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) | |||
There was a problem hiding this comment.
Should we add unset(required_link_flags) before this line?
| 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) |
There was a problem hiding this comment.
Should we add unset(ost_flags) before this line?
wantehchang
added a commit
to AOMediaCodec/libavif
that referenced
this pull request
Jun 21, 2024
We need the bug fix corrosion-rs/corrosion#511.
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.
As of rust-lang/rust#122268, rust emits /defaultlib:msvcrt in native-static-libs
This ends up in INTERFACE_LINK_LIBRARIES, and then ninja thinks it's a file:
ninja: error: '/defaultlib:msvcrt', needed by 'cpp-exe.exe', missing and no known rule to make it
This check to see if libraries start with a slash, and then assuming that they are a flag, and putting those in INTERFACE_LINK_OPTIONS instead. If there is some better way to detect flags, I'm all for it. We could also have it have anything that starts with
-but not start with-lbe detected as a flag, but that seems a bit more likely to be disruptive.I checked commandlines and it does indeed get propaged properly to the commandline.