Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix doc duplicate removal of root units. #9276

Merged
merged 1 commit into from
Mar 16, 2021
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
20 changes: 10 additions & 10 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1558,8 +1558,11 @@ fn remove_duplicate_doc(
// Keep track of units to remove so that they can be efficiently removed
// from the unit_deps.
let mut removed_units: HashSet<Unit> = HashSet::new();
let mut remove = |units: Vec<Unit>, reason: &str| {
for unit in units {
let mut remove = |units: Vec<Unit>, reason: &str, cb: &dyn Fn(&Unit) -> bool| -> Vec<Unit> {
let (to_remove, remaining_units): (Vec<Unit>, Vec<Unit>) = units
.into_iter()
.partition(|unit| cb(unit) && !root_units.contains(unit));
for unit in to_remove {
log::debug!(
"removing duplicate doc due to {} for package {} target `{}`",
reason,
Expand All @@ -1569,6 +1572,7 @@ fn remove_duplicate_doc(
unit_graph.remove(&unit);
removed_units.insert(unit);
}
remaining_units
};
// Iterate over the duplicates and try to remove them from unit_graph.
for (_crate_name, mut units) in all_docs {
Expand All @@ -1581,14 +1585,11 @@ fn remove_duplicate_doc(
.iter()
.all(CompileKind::is_host)
{
let (to_remove, remaining_units): (Vec<Unit>, Vec<Unit>) =
units.into_iter().partition(|unit| unit.kind.is_host());
// Note these duplicates may not be real duplicates, since they
// might get merged in rebuild_unit_graph_shared. Either way, it
// shouldn't hurt to remove them early (although the report in the
// log might be confusing).
remove(to_remove, "host/target merger");
units = remaining_units;
units = remove(units, "host/target merger", &|unit| unit.kind.is_host());
if units.len() == 1 {
continue;
}
Expand All @@ -1610,10 +1611,9 @@ fn remove_duplicate_doc(
units.sort_by(|a, b| a.pkg.version().partial_cmp(b.pkg.version()).unwrap());
// Remove any entries with version < newest.
let newest_version = units.last().unwrap().pkg.version().clone();
let (to_remove, keep_units): (Vec<Unit>, Vec<Unit>) = units
.into_iter()
.partition(|unit| unit.pkg.version() < &newest_version);
remove(to_remove, "older version");
let keep_units = remove(units, "older version", &|unit| {
unit.pkg.version() < &newest_version
});
remaining_units.extend(keep_units);
} else {
remaining_units.extend(units);
Expand Down
66 changes: 66 additions & 0 deletions tests/testsuite/collisions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,3 +478,69 @@ fn collision_doc_target() {
)
.run();
}

#[cargo_test]
fn collision_with_root() {
// Check for a doc collision between a root package and a dependency.
// In this case, `foo-macro` comes from both the workspace and crates.io.
// This checks that the duplicate correction code doesn't choke on this
// by removing the root unit.
Package::new("foo-macro", "1.0.0").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["abc", "foo-macro"]
"#,
)
.file(
"abc/Cargo.toml",
r#"
[package]
name = "abc"
version = "1.0.0"

[dependencies]
foo-macro = "1.0"
"#,
)
.file("abc/src/lib.rs", "")
.file(
"foo-macro/Cargo.toml",
r#"
[package]
name = "foo-macro"
version = "1.0.0"

[lib]
proc-macro = true

[dependencies]
abc = {path="../abc"}
"#,
)
.file("foo-macro/src/lib.rs", "")
.build();

p.cargo("doc")
.with_stderr_unordered("\
[UPDATING] [..]
[DOWNLOADING] crates ...
[DOWNLOADED] foo-macro v1.0.0 [..]
warning: output filename collision.
The lib target `foo-macro` in package `foo-macro v1.0.0` has the same output filename as the lib target `foo-macro` in package `foo-macro v1.0.0 [..]`.
Colliding filename is: [CWD]/target/doc/foo_macro/index.html
The targets should have unique names.
This is a known bug where multiple crates with the same name use
the same path; see <https://github.com/rust-lang/cargo/issues/6313>.
[CHECKING] foo-macro v1.0.0
[DOCUMENTING] foo-macro v1.0.0
[CHECKING] abc v1.0.0 [..]
[DOCUMENTING] foo-macro v1.0.0 [..]
[DOCUMENTING] abc v1.0.0 [..]
[FINISHED] [..]
")
.run();
}