Add prebuilt installers and cargo-binstall metadata#379
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebf79f85c0
ℹ️ 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".
|
|
||
| local tmp_dir | ||
| tmp_dir="$(mktemp -d)" | ||
| trap 'rm -rf "$tmp_dir"' EXIT |
There was a problem hiding this comment.
Avoid unbound tmp_dir in cargo-mono EXIT trap
tmp_dir is declared as a function-local variable, but the trap is registered for EXIT, which runs after install_direct returns. With set -u, that makes "$tmp_dir" unbound at process exit, so a successful install path still terminates with a non-zero status (tmp_dir: unbound variable). This will break automation that treats installer exit codes as authoritative.
Useful? React with 👍 / 👎.
|
|
||
| local tmp_dir | ||
| tmp_dir="$(mktemp -d)" | ||
| trap 'rm -rf "$tmp_dir"' EXIT |
There was a problem hiding this comment.
Avoid unbound tmp_dir in with-watch EXIT trap
This has the same failure mode as the cargo-mono installer: tmp_dir is local to install_direct, but the EXIT trap references it after the function scope ends. Under set -u, the trap throws an unbound-variable error and the script exits non-zero even when installation succeeded, which can fail CI/bootstrap flows.
Useful? React with 👍 / 👎.
Summary
cargo-monoandwith-watchon Bash and PowerShellcargo-binstallmetadata forcargo-mono,with-watch, andnodeupTesting
cargo testpnpm --filter public-docs testgit diff --checkscripts/install/cargo-mono.sh --helpscripts/install/with-watch.sh --helpNotes
pwsh,cosign, andcargo-binstallwere not available in the local environment, so their smoke tests were not run here.