fix: resolve latest BtbN ffmpeg asset dynamically (#459) - #632
Conversation
* Replace hardcoded `ffmpeg-n5.1` matcher with a regex that captures major/minor of any numbered build, then pick the highest version present in the release. BtbN/FFmpeg-Builds no longer ships 5.1 builds, which broke `installLatestFfmpeg()` on every run. * Fall back to `ffmpeg-master-latest-*` when no numbered build matches so future BtbN naming bumps don't break the action again. * Apply to both linux and win32 branches; darwin uses evermeet.cx and is unaffected. Fixes charmbracelet#459
* Use @octokit/openapi-types release-asset type instead of declaring a local BtbnAsset interface, per @dougmaitelli's suggestion. The octokit type is already available transitively and matches the shape returned by repos.getLatestRelease exactly. * Rename pickBtbnAsset to pickLatestVersionedAsset since the logic is generic (highest major.minor with master-name fallback) and is not BtbN-specific. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Just realised one of my original intentions was to add in more error logging, as one of the original issues is that it just failed without giving details of not being able to find the url, so let me add that in as well |
* Add describeMissingAsset helper that builds a descriptive error listing the patterns tried and the asset names actually present in the release, plus a pointer to file an issue. This addresses the silent "Failed to install ffmpeg" failure mode that motivated this PR: when upstream renamed BtbN artifacts, users got no context for why the download dropped out. * Throw the descriptive error from both BtbN call sites (linux, win32) and from both installTtyd call sites (linux, win32). * Log the resolved release tag and selected asset URL via core.info so successful runs also show what was picked. * Replace the non-null assertion on the evermeet.cx response with explicit status-code and shape checks. @actions/http-client does not throw on non-2xx, so the previous code would crash with an uninformative TypeError when evermeet was unreachable or changed its schema. * Wrap downloadTool and extract with try/catch that adds the source URL and archive path to any underlying error. * Drop the unreachable trailing reject and the now-redundant if (url) wrapper.
7eaea1b to
760919c
Compare
| asset => | ||
| asset.name.endsWith('win10.exe') || asset.name.endsWith('win32.exe') | ||
| )?.browser_download_url | ||
| if (!url) { |
There was a problem hiding this comment.
This and all the other error throwing might be a bit OOT, but I think it's better to be more explict than less, but happy to scale this back or remove it if it's not your vibe
There was a problem hiding this comment.
I think the amount of logs is good, I just wonder if we could make it less repetitive, like, instead of having the check / promise.reject for each case (os / arch) we could probably check after the switch and reject the promise in just one place
| break | ||
| } | ||
| case 'linux': { | ||
| url = release.data.assets.find(asset => |
There was a problem hiding this comment.
I wonder if it makes sense to use the new pickLatestVersionedAsset for determining the right asset here too
| } | ||
| switch (osPlatform) { | ||
| case 'win32': { | ||
| url = release.data.assets.find( |
There was a problem hiding this comment.
I wonder if it makes sense to use the new pickLatestVersionedAsset for determining the right asset here too
There was a problem hiding this comment.
Not really needed, just a maybe / suggestion
2 participants
Summary
installLatestFfmpeg()looked up release assets fromBtbN/FFmpeg-Buildswhose names start withffmpeg-n5.1and containlinux64-gpl-5.1/win64-gpl-5.1. That series is no longer published; the latest release only shipsn7.1,n8.1, andmaster-latestvariants. As a result every action run errored withFailed to install ffmpeg(Error: Failed to install ffmpeg #459).ffmpeg-master-latest-*asset if no numbered build matches, so future BtbN naming bumps don't break the action again.linuxandwin32branches;darwinuses evermeet.cx and is unaffected.dist/index.jsregenerated vianpm run package.Test plan
npm run build,npm run format-check,npm run package,npm testall succeed locally.nektos/actagainst.github/workflows/ci.yml(ubuntu-latest leg) that the action now installs ffmpeg + ttyd + VHS successfully andvhs -hruns (terminalupload-artifact@v7failure was an act-image quirk, not an action failure).Fixes #459