Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vcpkg_check_linkage: Better explain the ONLY_DYNAMIC_LIBRARY situation #42508

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+47 −36
Open

vcpkg_check_linkage: Better explain the ONLY_DYNAMIC_LIBRARY situation #42508

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ceca62d
vcpkg_check_linkage: Better explain the ONLY_DYNAMIC_LIBRARY situation
BillyONeal Dec 4, 2024
c49622c
What's a proofread?
BillyONeal Dec 4, 2024
b5217c3
Extract some parts to docs, go back to ! on the outside.
BillyONeal Dec 6, 2024
d1a6a21
Merge remote-tracking branch 'origin/master' into HEAD
BillyONeal Dec 9, 2024
ad795b2
Merge remote-tracking branch 'origin/master' into explain-check-linkage
BillyONeal Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions scripts/cmake/vcpkg_check_linkage.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,27 @@ function(vcpkg_check_linkage)
message(STATUS "Note: ${PORT} only supports static library linkage. Building static library.")
set(VCPKG_LIBRARY_LINKAGE static PARENT_SCOPE)
elseif(arg_ONLY_DYNAMIC_LIBRARY AND "${VCPKG_LIBRARY_LINKAGE}" STREQUAL "static")
message(STATUS "Note: ${PORT} only supports dynamic library linkage. Building dynamic library.")
if("${VCPKG_CRT_LINKAGE}" STREQUAL "static")
message(FATAL_ERROR "Refusing to build unexpected dynamic library against the static CRT.
If this is desired, please configure your triplet to directly request this configuration.")
if("${VCPKG_CRT_LINKAGE}" STREQUAL "static")
message(FATAL_ERROR "This port can only build as a dynamic library, but the triplet \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 \

ras0219-msft marked this conversation as resolved.
Show resolved Hide resolved
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 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 \

not change VCPKG_LIBRARY_LINKAGE to dynamic for you in this case.
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 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 \

(!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 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 \

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 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 \

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()")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please! This is way too long! Nobody will read this 17 lines long message!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

else()
message(STATUS "Note: ${PORT} only supports dynamic library linkage. Building dynamic library.")
endif()
set(VCPKG_LIBRARY_LINKAGE dynamic PARENT_SCOPE)
endif()
Expand Down
Loading