Make env var eval order during "use" deterministic (#742)

* Make env var eval order during "use" deterministic

Fixes #726.

* Merge delta after getting config

To make sure env vars are all in the engine state and not in the stack.

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

@@ -109,8 +109,6 @@ impl<'call> ExternalCommand<'call> {
 
         let ctrlc = engine_state.ctrlc.clone();
 
-        // TODO. We don't have a way to know the current directory
-        // This should be information from the EvaluationContex or EngineState
         let mut process = if let Some(d) = self.env_vars.get("PWD") {
             let mut process = self.create_command(d);
             process.current_dir(d);

crates/nu-parser/src/parse_keywords.rs

@@ -7,7 +7,7 @@ use nu_protocol::{
     engine::StateWorkingSet,
     span, Exportable, Overlay, PositionalArg, Span, SyntaxShape, Type, CONFIG_VARIABLE_ID,
 };
-use std::collections::{HashMap, HashSet};
+use std::collections::HashSet;
 
 use crate::{
     lex, lite_parse,
@@ -1074,25 +1074,13 @@ pub fn parse_hide(
             } else if import_pattern.members.is_empty() {
                 // The pattern head can be e.g. a function name, not just a module
                 if let Some(id) = working_set.find_decl(&import_pattern.head.name) {
-                    let mut decls = HashMap::new();
+                    let mut overlay = Overlay::new();
-                    decls.insert(import_pattern.head.name.clone(), id);
+                    overlay.add_decl(&import_pattern.head.name, id);
 
-                    (
+                    (false, overlay)
-                        false,
-                        Overlay {
-                            decls,
-                            env_vars: HashMap::new(),
-                        },
-                    )
                 } else {
                     // Or it could be an env var
-                    (
+                    (false, Overlay::new())
-                        false,
-                        Overlay {
-                            decls: HashMap::new(),
-                            env_vars: HashMap::new(),
-                        },
-                    )
                 }
             } else {
                 return (

crates/nu-protocol/src/engine/engine_state.rs

@@ -586,7 +586,27 @@ pub struct StateDelta {
     plugins_changed: bool, // marks whether plugin file should be updated
 }
 
+impl Default for StateDelta {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl StateDelta {
+    pub fn new() -> Self {
+        StateDelta {
+            files: vec![],
+            file_contents: vec![],
+            vars: vec![],
+            decls: vec![],
+            blocks: vec![],
+            overlays: vec![],
+            scope: vec![ScopeFrame::new()],
+            #[cfg(feature = "plugin")]
+            plugins_changed: false,
+        }
+    }
+
     pub fn num_files(&self) -> usize {
         self.files.len()
     }
@@ -615,17 +635,7 @@ impl StateDelta {
 impl<'a> StateWorkingSet<'a> {
     pub fn new(permanent_state: &'a EngineState) -> Self {
         Self {
-            delta: StateDelta {
+            delta: StateDelta::new(),
-                files: vec![],
-                file_contents: vec![],
-                vars: vec![],
-                decls: vec![],
-                blocks: vec![],
-                overlays: vec![],
-                scope: vec![ScopeFrame::new()],
-                #[cfg(feature = "plugin")]
-                plugins_changed: false,
-            },
             permanent_state,
         }
     }

crates/nu-protocol/src/overlay.rs

@@ -1,6 +1,6 @@
 use crate::{BlockId, DeclId};
 
-use std::collections::HashMap;
+use indexmap::IndexMap;
 
 // TODO: Move the import pattern matching logic here from use/hide commands and
 // parse_use/parse_hide
@@ -8,15 +8,15 @@ use std::collections::HashMap;
 /// Collection of definitions that can be exported from a module
 #[derive(Debug, Clone)]
 pub struct Overlay {
-    pub decls: HashMap<Vec<u8>, DeclId>,
+    pub decls: IndexMap<Vec<u8>, DeclId>,
-    pub env_vars: HashMap<Vec<u8>, BlockId>,
+    pub env_vars: IndexMap<Vec<u8>, BlockId>,
 }
 
 impl Overlay {
     pub fn new() -> Self {
         Overlay {
-            decls: HashMap::new(),
+            decls: IndexMap::new(),
-            env_vars: HashMap::new(),
+            env_vars: IndexMap::new(),
         }
     }
 

src/main.rs

@@ -15,7 +15,7 @@ use nu_engine::{convert_env_values, eval_block};
 use nu_parser::{lex, parse, trim_quotes, Token, TokenContents};
 use nu_protocol::{
     ast::Call,
-    engine::{EngineState, Stack, StateWorkingSet},
+    engine::{EngineState, Stack, StateDelta, StateWorkingSet},
     Config, PipelineData, ShellError, Span, Value, CONFIG_VARIABLE_ID,
 };
 use reedline::{
@@ -167,6 +167,20 @@ fn main() -> Result<()> {
             }
         };
 
+        // Merge the delta in case env vars changed in the config
+        match nu_engine::env::current_dir(&engine_state, &stack) {
+            Ok(cwd) => {
+                if let Err(e) = engine_state.merge_delta(StateDelta::new(), Some(&mut stack), cwd) {
+                    let working_set = StateWorkingSet::new(&engine_state);
+                    report_error(&working_set, &e);
+                }
+            }
+            Err(e) => {
+                let working_set = StateWorkingSet::new(&engine_state);
+                report_error(&working_set, &e);
+            }
+        }
+
         // Translate environment variables from Strings to Values
         if let Some(e) = convert_env_values(&mut engine_state, &stack, &config) {
             let working_set = StateWorkingSet::new(&engine_state);
@@ -314,6 +328,23 @@ fn main() -> Result<()> {
 
                     if let Ok(contents) = std::fs::read_to_string(&config_path) {
                         eval_source(&mut engine_state, &mut stack, &contents, &config_filename);
+                        // Merge the delta in case env vars changed in the config
+                        match nu_engine::env::current_dir(&engine_state, &stack) {
+                            Ok(cwd) => {
+                                if let Err(e) = engine_state.merge_delta(
+                                    StateDelta::new(),
+                                    Some(&mut stack),
+                                    cwd,
+                                ) {
+                                    let working_set = StateWorkingSet::new(&engine_state);
+                                    report_error(&working_set, &e);
+                                }
+                            }
+                            Err(e) => {
+                                let working_set = StateWorkingSet::new(&engine_state);
+                                report_error(&working_set, &e);
+                            }
+                        }
                     }
                 }
             }