rustpkg: Write more automated tests

Automate more tests described in the commands.txt file,
and add infrastructure for running them. Right now, tests shell
out to call rustpkg. This is not ideal.

10 files changed, 558 insertions, 120 deletions

diff --git a/src/librustpkg/messages.rs b/src/librustpkg/messages.rs
new file mode 100644
index 00000000000..d3962470e7c
--- /dev/null
+++ b/src/librustpkg/messages.rs
@@ -0,0 +1,40 @@
+// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use extra::term;
+use core::io;
+use core::result::*;
+
+pub fn note(msg: &str) {
+    pretty_message(msg, "note: ", term::color_green, io::stdout())
+}
+
+pub fn warn(msg: &str) {
+    pretty_message(msg, "warning: ", term::color_yellow, io::stdout())
+}
+
+pub fn error(msg: &str) {
+    pretty_message(msg, "error: ", term::color_red, io::stdout())
+}
+
+fn pretty_message<'a>(msg: &'a str, prefix: &'a str, color: u8, out: @io::Writer) {
+    let term = term::Terminal::new(out);
+    match term {
+        Ok(ref t) => {
+            t.fg(color);
+            out.write_str(prefix);
+            t.reset();
+        },
+        _ => {
+            out.write_str(prefix);
+        }
+    }
+    out.write_line(msg);
+}
diff --git a/src/librustpkg/package_id.rs b/src/librustpkg/package_id.rs
index 022acc29c8b..3ff0a6073b6 100644
--- a/src/librustpkg/package_id.rs
+++ b/src/librustpkg/package_id.rs
@@ -69,6 +69,7 @@ impl PkgId {
             }
         };
 
+        debug!("local_path = %s, remote_path = %s", local_path.to_str(), remote_path.to_str());
         PkgId {
             local_path: local_path,
             remote_path: remote_path,
@@ -90,11 +91,7 @@ impl PkgId {
 
 impl ToStr for PkgId {
     fn to_str(&self) -> ~str {
-        let maybe_dash = match self.version {
-            NoVersion => "",
-            _         => "-"
-        };
         // should probably use the filestem and not the whole path
-        fmt!("%s%s%s", self.local_path.to_str(), maybe_dash, self.version.to_str())
+        fmt!("%s-%s", self.local_path.to_str(), self.version.to_str())
     }
 }
diff --git a/src/librustpkg/package_source.rs b/src/librustpkg/package_source.rs
index 6d8a6095ca4..01cc48fc037 100644
--- a/src/librustpkg/package_source.rs
+++ b/src/librustpkg/package_source.rs
@@ -15,8 +15,9 @@ use core::option::*;
 use core::{os, run, str, vec};
 use context::*;
 use crate::Crate;
+use messages::*;
 use path_util::pkgid_src_in_workspace;
-use util::{compile_crate, note};
+use util::compile_crate;
 use version::{ExactRevision, SemanticVersion, NoVersion};
 
 // An enumeration of the unpacked source of a package workspace.
@@ -95,7 +96,7 @@ impl PkgSrc {
         };
 
 
-        note(fmt!("git clone %s %s %?", url, local.to_str(), branch_args));
+        note(fmt!("Fetching package: git clone %s %s %?", url, local.to_str(), branch_args));
 
         if run::process_output("git",
                                ~[~"clone", copy url, local.to_str()] + branch_args).status != 0 {
diff --git a/src/librustpkg/path_util.rs b/src/librustpkg/path_util.rs
index 964e1a54770..e68f48c8162 100644
--- a/src/librustpkg/path_util.rs
+++ b/src/librustpkg/path_util.rs
@@ -11,12 +11,16 @@
 // rustpkg utilities having to do with paths and directories
 
 use core::prelude::*;
-pub use package_path::{RemotePath, LocalPath};
+pub use package_path::{RemotePath, LocalPath, normalize};
 pub use package_id::PkgId;
 pub use target::{OutputType, Main, Lib, Test, Bench, Target, Build, Install};
+pub use version::{Version, NoVersion, split_version_general};
 use core::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR};
 use core::os::mkdir_recursive;
 use core::os;
+use core::iterator::IteratorUtil;
+use messages::*;
+use package_id::*;
 
 /// Returns the value of RUST_PATH, as a list
 /// of Paths. In general this should be read from the
@@ -38,8 +42,39 @@ pub fn make_dir_rwx(p: &Path) -> bool { os::make_dir(p, u_rwx) }
 /// True if there's a directory in <workspace> with
 /// pkgid's short name
 pub fn workspace_contains_package_id(pkgid: &PkgId, workspace: &Path) -> bool {
-    let pkgpath = workspace.push("src").push(pkgid.remote_path.to_str());
-    os::path_is_dir(&pkgpath)
+    let src_dir = workspace.push("src");
+    for os::list_dir(&src_dir).each |&p| {
+        let p = Path(p);
+        debug!("=> p = %s", p.to_str());
+        if !os::path_is_dir(&src_dir.push_rel(&p)) {
+            loop;
+        }
+        debug!("p = %s, remote_path = %s", p.to_str(), pkgid.remote_path.to_str());
+
+        if p == *pkgid.remote_path {
+            return true;
+        }
+        else {
+            let pf = p.filename();
+            for pf.iter().advance |&pf| {
+                let f_ = copy pf;
+                let g = f_.to_str();
+                match split_version_general(g, '-') {
+                    Some((ref might_match, ref vers)) => {
+                        debug!("might_match = %s, vers = %s", *might_match,
+                               vers.to_str());
+                        if *might_match == pkgid.short_name
+                            && (*vers == pkgid.version || pkgid.version == NoVersion)
+                        {
+                            return true;
+                        }
+                    }
+                    None => ()
+                }
+            }
+        }
+    }
+    false
 }
 
 /// Returns a list of possible directories
@@ -114,22 +149,22 @@ 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> {
-                        // passing in local_path here sounds fishy
-    library_in_workspace(pkgid.local_path.to_str(), pkgid.short_name, Build,
-                         workspace, "build")
+    library_in_workspace(&pkgid.local_path, pkgid.short_name,
+                         Build, workspace, "build")
 }
 
 /// Does the actual searching stuff
 pub fn installed_library_in_workspace(short_name: &str, workspace: &Path) -> Option<Path> {
-    library_in_workspace(short_name, short_name, Install, workspace, "lib")
+    library_in_workspace(&normalize(RemotePath(Path(short_name))),
+                         short_name, Install, workspace, "lib")
 }
 
 
 /// This doesn't take a PkgId, so we can use it for `extern mod` inference, where we
 /// don't know the entire package ID.
-/// `full_name` is used to figure out the directory to search.
+/// `workspace` is used to figure out the directory to search.
 /// `short_name` is taken as the link name of the library.
-fn library_in_workspace(full_name: &str, short_name: &str, where: Target,
+pub fn library_in_workspace(path: &LocalPath, short_name: &str, where: Target,
                         workspace: &Path, prefix: &str) -> Option<Path> {
     debug!("library_in_workspace: checking whether a library named %s exists",
            short_name);
@@ -137,8 +172,11 @@ fn library_in_workspace(full_name: &str, short_name: &str, where: Target,
     // We don't know what the hash is, so we have to search through the directory
     // contents
 
+    debug!("short_name = %s where = %? workspace = %s \
+            prefix = %s", short_name, where, workspace.to_str(), prefix);
+
     let dir_to_search = match where {
-        Build => workspace.push(prefix).push(full_name),
+        Build => workspace.push(prefix).push_rel(&**path),
         Install => workspace.push(prefix)
     };
     debug!("Listing directory %s", dir_to_search.to_str());
@@ -193,7 +231,11 @@ fn library_in_workspace(full_name: &str, short_name: &str, where: Target,
     // Return the filename that matches, which we now know exists
     // (if result_filename != None)
     match result_filename {
-        None => None,
+        None => {
+            warn(fmt!("library_in_workspace didn't find a library in %s for %s",
+                            dir_to_search.to_str(), short_name));
+            None
+        }
         Some(result_filename) => {
             let absolute_path = dir_to_search.push_rel(&result_filename);
             debug!("result_filename = %s", absolute_path.to_str());
@@ -210,17 +252,17 @@ pub fn target_executable_in_workspace(pkgid: &PkgId, workspace: &Path) -> Path {
 }
 
 
-/// Returns the installed path for <built_library> in <workspace>
+/// Returns the executable that would be installed for <pkgid>
+/// in <workspace>
 /// As a side effect, creates the lib-dir if it doesn't exist
-pub fn target_library_in_workspace(workspace: &Path,
-                                   built_library: &Path) -> Path {
+pub fn target_library_in_workspace(pkgid: &PkgId, workspace: &Path) -> Path {
     use conditions::bad_path::cond;
-    let result = workspace.push("lib");
-    if !os::path_exists(&result) && !mkdir_recursive(&result, u_rwx) {
-        cond.raise((copy result, ~"I couldn't create the library directory"));
+    if !os::path_is_dir(workspace) {
+        cond.raise((copy *workspace,
+                    fmt!("Workspace supplied to target_library_in_workspace \
+                          is not a directory! %s", workspace.to_str())));
     }
-    result.push(built_library.filename().expect(fmt!("I don't know how to treat %s as a library",
-                                                   built_library.to_str())))
+    target_file_in_workspace(pkgid, workspace, Lib, Install)
 }
 
 /// Returns the test executable that would be installed for <pkgid>
@@ -249,7 +291,9 @@ fn target_file_in_workspace(pkgid: &PkgId, workspace: &Path,
     };
     let result = workspace.push(subdir);
     if !os::path_exists(&result) && !mkdir_recursive(&result, u_rwx) {
-        cond.raise((copy result, fmt!("I couldn't create the %s dir", subdir)));
+        cond.raise((copy result, fmt!("target_file_in_workspace couldn't \
+            create the %s dir (pkgid=%s, workspace=%s, what=%?, where=%?",
+            subdir, pkgid.to_str(), workspace.to_str(), what, where)));
     }
     mk_output_path(what, where, pkgid, &result)
 }
@@ -275,7 +319,8 @@ pub fn build_pkg_id_in_workspace(pkgid: &PkgId, workspace: &Path) -> Path {
 /// given whether we're building a library and whether we're building tests
 pub fn mk_output_path(what: OutputType, where: Target,
                       pkg_id: &PkgId, workspace: &Path) -> Path {
-    let short_name_with_version = pkg_id.short_name_with_version();
+    let short_name_with_version = fmt!("%s-%s", pkg_id.short_name,
+                                       pkg_id.version.to_str());
     // Not local_path.dir_path()! For package foo/bar/blat/, we want
     // the executable blat-0.5 to live under blat/
     let dir = match where {
@@ -291,7 +336,7 @@ pub fn mk_output_path(what: OutputType, where: Target,
         // this code is duplicated from elsewhere; fix this
         Lib => dir.push(os::dll_filename(short_name_with_version)),
         // executable names *aren't* versioned
-        _ => dir.push(fmt!("%s%s%s", copy pkg_id.short_name,
+        _ => dir.push(fmt!("%s%s%s", pkg_id.short_name,
                            match what {
                                Test => "test",
                                Bench => "bench",
diff --git a/src/librustpkg/rustpkg.rc b/src/librustpkg/rustpkg.rc
index 2a851322356..9242e450e24 100644
--- a/src/librustpkg/rustpkg.rc
+++ b/src/librustpkg/rustpkg.rc
@@ -36,11 +36,12 @@ use rustc::metadata::filesearch;
 use extra::{getopts};
 use syntax::{ast, diagnostic};
 use util::*;
+use messages::*;
 use path_util::{build_pkg_id_in_workspace, first_pkgid_src_in_workspace};
-use path_util::u_rwx;
+use path_util::{u_rwx, rust_path};
 use path_util::{built_executable_in_workspace, built_library_in_workspace};
 use path_util::{target_executable_in_workspace, target_library_in_workspace};
-use workspace::pkg_parent_workspaces;
+use workspace::{each_pkg_parent_workspace, pkg_parent_workspaces};
 use context::Ctx;
 use package_id::PkgId;
 use package_source::PkgSrc;
@@ -48,6 +49,7 @@ use package_source::PkgSrc;
 mod conditions;
 mod context;
 mod crate;
+mod messages;
 mod package_id;
 mod package_path;
 mod package_source;
@@ -189,7 +191,7 @@ impl Ctx {
                 // The package id is presumed to be the first command-line
                 // argument
                 let pkgid = PkgId::new(copy args[0]);
-                for pkg_parent_workspaces(&pkgid) |workspace| {
+                for each_pkg_parent_workspace(&pkgid) |workspace| {
                     self.build(workspace, &pkgid);
                 }
             }
@@ -221,8 +223,19 @@ impl Ctx {
                 // The package id is presumed to be the first command-line
                 // argument
                 let pkgid = PkgId::new(args[0]);
-                for pkg_parent_workspaces(&pkgid) |workspace| {
-                    self.install(workspace, &pkgid);
+                let workspaces = pkg_parent_workspaces(&pkgid);
+                if workspaces.is_empty() {
+                    let rp = rust_path();
+                    assert!(!rp.is_empty());
+                    let src = PkgSrc::new(&rp[0], &build_pkg_id_in_workspace(&pkgid, &rp[0]),
+                                          &pkgid);
+                    src.fetch_git();
+                    self.install(&rp[0], &pkgid);
+                }
+                else {
+                    for each_pkg_parent_workspace(&pkgid) |workspace| {
+                        self.install(workspace, &pkgid);
+                    }
                 }
             }
             "prefer" => {
@@ -259,6 +272,8 @@ impl Ctx {
     }
 
     fn build(&self, workspace: &Path, pkgid: &PkgId) {
+        debug!("build: workspace = %s pkgid = %s", workspace.to_str(),
+               pkgid.to_str());
         let src_dir   = first_pkgid_src_in_workspace(pkgid, workspace);
         let build_dir = build_pkg_id_in_workspace(pkgid, workspace);
         debug!("Destination dir = %s", build_dir.to_str());
@@ -310,14 +325,14 @@ impl Ctx {
         // Do something reasonable for now
 
         let dir = build_pkg_id_in_workspace(id, workspace);
-        util::note(fmt!("Cleaning package %s (removing directory %s)",
+        note(fmt!("Cleaning package %s (removing directory %s)",
                         id.to_str(), dir.to_str()));
         if os::path_exists(&dir) {
             os::remove_dir_recursive(&dir);
-            util::note(fmt!("Removed directory %s", dir.to_str()));
+            note(fmt!("Removed directory %s", dir.to_str()));
         }
 
-        util::note(fmt!("Cleaned package %s", id.to_str()));
+        note(fmt!("Cleaned package %s", id.to_str()));
     }
 
     fn info(&self) {
@@ -338,7 +353,7 @@ impl Ctx {
         let maybe_executable = built_executable_in_workspace(id, workspace);
         let maybe_library = built_library_in_workspace(id, workspace);
         let target_exec = target_executable_in_workspace(id, workspace);
-        let target_lib = maybe_library.map(|p| target_library_in_workspace(workspace, p));
+        let target_lib = maybe_library.map(|_p| target_library_in_workspace(id, workspace));
 
         debug!("target_exec = %s target_lib = %? \
                 maybe_executable = %? maybe_library = %?",
@@ -392,7 +407,7 @@ pub fn main() {
     let matches = &match getopts::getopts(args, opts) {
         result::Ok(m) => m,
         result::Err(f) => {
-            util::error(fmt!("%s", getopts::fail_str(f)));
+            error(fmt!("%s", getopts::fail_str(f)));
 
             return;
         }
@@ -428,8 +443,12 @@ pub fn main() {
         };
     }
 
+    let sroot = match filesearch::get_rustpkg_sysroot() {
+        Ok(r) => Some(@r.pop().pop()), Err(_) => None
+    };
+    debug!("Using sysroot: %?", sroot);
     Ctx {
-        sysroot_opt: None, // Currently, only tests override this
+        sysroot_opt: sroot, // Currently, only tests override this
         json: json,
         dep_cache: @mut HashMap::new()
     }.run(cmd, args);
diff --git a/src/librustpkg/search.rs b/src/librustpkg/search.rs
index 987e01009fc..e5ffc5c9d84 100644
--- a/src/librustpkg/search.rs
+++ b/src/librustpkg/search.rs
@@ -22,4 +22,4 @@ pub fn find_library_in_search_path(sroot_opt: Option<@Path>, short_name: &str) -
         }
         None => None
     }
-}
\ No newline at end of file
+}
diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs
index b8cce8055d3..7d99a56e74a 100644
--- a/src/librustpkg/tests.rs
+++ b/src/librustpkg/tests.rs
@@ -12,21 +12,26 @@
 
 use context::Ctx;
 use core::hashmap::HashMap;
-use core::io;
-use core::os;
+use core::{io, libc, os, result, run, str};
 use core::prelude::*;
-use core::result;
 use extra::tempfile::mkdtemp;
+use core::run::ProcessOutput;
 use package_path::*;
-use package_id::PkgId;
+use package_id::{PkgId};
 use package_source::*;
 use version::{ExactRevision, NoVersion, Version};
 use path_util::{target_executable_in_workspace, target_library_in_workspace,
                target_test_in_workspace, target_bench_in_workspace,
-               make_dir_rwx, u_rwx,
+               make_dir_rwx, u_rwx, library_in_workspace,
                built_bench_in_workspace, built_test_in_workspace,
                built_library_in_workspace, built_executable_in_workspace,
                 installed_library_in_workspace};
+use target::*;
+
+/// Returns the last-modified date as an Option
+fn datestamp(p: &Path) -> Option<libc::time_t> {
+    p.stat().map(|stat| stat.st_mtime)
+}
 
 fn fake_ctxt(sysroot_opt: Option<@Path>) -> Ctx {
     Ctx {
@@ -65,23 +70,34 @@ fn writeFile(file_path: &Path, contents: &str) {
 }
 
 fn mk_empty_workspace(short_name: &LocalPath, version: &Version) -> Path {
-    let workspace = mkdtemp(&os::tmpdir(), "test").expect("couldn't create temp dir");
+    let workspace_dir = mkdtemp(&os::tmpdir(), "test").expect("couldn't create temp dir");
+    mk_workspace(&workspace_dir, short_name, version);
+    workspace_dir
+}
+
+fn mk_workspace(workspace: &Path, short_name: &LocalPath, version: &Version) -> Path {
     // include version number in directory name
-    let package_dir = workspace.push("src").push(fmt!("%s%s",
+    let package_dir = workspace.push("src").push(fmt!("%s-%s",
                                                       short_name.to_str(), version.to_str()));
     assert!(os::mkdir_recursive(&package_dir, u_rwx));
-    package_dir.pop().pop()
+    package_dir
 }
 
 fn mk_temp_workspace(short_name: &LocalPath, version: &Version) -> Path {
     let package_dir = mk_empty_workspace(short_name,
-                                         version).push("src").push(fmt!("%s%s",
+                                         version).push("src").push(fmt!("%s-%s",
                                                             short_name.to_str(),
                                                             version.to_str()));
 
     debug!("Created %s and does it exist? %?", package_dir.to_str(),
           os::path_is_dir(&package_dir));
     // Create main, lib, test, and bench files
+    debug!("mk_workspace: creating %s", package_dir.to_str());
+    assert!(os::mkdir_recursive(&package_dir, u_rwx));
+    debug!("Created %s and does it exist? %?", package_dir.to_str(),
+          os::path_is_dir(&package_dir));
+    // Create main, lib, test, and bench files
+
     writeFile(&package_dir.push("main.rs"),
               "fn main() { let _x = (); }");
     writeFile(&package_dir.push("lib.rs"),
@@ -90,7 +106,7 @@ fn mk_temp_workspace(short_name: &LocalPath, version: &Version) -> Path {
               "#[test] pub fn f() { (); }");
     writeFile(&package_dir.push("bench.rs"),
               "#[bench] pub fn f() { (); }");
-    package_dir.pop().pop()
+    package_dir
 }
 
 fn is_rwx(p: &Path) -> bool {
@@ -113,6 +129,186 @@ fn test_sysroot() -> Path {
     self_path.pop()
 }
 
+/// Runs `rustpkg` (based on the directory that this executable was
+/// invoked from) with the given arguments, in the given working directory.
+/// Returns the process's output.
+fn command_line_test(args: &[~str], cwd: &Path) -> ProcessOutput {
+    let cmd = test_sysroot().push("bin").push("rustpkg").to_str();
+    let cwd = normalize(RemotePath(copy *cwd));
+    debug!("About to run command: %? %? in %s", cmd, args, cwd.to_str());
+    assert!(os::path_is_dir(&*cwd));
+    let mut prog = run::Process::new(cmd, args, run::ProcessOptions { env: None,
+                                                           dir: Some(&*cwd),
+                                                           in_fd: None,
+                                                           out_fd: None,
+                                                           err_fd: None
+                                                          });
+    let output = prog.finish_with_output();
+    io::println(fmt!("Output from command %s with args %? was %s {%s}[%?]",
+                    cmd, args, str::from_bytes(output.output),
+                   str::from_bytes(output.error),
+                   output.status));
+/*
+By the way, rustpkg *won't* return a nonzero exit code if it fails --
+see #4547
+So tests that use this need to check the existence of a file
+to make sure the command succeeded
+*/
+    if output.status != 0 {
+        fail!("Command %s %? failed with exit code %?",
+              cmd, args, output.status);
+    }
+    output
+}
+
+fn make_git_repo(short_name: &str) -> Path {
+    let temp_d = mk_temp_workspace(&normalize(RemotePath(Path(short_name))), &NoVersion);
+    debug!("Dry run: would initialize %s as a git repository", temp_d.pop().pop().to_str());
+    temp_d.pop().pop()
+}
+
+fn add_git_tag(repo: &Path, tag: &str) {
+    debug!("Dry run: would add tag %s to repo %s", tag, repo.to_str());
+}
+
+fn create_local_package(pkgid: &PkgId) -> Path {
+    let parent_dir = mk_temp_workspace(&pkgid.local_path, &pkgid.version);
+    debug!("Created empty package dir for %s, returning %s", pkgid.to_str(), parent_dir.to_str());
+    parent_dir.pop().pop()
+}
+
+fn create_local_package_in(pkgid: &PkgId, pkgdir: &Path) -> Path {
+
+    let package_dir = pkgdir.push("src").push(pkgid.to_str());
+
+    // Create main, lib, test, and bench files
+    assert!(os::mkdir_recursive(&package_dir, u_rwx));
+    debug!("Created %s and does it exist? %?", package_dir.to_str(),
+          os::path_is_dir(&package_dir));
+    // Create main, lib, test, and bench files
+
+    writeFile(&package_dir.push("main.rs"),
+              "fn main() { let _x = (); }");
+    writeFile(&package_dir.push("lib.rs"),
+              "pub fn f() { let _x = (); }");
+    writeFile(&package_dir.push("test.rs"),
+              "#[test] pub fn f() { (); }");
+    writeFile(&package_dir.push("bench.rs"),
+              "#[bench] pub fn f() { (); }");
+    package_dir
+}
+
+fn create_local_package_with_test(pkgid: &PkgId) -> Path {
+    debug!("Dry run -- would create package %s with test");
+    create_local_package(pkgid) // Already has tests???
+}
+
+fn create_local_package_with_dep(pkgid: &PkgId, subord_pkgid: &PkgId) -> Path {
+    let package_dir = create_local_package(pkgid);
+    create_local_package_in(subord_pkgid, &package_dir);
+    // Write a main.rs file into pkgid that references subord_pkgid
+    writeFile(&package_dir.push("src").push(pkgid.to_str()).push("main.rs"),
+              fmt!("extern mod %s;\nfn main() {}",
+                   subord_pkgid.short_name));
+    // Write a lib.rs file into subord_pkgid that has something in it
+    writeFile(&package_dir.push("src").push(subord_pkgid.to_str()).push("lib.rs"),
+              "pub fn f() {}");
+    debug!("Dry run -- would create packages %s and %s in %s",
+           pkgid.to_str(),
+           subord_pkgid.to_str(),
+           package_dir.to_str());
+    package_dir
+}
+
+fn create_local_package_with_custom_build_hook(pkgid: &PkgId,
+                                               custom_build_hook: &str) -> Path {
+    debug!("Dry run -- would create package %s with custom build hook %s",
+           pkgid.to_str(), custom_build_hook);
+    create_local_package(pkgid)
+    // actually write the pkg.rs with the custom build hook
+
+}
+
+fn assert_lib_exists(repo: &Path, short_name: &str) {
+    debug!("assert_lib_exists: repo = %s, short_name = %s", repo.to_str(), short_name);
+    let lib = target_library_in_workspace(&PkgId::new(short_name), repo);
+    assert!(os::path_exists(&lib));
+    assert!(is_rwx(&lib));
+}
+
+fn assert_executable_exists(repo: &Path, short_name: &str) {
+    debug!("assert_executable_exists: repo = %s, short_name = %s", repo.to_str(), short_name);
+    let exec = target_executable_in_workspace(&PkgId::new(short_name), repo);
+    assert!(os::path_exists(&exec));
+    assert!(is_rwx(&exec));
+}
+
+fn command_line_test_output(args: &[~str]) -> ~[~str] {
+    let mut result = ~[];
+    let p_output = command_line_test(args, &os::getcwd());
+    let test_output = str::from_bytes(p_output.output);
+    for test_output.split_iter('\n').advance |s| {
+        result += [s.to_owned()];
+    }
+    result
+}
+
+// assumes short_name and local_path are one and the same -- I should fix
+fn lib_output_file_name(workspace: &Path, parent: &str, short_name: &str) -> Path {
+    debug!("lib_output_file_name: given %s and parent %s and short name %s",
+           workspace.to_str(), parent, short_name);
+    library_in_workspace(&normalize(RemotePath(Path(short_name))),
+                         short_name,
+                         Build,
+                         workspace,
+                         "build").expect("lib_output_file_name")
+}
+
+fn output_file_name(workspace: &Path, short_name: &str) -> Path {
+    workspace.push(fmt!("%s%s", short_name, os::EXE_SUFFIX))
+}
+
+fn touch_source_file(workspace: &Path, short_name: &str) {
+    use conditions::bad_path::cond;
+    let pkg_src_dir = workspace.push("src").push(short_name);
+    let contents = os::list_dir(&pkg_src_dir);
+    for contents.each() |p| {
+        if Path(copy *p).filetype() == Some(~".rs") {
+            // should be able to do this w/o a process
+            if run::process_output("touch", [p.to_str()]).status != 0 {
+                let _ = cond.raise((copy pkg_src_dir, ~"Bad path"));
+            }
+            break;
+        }
+    }
+}
+
+/// Add a blank line at the end
+fn frob_source_file(workspace: &Path, pkgid: &PkgId) {
+    use conditions::bad_path::cond;
+    let pkg_src_dir = workspace.push("src").push(pkgid.to_str());
+    let contents = os::list_dir(&pkg_src_dir);
+    let mut maybe_p = None;
+    for contents.each() |p| {
+        if Path(copy *p).filetype() == Some(~".rs") {
+            maybe_p = Some(p);
+            break;
+        }
+    }
+    match maybe_p {
+        Some(p) => {
+            let p = Path(copy *p);
+            let w = io::buffered_file_writer(&p);
+            match w {
+                Err(s) => { let _ = cond.raise((p, fmt!("Bad path: %s", s))); }
+                Ok(w)  => w.write_line("")
+            }
+        }
+        None => fail!(fmt!("frob_source_file failed to find a source file in %s",
+                           pkg_src_dir.to_str()))
+    }
+}
+
 #[test]
 fn test_make_dir_rwx() {
     let temp = &os::tmpdir();
@@ -134,7 +330,8 @@ fn test_install_valid() {
     debug!("sysroot = %s", sysroot.to_str());
     let ctxt = fake_ctxt(Some(@sysroot));
     let temp_pkg_id = fake_pkg();
-    let temp_workspace = mk_temp_workspace(&temp_pkg_id.local_path, &NoVersion);
+    let temp_workspace = mk_temp_workspace(&temp_pkg_id.local_path, &NoVersion).pop().pop();
+    debug!("temp_workspace = %s", temp_workspace.to_str());
     // should have test, bench, lib, and main
     ctxt.install(&temp_workspace, &temp_pkg_id);
     // Check that all files exist
@@ -192,10 +389,10 @@ fn test_install_url() {
     debug!("exec = %s", exec.to_str());
     assert!(os::path_exists(&exec));
     assert!(is_rwx(&exec));
-    let built_lib =
+    let _built_lib =
         built_library_in_workspace(&temp_pkg_id,
                                    &workspace).expect("test_install_url: built lib should exist");
-    let lib = target_library_in_workspace(&workspace, &built_lib);
+    let lib = target_library_in_workspace(&temp_pkg_id, &workspace);
     debug!("lib = %s", lib.to_str());
     assert!(os::path_exists(&lib));
     assert!(is_rwx(&lib));
@@ -231,8 +428,8 @@ fn test_package_ids_must_be_relative_path_like() {
 
     let whatever = PkgId::new("foo");
 
-    assert_eq!(~"foo", whatever.to_str());
-    assert!("github.com/catamorphism/test_pkg" ==
+    assert_eq!(~"foo-0.1", whatever.to_str());
+    assert!("github.com/catamorphism/test_pkg-0.1" ==
             PkgId::new("github.com/catamorphism/test-pkg").to_str());
 
     do cond.trap(|(p, e)| {
@@ -241,7 +438,7 @@ fn test_package_ids_must_be_relative_path_like() {
         copy whatever
     }).in {
         let x = PkgId::new("");
-        assert_eq!(~"foo", x.to_str());
+        assert_eq!(~"foo-0.1", x.to_str());
     }
 
     do cond.trap(|(p, e)| {
@@ -250,7 +447,7 @@ fn test_package_ids_must_be_relative_path_like() {
         copy whatever
     }).in {
         let z = PkgId::new(os::make_absolute(&Path("foo/bar/quux")).to_str());
-        assert_eq!(~"foo", z.to_str());
+        assert_eq!(~"foo-0.1", z.to_str());
     }
 
 }
@@ -317,3 +514,164 @@ fn test_package_request_version() {
             == temp.push("bin").push("test_pkg_version"));
 
 }
+
+// Tests above should (maybe) be converted to shell out to rustpkg, too
+
+#[test]
+#[ignore (reason = "http-client not ported to rustpkg yet")]
+fn rustpkg_install_url_2() {
+    let temp_dir = mkdtemp(&os::tmpdir(), "rustpkg_install_url_2").expect("rustpkg_install_url_2");
+    command_line_test([~"install", ~"github.com/mozilla-servo/rust-http-client"],
+                     &temp_dir);
+}
+
+#[test]
+fn rustpkg_library_target() {
+    let foo_repo = make_git_repo("foo");
+    add_git_tag(&foo_repo, "1.0");
+    command_line_test([~"install", ~"foo"], &foo_repo);
+    assert_lib_exists(&foo_repo, "foo");
+}
+
+#[test]
+fn rustpkg_local_pkg() {
+    let dir = create_local_package(&PkgId::new("foo"));
+    command_line_test([~"install", ~"foo"], &dir);
+    assert_executable_exists(&dir, "foo");
+}
+
+#[test]
+#[ignore (reason = "RUST_PATH not yet implemented -- #5682")]
+fn rust_path_test() {
+    let dir = mk_workspace(&Path("/home/more_rust"),
+                           &normalize(RemotePath(Path("foo"))),
+                           &NoVersion);
+  //  command_line_test("RUST_PATH=/home/rust:/home/more_rust rustpkg install foo");
+    command_line_test([~"install", ~"foo"], &dir);
+}
+
+#[test]
+#[ignore(reason = "Package database not yet implemented")]
+fn install_remove() {
+    let foo = PkgId::new("foo");
+    let bar = PkgId::new("bar");
+    let quux = PkgId::new("quux");
+    let dir = mkdtemp(&os::tmpdir(), "install_remove").expect("install_remove");
+    create_local_package_in(&foo, &dir);
+    create_local_package_in(&bar, &dir);
+    create_local_package_in(&quux, &dir);
+    command_line_test([~"install", ~"foo"], &dir);
+    command_line_test([~"install", ~"bar"], &dir);
+    command_line_test([~"install", ~"quux"], &dir);
+    let list_output = command_line_test_output([~"list"]);
+    assert!(list_output.contains(&~"foo"));
+    assert!(list_output.contains(&~"bar"));
+    assert!(list_output.contains(&~"quux"));
+    command_line_test([~"remove", ~"foo"], &dir);
+    let list_output = command_line_test_output([~"list"]);
+    assert!(!list_output.contains(&~"foo"));
+    assert!(list_output.contains(&~"bar"));
+    assert!(list_output.contains(&~"quux"));
+}
+
+#[test]
+#[ignore(reason = "Workcache not yet implemented -- see #7075")]
+fn no_rebuilding() {
+    let p_id = PkgId::new("foo");
+    let workspace = create_local_package(&p_id);
+    command_line_test([~"build", ~"foo"], &workspace);
+    let date = datestamp(&built_library_in_workspace(&p_id,
+                                                    &workspace).expect("no_rebuilding"));
+    command_line_test([~"build", ~"foo"], &workspace);
+    let newdate = datestamp(&built_library_in_workspace(&p_id,
+                                                       &workspace).expect("no_rebuilding (2)"));
+    assert_eq!(date, newdate);
+}
+
+#[test]
+#[ignore(reason = "Workcache not yet implemented -- see #7075")]
+fn no_rebuilding_dep() {
+    let p_id = PkgId::new("foo");
+    let dep_id = PkgId::new("bar");
+    let workspace = create_local_package_with_dep(&p_id, &dep_id);
+    command_line_test([~"build", ~"foo"], &workspace);
+    let bar_date = datestamp(&lib_output_file_name(&workspace,
+                                                  ".rust",
+                                                  "bar"));
+    let foo_date = datestamp(&output_file_name(&workspace, "foo"));
+    assert!(bar_date < foo_date);
+}
+
+#[test]
+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);
+    command_line_test([~"build", ~"foo"], &workspace);
+    let bar_date = datestamp(&lib_output_file_name(&workspace, "build", "bar"));
+    touch_source_file(&workspace, "bar");
+    command_line_test([~"build", ~"foo"], &workspace);
+    let new_bar_date = datestamp(&lib_output_file_name(&workspace, "build", "bar"));
+    assert!(new_bar_date > bar_date);
+}
+
+#[test]
+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);
+    command_line_test([~"build", ~"foo"], &workspace);
+    let bar_date = datestamp(&lib_output_file_name(&workspace, "build", "bar"));
+    frob_source_file(&workspace, &dep_id);
+    // should adjust the datestamp
+    command_line_test([~"build", ~"foo"], &workspace);
+    let new_bar_date = datestamp(&lib_output_file_name(&workspace, "build", "bar"));
+    assert!(new_bar_date > bar_date);
+}
+
+#[test]
+#[ignore(reason = "list not yet implemented")]
+fn test_versions() {
+    let workspace = create_local_package(&PkgId::new("foo#0.1"));
+    create_local_package(&PkgId::new("foo#0.2"));
+    command_line_test([~"install", ~"foo#0.1"], &workspace);
+    let output = command_line_test_output([~"list"]);
+    // make sure output includes versions
+    assert!(!output.contains(&~"foo#0.2"));
+}
+
+#[test]
+#[ignore(reason = "do not yet implemented")]
+fn test_build_hooks() {
+    let workspace = create_local_package_with_custom_build_hook(&PkgId::new("foo"), "frob");
+    command_line_test([~"do", ~"foo", ~"frob"], &workspace);
+}
+
+
+#[test]
+#[ignore(reason = "info not yet implemented")]
+fn test_info() {
+    let expected_info = ~"package foo"; // fill in
+    let workspace = create_local_package(&PkgId::new("foo"));
+    let output = command_line_test([~"info", ~"foo"], &workspace);
+    assert_eq!(str::from_bytes(output.output), expected_info);
+}
+
+#[test]
+#[ignore(reason = "test not yet implemented")]
+fn test_rustpkg_test() {
+    let expected_results = ~"1 out of 1 tests passed"; // fill in
+    let workspace = create_local_package_with_test(&PkgId::new("foo"));
+    let output = command_line_test([~"test", ~"foo"], &workspace);
+    assert_eq!(str::from_bytes(output.output), expected_results);
+}
+
+#[test]
+#[ignore(reason = "uninstall not yet implemented")]
+fn test_uninstall() {
+    let workspace = create_local_package(&PkgId::new("foo"));
+    let _output = command_line_test([~"info", ~"foo"], &workspace);
+    command_line_test([~"uninstall", ~"foo"], &workspace);
+    let output = command_line_test([~"list"], &workspace);
+    assert!(!str::from_bytes(output.output).contains("foo"));
+}
diff --git a/src/librustpkg/util.rs b/src/librustpkg/util.rs
index 1b863cd9a86..60fe7d52321 100644
--- a/src/librustpkg/util.rs
+++ b/src/librustpkg/util.rs
@@ -9,11 +9,10 @@
 // except according to those terms.
 
 use core::prelude::*;
-use core::{io, libc, os, result, str};
+use core::{libc, os, result, str};
 use rustc::driver::{driver, session};
 use rustc::metadata::filesearch;
 use extra::getopts::groups::getopts;
-use extra::term;
 use syntax::ast_util::*;
 use syntax::codemap::{dummy_sp, spanned};
 use syntax::codemap::dummy_spanned;
@@ -26,8 +25,8 @@ use rustc::driver::driver::compile_upto;
 use rustc::driver::session::{lib_crate, bin_crate};
 use context::Ctx;
 use package_id::PkgId;
-use path_util::{target_library_in_workspace, built_library_in_workspace};
 use search::find_library_in_search_path;
+use path_util::target_library_in_workspace;
 pub use target::{OutputType, Main, Lib, Bench, Test};
 
 static Commands: &'static [&'static str] =
@@ -160,33 +159,6 @@ pub fn need_dir(s: &Path) {
     }
 }
 
-fn pretty_message<'a>(msg: &'a str, prefix: &'a str, color: u8, out: @io::Writer) {
-    let term = term::Terminal::new(out);
-    match term {
-        Ok(ref t) => {
-            t.fg(color);
-            out.write_str(prefix);
-            t.reset();
-        },
-        _ => {
-            out.write_str(prefix);
-        }
-    }
-    out.write_line(msg);
-}
-
-pub fn note(msg: &str) {
-    pretty_message(msg, "note: ", term::color_green, io::stdout())
-}
-
-pub fn warn(msg: &str) {
-    pretty_message(msg, "warning: ", term::color_yellow, io::stdout())
-}
-
-pub fn error(msg: &str) {
-    pretty_message(msg, "error: ", term::color_red, io::stdout())
-}
-
 // FIXME (#4432): Use workcache to only compile when needed
 pub fn compile_input(ctxt: &Ctx,
                      pkg_id: &PkgId,
@@ -230,13 +202,18 @@ pub fn compile_input(ctxt: &Ctx,
         optimize: if opt { session::Aggressive } else { session::No },
         test: what == Test || what == Bench,
         maybe_sysroot: ctxt.sysroot_opt,
-        addl_lib_search_paths: @mut ~[copy *out_dir],
+        addl_lib_search_paths: @mut (~[copy *out_dir]),
         // output_type should be conditional
         output_type: output_type_exe, // Use this to get a library? That's weird
         .. copy *driver::build_session_options(binary, &matches, diagnostic::emit)
     };
 
     let addl_lib_search_paths = @mut options.addl_lib_search_paths;
+    // Make sure all the library directories actually exist, since the linker will complain
+    // otherwise
+    for addl_lib_search_paths.each() |p| {
+        assert!(os::path_is_dir(p));
+    }
 
     let sess = driver::build_session(options, diagnostic::emit);
 
@@ -274,7 +251,7 @@ pub fn compile_input(ctxt: &Ctx,
                  ~[@dummy_spanned(meta_name_value(@"name",
                                       mk_string_lit(short_name_to_use.to_managed()))),
                    @dummy_spanned(meta_name_value(@"vers",
-                         mk_string_lit(pkg_id.version.to_str_nonempty().to_managed())))])))],
+                         mk_string_lit(pkg_id.version.to_str().to_managed())))])))],
             ..copy crate.node});
     }
 
@@ -340,10 +317,11 @@ pub fn compile_crate(ctxt: &Ctx, pkg_id: &PkgId,
     compile_input(ctxt, pkg_id, crate, dir, flags, cfgs, opt, what)
 }
 
+
 /// Collect all `extern mod` directives in `c`, then
 /// try to install their targets, failing if any target
 /// can't be found.
-fn find_and_install_dependencies(ctxt: &Ctx,
+pub fn find_and_install_dependencies(ctxt: &Ctx,
                                  sess: session::Session,
                                  workspace: &Path,
                                  c: &ast::crate,
@@ -360,7 +338,7 @@ fn find_and_install_dependencies(ctxt: &Ctx,
             ast::view_item_extern_mod(lib_ident, _, _) => {
                 match my_ctxt.sysroot_opt {
                     Some(ref x) => debug!("sysroot: %s", x.to_str()),
-                    None => ()
+                    None => debug!("No sysroot given")
                 };
                 let lib_name = sess.str_of(lib_ident);
                 match find_library_in_search_path(my_ctxt.sysroot_opt, lib_name) {
@@ -371,14 +349,10 @@ fn find_and_install_dependencies(ctxt: &Ctx,
                         // Try to install it
                         let pkg_id = PkgId::new(lib_name);
                         my_ctxt.install(&my_workspace, &pkg_id);
-                        let built_lib =
-                            built_library_in_workspace(&pkg_id,
-                                &my_workspace).expect(fmt!("find_and_install_dependencies: \
-                                I thought I already built %s, but the library doesn't seem \
-                                to exist", lib_name));
                         // Also, add an additional search path
-                        let installed_path = target_library_in_workspace(&my_workspace,
-                                                                         &built_lib).pop();
+                        debug!("let installed_path...")
+                        let installed_path = target_library_in_workspace(&pkg_id,
+                                                                         &my_workspace).pop();
                         debug!("Great, I installed %s, and it's in %s",
                                lib_name, installed_path.to_str());
                         save(installed_path);
diff --git a/src/librustpkg/version.rs b/src/librustpkg/version.rs
index 0601c33b25e..7431b5e4c01 100644
--- a/src/librustpkg/version.rs
+++ b/src/librustpkg/version.rs
@@ -23,13 +23,14 @@ use extra::tempfile::mkdtemp;
 pub enum Version {
     ExactRevision(~str), // Should look like a m.n.(...).x
     SemanticVersion(semver::Version),
-    NoVersion // user didn't specify a version
+    NoVersion // user didn't specify a version -- prints as 0.1
 }
 
 
 impl Ord for Version {
     fn lt(&self, other: &Version) -> bool {
         match (self, other) {
+            (&NoVersion, _) => true,
             (&ExactRevision(ref f1), &ExactRevision(ref f2)) => f1 < f2,
             (&SemanticVersion(ref v1), &SemanticVersion(ref v2)) => v1 < v2,
             _ => false // incomparable, really
@@ -37,6 +38,7 @@ impl Ord for Version {
     }
     fn le(&self, other: &Version) -> bool {
         match (self, other) {
+            (&NoVersion, _) => true,
             (&ExactRevision(ref f1), &ExactRevision(ref f2)) => f1 <= f2,
             (&SemanticVersion(ref v1), &SemanticVersion(ref v2)) => v1 <= v2,
             _ => false // incomparable, really
@@ -64,23 +66,11 @@ impl ToStr for Version {
         match *self {
             ExactRevision(ref n) => fmt!("%s", n.to_str()),
             SemanticVersion(ref v) => fmt!("%s", v.to_str()),
-            NoVersion => ~""
+            NoVersion => ~"0.1"
         }
     }
 }
 
-impl Version {
-    /// Fills in a bogus default version for NoVersion -- for use when
-    /// injecting link_meta attributes
-    fn to_str_nonempty(&self) -> ~str {
-        match *self {
-            NoVersion => ~"0.1",
-            _ => self.to_str()
-        }
-    }
-}
-
-
 pub fn parse_vers(vers: ~str) -> result::Result<semver::Version, ~str> {
     match semver::parse(vers) {
         Some(vers) => result::Ok(vers),
@@ -98,7 +88,9 @@ pub fn try_getting_version(remote_path: &RemotePath) -> Option<Version> {
         debug!("Trying to fetch its sources..");
         let tmp_dir = mkdtemp(&os::tmpdir(),
                               "test").expect("try_getting_version: couldn't create temp dir");
-        debug!("executing {git clone https://%s %s}", remote_path.to_str(), tmp_dir.to_str());
+        debug!("(to get version) executing {git clone https://%s %s}",
+               remote_path.to_str(),
+               tmp_dir.to_str());
         let outp  = run::process_output("git", [~"clone", fmt!("https://%s", remote_path.to_str()),
                                                 tmp_dir.to_str()]);
         if outp.status == 0 {
@@ -106,7 +98,8 @@ pub fn try_getting_version(remote_path: &RemotePath) -> Option<Version> {
                    str::from_bytes(outp.output),
                    str::from_bytes(outp.error));
             let mut output = None;
-            debug!("executing {git --git-dir=%s tag -l}", tmp_dir.push(".git").to_str());
+            debug!("(getting version, now getting tags) executing {git --git-dir=%s tag -l}",
+                   tmp_dir.push(".git").to_str());
             let outp = run::process_output("git",
                                            [fmt!("--git-dir=%s", tmp_dir.push(".git").to_str()),
                                             ~"tag", ~"-l"]);
@@ -169,11 +162,18 @@ fn is_url_like(p: &RemotePath) -> 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, '#')
+}
+
+pub fn split_version_general<'a>(s: &'a str, sep: char) -> Option<(&'a str, Version)> {
     // reject strings with multiple '#'s
-    if s.splitn_iter('#', 2).count() > 2 {
+    for s.split_iter(sep).advance |st| {
+        debug!("whole = %s part = %s", s, st);
+    }
+    if s.split_iter(sep).count() > 2 {
         return None;
     }
-    match s.rfind('#') {
+    match s.rfind(sep) {
         Some(i) => {
             debug!("in %s, i = %?", s, i);
             let path = s.slice(0, i);
diff --git a/src/librustpkg/workspace.rs b/src/librustpkg/workspace.rs
index 3010e27385f..54144f8e31f 100644
--- a/src/librustpkg/workspace.rs
+++ b/src/librustpkg/workspace.rs
@@ -14,11 +14,10 @@ use path_util::{rust_path, workspace_contains_package_id};
 use package_id::PkgId;
 use core::path::Path;
 
-pub fn pkg_parent_workspaces(pkgid: &PkgId, action: &fn(&Path) -> bool) -> bool {
+pub fn each_pkg_parent_workspace(pkgid: &PkgId, action: &fn(&Path) -> bool) -> bool {
     // Using the RUST_PATH, find workspaces that contain
     // this package ID
-    let workspaces = rust_path().filtered(|ws|
-        workspace_contains_package_id(pkgid, ws));
+    let workspaces = pkg_parent_workspaces(pkgid);
     if workspaces.is_empty() {
         // tjc: make this a condition
         fail!("Package %s not found in any of \
@@ -33,3 +32,8 @@ pub fn pkg_parent_workspaces(pkgid: &PkgId, action: &fn(&Path) -> bool) -> bool
     }
     return true;
 }
+
+pub fn pkg_parent_workspaces(pkgid: &PkgId) -> ~[Path] {
+    rust_path().filtered(|ws|
+        workspace_contains_package_id(pkgid, ws))
+}
\ No newline at end of file