vcpkg_check_linkage: Better explain the ONLY_DYNAMIC_LIBRARY situation #42508
+47
−36
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
BillyONeal
added
category:vcpkg-bug
dg0yt
reviewed
Dec 4, 2024
Comment on lines
23
to
24
| if("${VCPKG_CRT_LINKAGE}" STREQUAL "static") | ||
| message(FATAL_ERROR "This port can only build as a dynamic library, but the triplet \ |
There was a problem hiding this comment.
Nitpick?
Suggested change
| if("${VCPKG_CRT_LINKAGE}" STREQUAL "static") | |
| message(FATAL_ERROR "This port can only build as a dynamic library, but the triplet \ | |
| if("${VCPKG_CRT_LINKAGE}" STREQUAL "static") | |
| message(FATAL_ERROR "This port can only build as a dynamic library, but the triplet \ | |
Comment on lines
28
to
41
| For example, on Windows, each DLL will get its own copy of the CRT, meaning such DLLs cannot share \ | ||
| standard library components over the DLL boundary. On non-Windows, different .sos or .dylibs may \ | ||
| cause mutually incompatible symbols from different CRT versions to be concurrently loaded. | ||
| If you can edit the port calling vcpkg_check_linkage emitting this message, consider adding \ | ||
| (!static | !static-crt) to the \"supports\" expression so that this combination can fail early. | ||
| If you are merely consuming this port, you can consider choosing a triplet which selects a dynamic \ | ||
| CRT and/or library linkage. If you really know what you're doing, understand the potential \ | ||
| problems a static CRT with a dynamic library can cause, and are confident that this port safely \ | ||
| handles that configuration and want to proceed anyway, author a custom triplet which explicitly \ | ||
| sets VCPKG_LIBRARY_LINKAGE to dynamic and VCPKG_CRT_LINKAGE to static. For example: | ||
| if(\"\${PORT}\" STREQUAL \"${PORT}\") | ||
| set(VCPKG_LIBRARY_LINKAGE dynamic) | ||
| set(VCPKG_CRT_LINKAGE static) | ||
| endif()") |
There was a problem hiding this comment.
I suggest to move these details to https://learn.microsoft.com/vcpkg/maintainers/functions/vcpkg_check_linkage, and to only print a link here.
(And perhaps print the original intro "Refusing to build unexpected dynamic library against the static CRT.")
There was a problem hiding this comment.
Yes, please! This is way too long! Nobody will read this 17 lines long message!
There was a problem hiding this comment.
Yes, please! This is way too long! Nobody will read this 17 lines long message!
I don't think someone unwilling to read the 17 line message will be willing to go to separate documentation pages and read the same message either.
ras0219-msft
reviewed
Dec 4, 2024
ras0219-msft
reviewed
Dec 4, 2024
| if("${VCPKG_CRT_LINKAGE}" STREQUAL "static") | ||
| message(FATAL_ERROR "This port can only build as a dynamic library, but the triplet \ | ||
| selects a static library and a static CRT. Building a dynamic library with a static CRT creates \ | ||
| conditions many developers find surprising, and for which most ports are unprepared, so vcpkg will \ |
There was a problem hiding this comment.
Suggested change
| conditions many developers find surprising, and for which most ports are unprepared, so vcpkg will \ | |
| conditions many developers find surprising and for which most ports are unprepared. Therefore, vcpkg will \ |
ras0219-msft
reviewed
Dec 4, 2024
| For example, on Windows, each DLL will get its own copy of the CRT, meaning such DLLs cannot share \ | ||
| standard library components over the DLL boundary. On non-Windows, different .sos or .dylibs may \ | ||
| cause mutually incompatible symbols from different CRT versions to be concurrently loaded. | ||
| If you can edit the port calling vcpkg_check_linkage emitting this message, consider adding \ |
There was a problem hiding this comment.
Suggested change
| If you can edit the port calling vcpkg_check_linkage emitting this message, consider adding \ | |
| If you can edit the port calling vcpkg_check_linkage that emits this message, consider adding \ | |
ras0219-msft
reviewed
Dec 4, 2024
| cause mutually incompatible symbols from different CRT versions to be concurrently loaded. | ||
| If you can edit the port calling vcpkg_check_linkage emitting this message, consider adding \ | ||
| (!static | !static-crt) to the \"supports\" expression so that this combination can fail early. | ||
| If you are merely consuming this port, you can consider choosing a triplet which selects a dynamic \ |
There was a problem hiding this comment.
Suggested change
| If you are merely consuming this port, you can consider choosing a triplet which selects a dynamic \ | |
| If you are consuming this port, consider choosing a triplet that selects a dynamic \ |
ras0219-msft
reviewed
Dec 4, 2024
Comment on lines
35
to
36
| problems a static CRT with a dynamic library can cause, and are confident that this port safely \ | ||
| handles that configuration and want to proceed anyway, author a custom triplet which explicitly \ |
There was a problem hiding this comment.
Suggested change
| problems a static CRT with a dynamic library can cause, and are confident that this port safely \ | |
| handles that configuration and want to proceed anyway, author a custom triplet which explicitly \ | |
| problems a static CRT with a dynamic library can cause, are confident that this port safely \ | |
| handles that configuration, and want to proceed anyway, author a custom triplet which explicitly \ |
|
I cancelled the build pending review of dg0yt's comment by other maintainers: #42508 (comment) and I'll apply everything else. (I just didn't want to cause another validation run with that outstanding) |
|
The failures are baseline issues. |
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.
Related: #42456
I know the formatting looks strange; this is a workaround for
FATAL_ERROR's behavior that inserts extra newlines :(Output looks like this: