cmake: Use builtin support for .manifest files #33585

purpleKarrot commented at 3:44 PM on October 9, 2025: contributor

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.

DrahtBot added the label Build system on Oct 9, 2025

DrahtBot commented at 3:44 PM on October 9, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33585.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	hebasto, 151henry151

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31507 (build: Use clang-cl to build on Windows natively by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake commented at 4:04 PM on October 9, 2025: member

This doesn't Guix build:

[100%] Built target bitcoin-qt
Running symbol and dynamic library checks...
/distsrc-base/distsrc-3277798895d9-x86_64-w64-mingw32/build/bin/bitcoin.exe: failed APPLICATION_MANIFEST
/distsrc-base/distsrc-3277798895d9-x86_64-w64-mingw32/build/bin/bitcoind.exe: failed APPLICATION_MANIFEST
/distsrc-base/distsrc-3277798895d9-x86_64-w64-mingw32/build/bin/bitcoin-qt.exe: failed APPLICATION_MANIFEST
/distsrc-base/distsrc-3277798895d9-x86_64-w64-mingw32/build/bin/bitcoin-cli.exe: failed APPLICATION_MANIFEST
/distsrc-base/distsrc-3277798895d9-x86_64-w64-mingw32/build/bin/bitcoin-tx.exe: failed APPLICATION_MANIFEST
/distsrc-base/distsrc-3277798895d9-x86_64-w64-mingw32/build/bin/bitcoin-util.exe: failed APPLICATION_MANIFEST
/distsrc-base/distsrc-3277798895d9-x86_64-w64-mingw32/build/bin/bitcoin-wallet.exe: failed APPLICATION_MANIFEST
/distsrc-base/distsrc-3277798895d9-x86_64-w64-mingw32/build/bin/test_bitcoin.exe: failed APPLICATION_MANIFEST
make[3]: *** [CMakeFiles/check-symbols.dir/build.make:71: CMakeFiles/check-symbols] Error 1
make[2]: *** [CMakeFiles/Makefile2:388: CMakeFiles/check-symbols.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:395: CMakeFiles/check-symbols.dir/rule] Error 2
make: *** [Makefile:179: check-symbols] Error 2

purpleKarrot force-pushed on Oct 9, 2025

purpleKarrot commented at 7:23 PM on October 9, 2025: contributor

This doesn't Guix build:

I added a workaround for upstream issue 23244. It should pass now.

hebasto commented at 7:50 AM on October 10, 2025: member

I agree on the refactoring first and last commits.

The mentioned upstream issue was exactly the reason why I implemented manifest handling this way in #32396.

Is there any CMake's docs regarding handling .manifest files?

purpleKarrot commented at 8:10 AM on October 10, 2025: contributor

The mentioned upstream issue was exactly the reason why I implemented manifest handling this way in #32396.

Got it. I think the advantage of using the workaround conditionally with a comment to the upstream issue is that it is self-documenting and simplifies future refactoring once the issue is solved upstream. If the workaround is used unconditionally for all platforms, it is not obvious to future contributors that this is in fact a workaround, and they may fall into the same trap as I did.

Is there any CMake's docs regarding handling .manifest files?

https://cmake.org/cmake/help/latest/release/3.4.html#other

CMake itself uses a similar condition: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/11112/diffs

hebasto commented at 11:16 AM on October 10, 2025: member

Is there any CMake's docs regarding handling .manifest files?

https://cmake.org/cmake/help/latest/release/3.4.html#other

Thank you for the link!

The release notes mentioned above state:

Manifest files ... will be merged with linker-generated manifests and embedded in the binary.

This PR enables linker-generated manifests by removing the /MANIFEST:NO linker flag. However, I still have a couple of questions:

What is the content of the linker-generated manifest? What is the logic behind merging ${target}.manifest with a linker-generated manifest? Why do we need linker-generated manifests?
I'm not entirely sure that any fix of https://gitlab.kitware.com/cmake/cmake/-/issues/23244 will follow the same logic for MinGW as for MSVC. Wouldn't it be more prudent to keep the MSVC-specific /MANIFEST:NO linker flag and maintain a single code path for both MinGW as for MSVC in the add_windows_application_manifest() function?

fanquake commented at 10:18 AM on February 9, 2026: member

However, I still have a couple of questions: @purpleKarrot are you able to followup to the Qs here?

purpleKarrot commented at 1:59 PM on February 9, 2026: contributor

What is the content of the linker-generated manifest? What is the logic behind merging ${target}.manifest with a linker-generated manifest? Why do we need linker-generated manifests?

I am not a Windows expert, but according to my research, the linker generated manifest encodes runtime dependencies and compatibility metadata. Merging ${target}.manifest with it allows us to set custom entries while preserving the required runtime binding. Without the linker-generated manifest, it depends on the runtime environment which CRT will be used.

I'm not entirely sure that any fix of https://gitlab.kitware.com/cmake/cmake/-/issues/23244 will follow the same logic for MinGW as for MSVC.

I don't think it could be considered a fix if it does not follow the same logic.

Wouldn't it be more prudent to keep the MSVC-specific /MANIFEST:NO linker flag and maintain a single code path for both MinGW as for MSVC in the add_windows_application_manifest() function?

I don't think so. The two toolchains already differ in ABI, CRT, exception handling, TLS, and linker semantics. Disabling manifests does not normalize any of that. All it does is remove explicit dependency metadata on Windows and force the loader to fall back to legacy, environment-dependent resolution. That reduces determinism even within the same toolchain, let alone across different ones.

in cmake/module/AddWindowsResources.cmake:14 in 58db133cb6 outdated

  17 | -    configure_file(${PROJECT_SOURCE_DIR}/cmake/windows-app.manifest.in ${target}.manifest USE_SOURCE_PERMISSIONS)
  18 | +  configure_file(${PROJECT_SOURCE_DIR}/cmake/windows-app.manifest.in ${target}.manifest)
  19 | +  if(MSVC)
  20 | +    target_sources(${target} PRIVATE ${target}.manifest)
  21 | +  else()
  22 | +    # TODO: Remove when upstream issue is fixed:

fanquake commented at 7:07 AM on March 23, 2026:

I can't tell if https://gitlab.kitware.com/cmake/cmake/-/issues/23244 was fixed by https://gitlab.kitware.com/cmake/cmake/-/merge_requests/11112, or not, given the issue is still open. If it was, then we can update this was a CMake version for which the workaround can be removed.

purpleKarrot commented at 3:42 PM on March 26, 2026:

It is not a fix for CMake users. It is just the same workaround for CMake's own codebase.

fanquake commented at 4:05 PM on March 26, 2026:

Ok, so https://gitlab.kitware.com/cmake/cmake/-/work_items/23244 remains unfixed.

DrahtBot added the label Needs rebase on Mar 30, 2026

sedited commented at 8:30 AM on June 5, 2026: contributor

@purpleKarrot can you rebase this?

purpleKarrot force-pushed on Jun 5, 2026

DrahtBot removed the label Needs rebase on Jun 5, 2026

maflcko added the label DrahtBot Guix build requested on Jul 21, 2026

DrahtBot commented at 11:07 PM on July 21, 2026: contributor

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File	commit b36c2d78a3ad4940e1d5eb466b0e6650b1001ee3<br>(master)	commit 2db33afa322c73642aaccf184924c23c14aaaa72<br>(pull/33585/merge)
*-aarch64-linux-gnu-debug.tar.gz	`6128720f5624239e...`	`72e4e70ea34bd005...`
*-aarch64-linux-gnu.tar.gz	`80deb1d91e896c04...`	`e7f87f6a534d6415...`
*-arm-linux-gnueabihf-debug.tar.gz	`8fe53c633f8b343b...`	`4c622d3544242fcc...`
*-arm-linux-gnueabihf.tar.gz	`26311f6942c6d252...`	`bc65498a7dcc2114...`
*-arm64-apple-darwin-codesigning.tar.gz	`667ce231d435ca7b...`	`9321d70b0c86880d...`
*-arm64-apple-darwin-unsigned.tar.gz	`18be5faecfabb8dd...`	`a6527d1102481202...`
*-arm64-apple-darwin-unsigned.zip	`45fa4aff38c3a7e5...`	`2e2b661caaece2be...`
*-powerpc64-linux-gnu-debug.tar.gz	`bb9f1bc3813b2f95...`	`8ce7ce0b795a0fee...`
*-powerpc64-linux-gnu.tar.gz	`c15be676f484ca6d...`	`3cad2c4bc11a8404...`
*-riscv64-linux-gnu-debug.tar.gz	`3930acbb4431117d...`	`81a412b7422f78aa...`
*-riscv64-linux-gnu.tar.gz	`e097d9332be88ef4...`	`44207e53859a7821...`
*-win64-codesigning.tar.gz	`d4135b3495c68457...`	`0c7c56e0c7766f6c...`
*-win64-debug.zip	`6d0dad6f35d48310...`	`4194af6c81e362db...`
*-win64-setup-unsigned.exe	`e1d25ad01a7a4f10...`	`f20a6f7e0f605283...`
*-win64-unsigned.zip	`88a96661a9f51a05...`	`22c88a16c97c0f66...`
*-x86_64-apple-darwin-codesigning.tar.gz	`371cb8c51da31987...`	`b50fe31748ee2267...`
*-x86_64-apple-darwin-unsigned.tar.gz	`b5cf1dd2abc032c8...`	`682bbd0d43538025...`
*-x86_64-apple-darwin-unsigned.zip	`658c69b4d293e9db...`	`70295edf8024223e...`
*-x86_64-linux-gnu-debug.tar.gz	`7f893e0ece08a3d2...`	`0abf5dd40f9a7695...`
*-x86_64-linux-gnu.tar.gz	`e90434b160e05674...`	`7ded5d664acb3d5f...`
*.tar.gz	`c9e46fbe8fd423f5...`	`bf28c284c0f53b7e...`
SHA256SUMS.part	`21f78bd45146f5b8...`	`1c2a42d485f9a751...`
guix_build.log	`00500b67f7e4cc1b...`	`f66c28868bf3e26b...`
guix_build.log.diff		`f4aa40351d99b50d...`

DrahtBot removed the label DrahtBot Guix build requested on Jul 21, 2026

purpleKarrot force-pushed on Jul 27, 2026

purpleKarrot commented at 2:34 PM on July 27, 2026: contributor

@purpleKarrot can you rebase this?

Done.

151henry151 commented at 9:16 PM on July 27, 2026: contributor

Tested ACK 13e6429a7d2fa258e6dac270a414c4b48a39e64c

Reviewed the three commits. Linux Release build is fine with the unconditional .rc / generated *-manifest.rc sources (CMake does not compile them on non-WIN32). CI mingw artifacts and a local Guix HOSTS=x86_64-w64-mingw32 build both pass APPLICATION_MANIFEST / symbol-check for the main apps, including bitcoin-qt and test_bitcoin. Dumped Guix and CI mingw PE manifests keep activeCodePage UTF-8, asInvoker, and assemblyIdentity from windows-app.manifest.in.

On the MSVC path, Windows native VS CI’s check_manifests on this tip (run 30275700389) dumps the embedded bitcoind.exe manifest via mt.exe (other exes are only -validate_manifest’d). That dump contains exactly our windows-app.manifest.in entries — assemblyIdentity, asInvoker, UTF-8 activeCodePage — with no linker-added sections in this configuration (expected — the modern CRT isn’t a SxS assembly, so the default linker manifest overlaps our template). That’s consistent with purpleKarrot’s Feb 9 explanation and the CMake 3.4 release-note behavior (custom manifests merge with linker-generated ones). The documented MinGW .rc workaround for CMake #23244 looks like the right tradeoff versus a single .rc path for both toolchains.

Guix build (<code>HOSTS=x86_64-w64-mingw32</code>)

x86_64
c56f846e1411e094ac9757ab781a7a3819d354c476f5fa22f9ff4a1f9d1d6e25  guix-build-13e6429a7d2f/output/dist-archive/bitcoin-13e6429a7d2f.tar.gz
2b81e4a1ad4bb46cc8953dcd4394d7793ac9e063735526cac5519bd14d339840  guix-build-13e6429a7d2f/output/x86_64-w64-mingw32/SHA256SUMS.part
7aa1e645cf4c00307aa3b37dca66b7e58d0f38de6f8e6334bba6b013aa6eee38  guix-build-13e6429a7d2f/output/x86_64-w64-mingw32/bitcoin-13e6429a7d2f-win64-codesigning.tar.gz
87451bc502e9f7d26846209d975e0209bb9843725a91fe00b8b2f7c26880d9a1  guix-build-13e6429a7d2f/output/x86_64-w64-mingw32/bitcoin-13e6429a7d2f-win64-debug.zip
82aeefc04ab22c96046790a311b90745854c82382504e13b91f8a084dd4d2d81  guix-build-13e6429a7d2f/output/x86_64-w64-mingw32/bitcoin-13e6429a7d2f-win64-setup-unsigned.exe
85346641763ec0c6dd8cba3d29bdb16ce9e30ef2e4836254dcb62691571bad6e  guix-build-13e6429a7d2f/output/x86_64-w64-mingw32/bitcoin-13e6429a7d2f-win64-unsigned.zip

in cmake/module/AddWindowsResources.cmake:10 in 13e6429a7d

  13 |  # Add a fusion manifest to Windows executables.
  14 |  # See: https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests
  15 |  function(add_windows_application_manifest target)
  16 | -  if(WIN32)
  17 | -    configure_file(${PROJECT_SOURCE_DIR}/cmake/windows-app.manifest.in ${target}.manifest USE_SOURCE_PERMISSIONS)
  18 | +  configure_file(${PROJECT_SOURCE_DIR}/cmake/windows-app.manifest.in ${target}.manifest)

hebasto commented at 1:57 PM on August 10, 2026:

Some time ago, we agreed to explicitly specify NO_SOURCE_PERMISSIONS or USE_SOURCE_PERMISSIONS (see here and #30823). Let's keep this convention:

  configure_file(${PROJECT_SOURCE_DIR}/cmake/windows-app.manifest.in ${target}.manifest USE_SOURCE_PERMISSIONS)

purpleKarrot commented at 2:17 PM on August 10, 2026:

OK, done.

hebasto commented at 1:58 PM on August 10, 2026: member

I've reviewed the current branch @ 13e6429a7d2fa258e6dac270a414c4b48a39e64c.

Almost ACK.

The build tool command lines change only when building on Window. And the resulting manifests are semantically the same as on the master branch.

cmake: Unconditionally set WIN32_EXECUTABLE target property

The property only has an effect when building for WIN32.
Checking for WIN32 before setting the property is redundant.

8f695379f3

cmake: Use builtin support for .manifest files

CMake ignores .rc and .manifest files when not building for WIN32.

654a4cf5e6

cmake: Unconditionally add .rc files to sources

CMake ignores .rc files when compiling for non-Windows platform.
Checking for WIN32 before adding an .rc file to sources is redundant.

99497b38f6

purpleKarrot force-pushed on Aug 10, 2026

hebasto commented at 2:53 PM on August 10, 2026: member

My partial Guix build:

x86_64
5f4fcdf6d16b0956dce32f3e62d603754c6ae2f1c0147cb5988e12eddd9f6072  guix-build-99497b38f6c6/output/dist-archive/bitcoin-99497b38f6c6.tar.gz
241f5f49e24d9ba4415689ca7da36292c42fad507d9a4ae3b6427d5deb2b9dd0  guix-build-99497b38f6c6/output/x86_64-w64-mingw32/SHA256SUMS.part
2069d17471090e9f748839d894d9a2f67411a99bc1be6471f39b9dc53dca8b54  guix-build-99497b38f6c6/output/x86_64-w64-mingw32/bitcoin-99497b38f6c6-win64-codesigning.tar.gz
98e8e37dbad2423cadf797b637ba96c005f69f5d4f66adde529c4bc86ed82f3e  guix-build-99497b38f6c6/output/x86_64-w64-mingw32/bitcoin-99497b38f6c6-win64-debug.zip
1bb4600a1976703c918ce947f53ff7c478603ebc35b9218f4a06f358404ba993  guix-build-99497b38f6c6/output/x86_64-w64-mingw32/bitcoin-99497b38f6c6-win64-setup-unsigned.exe
793c01c7dd516def1b6f9b6f986940ece4249f99f4c2c7d88e35e2db1e0ef68f  guix-build-99497b38f6c6/output/x86_64-w64-mingw32/bitcoin-99497b38f6c6-win64-unsigned.zip

hebasto approved

hebasto commented at 3:02 PM on August 10, 2026: member

ACK 99497b38f6c66462beaf1663b86029885e800834.

151henry151 commented at 7:27 PM on August 10, 2026: contributor

re-ACK 99497b38f6c66462beaf1663b86029885e800834

Only change since my prior Tested ACK is USE_SOURCE_PERMISSIONS on the configure_file that writes ${target}.manifest, matching the existing convention. That affects intermediate file mode bits only, not the embedded PE manifest content, so I did not re-run Guix.