From 9005ae7dbf15ab10c7d590a5fc54a46b506800ec Mon Sep 17 00:00:00 2001 From: Luca Ottaviano Date: Fri, 11 Mar 2016 19:35:03 +0100 Subject: [PATCH 1/5] tests/chmod: refactor tests to only create reference file once --- tests/chmod.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/chmod.rs b/tests/chmod.rs index ac476a573..5d4f893c0 100644 --- a/tests/chmod.rs +++ b/tests/chmod.rs @@ -24,18 +24,14 @@ fn mkfile(file: &str, mode: mode_t) { std::fs::set_permissions(file, perms).unwrap(); } -fn run_tests(tests: Vec) { - for test in tests { - let (at, mut ucmd) = testing(UTIL_NAME); - +fn run_single_test(test: &TestCase, at: AtPath, mut ucmd: UCommand) { mkfile(&at.plus_as_string(TEST_FILE), test.before); - mkfile(&at.plus_as_string(REFERENCE_FILE), REFERENCE_PERMS); let perms = at.metadata(TEST_FILE).permissions().mode(); if perms != test.before{ panic!(format!("{}: expected: {:o} got: {:o}", "setting permissions failed", test.after, perms)); } - for arg in test.args { + for arg in &test.args { ucmd.arg(arg); } let r = ucmd.run(); @@ -48,6 +44,12 @@ fn run_tests(tests: Vec) { if perms != test.after { panic!(format!("{:?}: expected: {:o} got: {:o}", ucmd.raw, test.after, perms)); } +} + +fn run_tests(tests: Vec) { + for test in tests { + let (at, ucmd) = testing(UTIL_NAME); + run_single_test(&test, at, ucmd); } } @@ -96,5 +98,7 @@ fn test_chmod_reference_file() { let tests = vec!{ TestCase{args: vec!{"--reference", REFERENCE_FILE, TEST_FILE}, before: 0o070, after: 0o247}, }; - run_tests(tests); + let (at, ucmd) = testing(UTIL_NAME); + mkfile(&at.plus_as_string(REFERENCE_FILE), REFERENCE_PERMS); + run_single_test(&tests[0], at, ucmd); } From 2686ea75d70e08ad9d04a82ac5cb90a5bfa7f85a Mon Sep 17 00:00:00 2001 From: Luca Ottaviano Date: Tue, 15 Mar 2016 14:18:39 +0100 Subject: [PATCH 2/5] chmod: handle -octal and -[rwx] The main issue is that -octal or -[rwx] is interpreted as an option by getopts. Search the args for such a pattern, remove it before parsing and manually handle it afterwards. Fixes #788. --- src/chmod/chmod.rs | 32 +++++++++++++++++++++++++++++--- tests/chmod.rs | 10 ++++++---- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/chmod/chmod.rs b/src/chmod/chmod.rs index 105021fea..651cb2f75 100644 --- a/src/chmod/chmod.rs +++ b/src/chmod/chmod.rs @@ -29,7 +29,7 @@ use walker::Walker; const NAME: &'static str = "chmod"; const VERSION: &'static str = env!("CARGO_PKG_VERSION"); -pub fn uumain(args: Vec) -> i32 { +pub fn uumain(mut args: Vec) -> i32 { let mut opts = Options::new(); opts.optflag("c", "changes", "like verbose but report only when a change is made (unimplemented)"); opts.optflag("f", "quiet", "suppress most error messages (unimplemented)"); // TODO: support --silent @@ -40,7 +40,27 @@ pub fn uumain(args: Vec) -> i32 { opts.optflag("R", "recursive", "change files and directories recursively"); opts.optflag("h", "help", "display this help and exit"); opts.optflag("V", "version", "output version information and exit"); - // TODO: sanitize input for - at beginning (e.g. chmod -x testfile). Solution is to add a to -x, making a-x + // sanitize input for - at beginning (e.g. chmod -x testfile). Remove + // the option and save it for later, after parsing is finished. + let mut negative_option = None; + for i in 0..args.len() { + if let Some(first) = args[i].chars().nth(0) { + if first != '-' { + continue; + } + + if let Some(second) = args[i].chars().nth(1) { + match second { + 'r' | 'w' | 'x' | 'X' | 's' | 't' | 'u' | 'g' | 'o' | '0' ... '7' => { + negative_option = Some(args.remove(i)); + break; + }, + _ => {} + } + } + } + } + let mut matches = match opts.parse(&args[1..]) { Ok(m) => m, Err(f) => { crash!(1, "{}", f) } @@ -86,7 +106,13 @@ Each MODE is of the form '[ugoa]*([-+=]([rwxXst]*|[ugo]))+|[-+=]?[0-7]+'.", }); let cmode = if fmode.is_none() { - Some(matches.free.remove(0)) + // If there was a negative option, now it's a good time to + // use it. + if negative_option.is_some() { + negative_option + } else { + Some(matches.free.remove(0)) + } } else { None }; diff --git a/tests/chmod.rs b/tests/chmod.rs index 5d4f893c0..f406c7727 100644 --- a/tests/chmod.rs +++ b/tests/chmod.rs @@ -59,10 +59,9 @@ fn test_chmod_octal() { TestCase{args: vec!{"0700", TEST_FILE}, before: 0o000, after: 0o700}, TestCase{args: vec!{"0070", TEST_FILE}, before: 0o000, after: 0o070}, TestCase{args: vec!{"0007", TEST_FILE}, before: 0o000, after: 0o007}, - // Known failues: #788 - // TestCase{args: vec!{"-0700", TEST_FILE}, before: 0o700, after: 0o000}, - // TestCase{args: vec!{"-0070", TEST_FILE}, before: 0o060, after: 0o000}, - // TestCase{args: vec!{"-0007", TEST_FILE}, before: 0o001, after: 0o000}, + TestCase{args: vec!{"-0700", TEST_FILE}, before: 0o700, after: 0o000}, + TestCase{args: vec!{"-0070", TEST_FILE}, before: 0o060, after: 0o000}, + TestCase{args: vec!{"-0007", TEST_FILE}, before: 0o001, after: 0o000}, TestCase{args: vec!{"+0100", TEST_FILE}, before: 0o600, after: 0o700}, TestCase{args: vec!{"+0020", TEST_FILE}, before: 0o050, after: 0o070}, TestCase{args: vec!{"+0004", TEST_FILE}, before: 0o003, after: 0o007}, @@ -77,6 +76,9 @@ fn test_chmod_ugoa() { TestCase{args: vec!{"g=rwx", TEST_FILE}, before: 0o000, after: 0o070}, TestCase{args: vec!{"o=rwx", TEST_FILE}, before: 0o000, after: 0o007}, TestCase{args: vec!{"a=rwx", TEST_FILE}, before: 0o000, after: 0o777}, + TestCase{args: vec!{"-r", TEST_FILE}, before: 0o777, after: 0o333}, + TestCase{args: vec!{"-w", TEST_FILE}, before: 0o777, after: 0o555}, + TestCase{args: vec!{"-x", TEST_FILE}, before: 0o777, after: 0o666}, }; run_tests(tests); } From 6ded76714b9b91b852c6073abddd80c1becdb3e2 Mon Sep 17 00:00:00 2001 From: Luca Ottaviano Date: Sat, 2 Apr 2016 10:25:31 +0200 Subject: [PATCH 3/5] chmod: remove unused dependencies --- Cargo.lock | 4 ---- src/chmod/Cargo.toml | 4 ---- src/chmod/chmod.rs | 2 -- 3 files changed, 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index db6789500..e27379698 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -156,12 +156,8 @@ dependencies = [ name = "chmod" version = "0.0.1" dependencies = [ - "aho-corasick 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "getopts 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", - "memchr 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", - "regex 0.1.44 (registry+https://github.com/rust-lang/crates.io-index)", - "regex-syntax 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "uucore 0.0.1", "walker 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/src/chmod/Cargo.toml b/src/chmod/Cargo.toml index a85059a93..40914154f 100644 --- a/src/chmod/Cargo.toml +++ b/src/chmod/Cargo.toml @@ -10,10 +10,6 @@ path = "chmod.rs" [dependencies] getopts = "*" libc = "*" -aho-corasick = "*" -memchr = "*" -regex = "*" -regex-syntax = "*" uucore = { path="../uucore" } walker = "*" diff --git a/src/chmod/chmod.rs b/src/chmod/chmod.rs index 651cb2f75..341cc8255 100644 --- a/src/chmod/chmod.rs +++ b/src/chmod/chmod.rs @@ -9,10 +9,8 @@ * file that was distributed with this source code. */ -extern crate aho_corasick; extern crate getopts; extern crate libc; -extern crate memchr; extern crate walker; #[macro_use] From bbe54bc0a8491974ebb5f5865271e698dea56cd0 Mon Sep 17 00:00:00 2001 From: Luca Ottaviano Date: Tue, 15 Mar 2016 14:20:11 +0100 Subject: [PATCH 4/5] tests/chmod: add missing test for many symbolic permissions at once --- tests/chmod.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/chmod.rs b/tests/chmod.rs index f406c7727..e380215ff 100644 --- a/tests/chmod.rs +++ b/tests/chmod.rs @@ -95,6 +95,14 @@ fn test_chmod_ugo_copy() { run_tests(tests); } +#[test] +fn test_chmod_many_options() { + let tests = vec!{ + TestCase{args: vec!{"-r,a+w", TEST_FILE}, before: 0o444, after: 0o222}, + }; + run_tests(tests); +} + #[test] fn test_chmod_reference_file() { let tests = vec!{ From f0186271ec71d77c4ac2f78b22e33aee2c816d29 Mon Sep 17 00:00:00 2001 From: Luca Ottaviano Date: Thu, 24 Mar 2016 19:01:40 +0100 Subject: [PATCH 5/5] tests/chmod: add test with both reference and symbolic mode Only reference should be taken into account. --- tests/chmod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/chmod.rs b/tests/chmod.rs index e380215ff..a00cb5417 100644 --- a/tests/chmod.rs +++ b/tests/chmod.rs @@ -107,6 +107,7 @@ fn test_chmod_many_options() { fn test_chmod_reference_file() { let tests = vec!{ TestCase{args: vec!{"--reference", REFERENCE_FILE, TEST_FILE}, before: 0o070, after: 0o247}, + TestCase{args: vec!{"a-w", "--reference", REFERENCE_FILE, TEST_FILE}, before: 0o070, after: 0o247}, }; let (at, ucmd) = testing(UTIL_NAME); mkfile(&at.plus_as_string(REFERENCE_FILE), REFERENCE_PERMS);