Fix code that was trying reuse an already installed nightly.

Before, we would just skip attempts to install a current nightly. (And then, we
would unconditionally fail the run as documented on issue #54, regardless
whether of whether the regression was actually visible on that version, due to
it not being installed with the name "bisector-nightly-xxx" that we expect.)

This commit fixes that. It makes `install` call out to `rustup toolchain link`
to link the current rustc (as reported via `rustc --print sysroot`) up to the
new "bisector-nightly-xxx" that we expect.

(I also refactored the code to construct `DownloadParams`; that refactoring was
more relevant in an earlier version of the code that was linking in a different
(buggy) way.)

Author: pnkfelix

src/main.rs

@@ -31,6 +31,7 @@ use std::env;
 use std::ffi::OsString;
 use std::fmt;
 use std::fs;
+use std::iter;
 use std::io::{self, Read, Write};
 use std::path::{Path, PathBuf};
 use std::process::{self, Command, Stdio};
@@ -309,19 +310,16 @@ impl DownloadParams {
             if cfg.args.alt { "-alt" } else { "" }
         );
 
-        DownloadParams {
-            url_prefix: url_prefix,
-            tmp_dir: cfg.rustup_tmp_path.clone(),
-            install_dir: cfg.toolchains_path.clone(),
-            install_cargo: cfg.args.with_cargo,
-            install_src: cfg.args.with_src,
-            force_install: cfg.args.force_install,
-        }
+        Self::from_cfg_with_url_prefix(cfg, url_prefix)
     }
 
     fn for_nightly(cfg: &Config) -> Self {
+        Self::from_cfg_with_url_prefix(cfg, NIGHTLY_SERVER.to_string())
+    }
+
+    fn from_cfg_with_url_prefix(cfg: &Config, url_prefix: String) -> Self {
         DownloadParams {
-            url_prefix: NIGHTLY_SERVER.to_string(),
+            url_prefix: url_prefix,
             tmp_dir: cfg.rustup_tmp_path.clone(),
             install_dir: cfg.toolchains_path.clone(),
             install_cargo: cfg.args.with_cargo,
@@ -450,6 +448,8 @@ enum InstallError {
     TempDir(#[cause] io::Error),
     #[fail(display = "Could not move tempdir into destination: {}", _0)]
     Move(#[cause] io::Error),
+    #[fail(display = "Could not run subcommand {}: {}", command, cause)]
+    Subcommand { command: String, #[cause] cause: io::Error }
 }
 
 #[derive(Debug)]
@@ -702,11 +702,6 @@ impl Toolchain {
     }
 
     fn install(&self, client: &Client, dl_params: &DownloadParams) -> Result<(), InstallError> {
-        if self.is_current_nightly() {
-            // pre existing installation
-            return Ok(());
-        }
-
         debug!("installing {}", self);
         let tmpdir = TempDir::new_in(&dl_params.tmp_dir, &self.rustup_name())
             .map_err(InstallError::TempDir)?;
@@ -720,6 +715,40 @@ impl Toolchain {
             return Ok(());
         }
 
+        if self.is_current_nightly() {
+            // make link to pre-existing installation
+            debug!("installing (via link) {}", self);
+
+            let nightly_path: String = {
+                let cmd = CommandTemplate::new(["rustc", "--print", "sysroot"]
+                                               .iter()
+                                               .map(|s| s.to_string()));
+                let stdout = cmd.output()?.stdout;
+                let output = String::from_utf8_lossy(&stdout);
+                // the output should be the path, terminated by a newline
+                let mut path = output.to_string();
+                let last = path.pop();
+                assert_eq!(last, Some('\n'));
+                path
+            };
+
+            let cmd = CommandTemplate::new(["rustup", "toolchain", "link"]
+                                           .iter()
+                                           .map(|s| s.to_string())
+                                           .chain(iter::once(self.rustup_name()))
+                                           .chain(iter::once(nightly_path)));
+            if cmd.status()?.success() {
+                return Ok(());
+            } else {
+                return Err(InstallError::Subcommand {
+                    command: cmd.string(),
+                    cause: io::Error::new(io::ErrorKind::Other, "failed to link via `rustup`"),
+                });
+            }
+        }
+
+        debug!("installing via download {}", self);
+
         let rustc_filename = format!("rustc-nightly-{}", self.host);
 
         let location = match self.spec {
@@ -794,6 +823,46 @@ impl Toolchain {
     }
 }
 
+// A simpler wrapper struct to make up for impoverished `Command` in libstd.
+struct CommandTemplate(Vec<String>);
+
+impl CommandTemplate {
+    fn new(strings: impl Iterator<Item=String>) -> Self {
+        CommandTemplate(strings.collect())
+    }
+
+    fn command(&self) -> Command {
+        assert!(self.0.len() > 0);
+        let mut cmd = Command::new(&self.0[0]);
+        for arg in &self.0[1..] {
+            cmd.arg(arg);
+        }
+        cmd
+    }
+
+    fn string(&self) -> String {
+        assert!(self.0.len() > 0);
+        let mut s = self.0[0].to_string();
+        for arg in &self.0[1..] {
+            s.push_str(" ");
+            s.push_str(arg);
+        }
+        s
+    }
+
+    fn status(&self) -> Result<process::ExitStatus, InstallError> {
+        self.command().status().map_err(|cause| {
+            InstallError::Subcommand { command: self.string(), cause }
+        })
+    }
+
+    fn output(&self) -> Result<process::Output, InstallError> {
+        self.command().output().map_err(|cause| {
+            InstallError::Subcommand { command: self.string(), cause }
+        })
+    }
+}
+
 struct Config {
     args: Opts,
     rustup_tmp_path: PathBuf,