[vcpkg] Fix lib uuid handling for x64-mingw-dynamic#17137
[vcpkg] Fix lib uuid handling for x64-mingw-dynamic#17137strega-nil-ms merged 2 commits intomicrosoft:masterfrom
Conversation
|
uuid probably does not need to be removed if VCPKG_TARGET_IS_MINGW. uuid is just removed due to spurious CI errors where uuid seems not to be available |
|
Hm. I found these errors quite reproducible wih I honestly couldn't take much out of the search engine results "linker path does not have real file for library" "luuid" "mingw" except for some issue related to static vs dynamic linking. |
|
It basically means it cannot find uuid for some reason. |
|
Sure,
However, there is a pure static archive, reported as "Last file checked". And I eventually figured out how to make this acceptable for libtool. Will update the PR. |
On the downside, this syntax is understood by gcc or libtool, but not by ld. So it depends on how the linker is invoked. |
|
As a heuristic for safely assuming usage of libtool, one might probably look for |
dg0yt
changed the title
|
I checked |
I cannot answer anything for MSVC. And even when the answer is yes, it probably needs different flags than for libtool. |
|
@JackBoosY I don't think the "requires:author-response" tag is still justified: Other than in the initial form of the PR, libuuid is not removed but marked to be linked statically for mingw. This is tested with some packages. So initial concerns regarding the removal are resolved. Formally, there remain concerns whether the syntax for linking statically is understood by all ports. But without CI support this cannot easily be answered upfront - someone has to stand up point to a port which breaks (for mingw) with this change. Last not least, a question was raised about Uuid.lib in general. But this is out of scope for this PR/author. |
JackBoosY
added
category:community-triplet
|
@dg0yt I will handle this PR in next week. |
|
Sorry for late, I will handle it this week. |
PhoebeHui
changed the title
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
libmicrohttpd:x64-osx Is this related to your changes? |
|
Waiting for #17853 merge. |
JackBoosY
added
the
depends:different-pr
|
I can confirm that this fixes the issue in my tests. 👍 |
|
Hmm could you test some port combination with a custom triplet with the following:
I want to know if libtool in A also chokes on B in this case. We might need to simply ignore all libtool stupidity and allow passing of all possible lib files. |
JackBoosY
removed
the
depends:different-pr
JackBoosY
added
the
info:needs-maintainer-attention
|
@Neumann-A Tested successfully with libmicrohttpd (A) in a modified triplet with static libiconv, gettext (B): i.e. libtool changes the order of arguments, but nothing bad happens. Based on my pre-vcpkg mingw experience, I did a more radical test now with the pristine x64-mingw-dynamic triplet: There is still So why generally pass CMake's implicit libraries at all (for mingw)? |
either I am blind or cannot find it linking against iconv or intl in the call
These might be implicit on MINGW and note that we are not passing https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_IMPLICIT_LINK_LIBRARIES.html but Passing those libraries is required for some ports on windows and all ports on uwp (and is different for UWP). |
Good point.
From a cmake user's POV, CMake (not the compiler) adds them implicitly. (CMake user experience.)
I don't want to dispute that it may be necessary for other triplets, possibly for all ports. But I wonder how many ports need these libs explicitly for mingw. Outside the vcpkg world, autotools with mingw has been used for a while, and I have difficulties to find references about these libraries being necessary to be given separately to |
|
I regard the following as somewhat off-topic to the original PR (which is to make *-mingw-dynamic triplets usable at all), but I want to answer the question by @Neumann-A.
Actually I fell over the consequences of static linkage and transitive dependencies. The reported build didn't use gettext because libmicrohttd's
As a simpler case, I looked at building dynamic gettext (=A) with static libiconv (=B). leading to: I don't feel like "simply ignore all libtool stupidity and allow passing of all possible lib files" is really the right thing to do in general. After all it works fairly well for pure static or dynamic configurations (apart from uuid). User-defined mixed configurations are a different story. |
The question is does libtool actually do anything relevant? If it only runs
I read somewhere that uuid on windows is always a static library, so it makes sense that it errors in dynamic builds. |
|
Since |
I don't think it can be counted as a cmake bug. IMO it is a convenience feature in cmake so that users don't have to explicitly specify frequently used (or actually required) libraries. It is used for linking executables and shared libraries, but cmake doesn't require these libraries to be shared libraries. Now here we are dealing with autoconf and libtool. Generally, it is convenient and fair to forward the libraries proposed by cmake. I'm not aware of another source of libraries which are frequently used or required for building for a particular triplet. However, in the case of uuid, we are faced with the fact that libtool normally refuses to accept a static candidate library when creating a shared library. This problem is specific to vcpkg because noone else attempts to blindly forward cmake's list of libs to autotools. I think what is needed is an "informed forwarding" based on the triplet. Options:
There is another option mentioned by @Neumann-A which is:
|
|
Personally, I prefer the current state of this PR over disabling the libtool check altogether. |
|
After some internal discussion, we think we can accept this PR. |
|
Thanks @dg0yt :) |
[vcpkg] Fix lib uuid handling for x64-mingw-dynamic (microsoft#17137)
…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 ```
…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 ```
5 participants
Describe the pull request
What does your PR fix?
This PR fixes the build of gettext (and possibly other packages) on Windows when triplet, host triplet and cmake are mingw. Here, the cmake-detected standard libraries come with
-luuid, notuuid, so it is not removed from LIBS by vcpkg. However, libtool refuses to create shared libraries for mingw because there is no shared library uuid. This change explicitly instructs libtool (or gcc) to statically link uuid.Which triplets are supported/not supported? Have you updated the CI baseline?
These flags do not work with plain
ld.Does your PR follow the maintainer guide?
-/-