Skip to content

Commit

Permalink
Parse depfiles with multiple targets
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
Colecf committed Feb 29, 2024
1 parent 668d9ab commit 4c78be8
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 50 deletions.
125 changes: 79 additions & 46 deletions src/depfile.rs
Original file line number Diff line number Diff line change
@@ -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<()> {
Expand Down Expand Up @@ -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;
}
Expand All @@ -61,43 +54,47 @@ 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)]
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);
Expand All @@ -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",]
)])
);
},
);
}
Expand All @@ -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",])])
);
});
}

Expand All @@ -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",]
)])
);
},
);
}
Expand All @@ -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]
Expand Down
30 changes: 29 additions & 1 deletion src/smallmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
}
6 changes: 3 additions & 3 deletions src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
64 changes: 64 additions & 0 deletions tests/e2e/discovered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

0 comments on commit 4c78be8

Please sign in to comment.