feat: use cached binary if present #510
Conversation
crazy-max
left a comment
crazy-max
left a comment
There was a problem hiding this comment.
Maybe we could do smth similar to https://github.com/magefile/mage-action/blob/6f50bbb8ea47d56e62dee92392788acbc8192d0b/src/installer.ts#L36 to use both tool-cache and GitHub Actions cache?
| required: false | ||
| cache-binary: | ||
| description: 'Cache binary to GitHub Actions cache backend' | ||
| default: 'false' |
There was a problem hiding this comment.
mage defaults to true here but for runners hosted on github i don't think there's much if anything to gain from caching the binary as they are both on the same infrastructure.
timbretimber
force-pushed
the
toolcache
branch
from
6a2df65 to
04e0e23
Compare
WalkthroughThis PR adds caching functionality to the GoReleaser GitHub Action. It introduces a new optional Sequence DiagramsequenceDiagram
participant main as main.ts
participant goreleaser as goreleaser.ts
participant cache as `@actions/cache`
main->>goreleaser: install(distribution, version, cacheBinary=true)
alt cacheBinary enabled
goreleaser->>goreleaser: Extract semver from version tag
goreleaser->>goreleaser: Create ~/.goreleaser directory
rect rgb(200, 220, 255)
Note over goreleaser: Cache lookup phase
goreleaser->>goreleaser: Check local cache for binary<br/>(validate executable)
alt Binary found in local cache
goreleaser-->>main: Return cached path
end
end
rect rgb(220, 200, 255)
Note over goreleaser,cache: GitHub Actions cache phase
goreleaser->>cache: restoreCache(key=semver)
cache-->>goreleaser: Restored path (if exists)
alt Cache restored successfully
goreleaser-->>main: Return cached binary path
end
end
rect rgb(255, 220, 200)
Note over goreleaser: Download and cache phase
goreleaser->>goreleaser: Download & extract binary<br/>to ~/.goreleaser
goreleaser->>cache: saveCache(key=semver, path)
cache-->>goreleaser: Cache saved
goreleaser-->>main: Return extracted binary path
end
else cacheBinary disabled
rect rgb(240, 240, 240)
Note over goreleaser: Standard download
goreleaser->>goreleaser: Download & extract binary<br/>(skip cache operations)
goreleaser-->>main: Return extracted path
end
end
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/licenses.txtis excluded by!**/dist/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
README.md(1 hunks)action.yml(1 hunks)package.json(1 hunks)src/context.ts(2 hunks)src/goreleaser.ts(4 hunks)src/main.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/goreleaser.ts (2)
src/github.ts (1)
GitHubRelease(29-31)dist/index.js (2)
getExePath(3-3)getCacheKey(3-3)
| const goreleaserHome = path.join(process.env.HOME, '.goreleaser'); | ||
| if (!fs.existsSync(goreleaserHome)) { | ||
| fs.mkdirSync(goreleaserHome, {recursive: true}); | ||
| } |
There was a problem hiding this comment.
Fix home directory resolution for Windows runners
process.env.HOME is undefined on Windows runners, so path.join throws and the action now fails before any install occurs whenever cache-binary (or even the default flow) hits this block. Use os.homedir() instead so all supported runners keep working.
-import * as path from 'path';
+import * as os from 'os';
+import * as path from 'path';
@@
- const goreleaserHome = path.join(process.env.HOME, '.goreleaser');
+ const goreleaserHome = path.join(os.homedir(), '.goreleaser');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const goreleaserHome = path.join(process.env.HOME, '.goreleaser'); | |
| if (!fs.existsSync(goreleaserHome)) { | |
| fs.mkdirSync(goreleaserHome, {recursive: true}); | |
| } | |
| const goreleaserHome = path.join(os.homedir(), '.goreleaser'); | |
| if (!fs.existsSync(goreleaserHome)) { | |
| fs.mkdirSync(goreleaserHome, {recursive: true}); | |
| } |
🤖 Prompt for AI Agents
In src/goreleaser.ts around lines 35 to 38, the code uses process.env.HOME which
is undefined on Windows and causes path.join to throw; replace usage with
os.homedir() (importing/require'ing the Node os module if not already imported)
so the home directory is resolved cross-platform, e.g. build goreleaserHome from
os.homedir() and leave the fs.existsSync/fs.mkdirSync logic unchanged.
| if (cacheBinary && cache.isFeatureAvailable()) { | ||
| core.debug(`Caching to GitHub actions cache`); | ||
| await cache.saveCache([getExePath(goreleaserHome)], getCacheKey(semver)); | ||
| } |
There was a problem hiding this comment.
Prevent cache.saveCache from failing on duplicate keys
cache.saveCache rejects with “Cache already exists” when another job has populated the same key; without handling, every later run with cache-binary enabled fails even though the install succeeded. Catch that case (while still surfacing real errors) so repeat installs remain healthy.
if (cacheBinary && cache.isFeatureAvailable()) {
core.debug(`Caching to GitHub actions cache`);
- await cache.saveCache([getExePath(goreleaserHome)], getCacheKey(semver));
+ try {
+ await cache.saveCache([getExePath(goreleaserHome)], getCacheKey(semver));
+ } catch (error) {
+ const message = (error as Error).message ?? '';
+ if (message.includes('Cache already exists')) {
+ core.debug(`Skipping cache save: ${message}`);
+ } else {
+ throw error;
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (cacheBinary && cache.isFeatureAvailable()) { | |
| core.debug(`Caching to GitHub actions cache`); | |
| await cache.saveCache([getExePath(goreleaserHome)], getCacheKey(semver)); | |
| } | |
| if (cacheBinary && cache.isFeatureAvailable()) { | |
| core.debug(`Caching to GitHub actions cache`); | |
| try { | |
| await cache.saveCache([getExePath(goreleaserHome)], getCacheKey(semver)); | |
| } catch (error) { | |
| const message = (error as Error).message ?? ''; | |
| if (message.includes('Cache already exists')) { | |
| core.debug(`Skipping cache save: ${message}`); | |
| } else { | |
| throw error; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/goreleaser.ts around lines 71 to 74, the call to cache.saveCache can
reject with "Cache already exists" when another job populated the same key; wrap
the await cache.saveCache(...) call in a try/catch, and in the catch check the
error message (or error.code) for the "Cache already exists" condition — if it
matches, log a debug/info message and continue (silently ignore that specific
error); for any other errors rethrow or surface them so real failures are not
swallowed.
There was a problem hiding this comment.
I don't see this behavior, and the string Cache already exists doesn't appear in the @actions/cache source code so I don't think this is correct. I did some manual testing by hardcoding the version in
goreleaser-action/src/goreleaser.ts
Line 11 in 9cf3611
One issue I did notice though is if using toolcache with a semver range like ~> v2 you can't really invalidate the cache when a new version is released without splitting up the save/restore steps and capturing the version in between, making it only really practical to use with specific versions. However @actions/cache works fine with the same statement.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the @actions/cache source code for "Cache already exists" error message
curl -s "https://api.github.com/search/code?q=repo:actions/cache+%22Cache+already+exists%22" | jq -r '.items[] | .path'
Length of output: 58
🏁 Script executed:
#!/bin/bash
# Search for cache save error handling patterns in @actions/cache
curl -s "https://api.github.com/search/code?q=repo:actions/cache+saveCache+error" | jq -r '.items[0:5] | .[] | {path: .path, url: .html_url}'
Length of output: 58
🏁 Script executed:
#!/bin/bash
# Get the main save.ts or saveCache implementation from @actions/cache
curl -s "https://raw.githubusercontent.com/actions/toolkit/main/packages/cache/src/cache.ts" | grep -A 20 "export async function saveCache"
Length of output: 586
🏁 Script executed:
#!/bin/bash
# Look at the actual saveCache implementation details for v1 and v2
curl -s "https://raw.githubusercontent.com/actions/toolkit/main/packages/cache/src/cache.ts" | grep -A 100 "async function saveCacheV1" | head -50
Length of output: 1586
🏁 Script executed:
#!/bin/bash
# Check ReserveCacheError and other cache-related errors
curl -s "https://raw.githubusercontent.com/actions/toolkit/main/packages/cache/src/internal/cacheHttpClient.ts" | grep -B 5 -A 5 "ReserveCacheError\|already exists"
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for ReserveCacheError definition
curl -s "https://raw.githubusercontent.com/actions/toolkit/main/packages/cache/src/internal/cacheHttpClient.ts" | grep -B 10 -A 10 "class.*Error\|ReserveCacheError"
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the reserveCache function to see how it handles already-cached keys
curl -s "https://raw.githubusercontent.com/actions/toolkit/main/packages/cache/src/internal/cacheHttpClient.ts" | grep -A 50 "export async function reserveCache"
Length of output: 1295
🏁 Script executed:
#!/bin/bash
# Check if saveCache handles the case when reserveCache returns -1 (already exists)
curl -s "https://raw.githubusercontent.com/actions/toolkit/main/packages/cache/src/cache.ts" | grep -A 10 "reserveCacheResponse"
Length of output: 1134
🏁 Script executed:
#!/bin/bash
# Check how ReserveCacheError is handled in the catch block
curl -s "https://raw.githubusercontent.com/actions/toolkit/main/packages/cache/src/cache.ts" | grep -A 20 "typedError.name === ReserveCacheError.name"
Length of output: 1464
🏁 Script executed:
#!/bin/bash
# Look for ReserveCacheError class definition
curl -s "https://raw.githubusercontent.com/actions/toolkit/main/packages/cache/src/internal/cacheHttpClient.ts" -H "Accept: application/vnd.github.v3.raw" | head -100
Length of output: 2711
@timbretimber, you're absolutely right—thank you for testing and catching that! I reviewed the @actions/cache source, and I can confirm:
- The string "Cache already exists" doesn't appear anywhere in the codebase
- When
reserveCacheencounters a concurrent cache creation (another job with the same key), it throws aReserveCacheError - Critically,
saveCachealready catchesReserveCacheErrorinternally and handles it gracefully by loggingcore.info()instead of throwing—so no additional error handling is needed
My suggestion was based on incorrect assumptions about the error behavior. Your testing confirmed the actual behavior: the cache library already handles concurrent saves properly.
Regarding your observation about toolcache with semver ranges (~> v2): that's a valid limitation. Since tc.cacheDir uses the resolved semver as the key and tc.find looks up by that exact version, a semver range will always match the first resolved version—you'd need to capture the actual version between operations to invalidate properly. The @actions/cache approach with explicit cache keys sidesteps that issue entirely. Good catch on identifying that distinction!
٩(◕‿◕)۶
✏️ Learnings added
Learnt from: timbretimber
Repo: goreleaser/goreleaser-action PR: 510
File: src/goreleaser.ts:71-74
Timestamp: 2025-11-15T02:14:36.929Z
Learning: In `actions/cache`, the `saveCache` function already handles concurrent cache creation gracefully. When `reserveCache` throws a `ReserveCacheError` (because another job is creating the same cache key), `saveCache` catches it internally and logs it with `core.info()` rather than throwing—no additional error handling is required by callers.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
2 participants
Make use of goreleaser binary in cache if present, would also close #476. Useful for self-hosted runners.