Factor in .gitignore for build cmd freshness

When testing the freshness of a build command, the fingerprint of an entire
package is generated. The current method of fingerprinting a path source is to
just stat() the entire tree and look at mtimes. This works fine when the build
command places *all* output in the build directory.

Some systems, like autotools, place *most* output in the build directory and
some output in the source directory, however (see spidermonkey). In this case
the coarse-grained "consider all files in a directory as input" is too painful
for the build command as the package will forever be rebuilt.

This commit adds support for filtering the list of files in a directory
considered to be part of a package by using `git ls-files`. This git-specific
logic can be extended to other VCSs when cargo grows support for them.

Closes #379
This commit is contained in:
Alex Crichton 2014-08-15 17:40:08 -07:00
parent 35c1975e8b
commit 8a19eb7437
2 changed files with 118 additions and 28 deletions

View file

@ -5,7 +5,7 @@ use std::io::fs;
use core::{Package, PackageId, Summary, SourceId, Source, Dependency, Registry};
use ops;
use util::{CargoResult, internal, internal_error};
use util::{CargoResult, internal, internal_error, process};
pub struct PathSource {
id: SourceId,
@ -57,6 +57,74 @@ impl PathSource {
ops::read_packages(&self.path, &self.id)
}
}
/// List all files relevant to building this package inside this source.
///
/// This function will use the appropriate methods to determine what is the
/// set of files underneath this source's directory which are relevant for
/// building `pkg`.
///
/// The basic assumption of this method is that all files in the directory
/// are relevant for building this package, but it also contains logic to
/// use other methods like .gitignore to filter the list of files.
pub fn list_files(&self, pkg: &Package) -> CargoResult<Vec<Path>> {
// TODO: add an `excludes` section to the manifest which is another way
// to filter files out of this set that is returned.
return if self.path.join(".git").exists() {
self.list_files_git(pkg)
} else {
self.list_files_walk(pkg)
};
fn list_files_git(&self, pkg: &Package) -> CargoResult<Vec<Path>> {
let cwd = pkg.get_manifest_path().dir_path();
let mut cmd = process("git").cwd(cwd.clone());
cmd = cmd.arg("ls-files").arg("-z");
// Filter out all other packages with a filter directive
for pkg in self.packages.iter().filter(|p| *p != pkg) {
if cwd.is_ancestor_of(pkg.get_manifest_path()) {
let filter = pkg.get_manifest_path().dir_path()
.path_relative_from(&self.path).unwrap();
cmd = cmd.arg("-x").arg(filter);
}
}
log!(5, "listing git files with: {}", cmd);
let output = try!(cmd.arg(".").exec_with_output());
let output = output.output.as_slice();
Ok(output.split(|x| *x == 0).map(Path::new).collect())
}
fn list_files_walk(&self, pkg: &Package) -> CargoResult<Vec<Path>> {
let mut ret = Vec::new();
for pkg in self.packages.iter().filter(|p| *p == pkg) {
let loc = pkg.get_manifest_path().dir_path();
try!(walk(&loc, &mut ret, true));
}
return Ok(ret);
fn walk(path: &Path, ret: &mut Vec<Path>,
is_root: bool) -> CargoResult<()> {
if !path.is_dir() {
ret.push(path.clone());
return Ok(())
}
// Don't recurse into any sub-packages that we have
if !is_root && path.join("Cargo.toml").exists() { return Ok(()) }
for dir in try!(fs::readdir(path)).iter() {
match (is_root, dir.filename_str()) {
(_, Some(".git")) |
(true, Some("target")) |
(true, Some("Cargo.lock")) => continue,
_ => {}
}
try!(walk(dir, ret, false));
}
return Ok(())
}
}
}
}
impl Show for PathSource {
@ -105,33 +173,14 @@ impl Source for PathSource {
}
let mut max = 0;
for pkg in self.packages.iter().filter(|p| *p == pkg) {
let loc = pkg.get_manifest_path().dir_path();
max = cmp::max(max, try!(walk(&loc, true)));
}
return Ok(max.to_string());
fn walk(path: &Path, is_root: bool) -> CargoResult<u64> {
if !path.is_dir() {
// An fs::stat error here is either because path is a
// broken symlink, a permissions error, or a race
// condition where this path was rm'ed - either way,
// we can ignore the error and treat the path's mtime
// as 0.
return Ok(fs::stat(path).map(|s| s.modified).unwrap_or(0))
}
// Don't recurse into any sub-packages that we have
if !is_root && path.join("Cargo.toml").exists() { return Ok(0) }
let mut max = 0;
for dir in try!(fs::readdir(path)).iter() {
if is_root && dir.filename_str() == Some("target") { continue }
if is_root && dir.filename_str() == Some("Cargo.lock") { continue }
max = cmp::max(max, try!(walk(dir, false)));
}
return Ok(max)
for file in try!(self.list_files(pkg)).iter() {
// An fs::stat error here is either because path is a
// broken symlink, a permissions error, or a race
// condition where this path was rm'ed - either way,
// we can ignore the error and treat the path's mtime
// as 0.
max = cmp::max(max, file.stat().map(|s| s.modified).unwrap_or(0));
}
Ok(max.to_string())
}
}

View file

@ -1011,3 +1011,44 @@ test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured
", compiling = COMPILING, url = p.url(), running = RUNNING, bar = p2.url())));
})
test!(git_build_cmd_freshness {
let foo = git_repo("foo", |project| {
project.file("Cargo.toml", r#"
[package]
name = "foo"
version = "0.0.0"
authors = []
build = "true"
"#)
.file("src/lib.rs", "pub fn bar() -> int { 1 }")
.file(".gitignore", "
src/bar.rs
Cargo.lock
")
}).assert();
foo.root().move_into_the_past().assert();
assert_that(foo.process(cargo_dir().join("cargo-build")),
execs().with_status(0)
.with_stdout(format!("\
{compiling} foo v0.0.0 ({url})
", compiling = COMPILING, url = foo.url())));
// Smoke test to make sure it doesn't compile again
println!("first pass");
assert_that(foo.process(cargo_dir().join("cargo-build")),
execs().with_status(0)
.with_stdout(format!("\
{fresh} foo v0.0.0 ({url})
", fresh = FRESH, url = foo.url())));
// Modify an ignored file and make sure we don't rebuild
println!("second pass");
File::create(&foo.root().join("src/bar.rs")).assert();
assert_that(foo.process(cargo_dir().join("cargo-build")),
execs().with_status(0)
.with_stdout(format!("\
{fresh} foo v0.0.0 ({url})
", fresh = FRESH, url = foo.url())));
})