Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #12391: mkdir uses process startup directory instead of current script directory #12394

Merged
merged 2 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Next Next commit
Fix #12391: mkdir uses process startup directory instead of current s…
…cript directory

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.
  • Loading branch information
devyn committed Apr 4, 2024
commit 8b0024553085118fe5b67f4e393417c020a783c7
26 changes: 18 additions & 8 deletions crates/nu-command/src/filesystem/touch.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -58,7 +66,7 @@ impl Command for Touch {
let mut change_atime: bool = call.has_flag(engine_state, stack, "access")?;
let reference: Option<Spanned<String>> = call.get_flag(engine_state, stack, "reference")?;
let no_create: bool = call.has_flag(engine_state, stack, "no-create")?;
let files: Vec<String> = call.rest(engine_state, stack, 0)?;
let files: Vec<Spanned<NuGlob>> = get_rest_for_glob_pattern(engine_state, stack, call, 0)?;

if files.is_empty() {
return Err(ShellError::MissingParameter {
Expand Down Expand Up @@ -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() {
Expand All @@ -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}"),
Expand All @@ -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}"),
Expand All @@ -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}"),
Expand Down
13 changes: 7 additions & 6 deletions crates/nu-command/src/filesystem/umkdir.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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(
Expand All @@ -57,10 +58,10 @@ impl Command for UMkdir {
call: &Call,
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
let mut directories = call
.rest::<String>(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")?;
Expand Down
13 changes: 13 additions & 0 deletions crates/nu-command/tests/commands/touch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
})
}
14 changes: 14 additions & 0 deletions crates/nu-command/tests/commands/umkdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down