build: The vcpkg tool has introduced a proper way to use manifests #19960

sipsorcery · 2020-09-15T19:11:18Z

The vcpkg tool has introduced a proper way to use manifests. This PR replaces the custom text file mechanism with the new manifest approach.

It is planned that vckpg manifests will include the ability to version dependencies in the future. Dependency versions would solve a number of issues that currently require workarounds with the appveyor CI.

fanquake · 2020-09-15T22:06:44Z

Concept ACK

hebasto · 2020-09-19T09:43:43Z

Concept ACK.

build_msvc/vcpkg.json

maflcko · 2020-09-19T10:02:11Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets #19077 (wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets by achow101)

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.

build_msvc/README.md

maflcko

Please squash and format the commit message with subject line and body

.appveyor.yml

build_msvc/README.md

maflcko

Approach ACK b92a074

fanquake · 2020-09-29T12:38:58Z

This looks good now. However can you follow up with:

Please squash and format the commit message with subject line and body

The vcpkg tool has introduced a proper way to use manifests, https://devblogs.microsoft.com/cppblog/vcpkg-accelerate-your-team-development-environment-with-binary-caching-and-manifests/. This PR replaces the custom text file mechanism with the new manifest approach. It is planned that vckpg manifests will include the ability to version dependencies in the future. Dependency versions would solve a number of issues that currently require workarounds with the appveyor CI. Set vcpkg manifest version to 1 to avoid any perception it's related to any release or other version numbering.

sipsorcery · 2020-09-29T12:51:03Z

I've adjusted my commit message to have a better lead (subject) line. Was that the problem?

fanquake

ACK 712f95d - This is a nice simplification. I tested this in a Windows VM; packages were downloaded and installed automatically as required:

I've adjusted my commit message to have a better lead (subject) line. Was that the problem?

Thanks. Yes it was.

One other thought. Is there any Appveyor related cache me might need to nuke to make sure this is really working there as expected?

hebasto · 2020-09-29T13:14:27Z

.appveyor.yml

@@ -11,25 +11,19 @@ environment:
  QT_DOWNLOAD_HASH: '9a8c6eb20967873785057fdcd329a657c7f922b0af08c5fde105cc597dd37e21'
  QT_LOCAL_PATH: 'C:\Qt5.9.8_x64_static_vs2019'
  VCPKG_INSTALL_PATH: 'C:\tools\vcpkg\installed'
-  VCPKG_COMMIT_ID: 'f3f329a048eaff759c1992c458f2e12351486bc7'
+  VCPKG_COMMIT_ID: '13590753fec479c5b0a3d48dd553dde8d49615fc'


Why the latest release, i.e., microsoft/vcpkg@56fffbe, is not used?

No good reason. I've always built from the vcpkg master branch, checked that the dependencies work and used that commit. The vcpkg repo is a bit different to code only repos in that commits are made for the library port configs as well as the source code use to build the vcpkg executable.

hebasto · 2020-09-29T13:18:43Z

build_msvc/.gitignore

@@ -24,3 +24,4 @@ libtest_util/libtest_util.vcxproj
 */Win32
 libbitcoin_qt/QtGeneratedFiles/*
 test_bitcoin-qt/QtGeneratedFiles/*
+vcpkg_installed


nit: add EOL?

hebasto · 2020-09-29T13:23:07Z

build_msvc/README.md

- libevent
- Qt5
- ZeroMQ
+The [external dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) required for building are listed in the `build_msvc/vcpkg.json` file. The `msbuild` project files are configured to automatically install the `vcpkg` dependencies.


Suggested change

The [external dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) required for building are listed in the `build_msvc/vcpkg.json` file. The `msbuild` project files are configured to automatically install the `vcpkg` dependencies.

The [external dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) required for building are listed in the [`build_msvc/vcpkg.json`](https://github.com/bitcoin/bitcoin/tree/master/build_msvc/vcpkg.json) file. The `msbuild` project files are configured to automatically install the `vcpkg` dependencies.

sipsorcery · 2020-09-29T13:23:27Z

One other thought. Is there any Appveyor related cache me might need to nuke to make sure this is really working there as expected?

Nope. All appveyor caching has already been removed. Predominantly due to nasty issues with the cached vcpkg dependencies getting messed up between different PR branches.

With this new manifest approach it might be feasible to re-introduce a cache of the vcpkg dependencies at some point, and save 20mins per build, but best to leave that until the manifest change is bedded down.

hebasto · 2020-09-29T13:24:16Z

build_msvc/vcpkg.json

+    },
+    "zeromq"
+  ]
+}


hebasto

Approach ACK 712f95d, I've verified that changes comply MS docs:

…y to use manifests 712f95d Update msvc build to use new vcpkg manifest (Aaron Clauson) Pull request description: The vcpkg tool has introduced a proper way to use [manifests](https://devblogs.microsoft.com/cppblog/vcpkg-accelerate-your-team-development-environment-with-binary-caching-and-manifests/). This PR replaces the custom text file mechanism with the new manifest approach. It is planned that vckpg manifests will include the ability to version dependencies in the future. Dependency versions would solve a number of issues that currently require workarounds with the appveyor CI. ACKs for top commit: fanquake: ACK 712f95d - This is a nice simplification. I tested this in a Windows VM; packages were downloaded and installed automatically as required: hebasto: Approach ACK 712f95d, I've verified that changes comply MS docs: Tree-SHA512: ff9b3d6ad3cacabcbec6566fd289b179af163dc0c4545f8ba666fc14ba07527557f72bc84ba8abfa3bdffb22e2b8ff0b180f41d909c6de76894ac50ddcf8646b

hebasto · 2020-10-02T16:25:52Z

@sipsorcery Could you look into the recent "error MSB3073" AppVeyor fails?

sipsorcery · 2020-10-02T18:15:14Z

So far haven't found anything likely that changed around 5 hours ago when the builds started failing. Will keep digging.

maflcko · 2020-10-28T17:22:57Z

This may have increased the average msbuild time (at least on appveyor)

sipsorcery · 2020-10-29T10:08:13Z

@MarcoFalke yes I see what you mean. It seems that build takes approximately 10 minutes longer subsequent to the change in this PR.

One possibility is the vcpkg dependencies may now be being built for both debug and release. Previously the line below restricted them to only release. I'll do some testing.

Add-Content "C:\tools\vcpkg\triplets\$env:PLATFORM-windows-static.cmake" "set(VCPKG_BUILD_TYPE release)"

sipsorcery · 2020-11-25T10:54:54Z

Finally got around to running the build tests to compare the build durations using the original vcpkg install mechanism and the new vcpkg manifest mechanism.

The original way seems to be a few minutes faster but not a smoking gun for a 10 minute increase. Perhaps the manifest + the removal of the set(VCPKG_BUILD_TYPE release) could account for it. The VCPKG_BUILD_TYPE setting should be put back and I'll create a PR for that.

Build runs with vcpkg installed from command line:
Average duration approx. 25 minutes.

Run 1:
c:\Tools\vcpkg>vcpkg install --triplet x64-windows-static berkeleydb boost-filesystem boost-multi-index boost-process boost-signals2 boost-test boost-thread libevent[thread] zeromq double-conversion sqlite3 -> 14:12
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 10:37

Run 2:
c:\Tools\vcpkg>vcpkg install --triplet x64-windows-static berkeleydb boost-filesystem boost-multi-index boost-process boost-signals2 boost-test boost-thread libevent[thread] zeromq double-conversion sqlite3 -> 16:10
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 09:40

Run 3:
c:\Tools\vcpkg>vcpkg install --triplet x64-windows-static berkeleydb boost-filesystem boost-multi-index boost-process boost-signals2 boost-test boost-thread libevent[thread] zeromq double-conversion sqlite3 -> 15:29
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 09:36

Run 4:
c:\Tools\vcpkg>vcpkg install --triplet x64-windows-static berkeleydb boost-filesystem boost-multi-index boost-process boost-signals2 boost-test boost-thread libevent[thread] zeromq double-conversion sqlite3 -> 15:12
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 09:39

Build runs with vcpkg installed using manifest mechanism:
Average duration approx. 28 minutes.

Run 1:
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 26:58

Run 2:
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 27:00

Run 3:
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 27:15

Run 4:
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 30:31

Run 5:
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 30:31

Run 6:
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msvc-autogen.py
c:\Dev\github\sipsorcery_bitcoin\build_msvc>msbuild /m bitcoin.sln /p:Configuration=Release /p:Platform=x64 /t:clean,build -> 24:51

…ge introduced in bitcoin#19960. It re-applies a setting to inform vcpkg to only build release vesions of the dependencies rather than the default of debug and release. It had been expected that the vcpkg manifest mechanism introduced in bitcoin#19960 would do this automatically but it turns out not to be the case.

… debug) to reduce build times fa18e7c This change to the appveyor CI config for msvc builds reverses a change introduced in #19960. It re-applies a setting to inform vcpkg to only build release vesions of the dependencies rather than the default of debug and release. (Aaron Clauson) Pull request description: This change to the appveyor CI config for msvc builds reverses a change introduced in #19960. It re-applies a setting to inform vcpkg to only build release versions of the dependencies rather than the default of debug and release. It had been expected that the vcpkg manifest mechanism introduced in #19960 would do this automatically but it turns out not to be the case. ACKs for top commit: MarcoFalke: ACK fa18e7c if green hebasto: ACK fa18e7c, AppVeyor build takes 5 minutes less. Tree-SHA512: 427e7e78190c20e0d85dad9b29beed2b6fa13f99c6bc72bcc1839dfb51237a7cc785ab707b4f851c527c1bb0d3e7ebad9e640969e19d29778584bbaeec75cecf

…se (not debug) to reduce build times fa18e7c This change to the appveyor CI config for msvc builds reverses a change introduced in bitcoin#19960. It re-applies a setting to inform vcpkg to only build release vesions of the dependencies rather than the default of debug and release. (Aaron Clauson) Pull request description: This change to the appveyor CI config for msvc builds reverses a change introduced in bitcoin#19960. It re-applies a setting to inform vcpkg to only build release versions of the dependencies rather than the default of debug and release. It had been expected that the vcpkg manifest mechanism introduced in bitcoin#19960 would do this automatically but it turns out not to be the case. ACKs for top commit: MarcoFalke: ACK fa18e7c if green hebasto: ACK fa18e7c, AppVeyor build takes 5 minutes less. Tree-SHA512: 427e7e78190c20e0d85dad9b29beed2b6fa13f99c6bc72bcc1839dfb51237a7cc785ab707b4f851c527c1bb0d3e7ebad9e640969e19d29778584bbaeec75cecf

…ge introduced in bitcoin#19960. It re-applies a setting to inform vcpkg to only build release vesions of the dependencies rather than the default of debug and release. It had been expected that the vcpkg manifest mechanism introduced in bitcoin#19960 would do this automatically but it turns out not to be the case. Github-Pull: bitcoin#20489 Rebased-From: fa18e7c

b1c0f97 [doc] Add permissions to the getpeerinfo help. (Amiti Uttarwar) 1fda7db rpc: Add missing description of vout in getrawtransaction help text (Ben Carman) ef7a155 qt: Align layout of checkboxes (Hennadii Stepanov) 35a10e4 Add patch to make codesign_allocate compatible with Apple's (Pieter Wuille) e70ccb0 doc: update -externalip documentation in tor.md (Jon Atack) 2c8482d doc: add tor.md section on how to get tor info via bitcoind (Jon Atack) 0c1fa78 doc: update tor.md address examples from onion v2 to v3 (Jon Atack) 84e8d54 doc: warn that incoming conns are unlikely when not using default ports (Adam Jonas) e4440eb doc: Add warnings for http interfaces limitations (Fabian Jahr) 85dabd1 Removed redundant git pull from appveyor config. (Aaron Clauson) 249d61a Adjusted msvc compiler and linker settings to remove optimisations that are causing sporadic ABI issues on Visual Studio updates. (Aaron Clauson) e7b53d4 This change to the appveyor CI config for msvc builds reverses a change introduced in #19960. It re-applies a setting to inform vcpkg to only build release vesions of the dependencies rather than the default of debug and release. (Aaron Clauson) 8273ea3 Move signet onion seed from v2 to v3 (Sjors Provoost) Pull request description: The remaining backports to get rc4 out. Currently only waiting on the macOS build fix. ACKs for top commit: benthecarman: ACK b1c0f97 Sjors: ACK b1c0f97 Tree-SHA512: 53eaecd531ba461678917cb630d67f1e6bb737d64022abe971eaced6eca366c9ed593e44276bd9c7ad7b3aebe3850d2d29282eb310e10b547986d10fe77a8482

DrahtBot added Docs Tests labels Sep 15, 2020

sipsorcery force-pushed the vcpkg_manifest branch from 7109cb8 to 45f4d94 Compare September 15, 2020 20:51

fanquake added the Windows label Sep 15, 2020

sipsorcery changed the title ~~WIP: The vcpkg tool has introduced a proper way to use manifests~~ Appveyor CI: The vcpkg tool has introduced a proper way to use manifests Sep 15, 2020

kiminuo mentioned this pull request Sep 17, 2020

Replace boost::filesystem with std::filesystem #19245

Closed

5 tasks

maflcko reviewed Sep 19, 2020

View reviewed changes

build_msvc/vcpkg.json Outdated Show resolved Hide resolved

maflcko added Build system and removed Docs Tests labels Sep 19, 2020

maflcko changed the title ~~Appveyor CI: The vcpkg tool has introduced a proper way to use manifests~~ build: The vcpkg tool has introduced a proper way to use manifests Sep 19, 2020

fanquake reviewed Sep 19, 2020

View reviewed changes

build_msvc/README.md Outdated Show resolved Hide resolved

DrahtBot mentioned this pull request Sep 19, 2020

wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets #19077

Merged

maflcko approved these changes Sep 20, 2020

View reviewed changes

.appveyor.yml Outdated Show resolved Hide resolved

fanquake reviewed Sep 20, 2020

View reviewed changes

build_msvc/README.md Outdated Show resolved Hide resolved

sipsorcery force-pushed the vcpkg_manifest branch from 8c95190 to b92a074 Compare September 24, 2020 08:23

maflcko approved these changes Sep 24, 2020

View reviewed changes

sipsorcery force-pushed the vcpkg_manifest branch from b92a074 to 712f95d Compare September 29, 2020 12:50

fanquake approved these changes Sep 29, 2020

View reviewed changes

hebasto reviewed Sep 29, 2020

View reviewed changes

build_msvc/vcpkg.json

},

"zeromq"

]

}

Copy link

Member

hebasto Sep 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add EOL?

hebasto reviewed Sep 29, 2020

View reviewed changes

maflcko merged commit 36f5a58 into bitcoin:master Sep 30, 2020

sipsorcery mentioned this pull request Nov 25, 2020

CI msvc: only build vcpkg dependencies for release (not debug) to reduce build times #20489

Merged

sipsorcery mentioned this pull request Dec 2, 2020

ci: AppVeyor fixes for Visual Studio 2019 16.8.1 image #20506

Merged

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

	The [external dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) required for building are listed in the `build_msvc/vcpkg.json` file. The `msbuild` project files are configured to automatically install the `vcpkg` dependencies.
	The [external dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) required for building are listed in the [`build_msvc/vcpkg.json`](https://github.com/bitcoin/bitcoin/tree/master/build_msvc/vcpkg.json) file. The `msbuild` project files are configured to automatically install the `vcpkg` dependencies.

build: The vcpkg tool has introduced a proper way to use manifests #19960

build: The vcpkg tool has introduced a proper way to use manifests #19960

Uh oh!

Conversation

sipsorcery commented Sep 15, 2020

Uh oh!

fanquake commented Sep 15, 2020

Uh oh!

hebasto commented Sep 19, 2020

Uh oh!

Uh oh!

maflcko commented Sep 19, 2020 • edited by DrahtBot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake commented Sep 29, 2020

Uh oh!

sipsorcery commented Sep 29, 2020

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto Sep 29, 2020

Choose a reason for hiding this comment

Uh oh!

sipsorcery Sep 29, 2020

Choose a reason for hiding this comment

Uh oh!

hebasto Sep 29, 2020

Choose a reason for hiding this comment

Uh oh!

hebasto Sep 29, 2020

Choose a reason for hiding this comment

Uh oh!

sipsorcery commented Sep 29, 2020

Uh oh!

hebasto Sep 29, 2020

Choose a reason for hiding this comment

Uh oh!

hebasto left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto commented Oct 2, 2020

Uh oh!

sipsorcery commented Oct 2, 2020

Uh oh!

maflcko commented Oct 28, 2020

Uh oh!

sipsorcery commented Oct 29, 2020

Uh oh!

sipsorcery commented Nov 25, 2020

Uh oh!

Uh oh!

maflcko commented Sep 19, 2020 •

edited by DrahtBot

Loading

hebasto left a comment •

edited

Loading