From 8b0024553085118fe5b67f4e393417c020a783c7 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Thu, 4 Apr 2024 02:10:45 -0700 Subject: [PATCH 1/2] Fix #12391: mkdir uses process startup directory instead of current script directory nushell/nushell@87c5f6e455 accidentally introduced a bug where the path was not being properly expanded according to the cwd. This makes both 'touch' and 'mkdir' use globs just like the rest of the commands to preserve tilde behavior while still expanding the paths properly. --- crates/nu-command/src/filesystem/touch.rs | 26 +++++++++++++++------- crates/nu-command/src/filesystem/umkdir.rs | 13 ++++++----- crates/nu-command/tests/commands/touch.rs | 13 +++++++++++ crates/nu-command/tests/commands/umkdir.rs | 14 ++++++++++++ 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/crates/nu-command/src/filesystem/touch.rs b/crates/nu-command/src/filesystem/touch.rs index f9e6cc10ed37..f92e3ea4f75f 100644 --- a/crates/nu-command/src/filesystem/touch.rs +++ b/crates/nu-command/src/filesystem/touch.rs @@ -1,8 +1,12 @@ use filetime::FileTime; -use nu_engine::command_prelude::*; +use nu_engine::{command_prelude::*, current_dir}; +use nu_path::expand_path_with; +use nu_protocol::NuGlob; use std::{fs::OpenOptions, path::Path, time::SystemTime}; +use super::util::get_rest_for_glob_pattern; + #[derive(Clone)] pub struct Touch; @@ -18,7 +22,11 @@ impl Command for Touch { fn signature(&self) -> Signature { Signature::build("touch") .input_output_types(vec![(Type::Nothing, Type::Nothing)]) - .rest("files", SyntaxShape::Filepath, "The file(s) to create.") + .rest( + "files", + SyntaxShape::OneOf(vec![SyntaxShape::GlobPattern, SyntaxShape::Filepath]), + "The file(s) to create." + ) .named( "reference", SyntaxShape::String, @@ -58,7 +66,7 @@ impl Command for Touch { let mut change_atime: bool = call.has_flag(engine_state, stack, "access")?; let reference: Option> = call.get_flag(engine_state, stack, "reference")?; let no_create: bool = call.has_flag(engine_state, stack, "no-create")?; - let files: Vec = call.rest(engine_state, stack, 0)?; + let files: Vec> = get_rest_for_glob_pattern(engine_state, stack, call, 0)?; if files.is_empty() { return Err(ShellError::MissingParameter { @@ -105,8 +113,10 @@ impl Command for Touch { })?; } - for (index, item) in files.into_iter().enumerate() { - let path = Path::new(&item); + let cwd = current_dir(engine_state, stack)?; + + for (index, glob) in files.into_iter().enumerate() { + let path = expand_path_with(glob.item.as_ref(), &cwd, glob.item.is_expand()); // If --no-create is passed and the file/dir does not exist there's nothing to do if no_create && !path.exists() { @@ -119,7 +129,7 @@ impl Command for Touch { .write(true) .create(true) .truncate(false) - .open(path) + .open(&path) { return Err(ShellError::CreateNotPossible { msg: format!("Failed to create file: {err}"), @@ -132,7 +142,7 @@ impl Command for Touch { } if change_mtime { - if let Err(err) = filetime::set_file_mtime(&item, FileTime::from_system_time(mtime)) + if let Err(err) = filetime::set_file_mtime(&path, FileTime::from_system_time(mtime)) { return Err(ShellError::ChangeModifiedTimeNotPossible { msg: format!("Failed to change the modified time: {err}"), @@ -145,7 +155,7 @@ impl Command for Touch { } if change_atime { - if let Err(err) = filetime::set_file_atime(&item, FileTime::from_system_time(atime)) + if let Err(err) = filetime::set_file_atime(&path, FileTime::from_system_time(atime)) { return Err(ShellError::ChangeAccessTimeNotPossible { msg: format!("Failed to change the access time: {err}"), diff --git a/crates/nu-command/src/filesystem/umkdir.rs b/crates/nu-command/src/filesystem/umkdir.rs index 949e6ba794f2..5b8d8c33cdc3 100644 --- a/crates/nu-command/src/filesystem/umkdir.rs +++ b/crates/nu-command/src/filesystem/umkdir.rs @@ -1,10 +1,11 @@ -use nu_engine::command_prelude::*; +use nu_engine::{command_prelude::*, current_dir}; -use std::path::PathBuf; use uu_mkdir::mkdir; #[cfg(not(windows))] use uucore::mode; +use super::util::get_rest_for_glob_pattern; + #[derive(Clone)] pub struct UMkdir; @@ -39,7 +40,7 @@ impl Command for UMkdir { .input_output_types(vec![(Type::Nothing, Type::Nothing)]) .rest( "rest", - SyntaxShape::Directory, + SyntaxShape::OneOf(vec![SyntaxShape::GlobPattern, SyntaxShape::Directory]), "The name(s) of the path(s) to create.", ) .switch( @@ -57,10 +58,10 @@ impl Command for UMkdir { call: &Call, _input: PipelineData, ) -> Result { - let mut directories = call - .rest::(engine_state, stack, 0)? + let cwd = current_dir(engine_state, stack)?; + let mut directories = get_rest_for_glob_pattern(engine_state, stack, call, 0)? .into_iter() - .map(PathBuf::from) + .map(|dir| nu_path::expand_path_with(dir.item.as_ref(), &cwd, dir.item.is_expand())) .peekable(); let is_verbose = call.has_flag(engine_state, stack, "verbose")?; diff --git a/crates/nu-command/tests/commands/touch.rs b/crates/nu-command/tests/commands/touch.rs index f5b3d6f611f3..f91b525b9874 100644 --- a/crates/nu-command/tests/commands/touch.rs +++ b/crates/nu-command/tests/commands/touch.rs @@ -502,3 +502,16 @@ fn create_a_file_with_tilde() { assert!(files_exist_at(vec![Path::new("~tilde2")], dirs.test())); }) } + +#[test] +fn respects_cwd() { + Playground::setup("touch_respects_cwd", |dirs, _sandbox| { + nu!( + cwd: dirs.test(), + "mkdir dir; cd dir; touch i_will_be_created.txt" + ); + + let path = dirs.test().join("dir/i_will_be_created.txt"); + assert!(path.exists()); + }) +} diff --git a/crates/nu-command/tests/commands/umkdir.rs b/crates/nu-command/tests/commands/umkdir.rs index ebb2b932abb3..138192b9ed88 100644 --- a/crates/nu-command/tests/commands/umkdir.rs +++ b/crates/nu-command/tests/commands/umkdir.rs @@ -123,6 +123,20 @@ fn creates_directory_three_dots_quotation_marks() { }) } +#[test] +fn respects_cwd() { + Playground::setup("mkdir_respects_cwd", |dirs, _| { + nu!( + cwd: dirs.test(), + "mkdir some_folder; cd some_folder; mkdir another/deeper_one" + ); + + let expected = dirs.test().join("some_folder/another/deeper_one"); + + assert!(expected.exists()); + }) +} + #[cfg(not(windows))] #[test] fn mkdir_umask_permission() { From 8fbccfedf637772fc0b371f085eb737e6d142781 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Thu, 4 Apr 2024 04:30:40 -0700 Subject: [PATCH 2/2] fix the tests to actually cause the bug --- crates/nu-command/tests/commands/touch.rs | 2 +- crates/nu-command/tests/commands/umkdir.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/nu-command/tests/commands/touch.rs b/crates/nu-command/tests/commands/touch.rs index f91b525b9874..d7f5cb875a95 100644 --- a/crates/nu-command/tests/commands/touch.rs +++ b/crates/nu-command/tests/commands/touch.rs @@ -508,7 +508,7 @@ fn respects_cwd() { Playground::setup("touch_respects_cwd", |dirs, _sandbox| { nu!( cwd: dirs.test(), - "mkdir dir; cd dir; touch i_will_be_created.txt" + "mkdir 'dir'; cd 'dir'; touch 'i_will_be_created.txt'" ); let path = dirs.test().join("dir/i_will_be_created.txt"); diff --git a/crates/nu-command/tests/commands/umkdir.rs b/crates/nu-command/tests/commands/umkdir.rs index 138192b9ed88..3ca1b3bdfb12 100644 --- a/crates/nu-command/tests/commands/umkdir.rs +++ b/crates/nu-command/tests/commands/umkdir.rs @@ -128,7 +128,7 @@ fn respects_cwd() { Playground::setup("mkdir_respects_cwd", |dirs, _| { nu!( cwd: dirs.test(), - "mkdir some_folder; cd some_folder; mkdir another/deeper_one" + "mkdir 'some_folder'; cd 'some_folder'; mkdir 'another/deeper_one'" ); let expected = dirs.test().join("some_folder/another/deeper_one");