On master (1186910b6b7ba7c7e5193c76f33f25825e6cc0b7), when cross-compiling for Windows using our depends build system, we must manually choose the POSIX thread model for the x86_64-w64-mingw32-g++ compiler.
This PR improves the build system to make this choice automagically.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
0@@ -1,3 +1,7 @@
1+ifneq ($(shell which $(host)-g++-posix),)
2+mingw32_CXX := $(host)-g++-posix
:= here? Can we just do =?
Do we need
:=here?
No, we don’t.
Can we just do
=?
Yes, we can.
But I do prefer simply expanded make variables to recursively expanded ones. They make debugging and reasoning about Makefiles much easier. Actually, I think the choice of a simply expanded variable is the default, while the usage of a recursively expanded variable requires justification.
In this particular case the host variable is simply expanded, and all code is placed in an included file. Honestly, I cannot see advantages for mingw32_CXX being a recursively expanded variable here.
Or did I miss something?
re-ACK 2fda0c785188ae94fba921c1b8f6f2c005faf1d4
I think it is better to automatically set this rather than have a user manually configure this. Tested on Ubuntu 20.04.
Note that on another system where the mingw-cxx is not bundled with different versions like the ubuntu package is, such as Arch Linux, the following line will be in the depends output:
0which: no x86_64-w64-mingw32-g++-posix in ...
Guix Hashes:
0$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
1
2e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 guix-build-2fda0c785188/output/dist-archive/SKIPATTEST.TAG
3129fe7c0234cf1e1d2b8c71dc36469a7dcb07e1a7c1806c547ecb29824a43c78 guix-build-2fda0c785188/output/dist-archive/bitcoin-2fda0c785188.tar.gz
40e13399126a5acbc1ab62d32700dd37ae7a43490c0ef74aa35d347bb9e1dadb9 guix-build-2fda0c785188/output/x86_64-w64-mingw32/bitcoin-2fda0c785188-win-unsigned.tar.gz
55d0468c34d947251e472ac9bf6182d9bb1a48be16c1d8fe48ad6f6ed423f87b3 guix-build-2fda0c785188/output/x86_64-w64-mingw32/bitcoin-2fda0c785188-win64-debug.zip
6d757fa29ae8e7ee6ba709efca19549a9837df4173633c3b9fbf1753b156cd29a guix-build-2fda0c785188/output/x86_64-w64-mingw32/bitcoin-2fda0c785188-win64-setup-unsigned.exe
749057728c99484ca7b96727183a368ea5ffd0b8f58e2d51e806c17946ba8caf4 guix-build-2fda0c785188/output/x86_64-w64-mingw32/bitcoin-2fda0c785188-win64.zip
810e3dda06008af8c7db65f1ee4d943df7f6f5138d35eaa3931676ed25648fcf5 guix-build-2fda0c785188/output/x86_64-w64-mingw32/inputs.SHA256SUMS
I wonder if the mingw developers plan on supporting the C++ threading stuff in the -win32 variant (this is the only reason we require -posix). There is no inherent reason not to, for example MSVC can do C++ threading without explicit POSIX compatibility. But it might be a lot of work for some other reason.
That said, this makes sense for now. Concept ACK.
80@@ -81,27 +81,6 @@ The first step is to install the mingw-w64 cross-compilation tool chain:
81
82 sudo apt install g++-mingw-w64-x86-64
Apparently yes: https://packages.ubuntu.com/hirsute/g++-mingw-w64-x86-64
g++-mingw-w64-x86-64 is an umbrella package that depends on g++-mingw-w64-x86-64-posix and g++-mingw-w64-x86-64-win32. It does not contain any compiler itself, just some documentation files.
tACK 2fda0c785188ae94fba921c1b8f6f2c005faf1d4
Tested on Ubuntu 20.04
This PR adds functionality to automatically select the POSIX model for cross-compilation of Windows bitcoin-core on Ubuntu. It is achieved by modifying mingw32.mk file so that if the POSIX thread model is present in the system, then declare ming32_CXX variable using Simply Expanded Variable declaration and equals it to the POSIX model’s location.
Tested the PR by first selecting the POSIX model, and trying to cross-compile the PR. And then cross-compiling the PR again while setting a non-POSIX(win32) model. I was able to successfully cross-compile the PR in both cases.
On master: The cross-compilation was successful when the POSIX model was selected but failed when (win32) model was selected.
Error message while using (win32) model:
0make[2]: *** [Makefile:10778: qt/libbitcoinqt_a-bitcoin.o] Error 1
1make[2]: Leaving directory '/home/shaman/Documents/summerofbitcoin/window_cross/bitcoin/src'
2make[1]: *** [Makefile:16271: all-recursive] Error 1
3make[1]: Leaving directory '/home/shaman/Documents/summerofbitcoin/window_cross/bitcoin/src'
4make: *** [Makefile:823: all-recursive] Error 1
On PR: The cross-compilation was successful in both cases when the POSIX model was selected, and when the non-POSIX (win32) model was selected.
I agree with the changes suggested by op. These changes will lead to successful cross-compilation of bitcoin-core, even when the POSIX thread model is not selected. I think this particular change will help in two cases:
My apologies about this.
While this PR worked with Qt 5.12.11, it got broken with Qt 5.15.2.
Even though the correct compiler has been chosen
0$ make -j 9 -C depends --no-print-directory print-qt_cxx HOST=x86_64-w64-mingw32
1qt_cxx=x86_64-w64-mingw32-g++-posix
it appears that without update-alternatives step (i.e., -win32 has been chosen manually or by default) make -C depends qt HOST=x86_64-w64-mingw32 fails with error: ‘mutex’ in namespace ‘std’ does not name a type :man_facepalming:
Our CI task passes due to https://github.com/bitcoin/bitcoin/blob/6d859cbd794a36ed1c0c72324a4ec54d029b3d3b/ci/test/05_before_script.sh#L46-L48
I have no explanation for now, but this PR must be reverted.
Apparently yes: https://packages.ubuntu.com/hirsute/g++-mingw-w64-x86-64
g++-mingw-w64-x86-64is an umbrella package that depends ong++-mingw-w64-x86-64-posixandg++-mingw-w64-x86-64-win32. It does not contain any compiler itself, just some documentation files.
This is not the case for bionic or focal. But for hirsute, impish and jammy, if g++-mingw-w64-x86-64-posix has been installed but not g++-mingw-w64-x86-64-win32 this PR works again.
cc @laanwj @dongcarl @jarolrod @shaavan
UPDATE: Another possible solution:
0--- a/depends/packages/qt.mk
1+++ b/depends/packages/qt.mk
2@@ -158,6 +158,7 @@ $(package)_config_opts_mingw32 += -no-dbus
3 $(package)_config_opts_mingw32 += -no-freetype
4 $(package)_config_opts_mingw32 += -xplatform win32-g++
5 $(package)_config_opts_mingw32 += "QMAKE_CFLAGS = '$($(package)_cflags) $($(package)_cppflags)'"
6+$(package)_config_opts_mingw32 += "QMAKE_CXX = '$($(package)_cxx)'"
7 $(package)_config_opts_mingw32 += "QMAKE_CXXFLAGS = '$($(package)_cflags) $($(package)_cppflags)'"
8 $(package)_config_opts_mingw32 += "QMAKE_LFLAGS = '$($(package)_ldflags)'"
9 $(package)_config_opts_mingw32 += -device-option CROSS_COMPILE="$(host)-"
UPDATE 2: A fix suggested in #24131.