ci: Add vcpkg tools cache #23215

hebasto · 2021-10-07T10:12:27Z

On master (02feae5) vcpkg downloads some tools that are used internally:

...
  A suitable version of cmake was not found (required v3.20.0). Downloading portable cmake v3.20.0...
  Downloading cmake...
    https://github.com/Kitware/CMake/releases/download/v3.20.2/cmake-3.20.2-windows-i386.zip -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\cmake-3.20.2-windows-i386.zip
  Extracting cmake...
  A suitable version of 7zip was not found (required v18.1.0). Downloading portable 7zip v18.1.0...
  Downloading 7zip...
    https://www.nuget.org/api/v2/package/7-Zip.CommandLine/18.1.0 -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\7-zip.commandline.18.1.0.nupkg
  Extracting 7zip...
  A suitable version of nuget was not found (required v5.5.0). Downloading portable nuget v5.5.0...
  Downloading nuget...
    https://dist.nuget.org/win-x86-commandline/v5.5.1/nuget.exe -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\22ea847d-nuget.exe
  Detecting compiler hash for triplet x64-windows-static...
  A suitable version of powershell-core was not found (required v7.1.0). Downloading portable powershell-core v7.1.0...
  Downloading powershell-core...
    https://github.com/PowerShell/PowerShell/releases/download/v7.1.3/PowerShell-7.1.3-win-x86.zip -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\PowerShell-7.1.3-win-x86.zip
  Extracting powershell-core...
...

If any of downloads above fails the whole CI task fails (see #23107). The most recent failure examples in the master branch:

c001da3, log
12ff899, log

This PR adds vcpkg tools cache.

Closes #23107.

Note for reviewers. Some patches from the initial PR were split into separated PRs: #23310 and #23329. Therefore, a discussion here could be outdated or irrelevant until the recent push.

hebasto · 2021-10-07T11:51:51Z

The size of the vcpkg downloads cache will be lesser significantly after bumping vcpkg up to microsoft/vcpkg@0fd101a or later.

DrahtBot · 2021-10-07T17:35:29Z

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

Conflicts

No conflicts as of last run.

hebasto · 2021-10-13T17:29:03Z

cc @MarcoFalke

maflcko · 2021-10-15T12:23:27Z

Concept ACK, but I am not familiar with vcpkg

hebasto · 2021-10-15T12:48:41Z

cc @sipsorcery

sipsorcery · 2021-10-15T18:26:52Z

I think I mentioned it on the original PR that moved the Windows build to Cirrus but we did have a period of instability that was caused by caching vcpkg dependencies #19431 and #19444. Just thought I'd mention it in case the same issue crops up again and is traced back to this PR.

Other than that vcpkg has a new'ish binary caching feature which is recommended for CI use. If vcpkg dependencies are going to be cached again it would seem to make sense to enable it. By only caching the C:\Users\ContainerAdministrator\AppData\Local\vcpkg\archives directory, without the feature, it could cause the ABI mismatch between the dependencies and the compiler version we encountered in the past. In theory all that should be required is to set the environment variable to:

VCPKG_FEATURE_FLAGS: 'manifests,binarycaching'

hebasto · 2021-10-19T14:08:20Z

@sipsorcery

I think I mentioned it on the original PR that moved the Windows build to Cirrus but we did have a period of instability that was caused by caching vcpkg dependencies #19431 and #19444. Just thought I'd mention it in case the same issue crops up again and is traced back to this PR.

Other than that vcpkg has a new'ish binary caching feature which is recommended for CI use. If vcpkg dependencies are going to be cached again it would seem to make sense to enable it. By only caching the C:\Users\ContainerAdministrator\AppData\Local\vcpkg\archives directory, without the feature, it could cause the ABI mismatch between the dependencies and the compiler version we encountered in the past. In theory all that should be required is to set the environment variable to:

VCPKG_FEATURE_FLAGS: 'manifests,binarycaching'

Sorry, I didn't get your point.

Binary Caching

We already use binary caching. This PR does not introduce it. It rather makes sure that the cache will be re-populated when vcpkg version is bumped. Maybe also it's reasonable to invalidate the cache when msbuild -version is changed?

Btw, the "Binary Caching" feature is enabled by default now (as the "Manifest Mode" feature as well), and no need to pass binarycaching to the VCPKG_FEATURE_FLAGS variable.

Downloads Caching

This PR introduces new cache which is not an internal vcpkg feature, but rather a CI's one. These downloads actually are internal vcpkg dependencies, not ours.

sipsorcery · 2021-10-19T18:00:47Z

We already use binary caching.

I don't think so, at least the old appveyor job wasn't. I had it enabled on my personal appveyor job to save 10 minutes on the Bitcoin Core build but I'm pretty sure it was never turned on for the main Bitcoin Core CI job.

Maybe also it's reasonable to invalidate the cache when msbuild -version is changed?

I think that's very desireable due to the aforementioned ABI incompatabilites that crop up. It's been my experience that vcpkg, with binary caching enabled, rebuilds dependencies if it detects an updated compiler.

Btw, the "Binary Caching" feature is enabled by default

That's good news, I hadn't spotted that. The CI config in this PR is still pegging vcpkg to commit ID 75522bb1f2e7d863078bcd06322348f053a9e33f so won't be taking advantage of the new defaults.

An alternative to my suggestion of explicitly setting binary caching on, would be to update the vcpkg commit ID to 5568f110b509a9fd90711978a7cb76bae75bb092 which is the latest 2021.05.12 release and looks like it would have manifests and binary caching enabled by default.

hebasto · 2021-10-19T18:21:14Z

I don't think so, at least the old appveyor job wasn't. I had it enabled on my personal appveyor job to save 10 minutes on the Bitcoin Core build but I'm pretty sure it was never turned on for the main Bitcoin Core CI job.

On Cirrus CI Windows job it is enabled:

bitcoin/.cirrus.yml

Lines 124 to 125 in 986003a

    
           vcpkg_cache: 
        
             folder: 'C:\Users\ContainerAdministrator\AppData\Local\vcpkg\archives'

where the default location is used 🐅

sipsorcery · 2021-10-19T18:31:23Z

On Cirrus CI Windows job it is enabled

I'm not sure just caching the vcpkg download directories is enough.... It's the libraries that are built from the downloaded source archives that are used in the Bitcoin Core build. How those libraries are detected as out of date is the question.

Is there any reason not to bump the vcpkg commit to 75522bb1f2e7d863078bcd06322348f053a9e33f?

Now that you've pointed out manifests and binary caching are enabled by default it seems like a good idea to take advantage of them. And it would make our discussion academic :).

hebasto · 2021-10-19T19:16:30Z

@sipsorcery

An alternative to my suggestion of explicitly setting binary caching on, would be to update the vcpkg commit ID to 5568f110b509a9fd90711978a7cb76bae75bb092 which is the latest 2021.05.12 release and looks like it would have manifests and binary caching enabled by default.

Thanks! Done in #23310.

Converting this PR into a draft for now.

13ae568 ci: Bump vcpkg release tag (Hennadii Stepanov) Pull request description: In the new vcpkg release the "Binary Caching" and "Manifest Mode" features are [available by default](https://devblogs.microsoft.com/cppblog/all-vcpkg-enterprise-features-now-generally-available-versioning-binary-caching-manifests-and-registries/). Suggested in bitcoin/bitcoin#23215 (comment) and bitcoin/bitcoin#23215 (comment). ACKs for top commit: sipsorcery: tACK 13ae568. Tree-SHA512: 55558f2a2d395a3ad168eb1b36ffe3b0e6a3336b9a7c764d841345d8bb696b231d697dad36cc723d922d07be65c1b77a05cc9604a09e05e8f13ca8090ecb966d

This change avoids downloading of the cached vcpkg tools that could fail accidentally, and it makes CI task more robust.

hebasto · 2021-10-22T11:48:57Z

Rebased and reworked on top of #23310 and #23329 -- 7132a2b -> f778845 (pr23215.01 -> pr23215.02).

PR description updated.

hebasto · 2021-10-22T13:35:08Z

@sipsorcery

Do you mind having another look at this PR?

sipsorcery · 2021-10-22T20:08:19Z

Makes sense to me.

ACK f778845.

hebasto added Tests Windows labels Oct 7, 2021

hebasto mentioned this pull request Oct 7, 2021

ci: Windows build fails intermittently when downloading (Failed to download from mirror set) #23107

Closed

hebasto marked this pull request as draft October 7, 2021 10:16

hebasto force-pushed the 211007-ci branch 2 times, most recently from 7c8a2d9 to 7132a2b Compare October 7, 2021 10:31

hebasto marked this pull request as ready for review October 7, 2021 10:35

DrahtBot mentioned this pull request Oct 7, 2021

[DO NOT MERGE] ci: Test required resources for native Windows task #23217

Closed

hebasto mentioned this pull request Oct 19, 2021

ci: Bump vcpkg release tag #23310

Merged

hebasto marked this pull request as draft October 19, 2021 19:16

fanquake mentioned this pull request Oct 20, 2021

ci: Define cases when invalidate vcpkg binary cache #23329

Merged

ci: Add vcpkg tools cache

f778845

This change avoids downloading of the cached vcpkg tools that could fail accidentally, and it makes CI task more robust.

hebasto force-pushed the 211007-ci branch from 7132a2b to f778845 Compare October 22, 2021 11:44

hebasto marked this pull request as ready for review October 22, 2021 11:44

hebasto changed the title ~~ci: Add vcpkg downloads cache~~ ci: Add vcpkg tools cache Oct 22, 2021

fanquake merged commit 04437ee into bitcoin:master Oct 23, 2021

hebasto deleted the 211007-ci branch October 23, 2021 06:46

bitcoin locked and limited conversation to collaborators Oct 30, 2022

ci: Add vcpkg tools cache #23215

ci: Add vcpkg tools cache #23215

Conversation

hebasto commented Oct 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Oct 7, 2021

Uh oh!

DrahtBot commented Oct 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto commented Oct 13, 2021

Uh oh!

maflcko commented Oct 15, 2021

Uh oh!

hebasto commented Oct 15, 2021

Uh oh!

sipsorcery commented Oct 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Oct 19, 2021

Uh oh!

sipsorcery commented Oct 19, 2021

Uh oh!

hebasto commented Oct 19, 2021

Uh oh!

sipsorcery commented Oct 19, 2021

Uh oh!

hebasto commented Oct 19, 2021

Uh oh!

hebasto commented Oct 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Oct 22, 2021

Uh oh!

sipsorcery commented Oct 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hebasto commented Oct 7, 2021 •

edited

Loading

DrahtBot commented Oct 7, 2021 •

edited

Loading

sipsorcery commented Oct 15, 2021 •

edited

Loading

hebasto commented Oct 22, 2021 •

edited

Loading