Skip to content

ci: Invalidate depends caches when sources have been changed #22710

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

Merged
merged 2 commits into from
Aug 19, 2021

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Aug 15, 2021

On master (502d22c) the Cirrus CI do not invalidate depends caches when their sources has been changed, i.e. source version updates, a new dependency etc.

This behavior causes:

It is assumed that caches could be invalidated via Cirrus CI API/GUI, but it is inconvenient and it does not work as expected in all cases.

The common part of fingerprint_keys for both depends_sources_cache and depends_built_cache is the recent commit that changes sources in the src/depends sub-directory.

For depends_built_cache additionally CIRRUS_TASK_NAME is used to avoid sharing of the same cache between tasks with different OSes and different DEP_OPTS.


The depends_sources cache:

Screenshot from 2021-08-16 11-10-17

  • with this PR

Screenshot from 2021-08-16 11-10-42

@DrahtBot DrahtBot added the Tests label Aug 15, 2021
@hebasto hebasto changed the title [WIP] ci: Invalidate depends sources cache ci: Invalidate depends caches when sources have been changed Aug 16, 2021
@hebasto hebasto marked this pull request as ready for review August 16, 2021 08:16
@jonatack
Copy link
Member

Concept ACK

@hebasto hebasto force-pushed the 210815-ci branch 2 times, most recently from beac4f8 to 93f3427 Compare August 16, 2021 15:32
@Zero-1729
Copy link
Contributor

Approach ACK

@hebasto
Copy link
Member Author

hebasto commented Aug 16, 2021

CI is green, and this PR is ready for reviewing :)

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 16, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #22708 ([PoC] build, qt: Add Wayland support for Linux builds with depends by hebasto)
  • #21652 ([WIP NOMERGE DRAFT] ci: Switch more tasks to self-hosted by MarcoFalke)

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.

.cirrus.yml Outdated
@@ -41,9 +40,14 @@ global_task_template: &GLOBAL_TASK_TEMPLATE
folder: "/tmp/ccache_dir"
depends_built_cache:
folder: "depends/built"
fingerprint_script: echo $CIRRUS_TASK_NAME $(git log -1 --oneline ./depends)
Copy link
Member

Choose a reason for hiding this comment

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

Might use git rev-list -1 HEAD ./depends here; this prints only the (full, independent of core.abbrev) commit hash.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Updated.

@hebasto
Copy link
Member Author

hebasto commented Aug 18, 2021

Updated 93f3427 -> f52a72a (pr22710.04 -> pr22710.05, diff):

@maflcko
Copy link
Member

maflcko commented Aug 19, 2021

cr ACK f52a72a

Copy link
Contributor

@Zero-1729 Zero-1729 left a comment

Choose a reason for hiding this comment

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

crACK f52a72a

@maflcko maflcko merged commit b6a8e68 into bitcoin:master Aug 19, 2021
@hebasto hebasto deleted the 210815-ci branch August 19, 2021 15:17
maflcko pushed a commit that referenced this pull request Aug 20, 2021
e9cf506 ci: Make git available for all merge commits (Hennadii Stepanov)
040e4de scripted-diff: Rename template to avoid CI configuration parsing warning (Hennadii Stepanov)

Pull request description:

  1) Remove Cirrus CI configuration parsing warning:
  ![Screenshot from 2021-08-20 01-13-57](https://user-images.githubusercontent.com/32963518/130151265-551873cb-6670-48a6-b0c4-687cb341a342.png)
  ![Screenshot from 2021-08-20 01-13-41](https://user-images.githubusercontent.com/32963518/130151273-920fb316-e7e1-457b-8933-2ac31e9c605f.png)

  2) Make `git` available for merge commits, because it is now required for `depends_sources_cache` and `depends_built_cache`.

Top commit has no ACKs.

Tree-SHA512: d5cfb49492d2635fc2019220b0023179e09608bcb5a77c3d445e53c4cd714b145d6107ae02f2e3d0e2b196c2974b4688ae3e4fc233beb0917da29771795ed08e
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 20, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 20, 2021
e9cf506 ci: Make git available for all merge commits (Hennadii Stepanov)
040e4de scripted-diff: Rename template to avoid CI configuration parsing warning (Hennadii Stepanov)

Pull request description:

  1) Remove Cirrus CI configuration parsing warning:
  ![Screenshot from 2021-08-20 01-13-57](https://user-images.githubusercontent.com/32963518/130151265-551873cb-6670-48a6-b0c4-687cb341a342.png)
  ![Screenshot from 2021-08-20 01-13-41](https://user-images.githubusercontent.com/32963518/130151273-920fb316-e7e1-457b-8933-2ac31e9c605f.png)

  2) Make `git` available for merge commits, because it is now required for `depends_sources_cache` and `depends_built_cache`.

Top commit has no ACKs.

Tree-SHA512: d5cfb49492d2635fc2019220b0023179e09608bcb5a77c3d445e53c4cd714b145d6107ae02f2e3d0e2b196c2974b4688ae3e4fc233beb0917da29771795ed08e
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 19, 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.

6 participants