rustpkg: Un-ignore most of the remaining tests

This necessitated some cleanup to how we parse library filenames
when searching for libraries, since rustpkg may now create filenames
that contain '-' characters. Also cleaned up how rustpkg passes the
sysroot to a custom build script.

src/librustpkg/context.rs

@@ -33,6 +33,17 @@ pub fn sysroot_opt_str(&self) -> ~str {
             Some(p) => p.to_str()
         }
     }
+
+    // Hack so that rustpkg can run either out of a rustc target dir,
+    // or the host dir
+    pub fn sysroot_to_use(&self) -> Option<@Path> {
+        if !in_target(self.sysroot_opt) {
+            self.sysroot_opt
+        }
+        else {
+            self.sysroot_opt.map(|p| { @p.pop().pop().pop() })
+        }
+    }
 }
 
 /// We assume that if ../../rustc exists, then we're running

src/librustpkg/package_source.rs

@@ -54,7 +54,7 @@ fn check_dir(&self) -> Path {
         debug!("Pushing onto root: %s | %s", self.id.path.to_str(), self.root.to_str());
 
         let dirs = pkgid_src_in_workspace(&self.id, &self.root);
-        debug!("Checking dirs: %?", dirs);
+        debug!("Checking dirs: %?", dirs.map(|s| s.to_str()).connect(":"));
         let path = dirs.iter().find(|&d| os::path_exists(d));
 
         let dir = match path {

src/librustpkg/path_util.rs

@@ -12,7 +12,7 @@
 
 pub use package_id::PkgId;
 pub use target::{OutputType, Main, Lib, Test, Bench, Target, Build, Install};
-pub use version::{Version, NoVersion, split_version_general};
+pub use version::{Version, NoVersion, split_version_general, try_parsing_version};
 pub use rustc::metadata::filesearch::rust_path;
 
 use std::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
@@ -153,21 +153,19 @@ fn output_in_workspace(pkgid: &PkgId, workspace: &Path, what: OutputType) -> Opt
 /// Figure out what the library name for <pkgid> in <workspace>'s build
 /// directory is, and if the file exists, return it.
 pub fn built_library_in_workspace(pkgid: &PkgId, workspace: &Path) -> Option<Path> {
-    library_in_workspace(&pkgid.path, pkgid.short_name, Build, workspace, "build")
+    library_in_workspace(&pkgid.path, pkgid.short_name, Build, workspace, "build", &pkgid.version)
 }
 
 /// Does the actual searching stuff
 pub fn installed_library_in_workspace(short_name: &str, workspace: &Path) -> Option<Path> {
-    library_in_workspace(&Path(short_name), short_name, Install, workspace, "lib")
+    // NOTE: this could break once we're handling multiple versions better... want a test for it
+    library_in_workspace(&Path(short_name), short_name, Install, workspace, "lib", &NoVersion)
 }
 
-
-/// This doesn't take a PkgId, so we can use it for `extern mod` inference, where we
-/// don't know the entire package ID.
 /// `workspace` is used to figure out the directory to search.
 /// `short_name` is taken as the link name of the library.
 pub fn library_in_workspace(path: &Path, short_name: &str, where: Target,
-                        workspace: &Path, prefix: &str) -> Option<Path> {
+                        workspace: &Path, prefix: &str, version: &Version) -> Option<Path> {
     debug!("library_in_workspace: checking whether a library named %s exists",
            short_name);
 
@@ -209,36 +207,37 @@ pub fn library_in_workspace(path: &Path, short_name: &str, where: Target,
     for p_path in libraries {
         // Find a filename that matches the pattern: (lib_prefix)-hash-(version)(lib_suffix)
         // and remember what the hash was
-        let f_name = match p_path.filename() {
+        let mut f_name = match p_path.filestem() {
             Some(s) => s, None => loop
         };
-
-        let mut hash = None;
-        let mut which = 0;
-        for piece in f_name.split_iter('-') {
-            debug!("a piece = %s", piece);
-            if which == 0 && piece != lib_prefix {
-                break;
-            }
-            else if which == 0 {
-                which += 1;
-            }
-            else if which == 1 {
-                hash = Some(piece.to_owned());
-                break;
-            }
-            else {
-                // something went wrong
-                hash = None;
-                break;
-            }
-        }
-
-        if hash.is_some() {
-            result_filename = Some(p_path);
-            break;
-        }
-    }
+        // Already checked the filetype above
+
+         // This is complicated because library names and versions can both contain dashes
+         loop {
+            if f_name.is_empty() { break; }
+            match f_name.rfind('-') {
+                Some(i) => {
+                    debug!("Maybe %s is a version", f_name.slice(i + 1, f_name.len()));
+                    match try_parsing_version(f_name.slice(i + 1, f_name.len())) {
+                       Some(ref found_vers) if version == found_vers => {
+                           match f_name.slice(0, i).rfind('-') {
+                               Some(j) => {
+                                   debug!("Maybe %s equals %s", f_name.slice(0, j), lib_prefix);
+                                   if f_name.slice(0, j) == lib_prefix {
+                                       result_filename = Some(p_path);
+                                   }
+                                   break;
+                               }
+                               None => break
+                           }
+                       }
+                       _ => { f_name = f_name.slice(0, i).to_owned(); }
+                 }
+               }
+               None => break
+         } // match
+       } // loop
+    } // for
 
     if result_filename.is_none() {
         warn(fmt!("library_in_workspace didn't find a library in %s for %s",

src/librustpkg/rustpkg.rs

@@ -94,15 +94,18 @@ impl<'self> PkgScript<'self> {
     /// Given the path name for a package script
     /// and a package ID, parse the package script into
     /// a PkgScript that we can then execute
-    fn parse<'a>(script: Path, workspace: &Path, id: &'a PkgId) -> PkgScript<'a> {
+    fn parse<'a>(sysroot: @Path,
+                 script: Path,
+                 workspace: &Path,
+                 id: &'a PkgId) -> PkgScript<'a> {
         // Get the executable name that was invoked
         let binary = os::args()[0].to_managed();
         // Build the rustc session data structures to pass
         // to the compiler
-    debug!("pkgscript parse: %?", os::self_exe_path());
+        debug!("pkgscript parse: %s", sysroot.to_str());
         let options = @session::options {
             binary: binary,
-            maybe_sysroot: Some(@os::self_exe_path().unwrap().pop()),
+            maybe_sysroot: Some(sysroot),
             crate_type: session::bin_crate,
             .. (*session::basic_options()).clone()
         };
@@ -113,7 +116,7 @@ fn parse<'a>(script: Path, workspace: &Path, id: &'a PkgId) -> PkgScript<'a> {
         let crate = driver::phase_2_configure_and_expand(sess, cfg.clone(), crate);
         let work_dir = build_pkg_id_in_workspace(id, workspace);
 
-        debug!("Returning package script with id %?", id);
+        debug!("Returning package script with id %s", id.to_str());
 
         PkgScript {
             id: id,
@@ -138,15 +141,13 @@ fn run_custom(&self, sysroot: @Path) -> (~[~str], ExitCode) {
         let crate = util::ready_crate(sess, self.crate);
         debug!("Building output filenames with script name %s",
                driver::source_name(&self.input));
-        let root = filesearch::get_or_default_sysroot().pop().pop(); // :-\
-        debug!("Root is %s, calling compile_rest", root.to_str());
         let exe = self.build_dir.push(~"pkg" + util::exe_suffix());
         util::compile_crate_from_input(&self.input,
                                        &self.build_dir,
                                        sess,
                                        crate);
-        debug!("Running program: %s %s %s %s", exe.to_str(),
-               sysroot.to_str(), root.to_str(), "install");
+        debug!("Running program: %s %s %s", exe.to_str(),
+               sysroot.to_str(), "install");
         // FIXME #7401 should support commands besides `install`
         let status = run::process_status(exe.to_str(), [sysroot.to_str(), ~"install"]);
         if status != 0 {
@@ -154,8 +155,8 @@ fn run_custom(&self, sysroot: @Path) -> (~[~str], ExitCode) {
         }
         else {
             debug!("Running program (configs): %s %s %s",
-                   exe.to_str(), root.to_str(), "configs");
-            let output = run::process_output(exe.to_str(), [root.to_str(), ~"configs"]);
+                   exe.to_str(), sysroot.to_str(), "configs");
+            let output = run::process_output(exe.to_str(), [sysroot.to_str(), ~"configs"]);
             // Run the configs() function to get the configs
             let cfgs = str::from_bytes_slice(output.output).word_iter()
                 .map(|w| w.to_owned()).collect();
@@ -350,10 +351,11 @@ fn build(&self, workspace: &Path, pkgid: &PkgId) {
         debug!("Package source directory = %?", pkg_src_dir);
         let cfgs = match pkg_src_dir.chain_ref(|p| src.package_script_option(p)) {
             Some(package_script_path) => {
-                let pscript = PkgScript::parse(package_script_path,
+                let sysroot = self.sysroot_to_use().expect("custom build needs a sysroot");
+                let pscript = PkgScript::parse(sysroot,
+                                               package_script_path,
                                                workspace,
                                                pkgid);
-                let sysroot = self.sysroot_opt.expect("custom build needs a sysroot");
                 let (cfgs, hook_result) = pscript.run_custom(sysroot);
                 debug!("Command return code = %?", hook_result);
                 if hook_result != 0 {

src/librustpkg/tests.rs

@@ -222,7 +222,7 @@ fn command_line_test_with_env(args: &[~str], cwd: &Path, env: Option<~[(~str, ~s
     assert!(os::path_is_dir(&*cwd));
     let cwd = (*cwd).clone();
     let mut prog = run::Process::new(cmd, args, run::ProcessOptions {
-        env: env,
+        env: env.map(|e| e + os::env()),
         dir: Some(&cwd),
         in_fd: None,
         out_fd: None,
@@ -358,7 +358,8 @@ fn lib_output_file_name(workspace: &Path, parent: &str, short_name: &str) -> Pat
                          short_name,
                          Build,
                          workspace,
-                         "build").expect("lib_output_file_name")
+                         "build",
+                         &NoVersion).expect("lib_output_file_name")
 }
 
 fn output_file_name(workspace: &Path, short_name: &str) -> Path {
@@ -405,10 +406,7 @@ fn frob_source_file(workspace: &Path, pkgid: &PkgId) {
     }
 }
 
-// FIXME(#7249): these tests fail on multi-platform builds, so for now they're
-//               only run one x86
-
-#[test] #[ignore(cfg(target_arch = "x86"))]
+#[test]
 fn test_make_dir_rwx() {
     let temp = &os::tmpdir();
     let dir = temp.push("quux");
@@ -421,7 +419,7 @@ fn test_make_dir_rwx() {
     assert!(os::remove_dir_recursive(&dir));
 }
 
-#[test] #[ignore(cfg(target_arch = "x86"))]
+#[test]
 fn test_install_valid() {
     use path_util::installed_library_in_workspace;
 
@@ -451,7 +449,7 @@ fn test_install_valid() {
     assert!(!os::path_exists(&bench));
 }
 
-#[test] #[ignore(cfg(target_arch = "x86"))]
+#[test]
 fn test_install_invalid() {
     use conditions::nonexistent_package::cond;
     use cond1 = conditions::missing_pkg_files::cond;
@@ -476,8 +474,6 @@ fn test_install_invalid() {
 
 // Tests above should (maybe) be converted to shell out to rustpkg, too
 
-// FIXME: #7956: temporarily disabled
-#[ignore(cfg(target_arch = "x86"))]
 fn test_install_git() {
     let sysroot = test_sysroot();
     debug!("sysroot = %s", sysroot.to_str());
@@ -526,7 +522,7 @@ fn test_install_git() {
     assert!(!os::path_exists(&bench));
 }
 
-#[test] #[ignore(cfg(target_arch = "x86"))]
+#[test]
 fn test_package_ids_must_be_relative_path_like() {
     use conditions::bad_pkg_id::cond;
 
@@ -567,8 +563,6 @@ fn test_package_ids_must_be_relative_path_like() {
 
 }
 
-// FIXME: #7956: temporarily disabled
-#[ignore(cfg(target_arch = "x86"))]
 fn test_package_version() {
     let local_path = "mockgithub.com/catamorphism/test_pkg_version";
     let repo = init_git_repo(&Path(local_path));
@@ -655,7 +649,6 @@ fn rustpkg_install_url_2() {
                      &temp_dir);
 }
 
-// FIXME: #7956: temporarily disabled
 #[test]
 fn rustpkg_library_target() {
     let foo_repo = init_git_repo(&Path("foo"));
@@ -683,10 +676,7 @@ fn rustpkg_local_pkg() {
     assert_executable_exists(&dir, "foo");
 }
 
-// FIXME: #7956: temporarily disabled
-//  Failing on dist-linux bot
 #[test]
-#[ignore]
 fn package_script_with_default_build() {
     let dir = create_local_package(&PkgId::new("fancy-lib"));
     debug!("dir = %s", dir.to_str());
@@ -694,12 +684,12 @@ fn package_script_with_default_build() {
         push("testsuite").push("pass").push("src").push("fancy-lib").push("pkg.rs");
     debug!("package_script_with_default_build: %s", source.to_str());
     if !os::copy_file(&source,
-                      & dir.push("src").push("fancy_lib-0.1").push("pkg.rs")) {
+                      & dir.push("src").push("fancy-lib-0.1").push("pkg.rs")) {
         fail!("Couldn't copy file");
     }
     command_line_test([~"install", ~"fancy-lib"], &dir);
     assert_lib_exists(&dir, "fancy-lib", NoVersion);
-    assert!(os::path_exists(&dir.push("build").push("fancy_lib").push("generated.rs")));
+    assert!(os::path_exists(&dir.push("build").push("fancy-lib").push("generated.rs")));
 }
 
 #[test]
@@ -718,7 +708,7 @@ fn rustpkg_build_no_arg() {
 #[test]
 fn rustpkg_install_no_arg() {
     let tmp = mkdtemp(&os::tmpdir(),
-                      "rustpkg_install_no_arg").expect("rustpkg_build_no_arg failed");
+                      "rustpkg_install_no_arg").expect("rustpkg_install_no_arg failed");
     let package_dir = tmp.push("src").push("foo");
     assert!(os::mkdir_recursive(&package_dir, U_RWX));
     writeFile(&package_dir.push("lib.rs"),
@@ -745,7 +735,6 @@ fn rustpkg_clean_no_arg() {
 }
 
 #[test]
-#[ignore (reason = "Specifying env doesn't work -- see #8028")]
 fn rust_path_test() {
     let dir_for_path = mkdtemp(&os::tmpdir(), "more_rust").expect("rust_path_test failed");
     let dir = mk_workspace(&dir_for_path, &Path("foo"), &NoVersion);
@@ -755,20 +744,9 @@ fn rust_path_test() {
     let cwd = os::getcwd();
     debug!("cwd = %s", cwd.to_str());
                                      // use command_line_test_with_env
-    let mut prog = run::Process::new("rustpkg",
-                                     [~"install", ~"foo"],
-// This should actually extend the environment; then we can probably
-// un-ignore it
-                                     run::ProcessOptions { env: Some(~[(~"RUST_LOG",
-                                                                        ~"rustpkg"),
-                                                                       (~"RUST_PATH",
-                                                                       dir_for_path.to_str())]),
-                                                          dir: Some(&cwd),
-                                                          in_fd: None,
-                                                          out_fd: None,
-                                                          err_fd: None
-                                                         });
-    prog.finish_with_output();
+    command_line_test_with_env([~"install", ~"foo"],
+                               &cwd,
+                               Some(~[(~"RUST_PATH", dir_for_path.to_str())]));
     assert_executable_exists(&dir_for_path, "foo");
 }
 

src/librustpkg/testsuite/pass/src/fancy-lib/pkg.rs

@@ -38,7 +38,7 @@ pub fn main() {
         return;
     }
 
-    let out_path = Path("build/fancy_lib");
+    let out_path = Path("build/fancy-lib");
     if !os::path_exists(&out_path) {
         assert!(os::make_dir(&out_path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32));
     }

src/librustpkg/version.rs

@@ -27,6 +27,10 @@ pub enum Version {
     NoVersion // user didn't specify a version -- prints as 0.1
 }
 
+// Equality on versions is non-symmetric: if self is NoVersion, it's equal to
+// anything; but if self is a precise version, it's not equal to NoVersion.
+// We should probably make equality symmetric, and use less-than and greater-than
+// where we currently use eq
 impl Eq for Version {
     fn eq(&self, other: &Version) -> bool {
         match (self, other) {
@@ -176,7 +180,7 @@ enum ParseState {
     SawDot
 }
 
-fn try_parsing_version(s: &str) -> Option<Version> {
+pub fn try_parsing_version(s: &str) -> Option<Version> {
     let s = s.trim();
     debug!("Attempting to parse: %s", s);
     let mut parse_state = Start;
@@ -207,17 +211,16 @@ fn is_url_like(p: &Path) -> bool {
 /// number, return the prefix before the # and the version.
 /// Otherwise, return None.
 pub fn split_version<'a>(s: &'a str) -> Option<(&'a str, Version)> {
-    split_version_general(s, '#')
+    // Check for extra '#' characters separately
+    if s.split_iter('#').len() > 2 {
+        None
+    }
+    else {
+        split_version_general(s, '#')
+    }
 }
 
 pub fn split_version_general<'a>(s: &'a str, sep: char) -> Option<(&'a str, Version)> {
-    // reject strings with multiple '#'s
-    for st in s.split_iter(sep) {
-        debug!("whole = %s part = %s", s, st);
-    }
-    if s.split_iter(sep).len() > 2 {
-        return None;
-    }
     match s.rfind(sep) {
         Some(i) => {
             debug!("in %s, i = %?", s, i);