ci: Mitigate network issues in native Windows job#35024
ci: Mitigate network issues in native Windows job#35024hebasto wants to merge 2 commits intobitcoin:masterfrom
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. |
|
The size of Perhaps we could exclude |
hebasto
force-pushed
the
260407-vcpkg-dl-cache
branch
from
22c03c7 to
cd8092d
Compare
Done.
It's 800 MB now. |
|
Can you explain why the tools were previously cached, what they are, and why it is fine to no longer cache them? |
To skip downloading and unpacking them whenever possible.
They are Windows binaries used for building packages for the target triplet. Here are the contents of the
It is a tradeoff. The downloaded tool archives are now cached along with the package source tarballs. However, vcpkg has to unpack them on every run. |
| path: | | ||
| ~/AppData/Local/vcpkg/downloads/* | ||
| !~/AppData/Local/vcpkg/downloads/tools |
There was a problem hiding this comment.
Is there a need to set a path on a download? Wouldn't it be easier to just set the correct path on upload only?
There was a problem hiding this comment.
Yes, there is a need. The path input participates in cache entity identification. Therefore, a pair of actions/cache/restore and actions/cache/save must have the exact same path configured, or it will result in a cache miss.
This avoids code duplication and improves readability.
The new cache is keyed with the hash of 'vcpkg.json', which reduces cache storage consumption compared to keying by run ID. The `vcpkg/downloads/tools` subdirectory is excluded to further save space.
hebasto
force-pushed
the
260407-vcpkg-dl-cache
branch
from
cd8092d to
dc93091
Compare
|
The feedback from @maflcko has been addressed. |
| path: ${{ env.CCACHE_DIR }} | ||
| # https://github.com/actions/cache/blob/main/tips-and-workarounds.md#update-a-cache | ||
| key: ${{ github.job }}-${{ matrix.job-type }}-ccache-${{ github.run_id }} | ||
| key: ${{ steps.ccache-cache.outputs.cache-primary-key }} |
There was a problem hiding this comment.
you claim this is a refactor, but it would be good to link to the relevant docs, so that reviewers can confirm this.
to me this looks like dropping the run_id, making it impossible to save a new cache?
There was a problem hiding this comment.
you claim this is a refactor, but it would be good to link to the relevant docs, so that reviewers can confirm this.
Sure.
From the docs:
cache-primary-key- Cache primary key passed in the input to use in subsequent steps of the workflow.
to me this looks like dropping the run_id, making it impossible to save a new cache?
It now refers to:
bitcoin/.github/workflows/ci.yml
Line 192 in dc93091
If you think cache-primary-key is confusing, I’m happy to drop that commit.
3 participants
This PR mitigates network issues when vcpkg downloads source tarballs by caching the entire
vcpkg/downloadsdirectory.Closes #34996.
Note for Maintainers: To properly populate the new CI cache, all current vcpkg binary caches must be cleared to trigger a rebuild.