Run dumpbin.exe /imports on the cross-built bitcoind.exe in the "Windows, test cross-built" jobs to list the imported DLLs and to ensure the executable is linked against the expected C runtime.
This came up during a discussion in #35877 (here and here). Inspired by the analogous CI steps in https://github.com/hebasto/bitcoin-core-nightly.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35931.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline and AI policy for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | maflcko, jeanpablojp |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
Run `dumpbin.exe /imports` on the cross-built `bitcoind.exe` in the
"Windows, test cross-built" jobs to list the imported DLLs and to
ensure the executable is linked against the expected C runtime.
lgtm ACK 67fee5bf440c24eb708bfc40da46c23595e5a4f4
ACK 67fee5bf440c24eb708bfc40da46c23595e5a4f4
I have tested the code: I ran check_imports from the branch against a stand-in dumpbin.exe fed with this PR's own CI output (run 31188572449). The parsed list matches what both jobs printed, and the cases CI does not reach fail as they should: wrong CRT either way, both runtimes at once, and an empty DLL list all exit 1. So the check fails closed if dumpbin's output format ever changes.
43 | + dlls = {name.lower() for name in dlls} 44 | + uses_msvcrt = "msvcrt.dll" in dlls 45 | + uses_ucrt = any(name.startswith("api-ms-win-crt-") for name in dlls) 46 | + crt = os.environ["CRT"] 47 | + if crt == "msvcrt": 48 | + crt_ok = uses_msvcrt and not uses_ucrt
Can any of these mixed runtime conditions actually happen? These checks seem a bit convoluted.
Can any of these mixed runtime conditions actually happen?
Maybe due to a bug in one of the build tools?
Maybe due to a bug in one of the build tools?
Ok, so these are "tests" for a broken system packager, or OS? That seems a bit out of scope here.
The test is intended to verify the list of linked libraries against the expected subset, without enumerating the exact causes of a possible mismatch.
47 | + if crt == "msvcrt": 48 | + crt_ok = uses_msvcrt and not uses_ucrt 49 | + elif crt == "ucrt": 50 | + crt_ok = uses_ucrt and not uses_msvcrt 51 | + else: 52 | + sys.exit(f"Unexpected CRT value: {crt!r}")
How could this happen?
For example, a typo when updating this code: https://github.com/bitcoin/bitcoin/blob/1f5c46f7ce2527b926a1517e8901dbccf1503560/.github/workflows/ci.yml#L376-L382
Ok. Maybe we can write these checks in a way where the crt values are used, so any problem is a runtime failure, rather than additional code to check for typos.
Perhaps someone could address this in a follow-up?