From 1cf029c229660efeca92cf88350bba77b5b7d902 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Thu, 26 Sep 2013 12:15:54 -0700 Subject: [PATCH 1/3] rustpkg: Make rustpkg tests stop comparing dates Instead of scrutinizing modification times in rustpkg tests, change output files to be read-only and detect attempts to write to them (hack suggested by Jack). This avoids time granularity problems. As part of this change, I discovered that some dependencies weren't getting written correctly (involving built executables and library files), so this patch fixes that too. This partly addresses #9441, but one test (test_rebuild_when_needed) is still ignored on Linux. --- src/libextra/workcache.rs | 2 +- src/librustpkg/conditions.rs | 4 ++ src/librustpkg/package_source.rs | 23 ++++++---- src/librustpkg/rustpkg.rs | 30 ++++++++++-- src/librustpkg/tests.rs | 78 +++++++++++++++++++++----------- src/librustpkg/util.rs | 68 +++++++++++++++++++++++++--- 6 files changed, 160 insertions(+), 45 deletions(-) diff --git a/src/libextra/workcache.rs b/src/libextra/workcache.rs index 26309cf3b37ed..a30951890d592 100644 --- a/src/libextra/workcache.rs +++ b/src/libextra/workcache.rs @@ -219,7 +219,7 @@ impl Logger { } pub fn info(&self, i: &str) { - io::println(~"workcache: " + i); + info2!("workcache: {}", i); } } diff --git a/src/librustpkg/conditions.rs b/src/librustpkg/conditions.rs index f40f48e851fba..147e838d94c43 100644 --- a/src/librustpkg/conditions.rs +++ b/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; } diff --git a/src/librustpkg/package_source.rs b/src/librustpkg/package_source.rs index fba6cba911660..696f39a95d78e 100644 --- a/src/librustpkg/package_source.rs +++ b/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` diff --git a/src/librustpkg/rustpkg.rs b/src/librustpkg/rustpkg.rs index 985dcd805ce28..affd53216659c 100644 --- a/src/librustpkg/rustpkg.rs +++ b/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()); @@ -554,14 +556,16 @@ impl CtxMethods for BuildContext { debug2!("In declare inputs for {}", id.to_str()); for cs in to_do.iter() { for c in cs.iter() { - let path = pkg_src.start_dir.join(&c.file); + let path = pkg_src.start_dir.join(&c.file).normalize(); 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.to_str(), + workcache_support::digest_only_date(executable)); + } + for library in sublib.iter() { + exe_thing.discover_input("binary", + library.to_str(), + workcache_support::digest_only_date(library)); + } + + for transitive_dependency in sub_build_inputs.iter() { + exe_thing.discover_input( + "file", + transitive_dependency.to_str(), + workcache_support::digest_file_with_date(transitive_dependency)); + } + for exec in subex.iter() { debug2!("Copying: {} -> {}", exec.display(), sub_target_ex.display()); diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs index 0fe168625187a..d522bfb15e42d 100644 --- a/src/librustpkg/tests.rs +++ b/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,26 @@ 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", [~"-A", ~"02", p.to_str()]).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 +1052,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 +1073,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 +1091,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 +2027,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); diff --git a/src/librustpkg/util.rs b/src/librustpkg/util.rs index 66c1aaea1ed64..12b4ce2a8811e 100644 --- a/src/librustpkg/util.rs +++ b/src/librustpkg/util.rs @@ -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 { 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.to_str(), + (~"binary", installed_path.to_str())); 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 \ @@ -486,14 +497,35 @@ impl<'self> Visitor<()> for ViewItemVisitor<'self> { 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 target_workspace = outputs_disc[0].pop(); + for dep in outputs_disc.iter() { + debug2!("Discovering a binary input: {}", dep.to_str()); + self.exec.discover_input("binary", dep.to_str(), + digest_only_date(dep)); + add_dep(self.deps, + self.parent_crate.to_str(), + (~"binary", dep.to_str())); + } for &(ref what, ref dep) in inputs_disc.iter() { if *what == ~"file" { + add_dep(self.deps, + self.parent_crate.to_str(), + (~"file", dep.to_str())); + self.exec.discover_input(*what, *dep, digest_file_with_date( &Path::new(dep.as_slice()))); } else if *what == ~"binary" { + add_dep(self.deps, + self.parent_crate.to_str(), + (~"binary", dep.to_str())); self.exec.discover_input(*what, *dep, digest_only_date( @@ -502,6 +534,10 @@ impl<'self> Visitor<()> for ViewItemVisitor<'self> { else { fail2!("Bad kind: {}", *what); } + // Also, add an additional search path + debug2!("Installed {} into {}", + lib_name, target_workspace.to_str()); + (self.save)(target_workspace.clone()); } } } @@ -518,18 +554,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 +619,19 @@ pub fn datestamp(p: &Path) -> Option { 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]); + } +} From 4dbef017dd2bdc34a6ddfb0914244946c01f9d97 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Wed, 2 Oct 2013 18:17:46 -0700 Subject: [PATCH 2/3] rustc: Check that the output file is writeable before linking This is because on Linux, the linker will silently overwrite a read-only file. --- src/librustc/back/link.rs | 25 +++++++++++++++++++++++++ src/librustpkg/rustpkg.rs | 8 ++++---- src/librustpkg/tests.rs | 2 +- src/librustpkg/util.rs | 36 +++++++++++++++--------------------- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/src/librustc/back/link.rs b/src/librustc/back/link.rs index 404efa25ff332..7028d8c42196d 100644 --- a/src/librustc/back/link.rs +++ b/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 diff --git a/src/librustpkg/rustpkg.rs b/src/librustpkg/rustpkg.rs index affd53216659c..e806e515c80e8 100644 --- a/src/librustpkg/rustpkg.rs +++ b/src/librustpkg/rustpkg.rs @@ -556,7 +556,7 @@ impl CtxMethods for BuildContext { debug2!("In declare inputs for {}", id.to_str()); for cs in to_do.iter() { for c in cs.iter() { - let path = pkg_src.start_dir.join(&c.file).normalize(); + let path = pkg_src.start_dir.join(&c.file); debug2!("Recording input: {}", path.display()); // FIXME (#9639): This needs to handle non-utf8 paths inputs.push((~"file", path.as_str().unwrap().to_owned())); @@ -623,19 +623,19 @@ impl CtxMethods for BuildContext { // Declare all the *inputs* to the declared input too, as inputs for executable in subex.iter() { exe_thing.discover_input("binary", - executable.to_str(), + executable.as_str().unwrap().to_owned(), workcache_support::digest_only_date(executable)); } for library in sublib.iter() { exe_thing.discover_input("binary", - library.to_str(), + 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.to_str(), + transitive_dependency.as_str().unwrap().to_owned(), workcache_support::digest_file_with_date(transitive_dependency)); } diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs index d522bfb15e42d..8722a638c456d 100644 --- a/src/librustpkg/tests.rs +++ b/src/librustpkg/tests.rs @@ -516,7 +516,7 @@ fn touch_source_file(workspace: &Path, pkgid: &PkgId) { // 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", [~"-A", ~"02", p.to_str()]).status != 0 { + if run::process_output("touch", [~"-A", ~"02", p.as_str().unwrap().to_owned()]).status != 0 { let _ = cond.raise((pkg_src_dir.clone(), ~"Bad path")); } } diff --git a/src/librustpkg/util.rs b/src/librustpkg/util.rs index 12b4ce2a8811e..345518eddc7bb 100644 --- a/src/librustpkg/util.rs +++ b/src/librustpkg/util.rs @@ -441,8 +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.to_str(), - (~"binary", installed_path.to_str())); + 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)); @@ -492,6 +492,10 @@ 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()); @@ -502,41 +506,31 @@ impl<'self> Visitor<()> for ViewItemVisitor<'self> { lib_name, outputs_disc.len(), inputs_disc.len()); // It must have installed *something*... assert!(!outputs_disc.is_empty()); - let target_workspace = outputs_disc[0].pop(); - for dep in outputs_disc.iter() { - debug2!("Discovering a binary input: {}", dep.to_str()); - self.exec.discover_input("binary", dep.to_str(), - digest_only_date(dep)); - add_dep(self.deps, - self.parent_crate.to_str(), - (~"binary", dep.to_str())); - } + 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.to_str(), - (~"file", dep.to_str())); - + 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.to_str(), - (~"binary", dep.to_str())); + 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.to_str()); + lib_name, target_workspace.as_str().unwrap().to_owned()); (self.save)(target_workspace.clone()); } } From e779313b0786b5b9d852729005d1552b0b120d61 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Fri, 18 Oct 2013 15:55:28 -0700 Subject: [PATCH 3/3] rustpkg: invoke touch with a portable set of args --- src/librustpkg/tests.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs index 8722a638c456d..c38e0d66b14da 100644 --- a/src/librustpkg/tests.rs +++ b/src/librustpkg/tests.rs @@ -516,7 +516,9 @@ fn touch_source_file(workspace: &Path, pkgid: &PkgId) { // 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", [~"-A", ~"02", p.as_str().unwrap().to_owned()]).status != 0 { + 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")); } }