Skip to content
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

Update actions #428

Merged
merged 2 commits into from
Apr 29, 2024
Merged

Update actions #428

merged 2 commits into from
Apr 29, 2024

Conversation

simonhamp
Copy link
Contributor

What does this PR do?

  • Updates action versions to their latest ones
  • Fixes the dependency cache key naming logic, bringing the other environments into line with what's implemented in Windows build GitHub Action #420 - caches weren't being used at all when specifying an explicit set of extensions because the key contained commas

This reduces errors and improves build times.

Checklist before merging

If your PR involves the changes mentioned below and completed the action, please tick the corresponding option.
If a modification is not involved, please skip it directly.

  • If it's a extension or dependency update, make sure adding related extensions in src/global/test-extensions.php.
  • If you changed the behavior of static-php-cli, add docs in static-php/static-php-cli-docs .
  • If you updated config/xxxx.json content, run bin/spc dev:sort-config xxx.

@crazywhalecc crazywhalecc added bug Something isn't working kind/workflow Issues related to workflow or environment labels Apr 26, 2024
@crazywhalecc
Copy link
Owner

crazywhalecc commented Apr 26, 2024

Previously I modified test workflow for dynamic caching: Just cache downlods dir again and again. When new extension added, it will automatically skip downloading other sources that has been cached.

Also, in some particularly rare cases, as long as cache-hit != 'true' is used, the cache will not be hit, with no reason.

@simonhamp
Copy link
Contributor Author

@crazywhalecc I'm not sure I follow you. The cache in these didn't appear to be working at all because the key name was incorrect. This had the result of always downloading the extension files.

This PR fixes that so each action will now correctly cache the set of extensions that were requested.

This should make builds of the same PHP-version+OS+extension set a little faster, at least during the cache's lifetime.

@simonhamp
Copy link
Contributor Author

In case it's not clear, this dependency cache is the extension build files (the result of bin/spc download).

I think the variance you were seeing in cache-hit != true was exactly down to this problem of commas in the key name.

This is why I've hashed the details of the build into a key name that will never have commas regardless of how the build is configured.

@crazywhalecc
Copy link
Owner

What I mean is that today's best caching strategies don't perform well in Actions. It has nothing to do with this PR. After fixing this small problem, we merge it into the main branch and test it again.

@crazywhalecc crazywhalecc merged commit 15c2935 into crazywhalecc:main Apr 29, 2024
19 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working kind/workflow Issues related to workflow or environment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants