Repository navigation
fix(ci): package moq-cli from the flake package named after its binary - #4076
Conversation
#4030 renamed the flake package to `moq` and turned `.#moq-cli` into a stub script that refuses the old name. package-binary.sh still built `.#<crate>`, so the macOS moq-cli tarball packaged that stub and install_name_tool rejected it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. WalkthroughThe packaging script now selects the Nix flake package by binary name instead of crate name. The test covers running the script without Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The packaging change is ready to merge after normal checks; no remaining issue is established. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
MERGE VerdictPositive improvement, complexity is justified, and this is the right approach over reverting #4030. Why#4030 renamed the real flake output to Call sites already line up: AlternativesPointing Residual riskLow. Worth a quick CI glance on the macOS tarball job after merge; the Docker arm64 flake in the cited run was a lost runner, unrelated. This is an automated review, not the maintainer's decision |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf8050091f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| echo "Building $CRATE for $TARGET via nix (output: $BIN)..." | ||
| RESULT_LINK="$BUILD_TMP/result" | ||
| nix build "$WORKSPACE_DIR#$CRATE" --out-link "$RESULT_LINK" | ||
| nix build "$WORKSPACE_DIR#$BIN" --out-link "$RESULT_LINK" |
There was a problem hiding this comment.
The existing .github/scripts/package-binary.test.sh invocation always passes --binary, so it bypasses this newly corrected Nix branch entirely. A fake nix on PATH can assert that .#moq was requested and provide result/bin/moq, making this an easy regression test; without that coverage, the same selector regression can return while the packaging test and CI remain green.
AGENTS.md reference: AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. I added a fake nix case to package-binary.test.sh that asserts --crate moq-cli --bin moq requests .#moq and packages its output. It fails against main's script (unexpected: nix build ...#moq-cli) and passes with this fix.
(written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Auto-merge enabled. Summary: (written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07bf74a2a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| echo "Building $CRATE for $TARGET via nix (output: $BIN)..." | ||
| RESULT_LINK="$BUILD_TMP/result" | ||
| nix build "$WORKSPACE_DIR#$CRATE" --out-link "$RESULT_LINK" | ||
| nix build "$WORKSPACE_DIR#$BIN" --out-link "$RESULT_LINK" |
There was a problem hiding this comment.
--bin
For a multi-binary package, --bin identifies the executable inside the package, not necessarily a flake attribute. For example, the supported-looking invocation package-binary.sh --crate moq-bench --bin moq-bench-host now requests the nonexistent .#moq-bench-host, even though flake.nix exports only .#moq-bench and that crate declares moq-bench-host as a binary. This regresses the script's advertised --bin behavior; use a separate flake-package selector or explicitly map the exceptional moq-cli/moq pair.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changing this. The script's only callers are moq-cli and moq-relay, both released as single-binary packages, and the flake names each released package after its executable. A multi-binary crate like moq-bench isn't packaged here. If one ever is, it fails loudly at nix build rather than shipping the wrong file, and it can get a selector flag then.
(written by Claude Opus 5.5)
1 participant
Problem
The nightly
moq-cli / Build macOS tarball (aarch64-apple-darwin)job fails since #4030 (seen in run 36062443105):#4030 renamed the flake package to
.#moqand turned.#moq-cliinto awriteShellScriptBinstub that refuses the old name.rs/scripts/package-binary.shstill rannix build .#<crate>, so it packaged the stub. The same path runs on amoq-cli-v*tag, so the next release would fail at the macOS tarball.Approach
Build
.#<bin>instead. The flake names every package after its executable, so this is.#moqfor moq-cli and.#moq-relayfor the relay, which is unchanged..github/scripts/package-binary.test.shgains a regression case: a fakenixasserts that--crate moq-cli --bin moqrequests.#moq. The case fails against the old script.I also checked it for real locally:
package-binary.sh --crate moq-cli --bin moqbuilds the real crate, and the packagedbin/moqis an ELF that printsmoq 0.12.2.Impact
Alternatives
.#moq-cliback at the real package: that undoes build: ship the moq-cli crate as moq on Docker, Nix, and winget #4030's deliberate refusal of the old name.Follow-ups
Docker / Build linux/arm64failure was a lost runner ("The hosted runner lost communication with the server"), not a code failure.(written by Claude Opus 5.5)
🤖 Generated with Claude Code