Auto download wasm-opt to ./target/cargo-run/bin#4256
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a download feature to the cargo-run tool, enabling automatic installation of requirements like wasm-opt by downloading and extracting official Binaryen releases. It also changes the default prompt behavior for automatic resolution to default to 'No' ([y/N]). The review feedback identifies two key issues: a path separator mismatch on Windows that would prevent successful extraction of wasm-opt, and a potential workspace ambiguity when running cargo run without specifying the package name.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
6 issues found across 8 files
Confidence score: 2/5
tools/cargo-run/src/download.rstar unpacking currently permits symlink/hardlink entries and misses WindowsComponent::Prefix(_), so a crafted archive can write outside the target directory and overwrite unintended files. Block link entries and reject prefix/parent traversal (or switch to a guaranteed in-root unpack routine) before merging.tools/cargo-run/src/requirements/wasm_opt.rsuses the wrong Binaryen platform identifier for Intel macOS, which will breakwasm-optauto-install on x86_64 Macs and leave users without the required tool. Fix the asset mapping for Intel macOS and verify download/install on that platform before merge.tools/cargo-run/src/download.rspath rebuilding via native components can produce backslash-separated paths on Windows (for entries likebin\wasm-opt), which can cause extraction/lookup mismatches and missing binaries after install. Normalize archive entry paths to forward-slash form during strip/rewrite and add a Windows-focused test case.tools/cargo-run/src/requirements.rsintroduces anunreachable!()branch forInstallAction::None, so an unexpected input can panic the whole requirement-check flow instead of failing gracefully. Replace the panic with explicit handling/logging to keep the command resilient.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Performance Benchmark Results
|
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Performance Benchmark Results
|
9e05893 to
44726dd
Compare
f651f2b to
afa28cb
Compare
Performance Benchmark Results
|
depends on #4254