Move Win32 defines to configure.ac to ensure they are globally defined #15704

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:win32_defines_globally changing 6 files +2 −17

luke-jr commented at 9:11 AM on March 30, 2019: member

#9245 no longer needs this, since the main _WIN32_WINNT got bumped by something else.

So rather than just lose it, might as well get it merged in independently.

I'm not aware of any practical effects, but it seems safer to use the same API versions everywhere.
fanquake added the label Windows on Mar 30, 2019
fanquake added the label Build system on Mar 30, 2019
laanwj commented at 9:53 AM on March 30, 2019: member

I think this needs analogous MSVC build system changes.
luke-jr force-pushed on Mar 30, 2019
luke-jr commented at 12:48 PM on March 30, 2019: member

Fixed
DrahtBot commented at 1:36 PM on March 30, 2019: member


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #18427 (Bugfix? Restore linking to libmingwthrd by luke-jr)
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.
MarcoFalke added the label Needs gitian build on Mar 30, 2019
MarcoFalke renamed this:
~~Move Win32 defines to configure.ac to ensure they are globally defined~~
build: Move Win32 defines to configure.ac to ensure they are globally defined
on Mar 30, 2019
MarcoFalke renamed this:
~~build: Move Win32 defines to configure.ac to ensure they are globally defined~~
Move Win32 defines to configure.ac to ensure they are globally defined
on Mar 30, 2019
DrahtBot removed the label Needs gitian build on Mar 31, 2019
sipsorcery commented at 8:43 PM on April 1, 2019: member

tACK e38c1940ff45a155f8bb89ed102dbd2fe1b770dc with msvc x64.

(haven't tested the mingw build)
practicalswift commented at 7:33 AM on April 2, 2019: contributor

FWIW: I've verified that a disassembly of the bitcoind binary built on a Ubuntu 18.04 machine with this patch applied is identical to a disassembly of the bitcoind binary built against master (as expected).
fanquake requested review from ken2812221 on Apr 3, 2019
DrahtBot added the label Needs rebase on Jun 19, 2019
MarcoFalke deleted a comment on Sep 30, 2019
MarcoFalke commented at 12:46 PM on September 30, 2019: member

@luke-jr Needs rebase
luke-jr force-pushed on Oct 14, 2019
luke-jr commented at 9:04 PM on October 14, 2019: member

Rebased
DrahtBot removed the label Needs rebase on Oct 14, 2019
fanquake added the label Needs gitian build on Jan 11, 2020
DrahtBot removed the label Needs gitian build on Jan 11, 2020
DrahtBot added the label Needs rebase on Feb 10, 2020
fanquake commented at 4:54 AM on March 13, 2020: member

Concept ACK
luke-jr force-pushed on Mar 26, 2020
DrahtBot removed the label Needs rebase on Mar 26, 2020
luke-jr commented at 12:46 PM on March 26, 2020: member

Rebased (macOS native Travis failing is "normal" now, right?)
MarcoFalke deleted a comment on Mar 26, 2020
MarcoFalke added the label Needs gitian build on Mar 26, 2020
hebasto commented at 1:07 PM on March 26, 2020: member

@luke-jr

... macOS native Travis failing is "normal" now, right?

Fixed in #18438.
MarcoFalke commented at 2:11 PM on March 26, 2020: member

Open-Close to re-run ci. See #15847 (comment)
MarcoFalke closed this on Mar 26, 2020
MarcoFalke reopened this on Mar 26, 2020

DrahtBot commented at 4:31 PM on March 28, 2020: member

Gitian builds

File	commit 7f9dedb22dcd9550ca525c0e35fec38b2d59e029<br>(master)	commit 9f3c700e6c4c0e3207b4e7600676687e7840bd90<br>(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz	`8aa34845a916014d...`	`23303000f0b5188f...`
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz	`0fb3dd16db30ea00...`	`0c27113827028b32...`
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz	`797f185a8e62f6fe...`	`f1fc0efc24d7ae81...`
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz	`871094b510da0ac7...`	`8b57425f10b09806...`
bitcoin-0.19.99-osx-unsigned.dmg	`a159b1b99b607907...`	`0412b6088a2a57d1...`
bitcoin-0.19.99-osx64.tar.gz	`1fb6370553afcb22...`	`282732517f302b7b...`
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz	`0b86916151aea5a3...`	`503a1e400eb736f1...`
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz	`4efb63771485711b...`	`1c6b0d8864e2a125...`
bitcoin-0.19.99-win64-debug.zip	`dd825e6168d39c90...`	`da019700489cbb7f...`
bitcoin-0.19.99-win64-setup-unsigned.exe	`d1f790ad8c257da0...`	`75715436f1e5202d...`
bitcoin-0.19.99-win64.zip	`49dc4119ae2a01c4...`	`01c54170da20557e...`
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz	`2a144f502390b6e3...`	`5e9967340800b007...`
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz	`af6a5d2d555569a4...`	`b723bcca07a16af3...`
bitcoin-0.19.99.tar.gz	`900859565b6ee1fe...`	`f2b3f76cf3dca4b3...`
bitcoin-core-linux-0.20-res.yml	`70c720b679a19c93...`	`a20fc7454865e0f0...`
bitcoin-core-osx-0.20-res.yml	`5766a2f7c8d8547a...`	`0e18bba67b5f4ffd...`
bitcoin-core-win-0.20-res.yml	`a95a890f0d0cb882...`	`f81ff95c1bb4bb04...`
linux-build.log	`8510c43606df6b17...`	`bf7cdc495ec6276c...`
osx-build.log	`b2fce58471126628...`	`317e66cb99d4dcbc...`
win-build.log	`630e3d23ee0657f4...`	`949ca0d5dc8aca2f...`
bitcoin-core-linux-0.20-res.yml.diff		`fd82f4133c1676fb...`
bitcoin-core-osx-0.20-res.yml.diff		`c79b3e99c65bfa8e...`
bitcoin-core-win-0.20-res.yml.diff		`728981f0a31171d4...`
linux-build.log.diff		`e4719bcf8c1fe6a2...`
osx-build.log.diff		`89b8fdc525014cca...`
win-build.log.diff		`2b0aebdbb53a8ca7...`

DrahtBot removed the label Needs gitian build on Mar 28, 2020
DrahtBot added the label Needs rebase on May 12, 2020
Move Win32 defines to configure.ac to ensure they are globally defined
common.vcxproj used for MSVC builds
1ccb9f30c0
luke-jr commented at 5:55 PM on August 20, 2020: member

Rebased
luke-jr force-pushed on Aug 20, 2020
DrahtBot removed the label Needs rebase on Aug 20, 2020
fanquake approved

fanquake commented at 3:52 AM on August 25, 2020: member

ACK 1ccb9f30c040daf688f89f0d63e9f5e7b131d193 - checked that the binaries produced are the same.

If anyone wants to read into the origins/effects of these definitions, here are a couple articles: Where did WIN32_LEAN_AND_MEAN come from? What's the difference between WINVER, _WIN32_WINNT, _WIN32_WINDOWS, and _WIN32_IE?.

While reviewing I did check the time & lines saved in preprocessing by passing WIN32_LEAN_AND_MEAN. i.e:

#include <windows.h>

int main() {
	return 0;
}

# x86_64-w64-mingw32-g++ (GCC) 10-win32 20200525
# mingw-w64 7.0.0

time x86_64-w64-mingw32-g++ -E test.cpp | wc -l
80856 lines
avg time of 0.40s

time x86_64-w64-mingw32-g++ -E test.cpp -DWIN32_LEAN_AND_MEAN
54995 lines
avg time of 0.13s

# also checked with more of our defines
time x86_64-w64-mingw32-g++ -E test.cpp -D_WIN32_WINNT=0x0601 -D_WIN32_IE=0x0501 -DWIN32_LEAN_AND_MEAN -D_MT -DWIN32 -D_WINDOWS -D_FILE_OFFSET_BITS=64 | wc -l
56884 lines
avg time ~ the same as -DWIN32_LEAN_AND_MEAN

I might follow up with some documentation additions to configure.ac.