From 2421496c924560f2563e48622def2b2132d8512f Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 13 May 2025 22:01:17 -0400 Subject: [PATCH 1/4] test(publish): prepare tests for skipping unpublishable For `-Zpackage-workspace`. --- tests/testsuite/publish.rs | 232 ++++++++++++++++++++++++++++++++++--- 1 file changed, 218 insertions(+), 14 deletions(-) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index e014a3436e6..8a2c8838f4c 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -3990,11 +3990,7 @@ You may press ctrl-c to skip waiting; the crate should be available shortly. #[cargo_test] fn one_unpublishable_package() { - let _alt_reg = registry::RegistryBuilder::new() - .http_api() - .http_index() - .alternative() - .build(); + let registry = RegistryBuilder::new().http_api().http_index().build(); let p = project() .file( @@ -4018,7 +4014,7 @@ fn one_unpublishable_package() { publish = false [dependencies] - dep = { path = "../dep", version = "0.1.0", registry = "alternative" } + dep = { path = "../dep", version = "0.1.0" } "#, ) .file("main/src/main.rs", "fn main() {}") @@ -4033,7 +4029,6 @@ fn one_unpublishable_package() { license = "MIT" description = "dep" repository = "bar" - publish = ["alternative"] "#, ) .file("dep/src/lib.rs", "") @@ -4041,6 +4036,7 @@ fn one_unpublishable_package() { p.cargo("publish -Zpackage-workspace") .masquerade_as_nightly_cargo(&["package-workspace"]) + .replace_crates_io(registry.index_url()) .with_status(101) .with_stderr_data(str![[r#" [ERROR] `main` cannot be published. @@ -4051,19 +4047,15 @@ fn one_unpublishable_package() { } #[cargo_test] -fn multiple_unpublishable_package() { - let _alt_reg = registry::RegistryBuilder::new() - .http_api() - .http_index() - .alternative() - .build(); +fn virtual_ws_with_multiple_unpublishable_package() { + let registry = RegistryBuilder::new().http_api().http_index().build(); let p = project() .file( "Cargo.toml", r#" [workspace] - members = ["dep", "main"] + members = ["dep", "main", "publishable"] "#, ) .file( @@ -4099,10 +4091,24 @@ fn multiple_unpublishable_package() { "#, ) .file("dep/src/lib.rs", "") + .file( + "publishable/Cargo.toml", + r#" + [package] + name = "publishable" + version = "0.1.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + "#, + ) + .file("publishable/src/lib.rs", "") .build(); p.cargo("publish -Zpackage-workspace") .masquerade_as_nightly_cargo(&["package-workspace"]) + .replace_crates_io(registry.index_url()) .with_status(101) .with_stderr_data(str![[r#" [ERROR] `dep`, `main` cannot be published. @@ -4111,3 +4117,201 @@ fn multiple_unpublishable_package() { "#]]) .run(); } + +#[cargo_test] +fn workspace_flag_with_unpublishable_packages() { + let registry = RegistryBuilder::new().http_api().http_index().build(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["publishable", "non-publishable"] + + [package] + name = "cwd" + version = "0.0.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + publish = false + "#, + ) + .file("src/lib.rs", "") + .file( + "publishable/Cargo.toml", + r#" + [package] + name = "publishable" + version = "0.0.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + publish = true + "#, + ) + .file("publishable/src/lib.rs", "") + .file( + "non-publishable/Cargo.toml", + r#" + [package] + name = "non-publishable" + version = "0.0.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + publish = false + "#, + ) + .file("non-publishable/src/lib.rs", "") + .build(); + + p.cargo("publish --workspace -Zpackage-workspace") + .masquerade_as_nightly_cargo(&["package-workspace"]) + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] `non-publishable`, `cwd` cannot be published. +`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. + +"#]]) + .run(); +} + +#[cargo_test] +fn unpublishable_package_as_versioned_dev_dep() { + let registry = RegistryBuilder::new().http_api().http_index().build(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["dep", "main"] + "#, + ) + .file( + "main/Cargo.toml", + r#" + [package] + name = "main" + version = "0.0.1" + edition = "2015" + license = "MIT" + description = "main" + repository = "bar" + + [dev-dependencies] + dep = { path = "../dep", version = "0.1.0" } + "#, + ) + .file("main/src/main.rs", "fn main() {}") + .file( + "dep/Cargo.toml", + r#" + [package] + name = "dep" + version = "0.1.0" + edition = "2015" + license = "MIT" + description = "dep" + repository = "bar" + publish = false + "#, + ) + .file("dep/src/lib.rs", "") + .build(); + + // It is expected to find the versioned dev dep not being published, + // regardless with `--dry-run` or `--no-verify`. + p.cargo("publish -Zpackage-workspace") + .masquerade_as_nightly_cargo(&["package-workspace"]) + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] `dep` cannot be published. +`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. + +"#]]) + .run(); + + p.cargo("publish -Zpackage-workspace --dry-run") + .masquerade_as_nightly_cargo(&["package-workspace"]) + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] `dep` cannot be published. +`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. + +"#]]) + .run(); + + p.cargo("publish -Zpackage-workspace --no-verify") + .masquerade_as_nightly_cargo(&["package-workspace"]) + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] `dep` cannot be published. +`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. + +"#]]) + .run(); +} + +#[cargo_test] +fn all_unpublishable_packages() { + let registry = RegistryBuilder::new().http_api().http_index().build(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["non-publishable1", "non-publishable2"] + "#, + ) + .file( + "non-publishable1/Cargo.toml", + r#" + [package] + name = "non-publishable1" + version = "0.0.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + publish = false + "#, + ) + .file("non-publishable1/src/lib.rs", "") + .file( + "non-publishable2/Cargo.toml", + r#" + [package] + name = "non-publishable2" + version = "0.0.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + publish = false + "#, + ) + .file("non-publishable2/src/lib.rs", "") + .build(); + + p.cargo("publish --workspace -Zpackage-workspace") + .masquerade_as_nightly_cargo(&["package-workspace"]) + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] `non-publishable1`, `non-publishable2` cannot be published. +`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. + +"#]]) + .run(); +} From 6f8136488e389749eba43c7f284be8164452e35b Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 14 May 2025 01:21:10 -0400 Subject: [PATCH 2/4] refactor(publish): extract "no publishable found" error --- src/cargo/ops/registry/publish.rs | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 85cb6118023..80ee3a42f3d 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -93,6 +93,20 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { .filter(|(m, _)| specs.iter().any(|spec| spec.matches(m.package_id()))) .collect(); + let (unpublishable, pkgs): (Vec<_>, Vec<_>) = pkgs + .into_iter() + .partition(|(pkg, _)| pkg.publish() == &Some(vec![])); + if !unpublishable.is_empty() { + bail!( + "{} cannot be published.\n\ + `package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish.", + unpublishable + .iter() + .map(|(pkg, _)| format!("`{}`", pkg.name())) + .join(", "), + ); + } + let just_pkgs: Vec<_> = pkgs.iter().map(|p| p.0).collect(); let reg_or_index = match opts.reg_or_index.clone() { Some(r) => { @@ -705,19 +719,6 @@ fn package_list(pkgs: impl IntoIterator, final_sep: &str) -> S } fn validate_registry(pkgs: &[&Package], reg_or_index: Option<&RegistryOrIndex>) -> CargoResult<()> { - let unpublishable = pkgs - .iter() - .filter(|pkg| pkg.publish() == &Some(Vec::new())) - .map(|pkg| format!("`{}`", pkg.name())) - .collect::>(); - if !unpublishable.is_empty() { - bail!( - "{} cannot be published.\n\ - `package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish.", - unpublishable.join(", ") - ); - } - let reg_name = match reg_or_index { Some(RegistryOrIndex::Registry(r)) => Some(r.as_str()), None => Some(CRATES_IO_REGISTRY), From 4f7494c4ec5a8f7c2c47da67a23bb5c3507527af Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 13 May 2025 22:34:48 -0400 Subject: [PATCH 3/4] feat(publish): workspace publish skips `publish=false` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changes how cargo-publish works with the unstable `-Zpackage-workspace` feature. Before this, when publishing the entire workspace, like `cargo publish --workspace`, if there is a package with `package.pulibsh=false, it'll fail the entire publish process. After this, when `--workspace` is passed, or when publishing the virtual workspace, the intent is more like “publish all publishable in this workspace”, so skip `publish=false` packages and proceed to publish others. The new overall behavior looks like this: - `cargo publish` (inside a `package.publish = false` package): error - `cargo publish -p publishable -p unpublishable`: error - `cargo publish --workspace`: skips `package.publish = false See https://github.com/rust-lang/cargo/issues/15006#issuecomment-2847660911 --- src/cargo/ops/registry/publish.rs | 12 ++++- tests/testsuite/publish.rs | 79 ++++++++++++++++++++++++------- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 80ee3a42f3d..ea56a657493 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -96,7 +96,17 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { let (unpublishable, pkgs): (Vec<_>, Vec<_>) = pkgs .into_iter() .partition(|(pkg, _)| pkg.publish() == &Some(vec![])); - if !unpublishable.is_empty() { + // If `--workspace` is passed, + // the intent is more like "publish all publisable packages in this workspace", + // so skip `publish=false` packages. + let allow_unpublishable = multi_package_mode + && match &opts.to_publish { + Packages::Default => ws.is_virtual(), + Packages::All(_) => true, + Packages::OptOut(_) => true, + Packages::Packages(_) => false, + }; + if !unpublishable.is_empty() && !allow_unpublishable { bail!( "{} cannot be published.\n\ `package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish.", diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 8a2c8838f4c..8bf7cefe437 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -4037,10 +4037,18 @@ fn one_unpublishable_package() { p.cargo("publish -Zpackage-workspace") .masquerade_as_nightly_cargo(&["package-workspace"]) .replace_crates_io(registry.index_url()) - .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `main` cannot be published. -`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. +[UPDATING] crates.io index +[PACKAGING] dep v0.1.0 ([ROOT]/foo/dep) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[VERIFYING] dep v0.1.0 ([ROOT]/foo/dep) +[COMPILING] dep v0.1.0 ([ROOT]/foo/target/package/dep-0.1.0) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[UPLOADING] dep v0.1.0 ([ROOT]/foo/dep) +[UPLOADED] dep v0.1.0 to registry `crates-io` +[NOTE] waiting for `dep v0.1.0` to be available at registry `crates-io`. +You may press ctrl-c to skip waiting; the crate should be available shortly. +[PUBLISHED] dep v0.1.0 at registry `crates-io` "#]]) .run(); @@ -4109,10 +4117,18 @@ fn virtual_ws_with_multiple_unpublishable_package() { p.cargo("publish -Zpackage-workspace") .masquerade_as_nightly_cargo(&["package-workspace"]) .replace_crates_io(registry.index_url()) - .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `dep`, `main` cannot be published. -`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. +[UPDATING] crates.io index +[PACKAGING] publishable v0.1.0 ([ROOT]/foo/publishable) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[VERIFYING] publishable v0.1.0 ([ROOT]/foo/publishable) +[COMPILING] publishable v0.1.0 ([ROOT]/foo/target/package/publishable-0.1.0) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[UPLOADING] publishable v0.1.0 ([ROOT]/foo/publishable) +[UPLOADED] publishable v0.1.0 to registry `crates-io` +[NOTE] waiting for `publishable v0.1.0` to be available at registry `crates-io`. +You may press ctrl-c to skip waiting; the crate should be available shortly. +[PUBLISHED] publishable v0.1.0 at registry `crates-io` "#]]) .run(); @@ -4173,10 +4189,18 @@ fn workspace_flag_with_unpublishable_packages() { p.cargo("publish --workspace -Zpackage-workspace") .masquerade_as_nightly_cargo(&["package-workspace"]) .replace_crates_io(registry.index_url()) - .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `non-publishable`, `cwd` cannot be published. -`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. +[UPDATING] crates.io index +[PACKAGING] publishable v0.0.0 ([ROOT]/foo/publishable) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[VERIFYING] publishable v0.0.0 ([ROOT]/foo/publishable) +[COMPILING] publishable v0.0.0 ([ROOT]/foo/target/package/publishable-0.0.0) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[UPLOADING] publishable v0.0.0 ([ROOT]/foo/publishable) +[UPLOADED] publishable v0.0.0 to registry `crates-io` +[NOTE] waiting for `publishable v0.0.0` to be available at registry `crates-io`. +You may press ctrl-c to skip waiting; the crate should be available shortly. +[PUBLISHED] publishable v0.0.0 at registry `crates-io` "#]]) .run(); @@ -4233,8 +4257,15 @@ fn unpublishable_package_as_versioned_dev_dep() { .replace_crates_io(registry.index_url()) .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `dep` cannot be published. -`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. +[UPDATING] crates.io index +[PACKAGING] main v0.0.1 ([ROOT]/foo/main) +[UPDATING] crates.io index +[ERROR] failed to prepare local package for uploading + +Caused by: + no matching package named `dep` found + location searched: crates.io index + required by package `main v0.0.1 ([ROOT]/foo/main)` "#]]) .run(); @@ -4244,8 +4275,15 @@ fn unpublishable_package_as_versioned_dev_dep() { .replace_crates_io(registry.index_url()) .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `dep` cannot be published. -`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. +[UPDATING] crates.io index +[PACKAGING] main v0.0.1 ([ROOT]/foo/main) +[UPDATING] crates.io index +[ERROR] failed to prepare local package for uploading + +Caused by: + no matching package named `dep` found + location searched: crates.io index + required by package `main v0.0.1 ([ROOT]/foo/main)` "#]]) .run(); @@ -4255,8 +4293,15 @@ fn unpublishable_package_as_versioned_dev_dep() { .replace_crates_io(registry.index_url()) .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `dep` cannot be published. -`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. +[UPDATING] crates.io index +[PACKAGING] main v0.0.1 ([ROOT]/foo/main) +[UPDATING] crates.io index +[ERROR] failed to prepare local package for uploading + +Caused by: + no matching package named `dep` found + location searched: crates.io index + required by package `main v0.0.1 ([ROOT]/foo/main)` "#]]) .run(); @@ -4307,10 +4352,8 @@ fn all_unpublishable_packages() { p.cargo("publish --workspace -Zpackage-workspace") .masquerade_as_nightly_cargo(&["package-workspace"]) .replace_crates_io(registry.index_url()) - .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `non-publishable1`, `non-publishable2` cannot be published. -`package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish. +[UPDATING] crates.io index "#]]) .run(); From 090dcfe9af956ecf835db94e79eb2e3260263be8 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 13 May 2025 22:58:25 -0400 Subject: [PATCH 4/4] fix(publish): better message when nothing to publish This doesn't need to be an hard error because missing publish for the entire worspace should be a fairly visible. However, this is open for future to configure via Cargo lint system. --- src/cargo/ops/registry/publish.rs | 16 ++++++++++++++++ tests/testsuite/publish.rs | 3 ++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index ea56a657493..681cb88ce81 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -117,6 +117,22 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { ); } + if pkgs.is_empty() { + if allow_unpublishable { + let n = unpublishable.len(); + let plural = if n == 1 { "" } else { "s" }; + ws.gctx().shell().warn(format_args!( + "nothing to publish, but found {n} unpublishable package{plural}" + ))?; + ws.gctx().shell().note(format_args!( + "to publish packages, set `package.publish` to `true` or a non-empty list" + ))?; + return Ok(()); + } else { + unreachable!("must have at least one publishable package"); + } + } + let just_pkgs: Vec<_> = pkgs.iter().map(|p| p.0).collect(); let reg_or_index = match opts.reg_or_index.clone() { Some(r) => { diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 8bf7cefe437..29c1d288d15 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -4353,7 +4353,8 @@ fn all_unpublishable_packages() { .masquerade_as_nightly_cargo(&["package-workspace"]) .replace_crates_io(registry.index_url()) .with_stderr_data(str![[r#" -[UPDATING] crates.io index +[WARNING] nothing to publish, but found 2 unpublishable packages +[NOTE] to publish packages, set `package.publish` to `true` or a non-empty list "#]]) .run();