-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix caching by ignoring semver build metadata #60
base: default
Are you sure you want to change the base?
Conversation
Thanks for looking into this! Do I understand correctly that the issue is because the extracted path is different from the archive file name? I think the commit hash should be part of the cache key, else changing the version in your workflow will reuse a different version from cache. |
Yep - seems the final path returned by const cachePath = await toolCache.cacheDir(binPath, TOOL_NAME, useVersion)
if (useCache) {
actions.info(`adding zig ${useVersion} at ${cachePath} to local cache ${cacheKey}`)
await cache.saveCache([cachePath], cacheKey)
}
return cachePath Which Is what I noticed in the issue. Looking at /**
* Caches a directory and installs it into the tool cacheDir
*
* @param sourceDir the directory to cache into tools
* @param tool tool name
* @param version version of the tool. semver format
* @param arch architecture of the tool. Optional. Defaults to machine architecture
*/
export async function cacheDir(
sourceDir: string,
tool: string,
version: string,
arch?: string
): Promise<string> {
version = semver.clean(version) || version And
Adding this to assert.equal(
semver.clean('0.12.0-dev.1150+3c22cecee'),
'0.12.0-dev.1150+3c22cecee'
)
So basically, it's
Per semver, build metadata isn't actually valuable version information. But, since zig's versioning uses a nicely defined patch version with an ever-increasing "build" number (e.g, Also weird that Note PR is still a draft, since this is mostly for explanation purposes, but I'd be happy to clean it up and move it forward after consenus here. Also, once again, thanks for this repo! Def easier and cleaner than having folks |
Thanks very much for that research. Sorry it is taking so long to follow up! I'll hopefully find some time to wrap up the open PRs on this repo this week 😅 |
Any updates on this? |
@heysokam you might be interested in https://github.com/marketplace/actions/setup-zig-compiler which was created by a Zig core team member Some links that explain the differences: |
Fixes #59
Fixes #72
Fixes #75
See comment: #59 (comment)
Seems to fix the caching issue for me.