Use only $nu.env.PWD for getting the current directory (#587)

* Use only $nu.env.PWD for getting current directory

Because setting and reading to/from std::env changes the global state
shich is problematic if we call `cd` from multiple threads (e.g., in a
`par-each` block).

With this change, when engine-q starts, it will either inherit existing
PWD env var, or create a new one from `std::env::current_dir()`.
Otherwise, everything that needs the current directory will get it from
`$nu.env.PWD`. Each spawned external command will get its current
directory per-process which should be thread-safe.

One thing left to do is to patch nu-path for this as well since it uses
`std::env::current_dir()` in its expansions.

* Rename nu-path functions

*_with is not *_relative which should be more descriptive and frees
"with" for use in a followup commit.

* Clone stack every each iter; Fix some commands

Cloning the stack each iteration of `each` makes sure we're not reusing
PWD between iterations.

Some fixes in commands to make them use the new PWD.

* Post-rebase cleanup, fmt, clippy

* Change back _relative to _with in nu-path funcs

Didn't use the idea I had for the new "_with".

* Remove leftover current_dir from rebase

* Add cwd sync at merge_delta()

This makes sure the parser and completer always have up-to-date cwd.

* Always pass absolute path to glob in ls

* Do not allow PWD a relative path; Allow recovery

Makes it possible to recover PWD by proceeding with the REPL cycle.

* Clone stack in each also for byte/string stream

* (WIP) Start moving env variables to engine state

* (WIP) Move env vars to engine state (ugly)

Quick and dirty code.

* (WIP) Remove unused mut and args; Fmt

* (WIP) Fix dataframe tests

* (WIP) Fix missing args after rebase

* (WIP) Clone only env vars, not the whole stack

* (WIP) Add env var clone to `for` loop as well

* Minor edits

* Refactor merge_delta() to include stack merging.

Less error-prone than doing it manually.

* Clone env for each `update` command iteration

* Mark env var hidden only when found in eng. state

* Fix clippt warnings

* Add TODO about env var reading

* Do not clone empty environment in loops

* Remove extra cwd collection

* Split current_dir() into str and path; Fix autocd

* Make completions respect PWD env var

crates/nu-command/src/core_commands/for_.rs

@@ -71,12 +71,16 @@ impl Command for For {
         let engine_state = engine_state.clone();
         let block = engine_state.get_block(block_id).clone();
         let mut stack = stack.collect_captures(&block.captures);
+        let orig_env_vars = stack.env_vars.clone();
+        let orig_env_hidden = stack.env_hidden.clone();
 
         match values {
             Value::List { vals, .. } => Ok(vals
                 .into_iter()
                 .enumerate()
                 .map(move |(idx, x)| {
+                    stack.with_env(&orig_env_vars, &orig_env_hidden);
+
                     stack.add_var(
                         var_id,
                         if numbered {
@@ -107,6 +111,8 @@ impl Command for For {
                 .into_range_iter()?
                 .enumerate()
                 .map(move |(idx, x)| {
+                    stack.with_env(&orig_env_vars, &orig_env_hidden);
+
                     stack.add_var(
                         var_id,
                         if numbered {

crates/nu-command/src/core_commands/hide.rs

@@ -98,12 +98,12 @@ impl Command for Hide {
                     return Err(ShellError::NonUtf8(import_pattern.span()));
                 };
 
-                if stack.remove_env_var(&name).is_none() {
+                if stack.remove_env_var(engine_state, &name).is_none() {
                     return Err(ShellError::NotFound(call.positional[0].span));
                 }
             }
         } else if !import_pattern.hidden.contains(&import_pattern.head.name)
-            && stack.remove_env_var(&head_name_str).is_none()
+            && stack.remove_env_var(engine_state, &head_name_str).is_none()
         {
             return Err(ShellError::NotFound(call.positional[0].span));
         }

crates/nu-command/src/dataframe/test_dataframe.rs

@@ -32,7 +32,8 @@ pub fn test_dataframe(cmds: Vec<Box<dyn Command + 'static>>) {
         working_set.render()
     };
 
-    let _ = engine_state.merge_delta(delta);
+    let cwd = std::env::current_dir().expect("Could not get current working directory.");
+    let _ = engine_state.merge_delta(delta, None, &cwd);
 
     for example in examples {
         // Skip tests that don't have results to compare to
@@ -52,7 +53,7 @@ pub fn test_dataframe(cmds: Vec<Box<dyn Command + 'static>>) {
             (output, working_set.render())
         };
 
-        let _ = engine_state.merge_delta(delta);
+        let _ = engine_state.merge_delta(delta, None, &cwd);
 
         let mut stack = Stack::new();
 

crates/nu-command/src/default_context.rs

@@ -1,8 +1,10 @@
 use nu_protocol::engine::{EngineState, StateWorkingSet};
 
+use std::path::Path;
+
 use crate::*;
 
-pub fn create_default_context() -> EngineState {
+pub fn create_default_context(cwd: impl AsRef<Path>) -> EngineState {
     let mut engine_state = EngineState::new();
 
     let delta = {
@@ -306,7 +308,7 @@ pub fn create_default_context() -> EngineState {
         working_set.render()
     };
 
-    let _ = engine_state.merge_delta(delta);
+    let _ = engine_state.merge_delta(delta, None, &cwd);
 
     engine_state
 }

crates/nu-command/src/env/env_command.rs

@@ -29,7 +29,8 @@ impl Command for Env {
         let span = call.head;
         let config = stack.get_config().unwrap_or_default();
 
-        let mut env_vars: Vec<(String, Value)> = stack.get_env_vars().into_iter().collect();
+        let mut env_vars: Vec<(String, Value)> =
+            stack.get_env_vars(engine_state).into_iter().collect();
         env_vars.sort_by(|(name1, _), (name2, _)| name1.cmp(name2));
 
         let mut values = vec![];

crates/nu-command/src/example_test.rs

@@ -47,7 +47,8 @@ pub fn test_examples(cmd: impl Command + 'static) {
         working_set.render()
     };
 
-    let _ = engine_state.merge_delta(delta);
+    let cwd = std::env::current_dir().expect("Could not get current working directory.");
+    let _ = engine_state.merge_delta(delta, None, &cwd);
 
     for example in examples {
         // Skip tests that don't have results to compare to
@@ -67,10 +68,19 @@ pub fn test_examples(cmd: impl Command + 'static) {
             (output, working_set.render())
         };
 
-        let _ = engine_state.merge_delta(delta);
+        let _ = engine_state.merge_delta(delta, None, &cwd);
 
         let mut stack = Stack::new();
 
+        // Set up PWD
+        stack.add_env_var(
+            "PWD".to_string(),
+            Value::String {
+                val: cwd.to_string_lossy().to_string(),
+                span: Span::test_data(),
+            },
+        );
+
         // Set up our initial config to start from
         stack.vars.insert(
             CONFIG_VARIABLE_ID,

crates/nu-command/src/filesystem/cd.rs

@@ -1,3 +1,4 @@
+use nu_engine::env::current_dir_str;
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
@@ -32,7 +33,10 @@ impl Command for Cd {
 
         let (path, span) = match path_val {
             Some(v) => {
-                let path = nu_path::expand_path(v.as_string()?);
+                let path = nu_path::canonicalize_with(
+                    v.as_string()?,
+                    current_dir_str(engine_state, stack)?,
+                )?;
                 (path.to_string_lossy().to_string(), v.span()?)
             }
             None => {
@@ -40,7 +44,6 @@ impl Command for Cd {
                 (path.to_string_lossy().to_string(), call.head)
             }
         };
-        let _ = std::env::set_current_dir(&path);
 
         //FIXME: this only changes the current scope, but instead this environment variable
         //should probably be a block that loads the information from the state in the overlay

crates/nu-command/src/filesystem/cp.rs

@@ -1,7 +1,7 @@
-use std::env::current_dir;
 use std::path::PathBuf;
 
 use super::util::get_interactive_confirmation;
+use nu_engine::env::current_dir;
 use nu_engine::CallExt;
 use nu_path::canonicalize_with;
 use nu_protocol::ast::Call;
@@ -49,7 +49,7 @@ impl Command for Cp {
         let interactive = call.has_flag("interactive");
         let force = call.has_flag("force");
 
-        let path = current_dir()?;
+        let path = current_dir(engine_state, stack)?;
         let source = path.join(source.as_str());
         let destination = path.join(destination.as_str());
 
@@ -135,7 +135,7 @@ impl Command for Cp {
 
         for entry in sources.into_iter().flatten() {
             let mut sources = FileStructure::new();
-            sources.walk_decorate(&entry)?;
+            sources.walk_decorate(&entry, engine_state, stack)?;
 
             if entry.is_file() {
                 let sources = sources.paths_applying_with(|(source_file, _depth_level)| {

crates/nu-command/src/filesystem/ls.rs

@@ -1,4 +1,5 @@
 use chrono::{DateTime, Utc};
+use nu_engine::env::current_dir;
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
@@ -10,6 +11,7 @@ use nu_protocol::{
 use std::io::ErrorKind;
 #[cfg(unix)]
 use std::os::unix::fs::PermissionsExt;
+use std::path::PathBuf;
 
 #[derive(Clone)]
 pub struct Ls;
@@ -63,10 +65,17 @@ impl Command for Ls {
 
         let call_span = call.head;
 
-        let pattern = if let Some(mut result) =
+        let (pattern, prefix) = if let Some(result) =
             call.opt::<Spanned<String>>(engine_state, stack, 0)?
         {
-            let path = std::path::Path::new(&result.item);
+            let path = PathBuf::from(&result.item);
+
+            let (mut path, prefix) = if path.is_relative() {
+                let cwd = current_dir(engine_state, stack)?;
+                (cwd.join(path), Some(cwd))
+            } else {
+                (path, None)
+            };
 
             if path.is_dir() {
                 if permission_denied(&path) {
@@ -92,16 +101,14 @@ impl Command for Ls {
                 }
 
                 if path.is_dir() {
-                    if !result.item.ends_with(std::path::MAIN_SEPARATOR) {
-                        result.item.push(std::path::MAIN_SEPARATOR);
-                    }
-                    result.item.push('*');
+                    path = path.join("*");
                 }
             }
 
-            result.item
+            (path.to_string_lossy().to_string(), prefix)
         } else {
-            "*".into()
+            let cwd = current_dir(engine_state, stack)?;
+            (cwd.join("*").to_string_lossy().to_string(), Some(cwd))
         };
 
         let glob = glob::glob(&pattern).map_err(|err| {
@@ -144,11 +151,34 @@ impl Command for Ls {
                         return None;
                     }
 
-                    let entry =
-                        dir_entry_dict(&path, metadata.as_ref(), call_span, long, short_names);
+                    let display_name = if short_names {
+                        path.file_name().and_then(|s| s.to_str())
+                    } else if let Some(pre) = &prefix {
+                        match path.strip_prefix(pre) {
+                            Ok(stripped) => stripped.to_str(),
+                            Err(_) => path.to_str(),
+                        }
+                    } else {
+                        path.to_str()
+                    }
+                    .ok_or_else(|| {
+                        ShellError::SpannedLabeledError(
+                            format!("Invalid file name: {:}", path.to_string_lossy()),
+                            "invalid file name".into(),
+                            call_span,
+                        )
+                    });
 
-                    match entry {
-                        Ok(value) => Some(value),
+                    match display_name {
+                        Ok(name) => {
+                            let entry =
+                                dir_entry_dict(&path, name, metadata.as_ref(), call_span, long);
+
+                            match entry {
+                                Ok(value) => Some(value),
+                                Err(err) => Some(Value::Error { error: err }),
+                            }
+                        }
                         Err(err) => Some(Value::Error { error: err }),
                     }
                 }
@@ -213,7 +243,7 @@ fn path_contains_hidden_folder(path: &Path, folders: &[PathBuf]) -> bool {
 
 #[cfg(unix)]
 use std::os::unix::fs::FileTypeExt;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 
 pub fn get_file_type(md: &std::fs::Metadata) -> &str {
     let ft = md.file_type();
@@ -243,31 +273,18 @@ pub fn get_file_type(md: &std::fs::Metadata) -> &str {
 
 #[allow(clippy::too_many_arguments)]
 pub(crate) fn dir_entry_dict(
-    filename: &std::path::Path,
+    filename: &std::path::Path, // absolute path
+    display_name: &str,         // gile name to be displayed
     metadata: Option<&std::fs::Metadata>,
     span: Span,
     long: bool,
-    short_name: bool,
 ) -> Result<Value, ShellError> {
     let mut cols = vec![];
     let mut vals = vec![];
 
-    let name = if short_name {
-        filename.file_name().and_then(|s| s.to_str())
-    } else {
-        filename.to_str()
-    }
-    .ok_or_else(|| {
-        ShellError::SpannedLabeledError(
-            format!("Invalid file name: {:}", filename.to_string_lossy()),
-            "invalid file name".into(),
-            span,
-        )
-    })?;
-
     cols.push("name".into());
     vals.push(Value::String {
-        val: name.to_string(),
+        val: display_name.to_string(),
         span,
     });
 

crates/nu-command/src/filesystem/mkdir.rs

@@ -1,6 +1,6 @@
 use std::collections::VecDeque;
-use std::env::current_dir;
 
+use nu_engine::env::current_dir;
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
@@ -39,7 +39,7 @@ impl Command for Mkdir {
         call: &Call,
         _input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        let path = current_dir()?;
+        let path = current_dir(engine_state, stack)?;
         let mut directories = call
             .rest::<String>(engine_state, stack, 0)?
             .into_iter()

crates/nu-command/src/filesystem/mv.rs

@@ -1,7 +1,7 @@
-use std::env::current_dir;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 
 use super::util::get_interactive_confirmation;
+use nu_engine::env::current_dir;
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
@@ -50,7 +50,7 @@ impl Command for Mv {
         let interactive = call.has_flag("interactive");
         let force = call.has_flag("force");
 
-        let path: PathBuf = current_dir()?;
+        let path = current_dir(engine_state, stack)?;
         let source = path.join(spanned_source.item.as_str());
         let destination = path.join(destination.as_str());
 

crates/nu-command/src/filesystem/rm.rs

@@ -1,10 +1,10 @@
-use std::env::current_dir;
 #[cfg(unix)]
 use std::os::unix::prelude::FileTypeExt;
 use std::path::PathBuf;
 
 use super::util::get_interactive_confirmation;
 
+use nu_engine::env::current_dir;
 use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
@@ -86,7 +86,7 @@ fn rm(
         ));
     }
 
-    let current_path = current_dir()?;
+    let current_path = current_dir(engine_state, stack)?;
     let mut paths = call
         .rest::<String>(engine_state, stack, 0)?
         .into_iter()

crates/nu-command/src/filesystem/touch.rs

@@ -1,6 +1,8 @@
 use std::fs::OpenOptions;
 
+use nu_engine::env::current_dir_str;
 use nu_engine::CallExt;
+use nu_path::expand_path_with;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::{Category, PipelineData, ShellError, Signature, SyntaxShape};
@@ -39,7 +41,8 @@ impl Command for Touch {
         let rest: Vec<String> = call.rest(engine_state, stack, 1)?;
 
         for (index, item) in vec![target].into_iter().chain(rest).enumerate() {
-            match OpenOptions::new().write(true).create(true).open(&item) {
+            let path = expand_path_with(&item, current_dir_str(engine_state, stack)?);
+            match OpenOptions::new().write(true).create(true).open(&path) {
                 Ok(_) => continue,
                 Err(err) => {
                     return Err(ShellError::CreateNotPossible(

crates/nu-command/src/filesystem/util.rs

@@ -1,6 +1,8 @@
 use std::path::{Path, PathBuf};
 
+use nu_engine::env::current_dir_str;
 use nu_path::canonicalize_with;
+use nu_protocol::engine::{EngineState, Stack};
 use nu_protocol::ShellError;
 
 use dialoguer::Input;
@@ -39,16 +41,27 @@ impl FileStructure {
             .collect()
     }
 
-    pub fn walk_decorate(&mut self, start_path: &Path) -> Result<(), ShellError> {
+    pub fn walk_decorate(
+        &mut self,
+        start_path: &Path,
+        engine_state: &EngineState,
+        stack: &Stack,
+    ) -> Result<(), ShellError> {
         self.resources = Vec::<Resource>::new();
-        self.build(start_path, 0)?;
+        self.build(start_path, 0, engine_state, stack)?;
         self.resources.sort();
 
         Ok(())
     }
 
-    fn build(&mut self, src: &Path, lvl: usize) -> Result<(), ShellError> {
-        let source = canonicalize_with(src, std::env::current_dir()?)?;
+    fn build(
+        &mut self,
+        src: &Path,
+        lvl: usize,
+        engine_state: &EngineState,
+        stack: &Stack,
+    ) -> Result<(), ShellError> {
+        let source = canonicalize_with(src, current_dir_str(engine_state, stack)?)?;
 
         if source.is_dir() {
             for entry in std::fs::read_dir(src)? {
@@ -56,7 +69,7 @@ impl FileStructure {
                 let path = entry.path();
 
                 if path.is_dir() {
-                    self.build(&path, lvl + 1)?;
+                    self.build(&path, lvl + 1, engine_state, stack)?;
                 }
 
                 self.resources.push(Resource {

crates/nu-command/src/filters/each.rs

@@ -71,6 +71,8 @@ impl Command for Each {
         let engine_state = engine_state.clone();
         let block = engine_state.get_block(block_id).clone();
         let mut stack = stack.collect_captures(&block.captures);
+        let orig_env_vars = stack.env_vars.clone();
+        let orig_env_hidden = stack.env_hidden.clone();
         let span = call.head;
 
         match input {
@@ -80,6 +82,8 @@ impl Command for Each {
                 .into_iter()
                 .enumerate()
                 .map(move |(idx, x)| {
+                    stack.with_env(&orig_env_vars, &orig_env_hidden);
+
                     if let Some(var) = block.signature.get_positional(0) {
                         if let Some(var_id) = &var.var_id {
                             if numbered {
@@ -113,6 +117,8 @@ impl Command for Each {
                 .into_iter()
                 .enumerate()
                 .map(move |(idx, x)| {
+                    stack.with_env(&orig_env_vars, &orig_env_hidden);
+
                     let x = match x {
                         Ok(x) => Value::Binary { val: x, span },
                         Err(err) => return Value::Error { error: err },
@@ -151,6 +157,8 @@ impl Command for Each {
                 .into_iter()
                 .enumerate()
                 .map(move |(idx, x)| {
+                    stack.with_env(&orig_env_vars, &orig_env_hidden);
+
                     let x = match x {
                         Ok(x) => Value::String { val: x, span },
                         Err(err) => return Value::Error { error: err },
@@ -192,7 +200,7 @@ impl Command for Each {
                 for (col, val) in cols.into_iter().zip(vals.into_iter()) {
                     let block = engine_state.get_block(block_id);
 
-                    let mut stack = stack.clone();
+                    stack.with_env(&orig_env_vars, &orig_env_hidden);
 
                     if let Some(var) = block.signature.get_positional(0) {
                         if let Some(var_id) = &var.var_id {

crates/nu-command/src/filters/update.rs

@@ -74,9 +74,13 @@ fn update(
         let block = engine_state.get_block(block_id).clone();
 
         let mut stack = stack.collect_captures(&block.captures);
+        let orig_env_vars = stack.env_vars.clone();
+        let orig_env_hidden = stack.env_hidden.clone();
 
         input.map(
             move |mut input| {
+                stack.with_env(&orig_env_vars, &orig_env_hidden);
+
                 if let Some(var) = block.signature.get_positional(0) {
                     if let Some(var_id) = &var.var_id {
                         stack.add_var(*var_id, input.clone())

crates/nu-command/src/path/expand.rs

@@ -1,7 +1,8 @@
 use std::path::Path;
 
+use nu_engine::env::current_dir_str;
 use nu_engine::CallExt;
-use nu_path::{canonicalize, expand_path};
+use nu_path::{canonicalize_with, expand_path};
 use nu_protocol::{engine::Command, Example, ShellError, Signature, Span, SyntaxShape, Value};
 
 use super::PathSubcommandArguments;
@@ -9,6 +10,7 @@ use super::PathSubcommandArguments;
 struct Arguments {
     strict: bool,
     columns: Option<Vec<String>>,
+    cwd: String,
 }
 
 impl PathSubcommandArguments for Arguments {
@@ -55,6 +57,7 @@ impl Command for SubCommand {
         let args = Arguments {
             strict: call.has_flag("strict"),
             columns: call.get_flag(engine_state, stack, "columns")?,
+            cwd: current_dir_str(engine_state, stack)?,
         };
 
         input.map(
@@ -107,7 +110,7 @@ impl Command for SubCommand {
 }
 
 fn expand(path: &Path, span: Span, args: &Arguments) -> Value {
-    if let Ok(p) = canonicalize(path) {
+    if let Ok(p) = canonicalize_with(path, &args.cwd) {
         Value::string(p.to_string_lossy(), span)
     } else if args.strict {
         Value::Error {

crates/nu-command/src/system/run_external.rs

@@ -1,5 +1,4 @@
 use std::collections::HashMap;
-use std::env;
 use std::io::{BufRead, BufReader, Write};
 use std::process::{Command as CommandSys, Stdio};
 use std::sync::atomic::Ordering;
@@ -113,9 +112,19 @@ impl<'call> ExternalCommand<'call> {
 
         // TODO. We don't have a way to know the current directory
         // This should be information from the EvaluationContex or EngineState
-        let path = env::current_dir()?;
-
-        process.current_dir(path);
+        if let Some(d) = self.env_vars.get("PWD") {
+            process.current_dir(d);
+        } else {
+            return Err(ShellError::SpannedLabeledErrorHelp(
+                "Current directory not found".to_string(),
+                "did not find PWD environment variable".to_string(),
+                head,
+                concat!(
+                    "The environment variable 'PWD' was not found. ",
+                    "It is required to define the current directory when running an external command."
+                ).to_string(),
+            ));
+        }
 
         process.envs(&self.env_vars);
 

crates/nu-command/src/viewers/griddle.rs

@@ -62,7 +62,7 @@ prints out the list properly."#
         let color_param: bool = call.has_flag("color");
         let separator_param: Option<String> = call.get_flag(engine_state, stack, "separator")?;
         let config = stack.get_config().unwrap_or_default();
-        let env_str = match stack.get_env_var("LS_COLORS") {
+        let env_str = match stack.get_env_var(engine_state, "LS_COLORS") {
             Some(v) => Some(env_to_string("LS_COLORS", v, engine_state, stack, &config)?),
             None => None,
         };

crates/nu-command/src/viewers/table.rs

@@ -112,7 +112,7 @@ impl Command for Table {
                         let config = config.clone();
                         let ctrlc = ctrlc.clone();
 
-                        let ls_colors = match stack.get_env_var("LS_COLORS") {
+                        let ls_colors = match stack.get_env_var(engine_state, "LS_COLORS") {
                             Some(v) => LsColors::from_string(&env_to_string(
                                 "LS_COLORS",
                                 v,