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 --- src/makefile/mod.rs | 133 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 81 insertions(+), 52 deletions(-) (limited to 'src/makefile/mod.rs') 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) -- cgit v1.2.3