Use CPack with TARGET_RUNTIME_DLLS for Windows packaging#1897
Use CPack with TARGET_RUNTIME_DLLS for Windows packaging#1897vulcandth wants to merge 1 commit intogbdev:masterfrom
Conversation
ISSOtm
left a comment
ISSOtm
left a comment
There was a problem hiding this comment.
Alright, thank you! I had no idea CPack existed, so this was an occasion to read a lot of documentation ^^
| install(FILES ${${SECTION}} DESTINATION ${DEST}) | ||
| endforeach() | ||
|
|
||
| if(WIN32 AND CMAKE_VERSION VERSION_GREATER_EQUAL "3.21") |
There was a problem hiding this comment.
I'm thinking it wouldn't be a bad idea to enable CPack across all platforms. (In particular, it may be useful to generate our releases' source tarballs, though any tweaking related to that can come in a separate PR.)
| set(CPACK_SYSTEM_NAME "win32") | ||
| endif() | ||
|
|
||
| set(CPACK_GENERATOR "ZIP") |
There was a problem hiding this comment.
Looks like this gets overridden with the cpack -G flag, so probably we should omit this.
| set(CPACK_INSTALL_CMAKE_PROJECTS "${CMAKE_BINARY_DIR};${CMAKE_PROJECT_NAME};runtime;/") | ||
| set(CPACK_STRIP_FILES TRUE) | ||
| set(CPACK_VERBATIM_VARIABLES YES) | ||
| include(CPack) |
There was a problem hiding this comment.
Apparently we can also set some more variables?
| include(CPack) | |
| set(CPACK_PACKAGE_VENDOR "GBDev") | |
| set(CPACK_PACKAGE_DESCRIPTION "An assembly toolchain for the Nintendo Game Boy and Game Boy Color") # Same as our repo's description. | |
| set(CPACK_PACKAGE_HOMEPAGE_URL "https://rgbds.gbdev.io") | |
| set(CPACK_RESOURCE_FILE_LICENSE "LICENSE") | |
| include(CPack) |
Also, I'm noticing that this is not setting the CPACK_PACKAGE_VERSION_* variables, and we're not passing RGBDS' version to project(), meaning the fallback CMAKE_PROJECT_VERSION_* variables aren't defined. I'm thinking we may want to extract the Git ref before running project() so that we can extract the version from it and pass that to project().
(This has led me to a rabbit hole of realising how cursed our version setup is, and now I'm thinking of how the hell to fix that. Stay tuned lol)
| find_package(ZLIB REQUIRED CONFIG) | ||
| find_package(PNG REQUIRED CONFIG) |
There was a problem hiding this comment.
CONFIG disables Module-mode search. What is that required for?
|
|
||
| set(CPACK_GENERATOR "ZIP") | ||
| set(CPACK_INCLUDE_TOPLEVEL_DIRECTORY OFF) | ||
| set(CPACK_PACKAGE_DIRECTORY "${CMAKE_BINARY_DIR}") |
There was a problem hiding this comment.
This appears to be the default behaviour (though I'd need to check, but I don't have a Windows machine handy.)
| set(CPACK_PACKAGE_DIRECTORY "${CMAKE_BINARY_DIR}") |
| endif() | ||
|
|
||
| set(CPACK_GENERATOR "ZIP") | ||
| set(CPACK_INCLUDE_TOPLEVEL_DIRECTORY OFF) |
There was a problem hiding this comment.
AFAICT, this is best left undefined so that e.g. tarballs can be properly generated. (It would be necessary to pass it to generate the Windows zip files, but we can do that with -D on the CLI.)
| set(CPACK_INCLUDE_TOPLEVEL_DIRECTORY OFF) |
| if(CMAKE_SIZEOF_VOID_P EQUAL 8) | ||
| set(CPACK_SYSTEM_NAME "win64") | ||
| else() | ||
| set(CPACK_SYSTEM_NAME "win32") | ||
| endif() |
There was a problem hiding this comment.
This pointer size check looks cursed as hell, hopefully there's a better way to do this...
EDIT: Turns out that CPACK_SYSTEM_NAME has this exact behaviour by default anyway.
| if(CMAKE_SIZEOF_VOID_P EQUAL 8) | |
| set(CPACK_SYSTEM_NAME "win64") | |
| else() | |
| set(CPACK_SYSTEM_NAME "win32") | |
| endif() |
| target_link_libraries(rgbgfx PRIVATE PNG::PNG) | ||
| endif() | ||
|
|
||
| if(WIN32 AND CMAKE_VERSION VERSION_GREATER_EQUAL "3.21") |
| find_package(PkgConfig) | ||
| if(MSVC OR NOT PKG_CONFIG_FOUND) | ||
| if(MSVC) | ||
| find_package(ZLIB REQUIRED CONFIG) |
There was a problem hiding this comment.
Why is looking for zlib separately required? I see FindPNG.cmake calls find_package(ZLIB) on my system.
| target_compile_definitions(${TARGET} PRIVATE ${PNG_DEFINITIONS}) | ||
| target_include_directories(${TARGET} PRIVATE ${PNG_INCLUDE_DIRS}) | ||
| target_link_libraries(${TARGET} PRIVATE ${PNG_LIBRARIES}) | ||
| target_link_libraries(${TARGET} PRIVATE PNG::PNG) |
There was a problem hiding this comment.
Oh, very nice, thank you! We should do the same for the pkg-config path, tbh.
2 participants
Fixes #1896.
Summary
rgbgfxruntime DLLs with$<TARGET_RUNTIME_DLLS:rgbgfx>so the packaged dependencies come from CMake's target metadataZLIBandPNGon MSVC, and update the test helpers to link againstPNG::PNGValidation
cpackto verify the generated ZIP contains the executables plus the discovered runtime DLLsThis work was done by GPT-5.4.