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

test: Remove cached directories and associated script blocks from appveyor config #19444

Merged
merged 1 commit into from
Jul 4, 2020

Conversation

sipsorcery
Copy link
Member

@sipsorcery sipsorcery commented Jul 4, 2020

Appveyor CI jobs have been failing in the last 24 hours due to a seemingly corrupted cache, see #19440.

It's possible that the appveyor cache issue is related to the recent update of the Visual Studio 2019 image

PR #19431 changes the "save cache or error" to false in an attempt to avoid a failing CI job from potentially corrupting the cache. In theory the only way a PR could affect the cache is if the vcpkg install list changed. That happens very rarely and did not happen in the last 24 hours and so was not the cause of the current cache problems.

I have done some testing with appveyor build jobs on my own fork and found that installing the vcpkg dependencies from scratch and doing a full build can now be done in just under 60 minutes. This is the first time in over 5 months I have been able to build Bitcoin Core on appveyor. Either the new Visual Studio 2019 image has dramatically reduced the build time or appveyor images have had their CPU increased.

This PR removes all use of dependency caching from the appveyor CI config. The trade-off is the 15 minutes saved on each build from having the dependencies cached versus the hours maintainers need to spend investigating when the CI jobs start failing.

@maflcko
Copy link
Member

maflcko commented Jul 4, 2020

The functional tests were disabled a while ago, so the total run time is lower

Concept ACK. I did like the 10 minute speedup, but if this causes too much issues, removing seems fine for now.

@sipsorcery
Copy link
Member Author

The functional tests were disabled a while ago, so the total run time is lower

Ah that explains it. I saw some test results and assumed it was still the original command.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 4, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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.

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

@maflcko maflcko merged commit 8783bcc into bitcoin:master Jul 4, 2020
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

post-merge ACK 961e667, works fine in my forked repo: https://ci.appveyor.com/project/hebasto/bitcoin/builds/33907119

@laanwj laanwj mentioned this pull request Jul 8, 2020
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jul 10, 2020
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Aug 11, 2020
@fanquake fanquake mentioned this pull request Aug 11, 2020
maflcko pushed a commit that referenced this pull request Aug 11, 2020
be95147 Updated appveyor job to checkout a specific vcpkg commit ID. (Aaron Clauson)
1fd9cd2 appveyor: Remove clcache (MarcoFalke)
8c0a959 Remove cached directories and associated script blocks from appveyor CI configuration. (Aaron Clauson)
d70f700 lint: fix shellcheck URL in CI install (fanquake)
f8f7d91 test: remove Cirrus CI FreeBSD job (fanquake)
b7e16a8 Add missing QPainterPath include (Andrew Chow)
30a2814 gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged (João Barbosa)
0d87a5b QA: feature_segwit: Check that template "rules" includes "!segwit" as appropriate (Luke Dashjr)
bde6a5a Bugfix: Include "csv","!segwit" in "rules" (Luke Dashjr)
e422f65 build: Set libevent minimum version to 2.0.21 (Hennadii Stepanov)
0d0dd6a Update with new Windows code signing certificate (Andrew Chow)

Pull request description:

  Backports the following to the 0.19 branch:

  * #17946 - Fix GBT: Restore "!segwit" and "csv" to "rules" key
  * #18160 - gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged
  * #18425 - releases: Update with new Windows code signing certificate
  * #18676 - build: Check libevent minimum version in configure script
  * #19097 - qt: Add missing QPainterPath include (as per #19510)
  * #18640 - appveyor: Remove clcache
  * #19444 - test: Remove cached directories and associated script blocks from appveyor config
  * #19612 - lint: fix shellcheck URL in CI install
  * #18001 -  Updated appveyor job to checkout a specific vcpkg commit ID

  Closes: #19510.

ACKs for top commit:
  jnewbery:
    ACK be95147
  MarcoFalke:
    cherry-pick ACK be95147 🌎

Tree-SHA512: 2ec7e3ae1da99799ff6f8cfe26095d6885cffe6952b18a7e236dc5e657b3918225c2601b8c8e17cdff5319c40cb0a214d9fad49b0ff2f54af1db7c81d83a1df5
backpacker69 referenced this pull request in peercoin/peercoin Sep 8, 2020
…CI configuration.

Github-Pull: #19444
Rebased-From: 961e667
Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Oct 21, 2020
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants