diff --git a/src/env_var.rs b/src/env_var.rs index ae9670ae4b..ab48f5bc88 100644 --- a/src/env_var.rs +++ b/src/env_var.rs @@ -1,3 +1,4 @@ +use std::collections::VecDeque; use std::env; use std::path::PathBuf; use std::process::Command; @@ -21,15 +22,19 @@ fn append_path(name: &str, value: Vec, cmd: &mut Command) { } } -pub(crate) fn prepend_path(name: &str, value: Vec, cmd: &mut Command) { +pub(crate) fn prepend_path(name: &str, prepend: Vec, cmd: &mut Command) { let old_value = process().var_os(name); - let mut parts: Vec; - if let Some(ref v) = old_value { - parts = value; - parts.extend(env::split_paths(v).collect::>()); + let parts = if let Some(ref v) = old_value { + let mut tail = env::split_paths(v).collect::>(); + for path in prepend.into_iter().rev() { + if !tail.contains(&path) { + tail.push_front(path); + } + } + tail } else { - parts = value; - } + prepend.into() + }; if let Ok(new_value) = env::join_paths(parts) { cmd.env(name, new_value); diff --git a/tests/cli-misc.rs b/tests/cli-misc.rs index ca00dc8da4..bf199b451d 100644 --- a/tests/cli-misc.rs +++ b/tests/cli-misc.rs @@ -3,8 +3,8 @@ pub mod mock; -use std::env::consts::EXE_SUFFIX; use std::str; +use std::{env::consts::EXE_SUFFIX, path::Path}; use rustup::for_host; use rustup::test::this_host_triple; @@ -298,6 +298,92 @@ fn rustup_run_searches_path() { }); } +#[test] +fn rustup_doesnt_prepend_path_unnecessarily() { + setup(&|config| { + expect_ok(config, &["rustup", "default", "nightly"]); + + let expect_stderr_ok_env_first_then = + |config: &Config, + args: &[&str], + env: &[(&str, &str)], + first: &Path, + second: Option<&Path>| { + let out = run(config, args[0], &args[1..], env); + let first_then_second = |list: &str| -> bool { + let mut saw_first = false; + let mut saw_second = false; + for path in std::env::split_paths(list) { + if path == first { + if saw_second { + return false; + } + saw_first = true; + } + if Some(&*path) == second { + if !saw_first { + return false; + } + saw_second = true; + } + } + true + }; + if !out.ok || !first_then_second(&out.stderr) { + clitools::print_command(args, &out); + println!("expected.ok: true"); + clitools::print_indented( + "expected.stderr.first_then", + &format!("{} comes before {:?}", first.display(), second), + ); + panic!(); + } + }; + + // For all of these, CARGO_HOME/bin will be auto-prepended. + let cargo_home_bin = config.cargodir.join("bin"); + expect_stderr_ok_env_first_then( + config, + &["cargo", "--echo-path"], + &[], + &cargo_home_bin, + None, + ); + expect_stderr_ok_env_first_then( + config, + &["cargo", "--echo-path"], + &[("PATH", "")], + &cargo_home_bin, + None, + ); + + // Check that CARGO_HOME/bin is prepended to path. + expect_stderr_ok_env_first_then( + config, + &["cargo", "--echo-path"], + &[("PATH", &format!("{}", config.exedir.display()))], + &cargo_home_bin, + Some(&config.exedir), + ); + + // But if CARGO_HOME/bin is already on PATH, it will not be prepended again, + // so exedir will take precedence. + expect_stderr_ok_env_first_then( + config, + &["cargo", "--echo-path"], + &[( + "PATH", + std::env::join_paths([&config.exedir, &cargo_home_bin]) + .unwrap() + .to_str() + .unwrap(), + )], + &config.exedir, + Some(&cargo_home_bin), + ); + }); +} + #[test] fn rustup_failed_path_search() { setup(&|config| { diff --git a/tests/mock/clitools.rs b/tests/mock/clitools.rs index da1f7c8266..25e2fe3d7b 100644 --- a/tests/mock/clitools.rs +++ b/tests/mock/clitools.rs @@ -437,7 +437,7 @@ pub fn expect_component_not_executable(config: &Config, cmd: &str) { } } -fn print_command(args: &[&str], out: &SanitizedOutput) { +pub(crate) fn print_command(args: &[&str], out: &SanitizedOutput) { print!("\n>"); for arg in args { if arg.contains(' ') { @@ -452,7 +452,7 @@ fn print_command(args: &[&str], out: &SanitizedOutput) { print_indented("out.stderr", &out.stderr); } -fn print_indented(heading: &str, text: &str) { +pub(crate) fn print_indented(heading: &str, text: &str) { let mut lines = text.lines().count(); // The standard library treats `a\n` and `a` as both being one line. // This is confusing when the test fails because of a missing newline. diff --git a/tests/mock/mock_bin_src.rs b/tests/mock/mock_bin_src.rs index b386ebab02..7acafb67e1 100644 --- a/tests/mock/mock_bin_src.rs +++ b/tests/mock/mock_bin_src.rs @@ -67,6 +67,10 @@ fn main() { writeln!(out, "{}", arg.to_string_lossy()).unwrap(); } } + Some("--echo-path") => { + let mut out = io::stderr(); + writeln!(out, "{}", std::env::var("PATH").unwrap()).unwrap(); + } _ => panic!("bad mock proxy commandline"), } }