Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visual Studio build configuration for Bitcoin Core #11526

Merged
merged 1 commit into from Aug 13, 2018

Conversation

Projects
None yet
@sipsorcery
Copy link
Contributor

commented Oct 19, 2017

This PR allows Bitcoin Core to be relatively easily built with Visual Studio 2017. It's anticipated that it could be useful for devs familiar with Visual Studio and Microsoft's tooling. In particular the ability to use the VS debugger is a big benefit.

Caveats:

  • There are some minor code changes required on Bitcoin Core in order for msvc to be able to successfully compile. I'll submit them in a separate PR, The code changes are available in #11528 #11558 and #11562.
  • The vcpkg for SECP256K1 has not yet been accepted by Microsoft. The files are available from this PR and should be copied into a vcpkg/ports/secp256k1 directory prior to vcpkg install steps.

Update: For anyone wishing to test out the Visual Studio build with the various open pull requests the steps are:

  • Clone and build Vcpkg (Microsoft's new open source C/C++ package manager)
  • Set up Visual Studio to automatically reference vcpkg installs: .\vcpkg integrate install
  • Install the required packages (replace x86 with x64 as required):
    • vcpkg install boost:x86-windows-static
    • vcpkg install libevent:x86-windows-static
    • vcpkg install openssl:x86-windows-static
    • vcpkg install zeromq:x86-windows-static
    • vcpkg install berkeleydb:x86-windows-static
    • vcpkg install secp256k1:x86-windows-static
    • vcpkg install leveldb:x86-windows-static
  • git clone https://github.com/bitcoin/bitcoin.git
  • git checkout -b testbuild
  • git pull origin pull/11526/head # Visual Studio build configuration for Bitcoin Core
  • git pull origin pull/11558/head # Minimal code changes to allow msvc compilation
  • git pull origin pull/11562/head # bench: use std::chrono rather than gettimeofday
  • Copy and unzip attached bitcoin-config.h to src/config, edit as required bitcoin-config.zip
  • git pull origin pull/13031/head # gmtime fix for msvc
  • Build the Visual Studio solution which, if successful, will result in all but the Qt dependent libraries/programs being built. If the build fails please add a comment.

@fanquake fanquake added the Windows label Oct 19, 2017

@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 19, 2017

Concept ACK, this is much-requested.

However IMO to be accepted upstream this needs committed maintenance, not just a one-time drop. Will you maintain this in the future when new files are added? It's bound to break from time to time with build updates (most of us are not able to use this).

@sipa

This comment has been minimized.

Copy link
Member

commented Oct 19, 2017

Also, is there a way to have Travis do an MSVC build? That could make sure that the configuration doesn't bitrot.

@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 19, 2017

Also, is there a way to have Travis do an MSVC build? That could make sure that the configuration doesn't bitrot.

Maybe. I disagree that keeping the MSVC build up to date should be required for merging anything; this would require everyone that changes things to keep MSVC up to date, which most are not able to do because they do not run that platform.

build_msvc/README.md Outdated
---------------------
The instructions below use vcpkg to install the dependencies.

- Clone and vcpkg from the [github repository](https://github.com/Microsoft/vcpkg) and install as per the instructions in the main README.md,

This comment has been minimized.

Copy link
@practicalswift

practicalswift Oct 19, 2017

Member

Nit: s/,$/./

build_msvc/README.md Outdated

- Download the source code, build each dependency, add the required include paths, link libraries and binary tools to the Visual Studio project files,
- Use [nuget](https://www.nuget.org/) packages with the understanding that any binary files have been compiled by an untrusted third party,
- 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.

This comment has been minimized.

Copy link
@practicalswift

practicalswift Oct 19, 2017

Member

Nit: Start with the recommended approach.

Nit: s/,$/./ :-)

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch to e745900 Oct 19, 2017

@jonasschnelli

This comment has been minimized.

Copy link
Member

commented Oct 19, 2017

I'm not sure if we should include this in the main repository. We had similar discussions in #5276 (Xcode project).
AFAIK, those "project files" tend to get outdated very fast. IMO the authors of MSVC and Xcode do not take too much care about backwards compatibility of their file formats...
Also, merging this may lead to someone trying to include a CMake or XCode project.

I'd prefer an external gist/repository and maybe a link to that project (easier to remove if outdated).

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch 2 times, most recently from b2a30bb Oct 19, 2017

@meshcollider

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

Hmm the whitespace lint checker is going crazy, it might need some modifications because its pretty hard to me to see what its catching here

Edit: there's definitely some tab characters mixed up in there, maybe that's the problem

Edit 2: Nope there's definitely a lot of trailing whitespace too, looks like its working fine, there's just a lot to process. Might want to do a regex find and replace to save yourself doing it manually :)

@sipsorcery

This comment has been minimized.

Copy link
Contributor Author

commented Oct 20, 2017

@meshcollider I'm tracking down the lint failure. I've already removed all tabs and trimmed all end of line whitespace with sed. Looks like there's a rule somewhere that requires certain types of files to have end of line as the last character. Will keep digging.

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch Oct 20, 2017

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

@sipsorcery When saving the file in your editor, choose "Linux encoding" instead of "Windows encoding" for new lines.

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch Oct 20, 2017

@sipsorcery

This comment has been minimized.

Copy link
Contributor Author

commented Oct 20, 2017

No '\r|\t' are in the diff patch now so hopefully lint will be happy.

If this PR does go through there will be an issue with the lint rules. In my troubleshooting I've discovered Visual Studio will add tab characters into a solution (.sln) file whenever it's opened. It possibly does the same for it's other .vcproj, .vcxproj.user and .vcxproj.filters files as well although it didn't during my quick testing.

I could add a new rule to contrib/devtools/lint-whitespace.sh for these file types but I think I'll leave it until a decision on this PR is made.

@sipsorcery

This comment has been minimized.

Copy link
Contributor Author

commented Oct 20, 2017

@laanwj

However IMO to be accepted upstream this needs committed maintenance, not just a one-time drop. Will you maintain this in the future when new files are added? It's bound to break from time to time with build updates (most of us are not able to use this).

Yes.

If the VS build config gets added I will commit to maintaining it for a minimum of 3 years (end of 2020) excepting death, disability etc. As character assessment I've been an open source contributor since the early noughties: sourceforge, codeplex, bitbucket, github etc; no project highlights that will impress you guys but the relevant point is that I'm capable of hanging around.

I'd also expect the VS config to become popular in a short space of time which would reduce the maintenance burden.

@practicalswift

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

@sipsorcery

Is there some tool to generate the vcxproj, vcxproj.filters and sln files programmatically from an easy-to-read text format that allows for easy tracking over time?

The very verbose XML build files contain a lot of duplication and random artifacts, so the same data should be possible to represent in a much more compact and human friendly way.

The following files 34 files are quite hard to reason about and/or review in detail due to the large amount of redundancy and general verbosity:

build_msvc/bench_bitcoin/bench_bitcoin.vcxproj
build_msvc/bench_bitcoin/bench_bitcoin.vcxproj.filters
build_msvc/bitcoin-cli/bitcoin-cli.vcxproj
build_msvc/bitcoin-tx/bitcoin-tx.vcxproj
build_msvc/bitcoin-tx/bitcoin-tx.vcxproj.filters
build_msvc/bitcoin.sln
build_msvc/bitcoind/bitcoind.vcxproj
build_msvc/bitcoind/bitcoind.vcxproj.filters
build_msvc/libbitcoin_cli/libbitcoin_cli.vcxproj
build_msvc/libbitcoin_cli/libbitcoin_cli.vcxproj.filters
build_msvc/libbitcoin_common/libbitcoin_common.vcxproj
build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.filters
build_msvc/libbitcoin_crypto/libbitcoin_crypto.vcxproj
build_msvc/libbitcoin_crypto/libbitcoin_crypto.vcxproj.filters
build_msvc/libbitcoin_qt/libbitcoin_qt.vcxproj
build_msvc/libbitcoin_qt/libbitcoin_qt.vcxproj.filters
build_msvc/libbitcoin_server/libbitcoin_server.vcxproj
build_msvc/libbitcoin_server/libbitcoin_server.vcxproj.filters
build_msvc/libbitcoin_util/libbitcoin_util.vcxproj
build_msvc/libbitcoin_util/libbitcoin_util.vcxproj.filters
build_msvc/libbitcoin_wallet/libbitcoin_wallet.vcxproj
build_msvc/libbitcoin_wallet/libbitcoin_wallet.vcxproj.filters
build_msvc/libbitcoin_zmq/libbitcoin_zmq.vcxproj
build_msvc/libbitcoin_zmq/libbitcoin_zmq.vcxproj.filters
build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj
build_msvc/libleveldb/libleveldb.vcxproj
build_msvc/libleveldb/libleveldb.vcxproj.filters
build_msvc/libunivalue/libunivalue.vcxproj
build_msvc/libunivalue/libunivalue.vcxproj.filters
build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj
build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj.filters
build_msvc/test_bitcoin/test_bitcoin.vcxproj
build_msvc/test_bitcoin/test_bitcoin.vcxproj.filters
build_msvc/testconsensus/testconsensus.vcxproj
@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

Is there some tool to generate the vcxproj, vcxproj.filters and sln files programmatically from an easy-to-read text format that allows for easy tracking over time?

cmake can do that. Though I'm not sure about easy-to-read. Well easier to read than XML files, I guess. But I don't think adding an indirection step makes sense unless we'd use cmake as main build system, and that's certainly not planned and would probably piss of @theuni.
These files aren't really meant for reading, it's just the internal configuration of a GUI program.

If the VS build config gets added I will commit to maintaining it for a minimum of 3 years (end of 2020) excepting death, disability etc. As character assessment I've been an open source contributor since the early noughties: sourceforge, codeplex, bitbucket, github etc; no project highlights that will impress you guys but the relevant point is that I'm capable of hanging around.

Thanks. That's good enough for me :) You have the motiviation. You can never know for sure what happens in the future.

@jonasschnelli

I'd prefer an external gist/repository and maybe a link to that project (easier to remove if outdated).

Maybe - though it's easier for it to get lost in that case. There are AFAIK already tries at separate repositories with MSVC changes and build system, but they always get stuck at a certain version. Having it in the main repository is more of a incentive to keep it up to date. Probably.

@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

Maybe - though it's easier for it to get lost in that case. There are AFAIK already tries at separate repositories with MSVC changes and build system, but they always get stuck at a certain version. Having it in the main repository is more of a incentive to keep it up to date. Probably.

If we do go that route with a separate repository, we should create an 'official' repository under bitcoin-core at least instead of it only existing on some user's account.

An added advantage is that we can add @sipsorcery as committer. Delegating access is easier with separate repositories.

@practicalswift

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

@laanwj I'm also thinking in terms of reproducibility. Adding 34 files to the main Bitcoin repo with a lot of redundancy (between files and within each file) that cannot be created from scratch using widely available free tools doesn't feel quite right (almost like having some magic binary blob in the repo!) :-) I suggest we go with an approach that makes changes to the build process (read diffs) easy to reason about.

This is an approach taken by another project - a simple shell script generating the files largely from what is already in the repo: https://github.com/robotdad/vclinux/blob/master/bash/genvcxproj.sh

On the other hand if this is not part of the main repo then perhaps we can have a lower bar with regards to readability/reproducibility/understandability/reviewability :-)

@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

We need an approach that makes changes to the build process (read diffs) easy to reason about.

Sure, that would be good, but the primary goal here is to make it easier for windows developers to build. And having to run a bash script isn't contributing to that.

Edit: THough we could have a generation script that is automatically run before releases, so the actual vcproj files would be part of the tarball but never the repository. Only developers using checkouts from git would then be required to run it themselves.

On the other hand if this is not part of the main repo then perhaps we can have a lower bar with regards to readability/reproducibility/understandability/reviewability :-)

Ok that would be another argument for having a separate repo.

build_msvc/bitcoind/bitcoind.vcxproj Outdated
</ItemGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
<ImportGroup Label="ExtensionTargets">
<!--<Import Label="miniupnpcTarget" Project="f:\deps\miniupnpc\miniupnpc.targets" />-->

This comment has been minimized.

Copy link
@practicalswift

practicalswift Oct 20, 2017

Member

This can safely be removed? :-)

build_msvc/libbitcoin_server/libbitcoin_server.vcxproj Outdated
</ItemGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
<ImportGroup Label="ExtensionTargets">
<!--<Import Label="miniupnpcTarget" Project="F:\deps\miniupnpc\miniupnpc.targets" />-->

This comment has been minimized.

Copy link
@practicalswift

practicalswift Oct 20, 2017

Member

This can safely be removed? :-)

build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj Outdated
<PreprocessorDefinitions>WIN32;_DEBUG;_CONSOLE;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<SDLCheck>false</SDLCheck>
<AdditionalIncludeDirectories>..\..\src;</AdditionalIncludeDirectories>
<LanguageStandard>stdcpp14</LanguageStandard>

This comment has been minimized.

Copy link
@practicalswift

practicalswift Oct 20, 2017

Member

Bitcoin is currently at C++11, so please change this instance of "stdcpp14" and the other four instances :-)

@practicalswift

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

@laanwj I thought bash was widely available in Windows land these days thanks to the Windows Subsystem for Linux (WSL), but perhaps it is not yet as widely used as I thought :-)

@practicalswift

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

@laanwj Good point about scripted generation that is automatically run before releases. Seems like a good solution!

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch Oct 20, 2017

@sipsorcery

This comment has been minimized.

Copy link
Contributor Author

commented Oct 20, 2017

Attempting to generate the Visual Studio project files using a custom shell script will be a journey down the rabbit hole. Yes you might get a more succinct representation but maintaining the script would be a much larger burden.

The only viable approach I know of is to use cmake but even then you'll lose some devs due to the extra steps required. And while cmake is an excellent tool it does generate a lot of redundant configuration information in its VS config.

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch to dd5df9f Oct 20, 2017

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch 3 times, most recently Apr 11, 2018

build_msvc/bitcoin-config.h Outdated

#ifndef BITCOIN_CONFIG_H

#define BITCOIN_CONFIG_H

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Apr 11, 2018

Member

You might have to update the linter to avoid spurious warnings:

diff --git a/contrib/devtools/lint-include-guards.sh b/contrib/devtools/lint-include-guards.sh
index 6a0dd556bb..d9cf8effee 100755
--- a/contrib/devtools/lint-include-guards.sh
+++ b/contrib/devtools/lint-include-guards.sh
@@ -9,7 +9,7 @@
 HEADER_ID_PREFIX="BITCOIN_"
 HEADER_ID_SUFFIX="_H"
 
-REGEXP_EXCLUDE_FILES_WITH_PREFIX="src/(crypto/ctaes/|leveldb/|secp256k1/|tinyformat.h|univalue/)"
+REGEXP_EXCLUDE_FILES_WITH_PREFIX="(build_msvc/|src/(crypto/ctaes/|leveldb/|secp256k1/|tinyformat.h|univalue/))"
 
 EXIT_CODE=0
 for HEADER_FILE in $(git ls-files -- "*.h" | grep -vE "^${REGEXP_EXCLUDE_FILES_WITH_PREFIX}")

Warning was:

build_msvc/bitcoin-config.h seems to be missing the expected include guard:
  #ifndef BITCOIN_BITCOIN-CONFIG_H
  #define BITCOIN_BITCOIN-CONFIG_H
  ...
  #endif // BITCOIN_BITCOIN-CONFIG_H
^---- failure generated from contrib/devtools/lint-include-guards.sh
@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Apr 11, 2018

re-utACK fcc8dcb. Thanks for addressing the nits

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Apr 12, 2018

@sipsorcery Don't worry about the include guard. See #12956

@sipsorcery

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2018

@MarcoFalke the lint parser still catches that header as it's in the build_msvc directory.

REGEXP_EXCLUDE_FILES_WITH_PREFIX="src/(crypto/ctaes/|leveldb/|secp256k1/|tinyformat.h|univalue/)"

I'll find a way to workaround it.

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch Apr 27, 2018

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch 2 times, most recently Jun 13, 2018

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch Jul 16, 2018

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch Jul 25, 2018

@sipsorcery sipsorcery referenced this pull request Jul 30, 2018

Open

test_bitcoin results #28

@ken2812221

This comment has been minimized.

Copy link
Member

commented Aug 8, 2018

Installing the following boost packages should be enough instead of all boost packages:

  • boost-filesystem
  • boost-signals2
  • boost-interprocess
  • boost-test

@MarcoFalke MarcoFalke changed the title Visual Studio build configuration for Bitcoin Core. Visual Studio build configuration for Bitcoin Core Aug 8, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Aug 8, 2018

Could you please squash the commits a bit to get rid of the intermediate merge commit? For example something like this:

git fetch upstream
git merge upstream/master
git reset --soft upstream/master
git commit -m "Visual Studio build configuration for Bitcoin Core"
git push origin build_msvc -f

@sipsorcery sipsorcery force-pushed the sipsorcery:build_msvc branch to ef7beae Aug 10, 2018

@sipsorcery

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2018

@MarcoFalke sure, done.

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Aug 13, 2018

ACK ef7beae

@MarcoFalke MarcoFalke merged commit ef7beae into bitcoin:master Aug 13, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Aug 13, 2018

Merge bitcoin#11526: Visual Studio build configuration for Bitcoin Core
ef7beae Visual Studio build configuration for Bitcoin Core (Aaron Clauson)

Pull request description:

  This PR allows Bitcoin Core to be relatively easily built with Visual Studio 2017. It's anticipated that it could be useful for devs familiar with Visual Studio and Microsoft's tooling. In particular the ability to use the VS debugger is a big benefit.

  ~~Caveats:~~
  - ~~There are some minor code changes required on Bitcoin Core in order for msvc to be able to successfully compile. I'll submit them in a separate PR, The code changes are available in bitcoin#11528 bitcoin#11558 and bitcoin#11562~~.
  - ~~The vcpkg for SECP256K1 has not yet been accepted by Microsoft. The files are available from this [PR](microsoft/vcpkg#2005) and should be copied into a vcpkg/ports/secp256k1 directory prior to vcpkg install steps.~~

  **Update:** For anyone wishing to test out the Visual Studio build with the various open pull requests the steps are:

  - Clone and build [Vcpkg](https://github.com/Microsoft/vcpkg) (Microsoft's new open source C/C++ package manager)
      - git clone https://github.com/Microsoft/vcpkg
      - .\bootstrap-vcpkg.bat
  - Set up Visual Studio to automatically reference vcpkg installs: .\vcpkg integrate install
  - Install the required packages (replace x86 with x64 as required):
      - vcpkg install boost:x86-windows-static
      - vcpkg install libevent:x86-windows-static
      - vcpkg install openssl:x86-windows-static
      - vcpkg install zeromq:x86-windows-static
      - vcpkg install berkeleydb:x86-windows-static
      - vcpkg install secp256k1:x86-windows-static
      - vcpkg install leveldb:x86-windows-static
  - git clone https://github.com/bitcoin/bitcoin.git
  - git checkout -b testbuild
  - git pull origin pull/11526/head # Visual Studio build configuration for Bitcoin Core
  - ~~git pull origin pull/11558/head # Minimal code changes to allow msvc compilation~~
  - ~~git pull origin pull/11562/head # bench: use std::chrono rather than gettimeofday~~
  - ~~Copy and unzip attached bitcoin-config.h to src/config, edit as required [bitcoin-config.zip](https://github.com/bitcoin/bitcoin/files/1429484/bitcoin-config.zip)~~
  - ~~git pull origin pull/13031/head # gmtime fix for msvc~~
  - Build the Visual Studio solution which, if successful, will result in all but the Qt dependent libraries/programs being built. If the build fails please add a comment.

Tree-SHA512: 5cd17273d33a09c35d8534c9f49123dec60ec05383669c67674b2cac88ada177bf94d7731c2a827759444f18d4b67085b91b02458124d0c32ab3a8f72ba5dac9
@ysangkok

This comment has been minimized.

Copy link

commented Aug 20, 2018

The commit description is wrong, the secp256k1 PR has been merged into vcpkg in Oct 2017: microsoft/vcpkg#2005

@sipsorcery

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2018

@ysangkok not sure which commit message/comment you are referring to but yes I did a PR for libsecp256k1 and vcpkg at around the same time I did this PR. The upshot of it is that all the dependencies for building bitcoin core, sans the Qt projects, can be obtained with vcpkg.

@ysangkok

This comment has been minimized.

Copy link

commented Aug 20, 2018

I was referring to afa9600 . The issue seems to be that Git doesn't know about markdown so I couldn't see the strike-through...

MarcoFalke added a commit that referenced this pull request Apr 29, 2019

Merge #15908: docs: Align MSVC build options with Linux build ones
e47dc4f Include bitcoin_config.h in release process (Hennadii Stepanov)
48ed65b Align MSVC build options with Linux build ones (Hennadii Stepanov)

Pull request description:

  Ref:
  - #11526
  - #15903 (comment)
  - #15903 (comment) by MarcoFalke

ACKs for commit e47dc4:
  MarcoFalke:
    utACK e47dc4f
  Sjors:
    utACK e47dc4f
  fanquake:
    utACK e47dc4f
  practicalswift:
    utACK e47dc4f

Tree-SHA512: 32ac3e9fd0b41a4916dd520bdf8bb6c71cea7218434b67a173b51b3cdb0da3f10a68b9e5205c27a52f456ac9ed14f8f8363a50d108a80a5dd55b085a6bd435b9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.