feat(publish): idempotent workspace publish

weihanglo · weihanglo · commit e3d7ab58b699 · 2025-09-25T01:36:04.000-04:00
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.
diff --git 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, &registry, 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<Option<Summary>> {
     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
@@ -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,21 +4409,33 @@ 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();
 
     // 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();
