search

hendrikmuhs / ccache-action

github action to speedup building using ccache
MIT License
118 stars 53 forks source link

Make cache key timestamping optional. #126

Closed fruffy closed 1 year ago

fruffy commented 1 year ago

Prompted by #117.

For Github repositories with many different actions and concurrent PRs, ccache-action's timestamps can quickly fill up the cache and evict other cached entries. This PR adds an option to disable timestamping.

I am not familiar with either Javascript or Github Actions, so suggestions on how to improve this are very much welcome.

ppizarror commented 1 year ago

We have the same problem! Having this option might help us a lot @hendrikmuhs

hendrikmuhs commented 1 year ago

Looks good to me, instead of useTimestamp I suggest appendTimestamp.

hendrikmuhs commented 1 year ago

Thanks! I can merge and fix the js files myself. The last dependabot updates broke the build anyway.

ppizarror commented 1 year ago

Nice!! thanks a lot @hendrikmuhs, @fruffy

vadz commented 1 year ago

I was eager to try this as I also don't like having one cache for each of the runs, but it seems that this doesn't actually work, as saving the cache always fails when using the new option and the cache already exists.

According to this comment, an existing cache can't be overwritten, and if this is correct, then the new option is not very useful. Or am I missing something here?

fruffy commented 1 year ago

Part of the motivation for this PR was to get some control over the key signature. You can still define a key that is flexible enough to avoid that issue (using PR/commit IDs). This is what we have been doing.

But yes, it is an issue right now. Hope they will fix that bug at some point.

vadz commented 1 year ago

Thanks for the clarification, it wasn't obvious that using this option required making the key unique in some other way. I think it would be nice to mention this in the README.