Add Qt programs to msvc build (updated, no code changes) #15529

sipsorcery · 2019-03-04T20:27:57Z

This PR has ~~90%~~ all of the work done to allow the bitcoin Qt programs to be built with msvc and the appveyor script.

Outstanding issues:

~~There are 3 6 5 code tweaks required for the bitcoin Qt components to be built without warnings with msvc. They seem minor~~,
Building Qt as a static library for Windows is painful and time consuming. I doubt it will ever be possible to build Qt from source as part of an appveyor job (and it would probably take over an hour even if it was). My tentative solution is to build locally and upload the binaries as a github release. The msvc build is only for testing and tinkering but even so this doesn't feel like the ideal solution. Open to suggestions?
~~There is still an issue to sort out with the payment request URL handling. Building Qt with openssl is an extra headache. I will continue to work on getting this working.~~

The big benefit of this PR is the ability to run bitcoin-qt within a Visual Studio debugging session which could expedite tracking down issues on Windows.

On a side note the test-bitcoin-qt tests fail very early, probably due to *nix specific tests. I haven't dug into them at this point.

Update 28 Jun 2019: The ENABLE_BIP70 option is now off (it's flagged for removal as per #15584). With it disabled msbuild does not require any code changes to build the Bitcoin Core Qt applications.

DrahtBot · 2019-03-04T20:54:13Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15382 ([util] add runCommandParseJSON by Sjors)

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.

fanquake

Concept ACK

My tentative solution is to build locally and upload the binaries as a github release. The msvc build is only for testing and tinkering but even so this doesn't feel like the ideal solution. Open to suggestions?

If this download is required, we can always host it on a more "official" repo etc.

.appveyor.yml

practicalswift · 2019-03-05T15:57:10Z

Concept ACK

Thanks for doing this! Diversity in testing is good!

ken2812221 · 2019-03-06T09:57:56Z

Concept ACK.
Note that appveyor already have pre-built QT libraries

And the qt and vcpkg path could be arguments in msvc-autogen.py to generate the project file, so that the users could build qt on their own computer. like:

python build_msvc\msvc-autogen.py --qt=C:\Qt\5.9.7\msvc2017_64 --vcpkg=C:\Tools\vcpkg

sipsorcery · 2019-03-06T10:24:49Z

Note that appveyor already have pre-built QT libraries

I did spot that but I'm pretty sure the appveyor Qt builds are dynamic linking and therefore incompatible with the static linking builds for all the other bitcoin core libraries.

And the qt and vcpkg path could be arguments in msvc-autogen.py to generate the project file, so that the users could build qt on their own computer. like:
python build_msvc\msvc-autogen.py --qt=C:\Qt\5.9.7\msvc2017_64 --vcpkg=C:\Tools\vcpkg

That's a good idea. I think I can consolidate the location to set global properties to the Directory.Build.props file which would also simplify things.

sipsorcery · 2019-03-07T20:58:55Z

There are lots of warnings for the msvc build of libbitcoin_qt. A lot are generated from the Qt code templates however there is one that looks a bit nasty:

c:\projects\bitcoin-72c17\src\qt\walletmodel.cpp(490): warning C4717: 'WalletModel::UnlockContext::CopyFrom': recursive on all control paths, function will cause runtime stack overflow [C:\projects\bitcoin-72c17\build_msvc\libbitcoin_qt\libbitcoin_qt.vcxproj]
c:\projects\bitcoin-72c17\src\qt\walletmodel.h(199): warning C4717: 'WalletModel::UnlockContext::operator=': recursive on all control paths, function will cause runtime stack overflow [C:\projects\bitcoin-72c17\build_msvc\libbitcoin_qt\libbitcoin_qt.vcxproj]
Done Building Project "C:\projects\bitcoin-72c17\build_msvc\libbitcoin_qt\libbitcoin_qt.vcxproj" (default targets) -- FAILED.

The culprit method WalletModel::UnlockContext::CopyFrom doesn't appear to be used so I've refactored and commented it out. I thought it worth noting for anyone that may be more familiar with this portion of the code.

laanwj · 2019-03-08T14:33:31Z

Concept ACK, I did get bitcoin-qt to build once on MSVC in 2012 or so so, and know how much trouble it is I feel your pain.

sipsorcery · 2019-03-10T21:58:23Z

PR no longer work in progress and ready for review.

sipsorcery · 2019-03-11T19:44:36Z

ping @ken2812221.

ken2812221

Nice work!!

src/compat/byteswap.h

build_msvc/msvc-autogen.py

ken2812221 · 2019-03-12T14:08:44Z

build_msvc/msvc-autogen.py

I would prefer to use shutil to copy the files. Also, use absolute file path instead.

src/qt/walletmodel.h

ken2812221 · 2019-03-12T14:26:42Z

.appveyor.yml

The WholeProgramOptimization is to make sure that the generated cache is not too big to reduce cache save/load time. I remember that the cache could be over 1GB without this setting so that we can't store it with free appveyor plan.

The WholeProgramOptimization is off by default and I removed the cases where it was being set in individual project files.

ken2812221 · 2019-03-12T14:28:38Z

.appveyor.yml

If you change the python code not to use regular expression. It could be single slash here.

build_msvc/bitcoin.sln

src/qt/guiutil.cpp

practicalswift · 2019-03-12T21:11:03Z

Nit: A few of the files changed/added text files seem to be missing the expected \n as the last byte of each file.

sipsorcery · 2019-08-17T10:58:01Z

For anyone interested in testing this PR here are the bare minimum steps. First 6 are to download the required Qt5.9.7 static libraries. You can build Qt yourself (see the readme file in this PR) but that will a LOT of effort,

wget https://github.com/sipsorcery/qt_win_binary/releases/download/v1.0/sha256sum.asc
wget https://github.com/sipsorcery/qt_win_binary/releases/download/v1.0/Qt5.9.7_ssl_x64_static_vs2017.zip
gpg --search aaron@sipsorcery.com
gpg --verify sha256sum.asc
sha256sum --check sha256sum.asc
extract to C:\Qt5.9.7_ssl_x64_static_vs2017 (must be exactly this path due to Qt machinations)
Open "Developer Command Prompt for VS 2017"
cd bitcoin
py -3 build_msvc\msvc-autogen.py
msbuild /m build_msvc\bitcoin.sln /p:Configuration=Debug /p:Platform=x64 /t:build
build_msvc\x64\Debug\bitcoin-qt.exe -regtest

@NicolasDorier hint hint.

fanquake · 2019-08-19T12:07:07Z

Thanks @sipsorcery. I'll be testing this again tomorrow.

fanquake

ACK ada5dd91d3a05d077727b7e831551ccc38c61502 - apologies for taking so long to get back to this. I've tested using your build instructions in a Windows 10 VM. Works as expected. Left a single comment; as I forgot to install double-conversion and was seeing build errors.

fanquake · 2019-09-08T04:15:18Z

build_msvc/README.md

double-conversion is missing here

Thanks for reviewing! The secp256k1 and leveldb packages aren't required anymore either since they're now included as source projects. I'll fix it up.

fanquake · 2019-09-08T08:56:48Z

@sipsorcery Looks like the windows build files might need some updates for recent changes:

Build FAILED.
       "C:\projects\bitcoin\build_msvc\bitcoin.sln" (default target) (1) ->
       "C:\projects\bitcoin\build_msvc\bitcoin-qt\bitcoin-qt.vcxproj" (default target) (22) ->
       (Link target) -> 
         libbitcoin_qt.lib(walletcontroller.obj) : error LNK2001: unresolved external symbol "public: __cdecl CreateWalletDialog::CreateWalletDialog(class QWidget *)" (??0CreateWalletDialog@@QEAA@PEAVQWidget@@@Z) [C:\projects\bitcoin\build_msvc\bitcoin-qt\bitcoin-qt.vcxproj]
         libbitcoin_qt.lib(walletcontroller.obj) : error LNK2001: unresolved external symbol "public: class QString __cdecl CreateWalletDialog::walletName(void)const " (?walletName@CreateWalletDialog@@QEBA?AVQString@@XZ) [C:\projects\bitcoin\build_msvc\bitcoin-qt\bitcoin-qt.vcxproj]
         libbitcoin_qt.lib(walletcontroller.obj) : error LNK2001: unresolved external symbol "public: bool __cdecl CreateWalletDialog::encrypt(void)const " (?encrypt@CreateWalletDialog@@QEBA_NXZ) [C:\projects\bitcoin\build_msvc\bitcoin-qt\bitcoin-qt.vcxproj]
         libbitcoin_qt.lib(walletcontroller.obj) : error LNK2001: unresolved external symbol "public: bool __cdecl CreateWalletDialog::disablePrivateKeys(void)const " (?disablePrivateKeys@CreateWalletDialog@@QEBA_NXZ) [C:\projects\bitcoin\build_msvc\bitcoin-qt\bitcoin-qt.vcxproj]
         libbitcoin_qt.lib(walletcontroller.obj) : error LNK2001: unresolved external symbol "public: bool __cdecl CreateWalletDialog::blank(void)const " (?blank@CreateWalletDialog@@QEBA_NXZ) [C:\projects\bitcoin\build_msvc\bitcoin-qt\bitcoin-qt.vcxproj]
         C:\projects\bitcoin\build_msvc\x64\Release\bitcoin-qt.exe : fatal error LNK1120: 5 unresolved externals [C:\projects\bitcoin\build_msvc\bitcoin-qt\bitcoin-qt.vcxproj]
    0 Warning(s)
    6 Error(s)
Time Elapsed 00:19:58.92
Command exited with code 1

sipsorcery · 2019-09-08T09:03:43Z

@fanquake yep I'm on it.

fanquake

re-ACK 1619684 - AppVeyor looks ok now.

1619684 Added libbitcoin_qt and bitcoin-qt to the msbuild configuration. (Aaron Clauson) Pull request description: This PR has ~~90%~~ all of the work done to allow the bitcoin Qt programs to be built with msvc and the appveyor script. Outstanding issues: - ~~There are ~~3~~ ~~6~~ 5 code tweaks required for the bitcoin Qt components to be built without warnings with msvc. They seem minor~~, - Building Qt as a static library for Windows is painful and time consuming. I doubt it will ever be possible to build Qt from source as part of an appveyor job (and it would probably take over an hour even if it was). My tentative solution is to build locally and upload the binaries as a [github release](https://github.com/sipsorcery/qt_win_binary/releases). The msvc build is only for testing and tinkering but even so this doesn't feel like the ideal solution. Open to suggestions? - ~~There is still an issue to sort out with the payment request URL handling. Building Qt with openssl is an extra headache. I will continue to work on getting this working.~~ The big benefit of this PR is the ability to run bitcoin-qt within a Visual Studio debugging session which could expedite tracking down issues on Windows. On a side note the test-bitcoin-qt tests fail very early, probably due to *nix specific tests. I haven't dug into them at this point. **Update 28 Jun 2019**: The ENABLE_BIP70 option is now off (it's flagged for removal as per #15584). With it disabled msbuild does not require any code changes to build the Bitcoin Core Qt applications. ACKs for top commit: fanquake: re-ACK 1619684 - AppVeyor looks ok now. Tree-SHA512: c0d3fd53b3ff99096b2505d519ed5ca6791bc4bce77addf9c520dc042eec5980a51a1fb9f0aa72e9cc53773085c43218793ca7a915a47806a3a1ffb84d9409f9

…changes) 1619684 Added libbitcoin_qt and bitcoin-qt to the msbuild configuration. (Aaron Clauson) Pull request description: This PR has ~~90%~~ all of the work done to allow the bitcoin Qt programs to be built with msvc and the appveyor script. Outstanding issues: - ~~There are ~~3~~ ~~6~~ 5 code tweaks required for the bitcoin Qt components to be built without warnings with msvc. They seem minor~~, - Building Qt as a static library for Windows is painful and time consuming. I doubt it will ever be possible to build Qt from source as part of an appveyor job (and it would probably take over an hour even if it was). My tentative solution is to build locally and upload the binaries as a [github release](https://github.com/sipsorcery/qt_win_binary/releases). The msvc build is only for testing and tinkering but even so this doesn't feel like the ideal solution. Open to suggestions? - ~~There is still an issue to sort out with the payment request URL handling. Building Qt with openssl is an extra headache. I will continue to work on getting this working.~~ The big benefit of this PR is the ability to run bitcoin-qt within a Visual Studio debugging session which could expedite tracking down issues on Windows. On a side note the test-bitcoin-qt tests fail very early, probably due to *nix specific tests. I haven't dug into them at this point. **Update 28 Jun 2019**: The ENABLE_BIP70 option is now off (it's flagged for removal as per bitcoin#15584). With it disabled msbuild does not require any code changes to build the Bitcoin Core Qt applications. ACKs for top commit: fanquake: re-ACK 1619684 - AppVeyor looks ok now. Tree-SHA512: c0d3fd53b3ff99096b2505d519ed5ca6791bc4bce77addf9c520dc042eec5980a51a1fb9f0aa72e9cc53773085c43218793ca7a915a47806a3a1ffb84d9409f9

This part of the build system has been removed in bitcoin#15529 and thus no longer needs to be updated.

8d841ad doc: Remove MSVC update step from translation process (Wladimir J. van der Laan) Pull request description: This part of the build system has been removed in #15529 and thus no longer needs to be updated. ACKs for top commit: MarcoFalke: ACK 8d841ad sipsorcery: ACK 8d841ad Tree-SHA512: f561a6b1da806e8868a265c77725b94fabef60bc7b9d401e3f70c3d859323adc2e204e3d6fbfea4f1ff86e70667f8bd01157411106ea93974921c02d874e0083

…ocess 8d841ad doc: Remove MSVC update step from translation process (Wladimir J. van der Laan) Pull request description: This part of the build system has been removed in bitcoin#15529 and thus no longer needs to be updated. ACKs for top commit: MarcoFalke: ACK 8d841ad sipsorcery: ACK 8d841ad Tree-SHA512: f561a6b1da806e8868a265c77725b94fabef60bc7b9d401e3f70c3d859323adc2e204e3d6fbfea4f1ff86e70667f8bd01157411106ea93974921c02d874e0083

This part of the build system has been removed in bitcoin#15529 and thus no longer needs to be updated.

fanquake added the Windows label Mar 4, 2019

fanquake requested a review from ken2812221 March 4, 2019 23:05

fanquake added the GUI label Mar 4, 2019

fanquake reviewed Mar 4, 2019

View reviewed changes

.appveyor.yml Outdated Show resolved Hide resolved

.appveyor.yml Outdated Show resolved Hide resolved

sipsorcery force-pushed the qt_msvc branch 13 times, most recently from 751da0c to 5502529 Compare March 10, 2019 21:28

sipsorcery changed the title ~~WIP: Add Qt programs to msvc build~~ Add Qt programs to msvc build Mar 10, 2019

ken2812221 reviewed Mar 12, 2019

View reviewed changes

sipsorcery force-pushed the qt_msvc branch from 5502529 to dec5547 Compare March 12, 2019 21:10

sipsorcery force-pushed the qt_msvc branch from dec5547 to bacc988 Compare March 12, 2019 21:34

sipsorcery force-pushed the qt_msvc branch 2 times, most recently from cddccf6 to 997610f Compare August 10, 2019 17:27

DrahtBot added the Needs rebase label Aug 16, 2019

sipsorcery force-pushed the qt_msvc branch 2 times, most recently from c739577 to ada5dd9 Compare August 17, 2019 10:06

DrahtBot removed the Needs rebase label Aug 17, 2019

fanquake approved these changes Sep 8, 2019

View reviewed changes

sipsorcery force-pushed the qt_msvc branch from ada5dd9 to 87d2733 Compare September 8, 2019 08:28

Added libbitcoin_qt and bitcoin-qt to the msbuild configuration.

1619684

sipsorcery force-pushed the qt_msvc branch from 87d2733 to 1619684 Compare September 8, 2019 12:13

fanquake approved these changes Sep 8, 2019

View reviewed changes

fanquake merged commit 1619684 into bitcoin:master Sep 11, 2019

sipsorcery deleted the qt_msvc branch September 11, 2019 06:44

laanwj added a commit to laanwj/bitcoin that referenced this pull request Sep 26, 2019

doc: Remove MSVC update step from translation process

8d841ad

This part of the build system has been removed in bitcoin#15529 and thus no longer needs to be updated.

laanwj mentioned this pull request Sep 26, 2019

doc: Remove MSVC update step from translation process #16968

Merged

hebasto mentioned this pull request Oct 28, 2019

qt: Add privacy to the Overview page #16432

Merged

HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Nov 17, 2019

doc: Remove MSVC update step from translation process

7beddfe

This part of the build system has been removed in bitcoin#15529 and thus no longer needs to be updated.

This was referenced Sep 8, 2021

build: Copy appropriate MSVC build output to src/qt automatically #22917

Closed

doc: Replace a link to Qt precompiled binaries with compile instructions #22890

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2022

Add Qt programs to msvc build (updated, no code changes) #15529

Add Qt programs to msvc build (updated, no code changes) #15529

Uh oh!

Conversation

sipsorcery commented Mar 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Mar 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

practicalswift commented Mar 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 commented Mar 6, 2019

Uh oh!

sipsorcery commented Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sipsorcery commented Mar 7, 2019

Uh oh!

laanwj commented Mar 8, 2019

Uh oh!

sipsorcery commented Mar 10, 2019

Uh oh!

sipsorcery commented Mar 11, 2019

Uh oh!

ken2812221 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ken2812221 Mar 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ken2812221 Mar 12, 2019

Choose a reason for hiding this comment

Uh oh!

sipsorcery Mar 12, 2019

Choose a reason for hiding this comment

Uh oh!

ken2812221 Mar 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

practicalswift commented Mar 12, 2019

Uh oh!

sipsorcery commented Aug 17, 2019

Uh oh!

fanquake commented Aug 19, 2019

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake Sep 8, 2019

Choose a reason for hiding this comment

Uh oh!

sipsorcery Sep 8, 2019

Choose a reason for hiding this comment

Uh oh!

fanquake commented Sep 8, 2019

Uh oh!

sipsorcery commented Sep 8, 2019

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

sipsorcery commented Mar 4, 2019 •

edited

Loading

DrahtBot commented Mar 4, 2019 •

edited

Loading

practicalswift commented Mar 5, 2019 •

edited

Loading

sipsorcery commented Mar 6, 2019 •

edited

Loading