Skip to content

Improve dependency deduplication diagnostics #52080

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 6, 2018
Merged

Improve dependency deduplication diagnostics #52080

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
82a1328
Vertical list of tools to check
oli-obk Jul 5, 2018
f1e3a5a
Only display difference of features, not all features
oli-obk Jul 5, 2018
f352e98
Address review comments
oli-obk Jul 5, 2018
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
35 changes: 29 additions & 6 deletions src/bootstrap/tool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use std::env;
use std::iter;
use std::path::PathBuf;
use std::process::{Command, exit};
use std::collections::HashSet;

use Mode;
use Compiler;
Expand Down Expand Up @@ -118,8 +119,13 @@ impl Step for ToolBuild {
let mut duplicates = Vec::new();
let is_expected = compile::stream_cargo(builder, &mut cargo, &mut |msg| {
// Only care about big things like the RLS/Cargo for now
if tool != "rls" && tool != "cargo" && tool != "clippy-driver" {
return
match tool {
| "rls"
| "cargo"
| "clippy-driver"
=> {}

_ => return,
}
let (id, features, filenames) = match msg {
compile::CargoMessage::CompilerArtifact {
Expand Down Expand Up @@ -178,12 +184,29 @@ impl Step for ToolBuild {
typically means that something was recompiled because \
a transitive dependency has different features activated \
than in a previous build:\n");
println!("the following dependencies are duplicated although they \
have the same features enabled:");
for (id, cur, prev) in &duplicates {
println!(" {}", id);
if cur.2 == prev.2 {
// same features
println!(" `{}` ({:?})\n `{}` ({:?})", cur.0, cur.1, prev.0, prev.1);
}
}
println!("the following dependencies have different features:");
for (id, cur, prev) in duplicates {
println!(" {}", id);
println!(" `{}` enabled features {:?} at {:?}",
cur.0, cur.2, cur.1);
println!(" `{}` enabled features {:?} at {:?}",
prev.0, prev.2, prev.1);
if cur.2 == prev.2 {
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use duplicates.drain_filter(...) to avoid the double-checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^ I considered that and then was like oh it's just during failure and copy paste is so convenient. I'll do it anyway.

}
let cur_features: HashSet<_> = cur.2.into_iter().collect();
let prev_features: HashSet<_> = prev.2.into_iter().collect();
let cur_extra: Vec<_> = cur_features.difference(&prev_features).collect();
let prev_extra: Vec<_> = prev_features.difference(&cur_features).collect();
println!(" `{}` additionally enabled features {:?} at {:?}",
cur.0, cur_extra, cur.1);
println!(" `{}` additionally enabled features {:?} at {:?}",
prev.0, prev_extra, prev.1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 6 lines could be simply

println!("    `{}` additionally enabled features {:?} at {:?}",
         cur.0, &cur_features - &prev_features, cur.1);     
println!("    `{}` additionally enabled features {:?} at {:?}",
         prev.0, &prev_features - &cur_features, prev.1);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh. that's why - didn't work... you need references -.-

}
println!("");
panic!("tools should not compile multiple copies of the same crate");
Expand Down