From 5263f6ca9faedfe5567cbc11f8076cf5ee995b37 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Thu, 15 Jun 2023 11:34:53 -0700 Subject: [PATCH] add a rudimentary '-d explain' tool Would have helped with #62. --- src/graph.rs | 8 ++------ src/progress.rs | 8 ++++++++ src/run.rs | 21 ++++++++++++--------- src/work.rs | 50 ++++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/graph.rs b/src/graph.rs index 506023d..746bbb6 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -425,12 +425,8 @@ impl Hashes { self.0.insert(id, hash); } - pub fn changed(&self, id: BuildId, hash: Hash) -> bool { - let last_hash = match self.0.get(&id) { - None => return true, - Some(h) => *h, - }; - hash != last_hash + pub fn get(&self, id: BuildId) -> Option { + self.0.get(&id).copied() } } diff --git a/src/progress.rs b/src/progress.rs index 23ec48e..127fa30 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -29,6 +29,9 @@ pub trait Progress { /// Called when a task starts or completes. fn task_state(&mut self, id: BuildId, build: &Build, result: Option<&TaskResult>); + /// Log some (debug) information, without corrupting the progress display. + fn log(&mut self, msg: String); + /// Called when the overall build has completed (success or failure), to allow /// cleaning up the display. fn finish(&mut self); @@ -104,6 +107,11 @@ impl Progress for ConsoleProgress { self.maybe_print_progress(); } + fn log(&mut self, msg: String) { + self.clear_progress(); + println!("{}", msg); + } + fn flush(&mut self) { self.print_progress(); } diff --git a/src/run.rs b/src/run.rs index dabcc5b..473355c 100644 --- a/src/run.rs +++ b/src/run.rs @@ -113,6 +113,15 @@ fn run_impl() -> anyhow::Result { let args: Args = argh::from_env(); + let mut options = work::Options { + parallelism: match args.parallelism { + Some(p) => p, + None => default_parallelism()?, + }, + keep_going: args.keep_going, + explain: false, + }; + if fake_ninja_compat && args.version { println!("1.10.2"); return Ok(0); @@ -120,9 +129,11 @@ fn run_impl() -> anyhow::Result { if let Some(debug) = args.debug { match debug.as_str() { + "explain" => options.explain = true, "list" => { println!("debug tools:"); - println!(" trace generate json performance trace"); + println!(" explain print why each target is considered out of date"); + println!(" trace generate json performance trace"); return Ok(1); } "trace" => trace::open("trace.json")?, @@ -151,14 +162,6 @@ fn run_impl() -> anyhow::Result { std::env::set_current_dir(dir).map_err(|err| anyhow!("chdir {:?}: {}", dir, err))?; } - let options = work::Options { - parallelism: match args.parallelism { - Some(p) => p, - None => default_parallelism()?, - }, - keep_going: args.keep_going, - }; - let mut progress: ConsoleProgress = ConsoleProgress::new(args.verbose, terminal::use_fancy()); match build(options, args.build_file, args.targets, &mut progress)? { None => { diff --git a/src/work.rs b/src/work.rs index 844495d..872181e 100644 --- a/src/work.rs +++ b/src/work.rs @@ -285,6 +285,8 @@ impl BuildStates { pub struct Options { pub keep_going: usize, pub parallelism: usize, + /// When true, verbosely explain why targets are considered dirty. + pub explain: bool, } pub struct Work<'a> { @@ -292,6 +294,7 @@ pub struct Work<'a> { db: db::Writer, progress: &'a mut dyn Progress, keep_going: usize, + explain: bool, file_state: FileState, last_hashes: Hashes, build_states: BuildStates, @@ -314,6 +317,7 @@ impl<'a> Work<'a> { db, progress, keep_going: options.keep_going, + explain: options.explain, file_state, last_hashes, build_states: BuildStates::new(build_count, pools), @@ -484,7 +488,7 @@ impl<'a> Work<'a> { /// Returns a build error if any required input files are missing. /// Otherwise returns true if any expected but not required files, /// e.g. outputs, are missing, implying that the build needs to be executed. - fn check_build_files_missing(&mut self, id: BuildId) -> anyhow::Result { + fn check_build_files_missing(&mut self, id: BuildId) -> anyhow::Result> { { let build = self.graph.build(id); let phony = build.cmdline.is_none(); @@ -554,8 +558,9 @@ impl<'a> Work<'a> { // For discovered_ins, ensure we have mtimes for them. // But if they're missing, it isn't an error, it just means the build // is dirty. - if self.ensure_discovered_stats(id)?.is_some() { - return Ok(true); + let missing = self.ensure_discovered_stats(id)?; + if missing.is_some() { + return Ok(missing); } // Stat all the outputs. @@ -570,12 +575,12 @@ impl<'a> Work<'a> { } let mtime = self.file_state.restat(id, file.path())?; if mtime == MTime::Missing { - return Ok(true); + return Ok(Some(id)); } } // All files accounted for. - Ok(false) + Ok(None) } /// Check a ready build for whether it needs to run, returning true if so. @@ -593,15 +598,46 @@ impl<'a> Work<'a> { // If any files are missing, the build is dirty without needing // to consider hashes. - if file_missing { + if let Some(missing) = file_missing { + if self.explain { + self.progress.log(format!( + "explain: {}: input {} missing", + build.location, + self.graph.file(missing).name + )); + } return Ok(true); } // If we get here, all the relevant files are present and stat()ed, // so compare the hash against the last hash. + // TODO: skip this whole function if no previous hash is present. + // More complex than just moving this block up, because we currently + // assume that we've always checked inputs after we've run a build. + let prev_hash = match self.last_hashes.get(id) { + None => { + if self.explain { + self.progress.log(format!( + "explain: {}: no previous state known", + build.location + )); + } + return Ok(true); + } + Some(prev_hash) => prev_hash, + }; + let hash = hash_build(&self.graph, &mut self.file_state, build)?; - Ok(self.last_hashes.changed(id, hash)) + if prev_hash != hash { + if self.explain { + self.progress + .log(format!("explain: {}: input changed", build.location)); + } + return Ok(true); + } + + Ok(false) } /// Create the parent directories of a given list of fileids.