From 735f2c8611511b93758a26a9d1a9cd3073a46135 Mon Sep 17 00:00:00 2001 From: bors Date: Fri, 27 May 2022 00:43:33 +0000 Subject: [PATCH] Auto merge of #10677 - likzn:fix_publish_p, r=ehuss fix(publish): add more check when use `publish -p ` ### Main issue As issue say #10536 , we need add more check when user use `cargo publish -p ` >`@ehuss` point outs: >From a behavior standpoint, here are some things to check: > - In the root of a virtual workspace, it should be an error to run without -p. >- It should be an error to pass -p for a non-workspace member. >- It should be an error for -p to match multiple packages. >- When using -p, it should publish that package, not the one in the current directory (which can be different). --- src/cargo/ops/cargo_compile.rs | 4 +- src/cargo/ops/registry.rs | 18 ++ tests/testsuite/publish.rs | 344 ++++++++++++++++++++++++++++++--- 3 files changed, 341 insertions(+), 25 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 07dabce6d08..5d273295ae3 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -174,13 +174,13 @@ impl Packages { }; if specs.is_empty() { if ws.is_virtual() { - anyhow::bail!( + bail!( "manifest path `{}` contains no package: The manifest is virtual, \ and the workspace has no members.", ws.root().display() ) } - anyhow::bail!("no packages to compile") + bail!("no packages to compile") } Ok(specs) } diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 68f19392e0c..5bd8decb1b8 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -23,6 +23,7 @@ use crate::core::resolver::CliFeatures; use crate::core::source::Source; use crate::core::{Package, SourceId, Workspace}; use crate::ops; +use crate::ops::Packages; use crate::sources::{RegistrySource, SourceConfigMap, CRATES_IO_DOMAIN, CRATES_IO_REGISTRY}; use crate::util::config::{self, Config, SslVersionConfig, SslVersionConfigRange}; use crate::util::errors::CargoResult; @@ -90,7 +91,24 @@ pub struct PublishOpts<'cfg> { pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { let specs = opts.to_publish.to_package_id_specs(ws)?; + if specs.len() > 1 { + bail!("the `-p` argument must be specified to select a single package to publish") + } + if Packages::Default == opts.to_publish && ws.is_virtual() { + bail!("the `-p` argument must be specified in the root of a virtual workspace") + } + let member_ids = ws.members().map(|p| p.package_id()); + // Check that the spec matches exactly one member. + specs[0].query(member_ids)?; let mut pkgs = ws.members_with_features(&specs, &opts.cli_features)?; + // In `members_with_features_old`, it will add "current" package (determined by the cwd) + // So we need filter + pkgs = pkgs + .into_iter() + .filter(|(m, _)| specs.iter().any(|spec| spec.matches(m.package_id()))) + .collect(); + // Double check. It is safe theoretically, unless logic has updated. + assert_eq!(pkgs.len(), 1); let (pkg, cli_features) = pkgs.pop().unwrap(); diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 88f9fee8e86..1977aba7007 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -56,7 +56,7 @@ fn validate_upload_foo() { ); } -fn validate_upload_bar() { +fn validate_upload_li() { publish::validate_upload( r#" { @@ -64,7 +64,7 @@ fn validate_upload_bar() { "badges": {}, "categories": [], "deps": [], - "description": "bar", + "description": "li", "documentation": null, "features": {}, "homepage": null, @@ -72,14 +72,14 @@ fn validate_upload_bar() { "license": "MIT", "license_file": null, "links": null, - "name": "bar", + "name": "li", "readme": null, "readme_file": null, "repository": null, "vers": "0.0.1" } "#, - "bar-0.0.1.crate", + "li-0.0.1.crate", &["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"], ); } @@ -1665,15 +1665,154 @@ Caused by: } #[cargo_test] -fn in_workspace() { +fn in_package_workspace() { registry::init(); let p = project() .file( "Cargo.toml", r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2021" [workspace] - members = ["foo", "bar"] + members = ["li"] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "li/Cargo.toml", + r#" + [package] + name = "li" + version = "0.0.1" + description = "li" + license = "MIT" + "#, + ) + .file("li/src/main.rs", "fn main() {}") + .build(); + + p.cargo("publish -p li --no-verify --token sekrit") + .with_stderr( + "\ +[UPDATING] [..] +[WARNING] manifest has no documentation, homepage or repository. +See [..] +[PACKAGING] li v0.0.1 ([CWD]/li) +[UPLOADING] li v0.0.1 ([CWD]/li) +", + ) + .run(); + + validate_upload_li(); +} + +#[cargo_test] +fn with_duplicate_spec_in_members() { + registry::init(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + [workspace] + resolver = "2" + members = ["li","bar"] + default-members = ["li","bar"] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "li/Cargo.toml", + r#" + [package] + name = "li" + version = "0.0.1" + description = "li" + license = "MIT" + "#, + ) + .file("li/src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.0.1" + description = "bar" + license = "MIT" + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("publish --no-verify --token sekrit") + .with_status(101) + .with_stderr( + "error: the `-p` argument must be specified to select a single package to publish", + ) + .run(); +} + +#[cargo_test] +fn in_package_workspace_with_members_with_features_old() { + registry::init(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + [workspace] + members = ["li"] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "li/Cargo.toml", + r#" + [package] + name = "li" + version = "0.0.1" + description = "li" + license = "MIT" + "#, + ) + .file("li/src/main.rs", "fn main() {}") + .build(); + + p.cargo("publish -p li --no-verify --token sekrit") + .with_stderr( + "\ +[UPDATING] [..] +[WARNING] manifest has no documentation, homepage or repository. +See [..] +[PACKAGING] li v0.0.1 ([CWD]/li) +[UPLOADING] li v0.0.1 ([CWD]/li) +", + ) + .run(); + + validate_upload_li(); +} + +#[cargo_test] +fn in_virtual_workspace() { + registry::init(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["foo"] "#, ) .file( @@ -1688,46 +1827,205 @@ fn in_workspace() { "#, ) .file("foo/src/main.rs", "fn main() {}") + .build(); + + p.cargo("publish --no-verify --token sekrit") + .with_status(101) + .with_stderr( + "error: the `-p` argument must be specified in the root of a virtual workspace", + ) + .run(); +} + +#[cargo_test] +fn in_virtual_workspace_with_p() { + registry::init(); + + let p = project() .file( - "bar/Cargo.toml", + "Cargo.toml", + r#" + [workspace] + members = ["foo","li"] + "#, + ) + .file( + "foo/Cargo.toml", r#" [project] - name = "bar" + name = "foo" version = "0.0.1" authors = [] license = "MIT" - description = "bar" - workspace = ".." + description = "foo" "#, ) - .file("bar/src/main.rs", "fn main() {}") + .file("foo/src/main.rs", "fn main() {}") + .file( + "li/Cargo.toml", + r#" + [package] + name = "li" + version = "0.0.1" + description = "li" + license = "MIT" + "#, + ) + .file("li/src/main.rs", "fn main() {}") .build(); - p.cargo("publish --no-verify --token sekrit -p foo") + p.cargo("publish -p li --no-verify --token sekrit") .with_stderr( "\ [UPDATING] [..] -[WARNING] manifest has no documentation, [..] +[WARNING] manifest has no documentation, homepage or repository. See [..] -[PACKAGING] foo v0.0.1 ([CWD]/foo) -[UPLOADING] foo v0.0.1 ([CWD]/foo) +[PACKAGING] li v0.0.1 ([CWD]/li) +[UPLOADING] li v0.0.1 ([CWD]/li) ", ) .run(); +} - validate_upload_foo(); +#[cargo_test] +fn in_package_workspace_not_found() { + registry::init(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2021" + [workspace] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "li/Cargo.toml", + r#" + [package] + name = "li" + version = "0.0.1" + edition = "2021" + authors = [] + license = "MIT" + description = "li" + "#, + ) + .file("li/src/main.rs", "fn main() {}") + .build(); - p.cargo("publish --no-verify --token sekrit -p bar") + p.cargo("publish -p li --no-verify --token sekrit ") + .with_status(101) .with_stderr( "\ -[UPDATING] [..] -[WARNING] manifest has no documentation, [..] -See [..] -[PACKAGING] bar v0.0.1 ([CWD]/bar) -[UPLOADING] bar v0.0.1 ([CWD]/bar) +error: package ID specification `li` did not match any packages + +Did you mean `foo`? +", + ) + .run(); +} + +#[cargo_test] +fn in_package_workspace_found_multiple() { + registry::init(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2021" + [workspace] + members = ["li","lii"] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "li/Cargo.toml", + r#" + [package] + name = "li" + version = "0.0.1" + edition = "2021" + authors = [] + license = "MIT" + description = "li" + "#, + ) + .file("li/src/main.rs", "fn main() {}") + .file( + "lii/Cargo.toml", + r#" + [package] + name = "lii" + version = "0.0.1" + edition = "2021" + authors = [] + license = "MIT" + description = "lii" + "#, + ) + .file("lii/src/main.rs", "fn main() {}") + .build(); + + p.cargo("publish -p li* --no-verify --token sekrit ") + .with_status(101) + .with_stderr( + "\ +error: the `-p` argument must be specified to select a single package to publish ", ) .run(); +} + +#[cargo_test] +// https://github.com/rust-lang/cargo/issues/10536 +fn publish_path_dependency_without_workspace() { + registry::init(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2021" + [dependencies.bar] + path = "bar" + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.0.1" + edition = "2021" + authors = [] + license = "MIT" + description = "bar" + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); - validate_upload_bar(); + p.cargo("publish -p bar --no-verify --token sekrit ") + .with_status(101) + .with_stderr( + "\ +error: package ID specification `bar` did not match any packages + +Did you mean `foo`? +", + ) + .run(); }