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

sipsorcery commented at 8:27 pm on March 4, 2019: member

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 commented at 8:54 pm on March 4, 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:

#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 added the label Windows on Mar 4, 2019

fanquake requested review from ken2812221 on Mar 4, 2019

fanquake added the label GUI on Mar 4, 2019

in .appveyor.yml:28 in 099de447ae outdated

22@@ -22,6 +23,16 @@ install:
23 - cmd: vcpkg install --triplet %PLATFORM%-windows-static %PACKAGES% > NUL
24 before_build:
25 - ps:  clcache -M 536870912
26+- ps: |
27+      if(!(Test-Path -Path c:\Qt5.9.7_ssl_x64_static_vs2017)) { 
28+        Write-Host "Downloading Qtv5.9.7 binaries.";

fanquake commented at 11:13 pm on March 4, 2019:

Trailing whitespace here, and a few other places (causing Travis failure).

in .appveyor.yml:29 in 099de447ae outdated

22@@ -22,6 +23,16 @@ install:
23 - cmd: vcpkg install --triplet %PLATFORM%-windows-static %PACKAGES% > NUL
24 before_build:
25 - ps:  clcache -M 536870912
26+- ps: |
27+      if(!(Test-Path -Path c:\Qt5.9.7_ssl_x64_static_vs2017)) { 
28+        Write-Host "Downloading Qtv5.9.7 binaries."; 
29+        Invoke-WebRequest -Uri https://github.com/sipsorcery/qt_win_binary/releases/download/Binary/Qt5.9.7_ssl_x64_static_vs2017.zip -Out qtdownload.zip;

fanquake commented at 11:14 pm on March 4, 2019:

Can we check a shasum256 after downloading.

fanquake commented at 11:21 pm on March 4, 2019: member

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.

practicalswift commented at 3:57 pm on March 5, 2019: contributor

Concept ACK

Thanks for doing this! Diversity in testing is good!

ken2812221 commented at 9:57 am on March 6, 2019: contributor

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:

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

sipsorcery commented at 10:24 am on March 6, 2019: member

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:
0python 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 commented at 8:58 pm on March 7, 2019: member

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:

0c:\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]
1c:\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]
2Done 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 commented at 2:33 pm on March 8, 2019: member

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 force-pushed on Mar 8, 2019

sipsorcery force-pushed on Mar 9, 2019

sipsorcery force-pushed on Mar 10, 2019

sipsorcery renamed this:
~~WIP: Add Qt programs to msvc build~~
Add Qt programs to msvc build
on Mar 10, 2019

sipsorcery commented at 9:58 pm on March 10, 2019: member

PR no longer work in progress and ready for review.

sipsorcery commented at 7:44 pm on March 11, 2019: member

ping @ken2812221.

in src/compat/byteswap.h:68 in 5502529ef8 outdated

73-          | ((x & 0x000000ff00000000ull) >> 8)
74-          | ((x & 0x00000000ff000000ull) << 8)
75-          | ((x & 0x0000000000ff0000ull) << 24)
76-          | ((x & 0x000000000000ff00ull) << 40)
77-          | ((x & 0x00000000000000ffull) << 56));
78+    return (((x & 0xff00000000000000ull) >> 56) | ((x & 0x00ff000000000000ull) >> 40) | ((x & 0x0000ff0000000000ull) >> 24) | ((x & 0x000000ff00000000ull) >> 8) | ((x & 0x00000000ff000000ull) << 8) | ((x & 0x0000000000ff0000ull) << 24) | ((x & 0x000000000000ff00ull) << 40) | ((x & 0x00000000000000ffull) << 56));

ken2812221 commented at 2:03 am on March 12, 2019:

This seems like an unintentional change.

in src/compat/byteswap.h:47 in 5502529ef8 outdated

56@@ -42,22 +57,15 @@ inline uint16_t bswap_16(uint16_t x)
57 #if HAVE_DECL_BSWAP_32 == 0
58 inline uint32_t bswap_32(uint32_t x)
59 {
60-    return (((x & 0xff000000U) >> 24) | ((x & 0x00ff0000U) >>  8) |
61-            ((x & 0x0000ff00U) <<  8) | ((x & 0x000000ffU) << 24));
62+    return (((x & 0xff000000U) >> 24) | ((x & 0x00ff0000U) >> 8) |
63+            ((x & 0x0000ff00U) << 8) | ((x & 0x000000ffU) << 24));
64 }

ken2812221 commented at 2:03 am on March 12, 2019:

This seems like an unintentional change.

in build_msvc/msvc-autogen.py:47 in 5502529ef8 outdated

42@@ -42,8 +43,26 @@ def parse_makefile(makefile):
43                     lib_sources[current_lib] = []
44                     break
45 
46+def set_custom_paths(qtDir, protocPath):
47+    with open('build_msvc\\Directory.build.props', 'r', encoding='utf-8') as rfile:

ken2812221 commented at 2:01 pm on March 12, 2019:

I would prefer to use absolute path here because people may double click the script on their computer instead of running it in cmd, maybe you could add MSVC_DIR right after SOURCE_DIR

0MSVC_DIR =  os.path.abspath(os.path.dirname(__file__))

and use

0with open(os.path.join(MSVC_DIR, 'Directory.Build.props'), 'r', encoding='utf-8') as rfile:

in build_msvc/msvc-autogen.py:52 in 5502529ef8 outdated

47+    with open('build_msvc\\Directory.build.props', 'r', encoding='utf-8') as rfile:
48+        s = rfile.read()
49+        if qtDir:
50+            s = re.sub(r'(<QtBaseDir>)[^<]+', r'\1%s'%qtDir, s, re.M)
51+        if protocPath:
52+            s = re.sub(r'(<ProtocPath>)[^<]+', r'\1%s'%protocPath, s, re.M)

ken2812221 commented at 2:05 pm on March 12, 2019:

This would change the existing file that tracked by git. I think you could use @QtBaseDir@ or something else that easy to locate the content and save the template to Directory.Build.props.in. Write to Directory.Build.props when we run the script. Then, the code could be two simple replace() function.

sipsorcery commented at 8:02 pm on March 12, 2019:

Good point, didn’t think of that.

in build_msvc/msvc-autogen.py:81 in 5502529ef8 outdated

76@@ -58,7 +77,8 @@ def main():
77             with open(vcxproj_filename, 'w', encoding='utf-8') as vcxproj_file:
78                 vcxproj_file.write(vcxproj_in_file.read().replace(
79                     '@SOURCE_FILES@\n', content))
80-
81+    os.system('copy build_msvc\\bitcoin_config.h src\\config\\bitcoin-config.h')
82+    os.system('copy build_msvc\\libsecp256k1_config.h src\\secp256k1\\src\\libsecp256k1-config.h')

ken2812221 commented at 2:08 pm on March 12, 2019:

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

in src/compat/byteswap.h:33 in 5502529ef8 outdated

43+#if HAVE_DECL_BSWAP_64 == 0
44+#define bswap_64(x) _byteswap_uint64(x)
45+#endif
46+
47 #else
48 // Non-Mac OS X / non-Darwin

ken2812221 commented at 2:12 pm on March 12, 2019:

nit: and non-MSVC

sipsorcery commented at 7:58 pm on March 12, 2019:

Not sure what you mean here?

in src/qt/walletmodel.h:199 in 5502529ef8 outdated

194@@ -195,14 +195,26 @@ class WalletModel : public QObject
195         bool isValid() const { return valid; }
196 
197         // Copy operator and constructor transfer the context
198-        UnlockContext(const UnlockContext& obj) { CopyFrom(obj); }
199-        UnlockContext& operator=(const UnlockContext& rhs) { CopyFrom(rhs); return *this; }
200+        //UnlockContext(const UnlockContext& obj) { CopyFrom(obj); }
201+        //UnlockContext& operator=(const UnlockContext& rhs) { CopyFrom(rhs); return *this; }

ken2812221 commented at 2:14 pm on March 12, 2019:

Comment code should be removed.

in src/qt/walletmodel.cpp:490 in 5502529ef8 outdated

491+//void WalletModel::UnlockContext::CopyFrom(const UnlockContext& rhs)
492+//{
493+//    // Transfer context; old object no longer relocks wallet
494+//    *this = rhs;
495+//    rhs.relock = false;
496+//}

ken2812221 commented at 2:14 pm on March 12, 2019:

Comment code should be removed.

in src/qt/walletmodel.h:217 in 5502529ef8 outdated

215         WalletModel *wallet;
216         bool valid;
217         mutable bool relock; // mutable, as it can be set to false by copying
218 
219-        void CopyFrom(const UnlockContext& rhs);
220+        //void CopyFrom(const UnlockContext& rhs);

ken2812221 commented at 2:14 pm on March 12, 2019:

Comment code should be removed.

in .appveyor.yml:32 in 5502529ef8 outdated

41-- ps:  for (${i} = 0; ${i} -lt ${files}.length; ${i}++) {
42-           ${content} = (Get-Content ${files}[${i}]);
43-           ${content} = ${content}.Replace("</RuntimeLibrary>", "</RuntimeLibrary><DebugInformationFormat>None</DebugInformationFormat>");
44-           ${content} = ${content}.Replace("<WholeProgramOptimization>true", "<WholeProgramOptimization>false");
45-           Set-Content ${files}[${i}] ${content};
46-       }

ken2812221 commented at 2:26 pm on March 12, 2019:

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.

sipsorcery commented at 7:56 pm on March 12, 2019:

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

in .appveyor.yml:54 in 5502529ef8 outdated

59+        }
60+      }
61+      else {
62+         Write-Host "Qt binaries already present.";
63+      }
64+- cmd: python build_msvc\msvc-autogen.py -qtdir "C:\\Qt5.9.7_ssl_x64_static_vs2017" -protocpath "C:\\Tools\\vcpkg\\installed\\x64-windows-static\\tools\\protobuf\\protoc.exe"

ken2812221 commented at 2:28 pm on March 12, 2019:

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

in build_msvc/bitcoin.sln:244 in 5502529ef8 outdated

408+                                    SolutionGuid = {4ABD1207-9A90-4EC9-A8EB-203638A2605D}
409+                                            SolutionGuid = {2FB733C9-24CB-4BA5-A26B-F43DAD7996B7}
410+                                                    SolutionGuid = {D0CAE2D0-8DB1-4A0B-80EE-800AA6C64323}
411+                            SolutionGuid = {DA7D16A6-E5F0-45B3-B194-C3FE64F1BFCD}
412+          SolutionGuid = {F13BBDD0-1123-4C8F-8DAE-19C38DB7572F}
413+     EndGlobalSection

ken2812221 commented at 2:42 pm on March 12, 2019:

This seems like only changing tab size from 4 to 5?

in src/qt/guiutil.cpp:811 in 5502529ef8 outdated

807@@ -808,7 +808,7 @@ QString formatServicesStr(quint64 mask)
808 
809     // Just scan the last 8 bits for now.
810     for (int i = 0; i < 8; i++) {
811-        uint64_t check = 1 << i;
812+        uint64_t check = 1ULL << i;

ken2812221 commented at 2:50 pm on March 12, 2019:

0        uint64_t check = (uint64_t)1 << i;

follow #14151

ken2812221 commented at 2:51 pm on March 12, 2019: contributor

Nice work!!

sipsorcery force-pushed on Mar 12, 2019

practicalswift commented at 9:11 pm on March 12, 2019: contributor

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 force-pushed on Mar 12, 2019

sipsorcery commented at 9:03 am on March 13, 2019: member

@ken2812221 @practicalswift thx for the reviews, recommendations have been acted on.

Sjors commented at 8:21 pm on March 14, 2019: member

Concept ACK, will test later.

jonasschnelli commented at 6:35 am on March 15, 2019: contributor

Concept ACK

in src/compat/byteswap.h:33 in 514ef5eac4 outdated

29@@ -29,6 +30,20 @@
30 
31 #endif // !defined(bswap_16)
32 
33+#elif _MSC_VER

practicalswift commented at 1:14 pm on March 19, 2019:

Check if it is defined instead?

practicalswift commented at 1:19 pm on March 19, 2019: contributor

Has this snippet been tested?

0        UnlockContext& operator=(const UnlockContext& rhs) {
1            // Transfer context; old object no longer relocks wallet
2            *this = rhs;
3            rhs.relock = false;
4            return *this;
5        }

Could it result in infinite recursion?

in build_msvc/msvc-autogen.py:61 in 514ef5eac4 outdated

56+    with open(os.path.join(SOURCE_DIR, '..\\build_msvc\\Directory.build.props'), 'w', encoding='utf-8',newline='\n') as wfile:
57+        wfile.write(s)
58 
59 def main():
60+    parser = argparse.ArgumentParser(description='Bitcoin-core msbuild configuration initialiser.')
61+    parser.add_argument('-qtdir', nargs='?',help='Optionally sets the directory to use for the static '

practicalswift commented at 1:23 pm on March 19, 2019:

Nit: Could run yapf on the diff to get the proper formatting? See @MarcoFalke’s https://github.com/MarcoFalke/yapf-diff

sipa commented at 0:44 am on May 10, 2019: member

Concept ACK, assuming this causes Appveyor to run the Qt build config.

DrahtBot added the label Needs rebase on May 20, 2019

sipsorcery force-pushed on May 24, 2019

DrahtBot removed the label Needs rebase on May 24, 2019

MarcoFalke referenced this in commit 0221420d1a on Jun 19, 2019

DrahtBot added the label Needs rebase on Jun 19, 2019

sipsorcery force-pushed on Jun 28, 2019

sipsorcery renamed this:
~~Add Qt programs to msvc build~~
Add Qt programs to msvc build (updated, no code changes)
on Jun 28, 2019

sipsorcery force-pushed on Jun 28, 2019

DrahtBot removed the label Needs rebase on Jun 28, 2019

sipsorcery commented at 8:07 am on June 29, 2019: member

Ping @NicolasDorier. Given you’re working on the msbuild set up at the moment a review on this PR would be appreciated.

in build_msvc/README.md:49 in 28ac10e45f outdated

49+
50+The runtime library version (e.g. v141, v142) and platform type (x86 or x64) must also match. OpenSSL must also be linked into the Qt binaries in order to provide full functionality of the Bitcoin Core Qt programs. An example of the configure command to build Qtv5.9.7 locally to link with Bitcoin Core is shown below (adjust paths accordingly), note it can be expected that the configure and subsequent build will fail numerous times until dependency issues are resolved.
51+
52+````
53+..\Qtv5.9.7_src\configure -developer-build -confirm-license -debug-and-release -opensource -platform win32-msvc -opengl desktop -no-shared -static -no-static-runtime -mp -qt-zlib -qt-pcre -qt-libpng -qt-libjpeg -ltcg -make libs -make tools -nomake examples -no-compile-examples -no-dbus -no-libudev -no-qml-debug -no-icu -no-gtk -no-opengles3 -no-angle -no-sql-sqlite -no-sql-odbc -no-sqlite -no-libudev -skip qt3d -skip qtactiveqt -skip qtandroidextras -skip qtcanvas3d -skip qtcharts -skip qtconnectivity -skip qtdatavis3d -skip qtdeclarative -skip qtdoc -skip qtgamepad -skip qtgraphicaleffects -skip qtimageformats -skip qtlocation -skip qtmacextras -skip qtmultimedia -skip qtnetworkauth -skip qtpurchasing -skip qtquickcontrols -skip qtquickcontrols2 -skip qtscript -skip qtscxml -skip qtsensors -skip qtserialbus -skip qtserialport -skip qtspeech -skip qtvirtualkeyboard -skip qtwayland -skip qtwebchannel -skip qtwebengine -skip qtwebsockets -skip qtwebview -skip qtx11extras -skip qtxmlpatterns -nomake tests -openssl-linked -IC:\Dev\github\vcpkg\installed\x64-windows-static\include -LC:\Dev\github\vcpkg\installed\x64-windows-static\lib OPENSSL_LIBS="-llibeay32 -lssleay32 -lgdi32 -luser32 -lwsock32 -ladvapi32" -prefix C:\Qt5.9.7_ssl_x64_static_vs2017
54+````

NicolasDorier commented at 8:51 am on June 29, 2019:

is this meant to work if you ran the vcpkg above?

sipsorcery commented at 9:01 am on June 29, 2019:

Not necessarily. Building Qt is an exercise in perseverance. I typically find the build fails over 10 times before eventually working. This is not very comforting for repeatable and/or reliable builds and a superior alternative would be if the official Qt binaries contained static libraries. My compromise is to provide my own successful build as a download.

in build_msvc/bitcoin-qt/bitcoin-qt.vcxproj:59 in 28ac10e45f outdated

54+  <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
55+    <ClCompile>
56+      <AdditionalIncludeDirectories>$(QtIncludeDir);$(QtIncludeDir)\QtNetwork;$(QtIncludeDir)\QtCore;$(QtIncludeDir)\QtWidgets;$(QtIncludeDir)\QtGui;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
57+    </ClCompile>
58+    <Link>
59+      <AdditionalDependencies>$(QtPluginsLibraryDir)\platforms\qminimal.lib;$(QtPluginsLibraryDir)\platforms\qwindows.lib;$(QtLibraryDir)\qtfreetype.lib;$(QtLibraryDir)\qtharfbuzz.lib;$(QtLibraryDir)\qtlibpng.lib;$(QtLibraryDir)\qtpcre2.lib;$(QtLibraryDir)\Qt5AccessibilitySupport.lib;$(QtLibraryDir)\Qt5Core.lib;$(QtLibraryDir)\Qt5Concurrent.lib;$(QtLibraryDir)\Qt5EventDispatcherSupport.lib;$(QtLibraryDir)\Qt5FontDatabaseSupport.lib;$(QtLibraryDir)\Qt5Gui.lib;$(QtLibraryDir)\Qt5Network.lib;$(QtLibraryDir)\Qt5PlatformCompositorSupport.lib;$(QtLibraryDir)\Qt5ThemeSupport.lib;$(QtLibraryDir)\Qt5Widgets.lib;$(QtLibraryDir)\Qt5WinExtras.lib;$(QtLibraryDir)\qtmain.lib;userenv.lib;netapi32.lib;imm32.lib;Dwmapi.lib;version.lib;winmm.lib;UxTheme.lib;%(AdditionalDependencies)</AdditionalDependencies>

NicolasDorier commented at 8:56 am on June 29, 2019:

why are you duplicating those lines? If a property should be set for any configuration then just wrap it in a ItemDefinitionGroup which does not have a Condition clause.

in build_msvc/bitcoin.sln:228 in 28ac10e45f outdated

234     GlobalSection(SolutionProperties) = preSolution
235         HideSolutionNode = FALSE
236     EndGlobalSection
237     GlobalSection(ExtensibilityGlobals) = postSolution
238-        SolutionGuid = {DA7D16A6-E5F0-45B3-B194-C3FE64F1BFCD}
239+        SolutionGuid = {8AA72EDA-2CD4-4564-B1E4-688B760EEEE9}

NicolasDorier commented at 8:56 am on June 29, 2019:

normal that there is two guid?

in build_msvc/common.init.vcxproj:9 in 28ac10e45f outdated

 5@@ -6,7 +6,12 @@
 6     <VCProjectVersion>16.0</VCProjectVersion>
 7     <VcpkgTriplet Condition="'$(Platform)'=='Win32'">x86-windows-static</VcpkgTriplet>
 8     <VcpkgTriplet Condition="'$(Platform)'=='x64'">x64-windows-static</VcpkgTriplet>
 9-</PropertyGroup>
10+    <QtBaseDir>C:\Qt5.9.7_ssl_x64_static_vs2017</QtBaseDir>

NicolasDorier commented at 8:58 am on June 29, 2019:

<QtBaseDir Condition="'$(QtBaseDir)'==''"> this allow somebody to put Qt where he wants (and specify it in the common.init.vcxproj.user of #16309)

I think that the default value should be empty, even if it crashes.

hebasto commented at 9:22 am on September 8, 2021:

@NicolasDorier

See https://github.com/bitcoin/bitcoin/pull/22890/commits/5e42f2ad26d35950405480f3eea2ed73fdfba841 in #22890.

in build_msvc/common.init.vcxproj:10 in 28ac10e45f outdated

11+    <QtIncludeDir>$(QtBaseDir)\include</QtIncludeDir>
12+    <QtLibraryDir>$(QtBaseDir)\lib</QtLibraryDir>
13+    <QtToolsDir>$(QtBaseDir)\bin</QtToolsDir>
14+    <QtPluginsLibraryDir>$(QtBaseDir)\plugins</QtPluginsLibraryDir>
15+   </PropertyGroup>
16

NicolasDorier commented at 1:40 am on June 30, 2019:

This should be in the bitcoin-qt.vcxproj

in build_msvc/common.init.vcxproj:13 in 28ac10e45f outdated

 5@@ -6,7 +6,12 @@
 6     <VCProjectVersion>16.0</VCProjectVersion>
 7     <VcpkgTriplet Condition="'$(Platform)'=='Win32'">x86-windows-static</VcpkgTriplet>
 8     <VcpkgTriplet Condition="'$(Platform)'=='x64'">x64-windows-static</VcpkgTriplet>
 9-</PropertyGroup>
10+    <QtBaseDir>C:\Qt5.9.7_ssl_x64_static_vs2017</QtBaseDir>
11+    <QtIncludeDir>$(QtBaseDir)\include</QtIncludeDir>
12+    <QtLibraryDir>$(QtBaseDir)\lib</QtLibraryDir>
13+    <QtToolsDir>$(QtBaseDir)\bin</QtToolsDir>
14+    <QtPluginsLibraryDir>$(QtBaseDir)\plugins</QtPluginsLibraryDir>

NicolasDorier commented at 1:41 am on June 30, 2019:

If you remove what is repeated in the bitcoin-qt project, you don’t have to declare anything more than QtBaseDir

NicolasDorier commented at 8:03 am on July 1, 2019: contributor

Btw, would it be a good idea to use QT’s precompiled dynamic library instead of the static lib? We are using the msvc build mainly for debugging stuff so I don’t think it would matter too much and prevent a bunch of headache.

sipsorcery commented at 8:15 am on July 1, 2019: member

@NicolasDorier nooooo :(. You don’t want to know how long I’ve spent getting the Qt build working.

Putting aside the lost chunk of my life it would be a big change switching the build to dynamic linking. All the Bitcoin Core libraries would have to be built as dll’s and there are then bound to be missing exports that will need to be added which would mean code changes.

I’d highly recommend sticking with static linking for consistency with the other Bitcoin Core builds. Hopefully one day the vcpkg static build of Qt will work and allow it to be treated the same as the other dependencies for the msvc build. See https://github.com/microsoft/vcpkg/issues/4560#issuecomment-507157546.

NicolasDorier commented at 12:14 pm on July 3, 2019: contributor

ok ok ! nevermind ;)

So yes, my other points hold though.

sipsorcery force-pushed on Jul 3, 2019

sipsorcery commented at 8:21 pm on July 3, 2019: member

@NicolasDorier I’ve extracted the common Qt settings into common.qt.init.vcxproj. It didn’t make sense to put them into a .user file which should be reserved for config settings users want on their local machine to override the repo settings.

I think that addresses all the outstanding concerns you raised.

sipsorcery force-pushed on Jul 4, 2019

sipsorcery force-pushed on Jul 5, 2019

laanwj commented at 8:25 am on July 31, 2019: member

Looks like this is ready for a final round of ACKs (I cannot contribute here, I know nothing about msvc script and have no windows machines)

laanwj added this to the milestone 0.19.0 on Jul 31, 2019

NicolasDorier commented at 7:43 am on August 1, 2019: contributor

Will try it before final ACK (using the pre compiled binary)

in build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj:123 in 23fb61ee72 outdated

247+    <CleanDependsOn>
248+        QtTestCleanGeneratedFiles;
249+        $(CleanDependsOn);
250+    </CleanDependsOn>
251+  </PropertyGroup>
252+ </Project>

practicalswift commented at 8:33 am on August 1, 2019:

Nit: No newline at end of file.

sipsorcery commented at 6:22 pm on August 2, 2019:

Fixed.

in build_msvc/.gitignore:14 in 23fb61ee72 outdated

 9@@ -10,3 +10,5 @@ packages/*
10 *.vcxproj.user
11 *.vcxproj
12 */Win32
13+libbitcoin_qt/QtGeneratedFiles/*
14+test_bitcoin-qt/QtGeneratedFiles/*

practicalswift commented at 8:34 am on August 1, 2019:

Nit: No newline at end of file.

sipsorcery commented at 6:22 pm on August 2, 2019:

Fixed.

in build_msvc/bitcoin-qt/bitcoin-qt.vcxproj:81 in 23fb61ee72 outdated

76+    </ResourceCompile>
77+  </ItemDefinitionGroup>
78+
79+  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
80+  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
81+</Project>

practicalswift commented at 8:34 am on August 1, 2019:

Nit: No newline at end of file.

sipsorcery commented at 6:22 pm on August 2, 2019:

Fixed.

sipsorcery force-pushed on Aug 2, 2019

in .appveyor.yml:61 in df804502f2 outdated

57@@ -37,10 +58,15 @@ build_script:
58 after_build:
59 - ps:  fsutil behavior set disablelastaccess 1 # Disable Access time feature on Windows (better performance)
60 - ps:  clcache -z
61+#- 7z a bitcoin-%APPVEYOR_BUILD_VERSION%.zip %APPVEYOR_BUILD_FOLDER%\build_msvc\%platform%\%configuration%\*.exe

MarcoFalke commented at 5:01 pm on August 2, 2019:

What are the comments in this file?

sipsorcery commented at 6:21 pm on August 2, 2019:

The comments are there for devs who would like to get artifacts from the appveyor build. The official decision was not to generate binaries from the Bitcoin Core build since it’s not trusted. But it’s still useful to provide the commented steps for devs who make a deliberate decision to enable artifacts.

MarcoFalke commented at 7:08 pm on August 2, 2019:

Ah, so you could keep this 7z command uncommented, but leave the artifacts below commented?

no strong opinion, though

MarcoFalke commented at 5:02 pm on August 2, 2019: member

Concept ACK df804502f20ed229991a356ff4056cccbc598446 (checked that only the msvc folder is modified, so fine with me)

in .appveyor.yml:63 in df804502f2 outdated

57@@ -37,10 +58,15 @@ build_script:
58 after_build:
59 - ps:  fsutil behavior set disablelastaccess 1 # Disable Access time feature on Windows (better performance)
60 - ps:  clcache -z
61+#- 7z a bitcoin-%APPVEYOR_BUILD_VERSION%.zip %APPVEYOR_BUILD_FOLDER%\build_msvc\%platform%\%configuration%\*.exe
62+before_test:
63+- cmd:  move /Y "build_msvc\%PLATFORM%\%CONFIGURATION%\*.exe" src

MarcoFalke commented at 7:08 pm on August 2, 2019:

Why is this needed?

MarcoFalke commented at 7:09 pm on August 2, 2019:

Please see:

[MSVC] Copy build output to src/ automatically after build #16308

sipsorcery commented at 7:20 pm on August 2, 2019:

Good spot. No manual step required to move the binaries for testing since Nicolas added the equivalent task to the project file.

sipsorcery force-pushed on Aug 2, 2019

MarcoFalke commented at 7:34 pm on August 2, 2019: member

re-(unsigned Concept ACK) 37df503062ca60f4a8dd7ebf2c9751615d9b6a7c

in build_msvc/README.md:16 in 37df503062 outdated

12+---------------------
13+The minimal steps required to build Bitcoin Core with the msbuild toolchain are below. More detailed instructions are contained in the following sections.
14+
15+```
16+vcpkg install --triplet x64-windows-static boost-filesystem boost-signals2 boost-test libevent openssl zeromq berkeleydb secp256k1 leveldb rapidcheck
17+python build_msvc\msvc-autogen.py

fanquake commented at 1:13 am on August 4, 2019:

Should this use the new syntax from #16483? py -3 msvc-autogen.py

sipsorcery commented at 5:10 pm on August 10, 2019:

Yes it should. Fixed.

in build_msvc/README.md:30 in 37df503062 outdated

28 - Use Microsoft's [vcpkg](https://docs.microsoft.com/en-us/cpp/vcpkg) to download the source packages and build locally. This is the recommended approach.
29 - Download the source code, build each dependency, add the required include paths, link libraries and binary tools to the Visual Studio project files.
30 - Use [nuget](https://www.nuget.org/) packages with the understanding that any binary files have been compiled by an untrusted third party.
31 
32-The external dependencies required for the Visual Studio build are (see [dependencies.md](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) for more info):
33+The external dependencies required for are (see [dependencies.md](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) for more info):

fanquake commented at 1:17 am on August 4, 2019:

0The [external dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) required for building are:

in build_msvc/README.md:48 in 37df503062 outdated

48+All the Bitcoin Core applications are configured to build with static linking. In order to build the Bitcoin Core Qt applications a static build of Qt is required.
49+
50+The runtime library version (e.g. v141, v142) and platform type (x86 or x64) must also match. OpenSSL must also be linked into the Qt binaries in order to provide full functionality of the Bitcoin Core Qt programs. An example of the configure command to build Qtv5.9.7 locally to link with Bitcoin Core is shown below (adjust paths accordingly), note it can be expected that the configure and subsequent build will fail numerous times until dependency issues are resolved.
51+
52+````
53+..\Qtv5.9.7_src\configure -developer-build -confirm-license -debug-and-release -opensource -platform win32-msvc -opengl desktop -no-shared -static -no-static-runtime -mp -qt-zlib -qt-pcre -qt-libpng -qt-libjpeg -ltcg -make libs -make tools -nomake examples -no-compile-examples -no-dbus -no-libudev -no-qml-debug -no-icu -no-gtk -no-opengles3 -no-angle -no-sql-sqlite -no-sql-odbc -no-sqlite -no-libudev -skip qt3d -skip qtactiveqt -skip qtandroidextras -skip qtcanvas3d -skip qtcharts -skip qtconnectivity -skip qtdatavis3d -skip qtdeclarative -skip qtdoc -skip qtgamepad -skip qtgraphicaleffects -skip qtimageformats -skip qtlocation -skip qtmacextras -skip qtmultimedia -skip qtnetworkauth -skip qtpurchasing -skip qtquickcontrols -skip qtquickcontrols2 -skip qtscript -skip qtscxml -skip qtsensors -skip qtserialbus -skip qtserialport -skip qtspeech -skip qtvirtualkeyboard -skip qtwayland -skip qtwebchannel -skip qtwebengine -skip qtwebsockets -skip qtwebview -skip qtx11extras -skip qtxmlpatterns -nomake tests -openssl-linked -IC:\Dev\github\vcpkg\installed\x64-windows-static\include -LC:\Dev\github\vcpkg\installed\x64-windows-static\lib OPENSSL_LIBS="-llibeay32 -lssleay32 -lgdi32 -luser32 -lwsock32 -ladvapi32" -prefix C:\Qt5.9.7_ssl_x64_static_vs2017

fanquake commented at 1:28 am on August 4, 2019:

This is a bit of an ugly blob to have in the README.md. I assume it will remain fairly static?

You should be able to swap -qt-libjpeg for -no-libjpeg after #16441.

sipsorcery commented at 5:10 pm on August 10, 2019:

I agree it’s very ugly. It could be left out however if another dev wants to build Qt on Windows to link with Bitcoin Core it’s a very useful starting point. I’ve adjusted the libjpeg option in advance.

in build_msvc/README.md:69 in 37df503062 outdated

68@@ -47,4 +69,27 @@ The instructions below use `vcpkg` to install the dependencies.
69     PS >python msvc-autogen.py

fanquake commented at 1:35 am on August 4, 2019:

0     PS >py -3 msvc-autogen.py

fanquake commented at 1:38 am on August 4, 2019: member

Left a few comments inline. Will test on a Windows VM shortly.

sipsorcery force-pushed on Aug 10, 2019

DrahtBot added the label Needs rebase on Aug 16, 2019

sipsorcery force-pushed on Aug 17, 2019

DrahtBot removed the label Needs rebase on Aug 17, 2019

sipsorcery commented at 10:58 am on August 17, 2019: member

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,

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

@NicolasDorier hint hint.

fanquake commented at 12:07 pm on August 19, 2019: member

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

in build_msvc/README.md:15 in ada5dd91d3 outdated

11+Quick Start
12+---------------------
13+The minimal steps required to build Bitcoin Core with the msbuild toolchain are below. More detailed instructions are contained in the following sections.
14+
15+```
16+vcpkg install --triplet x64-windows-static boost-filesystem boost-signals2 boost-test libevent openssl zeromq berkeleydb secp256k1 leveldb rapidcheck

fanquake commented at 4:15 am on September 8, 2019:

double-conversion is missing here

sipsorcery commented at 8:07 am on September 8, 2019:

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 approved

fanquake commented at 4:20 am on September 8, 2019: member

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.

Windows_15529

sipsorcery force-pushed on Sep 8, 2019

fanquake commented at 8:56 am on September 8, 2019: member

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

 0Build FAILED.
 1       "C:\projects\bitcoin\build_msvc\bitcoin.sln" (default target) (1) ->
 2       "C:\projects\bitcoin\build_msvc\bitcoin-qt\bitcoin-qt.vcxproj" (default target) (22) ->
 3       (Link target) -> 
 4         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]
 5         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]
 6         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]
 7         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]
 8         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]
 9         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]
10    0 Warning(s)
11    6 Error(s)
12Time Elapsed 00:19:58.92
13Command exited with code 1

sipsorcery commented at 9:03 am on September 8, 2019: member

@fanquake yep I’m on it.

Added libbitcoin_qt and bitcoin-qt to the msbuild configuration. 1619684322

sipsorcery force-pushed on Sep 8, 2019

fanquake approved

fanquake commented at 1:37 pm on September 8, 2019: member

re-ACK 161968432205a5bdf9a98b99562e956be8c0db89 - AppVeyor looks ok now.

fanquake referenced this in commit 2324aa1dc4 on Sep 11, 2019

fanquake merged this on Sep 11, 2019

fanquake closed this on Sep 11, 2019

sipsorcery deleted the branch on Sep 11, 2019

sidhujag referenced this in commit 5185fb3d53 on Sep 12, 2019

laanwj referenced this in commit 10d96aefa3 on Sep 26, 2019

laanwj referenced this in commit 8d841ad492 on Sep 26, 2019

laanwj referenced this in commit 6288f15f50 on Sep 26, 2019

sidhujag referenced this in commit 647a455be7 on Sep 26, 2019

PierreRochard referenced this in commit b64731bb0a on Oct 12, 2019

HashUnlimited referenced this in commit 7beddfe9a1 on Nov 17, 2019

DrahtBot locked this on Sep 8, 2022

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

Conflicts