From 18da32f394dfa0519bfb40921c3ac2965eab3c58 Mon Sep 17 00:00:00 2001 From: Joseph Crail Date: Wed, 13 May 2015 21:05:36 -0400 Subject: [PATCH] Fix mv. --- deps/Cargo.toml | 2 +- src/mv/deps.mk | 1 + src/mv/mv.rs | 77 +++++++++++-------------- test/mv.rs | 147 +++++++++++++++++++++++++----------------------- 4 files changed, 113 insertions(+), 114 deletions(-) create mode 100644 src/mv/deps.mk diff --git a/deps/Cargo.toml b/deps/Cargo.toml index 0915db569..7c6086937 100644 --- a/deps/Cargo.toml +++ b/deps/Cargo.toml @@ -6,7 +6,7 @@ version = "0.0.0" name = "null" [dependencies] -libc = "0.1.6" +libc = "0.1.7" num_cpus = "*" rand = "0.3.8" regex = "0.1.30" diff --git a/src/mv/deps.mk b/src/mv/deps.mk new file mode 100644 index 000000000..b6534caec --- /dev/null +++ b/src/mv/deps.mk @@ -0,0 +1 @@ +DEPLIBS += time diff --git a/src/mv/mv.rs b/src/mv/mv.rs index 5ee0a162d..ed9873047 100644 --- a/src/mv/mv.rs +++ b/src/mv/mv.rs @@ -1,5 +1,6 @@ #![crate_name = "mv"] -#![feature(collections, core, old_io, old_path, rustc_private)] +#![feature(collections, fs_time, path_ext, rustc_private, slice_patterns, str_char)] +#![allow(deprecated)] /* * This file is part of the uutils coreutils package. @@ -12,26 +13,19 @@ */ extern crate getopts; +extern crate libc; -use std::old_io::{BufferedReader, IoResult, fs}; -use std::old_io::stdio::stdin_raw; -use std::old_io::fs::PathExtensions; -use std::old_path::GenericPath; -use getopts::{ - getopts, - optflag, - optflagopt, - optopt, - usage, -}; -use std::borrow::ToOwned; +use getopts::{getopts, optflag, optflagopt, optopt, usage}; +use std::fs::{self, PathExt}; +use std::io::{BufRead, BufReader, Result, stdin, Write}; +use std::path::PathBuf; #[path = "../common/util.rs"] #[macro_use] mod util; static NAME: &'static str = "mv"; -static VERSION: &'static str = "0.0.1"; +static VERSION: &'static str = "0.0.1"; pub struct Behaviour { overwrite: OverwriteMode, @@ -43,16 +37,14 @@ pub struct Behaviour { verbose: bool, } -#[derive(Eq, PartialEq)] +#[derive(Clone, Eq, PartialEq)] pub enum OverwriteMode { NoClobber, Interactive, Force, } -impl Copy for OverwriteMode {} - -#[derive(Eq, PartialEq)] +#[derive(Clone, Eq, PartialEq)] pub enum BackupMode { NoBackup, SimpleBackup, @@ -60,10 +52,8 @@ pub enum BackupMode { ExistingBackup, } -impl Copy for BackupMode {} - pub fn uumain(args: Vec) -> i32 { - let program = args[0].as_slice(); + let program = &args[0]; let opts = [ optflagopt("", "backup", "make a backup of each existing destination file", "CONTROL"), optflag("b", "", "like --backup but does not accept an argument"), @@ -84,7 +74,7 @@ pub fn uumain(args: Vec) -> i32 { optflag("V", "version", "output version information and exit"), ]; - let matches = match getopts(args.tail(), &opts) { + let matches = match getopts(&args[1..], &opts) { Ok(m) => m, Err(f) => { show_error!("Invalid options\n{}", f); @@ -111,7 +101,7 @@ pub fn uumain(args: Vec) -> i32 { } else if matches.opt_present("backup") { match matches.opt_str("backup") { None => BackupMode::SimpleBackup, - Some(mode) => match mode.as_slice() { + Some(mode) => match &mode[..] { "simple" | "never" => BackupMode::SimpleBackup, "numbered" | "t" => BackupMode::NumberedBackup, "existing" | "nil" => BackupMode::ExistingBackup, @@ -143,7 +133,7 @@ pub fn uumain(args: Vec) -> i32 { } } } else { - "~".to_owned() + "~".to_string() }; if matches.opt_present("T") && matches.opt_present("t") { @@ -161,17 +151,17 @@ pub fn uumain(args: Vec) -> i32 { verbose: matches.opt_present("v"), }; - let string_to_path = |s: &String| { Path::new(s.as_slice()) }; - let paths: Vec = matches.free.iter().map(string_to_path).collect(); + let string_to_path = |s: &String| { PathBuf::from(s) }; + let paths: Vec = matches.free.iter().map(string_to_path).collect(); if matches.opt_present("version") { version(); 0 } else if matches.opt_present("help") { - help(program.as_slice(), usage.as_slice()); + help(program, &usage); 0 } else { - exec(paths.as_slice(), behaviour) + exec(&paths[..], behaviour) } } @@ -187,9 +177,9 @@ fn help(progname: &str, usage: &str) { println!("{}", msg); } -fn exec(files: &[Path], b: Behaviour) -> i32 { +fn exec(files: &[PathBuf], b: Behaviour) -> i32 { match b.target_dir { - Some(ref name) => return move_files_into_dir(files, &Path::new(name.as_slice()), &b), + Some(ref name) => return move_files_into_dir(files, &PathBuf::from(name), &b), None => {} } match files { @@ -245,7 +235,7 @@ fn exec(files: &[Path], b: Behaviour) -> i32 { 0 } -fn move_files_into_dir(files: &[Path], target_dir: &Path, b: &Behaviour) -> i32 { +fn move_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behaviour) -> i32 { if !target_dir.is_dir() { show_error!("target ‘{}’ is not a directory", target_dir.display()); return 1; @@ -253,7 +243,7 @@ fn move_files_into_dir(files: &[Path], target_dir: &Path, b: &Behaviour) -> i32 let mut all_successful = true; for sourcepath in files.iter() { - let targetpath = match sourcepath.filename_str() { + let targetpath = match sourcepath.as_os_str().to_str() { Some(name) => target_dir.join(name), None => { show_error!("cannot stat ‘{}’: No such file or directory", @@ -276,7 +266,7 @@ fn move_files_into_dir(files: &[Path], target_dir: &Path, b: &Behaviour) -> i32 if all_successful { 0 } else { 1 } } -fn rename(from: &Path, to: &Path, b: &Behaviour) -> IoResult<()> { +fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> { let mut backup_path = None; if to.exists() { @@ -302,7 +292,7 @@ fn rename(from: &Path, to: &Path, b: &Behaviour) -> IoResult<()> { } if b.update { - if try!(from.stat()).modified <= try!(to.stat()).modified { + if try!(fs::metadata(from)).modified() <= try!(fs::metadata(to)).modified() { return Ok(()); } } @@ -321,8 +311,9 @@ fn rename(from: &Path, to: &Path, b: &Behaviour) -> IoResult<()> { } fn read_yes() -> bool { - match BufferedReader::new(stdin_raw()).read_line() { - Ok(s) => match s.as_slice().slice_shift_char() { + let mut s = String::new(); + match BufReader::new(stdin()).read_line(&mut s) { + Ok(_) => match s.slice_shift_char() { Some((x, _)) => x == 'y' || x == 'Y', _ => false }, @@ -330,13 +321,13 @@ fn read_yes() -> bool { } } -fn simple_backup_path(path: &Path, suffix: &String) -> Path { - let mut p = path.clone().into_vec(); - p.push_all(suffix.as_slice().as_bytes()); - return Path::new(p); +fn simple_backup_path(path: &PathBuf, suffix: &String) -> PathBuf { + let mut p = path.as_os_str().to_str().unwrap().to_string(); + p.push_str(suffix); + return PathBuf::from(p); } -fn numbered_backup_path(path: &Path) -> Path { +fn numbered_backup_path(path: &PathBuf) -> PathBuf { let mut i: u64 = 1; loop { let new_path = simple_backup_path(path, &format!(".~{}~", i)); @@ -347,8 +338,8 @@ fn numbered_backup_path(path: &Path) -> Path { } } -fn existing_backup_path(path: &Path, suffix: &String) -> Path { - let test_path = simple_backup_path(path, &".~1~".to_owned()); +fn existing_backup_path(path: &PathBuf, suffix: &String) -> PathBuf { + let test_path = simple_backup_path(path, &".~1~".to_string()); if test_path.exists() { return numbered_backup_path(path); } diff --git a/test/mv.rs b/test/mv.rs index 906cb3523..b074fe300 100644 --- a/test/mv.rs +++ b/test/mv.rs @@ -1,15 +1,15 @@ -#![allow(unstable)] +#![feature(fs_time, path_ext)] +extern crate libc; extern crate time; -use std::old_io::{process, fs, FilePermission}; -use std::old_io::process::Command; -use std::old_io::fs::PathExtensions; +use std::fs::{self, File, PathExt}; +use std::io::Write; +use std::path::Path; +use std::process::{Command, Stdio}; use std::str::from_utf8; -use std::borrow::ToOwned; - -static EXE: &'static str = "./mv"; +static PROGNAME: &'static str = "./mv"; macro_rules! assert_empty_stderr( ($cond:expr) => ( @@ -18,40 +18,50 @@ macro_rules! assert_empty_stderr( } ); ); + struct CmdResult { success: bool, stderr: String, stdout: String, } + fn run(cmd: &mut Command) -> CmdResult { - let prog = cmd.spawn().unwrap().wait_with_output().unwrap(); + let prog = cmd.output().unwrap(); CmdResult { success: prog.status.success(), - stderr: from_utf8(prog.error.as_slice()).unwrap().to_owned(), - stdout: from_utf8(prog.output.as_slice()).unwrap().to_owned(), + stderr: from_utf8(&prog.stderr).unwrap().to_string(), + stdout: from_utf8(&prog.stdout).unwrap().to_string(), } } -fn run_interactive(cmd: &mut Command, input: &[u8])-> CmdResult { - let stdin_cfg = process::CreatePipe(true, false); - let mut command = cmd.stdin(stdin_cfg).spawn().unwrap(); - command.stdin.as_mut().unwrap().write_all(input); +fn run_interactive(cmd: &mut Command, input: &[u8])-> CmdResult { + let mut command = cmd + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .unwrap(); + + command.stdin + .take() + .unwrap_or_else(|| panic!("Could not take child process stdin")) + .write_all(input) + .unwrap_or_else(|e| panic!("{}", e)); let prog = command.wait_with_output().unwrap(); CmdResult { success: prog.status.success(), - stderr: from_utf8(prog.error.as_slice()).unwrap().to_owned(), - stdout: from_utf8(prog.output.as_slice()).unwrap().to_owned(), + stderr: from_utf8(&prog.stderr).unwrap().to_string(), + stdout: from_utf8(&prog.stdout).unwrap().to_string(), } } fn mkdir(dir: &str) { - fs::mkdir(&Path::new(dir), FilePermission::from_bits_truncate(0o755 as u32)).unwrap(); -} -fn touch(file: &str) { - fs::File::create(&Path::new(file)).unwrap(); + fs::create_dir(Path::new(dir)).unwrap(); } +fn touch(file: &str) { + File::create(Path::new(file)).unwrap(); +} #[test] fn test_mv_rename_dir() { @@ -60,7 +70,7 @@ fn test_mv_rename_dir() { mkdir(dir1); - let result = run(Command::new(EXE).arg(dir1).arg(dir2)); + let result = run(Command::new(PROGNAME).arg(dir1).arg(dir2)); assert_empty_stderr!(result); assert!(result.success); @@ -74,7 +84,7 @@ fn test_mv_rename_file() { touch(file1); - let result = run(Command::new(EXE).arg(file1).arg(file2)); + let result = run(Command::new(PROGNAME).arg(file1).arg(file2)); assert_empty_stderr!(result); assert!(result.success); @@ -89,11 +99,11 @@ fn test_mv_move_file_into_dir() { mkdir(dir); touch(file); - let result = run(Command::new(EXE).arg(file).arg(dir)); + let result = run(Command::new(PROGNAME).arg(file).arg(dir)); assert_empty_stderr!(result); assert!(result.success); - assert!(Path::new(format!("{}/{}", dir, file)).is_file()); + assert!(Path::new(&format!("{}/{}", dir, file)).is_file()); } #[test] @@ -106,12 +116,12 @@ fn test_mv_multiple_files() { touch(file_a); touch(file_b); - let result = run(Command::new(EXE).arg(file_a).arg(file_b).arg(target_dir)); + let result = run(Command::new(PROGNAME).arg(file_a).arg(file_b).arg(target_dir)); assert_empty_stderr!(result); assert!(result.success); - assert!(Path::new(format!("{}/{}", target_dir, file_a)).is_file()); - assert!(Path::new(format!("{}/{}", target_dir, file_b)).is_file()); + assert!(Path::new(&format!("{}/{}", target_dir, file_a)).is_file()); + assert!(Path::new(&format!("{}/{}", target_dir, file_b)).is_file()); } #[test] @@ -124,12 +134,12 @@ fn test_mv_multiple_folders() { mkdir(dir_a); mkdir(dir_b); - let result = run(Command::new(EXE).arg(dir_a).arg(dir_b).arg(target_dir)); + let result = run(Command::new(PROGNAME).arg(dir_a).arg(dir_b).arg(target_dir)); assert_empty_stderr!(result); assert!(result.success); - assert!(Path::new(format!("{}/{}", target_dir, dir_a)).is_dir()); - assert!(Path::new(format!("{}/{}", target_dir, dir_b)).is_dir()); + assert!(Path::new(&format!("{}/{}", target_dir, dir_a)).is_dir()); + assert!(Path::new(&format!("{}/{}", target_dir, dir_b)).is_dir()); } #[test] @@ -141,7 +151,7 @@ fn test_mv_interactive() { touch(file_b); - let result1 = run_interactive(Command::new(EXE).arg("-i").arg(file_a).arg(file_b), b"n"); + let result1 = run_interactive(Command::new(PROGNAME).arg("-i").arg(file_a).arg(file_b), b"n"); assert_empty_stderr!(result1); assert!(result1.success); @@ -150,7 +160,7 @@ fn test_mv_interactive() { assert!(Path::new(file_b).is_file()); - let result2 = run_interactive(Command::new(EXE).arg("-i").arg(file_a).arg(file_b), b"Yesh"); + let result2 = run_interactive(Command::new(PROGNAME).arg("-i").arg(file_a).arg(file_b), b"Yesh"); assert_empty_stderr!(result2); assert!(result2.success); @@ -167,7 +177,7 @@ fn test_mv_no_clobber() { touch(file_a); touch(file_b); - let result = run(Command::new(EXE).arg("-n").arg(file_a).arg(file_b)); + let result = run(Command::new(PROGNAME).arg("-n").arg(file_a).arg(file_b)); assert_empty_stderr!(result); assert!(result.success); @@ -183,7 +193,7 @@ fn test_mv_replace_file() { touch(file_a); touch(file_b); - let result = run(Command::new(EXE).arg(file_a).arg(file_b)); + let result = run(Command::new(PROGNAME).arg(file_a).arg(file_b)); assert_empty_stderr!(result); assert!(result.success); @@ -199,7 +209,7 @@ fn test_mv_force_replace_file() { touch(file_a); touch(file_b); - let result = run(Command::new(EXE).arg("--force").arg(file_a).arg(file_b)); + let result = run(Command::new(PROGNAME).arg("--force").arg(file_a).arg(file_b)); assert_empty_stderr!(result); assert!(result.success); @@ -214,14 +224,14 @@ fn test_mv_simple_backup() { touch(file_a); touch(file_b); - let result = run(Command::new(EXE).arg("-b").arg(file_a).arg(file_b)); + let result = run(Command::new(PROGNAME).arg("-b").arg(file_a).arg(file_b)); assert_empty_stderr!(result); assert!(result.success); assert!(!Path::new(file_a).is_file()); assert!(Path::new(file_b).is_file()); - assert!(Path::new(format!("{}~", file_b)).is_file()); + assert!(Path::new(&format!("{}~", file_b)).is_file()); } #[test] @@ -232,7 +242,7 @@ fn test_mv_custom_backup_suffix() { touch(file_a); touch(file_b); - let result = run(Command::new(EXE) + let result = run(Command::new(PROGNAME) .arg("-b").arg(format!("--suffix={}", suffix)) .arg(file_a).arg(file_b)); @@ -241,7 +251,7 @@ fn test_mv_custom_backup_suffix() { assert!(!Path::new(file_a).is_file()); assert!(Path::new(file_b).is_file()); - assert!(Path::new(format!("{}{}", file_b, suffix)).is_file()); + assert!(Path::new(&format!("{}{}", file_b, suffix)).is_file()); } #[test] @@ -251,14 +261,14 @@ fn test_mv_backup_numbering() { touch(file_a); touch(file_b); - let result = run(Command::new(EXE).arg("--backup=t").arg(file_a).arg(file_b)); + let result = run(Command::new(PROGNAME).arg("--backup=t").arg(file_a).arg(file_b)); assert_empty_stderr!(result); assert!(result.success); assert!(!Path::new(file_a).is_file()); assert!(Path::new(file_b).is_file()); - assert!(Path::new(format!("{}.~1~", file_b)).is_file()); + assert!(Path::new(&format!("{}.~1~", file_b)).is_file()); } #[test] @@ -271,7 +281,7 @@ fn test_mv_existing_backup() { touch(file_a); touch(file_b); touch(file_b_backup); - let result = run(Command::new(EXE).arg("--backup=nil").arg(file_a).arg(file_b)); + let result = run(Command::new(PROGNAME).arg("--backup=nil").arg(file_a).arg(file_b)); assert_empty_stderr!(result); assert!(result.success); @@ -290,10 +300,10 @@ fn test_mv_update_option() { touch(file_a); touch(file_b); let now = (time::get_time().sec * 1000) as u64; - fs::change_file_times(&Path::new(file_a), now, now).unwrap(); - fs::change_file_times(&Path::new(file_b), now, now+3600).unwrap(); + fs::set_file_times(Path::new(file_a), now, now).unwrap(); + fs::set_file_times(Path::new(file_b), now, now+3600).unwrap(); - let result1 = run(Command::new(EXE).arg("--update").arg(file_a).arg(file_b)); + let result1 = run(Command::new(PROGNAME).arg("--update").arg(file_a).arg(file_b)); assert_empty_stderr!(result1); assert!(result1.success); @@ -301,7 +311,7 @@ fn test_mv_update_option() { assert!(Path::new(file_a).is_file()); assert!(Path::new(file_b).is_file()); - let result2 = run(Command::new(EXE).arg("--update").arg(file_b).arg(file_a)); + let result2 = run(Command::new(PROGNAME).arg("--update").arg(file_b).arg(file_a)); assert_empty_stderr!(result2); assert!(result2.success); @@ -319,15 +329,15 @@ fn test_mv_target_dir() { touch(file_a); touch(file_b); mkdir(dir); - let result = run(Command::new(EXE).arg("-t").arg(dir).arg(file_a).arg(file_b)); + let result = run(Command::new(PROGNAME).arg("-t").arg(dir).arg(file_a).arg(file_b)); assert_empty_stderr!(result); assert!(result.success); assert!(!Path::new(file_a).is_file()); assert!(!Path::new(file_b).is_file()); - assert!(Path::new(format!("{}/{}", dir, file_a)).is_file()); - assert!(Path::new(format!("{}/{}", dir, file_b)).is_file()); + assert!(Path::new(&format!("{}/{}", dir, file_a)).is_file()); + assert!(Path::new(&format!("{}/{}", dir, file_b)).is_file()); } #[test] @@ -337,7 +347,7 @@ fn test_mv_overwrite_dir() { mkdir(dir_a); mkdir(dir_b); - let result = run(Command::new(EXE).arg("-T").arg(dir_a).arg(dir_b)); + let result = run(Command::new(PROGNAME).arg("-T").arg(dir_a).arg(dir_b)); assert_empty_stderr!(result); assert!(result.success); @@ -355,7 +365,7 @@ fn test_mv_overwrite_nonempty_dir() { mkdir(dir_a); mkdir(dir_b); touch(dummy); - let result = run(Command::new(EXE).arg("-vT").arg(dir_a).arg(dir_b)); + let result = run(Command::new(PROGNAME).arg("-vT").arg(dir_a).arg(dir_b)); // Not same error as GNU; the error message is a rust builtin // TODO: test (and implement) correct error message (or at least decide whether to do so) @@ -378,16 +388,16 @@ fn test_mv_backup_dir() { mkdir(dir_a); mkdir(dir_b); - let result = run(Command::new(EXE).arg("-vbT").arg(dir_a).arg(dir_b)); + let result = run(Command::new(PROGNAME).arg("-vbT").arg(dir_a).arg(dir_b)); assert_empty_stderr!(result); - assert_eq!(result.stdout.as_slice(), - format!("‘{}’ -> ‘{}’ (backup: ‘{}~’)\n", dir_a, dir_b, dir_b).as_slice()); + assert_eq!(result.stdout, + format!("‘{}’ -> ‘{}’ (backup: ‘{}~’)\n", dir_a, dir_b, dir_b)); assert!(result.success); assert!(!Path::new(dir_a).is_dir()); assert!(Path::new(dir_b).is_dir()); - assert!(Path::new(format!("{}~", dir_b)).is_dir()); + assert!(Path::new(&format!("{}~", dir_b)).is_dir()); } #[test] @@ -401,8 +411,8 @@ fn test_mv_errors() { // $ mv -T -t a b // mv: cannot combine --target-directory (-t) and --no-target-directory (-T) - let result = run(Command::new(EXE).arg("-T").arg("-t").arg(dir).arg(file_a).arg(file_b)); - assert_eq!(result.stderr.as_slice(), + let result = run(Command::new(PROGNAME).arg("-T").arg("-t").arg(dir).arg(file_a).arg(file_b)); + assert_eq!(result.stderr, "mv: error: cannot combine --target-directory (-t) and --no-target-directory (-T)\n"); assert!(!result.success); @@ -410,15 +420,15 @@ fn test_mv_errors() { // $ touch file && mkdir dir // $ mv -T file dir // err == mv: cannot overwrite directory ‘dir’ with non-directory - let result = run(Command::new(EXE).arg("-T").arg(file_a).arg(dir)); - assert_eq!(result.stderr.as_slice(), - format!("mv: error: cannot overwrite directory ‘{}’ with non-directory\n", dir).as_slice()); + let result = run(Command::new(PROGNAME).arg("-T").arg(file_a).arg(dir)); + assert_eq!(result.stderr, + format!("mv: error: cannot overwrite directory ‘{}’ with non-directory\n", dir)); assert!(!result.success); // $ mkdir dir && touch file // $ mv dir file // err == mv: cannot overwrite non-directory ‘file’ with directory ‘dir’ - let result = run(Command::new(EXE).arg(dir).arg(file_a)); + let result = run(Command::new(PROGNAME).arg(dir).arg(file_a)); assert!(result.stderr.len() > 0); assert!(!result.success); } @@ -432,22 +442,21 @@ fn test_mv_verbose() { touch(file_a); touch(file_b); - let result = run(Command::new(EXE).arg("-v").arg(file_a).arg(file_b)); + let result = run(Command::new(PROGNAME).arg("-v").arg(file_a).arg(file_b)); assert_empty_stderr!(result); - assert_eq!(result.stdout.as_slice(), - format!("‘{}’ -> ‘{}’\n", file_a, file_b).as_slice()); + assert_eq!(result.stdout, + format!("‘{}’ -> ‘{}’\n", file_a, file_b)); assert!(result.success); touch(file_a); - let result = run(Command::new(EXE).arg("-vb").arg(file_a).arg(file_b)); + let result = run(Command::new(PROGNAME).arg("-vb").arg(file_a).arg(file_b)); assert_empty_stderr!(result); - assert_eq!(result.stdout.as_slice(), - format!("‘{}’ -> ‘{}’ (backup: ‘{}~’)\n", file_a, file_b, file_b).as_slice()); + assert_eq!(result.stdout, + format!("‘{}’ -> ‘{}’ (backup: ‘{}~’)\n", file_a, file_b, file_b)); assert!(result.success); } - // Todo: // $ touch a b @@ -460,5 +469,3 @@ fn test_mv_verbose() { // $ mv -v a b // mv: try to overwrite ‘b’, overriding mode 0444 (r--r--r--)? y // ‘a’ -> ‘b’ - -