Merge pull request #14 from anp/benchmarking-2

Support `by-commit` with nightly dates, additional logging, some up-front validation

Author: Mark-Simulacrum

Cargo.lock

@@ -23,6 +23,7 @@ structopt = "0.2.5"
 tar = "0.4"
 tee = "0.1"
 tempdir = "0.3.6"
+toml = "0.4"
 xz2 = "0.1.3"
 chrono = "0.4.0"
 

Cargo.toml

@@ -46,17 +46,23 @@ fn get_repo() -> Result<Repository, Error> {
     let loc = Path::new("rust.git");
     match (RUST_SRC_REPO, loc.exists()) {
         (Some(_), _) | (_, true) => {
-            let repo = Repository::open(RUST_SRC_REPO.map(Path::new).unwrap_or(loc))?;
+            let path = RUST_SRC_REPO.map(Path::new).unwrap_or(loc);
+            eprintln!("opening existing repository at {:?}", path);
+            let repo = Repository::open(path)?;
             {
+                eprintln!("refreshing repository");
                 let mut remote = repo.find_remote("origin")
                     .or_else(|_| repo.remote_anonymous("origin"))?;
                 remote.fetch(&["master"], None, None)?;
             }
             Ok(repo)
         }
-        (None, false) => Ok(RepoBuilder::new()
-            .bare(true)
-            .clone(RUST_SRC_URL, Path::new("rust.git"))?),
+        (None, false) => {
+            eprintln!("cloning rust repository");
+            Ok(RepoBuilder::new()
+                .bare(true)
+                .clone(RUST_SRC_URL, Path::new("rust.git"))?)
+        }
     }
 }
 
@@ -70,7 +76,9 @@ pub fn expand_commit(sha: &str) -> Result<String, Error> {
 /// (boundaries inclusive).
 pub fn get_commits_between(first_commit: &str, last_commit: &str) -> Result<Vec<Commit>, Error> {
     let repo = get_repo()?;
+    eprintln!("looking up first commit");
     let mut first = lookup_rev(&repo, first_commit)?;
+    eprintln!("looking up second commit");
     let last = lookup_rev(&repo, last_commit)?;
 
     // Sanity check -- our algorithm below only works reliably if the
@@ -82,13 +90,16 @@ pub fn get_commits_between(first_commit: &str, last_commit: &str) -> Result<Vec<
             None => bail!("No author for {}", c.id()),
         }
     };
+
+    eprintln!("checking that commits are by bors and thus have ci artifacts...");
     assert_by_bors(&first)?;
     assert_by_bors(&last)?;
     // Now find the commits
     // We search from the last and always take the first of its parents,
     // to only get merge commits.
     // This uses the fact that all bors merge commits have the earlier
     // merge commit as their first parent.
+    eprintln!("finding bors merge commits");
     let mut res = Vec::new();
     let mut current = last;
     loop {
@@ -117,5 +128,9 @@ pub fn get_commits_between(first_commit: &str, last_commit: &str) -> Result<Vec<
     res.push(Commit::from_git2_commit(&mut first));
     // Reverse in order to obtain chronological order
     res.reverse();
+    eprintln!(
+        "found {} bors merge commits in the specified range",
+        res.len()
+    );
     Ok(res)
 }

src/git.rs

@@ -17,7 +17,23 @@ where
     let mut predicate = |idx: usize| *cache.entry(idx).or_insert_with(|| predicate(&slice[idx]));
     let mut unknown_ranges: Vec<(usize, usize)> = Vec::new();
     let mut rm_no = 0; // presume that the slice starts with a no
+
+    eprintln!("verifying the start of the range does not reproduce the regression");
+    match predicate(rm_no) {
+        Satisfies::No => {
+            eprintln!("confirmed the start of the range does not reproduce the regression")
+        }
+        _ => panic!("the start of the range to test must not reproduce the regression"),
+    }
+
     let mut lm_yes = slice.len() - 1; // presume that the slice ends with a yes
+
+    eprintln!("verifying the end of the range reproduces the regression");
+    match predicate(lm_yes) {
+        Satisfies::Yes => eprintln!("confirmed the end of the range reproduces the regression"),
+        _ => panic!("the end of the range to test must reproduce the regression"),
+    }
+
     let mut next = (rm_no + lm_yes) / 2;
 
     loop {

src/least_satisfying.rs

@@ -23,6 +23,7 @@ extern crate structopt;
 extern crate tar;
 extern crate tee;
 extern crate tempdir;
+extern crate toml;
 extern crate xz2;
 
 use std::fmt;
@@ -42,6 +43,7 @@ use reqwest::{Client, Response};
 use pbr::{ProgressBar, Units};
 use tee::TeeReader;
 use tar::Archive;
+use toml::Value;
 use reqwest::header::ContentLength;
 use xz2::read::XzDecoder;
 use flate2::read::GzDecoder;
@@ -63,6 +65,7 @@ mod least_satisfying;
 use least_satisfying::{least_satisfying, Satisfies};
 
 fn get_commits(start: &str, end: &str) -> Result<Vec<git::Commit>, Error> {
+    eprintln!("fetching commits from {} to {}", start, end);
     let commits = git::get_commits_between(start, end)?;
     assert_eq!(commits.first().expect("at least one commit").sha, start);
 
@@ -144,6 +147,48 @@ impl FromStr for Bound {
     }
 }
 
+impl Bound {
+    fn as_commit(self) -> Result<Self, Error> {
+        match self {
+            Bound::Commit(commit) => Ok(Bound::Commit(commit)),
+            Bound::Date(date) => {
+                let date_str = date.format("%Y-%m-%d");
+                let url = format!("{}/{}/channel-rust-nightly.toml", NIGHTLY_SERVER, date_str);
+
+                eprintln!("fetching {}", url);
+                let client = Client::new();
+                let name = format!("nightly manifest {}", date_str);
+                let (response, mut bar) = download_progress(&client, &name, &url)?;
+                let mut response = TeeReader::new(response, &mut bar);
+                let mut toml_buf = String::new();
+                response.read_to_string(&mut toml_buf)?;
+
+                let manifest = toml_buf.parse::<Value>().unwrap();
+
+                let commit = match manifest {
+                    Value::Table(t) => match t.get("pkg") {
+                        Some(&Value::Table(ref t)) => match t.get("rust") {
+                            Some(&Value::Table(ref t)) => match t.get("git_commit_hash") {
+                                Some(&Value::String(ref hash)) => hash.to_owned(),
+                                _ => bail!(
+                                    "not a rustup manifest (no valid git_commit_hash key under rust"
+                                ),
+                            },
+                            _ => bail!("not a rustup manifest (no rust key under pkg)"),
+                        },
+                        _ => bail!("not a rustup manifest (missing pkg key)"),
+                    },
+                    _ => bail!("not a rustup manifest (not a table at root)"),
+                };
+
+                eprintln!("converted {} to {}", date_str, commit);
+
+                Ok(Bound::Commit(commit))
+            }
+        }
+    }
+}
+
 impl Opts {
     fn emit_cargo_output(&self) -> bool {
         self.verbosity >= 2
@@ -540,7 +585,7 @@ impl Config {
         }
 
         let target = args.target.clone().unwrap_or_else(|| args.host.clone());
-        let args = args;
+        let mut args = args;
 
         let mut toolchains_path = match env::var_os("RUSTUP_HOME") {
             Some(h) => PathBuf::from(h),
@@ -586,8 +631,14 @@ impl Config {
         };
 
         if is_commit == Some(false) && args.by_commit {
-            // FIXME: In theory, we could use the date range to narrow down the commit list...
-            bail!("cannot bisect by-commit if specifying date range");
+            eprintln!("finding commit range that corresponds to dates specified");
+            match (args.start, args.end) {
+                (Some(b1), Some(b2)) => {
+                    args.start = Some(b1.as_commit()?);
+                    args.end = Some(b2.as_commit()?);
+                }
+                _ => unreachable!(),
+            }
         }
 
         Ok(Config {
@@ -832,6 +883,7 @@ fn toolchains_between(cfg: &Config, a: ToolchainSpec, b: ToolchainSpec) -> Vec<T
 }
 
 fn bisect_ci(cfg: &Config, client: &Client) -> Result<BisectionResult, Error> {
+    eprintln!("bisecting ci builds");
     let dl_spec = DownloadParams::for_ci(cfg);
     let start = if let Some(Bound::Commit(ref sha)) = cfg.args.start {
         sha
@@ -845,6 +897,8 @@ fn bisect_ci(cfg: &Config, client: &Client) -> Result<BisectionResult, Error> {
         "origin/master"
     };
 
+    eprintln!("starting at {}, ending at {}", start, end);
+
     let mut commits = get_commits(start, end)?;
     let now = chrono::Utc::now();
     commits.retain(|c| now.signed_duration_since(c.date).num_days() < 90);
@@ -873,6 +927,8 @@ fn bisect_ci(cfg: &Config, client: &Client) -> Result<BisectionResult, Error> {
         }
     }
 
+    eprintln!("validated commits found, specifying toolchains");
+
     let toolchains = commits
         .into_iter()
         .map(|commit| {
@@ -890,9 +946,12 @@ fn bisect_ci(cfg: &Config, client: &Client) -> Result<BisectionResult, Error> {
         })
         .collect::<Vec<_>>();
 
+    eprintln!("testing commits");
     let found = least_satisfying(&toolchains, |t| {
+        eprintln!("installing {}", t);
         match t.install(&client, &dl_spec) {
             Ok(()) => {
+                eprintln!("testing {}", t);
                 let outcome = t.test(&cfg, &dl_spec);
                 // we want to fail, so a successful build doesn't satisfy us
                 let r = match outcome {