rustpkg: Address review comments from Jack

Author: catamorphism

src/librustpkg/api.rs

@@ -17,21 +17,22 @@ use workcache_support::*;
 
 use extra::arc::{Arc,RWArc};
 use extra::workcache;
-use extra::workcache::*;
-use std::os;
+use extra::workcache::{Database, Logger, FreshnessMap};
 use extra::treemap::TreeMap;
 
 /// Convenience functions intended for calling from pkg.rs
 /// p is where to put the cache file for dependencies
-pub fn default_ctxt(p: Path) -> BuildCtx {
-    new_default_ctx(new_workcache_cx(&p), p)
+pub fn default_context(p: Path) -> BuildContext {
+    new_default_context(new_workcache_context(&p), p)
 }
 
-pub fn new_default_ctx(c: Context, p: Path) -> BuildCtx {
-    BuildCtx {
-        cx: Ctx { use_rust_path_hack: false,
-                  sysroot_opt: p },
-        workcache_cx: c
+pub fn new_default_context(c: workcache::Context, p: Path) -> BuildContext {
+    BuildContext {
+        context: Context {
+            use_rust_path_hack: false,
+            sysroot: p
+        },
+        workcache_context: c
     }
 }
 
@@ -44,27 +45,27 @@ fn binary_is_fresh(path: &str, in_hash: &str) -> bool {
 }
 
 
-pub fn new_workcache_cx(p: &Path) -> Context {
+pub fn new_workcache_context(p: &Path) -> workcache::Context {
     let db_file = p.push("rustpkg_db.json"); // ??? probably wrong
     debug!("Workcache database file: %s", db_file.to_str());
     let db = RWArc::new(Database::new(db_file));
     let lg = RWArc::new(Logger::new());
     let cfg = Arc::new(TreeMap::new());
-    let mut rslt: FreshnessMap = TreeMap::new();
+    let mut freshness: FreshnessMap = TreeMap::new();
     // Set up freshness functions for every type of dependency rustpkg
     // knows about
-    rslt.insert(~"file", file_is_fresh);
-    rslt.insert(~"binary", binary_is_fresh);
-    workcache::Context::new_with_freshness(db, lg, cfg, Arc::new(rslt))
+    freshness.insert(~"file", file_is_fresh);
+    freshness.insert(~"binary", binary_is_fresh);
+    workcache::Context::new_with_freshness(db, lg, cfg, Arc::new(freshness))
 }
 
 pub fn build_lib(sysroot: Path, root: Path, name: ~str, version: Version,
                  lib: Path) {
-    let cx = default_ctxt(sysroot);
+    let cx = default_context(sysroot);
     let subroot = root.clone();
     let subversion = version.clone();
     let sublib = lib.clone();
-    do cx.workcache_cx.with_prep(name) |prep| {
+    do cx.workcache_context.with_prep(name) |prep| {
         let pkg_src = PkgSrc {
             workspace: subroot.clone(),
             start_dir: subroot.push("src").push(name),
@@ -78,17 +79,17 @@ pub fn build_lib(sysroot: Path, root: Path, name: ~str, version: Version,
         let subcx = cx.clone();
         let subsrc = pkg_src.clone();
         do prep.exec |exec| {
-            subsrc.clone().build(exec, &subcx.clone(), ~[]);
+            subsrc.build(exec, &subcx.clone(), ~[]);
         }
     };
 }
 
 pub fn build_exe(sysroot: Path, root: Path, name: ~str, version: Version,
                  main: Path) {
-    let cx = default_ctxt(sysroot);
+    let cx = default_context(sysroot);
     let subroot = root.clone();
     let submain = main.clone();
-    do cx.workcache_cx.with_prep(name) |prep| {
+    do cx.workcache_context.with_prep(name) |prep| {
         let pkg_src = PkgSrc {
             workspace: subroot.clone(),
             start_dir: subroot.push("src").push(name),
@@ -107,41 +108,8 @@ pub fn build_exe(sysroot: Path, root: Path, name: ~str, version: Version,
     }
 }
 
-pub fn install_lib(sysroot: Path,
-                   workspace: Path,
-                   name: ~str,
-                   lib_path: Path,
-                   version: Version) {
-    debug!("self_exe: %?", os::self_exe_path());
-    debug!("sysroot = %s", sysroot.to_str());
-    debug!("workspace = %s", workspace.to_str());
-    // make a PkgSrc
-    let pkg_id = PkgId{ version: version, ..PkgId::new(name)};
-    let cx = default_ctxt(sysroot);
-    let subpath = lib_path.clone();
-    do cx.workcache_cx.with_prep(pkg_id.to_str()) |prep| {
-        let pkg_src = PkgSrc {
-            workspace: workspace.clone(),
-            start_dir: subpath.push("src").push(name),
-            id: pkg_id.clone(),
-            libs: ~[mk_crate(subpath.clone())],
-            mains: ~[],
-            tests: ~[],
-            benchs: ~[]
-        };
-        pkg_src.declare_inputs(prep);
-        let subcx = cx.clone();
-        let subpkg_src = pkg_src.clone();
-        do prep.exec |exec| {
-            subpkg_src.clone().build(exec, &subcx.clone(), ~[]);
-        }
-    }
-    cx.install_no_build(&workspace, &pkg_id);
-}
-
-pub fn install_exe(sysroot: Path, workspace: Path, name: ~str, version: Version) {
-    let cx = default_ctxt(sysroot);
-    debug!("install_exe calling with_prep");
+pub fn install_pkg(sysroot: Path, workspace: Path, name: ~str, version: Version) {
+    let cx = default_context(sysroot);
     let pkgid = PkgId{ version: version, ..PkgId::new(name)};
     cx.install(PkgSrc::new(workspace, false, pkgid));
 }

src/librustpkg/context.rs

@@ -14,62 +14,61 @@ use std::os;
 use extra::workcache;
 
 #[deriving(Clone)]
-pub struct Ctx {
+pub struct Context {
     // If use_rust_path_hack is true, rustpkg searches for sources
     // in *package* directories that are in the RUST_PATH (for example,
     // FOO/src/bar-0.1 instead of FOO). The flag doesn't affect where
     // rustpkg stores build artifacts.
     use_rust_path_hack: bool,
     // The root directory containing the Rust standard libraries
-    sysroot_opt: Path
+    sysroot: Path
 }
 
 #[deriving(Clone)]
-pub struct BuildCtx {
+pub struct BuildContext {
     // Context for workcache
-    workcache_cx: workcache::Context,
+    workcache_context: workcache::Context,
     // Everything else
-    cx: Ctx
+    context: Context
 }
 
-impl BuildCtx {
-    pub fn sysroot_opt(&self) -> Path {
-        self.cx.sysroot_opt.clone()
+impl BuildContext {
+    pub fn sysroot(&self) -> Path {
+        self.context.sysroot.clone()
     }
 
     pub fn sysroot_to_use(&self) -> Path {
-        self.cx.sysroot_to_use()
+        self.context.sysroot_to_use()
     }
 }
 
-impl Ctx {
-    pub fn sysroot_opt(&self) -> Path {
-        self.sysroot_opt.clone()
+impl Context {
+    pub fn sysroot(&self) -> Path {
+        self.sysroot.clone()
     }
 }
 
-impl Ctx {
+impl Context {
     /// Debugging
-    pub fn sysroot_opt_str(&self) -> ~str {
-        self.sysroot_opt.to_str()
+    pub fn sysroot_str(&self) -> ~str {
+        self.sysroot.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) -> Path {
-        if !in_target(&self.sysroot_opt) {
-            self.sysroot_opt.clone()
+        if !in_target(&self.sysroot) {
+            self.sysroot.clone()
+        } else {
+            self.sysroot.pop().pop().pop()
         }
-        else {
-            self.sysroot_opt.pop().pop().pop()
-        }
-  }
+    }
 }
 
 /// We assume that if ../../rustc exists, then we're running
 /// rustpkg from a Rust target directory. This is part of a
 /// kludgy hack used to adjust the sysroot.
-pub fn in_target(sysroot_opt: &Path) -> bool {
-    debug!("Checking whether %s is in target", sysroot_opt.to_str());
-    os::path_is_dir(&sysroot_opt.pop().pop().push("rustc"))
+pub fn in_target(sysroot: &Path) -> bool {
+    debug!("Checking whether %s is in target", sysroot.to_str());
+    os::path_is_dir(&sysroot.pop().pop().push("rustc"))
 }

src/librustpkg/package_source.rs

@@ -28,9 +28,11 @@ use extra::workcache;
 // This contains a list of files found in the source workspace.
 #[deriving(Clone)]
 pub struct PkgSrc {
-    workspace: Path, // root of where the package source code lives
-    start_dir: Path, // dir to start looking in for packages -- normally
-                     // this is workspace/src/id but it may be just workspace
+    /// Root of where the package source code lives
+    workspace: Path,
+    // Directory to start looking in for packages -- normally
+    // this is workspace/src/id but it may be just workspace
+    start_dir: Path,
     id: PkgId,
     libs: ~[Crate],
     mains: ~[Crate],
@@ -61,8 +63,7 @@ impl PkgSrc {
         let mut to_try = ~[];
         if use_rust_path_hack {
             to_try.push(workspace.clone());
-        }
-        else {
+        } else {
             let result = workspace.push("src").push_rel(&id.path.pop()).push(fmt!("%s-%s",
                                                          id.short_name, id.version.to_str()));
             to_try.push(result);
@@ -251,7 +252,7 @@ impl PkgSrc {
     }
 
     fn build_crates(&self,
-                    ctx: &BuildCtx,
+                    ctx: &BuildContext,
                     exec: &mut workcache::Exec,
                     destination_dir: &Path,
                     crates: &[Crate],
@@ -263,23 +264,17 @@ impl PkgSrc {
             let path_str = path.to_str();
             let cfgs = crate.cfgs + cfgs;
 
-            let result = {
+            let result =
                 // compile_crate should return the path of the output artifact
-                match compile_crate(ctx,
-                                  exec,
-                                  &self.id,
-                                  &path,
-                                  destination_dir,
-                                  crate.flags,
-                                  cfgs,
-                                  false,
-                                  what).map(|p| p.to_str()) {
-                    Some(p) => p,
-                    None   => build_err::cond.raise(fmt!("build failure on %s",
-                                                         path_str))
-
-                }
-            };
+                compile_crate(ctx,
+                              exec,
+                              &self.id,
+                              &path,
+                              destination_dir,
+                              crate.flags,
+                              cfgs,
+                              false,
+                              what).to_str();
             debug!("Result of compiling %s was %s", path_str, result);
         }
     }
@@ -301,24 +296,25 @@ impl PkgSrc {
 
     // It would be better if build returned a Path, but then Path would have to derive
     // Encodable.
-    pub fn build(&self, exec: &mut workcache::Exec, ctx: &BuildCtx, cfgs: ~[~str]) -> ~str {
+    pub fn build(&self,
+                 exec: &mut workcache::Exec,
+                 build_context: &BuildContext,
+                 cfgs: ~[~str]) -> ~str {
         use conditions::not_a_workspace::cond;
 
         // Determine the destination workspace (which depends on whether
         // we're using the rust_path_hack)
         let destination_workspace = if is_workspace(&self.workspace) {
             debug!("%s is indeed a workspace", self.workspace.to_str());
             self.workspace.clone()
-        }
-            else {
+        } else {
             // It would be nice to have only one place in the code that checks
             // for the use_rust_path_hack flag...
-            if ctx.cx.use_rust_path_hack {
+            if build_context.context.use_rust_path_hack {
                 let rs = default_workspace();
                 debug!("Using hack: %s", rs.to_str());
                 rs
-            }
-            else {
+            } else {
                 cond.raise(fmt!("Package root %s is not a workspace; pass in --rust_path_hack \
                                         if you want to treat it as a package source",
                                 self.workspace.to_str()))
@@ -331,13 +327,13 @@ impl PkgSrc {
         let benchs = self.benchs.clone();
         debug!("Building libs in %s, destination = %s",
                destination_workspace.to_str(), destination_workspace.to_str());
-        self.build_crates(ctx, exec, &destination_workspace, libs, cfgs, Lib);
+        self.build_crates(build_context, exec, &destination_workspace, libs, cfgs, Lib);
         debug!("Building mains");
-        self.build_crates(ctx, exec, &destination_workspace, mains, cfgs, Main);
+        self.build_crates(build_context, exec, &destination_workspace, mains, cfgs, Main);
         debug!("Building tests");
-        self.build_crates(ctx, exec, &destination_workspace, tests, cfgs, Test);
+        self.build_crates(build_context, exec, &destination_workspace, tests, cfgs, Test);
         debug!("Building benches");
-        self.build_crates(ctx, exec, &destination_workspace, benchs, cfgs, Bench);
+        self.build_crates(build_context, exec, &destination_workspace, benchs, cfgs, Bench);
         destination_workspace.to_str()
     }
 }

src/librustpkg/path_util.rs

@@ -84,8 +84,7 @@ pub fn workspace_contains_package_id_(pkgid: &PkgId, workspace: &Path,
 
     if found.is_some() {
         debug!("Found %s in %s", pkgid.to_str(), workspace.to_str());
-    }
-    else {
+    } else {
         debug!("Didn't find %s in %s", pkgid.to_str(), workspace.to_str());
     }
     found