Merge pull request #76 from tomprince/docker-toolchains

Move all cargo invocations into docker.

Author: tomprince

src/docker.rs

@@ -1,4 +1,3 @@
-use dirs::{CARGO_HOME, RUSTUP_HOME};
 use errors::*;
 use run;
 use std::env;
@@ -55,23 +54,7 @@ pub struct ContainerConfig<'a> {
 }
 
 
-pub fn run(source_path: &Path, target_path: &Path, args: &[&str]) -> Result<()> {
-
-    info!("running: {}", args.join(" "));
-
-    let env = RustEnv {
-        args: args,
-        work_dir: (source_path.into(), Perm::ReadOnly),
-        cargo_home: (Path::new(CARGO_HOME).into(), Perm::ReadOnly),
-        rustup_home: (Path::new(RUSTUP_HOME).into(), Perm::ReadOnly),
-        // This is configured as CARGO_TARGET_DIR by the docker container itself
-        target_dir: (target_path.into(), Perm::ReadWrite),
-    };
-
-    run_container(rust_container(env))
-}
-
-pub fn run_container(config: ContainerConfig) -> Result<()> {
+pub fn run(config: ContainerConfig) -> Result<()> {
     let c = Container::create_container(config)?;
     defer!{{
         if let Err(e) = c.delete() {

src/ex.rs

@@ -12,7 +12,7 @@ use std::fs;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use toml_frobber;
-use toolchain::{self, Toolchain};
+use toolchain::{self, CargoState, Toolchain};
 use util;
 
 #[derive(Serialize, Deserialize)]
@@ -424,10 +424,9 @@ fn capture_lockfile(ex: &Experiment,
                     path: &Path,
                     toolchain: &Toolchain)
                     -> Result<()> {
-    let manifest_path = path.join("Cargo.toml").to_string_lossy().to_string();
-    let args = &["generate-lockfile", "--manifest-path", &*manifest_path];
+    let args = &["generate-lockfile", "--manifest-path", "Cargo.toml"];
     toolchain
-        .run_cargo(&ex.name, args)
+        .run_cargo(&ex.name, path, args, CargoState::Unlocked)
         .chain_err(|| format!("unable to generate lockfile for {}", crate_))?;
 
     let src_lockfile = &path.join("Cargo.lock");
@@ -472,10 +471,9 @@ pub fn fetch_deps(ex: &Experiment, toolchain: &Toolchain) -> Result<()> {
             with_frobbed_toml(ex, c, path)?;
             with_captured_lockfile(ex, c, path)?;
 
-            let manifest_path = path.join("Cargo.toml").to_string_lossy().to_string();
-            let args = &["fetch", "--locked", "--manifest-path", &*manifest_path];
+            let args = &["fetch", "--locked", "--manifest-path", "Cargo.toml"];
             toolchain
-                .run_cargo(&ex.name, args)
+                .run_cargo(&ex.name, path, args, CargoState::Unlocked)
                 .chain_err(|| format!("unable to fetch deps for {}", c))?;
 
             Ok(())

src/ex_run.rs

@@ -6,7 +6,7 @@ use results::{CrateResultWriter, ExperimentResultDB, FileDB, TestResult};
 use std::collections::HashSet;
 use std::path::Path;
 use std::time::Instant;
-use toolchain::Toolchain;
+use toolchain::{CargoState, Toolchain};
 use util;
 
 pub fn delete_all_results(ex_name: &str) -> Result<()> {
@@ -173,19 +173,24 @@ fn test_build_and_test(ex: &Experiment,
                        source_path: &Path,
                        toolchain: &Toolchain)
                        -> Result<TestResult> {
-    let build_r = toolchain.run_cargo_in_docker(&ex.name, source_path, &["build", "--frozen"]);
+    let build_r = toolchain.run_cargo(&ex.name,
+                                      source_path,
+                                      &["build", "--frozen"],
+                                      CargoState::Locked);
     let mut test_r;
 
     if build_r.is_ok() {
         // First build, with --no-run
-        test_r = Some(toolchain.run_cargo_in_docker(&ex.name,
-                                                    source_path.into(),
-                                                    &["test", "--frozen", "--no-run"]));
+        test_r = Some(toolchain.run_cargo(&ex.name,
+                                          source_path.into(),
+                                          &["test", "--frozen", "--no-run"],
+                                          CargoState::Locked));
         // Then run
         test_r = test_r.map(|_| {
-                                toolchain.run_cargo_in_docker(&ex.name,
-                                                              source_path.into(),
-                                                              &["test", "--frozen"])
+                                toolchain.run_cargo(&ex.name,
+                                                    source_path.into(),
+                                                    &["test", "--frozen"],
+                                                    CargoState::Locked)
                             });
     } else {
         test_r = None;
@@ -203,7 +208,10 @@ fn test_build_only(ex: &Experiment,
                    source_path: &Path,
                    toolchain: &Toolchain)
                    -> Result<TestResult> {
-    let r = toolchain.run_cargo_in_docker(&ex.name, source_path.into(), &["build", "--frozen"]);
+    let r = toolchain.run_cargo(&ex.name,
+                                source_path.into(),
+                                &["build", "--frozen"],
+                                CargoState::Locked);
 
     if r.is_ok() {
         Ok(TestResult::TestPass)
@@ -216,7 +224,10 @@ fn test_check_only(ex: &Experiment,
                    source_path: &Path,
                    toolchain: &Toolchain)
                    -> Result<TestResult> {
-    let r = toolchain.run_cargo_in_docker(&ex.name, source_path.into(), &["check", "--frozen"]);
+    let r = toolchain.run_cargo(&ex.name,
+                                source_path.into(),
+                                &["check", "--frozen"],
+                                CargoState::Locked);
 
     if r.is_ok() {
         Ok(TestResult::TestPass)

src/toolchain.rs

@@ -181,32 +181,22 @@ pub fn ex_target_dir(ex_name: &str) -> PathBuf {
     Path::new(TARGET_DIR).join(ex_name)
 }
 
+pub enum CargoState {
+    Locked,
+    Unlocked,
+}
+
 impl Toolchain {
     pub fn target_dir(&self, ex_name: &str) -> PathBuf {
         ex_target_dir(ex_name).join(self.to_string())
     }
 
-    pub fn run_cargo(&self, ex_name: &str, args: &[&str]) -> Result<()> {
-        let toolchain_name = self.rustup_name();
-        let ex_target_dir = self.target_dir(ex_name);
-
-        fs::create_dir_all(&ex_target_dir)?;
-
-        let toolchain_arg = "+".to_string() + &toolchain_name;
-        let mut full_args = vec![&*toolchain_arg];
-        full_args.extend_from_slice(args);
-
-        let cargo = Path::new(CARGO_HOME).join("bin/cargo");
-        rustup_run(&cargo.to_string_lossy(),
-                   &full_args,
-                   &[("CARGO_TARGET_DIR", &ex_target_dir.to_string_lossy())])
-    }
-
-    pub fn run_cargo_in_docker(&self,
-                               ex_name: &str,
-                               source_dir: &Path,
-                               args: &[&str])
-                               -> Result<()> {
+    pub fn run_cargo(&self,
+                     ex_name: &str,
+                     source_dir: &Path,
+                     args: &[&str],
+                     cargo_state: CargoState)
+                     -> Result<()> {
         let toolchain_name = self.rustup_name();
         let ex_target_dir = self.target_dir(ex_name);
 
@@ -216,6 +206,19 @@ impl Toolchain {
         let mut full_args = vec!["cargo", &*toolchain_arg];
         full_args.extend_from_slice(args);
 
-        docker::run(source_dir, &ex_target_dir, &full_args)
+        info!("running: {}", full_args.join(" "));
+        let rust_env = docker::RustEnv {
+            args: &full_args,
+            work_dir: (source_dir.into(), docker::Perm::ReadOnly),
+            cargo_home: (Path::new(CARGO_HOME).into(),
+                         match cargo_state {
+                             CargoState::Locked => docker::Perm::ReadOnly,
+                             CargoState::Unlocked => docker::Perm::ReadWrite,
+                         }),
+            rustup_home: (Path::new(RUSTUP_HOME).into(), docker::Perm::ReadOnly),
+            // This is configured as CARGO_TARGET_DIR by the docker container itself
+            target_dir: (ex_target_dir, docker::Perm::ReadWrite),
+        };
+        docker::run(docker::rust_container(rust_env))
     }
 }