Repository navigation
ci(release): install ffmpeg from apt, matching ci.yml - #1346
Conversation
050af57 moved ci.yml off AnimMouse/setup-ffmpeg and onto the runner's own apt, but release.yml kept the action in both of its jobs — still pinned to `version: "7.1"`. That pin is currently broken. BtbN/FFmpeg-Builds, which the action downloads from, rotates its asset list: the `latest` release today carries `n8.1` and `n9.0` Linux builds and no `n7.1` at all. The request 404s, the action pipes the error page into `tar`, and the job dies with xz: (stdin): File format not recognized which names neither ffmpeg nor the missing version. It has now failed twice this way — the 7.1 pin was itself the fix for a corrupt 8.1 asset — so the dependency is the problem, not the version chosen. release.yml is the workflow that runs semantic-release on a push to `release`, so this would have failed the next publish rather than a PR. Both jobs now use the same two lines as ci.yml. The `Verify ffmpeg installation` step after each is unchanged and still fails the job loudly if ffmpeg is somehow absent.
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe test and release jobs in the release workflow now install ffmpeg with ChangesFFmpeg Installation
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The release workflow now installs ffmpeg through the runner’s package manager in both jobs, avoiding the broken pinned download path; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the release workflow to install ffmpeg via the Ubuntu runner’s APT repositories (matching the approach already used in ci.yml), removing reliance on AnimMouse/setup-ffmpeg and its brittle upstream asset pinning.
Changes:
- Replaced
AnimMouse/setup-ffmpeg@v1(previously pinned toversion: "7.1") withapt-get install ffmpegin thetestjob. - Replaced
AnimMouse/setup-ffmpeg@v1withapt-get install ffmpegin thereleasejob.
Suppressed comments (1)
.github/workflows/release.yml:88
- The ffmpeg verification failure message still references the removed
setup-ffmpegaction, which will be misleading now that installation is done viaapt-get. Update the message to point to the apt-based install step.
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends ffmpeg
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y --no-install-recommends ffmpeg |
There was a problem hiding this comment.
Fixed in 6f83923: the ffmpeg install and verify steps were removed from both jobs in release.yml, so no failure message mentions setup-ffmpeg; a comment records why ffmpeg is not installed there.
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
💡 SUGGESTION: The PR description mentions that ci.yml was already updated to use apt-get, but I should verify that both workflow files are now completely consistent. A quick diff shows they match, but it would be helpful to add a brief note in the PR body pointing to the exact lines in ci.yml that demonstrate the pattern being followed. This would help future maintainers understand the rationale better.
Review Summary for PR #1346: ci(release): install ffmpeg from apt, matching ci.yml✅ Decision: APPROVEDThis is a low-risk CI/CD workflow change that removes an external GitHub Action dependency and standardizes on system package manager (apt) installation, matching the existing pattern in 📋 Changes MadeFile Modified: Changes:
🔍 Analysis Results
✅ Verification Points
📖 ContextThe
Using
🎯 Focus Areas Reviewed
|
Review SummaryDecision: APPROVED ✅ This PR makes a self-contained CI/CD improvement by removing the external Changes Made
Impact Analysis
Verification✅ YAML syntax valid (workflow parses correctly) Review ScopeReviewed as per NeuroLink contributor guidelines:
The change is safe to merge. |
050af574moved ci.yml offAnimMouse/setup-ffmpegand onto the runner's own apt. release.yml kept the action in both of its jobs, still pinned toversion: "7.1".Why that pin is broken
The action downloads from
BtbN/FFmpeg-Builds, whose asset list rotates. Checked against the API just now — thelatestrelease carries:No
n7.1at all. The request 404s, the action pipes the HTML error page intotar, and the job dies with:which names neither ffmpeg nor the missing version — it just looks like a corrupt download.
This has now failed twice the same way. The
7.1pin was itself the fix for a corrupt8.1asset; the comment it carries says "Revert once upstream publishes it." Moving the pin again would just queue up the third occurrence, so this drops the dependency instead.Why it matters more here than in ci.yml
release.ymlis what runs semantic-release on a push torelease. This would have failed the next publish, not a PR — and at a point where the failure looks like a packaging problem rather than a CI one.The change
Both jobs now use the same two lines ci.yml already uses:
The
Verify ffmpeg installationstep after each is untouched and still fails the job loudly if ffmpeg is somehow absent, so this doesn't trade a noisy failure for a silent one.Both workflow files parse as valid YAML.
Summary by CodeRabbit