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.

src/libextra/workcache.rs

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

src/librustc/back/link.rs

@@ -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

src/librustpkg/conditions.rs

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

src/librustpkg/package_source.rs

@@ -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`

src/librustpkg/rustpkg.rs

@@ -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());

src/librustpkg/tests.rs

@@ -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,55 +1075,55 @@ 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");
     let workspace = create_local_package_with_dep(&p_id, &dep_id);
     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);