Merge pull request #300 from pietroalbini/toolchain-refactoring

Toolchain refactorings

Author: pietroalbini

Cargo.lock

@@ -50,9 +50,9 @@ walkdir = "2"
 winapi = "0.3"
 regex = "0.2.10"
 ring = "0.12.1"
-rusqlite = { version = "0.13.0", features = ["chrono", "bundled"] }
+rusqlite = { version = "0.14.0", features = ["chrono", "functions", "bundled"] }
 r2d2 = "0.8.2"
-r2d2_sqlite = "0.5.0"
+r2d2_sqlite = "0.5.1"
 base64 = "0.9.0"
 tera = "0.11.7"
 minifier = "0.0.11"

Cargo.toml

@@ -22,7 +22,7 @@ use crater::report;
 use crater::results::FileDB;
 use crater::run_graph;
 use crater::server;
-use crater::toolchain::Toolchain;
+use crater::toolchain::{Toolchain, MAIN_TOOLCHAIN};
 use std::env;
 use std::path::PathBuf;
 use std::str::FromStr;
@@ -246,8 +246,7 @@ impl Crater {
             Crater::CreateLists => lists::create_all_lists(true)?,
             Crater::PrepareLocal { ref env } => {
                 let docker_env = &env.0;
-                let stable_tc = Toolchain::Dist("stable".into());
-                stable_tc.prepare()?;
+                MAIN_TOOLCHAIN.prepare()?;
                 docker::build_container(docker_env)?;
                 lists::create_all_lists(false)?;
             }
@@ -264,7 +263,7 @@ impl Crater {
                 ex::define(
                     ex::ExOpts {
                         name: ex.0.clone(),
-                        toolchains: vec![tc1.clone(), tc2.clone()],
+                        toolchains: [tc1.clone(), tc2.clone()],
                         mode: *mode,
                         crates: *crates,
                         cap_lints: *cap_lints,

src/cli.rs

@@ -42,5 +42,13 @@ error_chain! {
         ServerUnavailable {
             description("the server is not available at the moment")
         }
+
+        EmptyToolchainName {
+            description("empty toolchain name")
+        }
+        InvalidToolchainSourceName(name: String) {
+            description("invalid toolchain source name")
+            display("invalid toolchain source name: {}", name)
+        }
     }
 }

src/errors.rs

@@ -56,14 +56,14 @@ fn froml_path(ex_name: &str, name: &str, vers: &str) -> PathBuf {
 pub struct Experiment {
     pub name: String,
     pub crates: Vec<Crate>,
-    pub toolchains: Vec<Toolchain>,
+    pub toolchains: [Toolchain; 2],
     pub mode: ExMode,
     pub cap_lints: ExCapLints,
 }
 
 pub struct ExOpts {
     pub name: String,
-    pub toolchains: Vec<Toolchain>,
+    pub toolchains: [Toolchain; 2],
     pub mode: ExMode,
     pub crates: ExCrateSelect,
     pub cap_lints: ExCapLints,
@@ -141,7 +141,7 @@ fn top_100() -> Result<Vec<Crate>> {
 
 pub fn define_(
     ex_name: &str,
-    toolchains: Vec<Toolchain>,
+    toolchains: [Toolchain; 2],
     crates: Vec<Crate>,
     mode: ExMode,
     cap_lints: ExCapLints,
@@ -158,6 +158,9 @@ pub fn define_(
         mode,
         cap_lints,
     };
+
+    ex.validate()?;
+
     fs::create_dir_all(&ex_dir(&ex.name))?;
     let json = serde_json::to_string(&ex)?;
     info!("writing ex config to {}", config_file(ex_name).display());
@@ -166,6 +169,14 @@ pub fn define_(
 }
 
 impl Experiment {
+    pub fn validate(&self) -> Result<()> {
+        if self.toolchains[0] == self.toolchains[1] {
+            bail!("reusing the same toolchain isn't supported");
+        }
+
+        Ok(())
+    }
+
     pub fn fetch_repo_crates(&self) -> Result<()> {
         for repo in self.crates.iter().filter_map(|krate| krate.github()) {
             if let Err(e) = git::shallow_clone_or_pull(&repo.url(), &repo.mirror_dir()) {
@@ -434,3 +445,36 @@ pub fn delete(ex_name: &str) -> Result<()> {
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{ExCapLints, ExMode, Experiment};
+    use toolchain::{MAIN_TOOLCHAIN, TEST_TOOLCHAIN};
+
+    #[test]
+    fn test_validate_experiment() {
+        // Correct experiment
+        assert!(
+            Experiment {
+                name: "foo".to_string(),
+                crates: vec![],
+                toolchains: [MAIN_TOOLCHAIN.clone(), TEST_TOOLCHAIN.clone()],
+                mode: ExMode::BuildAndTest,
+                cap_lints: ExCapLints::Forbid,
+            }.validate()
+                .is_ok()
+        );
+
+        // Experiment with the same toolchain
+        assert!(
+            Experiment {
+                name: "foo".to_string(),
+                crates: vec![],
+                toolchains: [MAIN_TOOLCHAIN.clone(), MAIN_TOOLCHAIN.clone()],
+                mode: ExMode::BuildAndTest,
+                cap_lints: ExCapLints::Forbid,
+            }.validate()
+                .is_err()
+        );
+    }
+}

src/ex.rs

@@ -82,7 +82,11 @@ struct BuildTestResult {
 
 fn crate_to_path_fragment(toolchain: &Toolchain, krate: &Crate, encode: bool) -> PathBuf {
     let mut path = PathBuf::new();
-    path.push(toolchain.rustup_name());
+    if encode {
+        path.push(url_encode(&toolchain.to_string()));
+    } else {
+        path.push(toolchain.to_string());
+    }
 
     match *krate {
         Crate::Registry(ref details) => {
@@ -116,8 +120,6 @@ pub fn generate_report<DB: ReadResults>(
     ex: &Experiment,
 ) -> Result<TestResults> {
     let shas = db.load_all_shas(ex)?;
-    assert_eq!(ex.toolchains.len(), 2);
-
     let res = ex
         .crates
         .clone()
@@ -342,7 +344,7 @@ mod tests {
     use ex::{ExCapLints, ExMode, Experiment};
     use results::{DummyDB, TestResult};
     use std::collections::HashMap;
-    use toolchain::Toolchain;
+    use toolchain::{MAIN_TOOLCHAIN, TEST_TOOLCHAIN};
 
     #[derive(Default)]
     pub struct DummyWriter {
@@ -394,7 +396,6 @@ mod tests {
 
     #[test]
     fn test_crate_to_path_fragment() {
-        let tc = Toolchain::Dist("stable".into());
         let reg = Crate::Registry(RegistryCrate {
             name: "lazy_static".into(),
             version: "1.0".into(),
@@ -409,19 +410,19 @@ mod tests {
         });
 
         assert_eq!(
-            crate_to_path_fragment(&tc, &reg, false),
+            crate_to_path_fragment(&MAIN_TOOLCHAIN, &reg, false),
             PathBuf::from("stable/reg/lazy_static-1.0")
         );
         assert_eq!(
-            crate_to_path_fragment(&tc, &gh, false),
+            crate_to_path_fragment(&MAIN_TOOLCHAIN, &gh, false),
             PathBuf::from("stable/gh/brson.hello-rs")
         );
         assert_eq!(
-            crate_to_path_fragment(&tc, &plus, false),
+            crate_to_path_fragment(&MAIN_TOOLCHAIN, &plus, false),
             PathBuf::from("stable/reg/foo-1.0+bar")
         );
         assert_eq!(
-            crate_to_path_fragment(&tc, &plus, true),
+            crate_to_path_fragment(&MAIN_TOOLCHAIN, &plus, true),
             PathBuf::from("stable/reg/foo-1.0%2Bbar")
         );
     }
@@ -559,23 +560,40 @@ mod tests {
         };
         let gh = Crate::GitHub(repo.clone());
 
-        let stable = Toolchain::Dist("stable".to_string());
-        let beta = Toolchain::Dist("beta".to_string());
-
         let ex = Experiment {
             name: "foo".to_string(),
             crates: vec![gh.clone()],
-            toolchains: vec![stable.clone(), beta.clone()],
+            toolchains: [MAIN_TOOLCHAIN.clone(), TEST_TOOLCHAIN.clone()],
             mode: ExMode::BuildAndTest,
             cap_lints: ExCapLints::Forbid,
         };
 
         let mut db = DummyDB::default();
         db.add_dummy_sha(&ex, repo.clone(), "f00".to_string());
-        db.add_dummy_result(&ex, gh.clone(), stable.clone(), TestResult::TestPass);
-        db.add_dummy_result(&ex, gh.clone(), beta.clone(), TestResult::BuildFail);
-        db.add_dummy_log(&ex, gh.clone(), stable.clone(), b"stable log".to_vec());
-        db.add_dummy_log(&ex, gh.clone(), beta.clone(), b"beta log".to_vec());
+        db.add_dummy_result(
+            &ex,
+            gh.clone(),
+            MAIN_TOOLCHAIN.clone(),
+            TestResult::TestPass,
+        );
+        db.add_dummy_result(
+            &ex,
+            gh.clone(),
+            TEST_TOOLCHAIN.clone(),
+            TestResult::BuildFail,
+        );
+        db.add_dummy_log(
+            &ex,
+            gh.clone(),
+            MAIN_TOOLCHAIN.clone(),
+            b"stable log".to_vec(),
+        );
+        db.add_dummy_log(
+            &ex,
+            gh.clone(),
+            TEST_TOOLCHAIN.clone(),
+            b"beta log".to_vec(),
+        );
 
         let writer = DummyWriter::default();
         gen(&db, &ex, &writer, &config).unwrap();

src/report/mod.rs

@@ -31,7 +31,7 @@ impl FileDB {
 
         ex_dir(&ex.name)
             .join("res")
-            .join(toolchain.rustup_name())
+            .join(toolchain.to_string())
             .join(crate_path)
     }
 

src/results/file.rs

@@ -151,7 +151,7 @@ mod tests {
     use server::db::Database;
     use server::experiments::Experiments;
     use server::tokens::Tokens;
-    use toolchain::Toolchain;
+    use toolchain::{MAIN_TOOLCHAIN, TEST_TOOLCHAIN};
 
     #[test]
     fn test_agents_synchronize() {
@@ -231,8 +231,8 @@ mod tests {
         experiments
             .create(
                 "test".into(),
-                &Toolchain::Dist("stable".into()),
-                &Toolchain::Dist("beta".into()),
+                &MAIN_TOOLCHAIN,
+                &TEST_TOOLCHAIN,
                 ExMode::BuildAndTest,
                 ExCrateSelect::Demo,
                 ExCapLints::Forbid,