Fix the cargo-binstall self-update that can never succeed on Windows - #5380
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The fallback silently discards custom installation locations, potentially updating the wrong executable.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Improves Cargo’s cargo-binstall self-update recovery by retrying locally through cargo install.
Changes:
- Adds self-install/update detection and fallback behavior.
- Adds an override flag and regression tests.
File summaries
| File | Description |
|---|---|
CargoOperationHelperTests.cs |
Tests fallback and broker behavior. |
CargoPkgOperationHelper.cs |
Implements self-update retry logic. |
OverridenInstallationOptions.cs |
Adds the binstall bypass flag. |
Review details
Suppressed comments (1)
src/UniGetUI.PackageEngine.Managers.Cargo/Helpers/CargoPkgOperationHelper.cs:62
- The update fallback also loses
options.CustomInstallLocation. SinceCargo_DoNotUseBinstallmakeshasBinstallfalse, lines 87-88 no longer append the selected location andcargo installtargets the default Cargo home; the retry may report success while leaving the custom-location executable unchanged. Preserve that destination with an appropriate Cargo mapping/staging step, or do not auto-retry this case.
else if (targetsBinstallItself)
parameters = ["install", package.Id, "--locked", "--force"];
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The fallback is bounded, preserves existing custom-path behavior, and has comprehensive tests.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
There was a problem hiding this comment.
🤖 Pull request was approved automatically: the AI review is complete and all its review threads are resolved. 🎉
Integration Details
{
"deliveryId": "35f736a0-b1ca-11f1-812d-df653807bf9d",
"headSha": "9db3a5f69d3ab0090274703bff36a6b21b761608",
"reviewer": "copilot-pull-request-reviewer[bot]"
}
This pull request introduces improved handling for the installation and update of the
cargo-binstallpackage in the Cargo package manager integration. The main change ensures that whencargo-binstallis updating or installing itself, the system will automatically retry the operation using the standardcargo installcommand if the initial attempt withcargo-binstallfails. Additionally, new tests have been added to verify this logic and other related behaviors.Cargo package manager improvements:
Added logic in
CargoPkgOperationHelperto detect whencargo-binstallis installing or updating itself and, on failure, automatically retry the operation using the standardcargo installcommand instead ofcargo-binstall. This prevents issues with self-updates that can fail due to file locks or permission errors. [1] [2] [3] [4] [5]Introduced a new option
Cargo_DoNotUseBinstallin theOverridenInstallationOptionsstruct to control whethercargo-binstallshould be used for a particular package operation, and updated theToString()method to include this option. [1] [2]Testing enhancements:
CargoOperationHelperTeststo verify the new retry logic, parameter construction, and to ensure that retrying is not performed in brokered operations or for other packages.