Skip to content

Commit

Permalink
Fixup (#465)
Browse files Browse the repository at this point in the history 
* Cleanup timing

* Use direct_dependents

* Always print graphs for each pack

Previously if there was a diag that was skipped due to the max log level
that was then followed by additional diagnostics, those wouldn't have
a graph emitted for them

* Update snapshots
  • Loading branch information
@Jake-Shadle
Jake-Shadle committed Oct 25, 2022
1 parent ff2068a commit cb12586
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 374 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 4 additions & 21 deletions src/bans.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,29 +351,12 @@ pub fn check(
let is_allowed_by_wrapper =
if let Some(wrappers) = ban_wrappers.get(rm.index).and_then(|bw| bw.as_ref()) {
let nid = ctx.krates.nid_for_kid(&krate.id).unwrap();
let graph = ctx.krates.graph();

let mut direct_dependencies = Vec::new();
let mut stack = vec![nid];
let mut visited = std::collections::BTreeSet::new();

while let Some(nid) = stack.pop() {
for edge in graph.edges_directed(nid, Direction::Incoming) {
if let krates::Edge::Feature = edge.weight() {
stack.push(edge.source());
} else if visited.insert(edge.source()) {
direct_dependencies.push(edge.source());
}
}
}

// Ensure that every single crate that has a direct dependency
// on the banned crate is an allowed wrapper
direct_dependencies.into_iter().all(|nid| {
let src = &ctx.krates[nid];

ctx.krates.direct_dependents(nid).into_iter().all(|src| {
let (diag, is_allowed): (Diag, _) =
match wrappers.iter().find(|aw| aw.value == src.name) {
match wrappers.iter().find(|aw| aw.value == src.krate.name) {
Some(aw) => (
diags::BannedAllowedByWrapper {
ban_cfg: ban_cfg.clone(),
Expand All @@ -382,7 +365,7 @@ pub fn check(
span: aw.span.clone(),
},
banned_krate: krate,
wrapper_krate: src,
wrapper_krate: src.krate,
}
.into(),
true,
Expand All @@ -391,7 +374,7 @@ pub fn check(
diags::BannedUnmatchedWrapper {
ban_cfg: ban_cfg.clone(),
banned_krate: krate,
parent_krate: src,
parent_krate: src.krate,
}
.into(),
false,
Expand Down
21 changes: 8 additions & 13 deletions src/cargo-deny/check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,8 @@ pub(crate) fn cmd(
log::info!("checking licenses...");
let start = Instant::now();
licenses::check(ctx, summary, sink);
let end = Instant::now();

log::info!("licenses checked in {}ms", (end - start).as_millis());
log::info!("licenses checked in {}ms", start.elapsed().as_millis());
});
}

Expand Down Expand Up @@ -535,9 +534,8 @@ pub(crate) fn cmd(
log::info!("checking bans...");
let start = Instant::now();
bans::check(ctx, output_graph, cargo_spans, bans_sink);
let end = Instant::now();

log::info!("bans checked in {}ms", (end - start).as_millis());
log::info!("bans checked in {}ms", start.elapsed().as_millis());
});
}

Expand All @@ -559,9 +557,8 @@ pub(crate) fn cmd(
log::info!("checking sources...");
let start = Instant::now();
sources::check(ctx, sources_sink);
let end = Instant::now();

log::info!("sources checked in {}ms", (end - start).as_millis());
log::info!("sources checked in {}ms", start.elapsed().as_millis());
});
}

Expand Down Expand Up @@ -594,9 +591,8 @@ pub(crate) fn cmd(
};

advisories::check(ctx, &db, lf, audit_reporter, advisories_sink);
let end = Instant::now();

log::info!("advisories checked in {}ms", (end - start).as_millis());
log::info!("advisories checked in {}ms", start.elapsed().as_millis());
});
}
});
Expand All @@ -617,26 +613,25 @@ fn print_diagnostics(
match crate::common::DiagPrinter::new(log_ctx, krates) {
Some(printer) => {
for pack in rx {
let mut lock = printer.lock();

let check_stats = match pack.check {
Check::Advisories => stats.advisories.as_mut().unwrap(),
Check::Bans => stats.bans.as_mut().unwrap(),
Check::Licenses => stats.licenses.as_mut().unwrap(),
Check::Sources => stats.sources.as_mut().unwrap(),
};

for diag in pack {
for diag in pack.iter() {
match diag.diag.severity {
Severity::Error => check_stats.errors += 1,
Severity::Warning => check_stats.warnings += 1,
Severity::Note => check_stats.notes += 1,
Severity::Help => check_stats.helps += 1,
Severity::Bug => {}
}

lock.print_krate_diag(diag, &files);
}

let mut lock = printer.lock();
lock.print_krate_pack(pack, &files);
}
}
None => while rx.recv().is_ok() {},
Expand Down
60 changes: 36 additions & 24 deletions src/cargo-deny/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,10 @@ impl KrateContext {
});

if let Ok(ref krates) = graph {
let end = std::time::Instant::now();
log::info!(
"gathered {} crates in {}ms",
krates.len(),
(end - start).as_millis()
start.elapsed().as_millis()
);
}

Expand Down Expand Up @@ -382,39 +381,52 @@ impl<'a, 'b> OutputLock<'a, 'b> {
}
}

pub fn print_krate_diag(&mut self, mut diag: cargo_deny::diag::Diag, files: &Files) {
pub fn print_krate_pack(&mut self, pack: cargo_deny::diag::Pack, files: &Files) {
let mut emitted = std::collections::BTreeSet::new();

match self {
Self::Human(cfg, max, l) => {
if diag.diag.severity < *max {
return;
}
for mut diag in pack {
if diag.diag.severity < *max {
continue;
}

if let Some(grapher) = &cfg.grapher {
for gn in diag.graph_nodes {
if let Ok(graph) =
grapher.build_graph(&gn, if diag.with_features { 1 } else { 0 })
{
let graph_text = diag::write_graph_as_text(&graph);
diag.diag.notes.push(graph_text);
if let Some(grapher) = &cfg.grapher {
for gn in diag.graph_nodes {
if emitted.contains(&gn.kid) {
let krate =
&grapher.krates[grapher.krates.nid_for_kid(&gn.kid).unwrap()];
diag.diag
.notes
.push(format!("{} v{} (*)", krate.name, krate.version));
} else if let Ok(graph) =
grapher.build_graph(&gn, if diag.with_features { 1 } else { 0 })
{
let graph_text = diag::write_graph_as_text(&graph);
diag.diag.notes.push(graph_text);
emitted.insert(gn.kid);
}
}
}
}

let _ = term::emit(l, &cfg.config, files, &diag.diag);
let _ = term::emit(l, &cfg.config, files, &diag.diag);
}
}
Self::Json(cfg, max, w) => {
if diag.diag.severity < *max {
return;
}
for diag in pack {
if diag.diag.severity < *max {
continue;
}

let to_print = diag::diag_to_json(diag, files, cfg.grapher.as_ref());
let to_print = diag::diag_to_json(diag, files, cfg.grapher.as_ref());

use serde::Serialize;
use serde::Serialize;

let mut ser = serde_json::Serializer::new(w);
if to_print.serialize(&mut ser).is_ok() {
let w = ser.into_inner();
let _ = w.write(b"\n");
let mut ser = serde_json::Serializer::new(&mut *w);
if to_print.serialize(&mut ser).is_ok() {
let w = ser.into_inner();
let _ = w.write(b"\n");
}
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/diag.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl Pack {
pub(crate) fn push(&mut self, diag: impl Into<Diag>) -> &mut Diag {
let mut diag = diag.into();
if diag.graph_nodes.is_empty() {
if let Some(kid) = self.kid.take() {
if let Some(kid) = self.kid.clone() {
diag.graph_nodes.push(GraphNode { kid, feature: None });
}
}
Expand All @@ -114,6 +114,11 @@ impl Pack {
pub(crate) fn is_empty(&self) -> bool {
self.diags.is_empty()
}

#[inline]
pub fn iter(&self) -> impl Iterator<Item = &Diag> {
self.diags.iter()
}
}

impl IntoIterator for Pack {
Expand Down
31 changes: 12 additions & 19 deletions src/diag/grapher.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn is_empty(v: &Vec<GraphNode>) -> bool {
/// Provides the `InclusionGrapher::write_graph` method which creates a reverse
/// dependency graph rooted at a specific node
pub struct InclusionGrapher<'a> {
krates: &'a Krates,
pub krates: &'a Krates,
}

impl<'a> InclusionGrapher<'a> {
Expand Down Expand Up @@ -163,24 +163,17 @@ impl<'a> InclusionGrapher<'a> {
));
} else {
// If we're not adding features we need to walk up any feature edges
// until we reach
let mut node_stack = vec![np.node];
let mut visited_nodes = HashSet::new();

while let Some(node) = node_stack.pop() {
for edge in graph.edges_directed(node, pg::Direction::Incoming) {
if let Edge::Feature = edge.weight() {
if visited_nodes.insert(edge.source()) {
node_stack.push(edge.source());
}
} else if !node_parents.iter().any(|np| np.node == edge.source()) {
node_parents.push(NodePrint {
node: edge.source(),
edge: Some(edge.id()),
});
}
}
}
// until we reach an actual crate dependenc

node_parents.extend(
self.krates
.direct_dependents(np.node)
.into_iter()
.map(|dd| NodePrint {
node: dd.node_id,
edge: Some(dd.edge_id),
}),
);
}

let parents = if !node_parents.is_empty() {
Expand Down
Loading

0 comments on commit cb12586

Please sign in to comment.