The pinning should no-longer be needed; 4.6.2 was broken, latest is 4.7.4, and that includes the necessary fixes.
ci: remove ccache version pin in MSVC CI #26866
pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:fix_ccache_install changing 1 files +1 −1-
fanquake commented at 4:18 PM on January 10, 2023: member
-
DrahtBot commented at 4:18 PM on January 10, 2023: contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
Reviews
See the guideline for information on the review process.
Type Reviewers Concept NACK MarcoFalke If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
- DrahtBot added the label Tests on Jan 10, 2023
-
fanquake commented at 4:23 PM on January 10, 2023: member
Guess this might just be an intermitent Chocolatey issue?:
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call choco install --yes --no-progress ccache Chocolatey v1.1.0 Installing the following packages: ccache By installing, you accept licenses for the packages. ccache not installed. The package was not found with the source(s) listed. Source(s): 'https://community.chocolatey.org/api/v2/' NOTE: When you specify explicit sources, it overrides default sources. -
b39f79b128
ci: remove ccache version pin in MSVC CI
The pinning should no-longer be needed; 4.6.2 was broken, latest is 4.7.4, and that includes the necessary fixes.
- fanquake force-pushed on Jan 11, 2023
-
fanquake commented at 10:40 AM on January 11, 2023: member
Chocolately seems to have fixed itself, so I've changed the wording here to just drop the ccache version pin, as it's no-longer necessary, and I don't think we've got a reason to remain pinned.
-
maflcko commented at 11:01 AM on January 11, 2023: member
NACK. The reason is that a broken version will break our CI and we'll have to pin it to fix the problem. Everything else is pinned, so why should this not be pinned?
-
fanquake commented at 11:12 AM on January 11, 2023: member
Everything else is pinned, so why should this not be pinned?
In this case I don't care too much either way, but I think if we are going to have things pinned, (in general) we could document why. This was pinned because of a bug that has since been fixed, and staying pinned is at best, worse than using the latest working version (i.e pinning to 4.6.x is just missing multiple performance improvements / bug fixes over 4.7.x). I guess the better change here is too just change the pin to 4.7.4, but feel free to close.
- fanquake closed this on Jan 11, 2023
- fanquake deleted the branch on Jan 11, 2023
-
maflcko commented at 11:19 AM on January 11, 2023: member
No objection to bumping it or adding documentation.
ccache (for windows) isn't really used, looking at the download numbers, https://community.chocolatey.org/packages/ccache#versionhistory, so I'd prefer if someone else is doing the nightly testing outside of our production CI
- maflcko referenced this in commit 08d2a3ab4b on Jan 16, 2023
- bitcoin locked this on Jan 11, 2024
Labels