Skip to content

Conversation

@AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Nov 18, 2025

This action has never worked due to permission issues, and because of how GitHub handles caches it wouldn't reliably help cache anyway.

Adding the required token management to allow the workflow to delete the caches would require ensuring it isn't abused, and even without malicious attempts to attach the repo using permissions it opens up the risk for issues from incorrectly written CI code.

Most PRs won't have any cache entries anyway, as cache entries generally don't survive beyond 24 hours, unless they need to be rebased etc. just ahead of merging. And because GitHub deletes the oldest cache entries first, not the least recently used there's a fairly even chance deleting the cache entries of any one PR hurts the retention of the master cache rather than helping, if the cache of that PR is older than the master branch. Finally any PR that deletes their caches on merge will invariably hurt cache retention as their caches will always be older than the new current master. (Effectively we want to have old cache entries as a "buffer", as we cannot ever really guarantee that the cache is small enough, given that a single build of master takes several GB alone, and most PRs take at least a few dozen MB unless they are absolutely minimal)

As discussed elsewhere we will hopefully be able to migrate away from using GitHub's cache system in favor of using artifacts and similar, so we could remove this then, but this workflow just takes up space and generates noise even though it probably doesn't significantly affect the availability of runners as it is so fast.

This action has never worked due to permission issues, and because
of how GitHub handles caches it wouldn't reliably help cache anyway.
@AThousandShips AThousandShips added this to the 4.6 milestone Nov 18, 2025
@AThousandShips AThousandShips requested a review from a team as a code owner November 18, 2025 14:48
@AThousandShips AThousandShips added enhancement topic:buildsystem cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release labels Nov 18, 2025
@AThousandShips
Copy link
Member Author

Note that the alternative here would be to fix the permission issues by adding a GitHub token, but that comes with risks as mentioned in the main comment

Copy link
Contributor

@Repiteo Repiteo left a comment

Choose a reason for hiding this comment

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

This was looong overdue

@akien-mga
Copy link
Member

For context, this was implemented for 4.5 in #104077 and #104220 and indeed seemed not to work in the end with the default GH_TOKEN.

@AThousandShips AThousandShips removed the cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release label Nov 19, 2025
@Repiteo Repiteo merged commit ba0956e into godotengine:master Nov 19, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 19, 2025

Thanks!

@AThousandShips
Copy link
Member Author

Thank you!

@AThousandShips AThousandShips deleted the remove_ci_clean branch November 19, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release enhancement topic:buildsystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants