Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions src/cargo/ops/registry/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,37 @@ 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
// `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() {
// 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())?;
Expand Down Expand Up @@ -714,6 +738,8 @@ fn transmit(
struct PublishPlan {
/// Graph of publishable packages where the edges are `(dependency -> dependent)`
dependents: Graph<PackageId, ()>,
/// The original graph of publishable packages where the edges are `(dependent -> dependency)`
graph: Graph<PackageId, ()>,
/// The weight of a package is the number of unpublished dependencies it has.
dependencies_count: HashMap<PackageId, usize>,
}
Expand All @@ -729,6 +755,7 @@ impl PublishPlan {
.collect();
Self {
dependents,
graph: graph.clone(),
dependencies_count,
}
}
Expand All @@ -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<PackageId> {
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.
Expand Down
166 changes: 166 additions & 0 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}