Merge pull request #161 from brson/multirust-env

Rename MULTIRUST_ env vars to RUSTUP_. Leave compatibility code.

Author: brson

README.md

@@ -114,6 +114,10 @@ If the date is omitted, the toolchain will track the most recent version.
 The following environment variables can be used to customize the behaviour of
 multirust-rs:
 
+- `MULTIRUST_HOME` (default: `~/.multirust` or `%LOCALAPPDATA%/.multirust`)
+	Sets the root multirust folder, used for storing installed toolchains and configuration
+	options.
+
 - `MULTIRUST_TOOLCHAIN` (default: none)
 	If set, will override the toolchain used for all rust tool invocations. A toolchain
 	with this name should be installed, or invocations will fail.
@@ -122,9 +126,8 @@ multirust-rs:
 	Sets the root URL for downloading packages. You can change this to instead use
 	a local mirror, or to test the binaries from the staging directory.
 
-- `MULTIRUST_HOME` (default: `~/.multirust` or `%LOCALAPPDATA%/.multirust`)
-	Sets the root multirust folder, used for storing installed toolchains and configuration
-	options.
+- `MULTIRUST_UPDATE_ROOT` (default `https://static.rust-lang.org/rustup/dist`)
+        Sets the root URL for downloading self-updates.
 
 - `MULTIRUST_GPG_KEY` (default: none)
 	Sets the GPG key used to verify the signatures of downloaded files.

src/multirust-cli/main.rs

@@ -13,7 +13,6 @@ extern crate openssl;
 extern crate itertools;
 extern crate time;
 extern crate rand;
-#[macro_use]
 extern crate scopeguard;
 extern crate tempdir;
 
@@ -60,6 +59,10 @@ fn run_multirust() -> Result<()> {
         return Err(Error::InfiniteRecursion);
     }
 
+    // Map MULTIRUST_ env vars to RUSTUP_
+    // FIXME: Remove this soon to get it out of the proxy path
+    make_environment_compatible();
+
     // The name of arg0 determines how the program is going to behave
     let arg0 = env::args().next().map(|a| PathBuf::from(a));
     let name = arg0.as_ref()
@@ -108,3 +111,21 @@ fn run_multirust() -> Result<()> {
     }
 }
 
+// Convert any MULTIRUST_ env vars to RUSTUP_ and warn about them
+fn make_environment_compatible() {
+    let ref vars = ["HOME", "TOOLCHAIN", "DIST_ROOT", "UPDATE_ROOT", "GPG_KEY"];
+    for var in vars {
+        let ref mvar = format!("MULTIRUST_{}", var);
+        let ref rvar = format!("RUSTUP_{}", var);
+        let mval = env::var_os(mvar);
+        let rval = env::var_os(rvar);
+
+        match (mval, rval) {
+            (Some(mval), None) => {
+                env::set_var(rvar, mval);
+                warn!("environment variable {} is deprecated. Use {}.", mvar, rvar);
+            }
+            _ => ()
+        }
+    }
+}

src/multirust-cli/self_update.rs

@@ -23,7 +23,7 @@
 //!
 //! During uninstall (`multirust self uninstall`):
 //!
-//! * Delete `$MULTIRUST_HOME`.
+//! * Delete `$RUSTUP_HOME`.
 //! * Delete everything in `$CARGO_HOME`, including
 //!   the multirust binary and its hardlinks
 //!
@@ -43,6 +43,7 @@ use std::path::{Path, PathBuf};
 use std::process::{self, Command};
 use std::fs;
 use tempdir::TempDir;
+use scopeguard;
 
 // The big installation messages. These are macros because the first
 // argument of format! needs to be a literal.
@@ -233,7 +234,7 @@ fn pre_install_msg() -> Result<String> {
 }
 
 // Before multirust-rs installed bins to $CARGO_HOME/bin it installed
-// them to $MULTIRUST_HOME/bin. If those bins continue to exist after
+// them to $RUSTUP_HOME/bin. If those bins continue to exist after
 // upgrade and are on the $PATH, it would cause major confusion. This
 // method silently deletes them.
 fn cleanup_legacy() -> Result<()> {
@@ -323,7 +324,7 @@ pub fn uninstall(no_prompt: bool) -> Result<()> {
 
     info!("removing multirust home");
 
-    // Delete MULTIRUST_HOME
+    // Delete RUSTUP_HOME
     let ref multirust_dir = try!(utils::multirust_home());
     if multirust_dir.exists() {
         try!(utils::remove_dir("multirust_home", multirust_dir, ntfy!(&NotifyHandler::none())));
@@ -475,7 +476,7 @@ fn delete_multirust_and_cargo_home() -> Result<()> {
             return Err(Error::WindowsUninstallMadness(err));
         }
 
-        defer!{{ let _ = CloseHandle(gc_handle); }}
+        let _g = scopeguard::guard(gc_handle, |h| { let _ = CloseHandle(*h); });
 
         try!(Command::new(gc_exe).spawn()
              .map_err(|e| Error::WindowsUninstallMadness(e)));
@@ -537,7 +538,7 @@ fn wait_for_parent() -> Result<()> {
             return Err(Error::WindowsUninstallMadness(err));
         }
 
-        defer! {{ let _ = CloseHandle(snapshot); }}
+        let _g = scopeguard::guard(snapshot, |h| { let _ = CloseHandle(*h); });
 
         let mut entry: PROCESSENTRY32 = mem::zeroed();
         entry.dwSize = mem::size_of::<PROCESSENTRY32>() as DWORD;
@@ -570,7 +571,7 @@ fn wait_for_parent() -> Result<()> {
             return Ok(());
         }
 
-        defer! {{ let _ = CloseHandle(parent); }}
+        let _g = scopeguard::guard(parent, |h| { let _ = CloseHandle(*h); });
 
         // Wait for our parent to exit
         let res = WaitForSingleObject(parent, INFINITE);
@@ -816,7 +817,7 @@ pub fn prepare_update() -> Result<Option<PathBuf>> {
     // Get host triple
     let triple = dist::get_host_triple();
 
-    let update_root = env::var("MULTIRUST_UPDATE_ROOT")
+    let update_root = env::var("RUSTUP_UPDATE_ROOT")
         .unwrap_or(String::from(UPDATE_ROOT));
 
     let tempdir = try!(TempDir::new("multirust-update")

src/multirust-mock/src/clitools.rs

@@ -14,14 +14,15 @@ use dist::{MockDistServer, MockChannel, MockPackage,
            MockTargettedPackage, MockComponent, change_channel_date,
            ManifestVersion};
 use hyper::Url;
+use scopeguard;
 
 /// The configuration used by the tests in this module
 pub struct Config {
     /// Where we put the multirust / rustc / cargo bins
     pub exedir: PathBuf,
     /// The distribution server
     pub distdir: PathBuf,
-    /// MULTIRUST_HOME
+    /// RUSTUP_HOME
     pub rustupdir: PathBuf,
     /// Custom toolchains
     pub customdir: PathBuf,
@@ -56,7 +57,7 @@ pub static MULTI_ARCH1: &'static str = "i686-unknown-linux-gnu";
 /// a mock dist server.
 pub fn setup(s: Scenario, f: &Fn(&Config)) {
     // Unset env variables that will break our testing
-    env::remove_var("MULTIRUST_TOOLCHAIN");
+    env::remove_var("RUSTUP_TOOLCHAIN");
 
     let exedir = TempDir::new("rustup-exe").unwrap();
     let distdir = TempDir::new("rustup-dist").unwrap();
@@ -206,8 +207,8 @@ pub fn cmd(config: &Config, name: &str, args: &[&str]) -> Command {
 }
 
 pub fn env(config: &Config, cmd: &mut Command) {
-    cmd.env("MULTIRUST_HOME", config.rustupdir.to_string_lossy().to_string());
-    cmd.env("MULTIRUST_DIST_ROOT", format!("file://{}", config.distdir.join("dist").to_string_lossy()));
+    cmd.env("RUSTUP_HOME", config.rustupdir.to_string_lossy().to_string());
+    cmd.env("RUSTUP_DIST_ROOT", format!("file://{}", config.distdir.join("dist").to_string_lossy()));
     cmd.env("CARGO_HOME", config.cargodir.to_string_lossy().to_string());
 
     // This is only used for some installation tests on unix where CARGO_HOME
@@ -232,9 +233,7 @@ pub fn run(config: &Config, name: &str, args: &[&str], env: &[(&str, &str)]) ->
 pub fn change_dir(path: &Path, f: &Fn()) {
     let cwd = env::current_dir().unwrap();
     env::set_current_dir(path).unwrap();
-    defer! {
-        env::set_current_dir(&cwd).unwrap()
-    }
+    let _g = scopeguard::guard(cwd, |d| env::set_current_dir(d).unwrap());
     f();
 }
 

src/multirust-mock/src/lib.rs

@@ -3,7 +3,6 @@
 extern crate hyper;
 #[macro_use]
 extern crate lazy_static;
-#[macro_use]
 extern crate scopeguard;
 extern crate walkdir;
 extern crate flate2;

src/multirust-utils/src/errors.rs

@@ -195,7 +195,7 @@ impl error::Error for Error {
             OpeningBrowser { error: None } => "could not open browser: no browser installed",
             SettingPermissions {..} => "failed to set permissions",
             CargoHome => "couldn't find value of CARGO_HOME",
-            MultirustHome => "couldn't find value of MULTIRUST_HOME",
+            MultirustHome => "couldn't find value of RUSTUP_HOME",
         }
     }
 
@@ -365,7 +365,7 @@ impl Display for Error {
                        error)
             },
             CargoHome => write!(f, "couldn't find value of CARGO_HOME"),
-            MultirustHome => write!(f, "couldn't find value of MULTIRUST_HOME"),
+            MultirustHome => write!(f, "couldn't find value of RUSTUP_HOME"),
         }
     }
 }

src/multirust-utils/src/lib.rs

@@ -4,7 +4,6 @@
 extern crate hyper;
 extern crate openssl;
 extern crate rand;
-#[macro_use]
 extern crate scopeguard;
 
 #[cfg(windows)]

src/multirust-utils/src/utils.rs

@@ -12,6 +12,7 @@ use errors::{Error, Notification, NotifyHandler};
 use raw;
 #[cfg(windows)]
 use winapi::DWORD;
+use scopeguard;
 
 pub use raw::{is_directory, is_file, path_exists, if_not_empty, random_string, prefix_arg,
                     has_cmd, find_cmd};
@@ -333,7 +334,7 @@ pub fn home_dir() -> Option<PathBuf> {
         if OpenProcessToken(me, TOKEN_READ, &mut token) == 0 {
             return None
         }
-        defer! {{ let _ = CloseHandle(token); }}
+        let _g = scopeguard::guard(token, |h| { let _ = CloseHandle(*h); });
         fill_utf16_buf(|buf, mut sz| {
             match GetUserProfileDirectoryW(token, buf, &mut sz) {
                 0 if GetLastError() != ERROR_INSUFFICIENT_BUFFER => 0,
@@ -436,7 +437,7 @@ pub fn cargo_home() -> Result<PathBuf> {
 
 pub fn multirust_home() -> Result<PathBuf> {
     let cwd = try!(env::current_dir().map_err(|_| Error::MultirustHome));
-    let multirust_home = env::var_os("MULTIRUST_HOME").map(|home| {
+    let multirust_home = env::var_os("RUSTUP_HOME").map(|home| {
         cwd.join(home)
     });
     let user_home = home_dir().map(|p| p.join(".multirust"));

src/multirust/config.rs

@@ -25,7 +25,7 @@ pub enum OverrideReason {
 impl Display for OverrideReason {
     fn fmt(&self, f: &mut fmt::Formatter) -> ::std::result::Result<(), fmt::Error> {
         match *self {
-            OverrideReason::Environment => write!(f, "environment override by MULTIRUST_TOOLCHAIN"),
+            OverrideReason::Environment => write!(f, "environment override by RUSTUP_TOOLCHAIN"),
             OverrideReason::OverrideDB(ref path) => {
                 write!(f, "directory override for '{}'", path.display())
             }
@@ -69,19 +69,19 @@ impl Cfg {
                                       }));
 
         // GPG key
-        let gpg_key = if let Some(path) = env::var_os("MULTIRUST_GPG_KEY")
+        let gpg_key = if let Some(path) = env::var_os("RUSTUP_GPG_KEY")
                                               .and_then(utils::if_not_empty) {
             Cow::Owned(try!(utils::read_file("public key", Path::new(&path))))
         } else {
             Cow::Borrowed(include_str!("rust-key.gpg.ascii"))
         };
 
         // Environment override
-        let env_override = env::var("MULTIRUST_TOOLCHAIN")
+        let env_override = env::var("RUSTUP_TOOLCHAIN")
                                .ok()
                                .and_then(utils::if_not_empty);
 
-        let dist_root_url = env::var("MULTIRUST_DIST_ROOT")
+        let dist_root_url = env::var("RUSTUP_DIST_ROOT")
                                 .ok()
                                 .and_then(utils::if_not_empty)
                                 .map_or(Cow::Borrowed(dist::DEFAULT_DIST_ROOT), Cow::Owned);

src/multirust/toolchain.rs

@@ -240,7 +240,7 @@ impl<'a> Toolchain<'a> {
     pub fn create_fallback_command<T: AsRef<OsStr>>(&self, binary: T,
                                                     primary_toolchain: &Toolchain) -> Result<Command> {
         let mut cmd = try!(self.create_command(binary));
-        cmd.env("MULTIRUST_TOOLCHAIN", &primary_toolchain.name);
+        cmd.env("RUSTUP_TOOLCHAIN", &primary_toolchain.name);
         Ok(cmd)
     }
 
@@ -257,8 +257,8 @@ impl<'a> Toolchain<'a> {
 
         env_var::inc("RUST_RECURSION_COUNT", cmd);
 
-        cmd.env("MULTIRUST_TOOLCHAIN", &self.name);
-        cmd.env("MULTIRUST_HOME", &self.cfg.multirust_dir);
+        cmd.env("RUSTUP_TOOLCHAIN", &self.name);
+        cmd.env("RUSTUP_HOME", &self.cfg.multirust_dir);
     }
 
     pub fn set_ldpath(&self, cmd: &mut Command) {

tests/cli-misc.rs

@@ -33,7 +33,7 @@ fn no_colors_in_piped_error_output() {
 #[test]
 fn rustc_with_bad_multirust_toolchain_env_var() {
     setup(&|config| {
-        let out = run(config, "rustc", &[], &[("MULTIRUST_TOOLCHAIN", "bogus")]);
+        let out = run(config, "rustc", &[], &[("RUSTUP_TOOLCHAIN", "bogus")]);
         assert!(!out.ok);
         assert!(out.stderr.contains("toolchain 'bogus' is not installed"));
     });
@@ -722,3 +722,15 @@ fn custom_toolchain_cargo_fallback_run() {
     });
 }
 
+fn multirust_env_compat() {
+    setup(&|config| {
+        let mut cmd = clitools::cmd(config, "rustup", &["update", "nightly"]);
+        clitools::env(config, &mut cmd);
+        cmd.env_remove("RUSTUP_HOME");
+        cmd.env("MULTIRUST_HOME", &config.rustupdir);
+        let out = cmd.output().unwrap();
+        assert!(out.status.success());
+        let stderr = String::from_utf8(out.stderr).unwrap();
+        assert!(stderr.contains("environment variable MULTIRUST_HOME is deprecated. Use RUSTUP_HOME"));
+    });
+}

tests/cli-rustup.rs

@@ -278,11 +278,11 @@ fn show_toolchain_env() {
         expect_ok(config, &["rustup", "default", "nightly"]);
         let mut cmd = clitools::cmd(config, "rustup", &["show"]);
         clitools::env(config, &mut cmd);
-        cmd.env("MULTIRUST_TOOLCHAIN", "nightly");
+        cmd.env("RUSTUP_TOOLCHAIN", "nightly");
         let out = cmd.output().unwrap();
         assert!(out.status.success());
         let stdout = String::from_utf8(out.stdout).unwrap();
-        assert!(stdout == "nightly (environment override by MULTIRUST_TOOLCHAIN)\n");
+        assert!(stdout == "nightly (environment override by RUSTUP_TOOLCHAIN)\n");
     });
 }
 
@@ -291,7 +291,7 @@ fn show_toolchain_env_not_installed() {
     setup(&|config| {
         let mut cmd = clitools::cmd(config, "rustup", &["show"]);
         clitools::env(config, &mut cmd);
-        cmd.env("MULTIRUST_TOOLCHAIN", "nightly");
+        cmd.env("RUSTUP_TOOLCHAIN", "nightly");
         let out = cmd.output().unwrap();
         // I'm not sure this should really be erroring when the toolchain
         // is not installed; just capturing the behavior.

tests/cli-self-update.rs

@@ -5,7 +5,6 @@ extern crate multirust_utils;
 #[macro_use]
 extern crate lazy_static;
 extern crate tempdir;
-#[macro_use]
 extern crate scopeguard;
 
 #[cfg(windows)]
@@ -38,8 +37,8 @@ pub fn setup(f: &Fn(&Config)) {
 
         // An windows these tests mess with the user's PATH. Save
         // and restore them here to keep from trashing things.
-        let ref saved_path = get_path();
-        defer! { restore_path(saved_path) }
+        let saved_path = get_path();
+        let _g = scopeguard::guard(saved_path, |p| restore_path(p));
 
         f(config);
     });
@@ -65,7 +64,7 @@ pub fn update_setup(f: &Fn(&Config, &Path)) {
         create_hash(dist_exe, dist_hash);
 
         let ref root_url = format!("file://{}", self_dist.display());
-        env::set_var("MULTIRUST_UPDATE_ROOT", root_url);
+        env::set_var("RUSTUP_UPDATE_ROOT", root_url);
 
         f(config, self_dist);
     });

tests/cli-v1.rs

@@ -332,7 +332,7 @@ fn show_override_from_multirust_toolchain_env_var() {
             expect_ok(config, &["multirust", "override", "nightly"]);
             // change_dir has a lock so it's ok to futz the environment
             let out = run(config, "multirust", &["show-override"],
-                          &[("MULTIRUST_TOOLCHAIN", "beta")]);
+                          &[("RUSTUP_TOOLCHAIN", "beta")]);
             assert!(out.ok);
             assert!(out.stdout.contains("override toolchain: beta"));
             assert!(out.stdout.contains("override reason: environment override"));

tests/cli-v2.rs

@@ -335,7 +335,7 @@ fn show_override_from_multirust_toolchain_env_var() {
             expect_ok(config, &["multirust", "override", "nightly"]);
             // change_dir has a lock so it's ok to futz the environment
             let out = run(config, "multirust", &["show-override"],
-                          &[("MULTIRUST_TOOLCHAIN", "beta")]);
+                          &[("RUSTUP_TOOLCHAIN", "beta")]);
             assert!(out.ok);
             assert!(out.stdout.contains("override toolchain: beta"));
             assert!(out.stdout.contains("override reason: environment override"));