fix: detect circular publish dependency and error instead of silent timeout

Author: raushan728

src/cargo/ops/registry/publish.rs

@@ -190,13 +190,42 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
     // As a side effect, any given package's "effective" timeout may be much larger.
     let mut to_confirm = BTreeSet::new();
 
+    // Check for circular dependencies before entering the main loop.
+    // We check for cycles before entering the publish loop to provide a clear error
+    // message if the workspace cannot be published due to circular dependencies.
+    // `has_cycles()` detects any back-edges in the dependency graph, while
+    // `cycle_members()` identifies the specific packages involved in the cycle.
+    if plan.has_cycles() {
+        bail!(
+            "circular dependency detected while publishing {}\n\
+             help: to break a cycle between dev-dependencies \
+             and other dependencies, remove the version field \
+             on the dev-dependency so it will be implicitly \
+             stripped on publish",
+            package_list(plan.cycle_members(), "and")
+        );
+    }
+
     while !plan.is_empty() {
         // There might not be any ready package, if the previous confirmations
         // didn't unlock a new one. For example, if `c` depends on `a` and
         // `b`, and we uploaded `a` and `b` but only confirmed `a`, then on
         // the following pass through the outer loop nothing will be ready for
         // upload.
         let mut ready = plan.take_ready();
+
+        if ready.is_empty() {
+            // This situation should not be a circular dependency,
+            // since those are caught before the loop.
+            // It could be that waiting for confirmation timed out, or something else.
+            return Err(crate::util::internal(format!(
+                "no packages ready to publish but {} packages remain in plan with {} awaiting confirmation: {}",
+                plan.len(),
+                to_confirm.len(),
+                package_list(plan.iter(), "and")
+            )));
+        }
+
         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())?;
@@ -745,6 +774,45 @@ impl PublishPlan {
         self.dependencies_count.len()
     }
 
+    /// Returns `true` if the dependency graph contains
+    /// a cycle that would prevent any package from
+    /// being published.
+    ///
+    /// This happens when all remaining packages have
+    /// unmet dependencies with no package having
+    /// zero outstanding dependencies.
+    fn has_cycles(&self) -> bool {
+        // If the graph is not empty but no packages have 0 dependencies,
+        // it means there's a cycle preventing any progress.
+        !self.is_empty() && self.dependencies_count.values().all(|&weight| weight > 0)
+    }
+
+    /// Returns the members of any dependency cycles in the plan.
+    ///
+    /// This identifies the specific packages that form a circular dependency,
+    /// allowing for more precise error reporting.
+    fn cycle_members(&self) -> Vec<PackageId> {
+        let mut remaining: BTreeSet<_> = self.dependencies_count.keys().copied().collect();
+        loop {
+            let to_remove: Vec<_> = remaining
+                .iter()
+                .filter(|&id| {
+                    self.dependents
+                        .edges(id)
+                        .all(|(child, _)| !remaining.contains(child))
+                })
+                .copied()
+                .collect();
+            if to_remove.is_empty() {
+                break;
+            }
+            for id in to_remove {
+                remaining.remove(&id);
+            }
+        }
+        remaining.into_iter().collect()
+    }
+
     /// Returns the set of packages that are ready for publishing (i.e. have no outstanding dependencies).
     ///
     /// These will not be returned in future calls.

tests/testsuite/publish.rs

@@ -4650,8 +4650,8 @@ fn workspace_circular_publish_dependency() {
         .file("bar/src/lib.rs", "")
         .build();
 
-    // With current buggy code, it will silently wait for dependencies because plan.take_ready() returns empty
-    // but the `plan` isn't fully completed. It eventually hits a timeout with a blank string.
+    // The circular dependency between foo and bar (via dev-dependencies) is detected
+    // and reported with a helpful error message.
     p.cargo("publish --workspace --no-verify -Zpublish-timeout --config publish.timeout=1")
         .masquerade_as_nightly_cargo(&["publish-timeout"])
         .replace_crates_io(registry.index_url())
@@ -4664,12 +4664,8 @@ fn workspace_circular_publish_dependency() {
 [PACKAGING] bar v0.1.1 ([ROOT]/foo/bar)
 [UPDATING] crates.io index
 [PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
-[NOTE] waiting for  to be available at registry `crates-io`.
-      2 remaining crates to be published
-[WARNING] timed out waiting for  to be available in registry `crates-io`
-  |
-  = [NOTE] the registry may have a backlog that is delaying making the crates available. The crates should be available soon.
-[ERROR] unable to publish bar v0.1.1 and foo v0.1.1 due to a timeout while waiting for published dependencies to be available.
+[ERROR] circular dependency detected while publishing bar v0.1.1 and foo v0.1.1
+[HELP] to break a cycle between dev-dependencies and other dependencies, remove the version field on the dev-dependency so it will be implicitly stripped on publish
 
 "#]])
         .run();
@@ -4679,9 +4675,9 @@ fn workspace_circular_publish_dependency_with_non_cycle_package() {
     // Test that when a workspace has a circular dependency, only the packages involved
     // in the cycle are reported in the error message, even if other packages are
     // blocked by the cycle.
-    // With current buggy code, all 3 crates timeout
-    // together with blank crate names. Only a and b
-    // form the cycle but c is also blocked.
+    // c is standalone and publishes successfully.
+    // a and b form a cycle (a depends on b, b dev-depends on a)
+    // and are detected as unable to make progress.
     let registry = registry::RegistryBuilder::new()
         .http_api()
         .http_index()
@@ -4769,12 +4765,10 @@ fn workspace_circular_publish_dependency_with_non_cycle_package() {
 [NOTE] waiting for c v1.0.1 to be available at registry `crates-io`.
       2 remaining crates to be published
 [PUBLISHED] c v1.0.1 at registry `crates-io`
-[NOTE] waiting for  to be available at registry `crates-io`.
-      2 remaining crates to be published
-[WARNING] timed out waiting for  to be available in registry `crates-io`
-  |
-  = [NOTE] the registry may have a backlog that is delaying making the crates available. The crates should be available soon.
-[ERROR] unable to publish a v1.0.1 and b v1.0.1 due to a timeout while waiting for published dependencies to be available.
+[ERROR] no packages ready to publish but 2 packages remain in plan with 0 awaiting confirmation: a v1.0.1 and b v1.0.1
+[NOTE] this is an unexpected cargo internal error
+[NOTE] we would appreciate a bug report: https://github.com/rust-lang/cargo/issues/
+[NOTE] cargo [..]
 
 "#]])
         .run();