Skip to content

feat(yank): Support foo@version like cargo-add #10597

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

Merged
merged 2 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove the .map(|s| s.to_string()) for all these arguments and make ops::yank take Option<&str> instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo

  • this is out of scope for this PR
  • the use of String is wider than just yank, its most Op options (though sometimes we do see a &str or a mix of String and &str)
  • I personally find this a point I don't have a strong preference on besides consistency. String is slightly more ergonomic though that is diminished by having a &Config in the Options and no one will ever notice the performance difference in these (thinking back to the RustConf talk about "just clone and stop worrying about it).

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 [email protected] --token sekrit").run();

p.cargo("yank --undo [email protected] --token sekrit")
.with_status(101)
.with_stderr(
" Updating `[..]` index
Unyank [email protected]
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 [email protected] --version 0.0.1 --token sekrit")
.with_status(101)
.with_stderr("error: cannot specify both `@0.0.1` and `--version`")
.run();
}