Skip to content

Commit

Permalink
Merge pull request #41 from anderejd/scan_print_separation
Browse files Browse the repository at this point in the history 
Scan & print separation
  • Loading branch information
@anderejd
anderejd authored Jan 22, 2019
2 parents 32825f6 + bd43d71 commit 987851a
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 86 deletions.
150 changes: 88 additions & 62 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use cargo::util::paths;
use cargo::util::ProcessBuilder;
use cargo::util::{self, important_paths, CargoResult, Cfg};
use cargo::Config;
use colored::*;
use colored::Colorize;
use petgraph::graph::NodeIndex;
use petgraph::visit::EdgeRef;
use petgraph::EdgeDirection;
Expand Down Expand Up @@ -260,6 +260,11 @@ impl<'ast> visit::Visit<'ast> for GeigerSynVisitor {
.count(i.sig.unsafety.is_some(), self.used_by_build);
visit::visit_impl_item_method(self, i);
}

// TODO: Visit macros.
//
// TODO: Figure out if there are other visit methods that should be
// implemented here.
}

pub fn is_file_with_ext(entry: &DirEntry, file_ext: &str) -> bool {
Expand All @@ -277,26 +282,33 @@ pub fn is_file_with_ext(entry: &DirEntry, file_ext: &str) -> bool {
ext.to_string_lossy() == file_ext
}

pub fn find_unsafe(
p: &Path,
fn find_rs_files_in_dir(dir: &Path) -> impl Iterator<Item = PathBuf> {
let walker = WalkDir::new(dir).into_iter();
walker.filter_map(|entry| {
let entry = entry.expect("walkdir error."); // TODO: Return result.
if !is_file_with_ext(&entry, "rs") {
return None;
}
Some(
entry
.path()
.canonicalize()
.expect("Error converting to canonical path"),
) // TODO: Return result.
})
}

fn find_unsafe(
dir: &Path,
rs_files_used: &mut HashMap<PathBuf, u32>,
allow_partial_results: bool,
include_tests: IncludeTests,
verbosity: Verbosity,
) -> CounterBlock {
let mut vis = GeigerSynVisitor::new(include_tests, verbosity);
let walker = WalkDir::new(p).into_iter();
for entry in walker {
let entry =
entry.expect("walkdir error, TODO: Implement error handling");
if !is_file_with_ext(&entry, "rs") {
continue;
}
let pb = entry
.path()
.canonicalize()
.expect("Error converting to canonical path");
let p = pb.as_path();
let file_paths = find_rs_files_in_dir(dir);
for p in file_paths {
let p = p.as_path();
let scan_counter = rs_files_used.get_mut(p);
vis.used_by_build = match scan_counter {
Some(c) => {
Expand Down Expand Up @@ -441,12 +453,16 @@ impl From<PoisonError<CustomExecutorInnerContext>> for RsResolveError {
}
}

/// Trigger a `cargo clean` + `cargo check` and listen to the cargo/rustc
/// communication to figure out which source files were used by the build.
pub fn resolve_rs_file_deps(
copt: &CompileOptions,
ws: &Workspace,
) -> Result<HashMap<PathBuf, u32>, RsResolveError> {
let config = ws.config();
// Need to run a cargo clean to identify all new .d deps files.
// TODO: Figure out how this can be avoided to improve performance, clean
// Rust builds are __slow__.
let clean_opt = CleanOptions {
config: &config,
spec: vec![],
Expand Down Expand Up @@ -713,16 +729,13 @@ pub fn resolve<'a, 'cfg>(
) -> CargoResult<(PackageSet<'a>, Resolve)> {
let features =
Method::split_features(&features.into_iter().collect::<Vec<_>>());

let (packages, resolve) = ops::resolve_ws(ws)?;

let method = Method::Required {
dev_deps: true,
features: &features,
all_features,
uses_default_features: !no_default_features,
};

let resolve = ops::resolve_with_previous(
registry,
ws,
Expand Down Expand Up @@ -806,21 +819,53 @@ pub fn build_graph<'a>(
Ok(graph)
}

pub fn find_unsafe_in_packages(
packs: &PackageSet,
mut rs_files_used: HashMap<PathBuf, u32>,
allow_partial_results: bool,
include_tests: IncludeTests,
verbosity: Verbosity,
) -> GeigerContext {
let mut pack_id_to_counters = HashMap::new();
let pack_ids = packs.package_ids().cloned().collect::<Vec<_>>();
for id in pack_ids {
let pack = packs.get_one(&id).unwrap(); // FIXME
let counters = find_unsafe(
pack.root(),
&mut rs_files_used,
allow_partial_results,
include_tests,
verbosity,
);
pack_id_to_counters.insert(id, counters);
}
GeigerContext {
pack_id_to_counters,
rs_files_used,
}
}

/// TODO: Write documentation.
pub struct GeigerContext {
pub pack_id_to_counters: HashMap<PackageId, CounterBlock>,
pub rs_files_used: HashMap<PathBuf, u32>,
}

pub fn print_tree<'a>(
package: &'a PackageId,
root_pack_id: &'a PackageId,
graph: &Graph<'a>,
rs_files_used: &mut HashMap<PathBuf, u32>,
geiger_ctx: &GeigerContext,
pc: &PrintConfig,
) {
let mut visited_deps = HashSet::new();
let mut levels_continue = vec![];
let node = &graph.graph[graph.nodes[&package]];
let node = &graph.graph[graph.nodes[&root_pack_id]];
print_dependency(
node,
&graph,
&mut visited_deps,
&mut levels_continue,
rs_files_used,
geiger_ctx,
pc,
);
}
Expand All @@ -830,7 +875,7 @@ pub fn print_dependency<'a>(
graph: &Graph<'a>,
visited_deps: &mut HashSet<&'a PackageId>,
levels_continue: &mut Vec<bool>,
rs_files_used: &mut HashMap<PathBuf, u32>,
geiger_ctx: &GeigerContext,
pc: &PrintConfig,
) {
let new = pc.all || visited_deps.insert(package.id);
Expand All @@ -856,15 +901,7 @@ pub fn print_dependency<'a>(
Prefix::None => "".into(),
};

// TODO: Find and collect unsafe stats as a separate pass over the deps
// tree before printing.
let counters = find_unsafe(
package.pack.root(),
rs_files_used,
pc.allow_partial_results,
pc.include_tests,
pc.verbosity,
);
let counters = geiger_ctx.pack_id_to_counters.get(package.id).unwrap(); // TODO: Proper error handling.
let unsafe_found = counters.has_unsafe();
let colorize = |s: String| {
if unsafe_found {
Expand Down Expand Up @@ -903,42 +940,31 @@ pub fn print_dependency<'a>(
Kind::Development => development.push(dep),
}
}
print_dependency_kind(
Kind::Normal,
normal,
graph,
visited_deps,
levels_continue,
rs_files_used,
pc,
);
print_dependency_kind(
Kind::Build,
build,
graph,
visited_deps,
levels_continue,
rs_files_used,
pc,
);
print_dependency_kind(
Kind::Development,
development,
graph,
visited_deps,
levels_continue,
rs_files_used,
pc,
);
let mut kinds = [
(Kind::Normal, normal),
(Kind::Build, build),
(Kind::Development, development),
];
for (kind, kind_deps) in kinds.iter_mut() {
print_dependency_kind(
*kind,
kind_deps,
graph,
visited_deps,
levels_continue,
geiger_ctx,
pc,
);
}
}

pub fn print_dependency_kind<'a>(
kind: Kind,
mut deps: Vec<&Node<'a>>,
deps: &mut Vec<&Node<'a>>,
graph: &Graph<'a>,
visited_deps: &mut HashSet<&'a PackageId>,
levels_continue: &mut Vec<bool>,
rs_files_used: &mut HashMap<PathBuf, u32>,
geiger_ctx: &GeigerContext,
pc: &PrintConfig,
) {
if deps.is_empty() {
Expand Down Expand Up @@ -973,7 +999,7 @@ pub fn print_dependency_kind<'a>(
graph,
visited_deps,
levels_continue,
rs_files_used,
geiger_ctx,
pc,
);
levels_continue.pop();
Expand Down
78 changes: 54 additions & 24 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,25 @@ use cargo::core::resolver::Method;
use cargo::core::shell::Shell;
use cargo::core::shell::Verbosity;
use cargo::ops::CompileOptions;
use cargo::{CliResult, Config};
use cargo::CliResult;
use cargo::Config;
use cargo_geiger::build_graph;
use cargo_geiger::find_unsafe_in_packages;
use cargo_geiger::format::Pattern;
use cargo_geiger::*;
use colored::*;
use cargo_geiger::get_cfgs;
use cargo_geiger::print_tree;
use cargo_geiger::registry;
use cargo_geiger::resolve;
use cargo_geiger::resolve_rs_file_deps;
use cargo_geiger::workspace;
use cargo_geiger::Charset;
use cargo_geiger::IncludeTests;
use cargo_geiger::Prefix;
use cargo_geiger::PrintConfig;
use cargo_geiger::ASCII_SYMBOLS;
use cargo_geiger::UNSAFE_COUNTERS_HEADER;
use cargo_geiger::UTF8_SYMBOLS;
use colored::Colorize;
use petgraph::EdgeDirection;
use std::path::PathBuf;
use structopt::clap::AppSettings;
Expand Down Expand Up @@ -206,7 +221,7 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult {
let ids = packages.package_ids().cloned().collect::<Vec<_>>();
let packages = registry.get(&ids)?;

let root = match args.package {
let root_pack_id = match args.package {
Some(ref pkg) => resolve.query(pkg)?,
None => package.package_id(),
};
Expand Down Expand Up @@ -250,7 +265,7 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult {
};

let copt = build_compile_options(args, config);
let mut rs_files_used = resolve_rs_file_deps(&copt, &ws).unwrap();
let rs_files_used = resolve_rs_file_deps(&copt, &ws).unwrap();

if verbosity == Verbosity::Verbose {
// Print all .rs files found through the .d files, in sorted order.
Expand All @@ -264,22 +279,6 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult {
.for_each(|p| println!("Used by build (sorted): {}", p.display()));
}

println!();
println!("Metric output format: x/y");
println!("x = unsafe code used by the build");
println!("y = total unsafe code found in the crate");
println!();
println!(
"{}",
UNSAFE_COUNTERS_HEADER
.iter()
.map(|s| s.to_owned())
.collect::<Vec<_>>()
.join(" ")
.bold()
);
println!();

// TODO: Add command line flag for this and make it default to false?
let allow_partial_results = true;

Expand All @@ -298,16 +297,47 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult {
allow_partial_results,
include_tests,
};
print_tree(root, &graph, &mut rs_files_used, &pc);
rs_files_used

eprintln!("Scanning...");
let geiger_ctx = find_unsafe_in_packages(
&packages,
rs_files_used,
pc.allow_partial_results,
pc.include_tests,
pc.verbosity,
);
eprintln!("Scanning...Done.");

println!();
println!("Metric output format: x/y");
println!("x = unsafe code used by the build");
println!("y = total unsafe code found in the crate");
println!();
println!(
"{}",
UNSAFE_COUNTERS_HEADER
.iter()
.map(|s| s.to_owned())
.collect::<Vec<_>>()
.join(" ")
.bold()
);
println!();
print_tree(root_pack_id, &graph, &geiger_ctx, &pc);

geiger_ctx
.rs_files_used
.iter()
.filter(|(_k, v)| **v == 0)
.for_each(|(k, _v)| {
println!(
// This is likely a bug in cargo-geiger if it happens. Fail hard and
// exit instead?
eprintln!(
"WARNING: Dependency file was never scanned: {}",
k.display()
)
});

Ok(())
}

Expand Down

0 comments on commit 987851a

Please sign in to comment.