Publish workspace #14433

jneem · 2024-08-21T04:22:14Z

Adds support for simultaneously publishing multiple (possibly inter-dependent) packages in a workspace, gated by the -Zpackage-workspace flag.

Questions to be worked out through stabilization:

Are we ok stabilizing this and cargo package --workspace is not very useful #10948 at the same time? Currently, they are behind the same flag
What is the desired behavior for the publish timeout? This PR uploads the crates in batches (depending on the dependency graph), and we only timeout if nothing in the batch is available within the timeout, deferring the rest to the next wait-for-publish. So for example, if you have packages a, b, c then we'll wait up to 60 seconds and if only a and b were ready in that time, we'll then wait another 60 seconds for c.
What is the desired behavior when some packages in a workspace have publish = false? This PR raises an error whenever any of the selected packages has publish = false, so it will error on cargo publish --workspace in a workspace with an unpublishable package. An alternative interface would implicitly exclude unpublishable packages in this case, but still error out if you explicitly select an unpublishable package with -p package-name (see -Zpackage-workspace is not smart about publish = false #14356). This PR's behavior is the most conservative one as it can change from an error to implicit excludes later.

This is part of #1169

rustbot · 2024-08-21T04:22:18Z

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

ehuss · 2024-08-27T18:08:39Z

Is it worth adding a new -Zpublish-workspace flag, or is it ok to reuse the old one?

I don't have enough context to know if -Zpackage-workspace was intended as a stepping-stone for publishing, or if there really is an independent use-case for packaging versus publishing.

This breaks the publish_namespaced test, because we now try to parse all packages as package specs, and "foo::bar" fails to parse. This could probably be avoided by a refactor (adding a different package function that takes a list of packages instead of trying to get them from cli arguments), but I also think it's a bug in the package spec parser. Thoughts?

Can the check here be moved to an earlier stage, like in ops::registry::publish::publish by iterating over the just_pkgs list and checking if any of them have the open-namespaces feature?

ehuss · 2024-08-27T18:09:36Z

I realistically won't have time to give this a good review for at least a few days if not a week or more. I was wondering if @weihanglo or @epage would have more bandwidth in the short term? If not, that's ok, I'll try to review this when I can.

epage · 2024-08-28T15:43:20Z

This breaks the publish_namespaced test, because we now try to parse all packages as package specs, and "foo::bar" fails to parse. This could probably be avoided by a refactor (adding a different package function that takes a list of packages instead of trying to get them from cli arguments), but I also think it's a bug in the package spec parser. Thoughts?

@jneem I posted #14467

fix(pkgid): Allow open namespaces in PackageIdSpec's ### What does this PR try to resolve? This is a part of #13576 This unblocks #14433. We have a test to ensure you can't publish a namespaced package and the error for that is getting masked in #14433 because the package name is getting parsed as a `PackageIdSpec` which wasn't supported until this PR. ### How should we test and review this PR? ### Additional information

epage · 2024-08-29T21:21:03Z

Could you rebase on top of #14467?

jneem · 2024-08-30T03:19:52Z

Rebased, thanks!

src/cargo/ops/cargo_package.rs

epage · 2024-08-30T20:57:30Z

Is it worth adding a new -Zpublish-workspace flag, or is it ok to reuse the old one?

I don't have enough context to know if -Zpackage-workspace was intended as a stepping-stone for publishing, or if there really is an independent use-case for packaging versus publishing.

-Zpackage-workspace is a stepping stone. In that spirit, it seems like the feature would be named -Zpublish-workspace but probably not worth changing at this point.

The questions relevant for this is would we want to stabilize these together? Are there design risks for one that by separating the stabilization of them would make it easier to stabilize the other?

tests/testsuite/publish.rs

src/cargo/ops/registry/publish.rs

jneem · 2024-09-05T04:47:37Z

But I don't think a straight dependency tree was the concern. Is my update accurate?

Yes, your update is accurate. I thought that a long straight dependency tree might also be a concern, because it effectively extends the timeout.

Document -Zpackage-workspace Adds some unstable documentation on the `-Zpackage-workspace` feature, as requested in [#10948](#10948 (comment)). This documentation assumes that #14433 gets merged.

epage · 2024-09-05T14:26:27Z

Yes, your update is accurate. I thought that a long straight dependency tree might also be a concern, because it effectively extends the timeout.

Not concerned about the straight line because the timeout is for publishing a package, not publishing everything, so its natural they get concatenated when doing them back-to-back.

Co-authored-by: Tor Hovland <55164+torhovland@users.noreply.github.com>

Co-authored-by: Tor Hovland <55164+torhovland@users.noreply.github.com> Co-authored-by: Ed Page <eopage@gmail.com>

epage · 2024-09-06T15:06:34Z

Thanks!

@bors r+

bors · 2024-09-06T15:06:36Z

📌 Commit a016e5f has been approved by epage

It is now in the queue for this repository.

bors · 2024-09-06T15:07:44Z

⌛ Testing commit a016e5f with merge be1bbda...

epage · 2024-09-06T15:12:27Z

btw I've noted in #10948 the open questions

bors · 2024-09-06T15:37:08Z

☀️ Test successful - checks-actions
Approved by: epage
Pushing be1bbda to master...

Update cargo 24 commits in c1fa840a85eca53818895901a53fae34247448b2..468f1500bdca6591555b204ef31f92d725053190 2024-08-29 21:03:53 +0000 to 2024-09-14 19:24:54 +0000 - Disable the shell_completions tests (rust-lang/cargo#14546) - fix(vendor): trust crate version only when coming from registries (rust-lang/cargo#14530) - docs: Feature resolver version 2: clarify use of 'target' (rust-lang/cargo#14540) - Update docs for how cargo is published (rust-lang/cargo#14539) - feat: Add native comlpetion with CompleteEnv under the nightly (rust-lang/cargo#14493) - fix(new): Add to workspace relative to manifest, not current-dir (rust-lang/cargo#14505) - Fix parsing of comma separated values in --crate-type flag (rust-lang/cargo#14499) - Include public/private dependency status in `cargo metadata` (rust-lang/cargo#14504) - Remove unnecessary symbols (rust-lang/cargo#14519) - docs: bin source can be `src/main.rs` and/or in `src/bin/` (rust-lang/cargo#14515) - fix(toml): Don't require MSRV bump for pub/priv (rust-lang/cargo#14507) - bail before packaging on same version (rust-lang/cargo#14448) - Implement path-bases (RFC 3529) 2/n: `cargo [add|remove|update]` support (rust-lang/cargo#14427) - Publish workspace (rust-lang/cargo#14433) - Bump ci's version of cargo-semver-version (rust-lang/cargo#14503) - Document -Zpackage-workspace (rust-lang/cargo#14496) - uplift windows gnullvm import libraries (rust-lang/cargo#14451) - Bump to 0.84.0; update changelog (rust-lang/cargo#14495) - Fix cargo add behaving different when translating package name (rust-lang/cargo#13765) - chore(deps): update rust crate core-foundation to 0.10.0 (rust-lang/cargo#14475) - feat(resolve): Report MSRV compatible version instead of incomptible (rust-lang/cargo#14471) - Don't automatically include the current crate when packaging (rust-lang/cargo#14488) - Fix elided lifetime (rust-lang/cargo#14487) - chore(deps): update rust crate pasetors to 0.7.0 (rust-lang/cargo#14478) r? ghost

Update cargo 25 commits in c1fa840a85eca53818895901a53fae34247448b2..a9a418d1a22f29e7dfd034e3b93f15657e608a29 2024-08-29 21:03:53 +0000 to 2024-09-15 19:13:12 +0000 - chore: revert change to Cargo.lock in f25806c (rust-lang/cargo#14547) - Disable the shell_completions tests (rust-lang/cargo#14546) - fix(vendor): trust crate version only when coming from registries (rust-lang/cargo#14530) - docs: Feature resolver version 2: clarify use of 'target' (rust-lang/cargo#14540) - Update docs for how cargo is published (rust-lang/cargo#14539) - feat: Add native comlpetion with CompleteEnv under the nightly (rust-lang/cargo#14493) - fix(new): Add to workspace relative to manifest, not current-dir (rust-lang/cargo#14505) - Fix parsing of comma separated values in --crate-type flag (rust-lang/cargo#14499) - Include public/private dependency status in `cargo metadata` (rust-lang/cargo#14504) - Remove unnecessary symbols (rust-lang/cargo#14519) - docs: bin source can be `src/main.rs` and/or in `src/bin/` (rust-lang/cargo#14515) - fix(toml): Don't require MSRV bump for pub/priv (rust-lang/cargo#14507) - bail before packaging on same version (rust-lang/cargo#14448) - Implement path-bases (RFC 3529) 2/n: `cargo [add|remove|update]` support (rust-lang/cargo#14427) - Publish workspace (rust-lang/cargo#14433) - Bump ci's version of cargo-semver-version (rust-lang/cargo#14503) - Document -Zpackage-workspace (rust-lang/cargo#14496) - uplift windows gnullvm import libraries (rust-lang/cargo#14451) - Bump to 0.84.0; update changelog (rust-lang/cargo#14495) - Fix cargo add behaving different when translating package name (rust-lang/cargo#13765) - chore(deps): update rust crate core-foundation to 0.10.0 (rust-lang/cargo#14475) - feat(resolve): Report MSRV compatible version instead of incomptible (rust-lang/cargo#14471) - Don't automatically include the current crate when packaging (rust-lang/cargo#14488) - Fix elided lifetime (rust-lang/cargo#14487) - chore(deps): update rust crate pasetors to 0.7.0 (rust-lang/cargo#14478)

rustbot assigned ehuss Aug 21, 2024

rustbot added A-interacts-with-crates.io Area: interaction with registries Command-package Command-publish S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 21, 2024

jneem force-pushed the multi-package-publishing-rebased branch from ca54e9c to 8f03308 Compare August 21, 2024 04:24

epage mentioned this pull request Aug 28, 2024

fix(pkgid): Allow open namespaces in PackageIdSpec's #14467

Merged

jneem force-pushed the multi-package-publishing-rebased branch from 8f03308 to 28420d8 Compare August 30, 2024 03:18