Fix CMake Windows system library deps #6980
Merged
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
fooishbar
commented
Apr 15, 2020
mensinda
requested changes
Apr 15, 2020
|
The three CI failures ( |
|
Yes, these CI failures are currently a know issue. |
fooishbar
force-pushed
the
cmake-windows-system-lib-deps
branch
from
dd26711
to
6108832
Compare
jenatali
suggested changes
Apr 15, 2020
fooishbar
force-pushed
the
cmake-windows-system-lib-deps
branch
from
6108832
to
b485144
Compare
jenatali
approved these changes
Apr 15, 2020
When finding dependencies from CMake, use a smarter regex to determine whether or not a dependency is actually a link argument, and pass through Windows link arguments properly.
On Windows, library dependencies can be passed with no prefix or suffix; rather than -lfoo or foo.dll, they can just be passed as 'foo'. CMake handles this and suffixes the library with '.lib' or '.dll', depending on the link mode. Do the same here, and if we've been passed an unqualified non-option bare name on Windows, add the appropriate suffix and pass it through to the linker. This fixes dependencies on system libraries.
When taking library dependencies from CMake, we first attempt to look the dependency up in the target list, then fall back to treating it as a path, which we add if the path exists. As there is no check for whether or not the path is really a path, this can cause false positives; for example if a 'uuid' dependency was passed intending to be a target, but it cannot be found and the current directory also contains a file or directory named 'uuid', we will just include the string 'uuid' in library dependencies. This is particularly prevalent on Windows, where a system library called 'version' exists, and thanks to case insensitivity will match a file called 'VERSION' if found in the source root when running Meson from the source directory, or a generated file when running Meson from the build directory. Fix this check to only look up filesystem existence on absolute paths, not unqualified. This also adds a fallback warning in case an argument cannot be found, rather than silently falling back.
fooishbar
force-pushed
the
cmake-windows-system-lib-deps
branch
from
b485144
to
f5f3805
Compare
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.
These three commits fix linking of system libraries when declared as dependencies of CMake targets we pick up as Meson dependencies. In particular, when linking Clang into Mesa via CMake, we were dropping all of the system library dependencies such as
ole32anduuid… except if Meson was executed from the source directory, in which case we would retainversionas a bare argument (since the Mesa source root has a file calledVERSION), which would get passed through to the linker without a suffix qualification and fail the build.This PR attempts to do something more reasonable with system-library dependencies under Windows.
cc @dcbaker @jenatali