From 4bf1af0cd0acb09e45cb331345ff4a771ebd5a66 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 23 Aug 2023 09:13:34 -0500 Subject: [PATCH 1/2] fix(update): Make `-p` more convenient by being positional Generally, cargo avoids positional arguments. Mostly for the commands that might forward arguments to another command, like `cargo test`. It also allows some flexibility in turning flags into options. For `cargo add` and `cargo remove`, we decided to accept positionals because the motivations didn't seem to apply as much (similar to `cargo install`). This applies the pattern to `cargo update` as well which is in the same category of commands as `cargo add` and `cargo remove`. As for `--help` formatting, I'm mixed on whether `[SPEC]...` should be at the top like other positionals or should be relegated to "Package selection". I went with the latter mostly to make it easier to visualize the less common choice. Switching to a positional for `cargo update` (while keeping `-p` for backwards compatibility) was referenced in #12425. --- src/bin/cargo/commands/update.rs | 25 ++++++++++++++-- src/cargo/ops/cargo_compile/mod.rs | 2 +- src/cargo/ops/resolve.rs | 4 +-- src/cargo/sources/registry/mod.rs | 2 +- src/doc/man/cargo-update.md | 16 +++++----- src/doc/man/generated_txt/cargo-update.txt | 26 ++++++++-------- src/doc/src/commands/cargo-update.md | 17 +++++------ src/doc/src/guide/cargo-toml-vs-cargo-lock.md | 4 +-- .../src/reference/overriding-dependencies.md | 2 +- src/doc/src/reference/resolver.md | 4 +-- .../src/reference/specifying-dependencies.md | 2 +- src/etc/_cargo | 3 +- src/etc/man/cargo-update.1 | 17 +++++------ tests/testsuite/cargo_update/help/stdout.log | 6 ++-- tests/testsuite/future_incompat_report.rs | 2 +- tests/testsuite/git.rs | 18 +++++------ tests/testsuite/offline.rs | 4 +-- tests/testsuite/patch.rs | 6 ++-- tests/testsuite/registry.rs | 20 ++++++------- tests/testsuite/replace.rs | 4 +-- tests/testsuite/rust_version.rs | 2 +- tests/testsuite/update.rs | 30 +++++++++---------- 22 files changed, 117 insertions(+), 99 deletions(-) diff --git a/src/bin/cargo/commands/update.rs b/src/bin/cargo/commands/update.rs index 31175ef168c..0ceeb7071e0 100644 --- a/src/bin/cargo/commands/update.rs +++ b/src/bin/cargo/commands/update.rs @@ -6,6 +6,20 @@ use cargo::util::print_available_packages; pub fn cli() -> Command { subcommand("update") .about("Update dependencies as recorded in the local lock file") + .args([clap::Arg::new("package2") + .action(clap::ArgAction::Append) + .num_args(1..) + .value_name("SPEC") + .help_heading(heading::PACKAGE_SELECTION) + .group("package-group") + .help("Package to update")]) + .arg( + optional_multi_opt("package", "SPEC", "Package to update") + .short('p') + .hide(true) + .help_heading(heading::PACKAGE_SELECTION) + .group("package-group"), + ) .arg_dry_run("Don't actually write the lockfile") .arg( flag( @@ -20,7 +34,7 @@ pub fn cli() -> Command { "Update a single dependency to exactly PRECISE when used with -p", ) .value_name("PRECISE") - .requires("package"), + .requires("package-group"), ) .arg_quiet() .arg( @@ -28,7 +42,6 @@ pub fn cli() -> Command { .short('w') .help_heading(heading::PACKAGE_SELECTION), ) - .arg_package_spec_simple("Package to update") .arg_manifest_path() .after_help("Run `cargo help update` for more detailed information.\n") } @@ -40,10 +53,16 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { print_available_packages(&ws)?; } + let to_update = if args.contains_id("package") { + "package" + } else { + "package2" + }; + let update_opts = UpdateOptions { aggressive: args.flag("aggressive"), precise: args.get_one::("precise").map(String::as_str), - to_update: values(args, "package"), + to_update: values(args, to_update), dry_run: args.dry_run(), workspace: args.flag("workspace"), config, diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index 9490246cdee..ba677032d70 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -508,7 +508,7 @@ pub fn create_bcx<'a, 'cfg>( } else if !unit.is_local() { format!( "Either upgrade to rustc {} or newer, or use\n\ - cargo update -p {}@{} --precise ver\n\ + cargo update {}@{} --precise ver\n\ where `ver` is the latest version of `{}` supporting rustc {}", version, unit.pkg.name(), diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 195a03616e3..d528e8a0441 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -612,7 +612,7 @@ fn register_previous_locks( // Ok so we've been passed in a `keep` function which basically says "if I // return `true` then this package wasn't listed for an update on the command - // line". That is, if we run `cargo update -p foo` then `keep(bar)` will return + // line". That is, if we run `cargo update foo` then `keep(bar)` will return // `true`, whereas `keep(foo)` will return `false` (roughly speaking). // // This isn't actually quite what we want, however. Instead we want to @@ -622,7 +622,7 @@ fn register_previous_locks( // * There's a crate `log`. // * There's a crate `serde` which depends on `log`. // - // Let's say we then run `cargo update -p serde`. This may *also* want to + // Let's say we then run `cargo update serde`. This may *also* want to // update the `log` dependency as our newer version of `serde` may have a // new minimum version required for `log`. Now this isn't always guaranteed // to work. What'll happen here is we *won't* lock the `log` dependency nor diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index c1ff8b12e09..a5b6972d8dd 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -251,7 +251,7 @@ pub struct RegistrySource<'cfg> { /// yanked. /// /// This is populated from the entries in `Cargo.lock` to ensure that - /// `cargo update -p somepkg` won't unlock yanked entries in `Cargo.lock`. + /// `cargo update somepkg` won't unlock yanked entries in `Cargo.lock`. /// Otherwise, the resolver would think that those entries no longer /// exist, and it would trigger updates to unrelated packages. yanked_whitelist: HashSet, diff --git a/src/doc/man/cargo-update.md b/src/doc/man/cargo-update.md index e91606a6a09..a24330fad4f 100644 --- a/src/doc/man/cargo-update.md +++ b/src/doc/man/cargo-update.md @@ -6,7 +6,7 @@ cargo-update --- Update dependencies as recorded in the local lock file ## SYNOPSIS -`cargo update` [_options_] +`cargo update` [_options_] _spec_ ## DESCRIPTION @@ -20,26 +20,26 @@ latest available versions. {{#options}} -{{#option "`-p` _spec_..." "`--package` _spec_..." }} +{{#option "_spec_..." }} Update only the specified packages. This flag may be specified multiple times. See {{man "cargo-pkgid" 1}} for the SPEC format. -If packages are specified with the `-p` flag, then a conservative update of +If packages are specified with _spec_, then a conservative update of the lockfile will be performed. This means that only the dependency specified by SPEC will be updated. Its transitive dependencies will be updated only if SPEC cannot be updated without updating dependencies. All other dependencies will remain locked at their currently recorded versions. -If `-p` is not specified, all dependencies are updated. +If _spec_ is not specified, all dependencies are updated. {{/option}} {{#option "`--aggressive`" }} -When used with `-p`, dependencies of _spec_ are forced to update as well. +When used with _spec_, dependencies of _spec_ are forced to update as well. Cannot be used with `--precise`. {{/option}} {{#option "`--precise` _precise_" }} -When used with `-p`, allows you to specify a specific version number to set +When used with _spec_, allows you to specify a specific version number to set the package to. If the package comes from a git repository, this can be a git revision (such as a SHA hash or tag). {{/option}} @@ -87,11 +87,11 @@ Displays what would be updated, but doesn't actually write the lockfile. 2. Update only specific dependencies: - cargo update -p foo -p bar + cargo update foo bar 3. Set a specific dependency to a specific version: - cargo update -p foo --precise 1.2.3 + cargo update foo --precise 1.2.3 ## SEE ALSO {{man "cargo" 1}}, {{man "cargo-generate-lockfile" 1}} diff --git a/src/doc/man/generated_txt/cargo-update.txt b/src/doc/man/generated_txt/cargo-update.txt index fb662c3890f..c7554c197f1 100644 --- a/src/doc/man/generated_txt/cargo-update.txt +++ b/src/doc/man/generated_txt/cargo-update.txt @@ -4,7 +4,7 @@ NAME cargo-update — Update dependencies as recorded in the local lock file SYNOPSIS - cargo update [options] + cargo update [options] spec DESCRIPTION This command will update dependencies in the Cargo.lock file to the @@ -13,25 +13,25 @@ DESCRIPTION OPTIONS Update Options - -p spec…, --package spec… + spec… Update only the specified packages. This flag may be specified multiple times. See cargo-pkgid(1) for the SPEC format. - If packages are specified with the -p flag, then a conservative - update of the lockfile will be performed. This means that only the - dependency specified by SPEC will be updated. Its transitive - dependencies will be updated only if SPEC cannot be updated without - updating dependencies. All other dependencies will remain locked at - their currently recorded versions. + If packages are specified with spec, then a conservative update of + the lockfile will be performed. This means that only the dependency + specified by SPEC will be updated. Its transitive dependencies will + be updated only if SPEC cannot be updated without updating + dependencies. All other dependencies will remain locked at their + currently recorded versions. - If -p is not specified, all dependencies are updated. + If spec is not specified, all dependencies are updated. --aggressive - When used with -p, dependencies of spec are forced to update as + When used with spec, dependencies of spec are forced to update as well. Cannot be used with --precise. --precise precise - When used with -p, allows you to specify a specific version number + When used with spec, allows you to specify a specific version number to set the package to. If the package comes from a git repository, this can be a git revision (such as a SHA hash or tag). @@ -155,11 +155,11 @@ EXAMPLES 2. Update only specific dependencies: - cargo update -p foo -p bar + cargo update foo bar 3. Set a specific dependency to a specific version: - cargo update -p foo --precise 1.2.3 + cargo update foo --precise 1.2.3 SEE ALSO cargo(1), cargo-generate-lockfile(1) diff --git a/src/doc/src/commands/cargo-update.md b/src/doc/src/commands/cargo-update.md index 3cfd6928222..e08f6f91fed 100644 --- a/src/doc/src/commands/cargo-update.md +++ b/src/doc/src/commands/cargo-update.md @@ -6,7 +6,7 @@ cargo-update --- Update dependencies as recorded in the local lock file ## SYNOPSIS -`cargo update` [_options_] +`cargo update` [_options_] _spec_ ## DESCRIPTION @@ -20,25 +20,24 @@ latest available versions.
-
-p spec…
-
--package spec…
+
spec…
Update only the specified packages. This flag may be specified multiple times. See cargo-pkgid(1) for the SPEC format.

-

If packages are specified with the -p flag, then a conservative update of +

If packages are specified with spec, then a conservative update of the lockfile will be performed. This means that only the dependency specified by SPEC will be updated. Its transitive dependencies will be updated only if SPEC cannot be updated without updating dependencies. All other dependencies will remain locked at their currently recorded versions.

-

If -p is not specified, all dependencies are updated.

+

If spec is not specified, all dependencies are updated.

--aggressive
-
When used with -p, dependencies of spec are forced to update as well. +
When used with spec, dependencies of spec are forced to update as well. Cannot be used with --precise.
--precise precise
-
When used with -p, allows you to specify a specific version number to set +
When used with spec, allows you to specify a specific version number to set the package to. If the package comes from a git repository, this can be a git revision (such as a SHA hash or tag).
@@ -187,11 +186,11 @@ details on environment variables that Cargo reads. 2. Update only specific dependencies: - cargo update -p foo -p bar + cargo update foo bar 3. Set a specific dependency to a specific version: - cargo update -p foo --precise 1.2.3 + cargo update foo --precise 1.2.3 ## SEE ALSO [cargo(1)](cargo.html), [cargo-generate-lockfile(1)](cargo-generate-lockfile.html) diff --git a/src/doc/src/guide/cargo-toml-vs-cargo-lock.md b/src/doc/src/guide/cargo-toml-vs-cargo-lock.md index ea2f04e1fc3..e6f6e135dd4 100644 --- a/src/doc/src/guide/cargo-toml-vs-cargo-lock.md +++ b/src/doc/src/guide/cargo-toml-vs-cargo-lock.md @@ -91,8 +91,8 @@ When we’re ready to opt in to a new version of the library, Cargo can re-calculate the dependencies and update things for us: ```console -$ cargo update # updates all dependencies -$ cargo update -p regex # updates just “regex” +$ cargo update # updates all dependencies +$ cargo update regex # updates just “regex” ``` This will write out a new `Cargo.lock` with the new version information. Note diff --git a/src/doc/src/reference/overriding-dependencies.md b/src/doc/src/reference/overriding-dependencies.md index c04a7929dcd..d5efac45209 100644 --- a/src/doc/src/reference/overriding-dependencies.md +++ b/src/doc/src/reference/overriding-dependencies.md @@ -97,7 +97,7 @@ $ cargo build And that's it! You're now building with the local version of `uuid` (note the path in parentheses in the build output). If you don't see the local path version getting -built then you may need to run `cargo update -p uuid --precise $version` where +built then you may need to run `cargo update uuid --precise $version` where `$version` is the version of the locally checked out copy of `uuid`. Once you've fixed the bug you originally found the next thing you'll want to do diff --git a/src/doc/src/reference/resolver.md b/src/doc/src/reference/resolver.md index ffb194c5e4a..49beb5d60af 100644 --- a/src/doc/src/reference/resolver.md +++ b/src/doc/src/reference/resolver.md @@ -464,10 +464,10 @@ situations may require specifying unusual requirements. If you fail to do this, it may not be immediately obvious because Cargo can opportunistically choose the newest version when you run a blanket `cargo update`. However, if another user depends on your library, and runs `cargo - update -p your-library`, it will *not* automatically update "bar" if it is + update your-library`, it will *not* automatically update "bar" if it is locked in their `Cargo.lock`. It will only update "bar" in that situation if the dependency declaration is also updated. Failure to do so can cause - confusing build errors for the user using `cargo update -p`. + confusing build errors for the user using `cargo update your-library`. * If two packages are tightly coupled, then an `=` dependency requirement may help ensure that they stay in sync. For example, a library with a companion proc-macro library will sometimes make assumptions between the two libraries diff --git a/src/doc/src/reference/specifying-dependencies.md b/src/doc/src/reference/specifying-dependencies.md index 56a33eb2cc6..f08d508a21d 100644 --- a/src/doc/src/reference/specifying-dependencies.md +++ b/src/doc/src/reference/specifying-dependencies.md @@ -23,7 +23,7 @@ The string `"0.1.12"` is a version requirement. Although it looks like a specific *version* of the `time` crate, it actually specifies a *range* of versions and allows [SemVer] compatible updates. An update is allowed if the new version number does not modify the left-most non-zero number in the major, minor, -patch grouping. In this case, if we ran `cargo update -p time`, cargo should +patch grouping. In this case, if we ran `cargo update time`, cargo should update us to version `0.1.13` if it is the latest `0.1.z` release, but would not update us to `0.2.0`. If instead we had specified the version string as `1.0`, cargo should update to `1.1` if it is the latest `1.y` release, but not `2.0`. diff --git a/src/etc/_cargo b/src/etc/_cargo index 164d7679f2b..a017b51c552 100644 --- a/src/etc/_cargo +++ b/src/etc/_cargo @@ -344,7 +344,8 @@ _cargo() { '--aggressive=[force dependency update]' \ "--dry-run[don't actually write the lockfile]" \ '(-p --package)'{-p+,--package=}'[specify package to update]:package:_cargo_package_names' \ - '--precise=[update single dependency to precise release]:release' + '--precise=[update single dependency to precise release]:release' \ + '*:package:_cargo_package_names' ;; verify-project) diff --git a/src/etc/man/cargo-update.1 b/src/etc/man/cargo-update.1 index 6f697b3ab19..0ea8db3a881 100644 --- a/src/etc/man/cargo-update.1 +++ b/src/etc/man/cargo-update.1 @@ -6,7 +6,7 @@ .SH "NAME" cargo\-update \[em] Update dependencies as recorded in the local lock file .SH "SYNOPSIS" -\fBcargo update\fR [\fIoptions\fR] +\fBcargo update\fR [\fIoptions\fR] \fIspec\fR .SH "DESCRIPTION" This command will update dependencies in the \fBCargo.lock\fR file to the latest version. If the \fBCargo.lock\fR file does not exist, it will be created with the @@ -14,30 +14,29 @@ latest available versions. .SH "OPTIONS" .SS "Update Options" .sp -\fB\-p\fR \fIspec\fR\[u2026], -\fB\-\-package\fR \fIspec\fR\[u2026] +\fIspec\fR\[u2026] .RS 4 Update only the specified packages. This flag may be specified multiple times. See \fBcargo\-pkgid\fR(1) for the SPEC format. .sp -If packages are specified with the \fB\-p\fR flag, then a conservative update of +If packages are specified with \fIspec\fR, then a conservative update of the lockfile will be performed. This means that only the dependency specified by SPEC will be updated. Its transitive dependencies will be updated only if SPEC cannot be updated without updating dependencies. All other dependencies will remain locked at their currently recorded versions. .sp -If \fB\-p\fR is not specified, all dependencies are updated. +If \fIspec\fR is not specified, all dependencies are updated. .RE .sp \fB\-\-aggressive\fR .RS 4 -When used with \fB\-p\fR, dependencies of \fIspec\fR are forced to update as well. +When used with \fIspec\fR, dependencies of \fIspec\fR are forced to update as well. Cannot be used with \fB\-\-precise\fR\&. .RE .sp \fB\-\-precise\fR \fIprecise\fR .RS 4 -When used with \fB\-p\fR, allows you to specify a specific version number to set +When used with \fIspec\fR, allows you to specify a specific version number to set the package to. If the package comes from a git repository, this can be a git revision (such as a SHA hash or tag). .RE @@ -200,7 +199,7 @@ cargo update .sp .RS 4 .nf -cargo update \-p foo \-p bar +cargo update foo bar .fi .RE .RE @@ -210,7 +209,7 @@ cargo update \-p foo \-p bar .sp .RS 4 .nf -cargo update \-p foo \-\-precise 1.2.3 +cargo update foo \-\-precise 1.2.3 .fi .RE .RE diff --git a/tests/testsuite/cargo_update/help/stdout.log b/tests/testsuite/cargo_update/help/stdout.log index 6cc10915179..871b4b23ae4 100644 --- a/tests/testsuite/cargo_update/help/stdout.log +++ b/tests/testsuite/cargo_update/help/stdout.log @@ -1,6 +1,6 @@ Update dependencies as recorded in the local lock file -Usage: cargo[EXE] update [OPTIONS] +Usage: cargo[EXE] update [OPTIONS] [SPEC]... Options: --dry-run Don't actually write the lockfile @@ -14,8 +14,8 @@ Options: -h, --help Print help Package Selection: - -w, --workspace Only update the workspace packages - -p, --package [] Package to update + -w, --workspace Only update the workspace packages + [SPEC]... Package to update Manifest Options: --manifest-path Path to Cargo.toml diff --git a/tests/testsuite/future_incompat_report.rs b/tests/testsuite/future_incompat_report.rs index 4d2c66d17f3..c52bf25a9e1 100644 --- a/tests/testsuite/future_incompat_report.rs +++ b/tests/testsuite/future_incompat_report.rs @@ -369,7 +369,7 @@ fn suggestions_for_updates() { // project or something). This could use some more consideration of how to // handle this better (maybe only trigger an update if it hasn't updated // in a long while?). - p.cargo("update -p without_updates").run(); + p.cargo("update without_updates").run(); let update_message = "\ - Some affected dependencies have newer versions available. diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index ff073451f1a..b47b3986722 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -742,11 +742,11 @@ fn update_with_shared_deps() { // By default, not transitive updates println!("dep1 update"); - p.cargo("update -p dep1").with_stdout("").run(); + p.cargo("update dep1").with_stdout("").run(); // Don't do anything bad on a weird --precise argument println!("bar bad precise update"); - p.cargo("update -p bar --precise 0.1.2") + p.cargo("update bar --precise 0.1.2") .with_status(101) .with_stderr( "\ @@ -764,14 +764,14 @@ Caused by: // Specifying a precise rev to the old rev shouldn't actually update // anything because we already have the rev in the db. println!("bar precise update"); - p.cargo("update -p bar --precise") + p.cargo("update bar --precise") .arg(&old_head.to_string()) .with_stdout("") .run(); // Updating aggressively should, however, update the repo. println!("dep1 aggressive update"); - p.cargo("update -p dep1 --aggressive") + p.cargo("update dep1 --aggressive") .with_stderr(&format!( "[UPDATING] git repository `{}`\n\ [UPDATING] bar v0.5.0 ([..]) -> #[..]\n\ @@ -795,7 +795,7 @@ Caused by: .run(); // We should be able to update transitive deps - p.cargo("update -p bar") + p.cargo("update bar") .with_stderr(&format!( "[UPDATING] git repository `{}`", git_project.url() @@ -1183,7 +1183,7 @@ fn two_deps_only_update_one() { let oid = git::commit(&repo); println!("dep1 head sha: {}", oid_to_short_sha(oid)); - p.cargo("update -p dep1") + p.cargo("update dep1") .with_stderr(&format!( "[UPDATING] git repository `{}`\n\ [UPDATING] dep1 v0.5.0 ([..]) -> #[..]\n\ @@ -1881,7 +1881,7 @@ fn update_ambiguous() { .build(); p.cargo("generate-lockfile").run(); - p.cargo("update -p bar") + p.cargo("update bar") .with_status(101) .with_stderr( "\ @@ -1928,7 +1928,7 @@ fn update_one_dep_in_repo_with_many_deps() { .build(); p.cargo("generate-lockfile").run(); - p.cargo("update -p bar") + p.cargo("update bar") .with_stderr(&format!("[UPDATING] git repository `{}`", bar.url())) .run(); } @@ -2091,7 +2091,7 @@ fn update_one_source_updates_all_packages_in_that_git_source() { git::add(&repo); git::commit(&repo); - p.cargo("update -p dep").run(); + p.cargo("update dep").run(); let lockfile = p.read_lockfile(); assert!( !lockfile.contains(&rev1.to_string()), diff --git a/tests/testsuite/offline.rs b/tests/testsuite/offline.rs index 5f164dbeb2b..e684253193f 100644 --- a/tests/testsuite/offline.rs +++ b/tests/testsuite/offline.rs @@ -673,7 +673,7 @@ fn main(){ .with_stdout("1.2.9") .run(); // updates happen without updating the index - p2.cargo("update -p present_dep --precise 1.2.3 --offline") + p2.cargo("update present_dep --precise 1.2.3 --offline") .with_status(0) .with_stderr( "\ @@ -706,7 +706,7 @@ fn main(){ .run(); // No v1.2.8 loaded into the cache so expect failure. - p2.cargo("update -p present_dep --precise 1.2.8 --offline") + p2.cargo("update present_dep --precise 1.2.8 --offline") .with_status(101) .with_stderr( "\ diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index f2f077d7d86..738e9244451 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -1965,8 +1965,8 @@ fn update_unused_new_version() { // Restore the lock file, and see if `update` will work, too. fs::copy(p.root().join("Cargo.lock.bak"), p.root().join("Cargo.lock")).unwrap(); - // Try `update -p`. - p.cargo("update -p bar") + // Try `update `. + p.cargo("update bar") .with_stderr( "\ [UPDATING] `dummy-registry` index @@ -2425,7 +2425,7 @@ fn can_update_with_alt_reg() { p.cargo("check").with_stderr("[FINISHED] [..]").run(); // This does nothing, due to `=` requirement. - p.cargo("update -p bar") + p.cargo("update bar") .with_stderr( "\ [UPDATING] `alternative` index diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 8982b1cb6c5..0569249e07f 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -807,7 +807,7 @@ required by package `foo v0.0.1 ([..])` ) .run(); - p.cargo("update -p baz") + p.cargo("update baz") .with_stderr_contains( "\ [UPDATING] `[..]` index @@ -952,7 +952,7 @@ fn update_lockfile() { Package::new("bar", "0.0.3").publish(); paths::home().join(".cargo/registry").rm_rf(); println!("0.0.2 update"); - p.cargo("update -p bar --precise 0.0.2") + p.cargo("update bar --precise 0.0.2") .with_stderr( "\ [UPDATING] `[..]` index @@ -975,7 +975,7 @@ fn update_lockfile() { .run(); println!("0.0.3 update"); - p.cargo("update -p bar") + p.cargo("update bar") .with_stderr( "\ [UPDATING] `[..]` index @@ -1000,7 +1000,7 @@ fn update_lockfile() { println!("new dependencies update"); Package::new("bar", "0.0.4").dep("spam", "0.2.5").publish(); Package::new("spam", "0.2.5").publish(); - p.cargo("update -p bar") + p.cargo("update bar") .with_stderr( "\ [UPDATING] `[..]` index @@ -1012,7 +1012,7 @@ fn update_lockfile() { println!("new dependencies update"); Package::new("bar", "0.0.5").publish(); - p.cargo("update -p bar") + p.cargo("update bar") .with_stderr( "\ [UPDATING] `[..]` index @@ -1433,7 +1433,7 @@ fn update_transitive_dependency() { Package::new("b", "0.1.1").publish(); - p.cargo("update -pb") + p.cargo("update b") .with_stderr( "\ [UPDATING] `[..]` index @@ -1504,7 +1504,7 @@ fn update_backtracking_ok() { .dep("cookie", "0.1.0") .publish(); - p.cargo("update -p hyper") + p.cargo("update hyper") .with_stderr( "\ [UPDATING] `[..]` index @@ -1555,7 +1555,7 @@ fn update_multiple_packages() { Package::new("b", "0.1.1").publish(); Package::new("c", "0.1.1").publish(); - p.cargo("update -pa -pb") + p.cargo("update a b") .with_stderr( "\ [UPDATING] `[..]` index @@ -1565,7 +1565,7 @@ fn update_multiple_packages() { ) .run(); - p.cargo("update -pb -pc") + p.cargo("update b c") .with_stderr( "\ [UPDATING] `[..]` index @@ -1671,7 +1671,7 @@ fn update_same_prefix_oh_my_how_was_this_a_bug() { .publish(); p.cargo("generate-lockfile").run(); - p.cargo("update -pfoobar --precise=0.2.0").run(); + p.cargo("update foobar --precise=0.2.0").run(); } #[cargo_test] diff --git a/tests/testsuite/replace.rs b/tests/testsuite/replace.rs index c11c49330d9..b583de7b7d5 100644 --- a/tests/testsuite/replace.rs +++ b/tests/testsuite/replace.rs @@ -533,7 +533,7 @@ fn override_adds_some_deps() { p.cargo("check").with_stdout("").run(); Package::new("baz", "0.1.2").publish(); - p.cargo("update -p") + p.cargo("update") .arg(&format!("{}#bar", foo.url())) .with_stderr( "\ @@ -542,7 +542,7 @@ fn override_adds_some_deps() { ", ) .run(); - p.cargo("update -p https://github.com/rust-lang/crates.io-index#bar") + p.cargo("update https://github.com/rust-lang/crates.io-index#bar") .with_stderr( "\ [UPDATING] `dummy-registry` index diff --git a/tests/testsuite/rust_version.rs b/tests/testsuite/rust_version.rs index 6bfaebda3d4..4f65dcdc6c8 100644 --- a/tests/testsuite/rust_version.rs +++ b/tests/testsuite/rust_version.rs @@ -182,7 +182,7 @@ fn dependency_rust_version_newer_than_rustc() { error: package `bar v0.0.1` cannot be built because it requires \ rustc 1.2345.0 or newer, while the currently active rustc version is [..]\n\ Either upgrade to rustc 1.2345.0 or newer, or use\n\ - cargo update -p bar@0.0.1 --precise ver\n\ + cargo update bar@0.0.1 --precise ver\n\ where `ver` is the latest version of `bar` supporting rustc [..]", ) .run(); diff --git a/tests/testsuite/update.rs b/tests/testsuite/update.rs index d4234535578..d7d008b2393 100644 --- a/tests/testsuite/update.rs +++ b/tests/testsuite/update.rs @@ -105,7 +105,7 @@ fn transitive_minor_update() { // // Also note that this is probably counterintuitive and weird. We may wish // to change this one day. - p.cargo("update -p serde") + p.cargo("update serde") .with_stderr( "\ [UPDATING] `[..]` index @@ -155,7 +155,7 @@ fn conservative() { Package::new("log", "0.1.1").publish(); Package::new("serde", "0.1.1").dep("log", "0.1").publish(); - p.cargo("update -p serde") + p.cargo("update serde") .with_stderr( "\ [UPDATING] `[..]` index @@ -381,7 +381,7 @@ fn update_precise() { Package::new("serde", "0.2.0").publish(); - p.cargo("update -p serde:0.2.1 --precise 0.2.0") + p.cargo("update serde:0.2.1 --precise 0.2.0") .with_stderr( "\ [UPDATING] `[..]` index @@ -417,7 +417,7 @@ fn update_precise_do_not_force_update_deps() { Package::new("log", "0.1.1").publish(); Package::new("serde", "0.2.2").dep("log", "0.1").publish(); - p.cargo("update -p serde:0.2.1 --precise 0.2.2") + p.cargo("update serde:0.2.1 --precise 0.2.2") .with_stderr( "\ [UPDATING] `[..]` index @@ -453,7 +453,7 @@ fn update_aggressive() { Package::new("log", "0.1.1").publish(); Package::new("serde", "0.2.2").dep("log", "0.1").publish(); - p.cargo("update -p serde:0.2.1 --aggressive") + p.cargo("update serde:0.2.1 --aggressive") .with_stderr( "\ [UPDATING] `[..]` index @@ -490,13 +490,13 @@ fn update_aggressive_conflicts_with_precise() { Package::new("log", "0.1.1").publish(); Package::new("serde", "0.2.2").dep("log", "0.1").publish(); - p.cargo("update -p serde:0.2.1 --precise 0.2.2 --aggressive") + p.cargo("update serde:0.2.1 --precise 0.2.2 --aggressive") .with_status(1) .with_stderr( "\ error: the argument '--precise ' cannot be used with '--aggressive' -Usage: cargo[EXE] update --package [] --precise +Usage: cargo[EXE] update --precise ]> For more information, try '--help'. ", @@ -528,7 +528,7 @@ fn update_precise_first_run() { .file("src/lib.rs", "") .build(); - p.cargo("update -p serde --precise 0.2.0") + p.cargo("update serde --precise 0.2.0") .with_stderr( "\ [UPDATING] `[..]` index @@ -682,7 +682,7 @@ fn update_precise_first_run() { ) .run(); - p.cargo("update -p serde --precise 0.2.0") + p.cargo("update serde --precise 0.2.0") .with_stderr( "\ [UPDATING] `[..]` index @@ -758,7 +758,7 @@ fn dry_run_update() { Package::new("log", "0.1.1").publish(); Package::new("serde", "0.1.1").dep("log", "0.1").publish(); - p.cargo("update -p serde --dry-run") + p.cargo("update serde --dry-run") .with_stderr( "\ [UPDATING] `[..]` index @@ -818,7 +818,7 @@ fn precise_with_build_metadata() { Package::new("bar", "0.1.1+extra-stuff.1").publish(); Package::new("bar", "0.1.2+extra-stuff.2").publish(); - p.cargo("update -p bar --precise 0.1") + p.cargo("update bar --precise 0.1") .with_status(101) .with_stderr( "\ @@ -830,7 +830,7 @@ Caused by: ) .run(); - p.cargo("update -p bar --precise 0.1.1+does-not-match") + p.cargo("update bar --precise 0.1.1+does-not-match") .with_status(101) .with_stderr( "\ @@ -842,7 +842,7 @@ required by package `foo v0.1.0 ([ROOT]/foo)` ) .run(); - p.cargo("update -p bar --precise 0.1.1") + p.cargo("update bar --precise 0.1.1") .with_stderr( "\ [UPDATING] [..] index @@ -852,7 +852,7 @@ required by package `foo v0.1.0 ([ROOT]/foo)` .run(); Package::new("bar", "0.1.3").publish(); - p.cargo("update -p bar --precise 0.1.3+foo") + p.cargo("update bar --precise 0.1.3+foo") .with_status(101) .with_stderr( "\ @@ -864,7 +864,7 @@ required by package `foo v0.1.0 ([ROOT]/foo)` ) .run(); - p.cargo("update -p bar --precise 0.1.3") + p.cargo("update bar --precise 0.1.3") .with_stderr( "\ [UPDATING] [..] index From dc3722254d6c6ea36c6d0162f2957a580907a147 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 23 Aug 2023 14:44:13 -0500 Subject: [PATCH 2/2] fix(update): Improve error on bad argument order This is inspired by `cargo install` --- src/bin/cargo/commands/update.rs | 13 ++++++++++++- tests/testsuite/cargo_update/mod.rs | 1 + .../toolchain_pkgname/in/Cargo.toml | 5 +++++ .../toolchain_pkgname/in/src/main.rs | 3 +++ .../cargo_update/toolchain_pkgname/mod.rs | 19 +++++++++++++++++++ .../cargo_update/toolchain_pkgname/stderr.log | 2 ++ .../cargo_update/toolchain_pkgname/stdout.log | 0 7 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 tests/testsuite/cargo_update/toolchain_pkgname/in/Cargo.toml create mode 100644 tests/testsuite/cargo_update/toolchain_pkgname/in/src/main.rs create mode 100644 tests/testsuite/cargo_update/toolchain_pkgname/mod.rs create mode 100644 tests/testsuite/cargo_update/toolchain_pkgname/stderr.log create mode 100644 tests/testsuite/cargo_update/toolchain_pkgname/stdout.log diff --git a/src/bin/cargo/commands/update.rs b/src/bin/cargo/commands/update.rs index 0ceeb7071e0..4871b2cb430 100644 --- a/src/bin/cargo/commands/update.rs +++ b/src/bin/cargo/commands/update.rs @@ -1,5 +1,6 @@ use crate::command_prelude::*; +use anyhow::anyhow; use cargo::ops::{self, UpdateOptions}; use cargo::util::print_available_packages; @@ -58,11 +59,21 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { } else { "package2" }; + let to_update = values(args, to_update); + for crate_name in to_update.iter() { + if let Some(toolchain) = crate_name.strip_prefix("+") { + return Err(anyhow!( + "invalid character `+` in package name: `+{toolchain}` + Use `cargo +{toolchain} update` if you meant to use the `{toolchain}` toolchain." + ) + .into()); + } + } let update_opts = UpdateOptions { aggressive: args.flag("aggressive"), precise: args.get_one::("precise").map(String::as_str), - to_update: values(args, to_update), + to_update, dry_run: args.dry_run(), workspace: args.flag("workspace"), config, diff --git a/tests/testsuite/cargo_update/mod.rs b/tests/testsuite/cargo_update/mod.rs index c0ce1118071..a809d9fe951 100644 --- a/tests/testsuite/cargo_update/mod.rs +++ b/tests/testsuite/cargo_update/mod.rs @@ -1 +1,2 @@ mod help; +mod toolchain_pkgname; diff --git a/tests/testsuite/cargo_update/toolchain_pkgname/in/Cargo.toml b/tests/testsuite/cargo_update/toolchain_pkgname/in/Cargo.toml new file mode 100644 index 00000000000..c844631b2e8 --- /dev/null +++ b/tests/testsuite/cargo_update/toolchain_pkgname/in/Cargo.toml @@ -0,0 +1,5 @@ +[package] +name = "test" +version = "0.1.0" + +[dependencies] diff --git a/tests/testsuite/cargo_update/toolchain_pkgname/in/src/main.rs b/tests/testsuite/cargo_update/toolchain_pkgname/in/src/main.rs new file mode 100644 index 00000000000..e7a11a969c0 --- /dev/null +++ b/tests/testsuite/cargo_update/toolchain_pkgname/in/src/main.rs @@ -0,0 +1,3 @@ +fn main() { + println!("Hello, world!"); +} diff --git a/tests/testsuite/cargo_update/toolchain_pkgname/mod.rs b/tests/testsuite/cargo_update/toolchain_pkgname/mod.rs new file mode 100644 index 00000000000..f1488b90d16 --- /dev/null +++ b/tests/testsuite/cargo_update/toolchain_pkgname/mod.rs @@ -0,0 +1,19 @@ +use cargo_test_support::curr_dir; +use cargo_test_support::prelude::*; +use cargo_test_support::Project; + +#[cargo_test] +fn case() { + let project = Project::from_template(curr_dir!().join("in")); + let project_root = project.root(); + let cwd = &project_root; + + snapbox::cmd::Command::cargo_ui() + .arg("update") + .arg("+stable") + .current_dir(cwd) + .assert() + .code(101) + .stdout_matches_path(curr_dir!().join("stdout.log")) + .stderr_matches_path(curr_dir!().join("stderr.log")); +} diff --git a/tests/testsuite/cargo_update/toolchain_pkgname/stderr.log b/tests/testsuite/cargo_update/toolchain_pkgname/stderr.log new file mode 100644 index 00000000000..7e5870c540a --- /dev/null +++ b/tests/testsuite/cargo_update/toolchain_pkgname/stderr.log @@ -0,0 +1,2 @@ +error: invalid character `+` in package name: `+stable` + Use `cargo +stable update` if you meant to use the `stable` toolchain. diff --git a/tests/testsuite/cargo_update/toolchain_pkgname/stdout.log b/tests/testsuite/cargo_update/toolchain_pkgname/stdout.log new file mode 100644 index 00000000000..e69de29bb2d