From b30e701c7e0d4ded6ca4cf7b84073110711dba1f Mon Sep 17 00:00:00 2001 From: Melody Horn Date: Wed, 14 Apr 2021 17:53:37 -0600 Subject: give targets an encapsulated set like macros have --- src/makefile/input.rs | 72 ++++++++++++++++++++++++++------------------ src/makefile/mod.rs | 51 ++++++++++++------------------- src/makefile/target.rs | 81 ++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 141 insertions(+), 63 deletions(-) diff --git a/src/makefile/input.rs b/src/makefile/input.rs index 0e0506a..e2add0c 100644 --- a/src/makefile/input.rs +++ b/src/makefile/input.rs @@ -19,7 +19,7 @@ use super::inference_rules::InferenceRule; #[cfg(feature = "full")] use super::r#macro::ExportConfig; use super::r#macro::{Macro, Set as MacroSet}; -use super::target::Target; +use super::target::{StaticTargetSet, Target}; use super::token::{tokenize, Token, TokenString}; use super::ItemSource; @@ -166,7 +166,7 @@ pub struct MakefileReader<'a, 'parent, R: BufRead> { file_name: String, pub inference_rules: Vec, pub macros: MacroSet<'parent, 'static>, - pub targets: HashMap, + pub targets: StaticTargetSet, built_in_targets: HashMap, pub first_non_special_target: Option, pub failed_includes: Vec, @@ -222,7 +222,7 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> { file_name: name.clone(), inference_rules: Vec::new(), macros, - targets: HashMap::new(), + targets: Default::default(), built_in_targets: HashMap::new(), first_non_special_target: None, failed_includes: Vec::new(), @@ -675,7 +675,7 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> { .and_then(|x| x.get(1).map(|x| x.as_str().to_owned())), already_updated: Cell::new(false), }; - self.targets.insert(real_target.to_owned(), new_target); + self.targets.put(new_target); } } } else { @@ -728,34 +728,26 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> { }); self.inference_rules.push(new_rule); } else { + log::trace!( + "{}:{}: new target {:?} based on {:?}", + &self.file_name, + line_number, + &targets, + &prerequisites + ); for target in targets { if self.first_non_special_target.is_none() && !target.starts_with('.') { self.first_non_special_target = Some(target.into()); } // TODO handle appending to built-in (it's Complicated) - match self.targets.get_mut(target) { - Some(old_target) - if commands.is_empty() - && !(target == ".SUFFIXES" && prerequisites.is_empty()) => - { - let new_prerequisites = prerequisites - .iter() - .filter(|x| !old_target.prerequisites.contains(x)) - .cloned() - .collect::>(); - old_target.prerequisites.extend(new_prerequisites); - } - _ => { - let new_target = Target { - name: target.into(), - prerequisites: prerequisites.clone(), - commands: commands.clone(), - stem: None, - already_updated: Cell::new(false), - }; - self.targets.insert(target.into(), new_target); - } - } + let new_target = Target { + name: target.into(), + prerequisites: prerequisites.clone(), + commands: commands.clone(), + stem: None, + already_updated: Cell::new(false), + }; + self.targets.put(new_target); } } @@ -886,7 +878,7 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> { macros: self.macros.data, #[cfg(feature = "full")] macro_exports: self.macros.exported, - targets: self.targets, + targets: self.targets.into(), first_non_special_target: self.first_non_special_target, failed_includes: self.failed_includes, } @@ -899,7 +891,9 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> { #[cfg(feature = "full")] new.macro_exports, ); - self.targets.extend(new.targets); + for (_, target) in new.targets { + self.targets.put(target); + } if self.first_non_special_target.is_none() { self.first_non_special_target = new.first_non_special_target; } @@ -976,6 +970,7 @@ endif "; let args = Args::empty(); let makefile = MakefileReader::read(&args, MacroSet::new(), Cursor::new(file), "")?; + let makefile = makefile.finish(); assert_eq!(makefile.targets["a"].commands.len(), 1); Ok(()) } @@ -1055,6 +1050,7 @@ clean: let args = Args::empty(); let makefile = MakefileReader::read(&args, MacroSet::new(), Cursor::new(file), "")?; + let makefile = makefile.finish(); assert!(makefile.targets.contains_key("server")); Ok(()) } @@ -1135,4 +1131,22 @@ cursed: assert_eq!(makefile.inference_rules.len(), 2); Ok(()) } + + #[test] + fn dependency_prepending_appending() -> R { + let file = " +test: a +test: b +\techo hi +test: c + "; + let args = Args::empty(); + let makefile = MakefileReader::read(&args, MacroSet::new(), Cursor::new(file), "")?; + let makefile = makefile.finish(); + assert_eq!( + makefile.targets["test"].prerequisites, + vec!["a".to_owned(), "b".to_owned(), "c".to_owned()] + ); + Ok(()) + } } diff --git a/src/makefile/mod.rs b/src/makefile/mod.rs index 1d4873e..5c38ab3 100644 --- a/src/makefile/mod.rs +++ b/src/makefile/mod.rs @@ -1,5 +1,4 @@ use std::cell::{Cell, RefCell}; -use std::collections::HashMap; use std::env; use std::fmt; use std::path::{Path, PathBuf}; @@ -12,7 +11,7 @@ use inference_rules::InferenceRule; use input::FinishedMakefileReader; pub use input::MakefileReader; use r#macro::{Macro, Set as MacroSet}; -use target::Target; +use target::{DynamicTargetSet, Target}; use token::TokenString; use crate::args::Args; @@ -42,7 +41,7 @@ pub struct Makefile<'a> { inference_rules: Vec, builtin_inference_rules: Vec, pub macros: MacroSet<'static, 'static>, - targets: RefCell>>>, + targets: RefCell, pub first_non_special_target: Option, args: &'a Args, // TODO borrow warnings from Python version @@ -52,7 +51,7 @@ impl<'a> Makefile<'a> { pub fn new(args: &'a Args) -> Self { let mut inference_rules = vec![]; let mut macros = MacroSet::new(); - let mut targets = HashMap::new(); + let mut targets = DynamicTargetSet::default(); let first_non_special_target = None; macros.set( @@ -70,11 +69,9 @@ impl<'a> Makefile<'a> { if !args.no_builtin_rules { inference_rules.extend(builtin_inference_rules()); macros.add_builtins(); - targets.extend( - builtin_targets() - .into_iter() - .map(|t| (t.name.clone(), Rc::new(RefCell::new(t)))), - ); + for target in builtin_targets() { + targets.put(target); + } } macros.add_env(); @@ -136,11 +133,9 @@ impl<'a> Makefile<'a> { #[cfg(feature = "full")] new.macro_exports, ); - self.targets.borrow_mut().extend( - new.targets - .into_iter() - .map(|(k, v)| (k, Rc::new(RefCell::new(v)))), - ); + for (_, target) in new.targets { + self.targets.borrow_mut().put(target); + } if self.first_non_special_target.is_none() { self.first_non_special_target = new.first_non_special_target; } @@ -216,7 +211,7 @@ impl<'a> Makefile<'a> { // we can't build this based on itself! fuck outta here return None; } - if self.targets.borrow().contains_key(&prereq_path_name) { + if self.targets.borrow().has(&prereq_path_name) { return Some(prereq_path_name); } let prereq_path = PathBuf::from(&prereq_path_name); @@ -239,7 +234,7 @@ impl<'a> Makefile<'a> { self.infer_target(&prereq_path_name, banned_rules, banned_names) .ok() .and_then(|_| { - if self.targets.borrow().contains_key(&prereq_path_name) { + if self.targets.borrow().has(&prereq_path_name) { Some(prereq_path_name) } else { None @@ -264,9 +259,7 @@ impl<'a> Makefile<'a> { } if let Some(new_target) = new_target { - self.targets - .borrow_mut() - .insert(new_target.name.clone(), Rc::new(RefCell::new(new_target))); + self.targets.borrow_mut().put(new_target); } Ok(()) @@ -284,14 +277,14 @@ impl<'a> Makefile<'a> { } else { false }; - if !self.targets.borrow().contains_key(name) || exists_but_infer_anyway { + if !self.targets.borrow().has(name) || exists_but_infer_anyway { log::trace!("trying to infer for {}", name); self.infer_target(name, vec![], vec![])?; } let mut new_target = None; let targets = self.targets.borrow(); - if !targets.contains_key(name) { + if !targets.has(name) { // well, inference didn't work. is there a default? if let Some(default) = targets.get(".DEFAULT") { let commands = default.borrow().commands.clone(); @@ -318,17 +311,13 @@ impl<'a> Makefile<'a> { drop(targets); if let Some(new_target) = new_target { - self.targets - .borrow_mut() - .insert(new_target.name.clone(), Rc::new(RefCell::new(new_target))); + self.targets.borrow_mut().put(new_target); } let targets = self.targets.borrow(); - Ok(Rc::clone( - targets - .get(name) - .ok_or_else(|| eyre!("Target {:?} not found!", name))?, - )) + Ok(targets + .get(name) + .ok_or_else(|| eyre!("Target {:?} not found!", name))?) } pub fn update_target(&self, name: &str) -> Result<()> { @@ -427,9 +416,7 @@ impl fmt::Display for Makefile<'_> { writeln!(f)?; header(f, "Targets")?; - for target in self.targets.borrow().values() { - writeln!(f, "{}", target.borrow())?; - } + writeln!(f, "{}", &self.targets.borrow())?; Ok(()) } diff --git a/src/makefile/target.rs b/src/makefile/target.rs index 506bde8..5de280d 100644 --- a/src/makefile/target.rs +++ b/src/makefile/target.rs @@ -1,6 +1,8 @@ -use std::cell::Cell; +use std::cell::{Cell, RefCell}; +use std::collections::HashMap; use std::fmt; use std::fs::metadata; +use std::rc::Rc; use std::time::SystemTime; use eyre::{Result, WrapErr}; @@ -19,6 +21,17 @@ pub struct Target { } impl Target { + pub fn extend(&mut self, other: Target) { + assert_eq!(&self.name, &other.name); + self.prerequisites.extend(other.prerequisites); + assert!(self.commands.is_empty() || other.commands.is_empty()); + self.commands.extend(other.commands); + assert!(self.stem.is_none() || other.stem.is_none()); + self.stem = self.stem.take().or(other.stem); + let already_updated = self.already_updated.get() || other.already_updated.get(); + self.already_updated.set(already_updated); + } + fn modified_time(&self) -> Option { metadata(&self.name) .and_then(|metadata| metadata.modified()) @@ -55,7 +68,7 @@ impl Target { .unwrap_or(false) }); log::trace!( - "{:>50} exists: {}, newer than dependencies: {}", + "{:} exists: {}, newer than dependencies: {}", self.name, exists, newer_than_all_dependencies @@ -64,6 +77,7 @@ impl Target { } pub fn update(&self, file: &Makefile) -> Result<()> { + log::info!("{}: {:?}", &self.name, &self.prerequisites); for prereq in &self.prerequisites { file.update_target(prereq) .wrap_err_with(|| format!("as a dependency for target {}", self.name))?; @@ -101,3 +115,66 @@ impl fmt::Display for Target { Ok(()) } } + +// for targets that won't change out from under us (don't need refcelling) +#[derive(Clone, Default)] +pub struct StaticTargetSet { + data: HashMap, +} + +impl StaticTargetSet { + pub fn get(&self, name: &str) -> Option<&Target> { + self.data.get(name) + } + + pub fn put(&mut self, target: Target) { + if target.name == ".SUFFIXES" && target.prerequisites.is_empty() { + self.data.remove(&target.name); + } + if let Some(existing_target) = self.data.get_mut(&target.name) { + existing_target.extend(target); + } else { + self.data.insert(target.name.clone(), target); + } + } +} + +impl Into> for StaticTargetSet { + fn into(self) -> HashMap { + self.data + } +} + +// for targets that might become updated and so need refcelling +#[derive(Clone, Default)] +pub struct DynamicTargetSet { + data: HashMap>>, +} + +impl DynamicTargetSet { + pub fn get(&self, name: &str) -> Option>> { + self.data.get(name).map(|x| Rc::clone(x)) + } + + pub fn put(&mut self, target: Target) { + if let Some(existing_target) = self.data.get_mut(&target.name) { + existing_target.borrow_mut().extend(target); + } else { + self.data + .insert(target.name.clone(), Rc::new(RefCell::new(target))); + } + } + + pub fn has(&self, name: &str) -> bool { + self.data.contains_key(name) + } +} + +impl fmt::Display for DynamicTargetSet { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + for target in self.data.values() { + writeln!(f, "{}", target.borrow())?; + } + Ok(()) + } +} -- cgit v1.2.3