From dd9e3121ee76e8723ed5fdf715d5a9946e1724bd 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 846b0689328..f823a627d01 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -4108,7 +4108,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 8f9ac5889a762e9b801dc32a99c1b7adbdd69b70 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 f823a627d01..89028c69075 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -4328,6 +4328,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 e3d7ab58b699c9f1634351005d1136c7d32550b5 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 d19da9f612c..6e181100b1a 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -28,6 +28,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; @@ -85,15 +86,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.", @@ -105,7 +108,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().warn(format_args!( @@ -154,13 +157,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!( @@ -213,15 +233,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, @@ -273,6 +316,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 { @@ -440,13 +489,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()), @@ -460,28 +514,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 89028c69075..5261cea818d 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -3954,10 +3954,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(); @@ -4391,10 +4409,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(); @@ -4402,10 +4426,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(); @@ -4417,7 +4447,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();