vcpkg_configure_make: Remove uuid and oldnames even when prefixed with -l #40936
Conversation
|
The other option is to not branch on |
…h -l `vcpkg_configure_make` will attempt to remove `uuid` from the `all_libs_list` (see . It will: 1. Standardize the list by removing the `.dll`, `.lib`, `.a`,... suffixes 2. Remove `uuid` from the list 3. Add the `-l` suffix for all libs if not present This works when using the msvc compiler, where uuid is specified as `uuid.lib`. When using the clang compiler, the uuid is specified as `-luuid`, and is not removed in step 2. Fix this by updating the algorithm like this: 1. Standardize the list by removing the `.dll`, `.lib`, `.a`,... suffixes 2. Add the `-l` suffix for all libs if not present 3. Remove `-luuid` from the list microsoft#17137 (comment) has some more detail. For reference, here is the output of `VCPKG_DETECTED_CMAKE_C_STANDARD_LIBRARIES` for msvc and clang: ``` set(VCPKG_DETECTED_CMAKE_C_STANDARD_LIBRARIES "-lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -loldnames") # clang set(VCPKG_DETECTED_CMAKE_C_STANDARD_LIBRARIES "kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib") # msvc ```
qmfrederik
force-pushed
the
make-remove-libs
branch
from
d70d1ab
to
1f4321f
Compare
LilyWangLL
added
the
category:tool-update
| list(TRANSFORM all_libs_list REPLACE "^([^-].*)" "-l\\1") | ||
| if(VCPKG_TARGET_IS_MINGW AND VCPKG_LIBRARY_LINKAGE STREQUAL "dynamic") | ||
| # libtool must be told explicitly that there is no dynamic linkage for uuid. | ||
| # The "-Wl,..." syntax is understood by libtool and gcc, but no by ld. | ||
| list(TRANSFORM all_libs_list REPLACE "^-luuid\$" "-Wl,-Bstatic,-luuid,-Bdynamic") | ||
| elseif(VCPKG_TARGET_IS_WINDOWS) | ||
| list(REMOVE_ITEM all_libs_list "-luuid") |
There was a problem hiding this comment.
This will now remove -luuid for static mingw. We have no test coverage for this change.
There was a problem hiding this comment.
Are you sure? The documentation says VCPKG_TARGET_IS_WINDOWS is also true for UWP and MinGW
, so it would have been removed on static MinGW int the current configuration, too.
Having said that, I believe the safest thing would be to never remove -luuid or -loldnames but tell the compiler that there is no dynamic linkage for these libraries. So:
list(TRANSFORM all_libs_list REPLACE "^-luuid\$" "-Wl,-Bstatic,-luuid,-Bdynamic")
list(TRANSFORM all_libs_list REPLACE "^-luuid\$" "-Wl,-Bstatic,-loldnames,-Bdynamic")
What do you think?
There was a problem hiding this comment.
Are you sure? The documentation says VCPKG_TARGET_IS_WINDOWS is also true for UWP and MinGW
Well, that is my point.
so it would have been removed on static MinGW int the current configuration, too.
No. For mingw, the input has -luuid.
Before this PR, the code never removes -luuid (but would wrap it for dynamic mingw).
With this PR (my "now"), the code removes -luuid unless building for dynamic mingw.
This is a change for static mingw.
I believe the safest thing would be to never remove -luuid or -loldnames but tell the compiler that there is no dynamic linkage for these libraries.
Indeed, if CMake will apply these libs, there is no need to remove them. (If there is a need to remove them, it is probably necessary to apply this already in the CMake toolchain.
This is a very central script. If a change has a (even) minimal potential of regressions for users, it is very hard to get changes accepted. Scope changes to the targets which strictly benefit from the fix.
Simply don't introduce the removal of oldnames.
More changes might be allowed for the promised vcpkg-make port.
(Disclaimer: Community feedback.)
There was a problem hiding this comment.
Well, the reason I suggested this change is because when you use the LLVM/clang native toolchain (i.e. clang, not clang-cl) on native Windows (i.e. without MinGW), you get -luuid and -loldnames from CMake, and this then gets passed down to libtool in an MSYS2 environment if building packages which use Makefiles.
The quick workaround is to create a toolchain and remove the default -luuid and -loldnames values and never think about it again.
I think the better fix is to always do:
list(TRANSFORM all_libs_list REPLACE "^-luuid\$" "-Wl,-Bstatic,-luuid,-Bdynamic")
list(TRANSFORM all_libs_list REPLACE "^-luuid\$" "-Wl,-Bstatic,-loldnames,-Bdynamic")
since we're very likely to be using the libtool linker.
I'll update the PR to reflect this, but no feelings will be hurt if this change isn't accepted ;-).
vcpkg_configure_makewill attempt to removeuuidfrom theall_libs_list.It will:
.dll,.lib,.a,... suffixesuuidfrom the list-lsuffix for all libs if not presentThis works when using the msvc compiler, where uuid is specified as
uuid.lib. When using the clang compiler, the uuid is specified as-luuid, and is not removed in step 2.Fix this by updating the algorithm like this:
.dll,.lib,.a,... suffixes-lsuffix for all libs if not present-luuidfrom the list#17137 (comment) has some more detail.
For reference, here is the output of
VCPKG_DETECTED_CMAKE_C_STANDARD_LIBRARIESfor msvc and clang: