summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCole Faust <colefaust@google.com>2024-01-10 18:11:29 -0800
committerEvan Martin <evan.martin@gmail.com>2024-03-05 11:03:56 -0800
commit8881a661ae63e1124eb47b45f08d9fa1225a5f00 (patch)
tree0d9a6e659f8bf87bfc1d8e7f1135e46e5bd5e175
parent668d9ab5cdbd493a8af356078066423487ac81e2 (diff)
downloadn2-8881a661ae63e1124eb47b45f08d9fa1225a5f00.tar.gz
Parse depfiles with multiple targets
This is needed for android. It just treats all the deps of all targets in the depfile as deps for the Build. In task.rs there is a TODO to verify that the targets refer to the outputs of the Build, but adding that verification breaks the android build. In android's case, there are some depfiles that contain `foo.o: path/to/some/dep.asm` where it should instead be `path/to/foo.o: path/to/some/dep.asm`.
-rw-r--r--src/depfile.rs125
-rw-r--r--src/smallmap.rs30
-rw-r--r--src/task.rs6
-rw-r--r--tests/e2e/discovered.rs64
4 files changed, 175 insertions, 50 deletions
diff --git a/src/depfile.rs b/src/depfile.rs
index 7ee5493..b241cb6 100644
--- a/src/depfile.rs
+++ b/src/depfile.rs
@@ -1,15 +1,9 @@
//! Parsing of Makefile syntax as found in `.d` files emitted by C compilers.
-use crate::scanner::{ParseResult, Scanner};
-
-/// Dependency information for a single target.
-#[derive(Debug)]
-pub struct Deps<'a> {
- /// Output name, as found in the `.d` input.
- pub target: &'a str,
- /// Input names, as found in the `.d` input.
- pub deps: Vec<&'a str>,
-}
+use crate::{
+ scanner::{ParseResult, Scanner},
+ smallmap::SmallMap,
+};
/// Skip spaces and backslashed newlines.
fn skip_spaces(scanner: &mut Scanner) -> ParseResult<()> {
@@ -44,8 +38,7 @@ fn read_path<'a>(scanner: &mut Scanner<'a>) -> ParseResult<Option<&'a str>> {
break;
}
'\\' => {
- let peek = scanner.peek();
- if peek == '\n' || peek == '\r' {
+ if scanner.peek_newline() {
scanner.back();
break;
}
@@ -61,29 +54,33 @@ fn read_path<'a>(scanner: &mut Scanner<'a>) -> ParseResult<Option<&'a str>> {
}
/// Parse a `.d` file into `Deps`.
-pub fn parse<'a>(scanner: &mut Scanner<'a>) -> ParseResult<Deps<'a>> {
- let target = match read_path(scanner)? {
- None => return scanner.parse_error("expected file"),
- Some(o) => o,
- };
- scanner.skip_spaces();
- let target = match target.strip_suffix(':') {
- None => {
- scanner.expect(':')?;
- target
+pub fn parse<'a>(scanner: &mut Scanner<'a>) -> ParseResult<SmallMap<&'a str, Vec<&'a str>>> {
+ let mut result = SmallMap::default();
+ loop {
+ while scanner.peek() == ' ' || scanner.peek_newline() {
+ scanner.next();
+ }
+ let target = match read_path(scanner)? {
+ None => break,
+ Some(o) => o,
+ };
+ scanner.skip_spaces();
+ let target = match target.strip_suffix(':') {
+ None => {
+ scanner.expect(':')?;
+ target
+ }
+ Some(target) => target,
+ };
+ let mut deps = Vec::new();
+ while let Some(p) = read_path(scanner)? {
+ deps.push(p);
}
- Some(target) => target,
- };
- let mut deps = Vec::new();
- while let Some(p) = read_path(scanner)? {
- deps.push(p);
+ result.insert(target, deps);
}
- scanner.skip('\r');
- scanner.skip('\n');
- scanner.skip_spaces();
scanner.expect('\0')?;
- Ok(Deps { target, deps })
+ Ok(result)
}
#[cfg(test)]
@@ -91,13 +88,13 @@ mod tests {
use super::*;
use std::path::Path;
- fn try_parse(buf: &mut Vec<u8>) -> Result<Deps, String> {
+ fn try_parse(buf: &mut Vec<u8>) -> Result<SmallMap<&str, Vec<&str>>, String> {
buf.push(0);
let mut scanner = Scanner::new(buf);
parse(&mut scanner).map_err(|err| scanner.format_parse_error(Path::new("test"), err))
}
- fn must_parse(buf: &mut Vec<u8>) -> Deps {
+ fn must_parse(buf: &mut Vec<u8>) -> SmallMap<&str, Vec<&str>> {
match try_parse(buf) {
Err(err) => {
println!("{}", err);
@@ -121,8 +118,13 @@ mod tests {
|text| {
let mut file = text.into_bytes();
let deps = must_parse(&mut file);
- assert_eq!(deps.target, "build/browse.o");
- assert_eq!(deps.deps.len(), 3);
+ assert_eq!(
+ deps,
+ SmallMap::from([(
+ "build/browse.o",
+ vec!["src/browse.cc", "src/browse.h", "build/browse_py.h",]
+ )])
+ );
},
);
}
@@ -132,8 +134,10 @@ mod tests {
test_for_crlf("build/browse.o: src/browse.cc \n", |text| {
let mut file = text.into_bytes();
let deps = must_parse(&mut file);
- assert_eq!(deps.target, "build/browse.o");
- assert_eq!(deps.deps.len(), 1);
+ assert_eq!(
+ deps,
+ SmallMap::from([("build/browse.o", vec!["src/browse.cc",])])
+ );
});
}
@@ -144,8 +148,13 @@ mod tests {
|text| {
let mut file = text.into_bytes();
let deps = must_parse(&mut file);
- assert_eq!(deps.target, "build/browse.o");
- assert_eq!(deps.deps.len(), 2);
+ assert_eq!(
+ deps,
+ SmallMap::from([(
+ "build/browse.o",
+ vec!["src/browse.cc", "build/browse_py.h",]
+ )])
+ );
},
);
}
@@ -154,25 +163,49 @@ mod tests {
fn test_parse_without_final_newline() {
let mut file = b"build/browse.o: src/browse.cc".to_vec();
let deps = must_parse(&mut file);
- assert_eq!(deps.target, "build/browse.o");
- assert_eq!(deps.deps.len(), 1);
+ assert_eq!(
+ deps,
+ SmallMap::from([("build/browse.o", vec!["src/browse.cc",])])
+ );
}
#[test]
fn test_parse_spaces_before_colon() {
let mut file = b"build/browse.o : src/browse.cc".to_vec();
let deps = must_parse(&mut file);
- assert_eq!(deps.target, "build/browse.o");
- assert_eq!(deps.deps.len(), 1);
+ assert_eq!(
+ deps,
+ SmallMap::from([("build/browse.o", vec!["src/browse.cc",])])
+ );
}
#[test]
fn test_parse_windows_dep_path() {
let mut file = b"odd/path.o: C:/odd\\path.c".to_vec();
let deps = must_parse(&mut file);
- assert_eq!(deps.target, "odd/path.o");
- assert_eq!(deps.deps[0], "C:/odd\\path.c");
- assert_eq!(deps.deps.len(), 1);
+ assert_eq!(
+ deps,
+ SmallMap::from([("odd/path.o", vec!["C:/odd\\path.c",])])
+ );
+ }
+
+ #[test]
+ fn test_parse_multiple_targets() {
+ let mut file = b"
+out/a.o: src/a.c \\
+ src/b.c
+
+out/b.o :
+"
+ .to_vec();
+ let deps = must_parse(&mut file);
+ assert_eq!(
+ deps,
+ SmallMap::from([
+ ("out/a.o", vec!["src/a.c", "src/b.c",]),
+ ("out/b.o", vec![])
+ ])
+ );
}
#[test]
diff --git a/src/smallmap.rs b/src/smallmap.rs
index 37cc424..da8298c 100644
--- a/src/smallmap.rs
+++ b/src/smallmap.rs
@@ -2,7 +2,7 @@
//! TODO: this may not be needed at all, but the code used this pattern in a
//! few places so I figured I may as well name it.
-use std::borrow::Borrow;
+use std::{borrow::Borrow, fmt::Debug};
/// A map-like object implemented as a list of pairs, for cases where the
/// number of entries in the map is small.
@@ -49,4 +49,32 @@ impl<K: PartialEq, V> SmallMap<K, V> {
pub fn into_iter(self) -> std::vec::IntoIter<(K, V)> {
self.0.into_iter()
}
+
+ pub fn values(&self) -> impl Iterator<Item = &V> + '_ {
+ self.0.iter().map(|x| &x.1)
+ }
+}
+
+impl<K: PartialEq, V, const N: usize> std::convert::From<[(K, V); N]> for SmallMap<K, V> {
+ fn from(value: [(K, V); N]) -> Self {
+ let mut result = SmallMap::default();
+ for (k, v) in value {
+ result.insert(k, v);
+ }
+ result
+ }
+}
+
+impl<K: Debug, V: Debug> Debug for SmallMap<K, V> {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ self.0.fmt(f)
+ }
+}
+
+// Only for tests because it is order-sensitive
+#[cfg(test)]
+impl<K: PartialEq, V: PartialEq> PartialEq for SmallMap<K, V> {
+ fn eq(&self, other: &Self) -> bool {
+ return self.0 == other.0;
+ }
}
diff --git a/src/task.rs b/src/task.rs
index 693ab52..dfb818b 100644
--- a/src/task.rs
+++ b/src/task.rs
@@ -51,9 +51,9 @@ fn read_depfile(path: &Path) -> anyhow::Result<Vec<String>> {
.map_err(|err| anyhow!(scanner.format_parse_error(path, err)))?;
// TODO verify deps refers to correct output
let deps: Vec<String> = parsed_deps
- .deps
- .iter()
- .map(|&dep| dep.to_string())
+ .values()
+ .flat_map(|x| x.iter())
+ .map(|&dep| dep.to_owned())
.collect();
Ok(deps)
}
diff --git a/tests/e2e/discovered.rs b/tests/e2e/discovered.rs
index e2bedbf..a4252e5 100644
--- a/tests/e2e/discovered.rs
+++ b/tests/e2e/discovered.rs
@@ -93,3 +93,67 @@ build out: gendep || in
Ok(())
}
+
+#[cfg(unix)]
+#[test]
+fn multi_output_depfile() -> anyhow::Result<()> {
+ let space = TestSpace::new()?;
+ space.write(
+ "build.ninja",
+ "
+rule myrule
+ command = echo \"out: foo\" > out.d && echo \"out2: foo2\" >> out.d && echo >> out.d && echo >> out.d && touch out out2
+ depfile = out.d
+
+build out out2: myrule
+",
+ )?;
+ space.write("foo", "")?;
+ space.write("foo2", "")?;
+
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "ran 1 task");
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "no work");
+ space.write("foo", "x")?;
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "ran 1 task");
+ space.write("foo2", "x")?;
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "ran 1 task");
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "no work");
+ Ok(())
+}
+
+#[cfg(unix)]
+#[test]
+fn escaped_newline_in_depfile() -> anyhow::Result<()> {
+ let space = TestSpace::new()?;
+ space.write(
+ "build.ninja",
+ "
+rule myrule
+ command = echo \"out: foo \\\\\" > out.d && echo \" foo2\" >> out.d && touch out
+ depfile = out.d
+
+build out: myrule
+",
+ )?;
+ space.write("foo", "")?;
+ space.write("foo2", "")?;
+
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "ran 1 task");
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "no work");
+ space.write("foo", "x")?;
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "ran 1 task");
+ space.write("foo2", "x")?;
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "ran 1 task");
+ let out = space.run_expect(&mut n2_command(vec!["out"]))?;
+ assert_output_contains(&out, "no work");
+ Ok(())
+}