From e1a0584936b3aa5ce971e875dec750d2ae937d2e Mon Sep 17 00:00:00 2001 From: Melody Horn Date: Wed, 31 Mar 2021 12:51:11 -0600 Subject: massively upgrade error handling --- .clippy.toml | 6 ++ Cargo.lock | 7 +++ Cargo.toml | 1 + src/main.rs | 36 ++++++++---- src/makefile/command_line.rs | 8 ++- src/makefile/conditional.rs | 55 ++++++++++-------- src/makefile/functions.rs | 84 +++++++++++++++------------ src/makefile/macro.rs | 31 +++++----- src/makefile/mod.rs | 133 ++++++++++++++++++++++++++----------------- src/makefile/pattern.rs | 40 ++++++------- src/makefile/target.rs | 18 ++++-- src/makefile/token.rs | 16 +++--- 12 files changed, 260 insertions(+), 175 deletions(-) create mode 100644 .clippy.toml diff --git a/.clippy.toml b/.clippy.toml new file mode 100644 index 0000000..ffa12fc --- /dev/null +++ b/.clippy.toml @@ -0,0 +1,6 @@ +disallowed-methods = [ + "core::option::Option::unwrap", + "core::option::Option::expect", + "core::result::Result::unwrap", + "core::result::Result::expect", +] diff --git a/Cargo.lock b/Cargo.lock index 925d565..c8f9b6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -18,6 +18,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "anyhow" +version = "1.0.40" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28b2cd92db5cbd74e8e5028f7e27dd7aa3090e89e4f2a197cc7c8dfb69c7063b" + [[package]] name = "arrayref" version = "0.3.6" @@ -221,6 +227,7 @@ checksum = "8916b1f6ca17130ec6568feccee27c156ad12037880833a3b842a823236502e7" name = "makers" version = "0.1.0" dependencies = [ + "anyhow", "dirs", "glob", "lazy_static", diff --git a/Cargo.toml b/Cargo.toml index 1e91e78..6cbf820 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ keywords = ["build", "make"] categories = ["development-tools"] [dependencies] +anyhow = "1.0.40" dirs = "3.0.1" glob = "0.3.0" lazy_static = "1.4.0" diff --git a/src/main.rs b/src/main.rs index 1c2faf5..d560048 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,4 @@ #![warn( - missing_docs, unreachable_pub, unsafe_code, unused_crate_dependencies, @@ -10,17 +9,20 @@ )] #![allow(clippy::redundant_pub_crate, clippy::non_ascii_literal)] +use std::env::current_dir; use std::fs::metadata; use std::io::stdin; use std::path::PathBuf; +use anyhow::bail; + mod args; mod makefile; use args::Args; use makefile::Makefile; -fn main() { +fn main() -> anyhow::Result<()> { let mut args = Args::from_env_and_args(); // If no makefile is specified, try some options. if args.makefile.is_empty() { @@ -29,8 +31,17 @@ fn main() { } else if metadata("./Makefile").is_ok() { "./Makefile".into() } else { - // TODO handle error gracefully - panic!("no makefile found"); + bail!( + "no makefile specified and neither {} nor {} was found.", + current_dir().map_or_else( + |_| "./makefile".to_string(), + |path| path.join("makefile").display().to_string() + ), + current_dir().map_or_else( + |_| "./Makefile".to_string(), + |path| path.join("Makefile").display().to_string() + ), + ); }]; } // Read in the makefile(s) specified. @@ -40,9 +51,9 @@ fn main() { let mut makefile = Makefile::new(&args); for filename in &args.makefile { if filename == &PathBuf::from("-") { - makefile.and_read(stdin().lock()); + makefile.and_read(stdin().lock())?; } else { - makefile.and_read_file(filename); + makefile.and_read_file(filename)?; }; } @@ -51,15 +62,18 @@ fn main() { } let targets = if args.targets().count() == 0 { - vec![makefile - .first_non_special_target - .clone() - .expect("couldn't find a target!")] + let first_target = makefile.first_non_special_target.clone(); + match first_target { + Some(x) => vec![x], + None => bail!("no targets given on command line or found in makefile."), + } } else { args.targets().cloned().collect() }; for target in targets { - makefile.update_target(&target); + makefile.update_target(&target)?; } + + Ok(()) } diff --git a/src/makefile/command_line.rs b/src/makefile/command_line.rs index 20b0d25..fd1ef97 100644 --- a/src/makefile/command_line.rs +++ b/src/makefile/command_line.rs @@ -69,7 +69,7 @@ impl CommandLine { } } - pub(crate) fn execute(&self, file: &Makefile, target: &Target) { + pub(crate) fn execute(&self, file: &Makefile, target: &Target) -> anyhow::Result<()> { let ignore_error = self.ignore_errors || file.args.ignore_errors || file.special_target_has_prereq(".IGNORE", &target.name); @@ -77,7 +77,7 @@ impl CommandLine { || file.args.silent || file.special_target_has_prereq(".SILENT", &target.name); - let execution_line = file.expand_macros(&self.execution_line, Some(target)); + let execution_line = file.expand_macros(&self.execution_line, Some(target))?; if !silent { println!("{}", execution_line); @@ -86,7 +86,7 @@ impl CommandLine { let should_execute = self.always_execute || !(file.args.dry_run || file.args.question || file.args.touch); if !should_execute { - return; + return Ok(()); } let return_value = execute_command_line(&execution_line, ignore_error); @@ -98,6 +98,8 @@ impl CommandLine { panic!("error from command execution!"); } } + + Ok(()) } } diff --git a/src/makefile/conditional.rs b/src/makefile/conditional.rs index f9d2d60..bf373bd 100644 --- a/src/makefile/conditional.rs +++ b/src/makefile/conditional.rs @@ -78,41 +78,50 @@ fn decode_condition_args(line_body: &str) -> Option<(TokenString, TokenString)> } impl ConditionalLine { - pub(crate) fn from(line: &str, expand_macro: impl Fn(&TokenString) -> String) -> Option { - if let Some(line) = line.strip_prefix("ifeq ") { - let (arg1, arg2) = decode_condition_args(line)?; - Some(Self::IfEqual(arg1, arg2)) + pub(crate) fn from( + line: &str, + expand_macro: impl Fn(&TokenString) -> anyhow::Result, + ) -> anyhow::Result> { + Ok(Some(if let Some(line) = line.strip_prefix("ifeq ") { + match decode_condition_args(line) { + Some((arg1, arg2)) => Self::IfEqual(arg1, arg2), + None => return Ok(None), + } } else if let Some(line) = line.strip_prefix("ifneq ") { - let (arg1, arg2) = decode_condition_args(line)?; - Some(Self::IfNotEqual(arg1, arg2)) + match decode_condition_args(line) { + Some((arg1, arg2)) => Self::IfNotEqual(arg1, arg2), + None => return Ok(None), + } } else if let Some(line) = line.strip_prefix("ifdef ") { - Some(Self::IfDefined(expand_macro(&line.parse().ok()?))) + Self::IfDefined(expand_macro(&line.parse()?)?) } else if let Some(line) = line.strip_prefix("ifndef ") { - Some(Self::IfNotDefined(expand_macro(&line.parse().ok()?))) + Self::IfNotDefined(expand_macro(&line.parse()?)?) } else if line == "else" { - Some(Self::Else) + Self::Else } else if let Some(line) = line.strip_prefix("else ") { - let sub_condition = Self::from(line, expand_macro)?; - Some(Self::ElseIf(Box::new(sub_condition))) + match Self::from(line, expand_macro)? { + Some(sub_condition) => Self::ElseIf(Box::new(sub_condition)), + None => return Ok(None), + } } else if line == "endif" { - Some(Self::EndIf) + Self::EndIf } else { - None - } + return Ok(None); + })) } pub(crate) fn action( &self, current_state: Option<&ConditionalState>, is_macro_defined: impl Fn(&str) -> bool, - expand_macro: impl Fn(&TokenString) -> String, - ) -> ConditionalStateAction { + expand_macro: impl Fn(&TokenString) -> anyhow::Result, + ) -> anyhow::Result { use ConditionalState as State; use ConditionalStateAction as Action; - match self { + Ok(match self { Self::IfEqual(arg1, arg2) => { - let arg1 = expand_macro(arg1); - let arg2 = expand_macro(arg2); + let arg1 = expand_macro(arg1)?; + let arg2 = expand_macro(arg2)?; if arg1 == arg2 { Action::Push(State::Executing) } else { @@ -120,8 +129,8 @@ impl ConditionalLine { } } Self::IfNotEqual(arg1, arg2) => { - let arg1 = expand_macro(arg1); - let arg2 = expand_macro(arg2); + let arg1 = expand_macro(arg1)?; + let arg2 = expand_macro(arg2)?; if arg1 == arg2 { Action::Push(State::SkippingUntilElseOrEndIf) } else { @@ -154,11 +163,11 @@ impl ConditionalLine { Action::Replace(State::SkippingUntilEndIf) } Some(State::SkippingUntilElseOrEndIf) => { - inner_condition.action(current_state, is_macro_defined, expand_macro) + inner_condition.action(current_state, is_macro_defined, expand_macro)? } None => panic!("got an ElseIf but not in a conditional"), }, Self::EndIf => Action::Pop, - } + }) } } diff --git a/src/makefile/functions.rs b/src/makefile/functions.rs index 61b9828..8087a94 100644 --- a/src/makefile/functions.rs +++ b/src/makefile/functions.rs @@ -2,7 +2,11 @@ use super::pattern::r#match; use super::r#macro::{MacroSet, MacroSource}; use super::token::TokenString; -pub(crate) fn expand_call(name: &str, args: &[TokenString], macros: &MacroSet) -> String { +pub(crate) fn expand_call( + name: &str, + args: &[TokenString], + macros: &MacroSet, +) -> anyhow::Result { match name { "filter" => { assert_eq!(args.len(), 2); @@ -63,50 +67,54 @@ mod text { use super::MacroSet; use super::TokenString; - pub(crate) fn filter(macros: &MacroSet, patterns: &TokenString, text: &TokenString) -> String { - let patterns = macros.expand(patterns); + pub(crate) fn filter( + macros: &MacroSet, + patterns: &TokenString, + text: &TokenString, + ) -> anyhow::Result { + let patterns = macros.expand(patterns)?; let patterns = patterns.split_whitespace().collect::>(); - let text = macros.expand(text); + let text = macros.expand(text)?; let text = text.split_whitespace(); let mut result_pieces = vec![]; for word in text { if patterns .iter() - .any(|pattern| r#match(pattern, word).is_some()) + .any(|pattern| r#match(pattern, word).map_or(false, |x| x.is_some())) { result_pieces.push(word); } } - result_pieces.join(" ") + Ok(result_pieces.join(" ")) } pub(crate) fn filter_out( macros: &MacroSet, patterns: &TokenString, text: &TokenString, - ) -> String { - let patterns = macros.expand(patterns); + ) -> anyhow::Result { + let patterns = macros.expand(patterns)?; let patterns = patterns.split_whitespace().collect::>(); - let text = macros.expand(text); + let text = macros.expand(text)?; let text = text.split_whitespace(); let mut result_pieces = vec![]; for word in text { if patterns .iter() - .all(|pattern| r#match(pattern, word).is_none()) + .all(|pattern| r#match(pattern, word).map_or(false, |x| x.is_none())) { result_pieces.push(word); } } - result_pieces.join(" ") + Ok(result_pieces.join(" ")) } - pub(crate) fn sort(macros: &MacroSet, words: &TokenString) -> String { - let words = macros.expand(words); + pub(crate) fn sort(macros: &MacroSet, words: &TokenString) -> anyhow::Result { + let words = macros.expand(words)?; let mut words = words.split_whitespace().collect::>(); words.sort_unstable(); words.dedup(); - words.join(" ") + Ok(words.join(" ")) } } @@ -116,11 +124,13 @@ mod file_name { use std::ffi::OsStr; use std::path::Path; + use anyhow::Context; + use super::MacroSet; use super::TokenString; - pub(crate) fn notdir(macros: &MacroSet, words: &TokenString) -> String { - let words = macros.expand(words); + pub(crate) fn notdir(macros: &MacroSet, words: &TokenString) -> anyhow::Result { + let words = macros.expand(words)?; let words = words .split_whitespace() .map(|word| { @@ -130,11 +140,11 @@ mod file_name { .unwrap_or("") }) .collect::>(); - words.join(" ") + Ok(words.join(" ")) } - pub(crate) fn basename(macros: &MacroSet, words: &TokenString) -> String { - let words = macros.expand(words); + pub(crate) fn basename(macros: &MacroSet, words: &TokenString) -> anyhow::Result { + let words = macros.expand(words)?; let words = words .split_whitespace() .map(|word| { @@ -144,25 +154,25 @@ mod file_name { .map_or_else(String::new, ToString::to_string) }) .collect::>(); - words.join(" ") + Ok(words.join(" ")) } pub(crate) fn addprefix( macros: &MacroSet, prefix: &TokenString, targets: &TokenString, - ) -> String { - let prefix = macros.expand(prefix); - let targets = macros.expand(targets); + ) -> anyhow::Result { + let prefix = macros.expand(prefix)?; + let targets = macros.expand(targets)?; let results = targets .split_whitespace() .map(|t| format!("{}{}", prefix, t)) .collect::>(); - results.join(" ") + Ok(results.join(" ")) } - pub(crate) fn wildcard(macros: &MacroSet, pattern: &TokenString) -> String { - let pattern = macros.expand(pattern); + pub(crate) fn wildcard(macros: &MacroSet, pattern: &TokenString) -> anyhow::Result { + let pattern = macros.expand(pattern)?; let home_dir = env::var("HOME") .ok() .or_else(|| dirs::home_dir().and_then(|p| p.to_str().map(String::from))); @@ -172,13 +182,13 @@ mod file_name { pattern }; let results = glob::glob(&pattern) - .expect("invalid glob pattern!") + .context("invalid glob pattern!")? .filter_map(|path| { path.ok() .map(|x| x.to_str().map(ToString::to_string).unwrap_or_default()) }) .collect::>(); - results.join(" ") + Ok(results.join(" ")) } } @@ -193,9 +203,9 @@ mod foreach { var: &TokenString, list: &TokenString, text: &TokenString, - ) -> String { - let var = macros.expand(var); - let list = macros.expand(list); + ) -> anyhow::Result { + let var = macros.expand(var)?; + let list = macros.expand(list)?; let words = list.split_whitespace(); let mut macros = macros.with_overlay(); @@ -204,8 +214,8 @@ mod foreach { macros.set(var.clone(), MacroSource::File, TokenString::text(word)); macros.expand(text) }) - .collect::>(); - results.join(" ") + .collect::, _>>()?; + Ok(results.join(" ")) } } @@ -218,8 +228,10 @@ mod call { pub(crate) fn call<'a>( macros: &MacroSet, args: impl Iterator, - ) -> String { - let args = args.map(|arg| macros.expand(arg)).collect::>(); + ) -> anyhow::Result { + let args = args + .map(|arg| macros.expand(arg)) + .collect::, _>>()?; let function = args[0].clone(); let mut macros = macros.with_overlay(); @@ -236,7 +248,7 @@ mod test { use crate::makefile::r#macro::{MacroSet, MacroSource}; - fn call(name: &str, args: &[TokenString], macros: &MacroSet) -> String { + fn call(name: &str, args: &[TokenString], macros: &MacroSet) -> anyhow::Result { super::expand_call(name, args, macros) } diff --git a/src/makefile/macro.rs b/src/makefile/macro.rs index 79df339..88397c5 100644 --- a/src/makefile/macro.rs +++ b/src/makefile/macro.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::env; use std::fmt; +use anyhow::Context; use regex::Regex; use super::functions; @@ -15,9 +16,9 @@ pub(crate) enum MacroSource { Builtin, } -pub(crate) trait LookupInternal: for<'a> Fn(&'a str) -> String {} +pub(crate) trait LookupInternal: for<'a> Fn(&'a str) -> anyhow::Result {} -impl Fn(&'a str) -> String> LookupInternal for F {} +impl Fn(&'a str) -> anyhow::Result> LookupInternal for F {} #[derive(Clone)] pub(crate) struct MacroSet<'parent, 'lookup> { @@ -50,13 +51,13 @@ impl<'parent, 'lookup> MacroSet<'parent, 'lookup> { } } - fn lookup_internal(&self, name: &str) -> String { + fn lookup_internal(&self, name: &str) -> anyhow::Result { if let Some(lookup) = self.lookup_internal { lookup(name) } else if let Some(parent) = self.parent { parent.lookup_internal(name) } else { - panic!("no lookup possible"); + anyhow::bail!("no lookup possible") } } @@ -79,7 +80,7 @@ impl<'parent, 'lookup> MacroSet<'parent, 'lookup> { self.data.remove(name) } - pub(crate) fn expand(&self, text: &TokenString) -> String { + pub(crate) fn expand(&self, text: &TokenString) -> anyhow::Result { let mut result = String::new(); for token in text.tokens() { match token { @@ -92,18 +93,20 @@ impl<'parent, 'lookup> MacroSet<'parent, 'lookup> { && name.starts_with(internal_macro_names) && name.ends_with(internal_macro_suffices); let macro_value = if just_internal || suffixed_internal { - self.lookup_internal(name) + self.lookup_internal(name)? } else { - self.get(name) - .map_or_else(String::new, |(_, macro_value)| self.expand(macro_value)) + self.get(name).map_or_else( + || Ok(String::new()), + |(_, macro_value)| self.expand(macro_value), + )? }; let macro_value = match replacement { Some((subst1, subst2)) => { - let subst1 = self.expand(subst1); + let subst1 = self.expand(subst1)?; let subst1_suffix = regex::escape(&subst1); - let subst1_suffix = - Regex::new(&format!(r"{}\b", subst1_suffix)).unwrap(); - let subst2 = self.expand(subst2); + let subst1_suffix = Regex::new(&format!(r"{}\b", subst1_suffix)) + .context("formed invalid regex somehow")?; + let subst2 = self.expand(subst2)?; subst1_suffix.replace_all(¯o_value, subst2).to_string() } None => macro_value, @@ -111,11 +114,11 @@ impl<'parent, 'lookup> MacroSet<'parent, 'lookup> { result.push_str(¯o_value); } Token::FunctionCall { name, args } => { - result.push_str(&functions::expand_call(name, args, self)); + result.push_str(&functions::expand_call(name, args, self)?); } } } - result + Ok(result) } pub(crate) fn with_lookup<'l, 's: 'l>( diff --git a/src/makefile/mod.rs b/src/makefile/mod.rs index 7c2663b..7077161 100644 --- a/src/makefile/mod.rs +++ b/src/makefile/mod.rs @@ -3,11 +3,12 @@ use std::collections::HashMap; use std::env; use std::fmt; use std::fs::File; -use std::io::{BufRead, BufReader, Error as IoError}; +use std::io::{BufRead, BufReader}; use std::path::Path; use std::rc::Rc; use crate::args::Args; +use anyhow::Context; use lazy_static::lazy_static; use regex::Regex; @@ -131,27 +132,36 @@ impl<'a> Makefile<'a> { } } - pub(crate) fn and_read_file(&mut self, path: impl AsRef) { + pub(crate) fn and_read_file(&mut self, path: impl AsRef) -> anyhow::Result<()> { let file = File::open(path); // TODO handle errors - let file = file.expect("couldn't open makefile!"); + let file = file.context("couldn't open makefile!")?; let file_reader = BufReader::new(file); - self.and_read(file_reader); + self.and_read(file_reader) } - pub(crate) fn and_read(&mut self, source: impl BufRead) { - let mut lines_iter = source.lines().enumerate().peekable(); + pub(crate) fn and_read(&mut self, source: impl BufRead) -> anyhow::Result<()> { + let mut lines_iter = source + .lines() + .enumerate() + .map(|(number, line)| (number + 1, line)) + .map(|(line, x)| { + ( + line, + x.with_context(|| format!("failed to read line {} of makefile", line)), + ) + }) + .peekable(); let mut conditional_stack: Vec = vec![]; while let Some((line_number, line)) = lines_iter.next() { - // TODO handle I/O errors at all - let mut line = line.expect("failed to read line of makefile!"); + let mut line = line?; // handle escaped newlines while line.ends_with('\\') { line.pop(); line.push(' '); if let Some((_, x)) = lines_iter.next() { - line.push_str(x.expect("failed to read line of makefile!").trim_start()) + line.push_str(x?.trim_start()) } } @@ -173,20 +183,21 @@ impl<'a> Makefile<'a> { if let Some(line) = line.strip_prefix("include ") { // remove extra leading space let line = line.trim_start(); - let line = self.expand_macros(&tokenize(line), None); + let line = self.expand_macros(&tokenize(line)?, None)?; let fields = line.split_whitespace(); // POSIX says we only have to handle a single filename, but GNU make // handles arbitrarily many filenames, and it's not like that's more work for field in fields { - self.and_read_file(field); + self.and_read_file(field)?; } - } else if let Some(line) = ConditionalLine::from(&line, |t| self.expand_macros(t, None)) + } else if let Some(line) = + ConditionalLine::from(&line, |t| self.expand_macros(t, None))? { line.action( conditional_stack.last(), |name| self.macros.is_defined(name), |t| self.expand_macros(t, None), - ) + )? .apply_to(&mut conditional_stack); } else if line.trim().is_empty() { // handle blank lines @@ -196,13 +207,15 @@ impl<'a> Makefile<'a> { // macro tokenizing. so that's suboptimal. // TODO errors - let line_tokens: TokenString = line.parse().unwrap(); + let line_tokens: TokenString = line + .parse() + .with_context(|| format!("failed to parse line {}", line_number))?; let line_type = LineType::of(&line_tokens); match line_type { - LineType::Rule => self.read_rule(&line_tokens, &mut lines_iter), - LineType::Macro => self.read_macro(&line_tokens), + LineType::Rule => self.read_rule(&line_tokens, line_number, &mut lines_iter)?, + LineType::Macro => self.read_macro(&line_tokens, line_number)?, LineType::Unknown => { panic!( "error: line {}: unknown line {:?}", @@ -212,28 +225,37 @@ impl<'a> Makefile<'a> { } } } + + Ok(()) } fn read_rule( &mut self, line_tokens: &TokenString, - lines_iter: &mut impl Iterator)>, - ) { + line_number: usize, + lines_iter: &mut impl Iterator)>, + ) -> anyhow::Result<()> { let mut lines_iter = lines_iter.peekable(); - let (targets, not_targets) = line_tokens.split_once(':').unwrap(); - let targets = self.expand_macros(&targets, None); + let (targets, not_targets) = line_tokens + .split_once(':') + .with_context(|| format!("read_rule couldn't find a ':' on line {}", line_number))?; + let targets = self.expand_macros(&targets, None)?; let targets = targets.split_whitespace().collect::>(); let (prerequisites, mut commands) = match not_targets.split_once(';') { Some((prerequisites, mut command)) => { - while command.ends_with("\\") && lines_iter.peek().is_some() { - command.strip_suffix("\\"); - command.extend(tokenize(&lines_iter.next().unwrap().1.unwrap())); + while command.ends_with("\\") { + if let Some((_, next_line)) = lines_iter.next() { + command.strip_suffix("\\"); + command.extend(tokenize(&next_line?)?); + } else { + break; + } } (prerequisites, vec![command]) } None => (not_targets, vec![]), }; - let prerequisites = self.expand_macros(&prerequisites, None); + let prerequisites = self.expand_macros(&prerequisites, None)?; let prerequisites = prerequisites .split_whitespace() .map(|x| x.into()) @@ -244,7 +266,7 @@ impl<'a> Makefile<'a> { .ok() .map_or(false, |line| line.starts_with('\t') || line.is_empty()) }) { - let mut line = x.unwrap(); + let mut line = x?; if !line.is_empty() { line.remove(0); } @@ -261,7 +283,10 @@ impl<'a> Makefile<'a> { _ => break, } } - commands.push(line.parse().unwrap()); + commands.push( + line.parse() + .with_context(|| format!("failed to parse line {}", line_number))?, + ); } let commands = commands @@ -270,7 +295,7 @@ impl<'a> Makefile<'a> { .collect::>(); if targets.is_empty() { - return; + return Ok(()); } // we don't know yet if it's a target rule or an inference rule @@ -319,11 +344,15 @@ impl<'a> Makefile<'a> { } } } + + Ok(()) } - fn read_macro(&mut self, line_tokens: &TokenString) { - let (name, mut value) = line_tokens.split_once('=').unwrap(); - let name = self.expand_macros(&name, None); + fn read_macro(&mut self, line_tokens: &TokenString, line_number: usize) -> anyhow::Result<()> { + let (name, mut value) = line_tokens + .split_once('=') + .with_context(|| format!("read_rule couldn't find a ':' on line {}", line_number))?; + let name = self.expand_macros(&name, None)?; // GNUisms are annoying, but popular let mut expand_value = false; let mut skip_if_defined = false; @@ -348,17 +377,17 @@ impl<'a> Makefile<'a> { value.trim_start(); let value = if expand_value { - TokenString::text(self.expand_macros(&value, None)) + TokenString::text(self.expand_macros(&value, None)?) } else { value }; match self.macros.get(name) { // We always let command line or MAKEFLAGS macros override macros from the file. - Some((MacroSource::CommandLineOrMakeflags, _)) => return, + Some((MacroSource::CommandLineOrMakeflags, _)) => return Ok(()), // We let environment variables override macros from the file only if the command-line argument to do that was given - Some((MacroSource::Environment, _)) if self.args.environment_overrides => return, - _ if skip_if_defined => return, + Some((MacroSource::Environment, _)) if self.args.environment_overrides => return Ok(()), + _ if skip_if_defined => return Ok(()), _ => {} } @@ -372,6 +401,8 @@ impl<'a> Makefile<'a> { _ => value, }; self.macros.set(name.into(), MacroSource::File, value); + + Ok(()) } fn special_target_has_prereq(&self, target: &str, name: &str) -> bool { @@ -385,13 +416,13 @@ impl<'a> Makefile<'a> { } } - pub(crate) fn get_target(&self, name: &str) -> Rc> { + pub(crate) fn get_target(&self, name: &str) -> anyhow::Result>> { // TODO implement .POSIX let follow_gnu = true; let vpath_options = match self.macros.get("VPATH") { Some((_, vpath)) if follow_gnu => { - let vpath = self.expand_macros(vpath, None); + let vpath = self.expand_macros(vpath, None)?; env::split_paths(&vpath).collect() } _ => vec![], @@ -480,19 +511,19 @@ impl<'a> Makefile<'a> { } let targets = self.targets.borrow(); - targets.get(name).expect("Target not found!").clone() + Ok(targets.get(name).context("Target not found!")?.clone()) } - pub(crate) fn update_target(&self, name: &str) { - self.get_target(name).borrow().update(self); + pub(crate) fn update_target(&self, name: &str) -> anyhow::Result<()> { + self.get_target(name)?.borrow().update(self) } - fn expand_macros(&self, text: &TokenString, target: Option<&Target>) -> String { + fn expand_macros(&self, text: &TokenString, target: Option<&Target>) -> anyhow::Result { let target = target.cloned(); let lookup_internal = move |name: &str| { let target = target .as_ref() - .expect("internal macro but no current target!"); + .context("internal macro but no current target!")?; let macro_pieces = if name.starts_with('@') { // The $@ shall evaluate to the full target name of the // current target. @@ -505,8 +536,8 @@ impl<'a> Makefile<'a> { .iter() .filter(|prereq| { self.get_target(prereq) - .borrow() - .newer_than(target) + .ok() + .and_then(|prereq| prereq.borrow().newer_than(target)) .unwrap_or(false) }) .cloned() @@ -531,27 +562,25 @@ impl<'a> Makefile<'a> { .map(|x| { Path::new(&x) .parent() - .expect("no parent") - .to_string_lossy() - .into() + .context("no parent") + .map(|x| x.to_string_lossy().into()) }) - .collect() + .collect::>()? } else if name.ends_with('F') { macro_pieces .into_iter() .map(|x| { Path::new(&x) .file_name() - .expect("no filename") - .to_string_lossy() - .into() + .context("no filename") + .map(|x| x.to_string_lossy().into()) }) - .collect() + .collect::>()? } else { macro_pieces }; - macro_pieces.join(" ") + Ok(macro_pieces.join(" ")) }; self.macros.with_lookup(&lookup_internal).expand(text) diff --git a/src/makefile/pattern.rs b/src/makefile/pattern.rs index 733721e..2d5f46c 100644 --- a/src/makefile/pattern.rs +++ b/src/makefile/pattern.rs @@ -1,37 +1,31 @@ use regex::{Captures, Regex}; -fn compile_pattern(pattern: &str) -> Regex { +fn compile_pattern(pattern: &str) -> anyhow::Result { let mut result = String::new(); for c in pattern.chars() { - match c { - // This is a nightmare, because we're escaping for regex syntax before we put - // things into result. - - // We don't end with a backslash, so this is an unescaped wildcard. - '%' if !result.ends_with(r"\\") => { - result.push_str(r"(\w*)"); - } - - // We end with two backslashes, so this is an escaped backslash and then an - // unescaped wildcard. - '%' if result.ends_with(r"\\\\") => { - result = result.strip_suffix(r"\\\\").unwrap().to_string(); + if c == '%' { + if let Some(real_result) = result.strip_suffix(r"\\\\") { + // We end with two backslashes, so this is an escaped backslash and then an + // unescaped wildcard. + result = real_result.to_string(); result.push_str(r"\\(\w*)"); - } - - // We end with one backslash, so this is an escaped wildcard. - '%' if result.ends_with(r"\\") => { - result = result.strip_suffix(r"\\").unwrap().to_string(); + } else if let Some(real_result) = result.strip_suffix(r"\\") { + // We end with one backslash, so this is an escaped wildcard. + result = real_result.to_string(); result.push('%'); + } else { + // We don't end with a backslash, so this is an unescaped wildcard. + result.push_str(r"(\w*)"); } - _ => result.push_str(®ex::escape(&c.to_string())), + } else { + result.push_str(®ex::escape(&c.to_string())); } } - Regex::new(&result).expect("built invalid regex!") + Ok(Regex::new(&result)?) } -pub(crate) fn r#match<'a>(pattern: &str, text: &'a str) -> Option> { - compile_pattern(pattern).captures(text) +pub(crate) fn r#match<'a>(pattern: &str, text: &'a str) -> anyhow::Result>> { + Ok(compile_pattern(pattern)?.captures(text)) } #[cfg(test)] diff --git a/src/makefile/target.rs b/src/makefile/target.rs index eb181cd..04603c9 100644 --- a/src/makefile/target.rs +++ b/src/makefile/target.rs @@ -42,26 +42,32 @@ impl Target { } let exists = metadata(&self.name).is_ok(); let newer_than_all_dependencies = self.prerequisites.iter().all(|t| { - self.newer_than(&file.get_target(t).borrow()) + file.get_target(t) + .ok() + .and_then(|t| self.newer_than(&t.borrow())) .unwrap_or(false) }); exists && newer_than_all_dependencies } - pub(crate) fn update(&self, file: &Makefile) { + pub(crate) fn update(&self, file: &Makefile) -> anyhow::Result<()> { for prereq in &self.prerequisites { - file.update_target(prereq); + file.update_target(prereq)?; } if !self.is_up_to_date(file) { - self.execute_commands(file); + self.execute_commands(file)?; } self.already_updated.set(true); + + Ok(()) } - fn execute_commands(&self, file: &Makefile) { + fn execute_commands(&self, file: &Makefile) -> anyhow::Result<()> { for command in &self.commands { - command.execute(file, self); + command.execute(file, self)?; } + + Ok(()) } } diff --git a/src/makefile/token.rs b/src/makefile/token.rs index 8b21f06..22ef24a 100644 --- a/src/makefile/token.rs +++ b/src/makefile/token.rs @@ -1,6 +1,7 @@ use std::fmt; use std::str::FromStr; +use anyhow::Context; use nom::{ branch::alt, bytes::complete::{tag, take_till1, take_while1}, @@ -250,18 +251,19 @@ fn full_text_tokens(input: &str) -> IResult<&str, TokenString> { all_consuming(tokens_but_not(vec![]))(input) } -pub(crate) fn tokenize(input: &str) -> TokenString { - // TODO handle errors gracefully - let (_, result) = full_text_tokens(input).expect("couldn't parse"); - result +pub(crate) fn tokenize(input: &str) -> anyhow::Result { + let (_, result) = full_text_tokens(input) + .finish() + .map_err(|err| anyhow::anyhow!(err.to_string())) + .with_context(|| format!("couldn't parse {:?}", input))?; + Ok(result) } impl FromStr for TokenString { - // TODO figure out how to get nom errors working (Error<&str> doesn't work because lifetimes) - type Err = (); + type Err = anyhow::Error; fn from_str(s: &str) -> Result { - full_text_tokens(s).finish().map(|(_, x)| x).map_err(|_| ()) + tokenize(s) } } -- cgit v1.2.3