Skip to content

Commit

Permalink
Fix #12391: mkdir uses process startup directory instead of current s…
Browse files Browse the repository at this point in the history
…cript directory (#12394)

# Description

This fixes #12391.

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.

This doesn't actually expand the globs. Should it?

# User-Facing Changes

- Restore behavior of `mkdir`, `touch`
- Help text now says they can take globs, but they won't actually expand
them, maybe this should be changed

# Tests + Formatting

Regression tests added.


# After Submitting

This is severe enough and should be included in the point release.
  • Loading branch information
devyn committed Apr 4, 2024
1 parent 25b9074 commit 51aa66f
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 14 deletions.
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

0 comments on commit 51aa66f

Please sign in to comment.