windows: Set _WIN32_WINNT to 0x0601 (Windows 7) #14922

2018-12-11T12:20:27Z

The current minimum support Windows version is Vista. So set it to 0x0600
https://github.com/mirror/mingw-w64/blob/5a88def8ad862ef8f4e5f2e69661bfb2d07f1ce2/mingw-w64-headers/include/sdkddkver.h#L19

2018-12-11T12:27:59Z

~~Is that so? I know we dropped Windows XP support, but did we drop support for Vista?~~

2018-12-11T12:36:53Z

@laanwj Oh, I saw #12546 and thought that we didn't support Vista anymore. Changed to 0x0600 (Vista)

2018-12-11T12:42:11Z

~~I mean, we could still decide to drop Vista support for 0.18.0 if there's a good reason for it, but it'll require some discussion.~~
~~(unless support for Vista was already dropped, but I don't know and cannot find any discussion of that?)~~

What does this change do?

2018-12-11T12:58:34Z

#14881 is using inet_pton and it's only for Vista or later. So I create this PR just for that.

2018-12-11T16:26:08Z

If inet_pton is the only reason, that could be easily re-implemented. Of course, if there is a need to keep Vista support. I personnaly don't care. :)

2018-12-11T17:23:11Z

@kristapsk Vista supports that, which is the minimum that is supported, so now after changing the minimum (initially it was changing it to W7) to Vista this PR is non-controversial.

2018-12-11T17:39:53Z

Ok, then it's strong utACK from me, nobody should run Bitcoin Core on XP anymore.

2018-12-12T01:27:05Z

But should be mentioned in release notes.

2018-12-12T02:40:50Z

Prefer to see this kind of change merged as part of a PR that needs/uses it.

Release notes are already clear that XP isn't supported, for several versions now.

2018-12-12T08:06:43Z

But should be mentioned in release notes.

XP has already not been supported since 0.13.0 in 2016, which was explicitly mentioned in the release notes then (and many a release after that): https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.13.0.md#compatibility
There's no need for any argument or discussion about this here.

2018-12-12T12:16:51Z

Gitian builds for commit 5f23460 (master):

b21ed43f3b0356196e56b469609a03e2... bitcoin-0.17.99-aarch64-linux-gnu-debug.tar.gz
e51de4e0f5ee0a628d99e44e42fdb949... bitcoin-0.17.99-aarch64-linux-gnu.tar.gz
a6a097c9180b124676f63cb857926784... bitcoin-0.17.99-arm-linux-gnueabihf-debug.tar.gz
b14f14b361f9071f727e4932928f4773... bitcoin-0.17.99-arm-linux-gnueabihf.tar.gz
3ff843f804aacd42b2abf89d05ce3b2b... bitcoin-0.17.99-i686-pc-linux-gnu-debug.tar.gz
6c8f151ba965af85ea61a54b3e62868c... bitcoin-0.17.99-i686-pc-linux-gnu.tar.gz
be2687505f1b43af99aef414d5db2388... bitcoin-0.17.99-osx-unsigned.dmg
dbad8b8e2abcf7303ae819caf8c98c81... bitcoin-0.17.99-osx64.tar.gz
09f68b16b4eca085dff64e3bb13a6afc... bitcoin-0.17.99-riscv64-linux-gnu-debug.tar.gz
b5de6c14a97e5286d876f381ddb42ee5... bitcoin-0.17.99-riscv64-linux-gnu.tar.gz
65d0d9bb359384d92338007965754423... bitcoin-0.17.99-win32-debug.zip
208dfc0c418a14aaca78434470057ff8... bitcoin-0.17.99-win32-setup-unsigned.exe
b94701919d83cb6ed0f2454cf811fac1... bitcoin-0.17.99-win32.zip
41e8f340f0bd12973f6be789a4c3e58b... bitcoin-0.17.99-win64-debug.zip
e24ecaf60e59269011ac3d2639678096... bitcoin-0.17.99-win64-setup-unsigned.exe
0af95f079697e0a3a868b9e87e82714b... bitcoin-0.17.99-win64.zip
694a79a86b87da27f8eed478552addb8... bitcoin-0.17.99-x86_64-linux-gnu-debug.tar.gz
7bb0ba614db73a9a0da7c04fb08f61e8... bitcoin-0.17.99-x86_64-linux-gnu.tar.gz
a40935b2fd19825c5a49d3be6d88c28c... bitcoin-0.17.99.tar.gz
ea6507f3772a887100366ef2cec15d15... bitcoin-linux-0.18-res.yml
07f39fdd07f214899a1ed7feb8ab2631... bitcoin-linux-build.log
630c07045139669883a014f34961d598... bitcoin-osx-0.18-res.yml
3d10144061b447b72bb643327f49bd7a... bitcoin-osx-build.log
737744285670cd840e4b53edd4fefe0b... bitcoin-win-0.18-res.yml
ac9ac16e55e179f79741594cdd94627b... bitcoin-win-build.log

Gitian builds for commit 5d233926dec4b3df52849cd15e4a86429adfd8bc (master and this pull):

556f498a36707b2501f312f724dc9406... bitcoin-0.17.99-aarch64-linux-gnu-debug.tar.gz
4f1c6f398a3ccf34e47be25f100ca335... bitcoin-0.17.99-aarch64-linux-gnu.tar.gz
ba6cb2b4669930932c194f44a001a394... bitcoin-0.17.99-arm-linux-gnueabihf-debug.tar.gz
03ae54e2a79351b70d1acb6e52082a13... bitcoin-0.17.99-arm-linux-gnueabihf.tar.gz
db3437c7e8b1f72db4970fa024c618e9... bitcoin-0.17.99-i686-pc-linux-gnu-debug.tar.gz
ca5aef3d0baab94a6021d7d74498cc87... bitcoin-0.17.99-i686-pc-linux-gnu.tar.gz
2ae99ea463f1cd6c279da445e69e3e2e... bitcoin-0.17.99-osx-unsigned.dmg
a904f0a71928999fb591c59f43451879... bitcoin-0.17.99-osx64.tar.gz
b0a41344b225b1f37a8c0cc497eafc17... bitcoin-0.17.99-riscv64-linux-gnu-debug.tar.gz
1d180a03ffcee84a04d6a09000fe3c63... bitcoin-0.17.99-riscv64-linux-gnu.tar.gz
e4c1c33a8038703902bfee522b76f34e... bitcoin-0.17.99-x86_64-linux-gnu-debug.tar.gz
8b424bcebfa45c3051e8f124226d9869... bitcoin-0.17.99-x86_64-linux-gnu.tar.gz
3e3d517eeab46d308ed1425b3eea064f... bitcoin-0.17.99.tar.gz
8880a2d516e3f4893eace62685e86aee... bitcoin-linux-0.18-res.yml
72da9df411ab56e0657fb06e315e7617... bitcoin-linux-build.log
19b4dc23675cdc6b5b1fcfaac3b6c18e... bitcoin-osx-0.18-res.yml
1131240eb505ff9e13e6553f3cd86974... bitcoin-osx-build.log
5b0cbf4d1f74048ed4a4e4d4d1a16b0f... bitcoin-win-build.log

2018-12-12T13:20:56Z

FWIW I spun up a Windows Vista (SP2) VM and downloaded the v0.17.0.1 binary. It "seems" to run ok, although I didn't test extensively:
0 17 0 1 on windows vista

However, Qt last listed Vista as a supported platform (deployment only) in 5.6 (LTS - Mar 2019), so I'd be happy to suggest tagging this for 0.18.0, revert to bumping to 0x0601 (Windows 7), and updating any release-notes/supported versions text in this PR.

I'd be pretty surprised if anyone is testing on Vista (at all), and given Qt has dropped support for it, I assume it's just a matter of time before it will stop working (possibly randomly), especially if we move to a newer Qt for release builds in 0.18.0.

2018-12-12T15:35:33Z

@ken2812221 Could you update this PR to include docs changes.
Is the src/zmq/zmqabstractnotifier.cpp change from your last force-push intended?

Related IRC discussion here, (lines 284 - ~335).

2018-12-12T16:08:11Z

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

Conflicts

No conflicts as of last run.

2018-12-12T16:45:00Z

Gitian fails with:

  CXX      zmq/libbitcoin_zmq_a-zmqabstractnotifier.o
In file included from ./compat.h:31:0,
                 from ./util/system.h:18,
                 from zmq/zmqabstractnotifier.cpp:6:
/usr/share/mingw-w64/include/mswsock.h:201:5: error: ‘WSAPOLLFD’ does not name a type
     WSAPOLLFD fdArray[0];
     ^~~~~~~~~
/usr/share/mingw-w64/include/mswsock.h:204:39: error: typedef ‘LPFN_WSAPOLL’ is initialized (use decltype instead)
   typedef INT (WSAAPI *LPFN_WSAPOLL) (LPWSAPOLLFD fdarray, ULONG nfds, INT timeout);
                                       ^~~~~~~~~~~
/usr/share/mingw-w64/include/mswsock.h:204:39: error: ‘LPWSAPOLLFD’ was not declared in this scope
/usr/share/mingw-w64/include/mswsock.h:204:39: note: suggested alternative: ‘LPWSAPOLLDATA’
   typedef INT (WSAAPI *LPFN_WSAPOLL) (LPWSAPOLLFD fdarray, ULONG nfds, INT timeout);
                                       ^~~~~~~~~~~
                                       LPWSAPOLLDATA
/usr/share/mingw-w64/include/mswsock.h:204:66: error: expected primary-expression before ‘nfds’
   typedef INT (WSAAPI *LPFN_WSAPOLL) (LPWSAPOLLFD fdarray, ULONG nfds, INT timeout);
                                                                  ^~~~
/usr/share/mingw-w64/include/mswsock.h:204:76: error: expected primary-expression before ‘timeout’
   typedef INT (WSAAPI *LPFN_WSAPOLL) (LPWSAPOLLFD fdarray, ULONG nfds, INT timeout);
                                                                            ^~~~~~~
Makefile:7050: recipe for target 'zmq/libbitcoin_zmq_a-zmqabstractnotifier.o' failed
make[2]: *** [zmq/libbitcoin_zmq_a-zmqabstractnotifier.o] Error 1
make[2]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-i686-w64-mingw32/src'
Makefile:10392: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-i686-w64-mingw32/src'
Makefile:775: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1

2018-12-12T16:48:21Z

@MarcoFalke Gitian build would still be failed now. It's still under investigation.

2018-12-12T17:09:15Z

One other thought, if we're changing this, do we want to explicitly pass the same version to anything in depends? i.e at the moment miniupnpc ends up being built with -D_WIN32_WINNT=0X501.

2018-12-13T13:30:02Z

One other thought, if we're changing this, do we want to explicitly pass the same version to anything in depends? i.e at the moment miniupnpc ends up being built with -D_WIN32_WINNT=0X501.

Not sure, that depends:

does any of the dependencies make use of higher _WIN32_WINNT APIs when explicitly defined?
does _WIN32_WINNT matter at all if code is not using any of the APIs exposed by the newer windows version? E.g. does it cause use of more efficient APIs for existing things, without code changes?

If either is yes, it might make sense to equalize them. If not, there's no point in doing this for dependencies.

2018-12-14T08:07:01Z

I found it be defined in many places. So I think it would be better to use -D_WIN32_WINNT=0x0601.

2018-12-14T09:17:24Z

I found it be defined in many places. So I think it would be better to use -D_WIN32_WINNT=0x0601.

TBH as it's part of the code itself (the "contract" with Windows), I personally prefer it to be defined in the code, not in the build system.
(this helps analysis tools, other IDEs, MSVC, ...)

2019-01-02T01:50:30Z

Since #14881 was closed, this PR shall not be necessary anymore. This change could cause compilation issue on Mingw or we would have to define _WIN32_WINNT in every file that we include windows related headers. I think it does not worth to do that.

2019-01-02T01:59:49Z

@ken2812221 #14881 has just been moved to #15052.

Also remove all defines in many places and define it in configure stage to keep consistency.

2019-01-23T08:50:51Z

Do we need to worry about WINVER as well?
https://docs.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt?view=vs-2019

2019-01-23T08:53:59Z

Looks like you could drop this code:

bitcoin/src/init.cpp

Lines 896 to 900 in 82cf681

    
           #ifndef PROCESS_DEP_ENABLE 
        
               // We define this here, because GCCs winbase.h limits this to _WIN32_WINNT >= 0x0601 (Windows 7), 
        
               // which is not correct. Can be removed, when GCCs winbase.h is fixed! 
        
           #define PROCESS_DEP_ENABLE 0x00000001 
        
           #endif

2019-01-23T08:54:20Z

Concept ACK

2019-01-24T14:32:14Z

@theuni can you weigh in here please

"The AI_ADDRCONFIG flag is defined on the Windows SDK for Windows Vista and later. The AI_ADDRCONFIG flag is supported on Windows Vista and later." https://docs.microsoft.com/en-us/windows/desktop/api/ws2tcpip/nf-ws2tcpip-getaddrinfo However, the version of MinGW we use on Travis is not current and does not carry the relevant definition, as such I defined it in compat. https://github.com/wine-mirror/wine/blob/master/include/ws2tcpip.h Testing confirms that the PROTECTION_LEVEL_UNRESTRICTED, IPV6_PROTECTION_LEVEL, PROCESS_DEP_ENABLE, AI_ADDRCONFIG, are now supported by the version of Windows that we test against, so can be removed. https://travis-ci.org/bitcoin/bitcoin/jobs/483255439 https://travis-ci.org/Empact/bitcoin/jobs/484123087

2019-01-25T00:03:09Z

This commit shows a few more settings that can be removed:
Empact@d0522ec

I used the FAIL_ON_EXTRANEOUS_COMPAT method from #15231 to identify them:
https://travis-ci.org/bitcoin/bitcoin/jobs/483255439
https://travis-ci.org/Empact/bitcoin/jobs/484123087

2019-01-25T00:24:42Z

utACK d8a2992

2019-01-25T04:25:56Z

re-utACK d0522ec

2019-01-25T23:41:06Z

Concept ACK.

Agreed with @laanwj about the depends. I would think that qt would be the most likely to be problematic, but it appears to be handled sanely there:

# Override MinGW's definition in _mingw.h
mingw: DEFINES += WINVER=0x600 _WIN32_WINNT=0x0600

Which made me curious about WINVER. This msdn Article doesn't specifically mention that they need to be the same value, but imo it's implied.

So, I think we should be setting both. Then there's a new snag. Leveldb sets WINVER but not _WIN32_WINNT (I thought this behavior was copied from their buildsystem, but see below):

src/Makefile.leveldb.include:LEVELDB_CPPFLAGS_INT += -DLEVELDB_PLATFORM_WINDOWS -DWINVER=0x0500 -D__USE_MINGW_ANSI_STDIO=1

I tracked that down, and we actually added it ourselves in b102466:

    * Define WINVER=0x0500 so MinGW32 can use the 64-bit-filesystem Windows
      api calls

So it should be safe to bump that to whatever.

I'd suggest:

Define WINVER and _WIN32_WINNT together
Remove the WINVER define in Makefile.leveldb.include

Edit: Whoops, I see now that @Empact already mentioned WINVER. IMO we should indeed "worry about it", if for no other reason than to save ourselves from hard-to-track-down compile failures in the future.

2019-01-26T01:30:41Z

WINVER has already been defined as _WIN32_WINNT at here since mingw v1.0 released. Do we really have to re-define it explicitly?

2019-01-26T05:07:07Z

@ken2812221 Ok, yes, I see that:

/* Choose WINVER Value */
#ifndef WINVER
#ifdef _WIN32_WINNT
#define WINVER      _WIN32_WINNT
#else
#define WINVER      0x0502
#endif
#endif

I guess the msvc headers work similarly? utACK as long as that's the case.

0164b0f build: Remove WINVER pre define in Makefile.leveldb.inlcude (Chun Kuan Lee) d0522ec Drop defunct Windows compat fixes (Ben Woosley) d8a2992 windows: Call SetProcessDEPPolicy directly (Chun Kuan Lee) 1bd9ffd windows: Set _WIN32_WINNT to 0x0601 (Windows 7) (Chun Kuan Lee) Pull request description: The current minimum support Windows version is Vista. So set it to 0x0600 https://github.com/mirror/mingw-w64/blob/5a88def8ad862ef8f4e5f2e69661bfb2d07f1ce2/mingw-w64-headers/include/sdkddkver.h#L19 Tree-SHA512: 38e2afc79426ae547131c8ad3db2e0a7f54a95512f341cfa0c06e4b2fe79521ae67d2795ef96b0192e683e4f1ba6183c010d7b4b8d6b3e68b9bf48c374c59e7d

Summary: Was set to 0x0501 (XP) which is no longer supported. Backport of core [[ bitcoin/bitcoin#14922 | PR14922]]. Test Plan: ``` cmake -GNinja ..\ -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Win64.cmake \ -DBUILD_BITCOIN_SEEDER=OFF ninja ``` Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7741

fanquake added the Windows label Dec 11, 2018

ken2812221 mentioned this pull request Dec 11, 2018

Tests: Contract testing for the procedure AddTimeData #14881

Closed

ken2812221 changed the title ~~windows: Set_WIN32_WINNT to 0x0601 (Windows 7)~~ windows: Set _WIN32_WINNT to 0x0600 (Windows Vista) Dec 11, 2018

maflcko added the Needs gitian build label Dec 11, 2018

bitcoin deleted a comment from Aekanan5906 Dec 12, 2018

DrahtBot removed the Needs gitian build label Dec 12, 2018

fanquake added this to the 0.18.0 milestone Dec 12, 2018

fanquake changed the title ~~windows: Set _WIN32_WINNT to 0x0600 (Windows Vista)~~ windows: Set _WIN32_WINNT to 0x0601 (Windows 7) Dec 12, 2018

maflcko added the Needs gitian build label Dec 12, 2018

ken2812221 changed the title ~~windows: Set _WIN32_WINNT to 0x0601 (Windows 7)~~ [WIP] windows: Set _WIN32_WINNT to 0x0601 (Windows 7) Dec 13, 2018

ken2812221 closed this Jan 2, 2019

ken2812221 deleted the patch-1 branch January 2, 2019 01:50

ken2812221 restored the patch-1 branch January 23, 2019 08:22

ken2812221 reopened this Jan 23, 2019

windows: Set _WIN32_WINNT to 0x0601 (Windows 7)

1bd9ffd

Also remove all defines in many places and define it in configure stage to keep consistency.

ken2812221 mentioned this pull request Jan 23, 2019

Drop defunct Windows compat fixes #15231

Closed

windows: Call SetProcessDEPPolicy directly

d8a2992

ken2812221 changed the title ~~[WIP] windows: Set _WIN32_WINNT to 0x0601 (Windows 7)~~ windows: Set _WIN32_WINNT to 0x0601 (Windows 7) Jan 23, 2019

build: Remove WINVER pre define in Makefile.leveldb.inlcude

0164b0f

laanwj merged commit 0164b0f into bitcoin:master Feb 5, 2019

ken2812221 deleted the patch-1 branch February 5, 2019 17:26

str4d mentioned this pull request Jun 5, 2021

build: Set minimum required Windows version to Windows 7 zcash/zcash#5204

Merged

jamescowens mentioned this pull request Jun 27, 2021

build, util: Prevent execution for Windows versions less than Windows 7 gridcoin-community/Gridcoin-Research#2203

Merged

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

windows: Set _WIN32_WINNT to 0x0601 (Windows 7) #14922

windows: Set _WIN32_WINNT to 0x0601 (Windows 7) #14922

ken2812221 commented Dec 11, 2018 •

edited

laanwj commented Dec 11, 2018 •

edited

ken2812221 commented Dec 11, 2018

laanwj commented Dec 11, 2018 •

edited

ken2812221 commented Dec 11, 2018

kristapsk commented Dec 11, 2018

laanwj commented Dec 11, 2018

kristapsk commented Dec 11, 2018

kristapsk commented Dec 12, 2018

luke-jr commented Dec 12, 2018

laanwj commented Dec 12, 2018 •

edited

DrahtBot commented Dec 12, 2018

fanquake commented Dec 12, 2018

fanquake commented Dec 12, 2018

DrahtBot commented Dec 12, 2018 •

edited

maflcko commented Dec 12, 2018

ken2812221 commented Dec 12, 2018 •

edited

fanquake commented Dec 12, 2018

laanwj commented Dec 13, 2018

ken2812221 commented Dec 14, 2018

laanwj commented Dec 14, 2018

ken2812221 commented Jan 2, 2019

fanquake commented Jan 2, 2019

Empact commented Jan 23, 2019

Empact commented Jan 23, 2019

Empact commented Jan 23, 2019

laanwj commented Jan 24, 2019

Empact commented Jan 25, 2019

Empact commented Jan 25, 2019

Empact commented Jan 25, 2019

theuni commented Jan 25, 2019 •

edited

ken2812221 commented Jan 26, 2019 •

edited

theuni commented Jan 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

windows: Set _WIN32_WINNT to 0x0601 (Windows 7) #14922

windows: Set _WIN32_WINNT to 0x0601 (Windows 7) #14922

Conversation

ken2812221 commented Dec 11, 2018 • edited

laanwj commented Dec 11, 2018 • edited

ken2812221 commented Dec 11, 2018

laanwj commented Dec 11, 2018 • edited

ken2812221 commented Dec 11, 2018

kristapsk commented Dec 11, 2018

laanwj commented Dec 11, 2018

kristapsk commented Dec 11, 2018

kristapsk commented Dec 12, 2018

luke-jr commented Dec 12, 2018

laanwj commented Dec 12, 2018 • edited

DrahtBot commented Dec 12, 2018

fanquake commented Dec 12, 2018

fanquake commented Dec 12, 2018

DrahtBot commented Dec 12, 2018 • edited

Conflicts

maflcko commented Dec 12, 2018

ken2812221 commented Dec 12, 2018 • edited

fanquake commented Dec 12, 2018

laanwj commented Dec 13, 2018

ken2812221 commented Dec 14, 2018

laanwj commented Dec 14, 2018

ken2812221 commented Jan 2, 2019

fanquake commented Jan 2, 2019

Empact commented Jan 23, 2019

Empact commented Jan 23, 2019

Empact commented Jan 23, 2019

laanwj commented Jan 24, 2019

Empact commented Jan 25, 2019

Empact commented Jan 25, 2019

Empact commented Jan 25, 2019

theuni commented Jan 25, 2019 • edited

ken2812221 commented Jan 26, 2019 • edited

theuni commented Jan 26, 2019

ken2812221 commented Dec 11, 2018 •

edited

laanwj commented Dec 11, 2018 •

edited

laanwj commented Dec 11, 2018 •

edited

laanwj commented Dec 12, 2018 •

edited

DrahtBot commented Dec 12, 2018 •

edited

ken2812221 commented Dec 12, 2018 •

edited

theuni commented Jan 25, 2019 •

edited

ken2812221 commented Jan 26, 2019 •

edited