diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 1abd00a4330..5afcb1a9d99 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -190,6 +190,18 @@ 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 publishing. + 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 @@ -197,6 +209,18 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { // the following pass through the outer loop nothing will be ready for // upload. let mut ready = plan.take_ready(); + + if ready.is_empty() { + // Circular dependencies are caught above, so this indicates a failure + // to progress, potentially due to a timeout while waiting for confirmations. + 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())?; @@ -714,6 +738,8 @@ fn transmit( struct PublishPlan { /// Graph of publishable packages where the edges are `(dependency -> dependent)` dependents: Graph, + /// The original graph of publishable packages where the edges are `(dependent -> dependency)` + graph: Graph, /// The weight of a package is the number of unpublished dependencies it has. dependencies_count: HashMap, } @@ -729,6 +755,7 @@ impl PublishPlan { .collect(); Self { dependents, + graph: graph.clone(), dependencies_count, } } @@ -745,6 +772,34 @@ impl PublishPlan { self.dependencies_count.len() } + /// Determines whether the dependency graph contains any circular dependencies. + fn has_cycles(&self) -> bool { + !self.cycle_members().is_empty() + } + + /// Identifies and returns the packages involved in a circular dependency. + fn cycle_members(&self) -> Vec { + let mut remaining: BTreeSet<_> = self.dependencies_count.keys().copied().collect(); + loop { + let to_remove: Vec<_> = remaining + .iter() + .filter(|&id| { + self.graph + .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. diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index aeba4cebf13..2207186872a 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -4595,3 +4595,169 @@ Caused by: "#]]) .run(); } + +#[cargo_test] +fn workspace_circular_publish_dependency() { + // Verify detection and reporting of workspace circular dependencies. + let registry = registry::RegistryBuilder::new() + .http_api() + .http_index() + .build(); + + cargo_test_support::registry::Package::new("foo", "0.1.0").publish(); + cargo_test_support::registry::Package::new("bar", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["foo", "bar"] + "#, + ) + .file( + "foo/Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.1" + edition = "2015" + license = "MIT" + description = "foo" + repository = "foo" + + [dependencies] + bar = { version = "0.1", path = "../bar" } + "#, + ) + .file("foo/src/lib.rs", "") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.1.1" + edition = "2015" + license = "MIT" + description = "bar" + repository = "bar" + + [dev-dependencies] + foo = { version = "0.1", path = "../foo" } + "#, + ) + .file("bar/src/lib.rs", "") + .build(); + + // Ensure the circular dependency is caught and reported clearly. + p.cargo("publish --workspace --no-verify -Zpublish-timeout --config publish.timeout=1") + .masquerade_as_nightly_cargo(&["publish-timeout"]) + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[PACKAGING] foo v0.1.1 ([ROOT]/foo/foo) +[UPDATING] crates.io index +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] bar v0.1.1 ([ROOT]/foo/bar) +[UPDATING] crates.io index +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[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(); +} +#[cargo_test] +fn workspace_circular_publish_dependency_with_non_cycle_package() { + // Verify that circular dependency errors only report packages actively involved in the cycle. + // Package 'c' is independent and should be excluded from the error message, + // even though the cycle between 'a' and 'b' blocks the overall workspace publish. + let registry = registry::RegistryBuilder::new() + .http_api() + .http_index() + .build(); + + cargo_test_support::registry::Package::new("a", "1.0.0").publish(); + cargo_test_support::registry::Package::new("b", "1.0.0").publish(); + cargo_test_support::registry::Package::new("c", "1.0.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b", "c"] + "#, + ) + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "1.0.1" + edition = "2015" + license = "MIT" + description = "a" + repository = "a" + + [dependencies] + b = { version = "1.0", path = "../b" } + c = { version = "1.0", path = "../c" } + "#, + ) + .file("a/src/lib.rs", "") + .file( + "b/Cargo.toml", + r#" + [package] + name = "b" + version = "1.0.1" + edition = "2015" + license = "MIT" + description = "b" + repository = "b" + + [dependencies] + c = { version = "1.0", path = "../c" } + + [dev-dependencies] + a = { version = "1.0", path = "../a" } + "#, + ) + .file("b/src/lib.rs", "") + .file( + "c/Cargo.toml", + r#" + [package] + name = "c" + version = "1.0.1" + edition = "2015" + license = "MIT" + description = "c" + repository = "c" + "#, + ) + .file("c/src/lib.rs", "") + .build(); + + p.cargo("publish --workspace --no-verify -Zpublish-timeout --config publish.timeout=1") + .masquerade_as_nightly_cargo(&["publish-timeout"]) + .replace_crates_io(registry.index_url()) + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] crates.io index +[PACKAGING] c v1.0.1 ([ROOT]/foo/c) +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] b v1.0.1 ([ROOT]/foo/b) +[UPDATING] crates.io index +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[PACKAGING] a v1.0.1 ([ROOT]/foo/a) +[UPDATING] crates.io index +[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) +[ERROR] circular dependency detected while publishing a v1.0.1 and b v1.0.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(); +}