auto merge of #9437 : catamorphism/rust/rustpkg-dates, r=alexcrichton

r? @alexcrichton On most platforms, the time granularity is 1 sec or more, so comparing
dates in tests that check whether rebuilding did or didn't happen leads
to spurious failures. Instead, test the same thing by making an output
file read-only and trapping attempts to write to it.
This commit is contained in:
bors 2013-10-18 19:36:22 -07:00
commit 3a7337ff17
7 changed files with 184 additions and 48 deletions

View File

@ -219,7 +219,7 @@ impl Logger {
}
pub fn info(&self, i: &str) {
io::println(~"workcache: " + i);
info2!("workcache: {}", i);
}
}

View File

@ -959,6 +959,16 @@ pub fn link_binary(sess: Session,
}
}
fn is_writeable(p: &Path) -> bool {
use std::libc::consts::os::posix88::S_IWUSR;
!os::path_exists(p) ||
(match p.get_mode() {
None => false,
Some(m) => m & S_IWUSR as uint == S_IWUSR as uint
})
}
pub fn link_args(sess: Session,
obj_filename: &Path,
out_filename: &Path,
@ -982,6 +992,21 @@ pub fn link_args(sess: Session,
out_filename.clone()
};
// Make sure the output and obj_filename are both writeable.
// Mac, FreeBSD, and Windows system linkers check this already --
// however, the Linux linker will happily overwrite a read-only file.
// We should be consistent.
let obj_is_writeable = is_writeable(obj_filename);
let out_is_writeable = is_writeable(&output);
if !out_is_writeable {
sess.fatal(format!("Output file {} is not writeable -- check its permissions.",
output.display()));
}
else if !obj_is_writeable {
sess.fatal(format!("Object file {} is not writeable -- check its permissions.",
obj_filename.display()));
}
// The default library location, we need this to find the runtime.
// The location of crates will be determined as needed.
// FIXME (#9639): This needs to handle non-utf8 paths

View File

@ -23,6 +23,10 @@ condition! {
pub bad_stat: (Path, ~str) -> stat;
}
condition! {
pub bad_kind: (~str) -> ();
}
condition! {
pub nonexistent_package: (PkgId, ~str) -> Path;
}

View File

@ -21,10 +21,11 @@ use source_control::{safe_git_clone, git_clone_url, DirToUse, CheckedOutSources}
use source_control::make_read_only;
use path_util::{find_dir_using_rust_path_hack, make_dir_rwx_recursive};
use path_util::{target_build_dir, versionize};
use util::compile_crate;
use util::{compile_crate, DepMap};
use workcache_support;
use workcache_support::crate_tag;
use extra::workcache;
use extra::treemap::TreeMap;
// An enumeration of the unpacked source of a package workspace.
// This contains a list of files found in the source workspace.
@ -370,6 +371,7 @@ impl PkgSrc {
fn build_crates(&self,
ctx: &BuildContext,
deps: &mut DepMap,
crates: &[Crate],
cfgs: &[~str],
what: OutputType) {
@ -389,12 +391,14 @@ impl PkgSrc {
let id = self.id.clone();
let sub_dir = self.build_workspace().clone();
let sub_flags = crate.flags.clone();
let sub_deps = deps.clone();
do prep.exec |exec| {
let result = compile_crate(&subcx,
exec,
&id,
&subpath,
&sub_dir,
&mut (sub_deps.clone()),
sub_flags,
subcfgs,
false,
@ -428,24 +432,27 @@ impl PkgSrc {
}
}
// It would be better if build returned a Path, but then Path would have to derive
// Encodable.
pub fn build(&self,
build_context: &BuildContext,
cfgs: ~[~str]) {
// DepMap is a map from str (crate name) to (kind, name) --
// it tracks discovered dependencies per-crate
cfgs: ~[~str]) -> DepMap {
let mut deps = TreeMap::new();
let libs = self.libs.clone();
let mains = self.mains.clone();
let tests = self.tests.clone();
let benchs = self.benchs.clone();
debug2!("Building libs in {}, destination = {}",
self.source_workspace.display(), self.build_workspace().display());
self.build_crates(build_context, libs, cfgs, Lib);
self.build_crates(build_context, &mut deps, libs, cfgs, Lib);
debug2!("Building mains");
self.build_crates(build_context, mains, cfgs, Main);
self.build_crates(build_context, &mut deps, mains, cfgs, Main);
debug2!("Building tests");
self.build_crates(build_context, tests, cfgs, Test);
self.build_crates(build_context, &mut deps, tests, cfgs, Test);
debug2!("Building benches");
self.build_crates(build_context, benchs, cfgs, Bench);
self.build_crates(build_context, &mut deps, benchs, cfgs, Bench);
deps
}
/// Return the workspace to put temporary files in. See the comment on `PkgSrc`

View File

@ -197,7 +197,8 @@ pub trait CtxMethods {
fn install(&self, src: PkgSrc, what: &WhatToBuild) -> (~[Path], ~[(~str, ~str)]);
/// Returns a list of installed files
fn install_no_build(&self,
source_workspace: &Path,
build_workspace: &Path,
build_inputs: &[Path],
target_workspace: &Path,
id: &PkgId) -> ~[~str];
fn prefer(&self, _id: &str, _vers: Option<~str>);
@ -542,6 +543,7 @@ impl CtxMethods for BuildContext {
let mut installed_files = ~[];
let mut inputs = ~[];
let mut build_inputs = ~[];
debug2!("Installing package source: {}", pkg_src.to_str());
@ -558,10 +560,12 @@ impl CtxMethods for BuildContext {
debug2!("Recording input: {}", path.display());
// FIXME (#9639): This needs to handle non-utf8 paths
inputs.push((~"file", path.as_str().unwrap().to_owned()));
build_inputs.push(path);
}
}
let result = self.install_no_build(pkg_src.build_workspace(),
build_inputs,
&pkg_src.destination_workspace,
&id).map(|s| Path::new(s.as_slice()));
debug2!("install: id = {}, about to call discover_outputs, {:?}",
@ -576,6 +580,7 @@ impl CtxMethods for BuildContext {
// again, working around lack of Encodable for Path
fn install_no_build(&self,
build_workspace: &Path,
build_inputs: &[Path],
target_workspace: &Path,
id: &PkgId) -> ~[~str] {
use conditions::copy_failed::cond;
@ -612,9 +617,28 @@ impl CtxMethods for BuildContext {
let sublib = maybe_library.clone();
let sub_target_ex = target_exec.clone();
let sub_target_lib = target_lib.clone();
let sub_build_inputs = build_inputs.to_owned();
do prep.exec |exe_thing| {
let mut outputs = ~[];
// Declare all the *inputs* to the declared input too, as inputs
for executable in subex.iter() {
exe_thing.discover_input("binary",
executable.as_str().unwrap().to_owned(),
workcache_support::digest_only_date(executable));
}
for library in sublib.iter() {
exe_thing.discover_input("binary",
library.as_str().unwrap().to_owned(),
workcache_support::digest_only_date(library));
}
for transitive_dependency in sub_build_inputs.iter() {
exe_thing.discover_input(
"file",
transitive_dependency.as_str().unwrap().to_owned(),
workcache_support::digest_file_with_date(transitive_dependency));
}
for exec in subex.iter() {
debug2!("Copying: {} -> {}", exec.display(), sub_target_ex.display());

View File

@ -37,7 +37,6 @@ use target::*;
use package_source::PkgSrc;
use source_control::{CheckedOutSources, DirToUse, safe_git_clone};
use exit_codes::{BAD_FLAG_CODE, COPY_FAILED_CODE};
use util::datestamp;
fn fake_ctxt(sysroot: Path, workspace: &Path) -> BuildContext {
let context = workcache::Context::new(
@ -507,6 +506,7 @@ fn output_file_name(workspace: &Path, short_name: ~str) -> Path {
os::EXE_SUFFIX))
}
#[cfg(target_os = "linux")]
fn touch_source_file(workspace: &Path, pkgid: &PkgId) {
use conditions::bad_path::cond;
let pkg_src_dir = workspace.join_many([~"src", pkgid.to_str()]);
@ -515,7 +515,28 @@ fn touch_source_file(workspace: &Path, pkgid: &PkgId) {
if p.extension_str() == Some("rs") {
// should be able to do this w/o a process
// FIXME (#9639): This needs to handle non-utf8 paths
if run::process_output("touch", [p.as_str().unwrap().to_owned()]).status != 0 {
// n.b. Bumps time up by 2 seconds to get around granularity issues
if run::process_output("touch", [~"--date",
~"+2 seconds",
p.as_str().unwrap().to_owned()]).status != 0 {
let _ = cond.raise((pkg_src_dir.clone(), ~"Bad path"));
}
}
}
}
#[cfg(not(target_os = "linux"))]
fn touch_source_file(workspace: &Path, pkgid: &PkgId) {
use conditions::bad_path::cond;
let pkg_src_dir = workspace.join_many([~"src", pkgid.to_str()]);
let contents = os::list_dir_path(&pkg_src_dir);
for p in contents.iter() {
if p.extension_str() == Some("rs") {
// should be able to do this w/o a process
// FIXME (#9639): This needs to handle non-utf8 paths
// n.b. Bumps time up by 2 seconds to get around granularity issues
if run::process_output("touch", [~"-A02",
p.as_str().unwrap().to_owned()]).status != 0 {
let _ = cond.raise((pkg_src_dir.clone(), ~"Bad path"));
}
}
@ -1033,12 +1054,17 @@ fn no_rebuilding() {
let workspace = create_local_package(&p_id);
let workspace = workspace.path();
command_line_test([~"build", ~"foo"], workspace);
let date = datestamp(&built_library_in_workspace(&p_id,
workspace).expect("no_rebuilding"));
let foo_lib = lib_output_file_name(workspace, "foo");
// Now make `foo` read-only so that subsequent rebuilds of it will fail
assert!(chmod_read_only(&foo_lib));
command_line_test([~"build", ~"foo"], workspace);
let newdate = datestamp(&built_library_in_workspace(&p_id,
workspace).expect("no_rebuilding (2)"));
assert_eq!(date, newdate);
match command_line_test_partial([~"build", ~"foo"], workspace) {
Success(*) => (), // ok
Fail(status) if status == 65 => fail2!("no_rebuilding failed: it tried to rebuild bar"),
Fail(_) => fail2!("no_rebuilding failed for some other reason")
}
}
#[test]
@ -1049,25 +1075,17 @@ fn no_rebuilding_dep() {
let workspace = workspace.path();
command_line_test([~"build", ~"foo"], workspace);
let bar_lib = lib_output_file_name(workspace, "bar");
let bar_date_1 = datestamp(&bar_lib);
frob_source_file(workspace, &p_id, "main.rs");
// Now make `bar` read-only so that subsequent rebuilds of it will fail
assert!(chmod_read_only(&bar_lib));
match command_line_test_partial([~"build", ~"foo"], workspace) {
Success(*) => (), // ok
Fail(status) if status == 65 => fail2!("no_rebuilding_dep failed: it tried to rebuild bar"),
Fail(_) => fail2!("no_rebuilding_dep failed for some other reason")
}
let bar_date_2 = datestamp(&bar_lib);
assert_eq!(bar_date_1, bar_date_2);
}
#[test]
#[ignore]
fn do_rebuild_dep_dates_change() {
let p_id = PkgId::new("foo");
let dep_id = PkgId::new("bar");
@ -1075,29 +1093,37 @@ fn do_rebuild_dep_dates_change() {
let workspace = workspace.path();
command_line_test([~"build", ~"foo"], workspace);
let bar_lib_name = lib_output_file_name(workspace, "bar");
let bar_date = datestamp(&bar_lib_name);
debug2!("Datestamp on {} is {:?}", bar_lib_name.display(), bar_date);
touch_source_file(workspace, &dep_id);
command_line_test([~"build", ~"foo"], workspace);
let new_bar_date = datestamp(&bar_lib_name);
debug2!("Datestamp on {} is {:?}", bar_lib_name.display(), new_bar_date);
assert!(new_bar_date > bar_date);
// Now make `bar` read-only so that subsequent rebuilds of it will fail
assert!(chmod_read_only(&bar_lib_name));
match command_line_test_partial([~"build", ~"foo"], workspace) {
Success(*) => fail2!("do_rebuild_dep_dates_change failed: it didn't rebuild bar"),
Fail(status) if status == 65 => (), // ok
Fail(_) => fail2!("do_rebuild_dep_dates_change failed for some other reason")
}
}
#[test]
#[ignore]
fn do_rebuild_dep_only_contents_change() {
let p_id = PkgId::new("foo");
let dep_id = PkgId::new("bar");
let workspace = create_local_package_with_dep(&p_id, &dep_id);
let workspace = workspace.path();
command_line_test([~"build", ~"foo"], workspace);
let bar_date = datestamp(&lib_output_file_name(workspace, "bar"));
frob_source_file(workspace, &dep_id, "lib.rs");
let bar_lib_name = lib_output_file_name(workspace, "bar");
// Now make `bar` read-only so that subsequent rebuilds of it will fail
assert!(chmod_read_only(&bar_lib_name));
// should adjust the datestamp
command_line_test([~"build", ~"foo"], workspace);
let new_bar_date = datestamp(&lib_output_file_name(workspace, "bar"));
assert!(new_bar_date > bar_date);
match command_line_test_partial([~"build", ~"foo"], workspace) {
Success(*) => fail2!("do_rebuild_dep_only_contents_change failed: it didn't rebuild bar"),
Fail(status) if status == 65 => (), // ok
Fail(_) => fail2!("do_rebuild_dep_only_contents_change failed for some other reason")
}
}
#[test]
@ -2003,7 +2029,7 @@ fn test_rustpkg_test_output() {
}
#[test]
#[ignore(reason = "See issue #9441")]
#[ignore(reason = "Issue 9441")]
fn test_rebuild_when_needed() {
let foo_id = PkgId::new("foo");
let foo_workspace = create_local_package(&foo_id);

View File

@ -30,6 +30,8 @@ use workspace::pkg_parent_workspaces;
use path_util::{U_RWX, system_library, target_build_dir};
use path_util::{default_workspace, built_library_in_workspace};
pub use target::{OutputType, Main, Lib, Bench, Test, JustOne, lib_name_of, lib_crate_filename};
pub use target::{Target, Build, Install};
use extra::treemap::TreeMap;
use workcache_support::{digest_file_with_date, digest_only_date};
// It would be nice to have the list of commands in just one place -- for example,
@ -166,6 +168,7 @@ pub fn compile_input(context: &BuildContext,
pkg_id: &PkgId,
in_file: &Path,
workspace: &Path,
deps: &mut DepMap,
flags: &[~str],
cfgs: &[~str],
opt: bool,
@ -265,7 +268,7 @@ pub fn compile_input(context: &BuildContext,
let mut crate = driver::phase_1_parse_input(sess, cfg.clone(), &input);
crate = driver::phase_2_configure_and_expand(sess, cfg.clone(), crate);
find_and_install_dependencies(context, pkg_id, sess, exec, &crate,
find_and_install_dependencies(context, pkg_id, in_file, sess, exec, &crate, deps,
|p| {
debug2!("a dependency: {}", p.display());
// Pass the directory containing a dependency
@ -329,6 +332,7 @@ pub fn compile_input(context: &BuildContext,
// If crate_opt is present, then finish compilation. If it's None, then
// call compile_upto and return the crate
// also, too many arguments
// Returns list of discovered dependencies
pub fn compile_crate_from_input(input: &Path,
exec: &mut workcache::Exec,
stop_before: StopBefore,
@ -390,29 +394,34 @@ pub fn exe_suffix() -> ~str { ~"" }
pub fn compile_crate(ctxt: &BuildContext,
exec: &mut workcache::Exec,
pkg_id: &PkgId,
crate: &Path, workspace: &Path,
flags: &[~str], cfgs: &[~str], opt: bool,
crate: &Path,
workspace: &Path,
deps: &mut DepMap,
flags: &[~str],
cfgs: &[~str],
opt: bool,
what: OutputType) -> Option<Path> {
debug2!("compile_crate: crate={}, workspace={}", crate.display(), workspace.display());
debug2!("compile_crate: short_name = {}, flags =...", pkg_id.to_str());
for fl in flags.iter() {
debug2!("+++ {}", *fl);
}
compile_input(ctxt, exec, pkg_id, crate, workspace, flags, cfgs, opt, what)
compile_input(ctxt, exec, pkg_id, crate, workspace, deps, flags, cfgs, opt, what)
}
struct ViewItemVisitor<'self> {
context: &'self BuildContext,
parent: &'self PkgId,
parent_crate: &'self Path,
sess: session::Session,
exec: &'self mut workcache::Exec,
c: &'self ast::Crate,
save: &'self fn(Path),
deps: &'self mut DepMap
}
impl<'self> Visitor<()> for ViewItemVisitor<'self> {
fn visit_view_item(&mut self, vi: &ast::view_item, env: ()) {
debug2!("A view item!");
match vi.node {
// ignore metadata, I guess
ast::view_item_extern_mod(lib_ident, path_opt, _, _) => {
@ -432,6 +441,8 @@ impl<'self> Visitor<()> for ViewItemVisitor<'self> {
// Now we know that this crate has a discovered dependency on
// installed_path
// FIXME (#9639): This needs to handle non-utf8 paths
add_dep(self.deps, self.parent_crate.as_str().unwrap().to_owned(),
(~"binary", installed_path.as_str().unwrap().to_owned()));
self.exec.discover_input("binary",
installed_path.as_str().unwrap(),
digest_only_date(installed_path));
@ -465,7 +476,7 @@ impl<'self> Visitor<()> for ViewItemVisitor<'self> {
// Use the rust_path_hack to search for dependencies iff
// we were already using it
self.context.context.use_rust_path_hack,
pkg_id);
pkg_id.clone());
let (outputs_disc, inputs_disc) =
self.context.install(pkg_src, &JustOne(Path::new(lib_crate_filename)));
debug2!("Installed {}, returned {:?} dependencies and \
@ -481,27 +492,46 @@ impl<'self> Visitor<()> for ViewItemVisitor<'self> {
self.exec.discover_input("binary",
dep.as_str().unwrap(),
digest_only_date(dep));
add_dep(self.deps,
self.parent_crate.as_str().unwrap().to_owned(),
(~"binary", dep.as_str().unwrap().to_owned()));
// Also, add an additional search path
let dep_dir = dep.dir_path();
debug2!("Installed {} into {}", dep.display(), dep_dir.display());
(self.save)(dep_dir);
}
debug2!("Installed {}, returned {} dependencies and \
{} transitive dependencies",
lib_name, outputs_disc.len(), inputs_disc.len());
// It must have installed *something*...
assert!(!outputs_disc.is_empty());
let mut target_workspace = outputs_disc[0].clone();
target_workspace.pop();
for &(ref what, ref dep) in inputs_disc.iter() {
if *what == ~"file" {
add_dep(self.deps,
self.parent_crate.as_str().unwrap().to_owned(),
(~"file", dep.clone()));
self.exec.discover_input(*what,
*dep,
digest_file_with_date(
&Path::new(dep.as_slice())));
}
else if *what == ~"binary" {
} else if *what == ~"binary" {
add_dep(self.deps,
self.parent_crate.as_str().unwrap().to_owned(),
(~"binary", dep.clone()));
self.exec.discover_input(*what,
*dep,
digest_only_date(
&Path::new(dep.as_slice())));
}
else {
} else {
fail2!("Bad kind: {}", *what);
}
// Also, add an additional search path
debug2!("Installed {} into {}",
lib_name, target_workspace.as_str().unwrap().to_owned());
(self.save)(target_workspace.clone());
}
}
}
@ -518,18 +548,22 @@ impl<'self> Visitor<()> for ViewItemVisitor<'self> {
/// can't be found.
pub fn find_and_install_dependencies(context: &BuildContext,
parent: &PkgId,
parent_crate: &Path,
sess: session::Session,
exec: &mut workcache::Exec,
c: &ast::Crate,
deps: &mut DepMap,
save: &fn(Path)) {
debug2!("In find_and_install_dependencies...");
let mut visitor = ViewItemVisitor {
context: context,
parent: parent,
parent_crate: parent_crate,
sess: sess,
exec: exec,
c: c,
save: save,
deps: deps
};
visit::walk_crate(&mut visitor, c, ())
}
@ -579,3 +613,19 @@ pub fn datestamp(p: &Path) -> Option<libc::time_t> {
debug2!("Date = {:?}", out);
out.map(|t| { t as libc::time_t })
}
pub type DepMap = TreeMap<~str, ~[(~str, ~str)]>;
/// Records a dependency from `parent` to the kind and value described by `info`,
/// in `deps`
fn add_dep(deps: &mut DepMap, parent: ~str, info: (~str, ~str)) {
let mut done = false;
let info_clone = info.clone();
match deps.find_mut(&parent) {
None => { }
Some(v) => { done = true; (*v).push(info) }
};
if !done {
deps.insert(parent, ~[info_clone]);
}
}