-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: skip publish=false
pkg when publishing entire workspace
#15525
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
tests/testsuite/publish.rs
Outdated
@@ -4103,10 +4103,183 @@ fn multiple_unpublishable_package() { | |||
|
|||
p.cargo("publish -Zpackage-workspace") | |||
.masquerade_as_nightly_cargo(&["package-workspace"]) | |||
.replace_crates_io(registry.index_url()) | |||
.with_stderr_data(str![[r#" | |||
[WARNING] nothing to publish, but found 2 unpublishable packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test isn't serving is no longer serving its purpose. We should add a publishable package to it and add a new test for "all are unpublishable".
tests/testsuite/publish.rs
Outdated
@@ -4103,10 +4103,183 @@ fn multiple_unpublishable_package() { | |||
|
|||
p.cargo("publish -Zpackage-workspace") | |||
.masquerade_as_nightly_cargo(&["package-workspace"]) | |||
.replace_crates_io(registry.index_url()) | |||
.with_stderr_data(str![[r#" | |||
[WARNING] nothing to publish, but found 2 unpublishable packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a new scenario that nothing is going to publish.
I went with a warning instead of 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.
What happens if I do cargo check --bins
and none are built due to required-features
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: target filter `bins` specified, but no targets matched; this is a no-op
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s
@@ -705,19 +745,6 @@ fn package_list(pkgs: impl IntoIterator<Item = PackageId>, final_sep: &str) -> S | |||
} | |||
|
|||
fn validate_registry(pkgs: &[&Package], reg_or_index: Option<&RegistryOrIndex>) -> CargoResult<()> { | |||
let unpublishable = pkgs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit, feel free to pass on. I'm mentioning this only since I had other comments.
Would make reviewing easier if ĵust moving this error check was a separate refactor commit.
Is there a reason this isn't marked as fixing #15006? |
For `-Zpackage-workspace`.
c8ee810
to
c364369
Compare
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 rust-lang#15006 (comment)
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.
c364369
to
090dcfe
Compare
All review comments were addressed.
I forgot 😬. |
Thanks! |
Update cargo 5 commits in 056f5f4f3c100cb36b5e9aed2d20b9ea70aae295..47c911e9e6f6461f90ce19142031fe16876a3b95 2025-05-09 14:54:18 +0000 to 2025-05-14 17:53:17 +0000 - Stabilize doctest-xcompile (rust-lang/cargo#15462) - feat: skip `publish=false` pkg when publishing entire workspace (rust-lang/cargo#15525) - chore: bump to 0.90.0; update changelog (rust-lang/cargo#15520) - chore(triagebot): add `[no-mentions]` and `[note]` (rust-lang/cargo#15517) - add glob pattern support for known_hosts (rust-lang/cargo#15508) r? ghost
Update cargo 5 commits in 056f5f4f3c100cb36b5e9aed2d20b9ea70aae295..47c911e9e6f6461f90ce19142031fe16876a3b95 2025-05-09 14:54:18 +0000 to 2025-05-14 17:53:17 +0000 - Stabilize doctest-xcompile (rust-lang/cargo#15462) - feat: skip `publish=false` pkg when publishing entire workspace (rust-lang/cargo#15525) - chore: bump to 0.90.0; update changelog (rust-lang/cargo#15520) - chore(triagebot): add `[no-mentions]` and `[note]` (rust-lang/cargo#15517) - add glob pattern support for known_hosts (rust-lang/cargo#15508) r? ghost
What does this PR try to resolve?
Fixes #15006
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 apackage.publish = false
package): errorcargo publish -p publishable -p unpublishable
: errorcargo publish --workspace
: skips `package.publish = falseSee #15006 (comment)
How should we test and review this PR?
workspace_flag_with_unpublishable_packages
was added to ensure--workspace
work with non-virtual workspace.unpublishable_package_as_versioned_dev_dep
was added to ensure versioned dev-dependencies won't be skipped and still fail cargo-publish, as they are required to be published.There is a new scenario that nothing is going to publish.
I went with a warning instead of 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.