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

cargo install should accept --version (in addition to --vers?) #4590

Closed
shepmaster opened this issue Oct 7, 2017 · 10 comments · Fixed by #4637
Closed

cargo install should accept --version (in addition to --vers?) #4590

shepmaster opened this issue Oct 7, 2017 · 10 comments · Fixed by #4637
Labels
A-cargo-api Area: cargo-the-library API and internal code issues C-bug Category: bug Command-install

Comments

@shepmaster
Copy link
Member

It's a long command line option; it shouldn't be needlessly truncated.

@alexcrichton
Copy link
Member

Unfortunately cargo install --version already prints out Cargo's version number (name clash here)

@shepmaster
Copy link
Member Author

shepmaster commented Oct 7, 2017

It appears that --version is currently unknown to cargo install:

$ cargo install diesel_cli --no-default-features --features postgres --version
error: Unknown flag: '--version'

$ cargo install diesel_cli --no-default-features --features postgres --version 0.16.0
error: Unknown flag: '--version'

$ cargo install --version diesel_cli
error: Unknown flag: '--version'

$ cargo install diesel_cli --version
error: Unknown flag: '--version'

$ cargo install --version
error: Unknown flag: '--version'

@alexcrichton
Copy link
Member

Heh, I'd consider that a bug!

@shepmaster
Copy link
Member Author

shepmaster commented Oct 7, 2017

Heh, I'd consider that a bug!

It's not unique:

$ cargo new --version
error: Unknown flag: '--version'

$ cargo build --version
error: Unknown flag: '--version'

$ cargo clean --version
error: Unknown flag: '--version'

$ cargo test --version
error: Unknown flag: '--version'

$ cargo search --version
error: Unknown flag: '--version'

Can we make lemonade out of this lemon and only support --version on plain cargo? Especially if no one is complaining about the "bug"...

@alexcrichton
Copy link
Member

Hm I suppose so yeah!

@shepmaster
Copy link
Member Author

shepmaster commented Oct 7, 2017

A quick skim through all the built-in subcommands shows they all have the same behavior.

Cleaned-up output of all commands
cargo bench --version
error: Unknown flag: '--version'

cargo build --version
error: Unknown flag: '--version'

cargo check --version
error: Unknown flag: '--version'

cargo clean --version
error: Unknown flag: '--version'

cargo doc --version
error: Unknown flag: '--version'

cargo fetch --version
error: Unknown flag: '--version'

cargo generate-lockfile --version
error: Unknown flag: '--version'

cargo git-checkout --version
error: Unknown flag: '--version'

cargo help --version
error: no such subcommand: `--version`

cargo init --version
error: Unknown flag: '--version'

cargo install --version
error: Unknown flag: '--version'

cargo locate-project --version
error: Unknown flag: '--version'

cargo login --version
error: Unknown flag: '--version'

cargo metadata --version
error: Unknown flag: '--version'

cargo new --version
error: Unknown flag: '--version'

cargo owner --version
error: Unknown flag: '--version'

cargo package --version
error: Unknown flag: '--version'

cargo pkgid --version
error: Unknown flag: '--version'

cargo publish --version
error: Unknown flag: '--version'

cargo read-manifest --version
error: Unknown flag: '--version'

cargo run --version
error: Unknown flag: '--version'

cargo rustc --version
error: Unknown flag: '--version'

cargo rustdoc --version
error: Unknown flag: '--version'

cargo search --version
error: Unknown flag: '--version'

cargo test --version
error: Unknown flag: '--version'

cargo uninstall --version
error: Unknown flag: '--version'

cargo update --version
error: Unknown flag: '--version'

cargo verify-project --version
error: Unknown flag: '--version'

cargo version --version
error: Unknown flag: '--version'. Did you mean 'version'?

cargo yank --version
error: Unknown flag: '--version'

Third-party cargo subcommands do frequently support --version, but that seems like a reasonable difference.

@carols10cents
Copy link
Member

cargo version --version
error: Unknown flag: '--version'. Did you mean 'version'?

lol

@carols10cents carols10cents added A-cargo-api Area: cargo-the-library API and internal code issues C-bug Category: bug Command-install labels Oct 16, 2017
@kivikakk
Copy link
Contributor

It's real easy to fix this naïvely:

diff --git a/src/bin/install.rs b/src/bin/install.rs
index c7062d40..35a09a74 100644
--- a/src/bin/install.rs
+++ b/src/bin/install.rs
@@ -23,7 +23,7 @@ pub struct Options {
     flag_locked: bool,

     arg_crate: Vec<String>,
-    flag_vers: Option<String>,
+    flag_version: Option<String>,

     flag_git: Option<String>,
     flag_branch: Option<String>,
@@ -43,7 +43,7 @@ Usage:
     cargo install [options] --list

 Specifying what crate to install:
-    --vers VERS               Specify a version to install from crates.io
+    --version VERSION         Specify a version to install from crates.io
     --git URL                 Git URL to install the specified crate from
     --branch BRANCH           Branch to use when installing from git
     --tag TAG                 Tag to use when installing from git
@@ -151,7 +151,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
     };

     let krates = options.arg_crate.iter().map(|s| &s[..]).collect::<Vec<_>>();
-    let vers = options.flag_vers.as_ref().map(|s| &s[..]);
+    let vers = options.flag_version.as_ref().map(|s| &s[..]);
     let root = options.flag_root.as_ref().map(|s| &s[..]);

The problem is we probably want to support --vers as well (for at least some period of time), but ideally without showing it in the USAGE string. This seems a bit complicated with the way cargo subcommands are put together currently.

@alexcrichton
Copy link
Member

@kivikakk that looks fine to me, but I don't think we'll want to deprecate the --vers flag any time soon, instead we can probably just accept both flags (and prefer one arbitrarily if both are passed)

@kivikakk
Copy link
Contributor

@alexcrichton agreed! Showing both in the USAGE feels a bit gross, is all. I'll see how it looks in practice.

bors added a commit that referenced this issue Oct 19, 2017
Allow cargo install --version as well (preferred)

Fixes #4590.

The usage looks pretty ugly with this extra line in it, but it's not really avoidable with our docopt use.

We fail if both are provided; accepting both feels like a path to demons.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo-api Area: cargo-the-library API and internal code issues C-bug Category: bug Command-install
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants