From bdb6daa4b5046629a88739fb0a2742d5729c04d8 Mon Sep 17 00:00:00 2001 From: YizhePKU Date: Fri, 3 May 2024 19:33:09 +0800 Subject: [PATCH] Migrate to a new PWD API (#12603) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the first PR towards migrating to a new `$env.PWD` API that returns potentially un-canonicalized paths. Refer to PR #12515 for motivations. ## New API: `EngineState::cwd()` The goal of the new API is to cover both parse-time and runtime use case, and avoid unintentional misuse. It takes an `Option` as argument, which if supplied, will search for `$env.PWD` on the stack in additional to the engine state. I think with this design, there's less confusion over parse-time and runtime environments. If you have access to a stack, just supply it; otherwise supply `None`. ## Deprecation of other PWD-related APIs Other APIs are re-implemented using `EngineState::cwd()` and properly documented. They're marked deprecated, but their behavior is unchanged. Unused APIs are deleted, and code that accesses `$env.PWD` directly without using an API is rewritten. Deprecated APIs: * `EngineState::current_work_dir()` * `StateWorkingSet::get_cwd()` * `env::current_dir()` * `env::current_dir_str()` * `env::current_dir_const()` * `env::current_dir_str_const()` Other changes: * `EngineState::get_cwd()` (deleted) * `StateWorkingSet::list_env()` (deleted) * `repl::do_run_cmd()` (rewritten with `env::current_dir_str()`) ## `cd` and `pwd` now use logical paths by default This pulls the changes from PR #12515. It's currently somewhat broken because using non-canonicalized paths exposed a bug in our path normalization logic (Issue #12602). Once that is fixed, this should work. ## Future plans This PR needs some tests. Which test helpers should I use, and where should I put those tests? I noticed that unquoted paths are expanded within `eval_filepath()` and `eval_directory()` before they even reach the `cd` command. This means every paths is expanded twice. Is this intended? Once this PR lands, the plan is to review all usages of the deprecated APIs and migrate them to `EngineState::cwd()`. In the meantime, these usages are annotated with `#[allow(deprecated)]` to avoid breaking CI. --------- Co-authored-by: Jakub Žádník --- Cargo.lock | 1 + benches/benchmarks.rs | 1 + .../src/completions/directory_completions.rs | 1 + .../src/completions/dotnu_completions.rs | 1 + .../src/completions/file_completions.rs | 1 + crates/nu-cli/src/config_files.rs | 3 + crates/nu-cli/src/eval_file.rs | 2 + crates/nu-cli/src/repl.rs | 219 ++++++-------- crates/nu-cli/src/syntax_highlight.rs | 1 + crates/nu-cli/tests/completions.rs | 6 +- .../tests/support/completions_helpers.rs | 15 +- crates/nu-cmd-base/src/util.rs | 1 + .../nu-cmd-plugin/src/commands/plugin/add.rs | 5 +- crates/nu-cmd-plugin/src/util.rs | 8 +- crates/nu-command/src/filesystem/cd.rs | 67 +++-- crates/nu-command/src/filesystem/du.rs | 2 + crates/nu-command/src/filesystem/glob.rs | 2 + crates/nu-command/src/filesystem/ls.rs | 2 + crates/nu-command/src/filesystem/mktemp.rs | 2 + crates/nu-command/src/filesystem/open.rs | 2 + crates/nu-command/src/filesystem/rm.rs | 2 + crates/nu-command/src/filesystem/save.rs | 2 + crates/nu-command/src/filesystem/touch.rs | 2 + crates/nu-command/src/filesystem/ucp.rs | 2 + crates/nu-command/src/filesystem/umkdir.rs | 2 + crates/nu-command/src/filesystem/umv.rs | 2 + crates/nu-command/src/filesystem/watch.rs | 2 + crates/nu-command/src/path/exists.rs | 3 + crates/nu-command/src/path/expand.rs | 3 + crates/nu-command/src/system/exec.rs | 2 + crates/nu-command/src/system/which_.rs | 1 + crates/nu-command/tests/commands/cd.rs | 8 + crates/nu-engine/src/env.rs | 112 +++---- crates/nu-engine/src/eval.rs | 8 +- crates/nu-parser/src/parse_keywords.rs | 4 + crates/nu-plugin-engine/src/context.rs | 1 + crates/nu-protocol/Cargo.toml | 1 + crates/nu-protocol/src/engine/engine_state.rs | 283 +++++++++++++++++- .../src/engine/state_working_set.rs | 27 +- crates/nu-protocol/src/eval_const.rs | 1 + crates/nu-std/src/lib.rs | 2 + crates/nu-std/std/mod.nu | 10 +- src/config_files.rs | 3 + src/test_bins.rs | 1 + 44 files changed, 551 insertions(+), 275 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0e7c9f9203..7fcf93f136 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3227,6 +3227,7 @@ dependencies = [ "serde_json", "strum", "strum_macros 0.26.2", + "tempfile", "thiserror", "typetag", ] diff --git a/benches/benchmarks.rs b/benches/benchmarks.rs index 84c0e4b02e..e6bec2e163 100644 --- a/benches/benchmarks.rs +++ b/benches/benchmarks.rs @@ -21,6 +21,7 @@ fn load_bench_commands() -> EngineState { } fn canonicalize_path(engine_state: &EngineState, path: &Path) -> PathBuf { + #[allow(deprecated)] let cwd = engine_state.current_work_dir(); if path.exists() { diff --git a/crates/nu-cli/src/completions/directory_completions.rs b/crates/nu-cli/src/completions/directory_completions.rs index f8ab2f400e..e8d463c19f 100644 --- a/crates/nu-cli/src/completions/directory_completions.rs +++ b/crates/nu-cli/src/completions/directory_completions.rs @@ -43,6 +43,7 @@ impl Completer for DirectoryCompletion { let AdjustView { prefix, span, .. } = adjust_if_intermediate(&prefix, working_set, span); // Filter only the folders + #[allow(deprecated)] let output: Vec<_> = directory_completion( span, &prefix, diff --git a/crates/nu-cli/src/completions/dotnu_completions.rs b/crates/nu-cli/src/completions/dotnu_completions.rs index 3c8abd93ff..8927738491 100644 --- a/crates/nu-cli/src/completions/dotnu_completions.rs +++ b/crates/nu-cli/src/completions/dotnu_completions.rs @@ -84,6 +84,7 @@ impl Completer for DotNuCompletion { partial = base_dir_partial; } else { // Fetch the current folder + #[allow(deprecated)] let current_folder = self.engine_state.current_work_dir(); is_current_folder = true; diff --git a/crates/nu-cli/src/completions/file_completions.rs b/crates/nu-cli/src/completions/file_completions.rs index d862975f86..1a99c995db 100644 --- a/crates/nu-cli/src/completions/file_completions.rs +++ b/crates/nu-cli/src/completions/file_completions.rs @@ -47,6 +47,7 @@ impl Completer for FileCompletion { readjusted, } = adjust_if_intermediate(&prefix, working_set, span); + #[allow(deprecated)] let output: Vec<_> = complete_item( readjusted, span, diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 3876272893..775d76382b 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -177,6 +177,7 @@ pub fn add_plugin_file( use std::path::Path; let working_set = StateWorkingSet::new(engine_state); + #[allow(deprecated)] let cwd = working_set.get_cwd(); if let Some(plugin_file) = plugin_file { @@ -235,6 +236,7 @@ pub fn eval_config_contents( engine_state.file = prev_file; // Merge the environment in case env vars changed in the config + #[allow(deprecated)] match nu_engine::env::current_dir(engine_state, stack) { Ok(cwd) => { if let Err(e) = engine_state.merge_env(stack, cwd) { @@ -272,6 +274,7 @@ pub fn migrate_old_plugin_file(engine_state: &EngineState, storage_path: &str) - let start_time = std::time::Instant::now(); + #[allow(deprecated)] let cwd = engine_state.current_work_dir(); let Some(config_dir) = nu_path::config_dir().and_then(|mut dir| { diff --git a/crates/nu-cli/src/eval_file.rs b/crates/nu-cli/src/eval_file.rs index 85b07d629d..90b1e840ee 100644 --- a/crates/nu-cli/src/eval_file.rs +++ b/crates/nu-cli/src/eval_file.rs @@ -1,6 +1,7 @@ use crate::util::eval_source; use log::{info, trace}; use miette::{IntoDiagnostic, Result}; +#[allow(deprecated)] use nu_engine::{convert_env_values, current_dir, eval_block}; use nu_parser::parse; use nu_path::canonicalize_with; @@ -29,6 +30,7 @@ pub fn evaluate_file( std::process::exit(1); } + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let file_path = canonicalize_with(&path, cwd).unwrap_or_else(|e| { diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 0e5e24d46b..30cccebbde 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -20,7 +20,8 @@ use nu_cmd_base::{ util::{get_editor, get_guaranteed_cwd}, }; use nu_color_config::StyleComputer; -use nu_engine::{convert_env_values, env_to_strings}; +#[allow(deprecated)] +use nu_engine::{convert_env_values, current_dir_str, env_to_strings}; use nu_parser::{lex, parse, trim_quotes_str}; use nu_protocol::{ config::NuCursorShape, @@ -799,6 +800,7 @@ fn prepare_history_metadata( line_editor: &mut Reedline, ) { if !s.is_empty() && line_editor.has_last_command_context() { + #[allow(deprecated)] let result = line_editor .update_last_command_context(&|mut c| { c.start_timestamp = Some(chrono::Utc::now()); @@ -869,6 +871,7 @@ fn parse_operation( ) -> Result { let tokens = lex(s.as_bytes(), 0, &[], &[], false); // Check if this is a single call to a directory, if so auto-cd + #[allow(deprecated)] let cwd = nu_engine::env::current_dir_str(engine_state, stack)?; let mut orig = s.clone(); if orig.starts_with('`') { @@ -909,8 +912,6 @@ fn do_auto_cd( }, ); } - let path = nu_path::canonicalize_with(path, &cwd) - .expect("internal error: cannot canonicalize known path"); path.to_string_lossy().to_string() }; @@ -1027,50 +1028,44 @@ fn run_shell_integration_osc2( stack: &mut Stack, use_color: bool, ) { - if let Some(cwd) = stack.get_env_var(engine_state, "PWD") { - match cwd.coerce_into_string() { - Ok(path) => { - let start_time = Instant::now(); + #[allow(deprecated)] + if let Ok(path) = current_dir_str(engine_state, stack) { + let start_time = Instant::now(); - // Try to abbreviate string for windows title - let maybe_abbrev_path = if let Some(p) = nu_path::home_dir() { - path.replace(&p.as_path().display().to_string(), "~") + // Try to abbreviate string for windows title + let maybe_abbrev_path = if let Some(p) = nu_path::home_dir() { + path.replace(&p.as_path().display().to_string(), "~") + } else { + path + }; + + let title = match command_name { + Some(binary_name) => { + let split_binary_name = binary_name.split_whitespace().next(); + if let Some(binary_name) = split_binary_name { + format!("{maybe_abbrev_path}> {binary_name}") } else { - path - }; - - let title = match command_name { - Some(binary_name) => { - let split_binary_name = binary_name.split_whitespace().next(); - if let Some(binary_name) = split_binary_name { - format!("{maybe_abbrev_path}> {binary_name}") - } else { - maybe_abbrev_path.to_string() - } - } - None => maybe_abbrev_path.to_string(), - }; - - // Set window title too - // https://tldp.org/HOWTO/Xterm-Title-3.html - // ESC]0;stringBEL -- Set icon name and window title to string - // ESC]1;stringBEL -- Set icon name to string - // ESC]2;stringBEL -- Set window title to string - run_ansi_sequence(&format!("\x1b]2;{title}\x07")); - - perf( - "set title with command osc2", - start_time, - file!(), - line!(), - column!(), - use_color, - ); + maybe_abbrev_path.to_string() + } } - Err(e) => { - warn!("Could not coerce working directory to string {e}"); - } - } + None => maybe_abbrev_path.to_string(), + }; + + // Set window title too + // https://tldp.org/HOWTO/Xterm-Title-3.html + // ESC]0;stringBEL -- Set icon name and window title to string + // ESC]1;stringBEL -- Set icon name to string + // ESC]2;stringBEL -- Set window title to string + run_ansi_sequence(&format!("\x1b]2;{title}\x07")); + + perf( + "set title with command osc2", + start_time, + file!(), + line!(), + column!(), + use_color, + ); } } @@ -1080,98 +1075,78 @@ fn run_shell_integration_osc7( stack: &mut Stack, use_color: bool, ) { - if let Some(cwd) = stack.get_env_var(engine_state, "PWD") { - match cwd.coerce_into_string() { - Ok(path) => { - let start_time = Instant::now(); + #[allow(deprecated)] + if let Ok(path) = current_dir_str(engine_state, stack) { + let start_time = Instant::now(); - // Otherwise, communicate the path as OSC 7 (often used for spawning new tabs in the same dir) - run_ansi_sequence(&format!( - "\x1b]7;file://{}{}{}\x1b\\", - percent_encoding::utf8_percent_encode( - hostname.unwrap_or("localhost"), - percent_encoding::CONTROLS - ), - if path.starts_with('/') { "" } else { "/" }, - percent_encoding::utf8_percent_encode(&path, percent_encoding::CONTROLS) - )); + // Otherwise, communicate the path as OSC 7 (often used for spawning new tabs in the same dir) + run_ansi_sequence(&format!( + "\x1b]7;file://{}{}{}\x1b\\", + percent_encoding::utf8_percent_encode( + hostname.unwrap_or("localhost"), + percent_encoding::CONTROLS + ), + if path.starts_with('/') { "" } else { "/" }, + percent_encoding::utf8_percent_encode(&path, percent_encoding::CONTROLS) + )); - perf( - "communicate path to terminal with osc7", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - } - Err(e) => { - warn!("Could not coerce working directory to string {e}"); - } - } + perf( + "communicate path to terminal with osc7", + start_time, + file!(), + line!(), + column!(), + use_color, + ); } } fn run_shell_integration_osc9_9(engine_state: &EngineState, stack: &mut Stack, use_color: bool) { - if let Some(cwd) = stack.get_env_var(engine_state, "PWD") { - match cwd.coerce_into_string() { - Ok(path) => { - let start_time = Instant::now(); + #[allow(deprecated)] + if let Ok(path) = current_dir_str(engine_state, stack) { + let start_time = Instant::now(); - // Otherwise, communicate the path as OSC 9;9 from ConEmu (often used for spawning new tabs in the same dir) - run_ansi_sequence(&format!( - "\x1b]9;9;{}{}\x1b\\", - if path.starts_with('/') { "" } else { "/" }, - percent_encoding::utf8_percent_encode(&path, percent_encoding::CONTROLS) - )); + // Otherwise, communicate the path as OSC 9;9 from ConEmu (often used for spawning new tabs in the same dir) + run_ansi_sequence(&format!( + "\x1b]9;9;{}{}\x1b\\", + if path.starts_with('/') { "" } else { "/" }, + percent_encoding::utf8_percent_encode(&path, percent_encoding::CONTROLS) + )); - perf( - "communicate path to terminal with osc9;9", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - } - Err(e) => { - warn!("Could not coerce working directory to string {e}"); - } - } + perf( + "communicate path to terminal with osc9;9", + start_time, + file!(), + line!(), + column!(), + use_color, + ); } } fn run_shell_integration_osc633(engine_state: &EngineState, stack: &mut Stack, use_color: bool) { - if let Some(cwd) = stack.get_env_var(engine_state, "PWD") { - match cwd.coerce_into_string() { - Ok(path) => { - // Supported escape sequences of Microsoft's Visual Studio Code (vscode) - // https://code.visualstudio.com/docs/terminal/shell-integration#_supported-escape-sequences - if stack.get_env_var(engine_state, "TERM_PROGRAM") - == Some(Value::test_string("vscode")) - { - let start_time = Instant::now(); + #[allow(deprecated)] + if let Ok(path) = current_dir_str(engine_state, stack) { + // Supported escape sequences of Microsoft's Visual Studio Code (vscode) + // https://code.visualstudio.com/docs/terminal/shell-integration#_supported-escape-sequences + if stack.get_env_var(engine_state, "TERM_PROGRAM") == Some(Value::test_string("vscode")) { + let start_time = Instant::now(); - // If we're in vscode, run their specific ansi escape sequence. - // This is helpful for ctrl+g to change directories in the terminal. - run_ansi_sequence(&format!( - "{}{}{}", - VSCODE_CWD_PROPERTY_MARKER_PREFIX, path, VSCODE_CWD_PROPERTY_MARKER_SUFFIX - )); + // If we're in vscode, run their specific ansi escape sequence. + // This is helpful for ctrl+g to change directories in the terminal. + run_ansi_sequence(&format!( + "{}{}{}", + VSCODE_CWD_PROPERTY_MARKER_PREFIX, path, VSCODE_CWD_PROPERTY_MARKER_SUFFIX + )); - perf( - "communicate path to terminal with osc633;P", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - } - } - Err(e) => { - warn!("Could not coerce working directory to string {e}"); - } + perf( + "communicate path to terminal with osc633;P", + start_time, + file!(), + line!(), + column!(), + use_color, + ); } } } diff --git a/crates/nu-cli/src/syntax_highlight.rs b/crates/nu-cli/src/syntax_highlight.rs index 6d890ad295..d4bbb7db92 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -37,6 +37,7 @@ impl Highlighter for NuHighlighter { let str_word = String::from_utf8_lossy(str_contents).to_string(); let paths = env::path_str(&self.engine_state, &self.stack, *span).ok(); + #[allow(deprecated)] let res = if let Ok(cwd) = env::current_dir_str(&self.engine_state, &self.stack) { diff --git a/crates/nu-cli/tests/completions.rs b/crates/nu-cli/tests/completions.rs index 1936546975..611dbfd35e 100644 --- a/crates/nu-cli/tests/completions.rs +++ b/crates/nu-cli/tests/completions.rs @@ -6,7 +6,7 @@ use nu_parser::parse; use nu_protocol::{debugger::WithoutDebug, engine::StateWorkingSet, PipelineData}; use reedline::{Completer, Suggestion}; use rstest::{fixture, rstest}; -use std::path::PathBuf; +use std::path::{PathBuf, MAIN_SEPARATOR}; use support::{ completions_helpers::{new_partial_engine, new_quote_engine}, file, folder, match_suggestions, new_engine, @@ -220,7 +220,7 @@ fn file_completions() { let mut completer = NuCompleter::new(std::sync::Arc::new(engine), stack); // Test completions for the current folder - let target_dir = format!("cp {dir_str}"); + let target_dir = format!("cp {dir_str}{MAIN_SEPARATOR}"); let suggestions = completer.complete(&target_dir, target_dir.len()); // Create the expected values @@ -664,7 +664,7 @@ fn folder_with_directorycompletions() { let mut completer = NuCompleter::new(std::sync::Arc::new(engine), stack); // Test completions for the current folder - let target_dir = format!("cd {dir_str}"); + let target_dir = format!("cd {dir_str}{MAIN_SEPARATOR}"); let suggestions = completer.complete(&target_dir, target_dir.len()); // Create the expected values diff --git a/crates/nu-cli/tests/support/completions_helpers.rs b/crates/nu-cli/tests/support/completions_helpers.rs index f7ae31c7f4..bdc52739ee 100644 --- a/crates/nu-cli/tests/support/completions_helpers.rs +++ b/crates/nu-cli/tests/support/completions_helpers.rs @@ -8,9 +8,7 @@ use nu_protocol::{ }; use nu_test_support::fs; use reedline::Suggestion; -use std::path::PathBuf; - -const SEP: char = std::path::MAIN_SEPARATOR; +use std::path::{PathBuf, MAIN_SEPARATOR}; fn create_default_context() -> EngineState { nu_command::add_shell_command_context(nu_cmd_lang::create_default_context()) @@ -20,12 +18,11 @@ fn create_default_context() -> EngineState { pub fn new_engine() -> (PathBuf, String, EngineState, Stack) { // Target folder inside assets let dir = fs::fixtures().join("completions"); - let mut dir_str = dir + let dir_str = dir .clone() .into_os_string() .into_string() .unwrap_or_default(); - dir_str.push(SEP); // Create a new engine with default context let mut engine_state = create_default_context(); @@ -77,12 +74,11 @@ pub fn new_engine() -> (PathBuf, String, EngineState, Stack) { pub fn new_quote_engine() -> (PathBuf, String, EngineState, Stack) { // Target folder inside assets let dir = fs::fixtures().join("quoted_completions"); - let mut dir_str = dir + let dir_str = dir .clone() .into_os_string() .into_string() .unwrap_or_default(); - dir_str.push(SEP); // Create a new engine with default context let mut engine_state = create_default_context(); @@ -113,12 +109,11 @@ pub fn new_quote_engine() -> (PathBuf, String, EngineState, Stack) { pub fn new_partial_engine() -> (PathBuf, String, EngineState, Stack) { // Target folder inside assets let dir = fs::fixtures().join("partial_completions"); - let mut dir_str = dir + let dir_str = dir .clone() .into_os_string() .into_string() .unwrap_or_default(); - dir_str.push(SEP); // Create a new engine with default context let mut engine_state = create_default_context(); @@ -165,7 +160,7 @@ pub fn match_suggestions(expected: Vec, suggestions: Vec) { // append the separator to the converted path pub fn folder(path: PathBuf) -> String { let mut converted_path = file(path); - converted_path.push(SEP); + converted_path.push(MAIN_SEPARATOR); converted_path } diff --git a/crates/nu-cmd-base/src/util.rs b/crates/nu-cmd-base/src/util.rs index 7a790725dd..8654975c2b 100644 --- a/crates/nu-cmd-base/src/util.rs +++ b/crates/nu-cmd-base/src/util.rs @@ -13,6 +13,7 @@ pub fn get_init_cwd() -> PathBuf { } pub fn get_guaranteed_cwd(engine_state: &EngineState, stack: &Stack) -> PathBuf { + #[allow(deprecated)] nu_engine::env::current_dir(engine_state, stack).unwrap_or_else(|e| { let working_set = StateWorkingSet::new(engine_state); report_error(&working_set, &e); diff --git a/crates/nu-cmd-plugin/src/commands/plugin/add.rs b/crates/nu-cmd-plugin/src/commands/plugin/add.rs index a0c9bd449a..70f1f417b6 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/add.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/add.rs @@ -1,8 +1,8 @@ -use std::sync::Arc; - +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir}; use nu_plugin_engine::{GetPlugin, PersistentPlugin}; use nu_protocol::{PluginGcConfig, PluginIdentity, PluginRegistryItem, RegisteredPlugin}; +use std::sync::Arc; use crate::util::{get_plugin_dirs, modify_plugin_file}; @@ -82,6 +82,7 @@ apparent the next time `nu` is next launched with that plugin registry file. let filename: Spanned = call.req(engine_state, stack, 0)?; let shell: Option> = call.get_flag(engine_state, stack, "shell")?; + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; // Check the current directory, or fall back to NU_PLUGIN_DIRS diff --git a/crates/nu-cmd-plugin/src/util.rs b/crates/nu-cmd-plugin/src/util.rs index 85818e0564..312ab9e81a 100644 --- a/crates/nu-cmd-plugin/src/util.rs +++ b/crates/nu-cmd-plugin/src/util.rs @@ -1,11 +1,11 @@ +#[allow(deprecated)] +use nu_engine::{command_prelude::*, current_dir}; +use nu_protocol::{engine::StateWorkingSet, PluginRegistryFile}; use std::{ fs::{self, File}, path::PathBuf, }; -use nu_engine::{command_prelude::*, current_dir}; -use nu_protocol::{engine::StateWorkingSet, PluginRegistryFile}; - pub(crate) fn modify_plugin_file( engine_state: &EngineState, stack: &mut Stack, @@ -13,6 +13,7 @@ pub(crate) fn modify_plugin_file( custom_path: Option>, operate: impl FnOnce(&mut PluginRegistryFile) -> Result<(), ShellError>, ) -> Result<(), ShellError> { + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let plugin_registry_file_path = if let Some(ref custom_path) = custom_path { @@ -58,6 +59,7 @@ pub(crate) fn canonicalize_possible_filename_arg( arg: &str, ) -> PathBuf { // This results in the best possible chance of a match with the plugin item + #[allow(deprecated)] if let Ok(cwd) = nu_engine::current_dir(engine_state, stack) { let path = nu_path::expand_path_with(arg, &cwd, true); // Try to canonicalize diff --git a/crates/nu-command/src/filesystem/cd.rs b/crates/nu-command/src/filesystem/cd.rs index 4dcfc46884..50ad0b35a1 100644 --- a/crates/nu-command/src/filesystem/cd.rs +++ b/crates/nu-command/src/filesystem/cd.rs @@ -1,4 +1,4 @@ -use nu_engine::{command_prelude::*, current_dir}; +use nu_engine::command_prelude::*; use nu_utils::filesystem::{have_permission, PermissionResult}; #[derive(Clone)] @@ -20,6 +20,7 @@ impl Command for Cd { fn signature(&self) -> nu_protocol::Signature { Signature::build("cd") .input_output_types(vec![(Type::Nothing, Type::Nothing)]) + .switch("physical", "use the physical directory structure; resolve symbolic links before processing instances of ..", Some('P')) .optional("path", SyntaxShape::Directory, "The path to change to.") .input_output_types(vec![ (Type::Nothing, Type::Nothing), @@ -36,8 +37,9 @@ impl Command for Cd { call: &Call, _input: PipelineData, ) -> Result { + let physical = call.has_flag(engine_state, stack, "physical")?; let path_val: Option> = call.opt(engine_state, stack, 0)?; - let cwd = current_dir(engine_state, stack)?; + let cwd = engine_state.cwd(Some(stack))?; let path_val = { if let Some(path) = path_val { @@ -53,54 +55,53 @@ impl Command for Cd { let (path, span) = match path_val { Some(v) => { if v.item == "-" { - let oldpwd = stack.get_env_var(engine_state, "OLDPWD"); - - if let Some(oldpwd) = oldpwd { - let path = oldpwd.to_path()?; - let path = match nu_path::canonicalize_with(path.clone(), &cwd) { - Ok(p) => p, - Err(_) => { - return Err(ShellError::DirectoryNotFound { - dir: path.to_string_lossy().to_string(), - span: v.span, - }); - } - }; - (path.to_string_lossy().to_string(), v.span) + if let Some(oldpwd) = stack.get_env_var(engine_state, "OLDPWD") { + (oldpwd.to_path()?, v.span) } else { - (cwd.to_string_lossy().to_string(), v.span) + (cwd, v.span) } } else { + // Trim whitespace from the end of path. let path_no_whitespace = &v.item.trim_end_matches(|x| matches!(x, '\x09'..='\x0d')); - let path = match nu_path::canonicalize_with(path_no_whitespace, &cwd) { - Ok(p) => { - if !p.is_dir() { + // If `--physical` is specified, canonicalize the path; otherwise expand the path. + let path = if physical { + if let Ok(path) = nu_path::canonicalize_with(path_no_whitespace, &cwd) { + if !path.is_dir() { return Err(ShellError::NotADirectory { span: v.span }); }; - p - } - - // if canonicalize failed, let's check to see if it's abbreviated - Err(_) => { + path + } else { return Err(ShellError::DirectoryNotFound { dir: path_no_whitespace.to_string(), span: v.span, }); } + } else { + let path = nu_path::expand_path_with(path_no_whitespace, &cwd, true); + if !path.exists() { + return Err(ShellError::DirectoryNotFound { + dir: path_no_whitespace.to_string(), + span: v.span, + }); + }; + if !path.is_dir() { + return Err(ShellError::NotADirectory { span: v.span }); + }; + path }; - (path.to_string_lossy().to_string(), v.span) + (path, v.span) } } None => { let path = nu_path::expand_tilde("~"); - (path.to_string_lossy().to_string(), call.head) + (path, call.head) } }; - let path_value = Value::string(path.clone(), span); - + // Set OLDPWD. + // We're using `Stack::get_env_var()` instead of `EngineState::cwd()` to avoid a conversion roundtrip. if let Some(oldpwd) = stack.get_env_var(engine_state, "PWD") { stack.add_env_var("OLDPWD".into(), oldpwd) } @@ -109,11 +110,15 @@ impl Command for Cd { //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 PermissionResult::PermissionOk => { - stack.add_env_var("PWD".into(), path_value); + stack.add_env_var("PWD".into(), Value::string(path.to_string_lossy(), span)); Ok(PipelineData::empty()) } PermissionResult::PermissionDenied(reason) => Err(ShellError::IOError { - msg: format!("Cannot change directory to {path}: {reason}"), + msg: format!( + "Cannot change directory to {}: {}", + path.to_string_lossy(), + reason + ), }), } } diff --git a/crates/nu-command/src/filesystem/du.rs b/crates/nu-command/src/filesystem/du.rs index 410f57bd2a..adf648f732 100644 --- a/crates/nu-command/src/filesystem/du.rs +++ b/crates/nu-command/src/filesystem/du.rs @@ -1,5 +1,6 @@ use super::util::get_rest_for_glob_pattern; use crate::{DirBuilder, DirInfo, FileInfo}; +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir}; use nu_glob::Pattern; use nu_protocol::NuGlob; @@ -98,6 +99,7 @@ impl Command for Du { let all = call.has_flag(engine_state, stack, "all")?; let deref = call.has_flag(engine_state, stack, "deref")?; let exclude = call.get_flag(engine_state, stack, "exclude")?; + #[allow(deprecated)] let current_dir = current_dir(engine_state, stack)?; let paths = get_rest_for_glob_pattern(engine_state, stack, call, 0)?; diff --git a/crates/nu-command/src/filesystem/glob.rs b/crates/nu-command/src/filesystem/glob.rs index 993d1fcb13..909dc36837 100644 --- a/crates/nu-command/src/filesystem/glob.rs +++ b/crates/nu-command/src/filesystem/glob.rs @@ -1,3 +1,4 @@ +#[allow(deprecated)] use nu_engine::{command_prelude::*, env::current_dir}; use std::sync::{atomic::AtomicBool, Arc}; use wax::{Glob as WaxGlob, WalkBehavior, WalkEntry}; @@ -178,6 +179,7 @@ impl Command for Glob { } }; + #[allow(deprecated)] let path = current_dir(engine_state, stack)?; let path = match nu_path::canonicalize_with(prefix, path) { Ok(path) => path, diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index 8985cd2649..762c8d300d 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -1,6 +1,7 @@ use super::util::get_rest_for_glob_pattern; use crate::{DirBuilder, DirInfo}; use chrono::{DateTime, Local, LocalResult, TimeZone, Utc}; +#[allow(deprecated)] use nu_engine::{command_prelude::*, env::current_dir}; use nu_glob::{MatchOptions, Pattern}; use nu_path::expand_to_real_path; @@ -91,6 +92,7 @@ impl Command for Ls { let use_mime_type = call.has_flag(engine_state, stack, "mime-type")?; let ctrl_c = engine_state.ctrlc.clone(); let call_span = call.head; + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let args = Args { diff --git a/crates/nu-command/src/filesystem/mktemp.rs b/crates/nu-command/src/filesystem/mktemp.rs index c6f51fb7d4..cbe67e65ce 100644 --- a/crates/nu-command/src/filesystem/mktemp.rs +++ b/crates/nu-command/src/filesystem/mktemp.rs @@ -1,3 +1,4 @@ +#[allow(deprecated)] use nu_engine::{command_prelude::*, env::current_dir}; use std::path::PathBuf; @@ -90,6 +91,7 @@ impl Command for Mktemp { } else if directory || tmpdir { Some(std::env::temp_dir()) } else { + #[allow(deprecated)] Some(current_dir(engine_state, stack)?) }; diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs index 06c63e1532..63b6aa8f04 100644 --- a/crates/nu-command/src/filesystem/open.rs +++ b/crates/nu-command/src/filesystem/open.rs @@ -1,4 +1,5 @@ use super::util::get_rest_for_glob_pattern; +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir, get_eval_block}; use nu_protocol::{BufferedReader, DataSource, NuGlob, PipelineMetadata, RawStream}; use std::{io::BufReader, path::Path}; @@ -51,6 +52,7 @@ impl Command for Open { let raw = call.has_flag(engine_state, stack, "raw")?; let call_span = call.head; let ctrlc = engine_state.ctrlc.clone(); + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let mut paths = get_rest_for_glob_pattern(engine_state, stack, call, 0)?; let eval_block = get_eval_block(engine_state); diff --git a/crates/nu-command/src/filesystem/rm.rs b/crates/nu-command/src/filesystem/rm.rs index 6306eb959d..fe14253982 100644 --- a/crates/nu-command/src/filesystem/rm.rs +++ b/crates/nu-command/src/filesystem/rm.rs @@ -1,4 +1,5 @@ use super::util::{get_rest_for_glob_pattern, try_interaction}; +#[allow(deprecated)] use nu_engine::{command_prelude::*, env::current_dir}; use nu_glob::MatchOptions; use nu_path::expand_path_with; @@ -130,6 +131,7 @@ fn rm( let mut unique_argument_check = None; + #[allow(deprecated)] let currentdir_path = current_dir(engine_state, stack)?; let home: Option = nu_path::home_dir().map(|path| { diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs index 852dbf529e..3b92416ab5 100644 --- a/crates/nu-command/src/filesystem/save.rs +++ b/crates/nu-command/src/filesystem/save.rs @@ -1,4 +1,5 @@ use crate::progress_bar; +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir}; use nu_path::expand_path_with; use nu_protocol::{ @@ -85,6 +86,7 @@ impl Command for Save { }; let span = call.head; + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let path_arg = call.req::>(engine_state, stack, 0)?; diff --git a/crates/nu-command/src/filesystem/touch.rs b/crates/nu-command/src/filesystem/touch.rs index f92e3ea4f7..ce8ed0dad3 100644 --- a/crates/nu-command/src/filesystem/touch.rs +++ b/crates/nu-command/src/filesystem/touch.rs @@ -1,4 +1,5 @@ use filetime::FileTime; +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir}; use nu_path::expand_path_with; use nu_protocol::NuGlob; @@ -113,6 +114,7 @@ impl Command for Touch { })?; } + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; for (index, glob) in files.into_iter().enumerate() { diff --git a/crates/nu-command/src/filesystem/ucp.rs b/crates/nu-command/src/filesystem/ucp.rs index 79980e30db..d73413d721 100644 --- a/crates/nu-command/src/filesystem/ucp.rs +++ b/crates/nu-command/src/filesystem/ucp.rs @@ -1,4 +1,5 @@ use super::util::get_rest_for_glob_pattern; +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir}; use std::path::PathBuf; use uu_cp::{BackupMode, CopyMode, UpdateMode}; @@ -177,6 +178,7 @@ impl Command for UCp { let target_path = PathBuf::from(&nu_utils::strip_ansi_string_unlikely( target.item.to_string(), )); + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let target_path = nu_path::expand_path_with(target_path, &cwd, target.item.is_expand()); if target.item.as_ref().ends_with(PATH_SEPARATOR) && !target_path.is_dir() { diff --git a/crates/nu-command/src/filesystem/umkdir.rs b/crates/nu-command/src/filesystem/umkdir.rs index 5b8d8c33cd..7565beee5e 100644 --- a/crates/nu-command/src/filesystem/umkdir.rs +++ b/crates/nu-command/src/filesystem/umkdir.rs @@ -1,3 +1,4 @@ +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir}; use uu_mkdir::mkdir; @@ -58,6 +59,7 @@ impl Command for UMkdir { call: &Call, _input: PipelineData, ) -> Result { + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let mut directories = get_rest_for_glob_pattern(engine_state, stack, call, 0)? .into_iter() diff --git a/crates/nu-command/src/filesystem/umv.rs b/crates/nu-command/src/filesystem/umv.rs index b60a4f70ed..d5c58f30e4 100644 --- a/crates/nu-command/src/filesystem/umv.rs +++ b/crates/nu-command/src/filesystem/umv.rs @@ -1,4 +1,5 @@ use super::util::get_rest_for_glob_pattern; +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir}; use nu_path::expand_path_with; use nu_protocol::NuGlob; @@ -77,6 +78,7 @@ impl Command for UMv { uu_mv::OverwriteMode::Force }; + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let mut paths = get_rest_for_glob_pattern(engine_state, stack, call, 0)?; if paths.is_empty() { diff --git a/crates/nu-command/src/filesystem/watch.rs b/crates/nu-command/src/filesystem/watch.rs index 91837ad5cb..224d58d0d0 100644 --- a/crates/nu-command/src/filesystem/watch.rs +++ b/crates/nu-command/src/filesystem/watch.rs @@ -5,6 +5,7 @@ use notify_debouncer_full::{ EventKind, RecursiveMode, Watcher, }, }; +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir, ClosureEval}; use nu_protocol::{ engine::{Closure, StateWorkingSet}, @@ -73,6 +74,7 @@ impl Command for Watch { _input: PipelineData, ) -> Result { let head = call.head; + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let path_arg: Spanned = call.req(engine_state, stack, 0)?; diff --git a/crates/nu-command/src/path/exists.rs b/crates/nu-command/src/path/exists.rs index a90a2b5c91..a99f04113e 100644 --- a/crates/nu-command/src/path/exists.rs +++ b/crates/nu-command/src/path/exists.rs @@ -1,4 +1,5 @@ use super::PathSubcommandArguments; +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir, current_dir_const}; use nu_path::expand_path_with; use nu_protocol::engine::StateWorkingSet; @@ -53,6 +54,7 @@ If you need to distinguish dirs and files, please use `path type`."# input: PipelineData, ) -> Result { let head = call.head; + #[allow(deprecated)] let args = Arguments { pwd: current_dir(engine_state, stack)?, not_follow_symlink: call.has_flag(engine_state, stack, "no-symlink")?, @@ -74,6 +76,7 @@ If you need to distinguish dirs and files, please use `path type`."# input: PipelineData, ) -> Result { let head = call.head; + #[allow(deprecated)] let args = Arguments { pwd: current_dir_const(working_set)?, not_follow_symlink: call.has_flag_const(working_set, "no-symlink")?, diff --git a/crates/nu-command/src/path/expand.rs b/crates/nu-command/src/path/expand.rs index 53e2e1c4e8..18497c6426 100644 --- a/crates/nu-command/src/path/expand.rs +++ b/crates/nu-command/src/path/expand.rs @@ -1,4 +1,5 @@ use super::PathSubcommandArguments; +#[allow(deprecated)] use nu_engine::{ command_prelude::*, env::{current_dir_str, current_dir_str_const}, @@ -57,6 +58,7 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + #[allow(deprecated)] let args = Arguments { strict: call.has_flag(engine_state, stack, "strict")?, cwd: current_dir_str(engine_state, stack)?, @@ -79,6 +81,7 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; + #[allow(deprecated)] let args = Arguments { strict: call.has_flag_const(working_set, "strict")?, cwd: current_dir_str_const(working_set)?, diff --git a/crates/nu-command/src/system/exec.rs b/crates/nu-command/src/system/exec.rs index 4ae44e44fd..21c98f121b 100644 --- a/crates/nu-command/src/system/exec.rs +++ b/crates/nu-command/src/system/exec.rs @@ -1,4 +1,5 @@ use super::run_external::create_external_command; +#[allow(deprecated)] use nu_engine::{command_prelude::*, current_dir}; use nu_protocol::OutDest; @@ -62,6 +63,7 @@ fn exec( external_command.out = OutDest::Inherit; external_command.err = OutDest::Inherit; + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let mut command = external_command.spawn_simple_command(&cwd.to_string_lossy())?; command.current_dir(cwd); diff --git a/crates/nu-command/src/system/which_.rs b/crates/nu-command/src/system/which_.rs index 4e3e8c5786..f593bcf9fe 100644 --- a/crates/nu-command/src/system/which_.rs +++ b/crates/nu-command/src/system/which_.rs @@ -229,6 +229,7 @@ fn which( let mut output = vec![]; + #[allow(deprecated)] let cwd = env::current_dir_str(engine_state, stack)?; let paths = env::path_str(engine_state, stack, call.head)?; diff --git a/crates/nu-command/tests/commands/cd.rs b/crates/nu-command/tests/commands/cd.rs index 41e955d626..9cdafc005a 100644 --- a/crates/nu-command/tests/commands/cd.rs +++ b/crates/nu-command/tests/commands/cd.rs @@ -207,7 +207,15 @@ fn filesystem_change_directory_to_symlink_relative() { $env.PWD " ); + assert_eq!(PathBuf::from(actual.out), dirs.test().join("foo_link")); + let actual = nu!( + cwd: dirs.test().join("boo"), + " + cd -P ../foo_link + $env.PWD + " + ); assert_eq!(PathBuf::from(actual.out), dirs.test().join("foo")); }) } diff --git a/crates/nu-engine/src/env.rs b/crates/nu-engine/src/env.rs index 12408312a0..19ed589dcf 100644 --- a/crates/nu-engine/src/env.rs +++ b/crates/nu-engine/src/env.rs @@ -2,7 +2,7 @@ use crate::ClosureEvalOnce; use nu_path::canonicalize_with; use nu_protocol::{ ast::{Call, Expr}, - engine::{EngineState, Stack, StateWorkingSet, PWD_ENV}, + engine::{EngineState, Stack, StateWorkingSet}, Config, ShellError, Span, Value, VarId, }; use std::{ @@ -156,85 +156,56 @@ pub fn env_to_strings( Ok(env_vars_str) } -/// Shorthand for env_to_string() for PWD with custom error +/// Returns the current working directory as a String, which is guaranteed to be canonicalized. +/// Unlike `current_dir_str_const()`, this also considers modifications to the current working directory made on the stack. +/// +/// Returns an error if $env.PWD doesn't exist, is not a String, or is not an absolute path. +#[deprecated(since = "0.92.3", note = "please use `EngineState::cwd()` instead")] pub fn current_dir_str(engine_state: &EngineState, stack: &Stack) -> Result { - if let Some(pwd) = stack.get_env_var(engine_state, PWD_ENV) { - // TODO: PWD should be string by default, we don't need to run ENV_CONVERSIONS on it - match env_to_string(PWD_ENV, &pwd, engine_state, stack) { - Ok(cwd) => { - if Path::new(&cwd).is_absolute() { - Ok(cwd) - } else { - Err(ShellError::GenericError { - error: "Invalid current directory".into(), - msg: format!("The 'PWD' environment variable must be set to an absolute path. Found: '{cwd}'"), - span: Some(pwd.span()), - help: None, - inner: vec![] - }) - } - } - Err(e) => Err(e), - } - } else { - Err(ShellError::GenericError { - error: "Current directory not found".into(), - msg: "".into(), - span: None, - help: Some("The environment variable 'PWD' was not found. It is required to define the current directory.".into()), - inner: vec![], - }) - } + #[allow(deprecated)] + current_dir(engine_state, stack).map(|path| path.to_string_lossy().to_string()) } -/// Simplified version of current_dir_str() for constant evaluation +/// Returns the current working directory as a String, which is guaranteed to be canonicalized. +/// +/// Returns an error if $env.PWD doesn't exist, is not a String, or is not an absolute path. +#[deprecated(since = "0.92.3", note = "please use `EngineState::cwd()` instead")] pub fn current_dir_str_const(working_set: &StateWorkingSet) -> Result { - if let Some(pwd) = working_set.get_env_var(PWD_ENV) { - let span = pwd.span(); - match pwd { - Value::String { val, .. } => { - if Path::new(val).is_absolute() { - Ok(val.clone()) - } else { - Err(ShellError::GenericError { - error: "Invalid current directory".into(), - msg: format!("The 'PWD' environment variable must be set to an absolute path. Found: '{val}'"), - span: Some(span), - help: None, - inner: vec![] - }) - } - } - _ => Err(ShellError::GenericError { - error: "PWD is not a string".into(), - msg: "".into(), - span: None, - help: Some( - "Cusrrent working directory environment variable 'PWD' must be a string." - .into(), - ), - inner: vec![], - }), - } - } else { - Err(ShellError::GenericError{ - error: "Current directory not found".into(), - msg: "".into(), - span: None, - help: Some("The environment variable 'PWD' was not found. It is required to define the current directory.".into()), - inner: vec![], - }) - } + #[allow(deprecated)] + current_dir_const(working_set).map(|path| path.to_string_lossy().to_string()) } -/// Calls current_dir_str() and returns the current directory as a PathBuf +/// Returns the current working directory, which is guaranteed to be canonicalized. +/// Unlike `current_dir_const()`, this also considers modifications to the current working directory made on the stack. +/// +/// Returns an error if $env.PWD doesn't exist, is not a String, or is not an absolute path. +#[deprecated(since = "0.92.3", note = "please use `EngineState::cwd()` instead")] pub fn current_dir(engine_state: &EngineState, stack: &Stack) -> Result { - current_dir_str(engine_state, stack).map(PathBuf::from) + let cwd = engine_state.cwd(Some(stack))?; + // `EngineState::cwd()` always returns absolute path. + // We're using `canonicalize_with` instead of `fs::canonicalize()` because + // we still need to simplify Windows paths. "." is safe because `cwd` should + // be an absolute path already. + canonicalize_with(&cwd, ".").map_err(|_| ShellError::DirectoryNotFound { + dir: cwd.to_string_lossy().to_string(), + span: Span::unknown(), + }) } -/// Version of current_dir() for constant evaluation +/// Returns the current working directory, which is guaranteed to be canonicalized. +/// +/// Returns an error if $env.PWD doesn't exist, is not a String, or is not an absolute path. +#[deprecated(since = "0.92.3", note = "please use `EngineState::cwd()` instead")] pub fn current_dir_const(working_set: &StateWorkingSet) -> Result { - current_dir_str_const(working_set).map(PathBuf::from) + let cwd = working_set.permanent_state.cwd(None)?; + // `EngineState::cwd()` always returns absolute path. + // We're using `canonicalize_with` instead of `fs::canonicalize()` because + // we still need to simplify Windows paths. "." is safe because `cwd` should + // be an absolute path already. + canonicalize_with(&cwd, ".").map_err(|_| ShellError::DirectoryNotFound { + dir: cwd.to_string_lossy().to_string(), + span: Span::unknown(), + }) } /// Get the contents of path environment variable as a list of strings @@ -315,6 +286,7 @@ pub fn find_in_dirs_env( Err(e) => return Err(e), } } else { + #[allow(deprecated)] current_dir_str(engine_state, stack)? }; diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 53b3b1ccdb..6324b35ae7 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -1,4 +1,5 @@ -use crate::{current_dir, current_dir_str, get_config, get_full_help}; +#[allow(deprecated)] +use crate::{current_dir, get_config, get_full_help}; use nu_path::expand_path_with; use nu_protocol::{ ast::{ @@ -325,6 +326,7 @@ fn eval_redirection( ) -> Result { match target { RedirectionTarget::File { expr, append, .. } => { + #[allow(deprecated)] let cwd = current_dir(engine_state, stack)?; let value = eval_expression::(engine_state, stack, expr)?; let path = Spanned::::from_value(value)?.item; @@ -633,7 +635,7 @@ impl Eval for EvalRuntime { if quoted { Ok(Value::string(path, span)) } else { - let cwd = current_dir_str(engine_state, stack)?; + let cwd = engine_state.cwd(Some(stack))?; let path = expand_path_with(path, cwd, true); Ok(Value::string(path.to_string_lossy(), span)) @@ -652,7 +654,7 @@ impl Eval for EvalRuntime { } else if quoted { Ok(Value::string(path, span)) } else { - let cwd = current_dir_str(engine_state, stack)?; + let cwd = engine_state.cwd(Some(stack))?; let path = expand_path_with(path, cwd, true); Ok(Value::string(path.to_string_lossy(), span)) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 4c2b2f7bd8..381a86d19e 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1949,6 +1949,7 @@ pub fn parse_module_file_or_dir( return None; } + #[allow(deprecated)] let cwd = working_set.get_cwd(); let module_path = @@ -3359,6 +3360,7 @@ pub fn parse_source(working_set: &mut StateWorkingSet, lite_command: &LiteComman let scoped = name == b"source-env"; if let Some(decl_id) = working_set.find_decl(name) { + #[allow(deprecated)] let cwd = working_set.get_cwd(); // Is this the right call to be using here? @@ -3564,6 +3566,7 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm let spans = &lite_command.parts; + #[allow(deprecated)] let cwd = working_set.get_cwd(); // Checking that the function is used with the correct name @@ -3785,6 +3788,7 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm pub fn parse_plugin_use(working_set: &mut StateWorkingSet, call: Box) -> Pipeline { use nu_protocol::{FromValue, PluginRegistryFile}; + #[allow(deprecated)] let cwd = working_set.get_cwd(); if let Err(err) = (|| { diff --git a/crates/nu-plugin-engine/src/context.rs b/crates/nu-plugin-engine/src/context.rs index 71773ff20a..3f77d85477 100644 --- a/crates/nu-plugin-engine/src/context.rs +++ b/crates/nu-plugin-engine/src/context.rs @@ -125,6 +125,7 @@ impl<'a> PluginExecutionContext for PluginExecutionCommandContext<'a> { } fn get_current_dir(&self) -> Result, ShellError> { + #[allow(deprecated)] let cwd = nu_engine::env::current_dir_str(&self.engine_state, &self.stack)?; // The span is not really used, so just give it call.head Ok(cwd.into_spanned(self.call.head)) diff --git a/crates/nu-protocol/Cargo.toml b/crates/nu-protocol/Cargo.toml index 9a06b97f09..e2420a6e83 100644 --- a/crates/nu-protocol/Cargo.toml +++ b/crates/nu-protocol/Cargo.toml @@ -46,6 +46,7 @@ strum_macros = "0.26" nu-test-support = { path = "../nu-test-support", version = "0.93.1" } pretty_assertions = { workspace = true } rstest = { workspace = true } +tempfile = { workspace = true } [package.metadata.docs.rs] all-features = true diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 287b4c4e27..cab3dcc48a 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -26,8 +26,6 @@ type PoisonDebuggerError<'a> = PoisonError>>; #[cfg(feature = "plugin")] use crate::{PluginRegistryFile, PluginRegistryItem, RegisteredPlugin}; -pub static PWD_ENV: &str = "PWD"; - #[derive(Clone, Debug)] pub enum VirtualPath { File(FileId), @@ -893,14 +891,6 @@ impl EngineState { self.num_files() - 1 } - pub fn get_cwd(&self) -> Option { - if let Some(pwd_value) = self.get_env_var(PWD_ENV) { - pwd_value.coerce_string().ok() - } else { - None - } - } - pub fn set_config_path(&mut self, key: &str, val: PathBuf) { self.config_path.insert(key.to_string(), val); } @@ -922,12 +912,71 @@ impl EngineState { .map(|comment_spans| self.build_usage(comment_spans)) } + /// Returns the current working directory, which is guaranteed to be canonicalized. + /// + /// Returns an empty String if $env.PWD doesn't exist. + #[deprecated(since = "0.92.3", note = "please use `EngineState::cwd()` instead")] pub fn current_work_dir(&self) -> String { - self.get_env_var("PWD") - .map(|d| d.coerce_string().unwrap_or_default()) + self.cwd(None) + .map(|path| path.to_string_lossy().to_string()) .unwrap_or_default() } + /// Returns the current working directory, which is guaranteed to be an + /// absolute path without trailing slashes, but might contain symlink + /// components. + /// + /// If `stack` is supplied, also considers modifications to the working + /// directory on the stack that have yet to be merged into the engine state. + pub fn cwd(&self, stack: Option<&Stack>) -> Result { + // Helper function to create a simple generic error. + // Its messages are not especially helpful, but these errors don't occur often, so it's probably fine. + fn error(msg: &str) -> Result { + Err(ShellError::GenericError { + error: msg.into(), + msg: "".into(), + span: None, + help: None, + inner: vec![], + }) + } + + // Helper function to check if a path has trailing slashes. + fn has_trailing_slash(path: &Path) -> bool { + nu_path::components(path).last() + == Some(std::path::Component::Normal(std::ffi::OsStr::new(""))) + } + + // Retrieve $env.PWD from the stack or the engine state. + let pwd = if let Some(stack) = stack { + stack.get_env_var(self, "PWD") + } else { + self.get_env_var("PWD").map(ToOwned::to_owned) + }; + + if let Some(pwd) = pwd { + if let Value::String { val, .. } = pwd { + let path = PathBuf::from(val); + + if has_trailing_slash(&path) { + error("$env.PWD contains trailing slashes") + } else if !path.is_absolute() { + error("$env.PWD is not an absolute path") + } else if !path.exists() { + error("$env.PWD points to a non-existent directory") + } else if !path.is_dir() { + error("$env.PWD points to a non-directory") + } else { + Ok(path) + } + } else { + error("$env.PWD is not a string") + } + } else { + error("$env.PWD not found") + } + } + // TODO: see if we can completely get rid of this pub fn get_file_contents(&self) -> &[CachedFile] { &self.files @@ -1077,3 +1126,213 @@ mod engine_state_tests { ); } } + +#[cfg(test)] +mod test_cwd { + //! Here're the test cases we need to cover: + //! + //! `EngineState::cwd()` computes the result from `self.env_vars["PWD"]` and + //! optionally `stack.env_vars["PWD"]`. + //! + //! PWD may be unset in either `env_vars`. + //! PWD should NOT be an empty string. + //! PWD should NOT be a non-string value. + //! PWD should NOT be a relative path. + //! PWD should NOT contain trailing slashes. + //! PWD may point to a directory or a symlink to directory. + //! PWD should NOT point to a file or a symlink to file. + //! PWD should NOT point to non-existent entities in the filesystem. + + use crate::{ + engine::{EngineState, Stack}, + Span, Value, + }; + use nu_path::assert_path_eq; + use std::path::Path; + use tempfile::{NamedTempFile, TempDir}; + + /// Creates a symlink. Works on both Unix and Windows. + #[cfg(any(unix, windows))] + fn symlink(original: impl AsRef, link: impl AsRef) -> std::io::Result<()> { + #[cfg(unix)] + { + std::os::unix::fs::symlink(original, link) + } + #[cfg(windows)] + { + if original.as_ref().is_dir() { + std::os::windows::fs::symlink_dir(original, link) + } else { + std::os::windows::fs::symlink_file(original, link) + } + } + } + + /// Create an engine state initialized with the given PWD. + fn engine_state_with_pwd(path: impl AsRef) -> EngineState { + let mut engine_state = EngineState::new(); + engine_state.add_env_var( + "PWD".into(), + Value::String { + val: path.as_ref().to_string_lossy().to_string(), + internal_span: Span::unknown(), + }, + ); + engine_state + } + + /// Create a stack initialized with the given PWD. + fn stack_with_pwd(path: impl AsRef) -> Stack { + let mut stack = Stack::new(); + stack.add_env_var( + "PWD".into(), + Value::String { + val: path.as_ref().to_string_lossy().to_string(), + internal_span: Span::unknown(), + }, + ); + stack + } + + #[test] + fn pwd_not_set() { + let engine_state = EngineState::new(); + engine_state.cwd(None).unwrap_err(); + } + + #[test] + fn pwd_is_empty_string() { + let engine_state = engine_state_with_pwd(""); + engine_state.cwd(None).unwrap_err(); + } + + #[test] + fn pwd_is_non_string_value() { + let mut engine_state = EngineState::new(); + engine_state.add_env_var( + "PWD".into(), + Value::Glob { + val: "*".into(), + no_expand: false, + internal_span: Span::unknown(), + }, + ); + engine_state.cwd(None).unwrap_err(); + } + + #[test] + fn pwd_is_relative_path() { + let engine_state = engine_state_with_pwd("./foo"); + + engine_state.cwd(None).unwrap_err(); + } + + #[test] + fn pwd_has_trailing_slash() { + let dir = TempDir::new().unwrap(); + let engine_state = engine_state_with_pwd(dir.path().join("")); + + engine_state.cwd(None).unwrap_err(); + } + + #[test] + fn pwd_points_to_normal_file() { + let file = NamedTempFile::new().unwrap(); + let engine_state = engine_state_with_pwd(file.path()); + + engine_state.cwd(None).unwrap_err(); + } + + #[test] + fn pwd_points_to_normal_directory() { + let dir = TempDir::new().unwrap(); + let engine_state = engine_state_with_pwd(dir.path()); + + let cwd = engine_state.cwd(None).unwrap(); + assert_path_eq!(cwd, dir.path()); + } + + #[test] + fn pwd_points_to_symlink_to_file() { + let file = NamedTempFile::new().unwrap(); + let dir = TempDir::new().unwrap(); + let link = dir.path().join("link"); + symlink(file.path(), &link).unwrap(); + let engine_state = engine_state_with_pwd(&link); + + engine_state.cwd(None).unwrap_err(); + } + + #[test] + fn pwd_points_to_symlink_to_directory() { + let dir = TempDir::new().unwrap(); + let link = dir.path().join("link"); + symlink(dir.path(), &link).unwrap(); + let engine_state = engine_state_with_pwd(&link); + + let cwd = engine_state.cwd(None).unwrap(); + assert_path_eq!(cwd, link); + } + + #[test] + fn pwd_points_to_broken_symlink() { + let dir = TempDir::new().unwrap(); + let link = dir.path().join("link"); + symlink(TempDir::new().unwrap().path(), &link).unwrap(); + let engine_state = engine_state_with_pwd(&link); + + engine_state.cwd(None).unwrap_err(); + } + + #[test] + fn pwd_points_to_nonexistent_entity() { + let engine_state = engine_state_with_pwd(TempDir::new().unwrap().path()); + + engine_state.cwd(None).unwrap_err(); + } + + #[test] + fn stack_pwd_not_set() { + let dir = TempDir::new().unwrap(); + let engine_state = engine_state_with_pwd(dir.path()); + let stack = Stack::new(); + + let cwd = engine_state.cwd(Some(&stack)).unwrap(); + assert_eq!(cwd, dir.path()); + } + + #[test] + fn stack_pwd_is_empty_string() { + let dir = TempDir::new().unwrap(); + let engine_state = engine_state_with_pwd(dir.path()); + let stack = stack_with_pwd(""); + + engine_state.cwd(Some(&stack)).unwrap_err(); + } + + #[test] + fn stack_pwd_points_to_normal_directory() { + let dir1 = TempDir::new().unwrap(); + let dir2 = TempDir::new().unwrap(); + let engine_state = engine_state_with_pwd(dir1.path()); + let stack = stack_with_pwd(dir2.path()); + + let cwd = engine_state.cwd(Some(&stack)).unwrap(); + assert_path_eq!(cwd, dir2.path()); + } + + #[test] + fn stack_pwd_points_to_normal_directory_with_symlink_components() { + // `/tmp/dir/link` points to `/tmp/dir`, then we set PWD to `/tmp/dir/link/foo` + let dir = TempDir::new().unwrap(); + let link = dir.path().join("link"); + symlink(dir.path(), &link).unwrap(); + let foo = link.join("foo"); + std::fs::create_dir(dir.path().join("foo")).unwrap(); + let engine_state = EngineState::new(); + let stack = stack_with_pwd(&foo); + + let cwd = engine_state.cwd(Some(&stack)).unwrap(); + assert_path_eq!(cwd, foo); + } +} diff --git a/crates/nu-protocol/src/engine/state_working_set.rs b/crates/nu-protocol/src/engine/state_working_set.rs index 1ef4ba5e05..0fe39682e2 100644 --- a/crates/nu-protocol/src/engine/state_working_set.rs +++ b/crates/nu-protocol/src/engine/state_working_set.rs @@ -2,7 +2,7 @@ use crate::{ ast::Block, engine::{ usage::build_usage, CachedFile, Command, CommandType, EngineState, OverlayFrame, - StateDelta, Variable, VirtualPath, Visibility, PWD_ENV, + StateDelta, Variable, VirtualPath, Visibility, }, BlockId, Category, Config, DeclId, FileId, Module, ModuleId, ParseError, ParseWarning, Span, Type, Value, VarId, VirtualPathId, @@ -601,13 +601,16 @@ impl<'a> StateWorkingSet<'a> { next_id } + /// Returns the current working directory as a String, which is guaranteed to be canonicalized. + /// Returns an empty string if $env.PWD doesn't exist, is not a String, or is not an absolute path. + /// + /// It does NOT consider modifications to the working directory made on a stack. + #[deprecated(since = "0.92.3", note = "please use `EngineState::cwd()` instead")] pub fn get_cwd(&self) -> String { - let pwd = self - .permanent_state - .get_env_var(PWD_ENV) - .expect("internal error: can't find PWD"); - pwd.coerce_string() - .expect("internal error: PWD not a string") + self.permanent_state + .cwd(None) + .map(|path| path.to_string_lossy().to_string()) + .unwrap_or_default() } pub fn get_env_var(&self, name: &str) -> Option<&Value> { @@ -622,16 +625,6 @@ impl<'a> StateWorkingSet<'a> { &self.permanent_state.config } - pub fn list_env(&self) -> Vec { - let mut env_vars = vec![]; - - for env_var in self.permanent_state.env_vars.iter() { - env_vars.push(env_var.0.clone()); - } - - env_vars - } - pub fn set_variable_type(&mut self, var_id: VarId, ty: Type) { let num_permanent_vars = self.permanent_state.num_vars(); if var_id < num_permanent_vars { diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index a3992ec945..e45e3b7e4b 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -14,6 +14,7 @@ use std::{ /// Create a Value for `$nu`. pub fn create_nu_constant(engine_state: &EngineState, span: Span) -> Result { fn canonicalize_path(engine_state: &EngineState, path: &Path) -> PathBuf { + #[allow(deprecated)] let cwd = engine_state.current_work_dir(); if path.exists() { diff --git a/crates/nu-std/src/lib.rs b/crates/nu-std/src/lib.rs index fea5cc2f2a..a3cf9271a5 100644 --- a/crates/nu-std/src/lib.rs +++ b/crates/nu-std/src/lib.rs @@ -1,4 +1,5 @@ use log::trace; +#[allow(deprecated)] use nu_engine::{env::current_dir, eval_block}; use nu_parser::parse; use nu_protocol::{ @@ -98,6 +99,7 @@ use std pwd eval_block::(engine_state, &mut stack, &block, pipeline_data)?; + #[allow(deprecated)] let cwd = current_dir(engine_state, &stack)?; engine_state.merge_env(&mut stack, cwd)?; diff --git a/crates/nu-std/std/mod.nu b/crates/nu-std/std/mod.nu index 9a58134b42..9d4387fa95 100644 --- a/crates/nu-std/std/mod.nu +++ b/crates/nu-std/std/mod.nu @@ -196,8 +196,14 @@ export def ellie [] { } # Return the current working directory -export def pwd [] { - $env.PWD +export def pwd [ + --physical (-P) # resolve symbolic links +] { + if $physical { + $env.PWD | path expand + } else { + $env.PWD + } } # repeat anything a bunch of times, yielding a list of *n* times the input diff --git a/src/config_files.rs b/src/config_files.rs index 3eb918796b..e67af7f2e1 100644 --- a/src/config_files.rs +++ b/src/config_files.rs @@ -31,6 +31,7 @@ pub(crate) fn read_config_file( // Load config startup file if let Some(file) = config_file { let working_set = StateWorkingSet::new(engine_state); + #[allow(deprecated)] let cwd = working_set.get_cwd(); if let Ok(path) = canonicalize_with(&file.item, cwd) { @@ -143,6 +144,7 @@ pub(crate) fn read_default_env_file(engine_state: &mut EngineState, stack: &mut info!("read_config_file {}:{}:{}", file!(), line!(), column!()); // Merge the environment in case env vars changed in the config + #[allow(deprecated)] match nu_engine::env::current_dir(engine_state, stack) { Ok(cwd) => { if let Err(e) = engine_state.merge_env(stack, cwd) { @@ -184,6 +186,7 @@ fn eval_default_config( ); // Merge the environment in case env vars changed in the config + #[allow(deprecated)] match nu_engine::env::current_dir(engine_state, stack) { Ok(cwd) => { if let Err(e) = engine_state.merge_env(stack, cwd) { diff --git a/src/test_bins.rs b/src/test_bins.rs index 97fe9cc357..5fef4976a7 100644 --- a/src/test_bins.rs +++ b/src/test_bins.rs @@ -249,6 +249,7 @@ pub fn nu_repl() { for (i, line) in source_lines.iter().enumerate() { let mut stack = Stack::with_parent(top_stack.clone()); + #[allow(deprecated)] let cwd = nu_engine::env::current_dir(&engine_state, &stack) .unwrap_or_else(|err| outcome_err(&engine_state, &err));