Skip to content

Commit e230aff

Browse files
committed
Merge bitcoin/bitcoin#32396: cmake: Add application manifests when cross-compiling for Windows
8f4fed7 symbol-check: Add check for application manifest in Windows binaries (Hennadii Stepanov) 2bb6ab8 ci: Add "Get bitcoind manifest" steps to Windows CI jobs (Hennadii Stepanov) 282b491 cmake: Add application manifests when cross-compiling for Windows (Hennadii Stepanov) Pull request description: Windows [application manifests ](https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests) provide several benefits—such as enhanced security settings, and the ability to set a process-wide code page (required for bitcoin/bitcoin#32380), as well as granular control over supported Windows versions. Most of these benefits lie beyond the scope of this PR and will be evaluated separately. On the current master branch @ fc6346d, the linker generates and embeds a manifest only when building with MSVC: ```xml <?xml version="1.0" encoding="UTF-8" standalone="yes"?> <assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0"> <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3"> <security> <requestedPrivileges> <requestedExecutionLevel level="asInvoker" uiAccess="false"></requestedExecutionLevel> </requestedPrivileges> </security> </trustInfo> </assembly> ``` However, this manifest fails validation: ``` > mt.exe -nologo -inputresource:build\bin\Release\bitcoind.exe -validate_manifest mt.exe : general error 10100ba: The manifest is missing the definition identity. ``` This PR unifies manifest embedding for both native and cross-compilation builds. Here is the change in the manifest on Windows: ```diff --- bitcoind-master.manifest +++ bitcoind-pr.manifest @@ -1,5 +1,6 @@ <?xml version="1.0" encoding="UTF-8" standalone="yes"?> <assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0"> + <assemblyIdentity type="win32" name="org.bitcoincore.bitcoind" version="29.99.0.0"></assemblyIdentity> <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3"> <security> <requestedPrivileges> ``` which effectively resolves the "missing the definition identity" error. Finally, “Get bitcoind manifest” steps have been added to the Windows CI jobs to ensure the manifest is embedded and validated. ACKs for top commit: sipsorcery: re-tACK 8f4fed7. hodlinator: re-ACK 8f4fed7 davidgumberg: Reviewed and tested ACK bitcoin/bitcoin@8f4fed7 Tree-SHA512: 6e2dbdc77083eafdc242410eb89a6678e37b11efd786505dcd7844f0bac8f44d68625e62924a03b26549bdb4aaec5330dc608e6b4d66789f0255092e23aef6cb
2 parents 51be79c + 8f4fed7 commit e230aff

File tree

8 files changed

+74
-2
lines changed

8 files changed

+74
-2
lines changed

.github/workflows/ci.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,15 @@ jobs:
237237
run: |
238238
cmake --build . -j $NUMBER_OF_PROCESSORS --config Release
239239
240+
- name: Get bitcoind manifest
241+
if: matrix.job-type == 'standard'
242+
working-directory: build
243+
run: |
244+
mt.exe -nologo -inputresource:bin/Release/bitcoind.exe -out:bitcoind.manifest
245+
cat bitcoind.manifest
246+
echo
247+
mt.exe -nologo -inputresource:bin/Release/bitcoind.exe -validate_manifest
248+
240249
- name: Run test suite
241250
if: matrix.job-type == 'standard'
242251
working-directory: build
@@ -347,6 +356,20 @@ jobs:
347356
- name: Run bitcoind.exe
348357
run: ./bin/bitcoind.exe -version
349358

359+
- name: Find mt.exe tool
360+
shell: pwsh
361+
run: |
362+
$sdk_dir = (Get-ItemProperty 'HKLM:\SOFTWARE\Wow6432Node\Microsoft\Windows Kits\Installed Roots' -Name KitsRoot10).KitsRoot10
363+
$sdk_latest = (Get-ChildItem "$sdk_dir\bin" -Directory | Where-Object { $_.Name -match '^\d+\.\d+\.\d+\.\d+$' } | Sort-Object Name -Descending | Select-Object -First 1).Name
364+
"MT_EXE=${sdk_dir}bin\${sdk_latest}\x64\mt.exe" >> $env:GITHUB_ENV
365+
366+
- name: Get bitcoind manifest
367+
shell: pwsh
368+
run: |
369+
& $env:MT_EXE -nologo -inputresource:bin\bitcoind.exe -out:bitcoind.manifest
370+
Get-Content bitcoind.manifest
371+
& $env:MT_EXE -nologo -inputresource:bin\bitcoind.exe -validate_manifest
372+
350373
- name: Run unit tests
351374
# Can't use ctest here like other jobs as we don't have a CMake build tree.
352375
run: |

CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ if(WIN32)
276276
/Zc:__cplusplus
277277
/sdl
278278
)
279+
target_link_options(core_interface INTERFACE
280+
# We embed our own manifests.
281+
/MANIFEST:NO
282+
)
279283
# Improve parallelism in MSBuild.
280284
# See: https://devblogs.microsoft.com/cppblog/improved-parallelism-in-msbuild/.
281285
list(APPEND CMAKE_VS_GLOBALS "UseMultiToolTask=true")

cmake/module/AddWindowsResources.cmake

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,24 @@
44

55
include_guard(GLOBAL)
66

7-
macro(add_windows_resources target rc_file)
7+
function(add_windows_resources target rc_file)
88
if(WIN32)
99
target_sources(${target} PRIVATE ${rc_file})
1010
set_property(SOURCE ${rc_file}
1111
APPEND PROPERTY COMPILE_DEFINITIONS WINDRES_PREPROC
1212
)
1313
endif()
14-
endmacro()
14+
endfunction()
15+
16+
# Add a fusion manifest to Windows executables.
17+
# See: https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests
18+
function(add_windows_application_manifest target)
19+
if(WIN32)
20+
configure_file(${PROJECT_SOURCE_DIR}/cmake/windows-app.manifest.in ${target}.manifest USE_SOURCE_PERMISSIONS)
21+
file(CONFIGURE
22+
OUTPUT ${target}-manifest.rc
23+
CONTENT "1 /* CREATEPROCESS_MANIFEST_RESOURCE_ID */ 24 /* RT_MANIFEST */ \"${target}.manifest\""
24+
)
25+
add_windows_resources(${target} ${CMAKE_CURRENT_BINARY_DIR}/${target}-manifest.rc)
26+
endif()
27+
endfunction()

cmake/windows-app.manifest.in

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
2+
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
3+
<assemblyIdentity
4+
type="win32"
5+
name="org.bitcoincore.${target}"
6+
version="${CLIENT_VERSION_MAJOR}.${CLIENT_VERSION_MINOR}.${CLIENT_VERSION_BUILD}.0"
7+
/>
8+
<trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
9+
<security>
10+
<requestedPrivileges>
11+
<requestedExecutionLevel level="asInvoker" uiAccess="false"></requestedExecutionLevel>
12+
</requestedPrivileges>
13+
</security>
14+
</trustInfo>
15+
</assembly>

contrib/guix/symbol-check.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,14 @@ def check_PE_subsystem_version(binary) -> bool:
278278
return True
279279
return False
280280

281+
def check_PE_application_manifest(binary) -> bool:
282+
if not binary.has_resources:
283+
# No resources at all.
284+
return False
285+
286+
rm = binary.resources_manager
287+
return rm.has_manifest
288+
281289
def check_ELF_interpreter(binary) -> bool:
282290
expected_interpreter = ELF_INTERPRETER_NAMES[binary.header.machine_type][binary.abstract.header.endianness]
283291

@@ -307,6 +315,7 @@ def check_ELF_ABI(binary) -> bool:
307315
lief.EXE_FORMATS.PE: [
308316
('DYNAMIC_LIBRARIES', check_PE_libraries),
309317
('SUBSYSTEM_VERSION', check_PE_subsystem_version),
318+
('APPLICATION_MANIFEST', check_PE_application_manifest),
310319
]
311320
}
312321

src/CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ if(ENABLE_WALLET)
206206
wallet/wallettool.cpp
207207
)
208208
add_windows_resources(bitcoin-wallet bitcoin-wallet-res.rc)
209+
add_windows_application_manifest(bitcoin-wallet)
209210
target_link_libraries(bitcoin-wallet
210211
core_interface
211212
bitcoin_wallet
@@ -339,6 +340,7 @@ if(BUILD_DAEMON)
339340
init/bitcoind.cpp
340341
)
341342
add_windows_resources(bitcoind bitcoind-res.rc)
343+
add_windows_application_manifest(bitcoind)
342344
target_link_libraries(bitcoind
343345
core_interface
344346
bitcoin_node
@@ -392,6 +394,7 @@ target_link_libraries(bitcoin_cli
392394
if(BUILD_CLI)
393395
add_executable(bitcoin-cli bitcoin-cli.cpp)
394396
add_windows_resources(bitcoin-cli bitcoin-cli-res.rc)
397+
add_windows_application_manifest(bitcoin-cli)
395398
target_link_libraries(bitcoin-cli
396399
core_interface
397400
bitcoin_cli
@@ -407,6 +410,7 @@ endif()
407410
if(BUILD_TX)
408411
add_executable(bitcoin-tx bitcoin-tx.cpp)
409412
add_windows_resources(bitcoin-tx bitcoin-tx-res.rc)
413+
add_windows_application_manifest(bitcoin-tx)
410414
target_link_libraries(bitcoin-tx
411415
core_interface
412416
bitcoin_common
@@ -420,6 +424,7 @@ endif()
420424
if(BUILD_UTIL)
421425
add_executable(bitcoin-util bitcoin-util.cpp)
422426
add_windows_resources(bitcoin-util bitcoin-util-res.rc)
427+
add_windows_application_manifest(bitcoin-util)
423428
target_link_libraries(bitcoin-util
424429
core_interface
425430
bitcoin_common

src/qt/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ add_executable(bitcoin-qt
256256
)
257257

258258
add_windows_resources(bitcoin-qt res/bitcoin-qt-res.rc)
259+
add_windows_application_manifest(bitcoin-qt)
259260

260261
target_link_libraries(bitcoin-qt
261262
core_interface

src/test/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,8 @@ target_raw_data_sources(test_bitcoin NAMESPACE test::data
142142
data/asmap.raw
143143
)
144144

145+
add_windows_application_manifest(test_bitcoin)
146+
145147
target_link_libraries(test_bitcoin
146148
core_interface
147149
test_util

0 commit comments

Comments
 (0)