feat: skip publish=false pkg when publishing entire workspace#15525
Merged
epage merged 4 commits intorust-lang:masterfrom May 14, 2025
Merged
feat: skip publish=false pkg when publishing entire workspace#15525epage merged 4 commits intorust-lang:masterfrom
publish=false pkg when publishing entire workspace#15525epage merged 4 commits intorust-lang:masterfrom
Conversation
Collaborator
epage
reviewed
May 14, 2025
tests/testsuite/publish.rs
Outdated
| .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 |
Contributor
There was a problem hiding this comment.
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
| .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 |
Contributor
There was a problem hiding this comment.
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?
Member
Author
There was a problem hiding this comment.
warning: target filter `bins` specified, but no targets matched; this is a no-op
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s
| } | ||
|
|
||
| fn validate_registry(pkgs: &[&Package], reg_or_index: Option<&RegistryOrIndex>) -> CargoResult<()> { | ||
| let unpublishable = pkgs |
Contributor
There was a problem hiding this comment.
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.
Contributor
|
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
Member
Author
|
All review comments were addressed.
I forgot 😬. |
epage
approved these changes
May 14, 2025
Contributor
|
Thanks! |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 16, 2025
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
github-actions bot
pushed a commit
to rust-lang/rustc-dev-guide
that referenced
this pull request
May 19, 2025
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR try to resolve?
Fixes #15006
This changes how cargo-publish works with the unstable
-Zpackage-workspacefeature.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
--workspaceis passed,or when publishing the virtual workspace,
the intent is more like “publish all publishable in this workspace”,
so skip
publish=falsepackages and proceed to publish others.The new overall behavior looks like this:
cargo publish(inside apackage.publish = falsepackage): 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_packageswas added to ensure--workspacework with non-virtual workspace.unpublishable_package_as_versioned_dev_depwas 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.