Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: abort install for specific version if not confirmed #125

Merged
merged 1 commit into from
Apr 30, 2022
Merged

fix: abort install for specific version if not confirmed #125

merged 1 commit into from
Apr 30, 2022

Conversation

somehowchris
Copy link
Contributor

This should fix the case for #113 where --no-confirm or --dry-run just continues to install

@ryankurte ryankurte merged commit 26711f3 into cargo-bins:main Apr 30, 2022
@passcod
Copy link
Member

passcod commented May 2, 2022

I'd actually done that intentionally. Like this particular branch is very much a warning. The behaviour is correct, but might not be what you meant, so it tells you to fix your command if you didn't mean that.

  • If you use 1.2.3 and meant =1.2.3, it will warn you and continue in scripting (with --no-confirm) but warn you and prompt you if you're human (without --no-confirm), just to check.
  • If you use =1.2.3 it won't use this branch at all.
  • If you use ^1.2.3, it also won't use this branch (essentially "I meant to do that").
  • But if you use 1.2.3, and meant ^1.2.3 because that's how versions work in Cargo, I don't think it should abort.

With this change, we're saying that using 1.2.3 to mean ^1.2.3, which is the expected behaviour when using Cargo-style version requirements, is always wrong unless it specifically happens to match the latest compatible version.

The alternative I think would be to ditch the current "--version is a version requirement" and instead go explicitly the other way, and say "--version is always an exact version", which incidentally is the cargo install behaviour so it would bring us more in line with being a "drop in replacement."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants