From fb9b234a4bedf97afbe6790df7ab9b89f2f1319d Mon Sep 17 00:00:00 2001 From: veetaha Date: Fri, 1 May 2020 23:13:59 +0300 Subject: [PATCH 1/2] Contribute a failing test on cyclic dependencies --- tests/bans.rs | 66 +++++++++++++++++++ .../test_data/cyclic_dependencies/Cargo.lock | 57 ++++++++++++++++ .../test_data/cyclic_dependencies/Cargo.toml | 2 + tests/test_data/cyclic_dependencies/deny.toml | 0 .../cyclic_dependencies/leaf/Cargo.toml | 9 +++ .../cyclic_dependencies/leaf/src/lib.rs | 1 + .../cyclic_dependencies/root/Cargo.toml | 9 +++ .../cyclic_dependencies/root/src/lib.rs | 1 + .../cyclic_dependencies/root/src/main.rs | 1 + 9 files changed, 146 insertions(+) create mode 100644 tests/bans.rs create mode 100644 tests/test_data/cyclic_dependencies/Cargo.lock create mode 100644 tests/test_data/cyclic_dependencies/Cargo.toml create mode 100644 tests/test_data/cyclic_dependencies/deny.toml create mode 100644 tests/test_data/cyclic_dependencies/leaf/Cargo.toml create mode 100644 tests/test_data/cyclic_dependencies/leaf/src/lib.rs create mode 100644 tests/test_data/cyclic_dependencies/root/Cargo.toml create mode 100644 tests/test_data/cyclic_dependencies/root/src/lib.rs create mode 100644 tests/test_data/cyclic_dependencies/root/src/main.rs diff --git a/tests/bans.rs b/tests/bans.rs new file mode 100644 index 00000000..040da768 --- /dev/null +++ b/tests/bans.rs @@ -0,0 +1,66 @@ +use cargo_deny::{ + bans::{ + self, + cfg::{Config, ValidConfig}, + }, + diag::KrateSpans, + CheckCtx, +}; +use std::{fs, path::PathBuf, time::Duration}; + +fn with_test_data(project_name: &str, accept_ctx: impl FnOnce(CheckCtx<'_, ValidConfig>)) { + let project_dir = PathBuf::from("./tests/test_data").join(project_name); + + let mut metadata_cmd = krates::cm::MetadataCommand::new(); + metadata_cmd.current_dir(&project_dir); + + let krates = &krates::Builder::new() + .build(metadata_cmd, krates::NoneFilter) + .unwrap(); + + let spans = KrateSpans::new(krates); + let mut files = codespan::Files::new(); + let spans_id = files.add(project_dir.join("Cargo.lock"), spans.1); + + let config: Config = fs::read_to_string(project_dir.join("deny.toml")) + .map(|it| toml::from_str(&it).unwrap()) + .unwrap_or_default(); + + accept_ctx(cargo_deny::CheckCtx { + krates, + krate_spans: &spans.0, + spans_id, + cfg: config.validate(spans_id).unwrap(), + }); +} + +// Covers issue https://github.com/EmbarkStudios/cargo-deny/issues/184 +#[test] +fn cyclic_dependencies_do_not_cause_infinite_loop() { + let (tx, rx) = crossbeam::unbounded(); + + let handle = std::thread::spawn(|| { + with_test_data("cyclic_dependencies", |check_ctx| { + let graph_output = Box::new(|_| Ok(())); + bans::check(check_ctx, Some(graph_output), tx); + }); + }); + + let timeout_duration = Duration::from_millis(10000); + let timeout = crossbeam::after(timeout_duration); + loop { + crossbeam::select! { + recv(rx) -> msg => { + if msg.is_err() { + // Yay, the sender was dopped (i.e. check was finished) + break; + } + } + recv(timeout) -> _ => { + panic!("Bans check timed out after {:?}", timeout_duration); + } + } + } + + handle.join().unwrap(); +} diff --git a/tests/test_data/cyclic_dependencies/Cargo.lock b/tests/test_data/cyclic_dependencies/Cargo.lock new file mode 100644 index 00000000..68a3ddfa --- /dev/null +++ b/tests/test_data/cyclic_dependencies/Cargo.lock @@ -0,0 +1,57 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +[[package]] +name = "ansi_term" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b" +dependencies = [ + "winapi", +] + +[[package]] +name = "ansi_term" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" +dependencies = [ + "winapi", +] + +[[package]] +name = "leaf" +version = "0.1.0" +dependencies = [ + "ansi_term 0.12.1", + "root", +] + +[[package]] +name = "root" +version = "0.1.0" +dependencies = [ + "ansi_term 0.11.0", + "leaf", +] + +[[package]] +name = "winapi" +version = "0.3.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8093091eeb260906a183e6ae1abdba2ef5ef2257a21801128899c3fc699229c6" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" diff --git a/tests/test_data/cyclic_dependencies/Cargo.toml b/tests/test_data/cyclic_dependencies/Cargo.toml new file mode 100644 index 00000000..8ce97027 --- /dev/null +++ b/tests/test_data/cyclic_dependencies/Cargo.toml @@ -0,0 +1,2 @@ +[workspace] +members = ["root", "leaf"] diff --git a/tests/test_data/cyclic_dependencies/deny.toml b/tests/test_data/cyclic_dependencies/deny.toml new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_data/cyclic_dependencies/leaf/Cargo.toml b/tests/test_data/cyclic_dependencies/leaf/Cargo.toml new file mode 100644 index 00000000..23b2e088 --- /dev/null +++ b/tests/test_data/cyclic_dependencies/leaf/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "leaf" +version = "0.1.0" +authors = ["veetaha "] +edition = "2018" + +[dev-dependencies] +root = { path = "../root" } +ansi_term = "0.12.1" diff --git a/tests/test_data/cyclic_dependencies/leaf/src/lib.rs b/tests/test_data/cyclic_dependencies/leaf/src/lib.rs new file mode 100644 index 00000000..3eec4e34 --- /dev/null +++ b/tests/test_data/cyclic_dependencies/leaf/src/lib.rs @@ -0,0 +1 @@ +pub fn leaf() {} diff --git a/tests/test_data/cyclic_dependencies/root/Cargo.toml b/tests/test_data/cyclic_dependencies/root/Cargo.toml new file mode 100644 index 00000000..882d006a --- /dev/null +++ b/tests/test_data/cyclic_dependencies/root/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "root" +version = "0.1.0" +authors = ["veetaha "] +edition = "2018" + +[dependencies] +ansi_term = "0.11.0" +leaf = { path = "../leaf" } diff --git a/tests/test_data/cyclic_dependencies/root/src/lib.rs b/tests/test_data/cyclic_dependencies/root/src/lib.rs new file mode 100644 index 00000000..38917767 --- /dev/null +++ b/tests/test_data/cyclic_dependencies/root/src/lib.rs @@ -0,0 +1 @@ +pub fn root() {} diff --git a/tests/test_data/cyclic_dependencies/root/src/main.rs b/tests/test_data/cyclic_dependencies/root/src/main.rs new file mode 100644 index 00000000..f328e4d9 --- /dev/null +++ b/tests/test_data/cyclic_dependencies/root/src/main.rs @@ -0,0 +1 @@ +fn main() {} From a00e135cce222f3bca4410b01e279906d5a72836 Mon Sep 17 00:00:00 2001 From: veetaha Date: Fri, 1 May 2020 23:20:04 +0300 Subject: [PATCH 2/2] Fix ininite loop in create_graph when there are cyclic dependencies --- src/bans/graph.rs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/bans/graph.rs b/src/bans/graph.rs index 3db66c61..ad1e2a9f 100644 --- a/src/bans/graph.rs +++ b/src/bans/graph.rs @@ -3,7 +3,10 @@ use crate::{DepKind, Kid, Krate}; use anyhow::{Context, Error}; use krates::petgraph as pg; use semver::Version; -use std::{collections::HashMap, fmt}; +use std::{ + collections::{hash_map::Entry, HashMap, HashSet}, + fmt, +}; #[derive(Hash, Copy, Clone, PartialEq, Eq)] struct Node<'a> { @@ -201,7 +204,7 @@ pub(crate) fn create_graph( // them stand out more in the graph for id in &duplicates { let dup_node = node_map[id]; - let mut set = std::collections::HashSet::new(); + let mut set = HashSet::new(); node_stack.push(dup_node); @@ -210,21 +213,22 @@ pub(crate) fn create_graph( let mut iditer = node.repr.splitn(3, ' '); let name = iditer.next().unwrap(); - match dupe_nodes.get_mut(name) { - Some(v) => { - if !v.contains(&nid) { - v.push(nid); + match dupe_nodes.entry(name) { + Entry::Occupied(it) => { + let it = it.into_mut(); + if !it.contains(&nid) { + it.push(nid); } } - None => { - dupe_nodes.insert(name, vec![nid]); + Entry::Vacant(it) => { + it.insert(vec![nid]); } } for edge in graph.edges_directed(nid, pg::Direction::Incoming) { - set.insert(edge.id()); - - node_stack.push(edge.source()); + if set.insert(edge.id()) { + node_stack.push(edge.source()) + } } }