Merge bitcoin/bitcoin#33585: cmake: Use builtin support for .manifest files

99497b38f6 cmake: Unconditionally add .rc files to sources (Daniel Pfeifer)
654a4cf5e6 cmake: Use builtin support for .manifest files (Daniel Pfeifer)
8f695379f3 cmake: Unconditionally set WIN32_EXECUTABLE target property (Daniel Pfeifer)

Pull request description:

  Remove some redundant logic from the CMake code:

  * The `WIN32_EXECUTABLE` target property only has an effect when building for `WIN32`. Checking `WIN32` is redundant.
  * CMake has builtin support for `.rc` and `.manifest` files. Both may be added to sources unconditionally. They only have an effect when building for `WIN32`.

ACKs for top commit:
  151henry151:
    re-ACK 99497b38f6
  hebasto:
    ACK 99497b38f6.

Tree-SHA512: 3d1a6a86feefff58e06d2e692809349f8e5d50e7ded821da775420a6bbfd54baf7dfdba957a0c7e9fc61b87455addfe67a1cc48dbfe4a8e3055f69bb027109c0

CMakeLists.txt

@@ -330,10 +330,6 @@ if(WIN32)
       /Zc:__cplusplus
       /sdl
     )
-    target_link_options(core_interface INTERFACE
-      # We embed our own manifests.
-      /MANIFEST:NO
-    )
     # Improve parallelism in MSBuild.
     # See: https://devblogs.microsoft.com/cppblog/improved-parallelism-in-msbuild/.
     list(APPEND CMAKE_VS_GLOBALS "UseMultiToolTask=true")

cmake/module/AddWindowsResources.cmake

@@ -4,21 +4,19 @@
 
 include_guard(GLOBAL)
 
-function(add_windows_resources target rc_file)
-  if(WIN32)
-    target_sources(${target} PRIVATE ${rc_file})
-  endif()
-endfunction()
-
 # Add a fusion manifest to Windows executables.
 # See: https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests
 function(add_windows_application_manifest target)
-  if(WIN32)
-    configure_file(${PROJECT_SOURCE_DIR}/cmake/windows-app.manifest.in ${target}.manifest USE_SOURCE_PERMISSIONS)
+  configure_file(${PROJECT_SOURCE_DIR}/cmake/windows-app.manifest.in ${target}.manifest USE_SOURCE_PERMISSIONS)
+  if(MSVC)
+    target_sources(${target} PRIVATE ${target}.manifest)
+  else()
+    # TODO: Remove when upstream issue is fixed:
+    # https://gitlab.kitware.com/cmake/cmake/-/issues/23244
     file(CONFIGURE
       OUTPUT ${target}-manifest.rc
       CONTENT "1 /* CREATEPROCESS_MANIFEST_RESOURCE_ID */ 24 /* RT_MANIFEST */ \"${target}.manifest\""
     )
-    add_windows_resources(${target} ${CMAKE_CURRENT_BINARY_DIR}/${target}-manifest.rc)
+    target_sources(${target} PRIVATE ${target}-manifest.rc)
   endif()
 endfunction()

src/CMakeLists.txt

@@ -164,10 +164,10 @@ if(ENABLE_WALLET)
   if(BUILD_WALLET_TOOL)
     add_executable(bitcoin-wallet
       bitcoin-wallet.cpp
+      bitcoin-wallet-res.rc
       init/bitcoin-wallet.cpp
       wallet/wallettool.cpp
     )
-    add_windows_resources(bitcoin-wallet bitcoin-wallet-res.rc)
     add_windows_application_manifest(bitcoin-wallet)
     target_link_libraries(bitcoin-wallet
       core_interface
@@ -302,8 +302,10 @@ endif()
 
 # Bitcoin wrapper executable that can call other executables.
 if(BUILD_BITCOIN_BIN)
-  add_executable(bitcoin bitcoin.cpp)
-  add_windows_resources(bitcoin bitcoin-res.rc)
+  add_executable(bitcoin
+    bitcoin.cpp
+    bitcoin-res.rc
+  )
   add_windows_application_manifest(bitcoin)
   target_link_libraries(bitcoin core_interface bitcoin_common bitcoin_util)
   install_binary_component(bitcoin HAS_MANPAGE)
@@ -313,9 +315,9 @@ endif()
 if(BUILD_DAEMON)
   add_executable(bitcoind
     bitcoind.cpp
+    bitcoind-res.rc
     init/bitcoind.cpp
   )
-  add_windows_resources(bitcoind bitcoind-res.rc)
   add_windows_application_manifest(bitcoind)
   target_link_libraries(bitcoind
     core_interface
@@ -352,8 +354,11 @@ target_link_libraries(bitcoin_cli
 
 # Bitcoin Core RPC client
 if(BUILD_CLI)
-  add_executable(bitcoin-cli bitcoin-cli.cpp init/basic.cpp)
-  add_windows_resources(bitcoin-cli bitcoin-cli-res.rc)
+  add_executable(bitcoin-cli
+    bitcoin-cli.cpp
+    bitcoin-cli-res.rc
+    init/basic.cpp
+  )
   add_windows_application_manifest(bitcoin-cli)
   target_link_libraries(bitcoin-cli
     core_interface
@@ -367,8 +372,10 @@ endif()
 
 
 if(BUILD_TX)
-  add_executable(bitcoin-tx bitcoin-tx.cpp)
-  add_windows_resources(bitcoin-tx bitcoin-tx-res.rc)
+  add_executable(bitcoin-tx
+    bitcoin-tx.cpp
+    bitcoin-tx-res.rc
+  )
   add_windows_application_manifest(bitcoin-tx)
   target_link_libraries(bitcoin-tx
     core_interface
@@ -381,8 +388,10 @@ endif()
 
 
 if(BUILD_UTIL)
-  add_executable(bitcoin-util bitcoin-util.cpp)
-  add_windows_resources(bitcoin-util bitcoin-util-res.rc)
+  add_executable(bitcoin-util
+    bitcoin-util.cpp
+    bitcoin-util-res.rc
+  )
   add_windows_application_manifest(bitcoin-util)
   target_link_libraries(bitcoin-util
     core_interface

src/qt/CMakeLists.txt

@@ -255,12 +255,12 @@ endif()
 
 add_subdirectory(locale)
 
-add_executable(bitcoin-qt
+add_executable(bitcoin-qt WIN32
   main.cpp
+  res/bitcoin-qt-res.rc
   ../init/bitcoin-qt.cpp
 )
 
-add_windows_resources(bitcoin-qt res/bitcoin-qt-res.rc)
 add_windows_application_manifest(bitcoin-qt)
 
 target_link_libraries(bitcoin-qt
@@ -271,12 +271,9 @@ target_link_libraries(bitcoin-qt
 
 import_plugins(bitcoin-qt)
 install_binary_component(bitcoin-qt HAS_MANPAGE)
-if(WIN32)
-  set_target_properties(bitcoin-qt PROPERTIES WIN32_EXECUTABLE TRUE)
-endif()
 
 if(ENABLE_IPC)
-  add_executable(bitcoin-gui
+  add_executable(bitcoin-gui WIN32
     main.cpp
     ../init/bitcoin-gui.cpp
   )
@@ -288,9 +285,6 @@ if(ENABLE_IPC)
   )
   import_plugins(bitcoin-gui)
   install_binary_component(bitcoin-gui INTERNAL)
-  if(WIN32)
-    set_target_properties(bitcoin-gui PROPERTIES WIN32_EXECUTABLE TRUE)
-  endif()
 endif()
 
 if(BUILD_GUI_TESTS)