From 7d11c28eea72b940a1babc11fc17072b532f9a03 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Fri, 24 May 2024 11:09:59 -0500 Subject: [PATCH] Revert "Remove `std::env::set_current_dir()` call from `EngineState::merge_env()`" (#12954) Reverts nushell/nushell#12922 --- crates/nu-cli/src/config_files.rs | 11 ++++- crates/nu-cli/src/repl.rs | 12 +++-- crates/nu-cli/tests/completions/mod.rs | 46 +++++++++---------- .../support/completions_helpers.rs | 9 ++-- crates/nu-cmd-base/src/hook.rs | 4 +- crates/nu-cmd-lang/src/example_support.rs | 2 +- crates/nu-protocol/src/engine/engine_state.rs | 9 +++- crates/nu-std/src/lib.rs | 3 +- src/config_files.rs | 22 +++++++-- src/test_bins.rs | 6 ++- 10 files changed, 83 insertions(+), 41 deletions(-) diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 0684623226..ec7ad2f412 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -235,8 +235,15 @@ pub fn eval_config_contents( engine_state.file = prev_file; // Merge the environment in case env vars changed in the config - if let Err(e) = engine_state.merge_env(stack) { - report_error_new(engine_state, &e); + match engine_state.cwd(Some(stack)) { + Ok(cwd) => { + if let Err(e) = engine_state.merge_env(stack, cwd) { + report_error_new(engine_state, &e); + } + } + Err(e) => { + report_error_new(engine_state, &e); + } } } } diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index e79f41eb21..29c2f62734 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -15,7 +15,10 @@ use crate::{ use crossterm::cursor::SetCursorStyle; use log::{error, trace, warn}; use miette::{ErrReport, IntoDiagnostic, Result}; -use nu_cmd_base::{hook::eval_hook, util::get_editor}; +use nu_cmd_base::{ + hook::eval_hook, + util::{get_editor, get_guaranteed_cwd}, +}; use nu_color_config::StyleComputer; #[allow(deprecated)] use nu_engine::{convert_env_values, current_dir_str, env_to_strings}; @@ -115,7 +118,8 @@ pub fn evaluate_repl( PipelineData::empty(), false, ); - engine_state.merge_env(&mut unique_stack)?; + let cwd = get_guaranteed_cwd(engine_state, &unique_stack); + engine_state.merge_env(&mut unique_stack, cwd)?; } let hostname = System::host_name(); @@ -277,10 +281,12 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { hostname, } = ctx; + let cwd = get_guaranteed_cwd(engine_state, &stack); + let mut start_time = std::time::Instant::now(); // Before doing anything, merge the environment from the previous REPL iteration into the // permanent state. - if let Err(err) = engine_state.merge_env(&mut stack) { + if let Err(err) = engine_state.merge_env(&mut stack, cwd) { report_error_new(engine_state, &err); } perf( diff --git a/crates/nu-cli/tests/completions/mod.rs b/crates/nu-cli/tests/completions/mod.rs index eb245e2fb2..a5b0b13aa8 100644 --- a/crates/nu-cli/tests/completions/mod.rs +++ b/crates/nu-cli/tests/completions/mod.rs @@ -18,11 +18,11 @@ use support::{ #[fixture] fn completer() -> NuCompleter { // Create a new engine - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Add record value as example let record = "def tst [--mod -s] {}"; - assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack, dir).is_ok()); // Instantiate a new completer NuCompleter::new(Arc::new(engine), Arc::new(stack)) @@ -31,12 +31,12 @@ fn completer() -> NuCompleter { #[fixture] fn completer_strings() -> NuCompleter { // Create a new engine - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Add record value as example let record = r#"def animals [] { ["cat", "dog", "eel" ] } def my-command [animal: string@animals] { print $animal }"#; - assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack, dir).is_ok()); // Instantiate a new completer NuCompleter::new(Arc::new(engine), Arc::new(stack)) @@ -45,7 +45,7 @@ fn completer_strings() -> NuCompleter { #[fixture] fn extern_completer() -> NuCompleter { // Create a new engine - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Add record value as example let record = r#" @@ -56,7 +56,7 @@ fn extern_completer() -> NuCompleter { -b: string@animals ] "#; - assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack, dir).is_ok()); // Instantiate a new completer NuCompleter::new(Arc::new(engine), Arc::new(stack)) @@ -65,7 +65,7 @@ fn extern_completer() -> NuCompleter { #[fixture] fn custom_completer() -> NuCompleter { // Create a new engine - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Add record value as example let record = r#" @@ -79,7 +79,7 @@ fn custom_completer() -> NuCompleter { completer: $external_completer } "#; - assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack, dir).is_ok()); // Instantiate a new completer NuCompleter::new(Arc::new(engine), Arc::new(stack)) @@ -751,11 +751,11 @@ fn folder_with_directorycompletions() { #[test] fn variables_completions() { // Create a new engine - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Add record value as example let record = "let actor = { name: 'Tom Hardy', age: 44 }"; - assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack, dir).is_ok()); // Instantiate a new completer let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack)); @@ -858,11 +858,11 @@ fn variables_completions() { #[test] fn alias_of_command_and_flags() { - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Create an alias let alias = r#"alias ll = ls -l"#; - assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack, dir).is_ok()); let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack)); @@ -877,11 +877,11 @@ fn alias_of_command_and_flags() { #[test] fn alias_of_basic_command() { - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Create an alias let alias = r#"alias ll = ls "#; - assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack, dir).is_ok()); let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack)); @@ -896,14 +896,14 @@ fn alias_of_basic_command() { #[test] fn alias_of_another_alias() { - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Create an alias let alias = r#"alias ll = ls -la"#; - assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack, dir.clone()).is_ok()); // Create the second alias let alias = r#"alias lf = ll -f"#; - assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack, dir).is_ok()); let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack)); @@ -920,7 +920,7 @@ fn run_external_completion(completer: &str, input: &str) -> Vec { let completer = format!("$env.config.completions.external.completer = {completer}"); // Create a new engine - let (_, _, mut engine_state, mut stack) = new_engine(); + let (dir, _, mut engine_state, mut stack) = new_engine(); let (block, delta) = { let mut working_set = StateWorkingSet::new(&engine_state); let block = parse(&mut working_set, None, completer.as_bytes(), false); @@ -936,7 +936,7 @@ fn run_external_completion(completer: &str, input: &str) -> Vec { ); // Merge environment into the permanent state - assert!(engine_state.merge_env(&mut stack).is_ok()); + assert!(engine_state.merge_env(&mut stack, &dir).is_ok()); // Instantiate a new completer let mut completer = NuCompleter::new(Arc::new(engine_state), Arc::new(stack)); @@ -1114,11 +1114,11 @@ fn custom_completer_triggers_cursor_after_word(mut custom_completer: NuCompleter #[ignore = "was reverted, still needs fixing"] #[rstest] fn alias_offset_bug_7648() { - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Create an alias let alias = r#"alias ea = ^$env.EDITOR /tmp/test.s"#; - assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack, dir).is_ok()); let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack)); @@ -1133,11 +1133,11 @@ fn alias_offset_bug_7648() { #[ignore = "was reverted, still needs fixing"] #[rstest] fn alias_offset_bug_7754() { - let (_, _, mut engine, mut stack) = new_engine(); + let (dir, _, mut engine, mut stack) = new_engine(); // Create an alias let alias = r#"alias ll = ls -l"#; - assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok()); + assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack, dir).is_ok()); let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack)); diff --git a/crates/nu-cli/tests/completions/support/completions_helpers.rs b/crates/nu-cli/tests/completions/support/completions_helpers.rs index df92979e23..47f46ab00e 100644 --- a/crates/nu-cli/tests/completions/support/completions_helpers.rs +++ b/crates/nu-cli/tests/completions/support/completions_helpers.rs @@ -62,7 +62,7 @@ pub fn new_engine() -> (PathBuf, String, EngineState, Stack) { ); // Merge environment into the permanent state - let merge_result = engine_state.merge_env(&mut stack); + let merge_result = engine_state.merge_env(&mut stack, &dir); assert!(merge_result.is_ok()); (dir, dir_str, engine_state, stack) @@ -97,7 +97,7 @@ pub fn new_quote_engine() -> (PathBuf, String, EngineState, Stack) { ); // Merge environment into the permanent state - let merge_result = engine_state.merge_env(&mut stack); + let merge_result = engine_state.merge_env(&mut stack, &dir); assert!(merge_result.is_ok()); (dir, dir_str, engine_state, stack) @@ -132,7 +132,7 @@ pub fn new_partial_engine() -> (PathBuf, String, EngineState, Stack) { ); // Merge environment into the permanent state - let merge_result = engine_state.merge_env(&mut stack); + let merge_result = engine_state.merge_env(&mut stack, &dir); assert!(merge_result.is_ok()); (dir, dir_str, engine_state, stack) @@ -173,6 +173,7 @@ pub fn merge_input( input: &[u8], engine_state: &mut EngineState, stack: &mut Stack, + dir: PathBuf, ) -> Result<(), ShellError> { let (block, delta) = { let mut working_set = StateWorkingSet::new(engine_state); @@ -195,5 +196,5 @@ pub fn merge_input( .is_ok()); // Merge environment into the permanent state - engine_state.merge_env(stack) + engine_state.merge_env(stack, &dir) } diff --git a/crates/nu-cmd-base/src/hook.rs b/crates/nu-cmd-base/src/hook.rs index 9133cf4167..76c13bd5c3 100644 --- a/crates/nu-cmd-base/src/hook.rs +++ b/crates/nu-cmd-base/src/hook.rs @@ -1,3 +1,4 @@ +use crate::util::get_guaranteed_cwd; use miette::Result; use nu_engine::{eval_block, eval_block_with_early_return}; use nu_parser::parse; @@ -283,7 +284,8 @@ pub fn eval_hook( } } - engine_state.merge_env(stack)?; + let cwd = get_guaranteed_cwd(engine_state, stack); + engine_state.merge_env(stack, cwd)?; Ok(output) } diff --git a/crates/nu-cmd-lang/src/example_support.rs b/crates/nu-cmd-lang/src/example_support.rs index d938729fef..bb03bbaf8c 100644 --- a/crates/nu-cmd-lang/src/example_support.rs +++ b/crates/nu-cmd-lang/src/example_support.rs @@ -138,7 +138,7 @@ pub fn check_example_evaluates_to_expected_output( stack.add_env_var("PWD".to_string(), Value::test_string(cwd.to_string_lossy())); engine_state - .merge_env(&mut stack) + .merge_env(&mut stack, cwd) .expect("Error merging environment"); let empty_input = PipelineData::empty(); diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 10c96b7463..710ca77d4c 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -277,7 +277,11 @@ impl EngineState { } /// Merge the environment from the runtime Stack into the engine state - pub fn merge_env(&mut self, stack: &mut Stack) -> Result<(), ShellError> { + pub fn merge_env( + &mut self, + stack: &mut Stack, + cwd: impl AsRef, + ) -> Result<(), ShellError> { let mut config_updated = false; for mut scope in stack.env_vars.drain(..) { @@ -307,6 +311,9 @@ impl EngineState { } } + // TODO: better error + std::env::set_current_dir(cwd)?; + if config_updated { // Make plugin GC config changes take effect immediately. #[cfg(feature = "plugin")] diff --git a/crates/nu-std/src/lib.rs b/crates/nu-std/src/lib.rs index e6c8f30556..a20e3fc5da 100644 --- a/crates/nu-std/src/lib.rs +++ b/crates/nu-std/src/lib.rs @@ -98,7 +98,8 @@ use std pwd eval_block::(engine_state, &mut stack, &block, pipeline_data)?; - engine_state.merge_env(&mut stack)?; + let cwd = engine_state.cwd(Some(&stack))?; + engine_state.merge_env(&mut stack, cwd)?; Ok(()) } diff --git a/src/config_files.rs b/src/config_files.rs index 3df076045f..ec4511860f 100644 --- a/src/config_files.rs +++ b/src/config_files.rs @@ -164,8 +164,15 @@ pub(crate) fn read_default_env_file(engine_state: &mut EngineState, stack: &mut ); // Merge the environment in case env vars changed in the config - if let Err(e) = engine_state.merge_env(stack) { - report_error_new(engine_state, &e); + match engine_state.cwd(Some(stack)) { + Ok(cwd) => { + if let Err(e) = engine_state.merge_env(stack, cwd) { + report_error_new(engine_state, &e); + } + } + Err(e) => { + report_error_new(engine_state, &e); + } } } @@ -195,8 +202,15 @@ fn eval_default_config( ); // Merge the environment in case env vars changed in the config - if let Err(e) = engine_state.merge_env(stack) { - report_error_new(engine_state, &e); + match engine_state.cwd(Some(stack)) { + Ok(cwd) => { + if let Err(e) = engine_state.merge_env(stack, cwd) { + report_error_new(engine_state, &e); + } + } + Err(e) => { + report_error_new(engine_state, &e); + } } } diff --git a/src/test_bins.rs b/src/test_bins.rs index 7cd542e732..b20c01f997 100644 --- a/src/test_bins.rs +++ b/src/test_bins.rs @@ -251,9 +251,13 @@ pub fn nu_repl() { for (i, line) in source_lines.iter().enumerate() { let mut stack = Stack::with_parent(top_stack.clone()); + let cwd = engine_state + .cwd(Some(&stack)) + .unwrap_or_else(|err| outcome_err(&engine_state, &err)); + // Before doing anything, merge the environment from the previous REPL iteration into the // permanent state. - if let Err(err) = engine_state.merge_env(&mut stack) { + if let Err(err) = engine_state.merge_env(&mut stack, &cwd) { outcome_err(&engine_state, &err); }