Skip to content

Commit

Permalink
Auto merge of #10597 - epage:yank, r=ehuss
Browse files Browse the repository at this point in the history
feat(yank): Support foo@version like cargo-add

### What does this PR try to resolve?

In #10472, cargo-add was merged with support for an inline version
syntax of `cargo add foo@version`.  That also served as the change proposal for
extending that syntax to `cargo yank` for convenience and consistency.

### How should we test and review this PR?

Documentation updates are split into their own commit to not clog up browsing the code.

The ops API is generic enough that this is implemented purely in the command.

For now, the `foo@version` syntax parser is being left in the command, rather than being shared, as we see how the behavior of these different parsers diverge for their target needs to see what makes sense for refactoring.  See also The Rule of Three
- This doesn't use the full `pkgid` syntax (modified in #10582) because that allows syntax that is unsupported here.
- This doesn't use `cargo-add`s parser because that is for version reqs.

Tests were added for various combinations of flags and behavior.

### Additional information

The major difference is that `cargo-add` is specifying a version-req
while `cargo-yank` is specifying a version.  This was originally discussed on [zulip](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Multiple.20ways.20of.20specifying.20versions) and there seemed to be a desire to have one syntax rather than the user thinking about a syntax per type of version (which users won't always think about).  See also #10582 which extended the pkgid spec syntax and has some more discussion on this general trend.

`cargo-install` will be updated in a subsequent PR.
  • Loading branch information
bors committed May 10, 2022
2 parents f643459 + 790e4ce commit dd53196
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 9 deletions.
30 changes: 26 additions & 4 deletions src/bin/cargo/commands/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ pub fn cli() -> App {
.arg(
opt("version", "The version to yank or un-yank")
.alias("vers")
.value_name("VERSION")
.required(true),
.value_name("VERSION"),
)
.arg(opt(
"undo",
Expand All @@ -28,14 +27,37 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {

let registry = args.registry(config)?;

let (krate, version) = resolve_crate(args.value_of("crate"), args.value_of("version"))?;
if version.is_none() {
return Err(anyhow::format_err!("`--version` is required").into());
}

ops::yank(
config,
args.value_of("crate").map(|s| s.to_string()),
args.value_of("version").map(|s| s.to_string()),
krate.map(|s| s.to_string()),
version.map(|s| s.to_string()),
args.value_of("token").map(|s| s.to_string()),
args.value_of("index").map(|s| s.to_string()),
args.is_present("undo"),
registry,
)?;
Ok(())
}

fn resolve_crate<'k>(
mut krate: Option<&'k str>,
mut version: Option<&'k str>,
) -> crate::CargoResult<(Option<&'k str>, Option<&'k str>)> {
if let Some((k, v)) = krate.and_then(|k| k.split_once('@')) {
if version.is_some() {
anyhow::bail!("cannot specify both `@{v}` and `--version`");
}
if k.is_empty() {
// by convention, arguments starting with `@` are response files
anyhow::bail!("missing crate name for `@{v}`");
}
krate = Some(k);
version = Some(v);
}
Ok((krate, version))
}
3 changes: 2 additions & 1 deletion src/doc/man/cargo-yank.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ cargo-yank - Remove a pushed crate from the index

## SYNOPSIS

`cargo yank` [_options_] _crate_@_version_\
`cargo yank` [_options_] `--version` _version_ [_crate_]

## DESCRIPTION
Expand Down Expand Up @@ -64,7 +65,7 @@ Undo a yank, putting a version back into the index.

1. Yank a crate from the index:

cargo yank --version 1.0.7 foo
cargo yank foo@1.0.7

## SEE ALSO
{{man "cargo" 1}}, {{man "cargo-login" 1}}, {{man "cargo-publish" 1}}
3 changes: 2 additions & 1 deletion src/doc/man/generated_txt/cargo-yank.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ NAME
cargo-yank - Remove a pushed crate from the index

SYNOPSIS
cargo yank [options] crate@version
cargo yank [options] --version version [crate]

DESCRIPTION
Expand Down Expand Up @@ -104,7 +105,7 @@ EXIT STATUS
EXAMPLES
1. Yank a crate from the index:

cargo yank --version 1.0.7 foo
cargo yank foo@1.0.7

SEE ALSO
cargo(1), cargo-login(1), cargo-publish(1)
Expand Down
3 changes: 2 additions & 1 deletion src/doc/src/commands/cargo-yank.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ cargo-yank - Remove a pushed crate from the index

## SYNOPSIS

`cargo yank` [_options_] _crate_@_version_\
`cargo yank` [_options_] `--version` _version_ [_crate_]

## DESCRIPTION
Expand Down Expand Up @@ -140,7 +141,7 @@ details on environment variables that Cargo reads.

1. Yank a crate from the index:

cargo yank --version 1.0.7 foo
cargo yank foo@1.0.7

## SEE ALSO
[cargo(1)](cargo.html), [cargo-login(1)](cargo-login.html), [cargo-publish(1)](cargo-publish.html)
4 changes: 3 additions & 1 deletion src/etc/man/cargo-yank.1
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
.SH "NAME"
cargo\-yank \- Remove a pushed crate from the index
.SH "SYNOPSIS"
\fBcargo yank\fR [\fIoptions\fR] \fIcrate\fR@\fIversion\fR
.br
\fBcargo yank\fR [\fIoptions\fR] \fB\-\-version\fR \fIversion\fR [\fIcrate\fR]
.SH "DESCRIPTION"
The yank command removes a previously published crate's version from the
Expand Down Expand Up @@ -139,7 +141,7 @@ details on environment variables that Cargo reads.
.sp
.RS 4
.nf
cargo yank \-\-version 1.0.7 foo
cargo yank foo@1.0.7
.fi
.RE
.RE
Expand Down
115 changes: 114 additions & 1 deletion tests/testsuite/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn setup(name: &str, version: &str) {
}

#[cargo_test]
fn simple() {
fn explicit_version() {
registry::init();
setup("foo", "0.0.1");

Expand Down Expand Up @@ -46,3 +46,116 @@ Caused by:
)
.run();
}

#[cargo_test]
fn inline_version() {
registry::init();
setup("foo", "0.0.1");

let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
authors = []
license = "MIT"
description = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("yank foo@0.0.1 --token sekrit").run();

p.cargo("yank --undo foo@0.0.1 --token sekrit")
.with_status(101)
.with_stderr(
" Updating `[..]` index
Unyank foo@0.0.1
error: failed to undo a yank from the registry at file:///[..]
Caused by:
EOF while parsing a value at line 1 column 0",
)
.run();
}

#[cargo_test]
fn version_required() {
registry::init();
setup("foo", "0.0.1");

let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
authors = []
license = "MIT"
description = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("yank foo --token sekrit")
.with_status(101)
.with_stderr("error: `--version` is required")
.run();
}

#[cargo_test]
fn inline_version_without_name() {
registry::init();
setup("foo", "0.0.1");

let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
authors = []
license = "MIT"
description = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("yank @0.0.1 --token sekrit")
.with_status(101)
.with_stderr("error: missing crate name for `@0.0.1`")
.run();
}

#[cargo_test]
fn inline_and_explicit_version() {
registry::init();
setup("foo", "0.0.1");

let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
authors = []
license = "MIT"
description = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("yank foo@0.0.1 --version 0.0.1 --token sekrit")
.with_status(101)
.with_stderr("error: cannot specify both `@0.0.1` and `--version`")
.run();
}

0 comments on commit dd53196

Please sign in to comment.