From dc8268be0ccc7e7dc967adfefd5633ef2fadc030 Mon Sep 17 00:00:00 2001 From: Simon Guest Date: Wed, 26 Jun 2024 15:38:36 +1200 Subject: [PATCH] Use Path::real_parent instead of Path::parent Except for places where this makes no sense. Note that real_parent touches the filesystem and requires the path to exist. Also, unable to use it in nu-test-support without breaking the test commands::ucp::copies_the_file_inside_directory_if_path_to_copy_is_directory which switches cwd out from under its feet (I think). --- Cargo.lock | 16 ++++++++++++++++ Cargo.toml | 2 ++ crates/nu-cli/Cargo.toml | 1 + crates/nu-cli/src/config_files.rs | 6 ++++-- crates/nu-cli/src/eval_file.rs | 7 ++++--- crates/nu-cmd-lang/Cargo.toml | 1 + crates/nu-cmd-lang/src/core_commands/use_.rs | 3 ++- crates/nu-command/Cargo.toml | 1 + crates/nu-command/src/debug/info.rs | 3 ++- crates/nu-command/src/env/source_env.rs | 3 ++- crates/nu-engine/Cargo.toml | 1 + crates/nu-engine/src/glob_from.rs | 3 ++- crates/nu-parser/Cargo.toml | 1 + crates/nu-plugin-engine/Cargo.toml | 1 + crates/nu-plugin-engine/src/init.rs | 5 +++-- crates/nu-protocol/Cargo.toml | 1 + crates/nu-protocol/src/parser_path.rs | 9 +++++---- crates/nu-system/Cargo.toml | 1 + crates/nu-system/src/macos.rs | 5 +++-- crates/nu-test-support/src/macros.rs | 4 +++- crates/nu_plugin_gstat/Cargo.toml | 1 + crates/nu_plugin_gstat/src/gstat.rs | 6 ++++-- tests/plugins/registry_file.rs | 5 +++-- 23 files changed, 64 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cd7fe99800..400c091299 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2794,6 +2794,7 @@ dependencies = [ "nu-utils", "openssl", "pretty_assertions", + "real_parent", "reedline", "rstest", "serde_json", @@ -2840,6 +2841,7 @@ dependencies = [ "nu-utils", "once_cell", "percent-encoding", + "real_parent", "reedline", "rstest", "sysinfo", @@ -2895,6 +2897,7 @@ dependencies = [ "nu-parser", "nu-protocol", "nu-utils", + "real_parent", "shadow-rs", ] @@ -2992,6 +2995,7 @@ dependencies = [ "quickcheck_macros", "rand", "rayon", + "real_parent", "regex", "rmp", "roxmltree", @@ -3049,6 +3053,7 @@ dependencies = [ "nu-path", "nu-protocol", "nu-utils", + "real_parent", ] [[package]] @@ -3126,6 +3131,7 @@ dependencies = [ "nu-path", "nu-plugin-engine", "nu-protocol", + "real_parent", "rstest", "serde_json", ] @@ -3178,6 +3184,7 @@ dependencies = [ "nu-plugin-protocol", "nu-protocol", "nu-system", + "real_parent", "serde", "typetag", "windows 0.54.0", @@ -3244,6 +3251,7 @@ dependencies = [ "num-format", "os_pipe", "pretty_assertions", + "real_parent", "rmp-serde", "rstest", "serde", @@ -3280,6 +3288,7 @@ dependencies = [ "ntapi", "once_cell", "procfs", + "real_parent", "sysinfo", "windows 0.54.0", ] @@ -3374,6 +3383,7 @@ dependencies = [ "git2", "nu-plugin", "nu-protocol", + "real_parent", ] [[package]] @@ -4793,6 +4803,12 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "real_parent" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "03ad1715ba6758e16005557a376d29beb8edbfc0a8c272a5412b2044d997ca7f" + [[package]] name = "recursive" version = "0.1.1" diff --git a/Cargo.toml b/Cargo.toml index c88eee88ea..69e025ce95 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -136,6 +136,7 @@ quote = "1.0" rand = "0.8" ratatui = "0.26" rayon = "1.10" +real_parent = "0.3.0" reedline = "0.32.0" regex = "1.9.5" rmp = "0.8" @@ -232,6 +233,7 @@ assert_cmd = "2.0" dirs-next = { workspace = true } tango-bench = "0.5" pretty_assertions = { workspace = true } +real_parent = { workspace = true } rstest = { workspace = true, default-features = false } serial_test = "3.1" tempfile = { workspace = true } diff --git a/crates/nu-cli/Cargo.toml b/crates/nu-cli/Cargo.toml index 2ed504d502..ffad8c3760 100644 --- a/crates/nu-cli/Cargo.toml +++ b/crates/nu-cli/Cargo.toml @@ -39,6 +39,7 @@ miette = { workspace = true, features = ["fancy-no-backtrace"] } lscolors = { workspace = true, default-features = false, features = ["nu-ansi-term"] } once_cell = { workspace = true } percent-encoding = { workspace = true } +real_parent = { workspace = true } sysinfo = { workspace = true } unicode-segmentation = { workspace = true } uuid = { workspace = true, features = ["v4"] } diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 6cf4735c46..d8d20dd3e2 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -9,6 +9,8 @@ use nu_protocol::{ }; #[cfg(feature = "plugin")] use nu_utils::utils::perf; +#[cfg(feature = "plugin")] +use real_parent::PathExt; use std::path::PathBuf; #[cfg(feature = "plugin")] @@ -180,9 +182,9 @@ pub fn add_plugin_file( if let Ok(cwd) = engine_state.cwd_as_string(None) { if let Some(plugin_file) = plugin_file { let path = Path::new(&plugin_file.item); - let path_dir = path.parent().unwrap_or(path); + let path_dir = path.real_parent().unwrap_or(path.to_path_buf()); // Just try to canonicalize the directory of the plugin file first. - if let Ok(path_dir) = canonicalize_with(path_dir, &cwd) { + if let Ok(path_dir) = canonicalize_with(&path_dir, &cwd) { // Try to canonicalize the actual filename, but it's ok if that fails. The file doesn't // have to exist. let path = path_dir.join(path.file_name().unwrap_or(path.as_os_str())); diff --git a/crates/nu-cli/src/eval_file.rs b/crates/nu-cli/src/eval_file.rs index ff6ba36fe3..063914c2dd 100644 --- a/crates/nu-cli/src/eval_file.rs +++ b/crates/nu-cli/src/eval_file.rs @@ -8,6 +8,7 @@ use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, report_error, PipelineData, ShellError, Span, Value, }; +use real_parent::PathExt; use std::sync::Arc; /// Entry point for evaluating a file. @@ -49,9 +50,9 @@ pub fn evaluate_file( engine_state.file = Some(file_path.clone()); let parent = file_path - .parent() - .ok_or_else(|| ShellError::FileNotFoundCustom { - msg: format!("The file path '{file_path_str}' does not have a parent"), + .real_parent() + .map_err(|e| ShellError::FileNotFoundCustom { + msg: format!("Cannot determine parent for file path '{file_path_str}': {e}"), span: Span::unknown(), })?; diff --git a/crates/nu-cmd-lang/Cargo.toml b/crates/nu-cmd-lang/Cargo.toml index 16ac1b893a..04f7412897 100644 --- a/crates/nu-cmd-lang/Cargo.toml +++ b/crates/nu-cmd-lang/Cargo.toml @@ -18,6 +18,7 @@ nu-protocol = { path = "../nu-protocol", version = "0.95.1" } nu-utils = { path = "../nu-utils", version = "0.95.1" } itertools = { workspace = true } +real_parent = { workspace = true } shadow-rs = { version = "0.29", default-features = false } [build-dependencies] diff --git a/crates/nu-cmd-lang/src/core_commands/use_.rs b/crates/nu-cmd-lang/src/core_commands/use_.rs index b0f3648304..01b74334fe 100644 --- a/crates/nu-cmd-lang/src/core_commands/use_.rs +++ b/crates/nu-cmd-lang/src/core_commands/use_.rs @@ -5,6 +5,7 @@ use nu_protocol::{ ast::{Expr, Expression}, engine::CommandType, }; +use real_parent::PathExt; #[derive(Clone)] pub struct Use; @@ -103,7 +104,7 @@ This command is a parser keyword. For details, check: )?; let maybe_parent = maybe_file_path .as_ref() - .and_then(|path| path.parent().map(|p| p.to_path_buf())); + .and_then(|path| path.real_parent().ok()); let mut callee_stack = caller_stack .gather_captures(engine_state, &block.captures) diff --git a/crates/nu-command/Cargo.toml b/crates/nu-command/Cargo.toml index 474742ae0a..dab6d812e4 100644 --- a/crates/nu-command/Cargo.toml +++ b/crates/nu-command/Cargo.toml @@ -72,6 +72,7 @@ print-positions = { workspace = true } quick-xml = { workspace = true } rand = { workspace = true } rayon = { workspace = true } +real_parent = { workspace = true } regex = { workspace = true } roxmltree = { workspace = true } rusqlite = { workspace = true, features = ["bundled", "backup", "chrono"], optional = true } diff --git a/crates/nu-command/src/debug/info.rs b/crates/nu-command/src/debug/info.rs index 76317ed866..0b6f35bfba 100644 --- a/crates/nu-command/src/debug/info.rs +++ b/crates/nu-command/src/debug/info.rs @@ -1,4 +1,5 @@ use nu_engine::command_prelude::*; +use real_parent::PathExt; use sysinfo::{MemoryRefreshKind, Pid, ProcessRefreshKind, RefreshKind, System}; const ENV_PATH_SEPARATOR_CHAR: char = { @@ -80,7 +81,7 @@ fn all_columns(span: Span) -> Value { ); let process = if let Some(p) = sys.process(pid) { - let root = if let Some(path) = p.exe().and_then(|p| p.parent()) { + let root = if let Some(path) = p.exe().and_then(|p| p.real_parent().ok()) { Value::string(path.to_string_lossy().to_string(), span) } else { Value::nothing(span) diff --git a/crates/nu-command/src/env/source_env.rs b/crates/nu-command/src/env/source_env.rs index 0d8b118e8d..6777dc2263 100644 --- a/crates/nu-command/src/env/source_env.rs +++ b/crates/nu-command/src/env/source_env.rs @@ -3,6 +3,7 @@ use nu_engine::{ redirect_env, }; use nu_protocol::engine::CommandType; +use real_parent::PathExt; use std::path::PathBuf; /// Source a file for environment variables. @@ -66,7 +67,7 @@ impl Command for SourceEnv { }); }; - if let Some(parent) = file_path.parent() { + if let Ok(parent) = file_path.real_parent() { let file_pwd = Value::string(parent.to_string_lossy(), call.head); caller_stack.add_env_var("FILE_PWD".to_string(), file_pwd); diff --git a/crates/nu-engine/Cargo.toml b/crates/nu-engine/Cargo.toml index 3e6c3f787b..ef149e8d94 100644 --- a/crates/nu-engine/Cargo.toml +++ b/crates/nu-engine/Cargo.toml @@ -15,6 +15,7 @@ nu-protocol = { path = "../nu-protocol", features = ["plugin"], version = "0.95. nu-path = { path = "../nu-path", version = "0.95.1" } nu-glob = { path = "../nu-glob", version = "0.95.1" } nu-utils = { path = "../nu-utils", version = "0.95.1" } +real_parent = { workspace = true } [features] plugin = [] \ No newline at end of file diff --git a/crates/nu-engine/src/glob_from.rs b/crates/nu-engine/src/glob_from.rs index 2847a3c5b4..6864e069a7 100644 --- a/crates/nu-engine/src/glob_from.rs +++ b/crates/nu-engine/src/glob_from.rs @@ -1,6 +1,7 @@ use nu_glob::MatchOptions; use nu_path::{canonicalize_with, expand_path_with}; use nu_protocol::{NuGlob, ShellError, Span, Spanned}; +use real_parent::PathExt; use std::{ fs, path::{Component, Path, PathBuf}, @@ -87,7 +88,7 @@ pub fn glob_from( span: pattern.span, }); }; - (path.parent().map(|parent| parent.to_path_buf()), path) + (path.real_parent().ok(), path) } }; diff --git a/crates/nu-parser/Cargo.toml b/crates/nu-parser/Cargo.toml index b06e81387d..9888144215 100644 --- a/crates/nu-parser/Cargo.toml +++ b/crates/nu-parser/Cargo.toml @@ -21,6 +21,7 @@ bytesize = { workspace = true } chrono = { default-features = false, features = ['std'], workspace = true } itertools = { workspace = true } log = { workspace = true } +real_parent = { workspace = true } serde_json = { workspace = true } [dev-dependencies] diff --git a/crates/nu-plugin-engine/Cargo.toml b/crates/nu-plugin-engine/Cargo.toml index f86c1ae703..583c4d6ee3 100644 --- a/crates/nu-plugin-engine/Cargo.toml +++ b/crates/nu-plugin-engine/Cargo.toml @@ -17,6 +17,7 @@ nu-system = { path = "../nu-system", version = "0.95.1" } nu-plugin-protocol = { path = "../nu-plugin-protocol", version = "0.95.1" } nu-plugin-core = { path = "../nu-plugin-core", version = "0.95.1", default-features = false } +real_parent = { workspace = true } serde = { workspace = true } log = { workspace = true } diff --git a/crates/nu-plugin-engine/src/init.rs b/crates/nu-plugin-engine/src/init.rs index 44fa4f031f..b0c35435a0 100644 --- a/crates/nu-plugin-engine/src/init.rs +++ b/crates/nu-plugin-engine/src/init.rs @@ -1,3 +1,4 @@ +use real_parent::PathExt; use std::{ io::{BufReader, BufWriter}, path::Path, @@ -92,8 +93,8 @@ pub fn create_command( // In order to make bugs with improper use of filesystem without getting the engine current // directory more obvious, the plugin always starts in the directory of its executable - if let Some(dirname) = path.parent() { - process.current_dir(dirname); + if let Ok(dirname) = path.real_parent() { + process.current_dir(&dirname); } process diff --git a/crates/nu-protocol/Cargo.toml b/crates/nu-protocol/Cargo.toml index ee0f5a8221..01ae527b2b 100644 --- a/crates/nu-protocol/Cargo.toml +++ b/crates/nu-protocol/Cargo.toml @@ -28,6 +28,7 @@ indexmap = { workspace = true } lru = { workspace = true } miette = { workspace = true, features = ["fancy-no-backtrace"] } num-format = { workspace = true } +real_parent = { workspace = true } rmp-serde = { workspace = true, optional = true } serde = { workspace = true, default-features = false } thiserror = "1.0" diff --git a/crates/nu-protocol/src/parser_path.rs b/crates/nu-protocol/src/parser_path.rs index 4935d9fbe2..bde5c63203 100644 --- a/crates/nu-protocol/src/parser_path.rs +++ b/crates/nu-protocol/src/parser_path.rs @@ -1,4 +1,5 @@ use crate::engine::{StateWorkingSet, VirtualPath}; +use real_parent::PathExt; use std::{ ffi::OsStr, path::{Path, PathBuf}, @@ -57,11 +58,11 @@ impl ParserPath { } } - pub fn parent(&self) -> Option<&Path> { + pub fn parent(&self) -> Option { match self { - ParserPath::RealPath(p) => p.parent(), - ParserPath::VirtualFile(p, _) => p.parent(), - ParserPath::VirtualDir(p, _) => p.parent(), + ParserPath::RealPath(p) => p.real_parent().ok(), + ParserPath::VirtualFile(p, _) => p.parent().map(|p| p.to_path_buf()), + ParserPath::VirtualDir(p, _) => p.parent().map(|p| p.to_path_buf()), } } diff --git a/crates/nu-system/Cargo.toml b/crates/nu-system/Cargo.toml index 19ab1259fb..264ac1a249 100644 --- a/crates/nu-system/Cargo.toml +++ b/crates/nu-system/Cargo.toml @@ -27,6 +27,7 @@ procfs = { workspace = true } [target.'cfg(target_os = "macos")'.dependencies] libproc = { workspace = true } mach2 = { workspace = true } +real_parent = { workspace = true } [target.'cfg(target_os = "windows")'.dependencies] chrono = { workspace = true, default-features = false, features = ["clock"] } diff --git a/crates/nu-system/src/macos.rs b/crates/nu-system/src/macos.rs index 06683da6fd..66fe546c01 100644 --- a/crates/nu-system/src/macos.rs +++ b/crates/nu-system/src/macos.rs @@ -8,6 +8,7 @@ use libproc::libproc::task_info::{TaskAllInfo, TaskInfo}; use libproc::libproc::thread_info::ThreadInfo; use libproc::processes::{pids_by_type, ProcFilter}; use mach2::mach_time; +use real_parent::PathExt; use std::cmp; use std::path::{Path, PathBuf}; use std::thread; @@ -190,8 +191,8 @@ fn get_path_info(pid: i32, mut size: size_t) -> Option { let mut need_root = true; let mut root = Default::default(); if exe.is_absolute() { - if let Some(parent) = exe.parent() { - root = parent.to_path_buf(); + if let Ok(parent) = exe.real_parent() { + root = parent; need_root = false; } } diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index e83b4354da..7eda30aebd 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -349,8 +349,10 @@ where temp_file }); - // We don't have to write the plugin registry file, it's ok for it to not exist + // We have to write the plugin registry file, it must exist now we're using real_parent let temp_plugin_file = temp.path().join("plugin.msgpackz"); + std::fs::File::create(&temp_plugin_file) + .expect("couldn't create temporary plugin registry file"); crate::commands::ensure_plugins_built(); diff --git a/crates/nu_plugin_gstat/Cargo.toml b/crates/nu_plugin_gstat/Cargo.toml index da1c13a88e..ae3e620950 100644 --- a/crates/nu_plugin_gstat/Cargo.toml +++ b/crates/nu_plugin_gstat/Cargo.toml @@ -18,5 +18,6 @@ bench = false [dependencies] nu-plugin = { path = "../nu-plugin", version = "0.95.1" } nu-protocol = { path = "../nu-protocol", version = "0.95.1" } +real_parent = { workspace = true } git2 = "0.19" \ No newline at end of file diff --git a/crates/nu_plugin_gstat/src/gstat.rs b/crates/nu_plugin_gstat/src/gstat.rs index 849531683d..f2711336cf 100644 --- a/crates/nu_plugin_gstat/src/gstat.rs +++ b/crates/nu_plugin_gstat/src/gstat.rs @@ -1,5 +1,6 @@ use git2::{Branch, BranchType, DescribeOptions, Repository}; use nu_protocol::{record, IntoSpanned, LabeledError, Span, Spanned, Value}; +use real_parent::PathExt; use std::{fmt::Write, ops::BitAnd, path::Path}; // git status @@ -91,8 +92,9 @@ impl GStat { let repo_name = repo .path() - .parent() - .and_then(|p| p.file_name()) + .real_parent() + .map_err(|e| LabeledError::new(e.to_string()))? + .file_name() .map(|p| p.to_string_lossy().to_string()) .unwrap_or_else(|| "".to_string()); diff --git a/tests/plugins/registry_file.rs b/tests/plugins/registry_file.rs index c0f0fa0724..bc3a0328fd 100644 --- a/tests/plugins/registry_file.rs +++ b/tests/plugins/registry_file.rs @@ -1,3 +1,4 @@ +use real_parent::PathExt; use std::{fs::File, path::PathBuf}; use nu_protocol::{PluginRegistryFile, PluginRegistryItem, PluginRegistryItemData}; @@ -49,7 +50,7 @@ fn plugin_add_then_restart_nu() { fn plugin_add_in_nu_plugin_dirs_const() { let example_plugin_path = example_plugin_path(); - let dirname = example_plugin_path.parent().expect("no parent"); + let dirname = example_plugin_path.real_parent().expect("no parent"); let filename = example_plugin_path .file_name() .expect("no file_name") @@ -84,7 +85,7 @@ fn plugin_add_in_nu_plugin_dirs_const() { fn plugin_add_in_nu_plugin_dirs_env() { let example_plugin_path = example_plugin_path(); - let dirname = example_plugin_path.parent().expect("no parent"); + let dirname = example_plugin_path.real_parent().expect("no parent"); let filename = example_plugin_path .file_name() .expect("no file_name")