From b55e4be449a87ca4e70cc1aa981bdccf8e1c4f0b Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 24 Sep 2025 23:17:20 -0400 Subject: [PATCH 1/3] test(publish): clarify test name --- tests/testsuite/publish.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index aeba4cebf13..fdabe846bbb 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -4192,7 +4192,7 @@ fn virtual_ws_with_multiple_unpublishable_package() { } #[cargo_test] -fn workspace_flag_with_unpublishable_packages() { +fn workspace_flag_with_nonpublishable_packages() { let registry = RegistryBuilder::new().http_api().http_index().build(); let p = project() From e883b023b20ec43330a3bd33d731dc5594946cef Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 25 Sep 2025 01:22:08 -0400 Subject: [PATCH 2/3] test(publish): show no idempotent workspace publish --- tests/testsuite/publish.rs | 95 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index fdabe846bbb..89bfd10c873 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -4413,6 +4413,101 @@ fn all_unpublishable_packages() { .run(); } +#[cargo_test] +fn all_published_packages() { + let registry = RegistryBuilder::new().http_api().http_index().build(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["foo", "bar"] + "#, + ) + .file( + "foo/Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + "#, + ) + .file("foo/src/lib.rs", "") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.0.0" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + "#, + ) + .file("bar/src/lib.rs", "") + .build(); + + // First, publish all members + p.cargo("publish --workspace --no-verify") + .replace_crates_io(registry.index_url()) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[PACKAGING] bar v0.0.0 ([ROOT]/foo/bar) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] foo v0.0.0 ([ROOT]/foo/foo) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[UPLOADING] bar v0.0.0 ([ROOT]/foo/bar) +[UPLOADED] bar v0.0.0 to registry `crates-io` +[UPLOADING] foo v0.0.0 ([ROOT]/foo/foo) +[UPLOADED] foo v0.0.0 to registry `crates-io` +[NOTE] waiting for bar v0.0.0 or foo v0.0.0 to be available at registry `crates-io` +[HELP] you may press ctrl-c to skip waiting; the crates should be available shortly +[PUBLISHED] bar v0.0.0 and foo v0.0.0 at registry `crates-io` + +"#]]) + .run(); + + // Publishing all members again works + p.cargo("publish --workspace --no-verify") + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[ERROR] crate foo@0.0.0 already exists on crates.io index + +"#]]) + .run(); + + // Without `--workspace` works as it is a virtual workspace + p.cargo("publish --no-verify") + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[ERROR] crate foo@0.0.0 already exists on crates.io index + +"#]]) + .run(); + + // Change a file. It should fail due to checksum verification failure. + p.change_file("bar/src/lib.rs", "//! foo"); + p.cargo("publish --no-verify") + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[ERROR] crate foo@0.0.0 already exists on crates.io index + +"#]]) + .run(); +} + #[cargo_test] fn checksum_changed() { let registry = RegistryBuilder::new().http_api().http_index().build(); From 17d5186b2a49138743e5a43983bb2f53da4e29c8 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 25 Sep 2025 01:24:44 -0400 Subject: [PATCH 3/3] feat(publish): idempotent workspace publish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this, `cargo publish --workspace` would fail if any member packages were already published. After this, it skips already published packages and continues with the rest. To make sure the local package is really published, we verify the checksum of the newly packed `.crate` tarball against the checksum from the registry index. This helps catch cases where the package contents changed but the version wasn’t bumped, which would otherwise be treated as already published. --- src/cargo/ops/registry/publish.rs | 124 ++++++++++++++++++++++-------- tests/testsuite/publish.rs | 51 ++++++++++-- 2 files changed, 137 insertions(+), 38 deletions(-) diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 1abd00a4330..1219ea74eb0 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -29,6 +29,7 @@ use crate::core::Package; use crate::core::PackageId; use crate::core::PackageIdSpecQuery; use crate::core::SourceId; +use crate::core::Summary; use crate::core::Workspace; use crate::core::dependency::DepKind; use crate::core::manifest::ManifestMetadata; @@ -86,15 +87,17 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { .into_iter() .partition(|(pkg, _)| pkg.publish() == &Some(vec![])); // If `--workspace` is passed, - // the intent is more like "publish all publisable packages in this workspace", - // so skip `publish=false` packages. - let allow_unpublishable = match &opts.to_publish { + // the intent is more like "publish all publisable packages in this workspace". + // Hence, + // * skip `publish=false` packages + // * skip already published packages + let is_workspace_publish = match &opts.to_publish { Packages::Default => ws.is_virtual(), Packages::All(_) => true, Packages::OptOut(_) => true, Packages::Packages(_) => false, }; - if !unpublishable.is_empty() && !allow_unpublishable { + if !unpublishable.is_empty() && !is_workspace_publish { bail!( "{} cannot be published.\n\ `package.publish` must be set to `true` or a non-empty list in Cargo.toml to publish.", @@ -106,7 +109,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { } if pkgs.is_empty() { - if allow_unpublishable { + if is_workspace_publish { let n = unpublishable.len(); let plural = if n == 1 { "" } else { "s" }; ws.gctx().shell().print_report( @@ -140,13 +143,30 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { Some(Operation::Read).filter(|_| !opts.dry_run), )?; + // `maybe_published` tracks package versions that already exist in the registry, + // meaning they might have been published before. + // Later, we verify the tarball checksum to see + // if the local package matches the registry. + // This helps catch cases where the local version + // wasn’t bumped but files changed. + let mut maybe_published = HashMap::new(); + { let _lock = opts .gctx .acquire_package_cache_lock(CacheLockMode::DownloadExclusive)?; for (pkg, _) in &pkgs { - verify_unpublished(pkg, &mut source, &source_ids, opts.dry_run, opts.gctx)?; + if let Some(summary) = verify_unpublished( + pkg, + &mut source, + &source_ids, + opts.dry_run, + is_workspace_publish, + opts.gctx, + )? { + maybe_published.insert(pkg.package_id(), summary); + } verify_dependencies(pkg, ®istry, source_ids.original).map_err(|err| { ManifestError::new( err.context(format!( @@ -199,15 +219,38 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { let mut ready = plan.take_ready(); while let Some(pkg_id) = ready.pop_first() { let (pkg, (_features, tarball)) = &pkg_dep_graph.packages[&pkg_id]; - opts.gctx.shell().status("Uploading", pkg.package_id())?; - - if !opts.dry_run { - let ver = pkg.version().to_string(); + if opts.dry_run { + opts.gctx.shell().status("Uploading", pkg.package_id())?; + } else { tarball.file().seek(SeekFrom::Start(0))?; let hash = cargo_util::Sha256::new() .update_file(tarball.file())? .finish_hex(); + + if let Some(summary) = maybe_published.get(&pkg.package_id()) { + if summary.checksum() == Some(hash.as_str()) { + opts.gctx.shell().warn(format_args!( + "skipping upload for crate {}@{}: already exists on {}", + pkg.name(), + pkg.version(), + source.describe() + ))?; + plan.mark_confirmed([pkg.package_id()]); + continue; + } + bail!( + "crate {}@{} already exists on {} but tarball checksum mismatched\n\ + perhaps local files have changed but forgot to bump the version?", + pkg.name(), + pkg.version(), + source.describe() + ); + } + + opts.gctx.shell().status("Uploading", pkg.package_id())?; + + let ver = pkg.version().to_string(); let operation = Operation::Publish { name: pkg.name().as_str(), vers: &ver, @@ -259,6 +302,12 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { } } + if to_confirm.is_empty() { + // nothing to confirm because some are already uploaded before + // this cargo invocation. + continue; + } + let confirmed = if opts.dry_run { to_confirm.clone() } else { @@ -431,13 +480,18 @@ fn poll_one_package( Ok(!summaries.is_empty()) } +/// Checks if a package is already published. +/// +/// Returns a [`Summary`] for computing the tarball checksum +/// to compare with the registry index later, if needed. fn verify_unpublished( pkg: &Package, source: &mut RegistrySource<'_>, source_ids: &RegistrySourceIds, dry_run: bool, + skip_already_publish: bool, gctx: &GlobalContext, -) -> CargoResult<()> { +) -> CargoResult> { let query = Dependency::parse( pkg.name(), Some(&pkg.version().to_exact_req().to_string()), @@ -451,28 +505,36 @@ fn verify_unpublished( std::task::Poll::Pending => source.block_until_ready()?, } }; - if !duplicate_query.is_empty() { - // Move the registry error earlier in the publish process. - // Since dry-run wouldn't talk to the registry to get the error, we downgrade it to a - // warning. - if dry_run { - gctx.shell().warn(format!( - "crate {}@{} already exists on {}", - pkg.name(), - pkg.version(), - source.describe() - ))?; - } else { - bail!( - "crate {}@{} already exists on {}", - pkg.name(), - pkg.version(), - source.describe() - ); - } + if duplicate_query.is_empty() { + return Ok(None); } - Ok(()) + // Move the registry error earlier in the publish process. + // Since dry-run wouldn't talk to the registry to get the error, + // we downgrade it to a warning. + if skip_already_publish || dry_run { + gctx.shell().warn(format!( + "crate {}@{} already exists on {}", + pkg.name(), + pkg.version(), + source.describe() + ))?; + } else { + bail!( + "crate {}@{} already exists on {}", + pkg.name(), + pkg.version(), + source.describe() + ); + } + + assert_eq!( + duplicate_query.len(), + 1, + "registry must not have duplicat versions", + ); + let summary = duplicate_query.into_iter().next().unwrap().into_summary(); + Ok(skip_already_publish.then_some(summary)) } fn verify_dependencies( diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 89bfd10c873..ebcb7f63673 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -4038,10 +4038,28 @@ Caused by: // Publishing the whole workspace now will fail, as `a` is already published. p.cargo("publish") .replace_crates_io(registry.index_url()) - .with_status(101) .with_stderr_data(str![[r#" [UPDATING] crates.io index -[ERROR] crate a@0.0.1 already exists on crates.io index +[WARNING] crate a@0.0.1 already exists on crates.io index +[PACKAGING] a v0.0.1 ([ROOT]/foo/a) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] b v0.0.1 ([ROOT]/foo/b) +[UPDATING] crates.io index +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[VERIFYING] a v0.0.1 ([ROOT]/foo/a) +[COMPILING] a v0.0.1 ([ROOT]/foo/target/package/a-0.0.1) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[VERIFYING] b v0.0.1 ([ROOT]/foo/b) +[UNPACKING] a v0.0.1 (registry `[ROOT]/foo/target/package/tmp-registry`) +[COMPILING] a v0.0.1 +[COMPILING] b v0.0.1 ([ROOT]/foo/target/package/b-0.0.1) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[WARNING] skipping upload for crate a@0.0.1: already exists on crates.io index +[UPLOADING] b v0.0.1 ([ROOT]/foo/b) +[UPLOADED] b v0.0.1 to registry `crates-io` +[NOTE] waiting for b v0.0.1 to be available at registry `crates-io` +[HELP] you may press ctrl-c to skip waiting; the crate should be available shortly +[PUBLISHED] b v0.0.1 at registry `crates-io` "#]]) .run(); @@ -4476,10 +4494,16 @@ fn all_published_packages() { // Publishing all members again works p.cargo("publish --workspace --no-verify") .replace_crates_io(registry.index_url()) - .with_status(101) .with_stderr_data(str![[r#" [UPDATING] crates.io index -[ERROR] crate foo@0.0.0 already exists on crates.io index +[WARNING] crate foo@0.0.0 already exists on crates.io index +[WARNING] crate bar@0.0.0 already exists on crates.io index +[PACKAGING] bar v0.0.0 ([ROOT]/foo/bar) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] foo v0.0.0 ([ROOT]/foo/foo) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[WARNING] skipping upload for crate bar@0.0.0: already exists on crates.io index +[WARNING] skipping upload for crate foo@0.0.0: already exists on crates.io index "#]]) .run(); @@ -4487,10 +4511,16 @@ fn all_published_packages() { // Without `--workspace` works as it is a virtual workspace p.cargo("publish --no-verify") .replace_crates_io(registry.index_url()) - .with_status(101) .with_stderr_data(str![[r#" [UPDATING] crates.io index -[ERROR] crate foo@0.0.0 already exists on crates.io index +[WARNING] crate foo@0.0.0 already exists on crates.io index +[WARNING] crate bar@0.0.0 already exists on crates.io index +[PACKAGING] bar v0.0.0 ([ROOT]/foo/bar) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] foo v0.0.0 ([ROOT]/foo/foo) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[WARNING] skipping upload for crate bar@0.0.0: already exists on crates.io index +[WARNING] skipping upload for crate foo@0.0.0: already exists on crates.io index "#]]) .run(); @@ -4502,7 +4532,14 @@ fn all_published_packages() { .with_status(101) .with_stderr_data(str![[r#" [UPDATING] crates.io index -[ERROR] crate foo@0.0.0 already exists on crates.io index +[WARNING] crate foo@0.0.0 already exists on crates.io index +[WARNING] crate bar@0.0.0 already exists on crates.io index +[PACKAGING] bar v0.0.0 ([ROOT]/foo/bar) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] foo v0.0.0 ([ROOT]/foo/foo) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[ERROR] crate bar@0.0.0 already exists on crates.io index but tarball checksum mismatched +perhaps local files have changed but forgot to bump the version? "#]]) .run();