Skip to content

ci: Fix previous releases cache order #21653

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 13, 2021
Merged

ci: Fix previous releases cache order #21653

merged 1 commit into from
Apr 13, 2021

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Apr 11, 2021

No description provided.

ci: Fix previous releases cache order
fa4f0b3 
The order was broken in commit ffff4e7
@maflcko
Copy link
Member Author

maflcko commented Apr 11, 2021

Reported in #21619 (comment) by @hebasto . Thanks!

@fanquake fanquake added the Tests label Apr 11, 2021
@maflcko maflcko mentioned this pull request Apr 11, 2021
Merged
hebasto
hebasto reviewed Apr 11, 2021
View reviewed changes
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.

Concept ACK fa4f0b3.

Does the populated cache prevent from downloading releases again:

bitcoin/ci/test/05_before_script.sh

Lines 58 to 60 in f6c44e9

if [ -n "$PREVIOUS_RELEASES_TO_DOWNLOAD" ]; then
DOCKER_EXEC test/get_previous_releases.py -b -t "$PREVIOUS_RELEASES_DIR" "${PREVIOUS_RELEASES_TO_DOWNLOAD}"
fi
?

Maybe the populate_script field could be useful?

@maflcko
Copy link
Member Author

maflcko commented Apr 12, 2021

Not sure what you mean. The cache is populated and everything is green: https://cirrus-ci.com/task/6695630107049984?logs=previous_releases#L2

@maflcko
Copy link
Member Author

maflcko commented Apr 12, 2021

It is also greeen without cache: https://cirrus-ci.com/task/5560067312320512?logs=previous_releases#L1

@hebasto
Copy link
Member

hebasto commented Apr 12, 2021

Not sure what you mean.

Just trying to answer to myself "What is the role of the previous releases cache in the CI setup? Speedup what?"

@laanwj
Copy link
Member

laanwj commented Apr 12, 2021

I think the idea of the cache is to avoid downloading the previous releases from bitcoincore.org every test run?

@hebasto
Copy link
Member

hebasto commented Apr 12, 2021

I think the idea of the cache is to avoid downloading the previous releases from bitcoincore.org every test run?

Agree. But currently it does not work like that.

@maflcko
Copy link
Member Author

maflcko commented Apr 13, 2021

It does:

Releases directory: /tmp/cirrus-build/releases/x86_64-pc-linux-gnu
Using cached v0.15.2
Using cached v0.16.3
Using cached v0.17.2
Using cached v0.18.1
Using cached v0.19.1

https://cirrus-ci.com/task/6695630107049984?logs=ci#L1993

@hebasto
Copy link
Member

hebasto commented Apr 13, 2021

It does:

Right. It is responsibility of test/get_previous_releases.py.

hebasto
hebasto approved these changes Apr 13, 2021
View reviewed changes
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.

ACK fa4f0b3

@maflcko maflcko merged commit c1f480f into bitcoin:master Apr 13, 2021
@maflcko maflcko deleted the 2104-ciFixCacheOrder branch April 13, 2021 11:24
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 13, 2021
Merge bitcoin#21653: ci: Fix previous releases cache order
1dc8e06
@maflcko maflcko mentioned this pull request Apr 14, 2021
Merged
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hebasto hebasto hebasto approved these changes

Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@maflcko @hebasto @laanwj @fanquake