Don't insert PATH variable on Windows (#3422)

* Don't insert PATH variable on Windows

* Simplify fix

* Just centralize the var

* Add a message about why we have to workaround the issue
This commit is contained in:
JT 2021-05-13 15:03:49 +12:00 committed by GitHub
parent 874ecd6c88
commit 9b8b1bad57
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 42 additions and 25 deletions

View file

@ -6,6 +6,7 @@ use indexmap::set::IndexSet;
use super::matchers::Matcher;
use crate::completion::{Completer, CompletionContext, Suggestion};
use nu_engine::EvaluationContext;
use nu_test_support::NATIVE_PATH_ENV_VAR;
pub struct CommandCompleter;
@ -121,7 +122,7 @@ fn is_executable(path: &Path) -> bool {
// TODO cache these, but watch for changes to PATH
fn find_path_executables() -> Option<IndexSet<String>> {
let path_var = std::env::var_os("PATH")?;
let path_var = std::env::var_os(NATIVE_PATH_ENV_VAR)?;
let paths: Vec<_> = std::env::split_paths(&path_var).collect();
let mut executables: IndexSet<String> = IndexSet::new();

View file

@ -1,6 +1,7 @@
use crate::prelude::*;
use nu_engine::{evaluate_baseline_expr, BufCodecReader};
use nu_engine::{MaybeTextCodec, StringOrBinary};
use nu_test_support::NATIVE_PATH_ENV_VAR;
use parking_lot::Mutex;
use std::io::Write;
@ -518,7 +519,7 @@ fn remove_quotes(argument: &str) -> Option<&str> {
fn shell_os_paths() -> Vec<std::path::PathBuf> {
let mut original_paths = vec![];
if let Some(paths) = std::env::var_os("PATH") {
if let Some(paths) = std::env::var_os(NATIVE_PATH_ENV_VAR) {
original_paths = std::env::split_paths(&paths).collect::<Vec<_>>();
}

View file

@ -3,6 +3,7 @@ use indexmap::IndexMap;
use nu_errors::ShellError;
use nu_protocol::Value;
use nu_source::Tag;
use nu_test_support::NATIVE_PATH_ENV_VAR;
use std::{fmt::Debug, path::PathBuf};
use super::write;
@ -147,7 +148,7 @@ impl NuConfig {
pub fn path(&self) -> Result<Option<Vec<PathBuf>>, ShellError> {
let vars = &self.vars;
if let Some(path) = vars.get("path").or_else(|| vars.get("PATH")) {
if let Some(path) = vars.get("path").or_else(|| vars.get(NATIVE_PATH_ENV_VAR)) {
path
.table_entries()
.map(|p| {

View file

@ -16,6 +16,7 @@ nu-source = { version = "0.31.1", path = "../nu-source" }
nu-stream = { version = "0.31.1", path = "../nu-stream" }
nu-value-ext = { version = "0.31.1", path = "../nu-value-ext" }
nu-ansi-term = { version = "0.31.1", path = "../nu-ansi-term" }
nu-test-support = { version = "0.31.1", path = "../nu-test-support" }
trash = { version = "1.3.0", optional = true }
which = { version = "4.0.2", optional = true }

View file

@ -16,6 +16,7 @@ pub fn nu(
let mut nu_dict = TaggedDictBuilder::new(&tag);
let mut dict = TaggedDictBuilder::new(&tag);
for v in env.iter() {
if v.0 != "PATH" && v.0 != "Path" {
dict.insert_untagged(v.0, UntaggedValue::string(v.1));
@ -49,6 +50,19 @@ pub fn nu(
}
}
// A note about environment variables:
//
// Environment variables in Unix platforms are case-sensitive. On Windows, case-sensitivity is context-dependent.
// In DOS, running `SET` will show you the list of environment variables, but their names will be all upper-cased.
// In PowerShell, running `Get-ChildItem Env:` will show you a list of environment variables, and they will match
// the case in the environment variable section of the user configuration
//
// Rust currently returns the DOS-style, all-uppercase environment variables on Windows (as of 1.52) when running
// std::env::vars(), rather than the case-sensitive Environment.GetEnvironmentVariables() of .NET that PowerShell
// uses.
//
// For now, we work around the discrepency as best we can by merging the two into what is shown to the user as the
// 'path' column of `$nu`
let mut table = vec![];
for v in env.iter() {
if v.0 == "PATH" || v.0 == "Path" {

View file

@ -11,6 +11,7 @@ use nu_errors::ShellError;
use nu_protocol::{hir, ConfigPath};
use nu_source::{Span, Tag};
use nu_stream::InputStream;
use nu_test_support::NATIVE_PATH_ENV_VAR;
use parking_lot::Mutex;
use std::sync::atomic::AtomicBool;
use std::{path::Path, sync::Arc};
@ -112,7 +113,7 @@ impl EvaluationContext {
let env_vars = self.scope.get_env_vars();
for (var, val) in env_vars {
if var == "PATH" || var == "Path" || var == "path" {
if var == NATIVE_PATH_ENV_VAR {
std::env::set_var(var, val);
break;
}
@ -174,13 +175,7 @@ impl EvaluationContext {
let joined_paths = cfg_paths
.map(|mut cfg_paths| {
//existing paths are prepended to path
let env_paths = if let Some(env_paths) = self.scope.get_env("PATH") {
Some(env_paths)
} else if let Some(env_paths) = self.scope.get_env("Path") {
Some(env_paths)
} else {
self.scope.get_env("path")
};
let env_paths = self.scope.get_env(NATIVE_PATH_ENV_VAR);
if let Some(env_paths) = env_paths {
let mut env_paths = std::env::split_paths(&env_paths).collect::<Vec<_>>();
@ -210,7 +205,7 @@ impl EvaluationContext {
self.scope.enter_scope_with_tag(tag);
self.scope.add_env(cfg.env_map());
if let Some(path) = joined_paths {
self.scope.add_env_var("PATH", path);
self.scope.add_env_var(NATIVE_PATH_ENV_VAR, path);
}
self.scope.set_exit_scripts(exit_scripts);
@ -243,13 +238,7 @@ impl EvaluationContext {
let joined_paths = cfg_paths
.map(|mut cfg_paths| {
//existing paths are prepended to path
let env_paths = if let Some(env_paths) = self.scope.get_env("PATH") {
Some(env_paths)
} else if let Some(env_paths) = self.scope.get_env("Path") {
Some(env_paths)
} else {
self.scope.get_env("path")
};
let env_paths = self.scope.get_env(NATIVE_PATH_ENV_VAR);
if let Some(env_paths) = env_paths {
let mut env_paths = std::env::split_paths(&env_paths).collect::<Vec<_>>();
@ -279,7 +268,7 @@ impl EvaluationContext {
frame.env = cfg.env_map();
if let Some(path) = joined_paths {
frame.env.insert("PATH".to_string(), path);
frame.env.insert(NATIVE_PATH_ENV_VAR.to_string(), path);
}
frame.exitscripts = exit_scripts;

View file

@ -9,6 +9,11 @@ pub struct Outcome {
pub err: String,
}
#[cfg(windows)]
pub const NATIVE_PATH_ENV_VAR: &str = "Path";
#[cfg(not(windows))]
pub const NATIVE_PATH_ENV_VAR: &str = "PATH";
impl Outcome {
pub fn new(out: String, err: String) -> Outcome {
Outcome { out, err }
@ -29,7 +34,7 @@ pub fn pipeline(commands: &str) -> String {
pub fn shell_os_paths() -> Vec<std::path::PathBuf> {
let mut original_paths = vec![];
if let Some(paths) = std::env::var_os("PATH") {
if let Some(paths) = std::env::var_os(NATIVE_PATH_ENV_VAR) {
original_paths = std::env::split_paths(&paths).collect::<Vec<_>>();
}

View file

@ -18,6 +18,7 @@ macro_rules! nu {
pub use std::error::Error;
pub use std::io::prelude::*;
pub use std::process::{Command, Stdio};
pub use $crate::NATIVE_PATH_ENV_VAR;
let commands = &*format!(
"
@ -46,7 +47,7 @@ macro_rules! nu {
};
let mut process = match Command::new($crate::fs::executable_path())
.env("PATH", paths_joined)
.env(NATIVE_PATH_ENV_VAR, paths_joined)
.arg("--skip-plugins")
.arg("--no-history")
.arg("--config-file")
@ -98,6 +99,7 @@ macro_rules! nu_with_plugins {
pub use std::error::Error;
pub use std::io::prelude::*;
pub use std::process::{Command, Stdio};
pub use crate::NATIVE_PATH_ENV_VAR;
let commands = &*format!(
"
@ -126,7 +128,7 @@ macro_rules! nu_with_plugins {
};
let mut process = match Command::new($crate::fs::executable_path())
.env("PATH", paths_joined)
.env(NATIVE_PATH_ENV_VAR, paths_joined)
.stdout(Stdio::piped())
.stdin(Stdio::piped())
.stderr(Stdio::piped())

View file

@ -99,7 +99,7 @@ impl NuProcess {
Err(_) => panic!("Couldn't join paths for PATH var."),
};
command.env("PATH", paths_joined);
command.env(crate::NATIVE_PATH_ENV_VAR, paths_joined);
for env_var in &self.environment_vars {
command.env(&env_var.name, &env_var.value);

View file

@ -103,7 +103,10 @@ fn inherited_environment_path_values_not_present_in_configuration_should_pick_up
"#,
)])
.with_config(&file)
.with_env("PATH", &PathBuf::from("/path/to/be/added").display_path());
.with_env(
nu_test_support::NATIVE_PATH_ENV_VAR,
&PathBuf::from("/path/to/be/added").display_path(),
);
assert_that!(
nu.pipeline("echo $nu.path | str collect '-'"),