[WIP] Feature/c++17 #927
Closed
[WIP] Feature/c++17 #927
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changes in XCode 11 Objective-C Runtime cause an error during build on MacOS 10.15 Catalina. Ported solution from bitcoin PR 16720. Updated instruction on how to build berkeley DB for macOS 10.15 Catalina.
(Ported from bitcoin)
After this:
- `boost::optional` is no longer used directly (only through `Optional`
which is an alias for it)
- `boost/optional.hpp` is only included in one place
cherry-picking commit 7cbfebbf3 from https://github.com/bitcoin/bitcoin
|
This pull request is in progress. There will likely be a slew of changes, so I created it now to allow any discussion to be added as necessary. |
|
You can leave the pull request in draft state and also add [WIP] to the pull request title for better clarity of work to be done. |
On OS X, when searching Homebrew keg-only packages for BDB 4.8, if we find it, use BDB_CPPFLAGS and BDB_LIBS instead of CFLAGS and LIBS for the result. This is (1) more correct, and (2) necessary in order to give this location priority over other directories in the include search path, which may include system include directories with other versions of BDB.
This enables of the use of AI_* definitions in the Windows headers, specifically AI_ADDRCONFIG, which fixes an issue with libevent and ipv6 on Windows. It also aligns with what we define in configure when building Core.
libevent uses getaddrinfo when available, and falls back to gethostbyname Windows has both, but gethostbyname only supports IPv4 libevent fails to detect Windows's getaddrinfo due to not including the right headers This patches libevent's configure script to check it correctly
to compulsory C++17.
HexStr can be called with anything that bas `begin()` and `end()` functions, so clean up the redundant calls.
Also moves $PTHREAD_CFLAGS to the CFLAGS.
Note that with this change we are no-longer including PTHREAD_* flags when building libbitcoinconsensus. Also note that we are including PTHREAD_LIBS in AM_PTHREAD_FLAGS
HexStr can be called with anything that bas `begin()` and `end()` functions, so clean up the redundant calls.
|
I do not envy the thought of doing the work you're doing here. Good work so far though :) |
Update macOS installation guide Reference https://github.com/bitcoin/bitcoin 96124a204193ed114ca9594df7d5151206990e91 thru 48134a09adef3b5302cdd6e95500db404c9ac961
LOL. Starting to go bug eyed already!!! |
|
I've created a c++17 branch so that we can handle this with multiple PRs to that branch, rather than just having a big giant PR. |
Ok. That's great! |
This has been around since the introduction of autotools. However at this point I'm not sure we'd every want to suppress all warnings when performing a build, and given that CXX FLAGS will have been overriden when cross-compiling for Windows (using depends), this would rarely, if-ever be used anyways. From https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html: -w Inhibit all warning messages.
Instruct the linker to set the major & minor subsystem versions in the PE header to 6 & 1 (NT 6.1 which corresponds to Windows 7). Similar to macOS, the binary will now refuse to run on unsupported versions of Windows.
This change adds the correct suffix to debug mode .pc filenames for MinGW and also to the Qt libraries listed in the `Requires` field. The filename adjustment fixes the accidental overwriting of release mode .pc files with the debug mode variant which required the wrong variant of the libraries when `debug_and_release` is active. Note that macOS also supports the `debug_and_release' configuration but may use the regular library names together with DYLD_IMAGE_SUFFIX. Creation of *_debug.pc files is turned off as they're identical to their non-debug counterparts. More info: - QTBUG-4155 - Qt commit a0d8fb4ac3cb7bafdb39f340055eacee4f957513
This change adds to the BITCOIN_QT_CONFIGURE script ability to use pkg-config for MinGW. All of the non-pkg-config paths are removed as needless. If depends is built with DEBUG=1 the configure script fails to pickup Qt: - for macOS host (similar, but not the same as issue 16391) - for Windows host (regression)
QT_STATICPLUGIN is defined in BITCOIN_QT_CONFIGURE macro.
The Mingw-w64 5.0 (Ubuntu 18.04 Bionic) is used to build the Windows binaries now.
It is safe to use pthread_setname_np since #17538 when the minimal glibc version is set to 2.17.
common.vcxproj used for MSVC builds
|
Moved to C++17 branch for now |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Upgrade code base to support C++17 compiler.
Using the updated compiler should hopefully allow us to:
The bitcoin team began the same transition to C++17 in early 2020 as per bitcoin/bitcoin#16684. The majority of changes brought in will be from their repo.
Root Cause
Compilation issues with OSX Catalina impeding progress
Compilation issues with QT5.15 impeding progress
Unable to bring in new versions of Berkely DB (ie. 5.1.29).
Solution
Surgically bring in well-tested migration commits from the bitcoin repo and refactor veil specific code where necessary.
Bounty PR
Bounty Payment Address
sv1qqp6aptgvgp9t9h8sgzkqmu6cgq2e20l9x6fsl5ask7t3ygy2jagftcpq0x5z0h522ca5h06qq3hx33pke00r7gjt3j24n896gf55y68ptrmjqqqqd8lz3Unit Testing Results
Will try to bring in changes to unit tests where they were updated for bitcoin.