Set current working directory at startup (#12953)

This PR sets the current working directory to the location of the
Nushell executable at startup, using `std::env::set_current_dir()`. This
is desirable because after PR
https://github.com/nushell/nushell/pull/12922, we no longer change our
current working directory even after `cd` is executed, and some OS might
lock the directory where Nushell started.

The location of the Nushell executable is chosen because it cannot be
removed while Nushell is running anyways, so we don't have to worry
about OS locking it.

This PR has the side effect that it breaks buggy command even harder.
I'll keep this PR as a draft until these commands are fixed, but it
might be helpful to pull this PR if you're working on fixing one of
those bugs.

---------

Co-authored-by: Devyn Cairns <devyn.cairns@gmail.com>
Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
This commit is contained in:
YizhePKU 2024-09-26 02:04:26 +08:00 committed by GitHub
parent 54e9aa92bc
commit 13df0af514
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 91 additions and 127 deletions

View File

@ -228,15 +228,8 @@ pub fn eval_config_contents(
engine_state.file = prev_file;
// Merge the environment in case env vars changed in the config
match engine_state.cwd(Some(stack)) {
Ok(cwd) => {
if let Err(e) = engine_state.merge_env(stack, cwd) {
report_shell_error(engine_state, &e);
}
}
Err(e) => {
report_shell_error(engine_state, &e);
}
if let Err(e) = engine_state.merge_env(stack) {
report_shell_error(engine_state, &e);
}
}
}

View File

@ -21,7 +21,6 @@ pub use config_files::eval_config_contents;
pub use eval_cmds::{evaluate_commands, EvaluateCommandsOpts};
pub use eval_file::evaluate_file;
pub use menus::NuHelpCompleter;
pub use nu_cmd_base::util::get_init_cwd;
pub use nu_highlight::NuHighlight;
pub use print::Print;
pub use prompt::NushellPrompt;

View File

@ -16,10 +16,7 @@ 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, get_guaranteed_cwd},
};
use nu_cmd_base::{hook::eval_hook, util::get_editor};
use nu_color_config::StyleComputer;
#[allow(deprecated)]
use nu_engine::{convert_env_values, current_dir_str, env_to_strings};
@ -112,8 +109,7 @@ pub fn evaluate_repl(
PipelineData::empty(),
false,
);
let cwd = get_guaranteed_cwd(engine_state, &unique_stack);
engine_state.merge_env(&mut unique_stack, cwd)?;
engine_state.merge_env(&mut unique_stack)?;
}
let hostname = System::host_name();
@ -280,12 +276,10 @@ 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, cwd) {
if let Err(err) = engine_state.merge_env(&mut stack) {
report_shell_error(engine_state, &err);
}
// Check whether $env.NU_DISABLE_IR is set, so that the user can change it in the REPL

View File

@ -18,11 +18,11 @@ use support::{
#[fixture]
fn completer() -> NuCompleter {
// Create a new engine
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, 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, dir).is_ok());
assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok());
// Instantiate a new completer
NuCompleter::new(Arc::new(engine), Arc::new(stack))
@ -31,11 +31,12 @@ fn completer() -> NuCompleter {
#[fixture]
fn completer_strings() -> NuCompleter {
// Create a new engine
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, 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, dir).is_ok());
assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok());
// Instantiate a new completer
NuCompleter::new(Arc::new(engine), Arc::new(stack))
@ -44,7 +45,7 @@ fn completer_strings() -> NuCompleter {
#[fixture]
fn extern_completer() -> NuCompleter {
// Create a new engine
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, mut engine, mut stack) = new_engine();
// Add record value as example
let record = r#"
@ -55,7 +56,7 @@ fn extern_completer() -> NuCompleter {
-b: string@animals
]
"#;
assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack, dir).is_ok());
assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok());
// Instantiate a new completer
NuCompleter::new(Arc::new(engine), Arc::new(stack))
@ -64,7 +65,7 @@ fn extern_completer() -> NuCompleter {
#[fixture]
fn completer_strings_with_options() -> NuCompleter {
// Create a new engine
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, mut engine, mut stack) = new_engine();
// Add record value as example
let record = r#"
# To test that the config setting has no effect on the custom completions
@ -81,7 +82,7 @@ fn completer_strings_with_options() -> NuCompleter {
}
}
def my-command [animal: string@animals] { print $animal }"#;
assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack, dir).is_ok());
assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok());
// Instantiate a new completer
NuCompleter::new(Arc::new(engine), Arc::new(stack))
@ -90,7 +91,7 @@ fn completer_strings_with_options() -> NuCompleter {
#[fixture]
fn custom_completer() -> NuCompleter {
// Create a new engine
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, mut engine, mut stack) = new_engine();
// Add record value as example
let record = r#"
@ -104,7 +105,7 @@ fn custom_completer() -> NuCompleter {
completer: $external_completer
}
"#;
assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack, dir).is_ok());
assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok());
// Instantiate a new completer
NuCompleter::new(Arc::new(engine), Arc::new(stack))
@ -113,7 +114,7 @@ fn custom_completer() -> NuCompleter {
#[fixture]
fn subcommand_completer() -> NuCompleter {
// Create a new engine
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, mut engine, mut stack) = new_engine();
let commands = r#"
$env.config.completions.algorithm = "fuzzy"
@ -123,7 +124,7 @@ fn subcommand_completer() -> NuCompleter {
def "foo aabcrr" [] {}
def food [] {}
"#;
assert!(support::merge_input(commands.as_bytes(), &mut engine, &mut stack, dir).is_ok());
assert!(support::merge_input(commands.as_bytes(), &mut engine, &mut stack).is_ok());
// Instantiate a new completer
NuCompleter::new(Arc::new(engine), Arc::new(stack))
@ -133,13 +134,13 @@ fn subcommand_completer() -> NuCompleter {
#[fixture]
fn fuzzy_alpha_sort_completer() -> NuCompleter {
// Create a new engine
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, mut engine, mut stack) = new_engine();
let config = r#"
$env.config.completions.algorithm = "fuzzy"
$env.config.completions.sort = "alphabetical"
"#;
assert!(support::merge_input(config.as_bytes(), &mut engine, &mut stack, dir).is_ok());
assert!(support::merge_input(config.as_bytes(), &mut engine, &mut stack).is_ok());
// Instantiate a new completer
NuCompleter::new(Arc::new(engine), Arc::new(stack))
@ -1196,11 +1197,11 @@ fn folder_with_directorycompletions_do_not_collapse_dots() {
#[test]
fn variables_completions() {
// Create a new engine
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, 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, dir).is_ok());
assert!(support::merge_input(record.as_bytes(), &mut engine, &mut stack).is_ok());
// Instantiate a new completer
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
@ -1311,11 +1312,11 @@ fn variables_completions() {
#[test]
fn alias_of_command_and_flags() {
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, 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, dir).is_ok());
assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok());
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
@ -1330,11 +1331,11 @@ fn alias_of_command_and_flags() {
#[test]
fn alias_of_basic_command() {
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, 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, dir).is_ok());
assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok());
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
@ -1349,14 +1350,14 @@ fn alias_of_basic_command() {
#[test]
fn alias_of_another_alias() {
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, 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, dir.clone()).is_ok());
assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok());
// Create the second alias
let alias = r#"alias lf = ll -f"#;
assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack, dir).is_ok());
assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok());
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
@ -1373,7 +1374,7 @@ fn run_external_completion(completer: &str, input: &str) -> Vec<Suggestion> {
let completer = format!("$env.config.completions.external.completer = {completer}");
// Create a new engine
let (dir, _, mut engine_state, mut stack) = new_engine();
let (_, _, 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);
@ -1389,7 +1390,7 @@ fn run_external_completion(completer: &str, input: &str) -> Vec<Suggestion> {
);
// Merge environment into the permanent state
assert!(engine_state.merge_env(&mut stack, &dir).is_ok());
assert!(engine_state.merge_env(&mut stack).is_ok());
// Instantiate a new completer
let mut completer = NuCompleter::new(Arc::new(engine_state), Arc::new(stack));
@ -1578,11 +1579,11 @@ fn sort_fuzzy_completions_in_alphabetical_order(mut fuzzy_alpha_sort_completer:
#[ignore = "was reverted, still needs fixing"]
#[rstest]
fn alias_offset_bug_7648() {
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, 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, dir).is_ok());
assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok());
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
@ -1597,11 +1598,11 @@ fn alias_offset_bug_7648() {
#[ignore = "was reverted, still needs fixing"]
#[rstest]
fn alias_offset_bug_7754() {
let (dir, _, mut engine, mut stack) = new_engine();
let (_, _, 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, dir).is_ok());
assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok());
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));

View File

@ -63,7 +63,7 @@ pub fn new_engine() -> (AbsolutePathBuf, String, EngineState, Stack) {
);
// Merge environment into the permanent state
let merge_result = engine_state.merge_env(&mut stack, &dir);
let merge_result = engine_state.merge_env(&mut stack);
assert!(merge_result.is_ok());
(dir, dir_str, engine_state, stack)
@ -109,7 +109,7 @@ pub fn new_dotnu_engine() -> (AbsolutePathBuf, String, EngineState, Stack) {
);
// Merge environment into the permanent state
let merge_result = engine_state.merge_env(&mut stack, &dir);
let merge_result = engine_state.merge_env(&mut stack);
assert!(merge_result.is_ok());
(dir, dir_str, engine_state, stack)
@ -144,7 +144,7 @@ pub fn new_quote_engine() -> (AbsolutePathBuf, String, EngineState, Stack) {
);
// Merge environment into the permanent state
let merge_result = engine_state.merge_env(&mut stack, &dir);
let merge_result = engine_state.merge_env(&mut stack);
assert!(merge_result.is_ok());
(dir, dir_str, engine_state, stack)
@ -179,7 +179,7 @@ pub fn new_partial_engine() -> (AbsolutePathBuf, String, EngineState, Stack) {
);
// Merge environment into the permanent state
let merge_result = engine_state.merge_env(&mut stack, &dir);
let merge_result = engine_state.merge_env(&mut stack);
assert!(merge_result.is_ok());
(dir, dir_str, engine_state, stack)
@ -223,7 +223,6 @@ pub fn merge_input(
input: &[u8],
engine_state: &mut EngineState,
stack: &mut Stack,
dir: AbsolutePathBuf,
) -> Result<(), ShellError> {
let (block, delta) = {
let mut working_set = StateWorkingSet::new(engine_state);
@ -246,5 +245,5 @@ pub fn merge_input(
.is_ok());
// Merge environment into the permanent state
engine_state.merge_env(stack, &dir)
engine_state.merge_env(stack)
}

View File

@ -1,4 +1,3 @@
use crate::util::get_guaranteed_cwd;
use miette::Result;
use nu_engine::{eval_block, eval_block_with_early_return};
use nu_parser::parse;
@ -284,8 +283,7 @@ pub fn eval_hook(
}
}
let cwd = get_guaranteed_cwd(engine_state, stack);
engine_state.merge_env(stack, cwd)?;
engine_state.merge_env(stack)?;
Ok(output)
}

View File

@ -1,30 +1,9 @@
use nu_path::AbsolutePathBuf;
use nu_protocol::{
engine::{EngineState, Stack},
Range, ShellError, Span, Value,
};
use std::ops::Bound;
pub fn get_init_cwd() -> AbsolutePathBuf {
std::env::current_dir()
.ok()
.and_then(|path| AbsolutePathBuf::try_from(path).ok())
.or_else(|| {
std::env::var("PWD")
.ok()
.and_then(|path| AbsolutePathBuf::try_from(path).ok())
})
.or_else(nu_path::home_dir)
.expect("Failed to get current working directory")
}
pub fn get_guaranteed_cwd(engine_state: &EngineState, stack: &Stack) -> AbsolutePathBuf {
engine_state
.cwd(Some(stack))
.ok()
.unwrap_or_else(get_init_cwd)
}
type MakeRangeError = fn(&str, Span) -> ShellError;
/// Returns a inclusive pair of boundary in given `range`.

View File

@ -149,7 +149,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, cwd)
.merge_env(&mut stack)
.expect("Error merging environment");
let empty_input = PipelineData::empty();

View File

@ -1,4 +1,3 @@
use nu_cmd_base::util::get_init_cwd;
use nu_engine::command_prelude::*;
use nu_utils::filesystem::{have_permission, PermissionResult};
@ -41,12 +40,14 @@ impl Command for Cd {
let physical = call.has_flag(engine_state, stack, "physical")?;
let path_val: Option<Spanned<String>> = call.opt(engine_state, stack, 0)?;
// If getting PWD failed, default to the initial directory. This way, the
// user can use `cd` to recover PWD to a good state.
// If getting PWD failed, default to the home directory. The user can
// use `cd` to reset PWD to a good state.
let cwd = engine_state
.cwd(Some(stack))
.ok()
.unwrap_or_else(get_init_cwd);
.or_else(nu_path::home_dir)
.map(|path| path.into_std_path_buf())
.unwrap_or_default();
let path_val = {
if let Some(path) = path_val {
@ -65,7 +66,7 @@ impl Command for Cd {
if let Some(oldpwd) = stack.get_env_var(engine_state, "OLDPWD") {
oldpwd.to_path()?
} else {
cwd.into()
cwd
}
} else {
// Trim whitespace from the end of path.

View File

@ -58,9 +58,8 @@ impl Command for Start {
open_path(url.as_str(), engine_state, stack, path.span)?;
} else {
// try to distinguish between file not found and opening url without prefix
if let Ok(canon_path) =
canonicalize_with(path_no_whitespace, std::env::current_dir()?.as_path())
{
let cwd = engine_state.cwd(Some(stack))?;
if let Ok(canon_path) = canonicalize_with(path_no_whitespace, cwd) {
open_path(canon_path, engine_state, stack, path.span)?;
} else {
// open crate does not allow opening URL without prefix

View File

@ -17,7 +17,7 @@ use nu_path::AbsolutePathBuf;
use std::{
collections::HashMap,
num::NonZeroUsize,
path::{Path, PathBuf},
path::PathBuf,
sync::{
atomic::{AtomicBool, AtomicU32, Ordering},
Arc, Mutex, MutexGuard, PoisonError,
@ -307,11 +307,7 @@ impl EngineState {
}
/// Merge the environment from the runtime Stack into the engine state
pub fn merge_env(
&mut self,
stack: &mut Stack,
cwd: impl AsRef<Path>,
) -> Result<(), ShellError> {
pub fn merge_env(&mut self, stack: &mut Stack) -> Result<(), ShellError> {
for mut scope in stack.env_vars.drain(..) {
for (overlay_name, mut env) in Arc::make_mut(&mut scope).drain() {
if let Some(env_vars) = Arc::make_mut(&mut self.env_vars).get_mut(&overlay_name) {
@ -324,9 +320,6 @@ impl EngineState {
}
}
// TODO: better error
std::env::set_current_dir(cwd)?;
if let Some(config) = stack.config.take() {
// If config was updated in the stack, replace it.
self.config = config;

View File

@ -99,8 +99,7 @@ use std pwd
eval_block::<WithoutDebug>(engine_state, &mut stack, &block, pipeline_data)?;
let cwd = engine_state.cwd(Some(&stack))?;
engine_state.merge_env(&mut stack, cwd)?;
engine_state.merge_env(&mut stack)?;
Ok(())
}

View File

@ -163,15 +163,8 @@ pub(crate) fn read_default_env_file(engine_state: &mut EngineState, stack: &mut
);
// Merge the environment in case env vars changed in the config
match engine_state.cwd(Some(stack)) {
Ok(cwd) => {
if let Err(e) = engine_state.merge_env(stack, cwd) {
report_shell_error(engine_state, &e);
}
}
Err(e) => {
report_shell_error(engine_state, &e);
}
if let Err(e) = engine_state.merge_env(stack) {
report_shell_error(engine_state, &e);
}
}
@ -249,15 +242,8 @@ fn eval_default_config(
);
// Merge the environment in case env vars changed in the config
match engine_state.cwd(Some(stack)) {
Ok(cwd) => {
if let Err(e) = engine_state.merge_env(stack, cwd) {
report_shell_error(engine_state, &e);
}
}
Err(e) => {
report_shell_error(engine_state, &e);
}
if let Err(e) = engine_state.merge_env(stack) {
report_shell_error(engine_state, &e);
}
}

View File

@ -21,7 +21,6 @@ use command::gather_commandline_args;
use log::{trace, Level};
use miette::Result;
use nu_cli::gather_parent_env_vars;
use nu_cmd_base::util::get_init_cwd;
use nu_lsp::LanguageServer;
use nu_path::canonicalize_with;
use nu_protocol::{
@ -44,6 +43,27 @@ fn get_engine_state() -> EngineState {
nu_explore::add_explore_context(engine_state)
}
/// Get the directory where the Nushell executable is located.
fn current_exe_directory() -> PathBuf {
let mut path = std::env::current_exe().expect("current_exe() should succeed");
path.pop();
path
}
/// Get the current working directory from the environment.
fn current_dir_from_environment() -> PathBuf {
if let Ok(cwd) = std::env::current_dir() {
return cwd;
}
if let Ok(cwd) = std::env::var("PWD") {
return cwd.into();
}
if let Some(home) = nu_path::home_dir() {
return home.into_std_path_buf();
}
current_exe_directory()
}
fn main() -> Result<()> {
let entire_start_time = std::time::Instant::now();
let mut start_time = std::time::Instant::now();
@ -54,10 +74,11 @@ fn main() -> Result<()> {
miette_hook(x);
}));
// Get initial current working directory.
let init_cwd = get_init_cwd();
let mut engine_state = get_engine_state();
// Get the current working directory from the environment.
let init_cwd = current_dir_from_environment();
// Custom additions
let delta = {
let mut working_set = nu_protocol::engine::StateWorkingSet::new(&engine_state);
@ -319,6 +340,12 @@ fn main() -> Result<()> {
_ => std::process::exit(1),
}
std::process::exit(0)
} else {
// If we're not running a testbin, set the current working directory to
// the location of the Nushell executable. This prevents the OS from
// locking the directory where the user launched Nushell.
std::env::set_current_dir(current_exe_directory())
.expect("set_current_dir() should succeed");
}
perf!("run test_bins", start_time, use_color);

View File

@ -256,13 +256,9 @@ 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, &cwd) {
if let Err(err) = engine_state.merge_env(&mut stack) {
outcome_err(&engine_state, &err);
}

View File

@ -131,9 +131,9 @@ fn command_not_found_error_suggests_typo_fix() {
#[test]
fn command_not_found_error_recognizes_non_executable_file() {
let actual = nu!("./Cargo.toml");
assert!(actual.err.contains(
"refers to a file that is not executable. Did you forget to to set execute permissions?"
));
assert!(actual
.err
.contains("is neither a Nushell built-in or a known external command"));
}
#[test]