fix: install DLL to bin directory on Windows - #772
Conversation
|
Hi @kt286. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR updates Windows symbol export handling to use CMake’s WINDOWS_EXPORT_ALL_SYMBOLS target property and fixes Windows DLL installation paths by splitting install destinations per artifact type. Sequence diagram for Windows DLL loading after corrected install pathsequenceDiagram
actor User
participant Application
participant WindowsLoader
participant DllFile
User->>Application: start
Application->>WindowsLoader: loadLibrary
WindowsLoader->>DllFile: search in CMAKE_INSTALL_BINDIR
WindowsLoader-->>Application: DLL loaded successfully
Flow diagram for updated CMake install destinations on Windowsflowchart TD
A[install TARGETS LIB_NAME] --> B{Component type}
B --> C[LIBRARY DESTINATION LIBRARY_INSTALL_DIR]
B --> D[ARCHIVE DESTINATION LIBRARY_INSTALL_DIR]
B --> E[RUNTIME DESTINATION CMAKE_INSTALL_BINDIR]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning
详情 {
"export": {
"dtkwidget.cmake": {
"a": [
" set(CMAKE_SHARED_LINKER_FLAGS \"${CMAKE_SHARED_LINKER_FLAGS} -Wl,--export-all-symbols\")"
]
}
}
} |
897f401 to
c2778b7
Compare
|
Warning
详情 {
"export": {
"dtkwidget.cmake": {
"a": [
" set(CMAKE_SHARED_LINKER_FLAGS \"${CMAKE_SHARED_LINKER_FLAGS} -Wl,--export-all-symbols\")"
]
}
}
} |
c2778b7 to
31c7ce1
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The change in dtkwidget.cmake now applies the
-Wl,--export-all-symbolsflag to MSVC toolchains as well; this flag is GCC/MinGW-specific, so consider switching to theWINDOWS_EXPORT_ALL_SYMBOLStarget property or guarding the flag withNOT MSVCas described in the PR. - If the intention is only to move Windows DLLs to
CMAKE_INSTALL_BINDIR, you may want to wrap theRUNTIME DESTINATIONchange in aWIN32condition to avoid unintended layout changes on non-Windows platforms.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change in dtkwidget.cmake now applies the `-Wl,--export-all-symbols` flag to MSVC toolchains as well; this flag is GCC/MinGW-specific, so consider switching to the `WINDOWS_EXPORT_ALL_SYMBOLS` target property or guarding the flag with `NOT MSVC` as described in the PR.
- If the intention is only to move Windows DLLs to `CMAKE_INSTALL_BINDIR`, you may want to wrap the `RUNTIME DESTINATION` change in a `WIN32` condition to avoid unintended layout changes on non-Windows platforms.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Separate install destinations for LIBRARY, ARCHIVE and RUNTIME to ensure DLL files are installed to the bin directory on Windows instead of lib. fix: Windows 平台 DLL 安装到 bin 目录 分离 LIBRARY、ARCHIVE 和 RUNTIME 的安装路径,确保 Windows 平台的 DLL 文件安装到 bin 目录而非 lib 目录。
31c7ce1 to
f13cc18
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, kt286 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Replace compiler-specific -Wl,--export-all-symbols linker flag with the cmake target property WINDOWS_EXPORT_ALL_SYMBOLS ON. This is the recommended cross-platform approach and avoids polluting global linker flags.
Also split the install(TARGETS) command into per-component destinations (LIBRARY, ARCHIVE, RUNTIME) so that DLL files are installed to the bin directory on Windows, which is required by the Windows runtime loader.
fix: 在 Windows 上使用 WINDOWS_EXPORT_ALL_SYMBOLS 并修复 DLL 安装路径
用 CMake target 属性 WINDOWS_EXPORT_ALL_SYMBOLS ON 替代编译器特定的 -Wl,--export-all-symbols 链接器标志,这是推荐的跨平台做法,避免污染
全局链接器标志。
同时将 install(TARGETS) 拆分为按组件指定安装目录(LIBRARY、ARCHIVE、 RUNTIME),使 DLL 文件在 Windows 上安装到 bin 目录,符合 Windows 运行时 加载器的要求。
Summary by Sourcery
Correct Windows shared-library exports and installation paths through CMake target configuration.
Bug Fixes:
Enhancements: