Skip to content

Commit

Permalink
Merge branch 'main' into cp-symbolic-link-loop
Browse files Browse the repository at this point in the history
  • Loading branch information
@sylvestre
sylvestre authored Oct 6, 2022
2 parents 24630db + 6b36a26 commit 88fb43c
Show file tree
Hide file tree
Showing 9 changed files with 586 additions and 99 deletions.
14 changes: 0 additions & 14 deletions .github/workflows/CICD.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,20 +138,6 @@ jobs:
# * convert any errors/warnings to GHA UI annotations; ref: <https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-a-warning-message>
S=$(cargo fmt -- --check) && printf "%s\n" "$S" || { printf "%s\n" "$S" ; printf "%s\n" "$S" | sed -E -n -e "s/^Diff[[:space:]]+in[[:space:]]+${PWD//\//\\/}\/(.*)[[:space:]]+at[[:space:]]+[^0-9]+([0-9]+).*$/::${fault_type} file=\1,line=\2::${fault_prefix}: \`cargo fmt\`: style violation (file:'\1', line:\2; use \`cargo fmt -- \"\1\"\`)/p" ; fault=true ; }
if [ -n "${{ steps.vars.outputs.FAIL_ON_FAULT }}" ] && [ -n "$fault" ]; then exit 1 ; fi
- name: "`cargo fmt` testing of integration tests"
if: success() || failure() # run regardless of prior step success/failure
shell: bash
run: |
## `cargo fmt` testing of integration tests
unset fault
fault_type="${{ steps.vars.outputs.FAULT_TYPE }}"
fault_prefix=$(echo "$fault_type" | tr '[:lower:]' '[:upper:]')
# 'tests' is the standard/usual integration test directory
if [ -d tests ]; then
# * convert any errors/warnings to GHA UI annotations; ref: <https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-a-warning-message>
S=$(find tests -name "*.rs" -print0 | xargs -0 cargo fmt -- --check) && printf "%s\n" "$S" || { printf "%s\n" "$S" ; printf "%s\n" "$S" | sed -E -n "s/^Diff[[:space:]]+in[[:space:]]+${PWD//\//\\/}\/(.*)[[:space:]]+at[[:space:]]+[^0-9]+([0-9]+).*$/::${fault_type} file=\1,line=\2::${fault_prefix}: \`cargo fmt\`: style violation (file:'\1', line:\2; use \`cargo fmt \"\1\"\`)/p" ; fault=true ; }
fi
if [ -n "${{ steps.vars.outputs.FAIL_ON_FAULT }}" ] && [ -n "$fault" ]; then exit 1 ; fi
style_lint:
name: Style/lint
Expand Down
51 changes: 0 additions & 51 deletions build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ pub fn main() {

let out_dir = env::var("OUT_DIR").unwrap();
// println!("cargo:warning=out_dir={}", out_dir);
let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap().replace('\\', "/");
// println!("cargo:warning=manifest_dir={}", manifest_dir);
let util_tests_dir = format!("{}/tests/by-util", manifest_dir);
// println!("cargo:warning=util_tests_dir={}", util_tests_dir);

let mut crates = Vec::new();
for (key, val) in env::vars() {
Expand All @@ -41,7 +37,6 @@ pub fn main() {
crates.sort();

let mut mf = File::create(Path::new(&out_dir).join("uutils_map.rs")).unwrap();
let mut tf = File::create(Path::new(&out_dir).join("test_modules.rs")).unwrap();

mf.write_all(
"type UtilityMap<T> = phf::Map<&'static str, (fn(T) -> i32, fn() -> Command<'static>)>;\n\
Expand All @@ -60,42 +55,15 @@ pub fn main() {
"uu_test" => {
phf_map.entry("test", &map_value);
phf_map.entry("[", &map_value);

tf.write_all(
format!(
"#[path=\"{dir}/test_test.rs\"]\nmod test_test;\n",
dir = util_tests_dir,
)
.as_bytes(),
)
.unwrap();
}
k if k.starts_with(OVERRIDE_PREFIX) => {
phf_map.entry(&k[OVERRIDE_PREFIX.len()..], &map_value);
tf.write_all(
format!(
"#[path=\"{dir}/test_{k}.rs\"]\nmod test_{k};\n",
k = &krate[OVERRIDE_PREFIX.len()..],
dir = util_tests_dir,
)
.as_bytes(),
)
.unwrap();
}
"false" | "true" => {
phf_map.entry(
krate,
&format!("(r#{krate}::uumain, r#{krate}::uu_app)", krate = krate),
);
tf.write_all(
format!(
"#[path=\"{dir}/test_{krate}.rs\"]\nmod test_{krate};\n",
krate = krate,
dir = util_tests_dir,
)
.as_bytes(),
)
.unwrap();
}
"hashsum" => {
phf_map.entry(
Expand Down Expand Up @@ -123,33 +91,14 @@ pub fn main() {
phf_map.entry("shake256sum", &map_value_bits);
phf_map.entry("b2sum", &map_value);
phf_map.entry("b3sum", &map_value_b3sum);
tf.write_all(
format!(
"#[path=\"{dir}/test_{krate}.rs\"]\nmod test_{krate};\n",
krate = krate,
dir = util_tests_dir,
)
.as_bytes(),
)
.unwrap();
}
_ => {
phf_map.entry(krate, &map_value);
tf.write_all(
format!(
"#[path=\"{dir}/test_{krate}.rs\"]\nmod test_{krate};\n",
krate = krate,
dir = util_tests_dir,
)
.as_bytes(),
)
.unwrap();
}
}
}
write!(mf, "{}", phf_map.build()).unwrap();
mf.write_all(b"\n}\n").unwrap();

mf.flush().unwrap();
tf.flush().unwrap();
}
2 changes: 1 addition & 1 deletion src/uu/cp/src/cp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ fn copy_directory(
return Err(format!(
"cannot copy a directory, {}, into itself, {}",
root.quote(),
target.quote()
target.join(root.file_name().unwrap()).quote()
)
.into());
}
Expand Down
88 changes: 58 additions & 30 deletions src/uu/ls/src/ls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
#[macro_use]
extern crate uucore;

use clap::{builder::ValueParser, crate_version, Arg, Command};
use clap::{
builder::{NonEmptyStringValueParser, ValueParser},
crate_version, Arg, Command,
};
use glob::Pattern;
use lscolors::LsColors;
use number_prefix::NumberPrefix;
Expand Down Expand Up @@ -156,6 +159,7 @@ enum LsError {
IOErrorContext(std::io::Error, PathBuf, bool),
BlockSizeParseError(String),
AlreadyListedError(PathBuf),
TimeStyleParseError(String, Vec<String>),
}

impl UError for LsError {
Expand All @@ -167,6 +171,7 @@ impl UError for LsError {
Self::IOErrorContext(_, _, true) => 2,
Self::BlockSizeParseError(_) => 1,
Self::AlreadyListedError(_) => 2,
Self::TimeStyleParseError(_, _) => 1,
}
}
}
Expand All @@ -179,6 +184,14 @@ impl Display for LsError {
Self::BlockSizeParseError(s) => {
write!(f, "invalid --block-size argument {}", s.quote())
}
Self::TimeStyleParseError(s, possible_time_styles) => {
write!(
f,
"invalid --time-style argument {}\nPossible values are: {:?}\n\nFor more information try --help",
s.quote(),
possible_time_styles
)
}
Self::InvalidLineWidth(s) => write!(f, "invalid line width: {}", s.quote()),
Self::IOError(e) => write!(f, "general io error: {}", e),
Self::IOErrorContext(e, p, _) => {
Expand Down Expand Up @@ -300,8 +313,46 @@ enum TimeStyle {
LongIso,
Iso,
Locale,
Format(String),
}

fn parse_time_style(options: &clap::ArgMatches) -> Result<TimeStyle, LsError> {
let possible_time_styles = vec![
"full-iso".to_string(),
"long-iso".to_string(),
"iso".to_string(),
"locale".to_string(),
"+FORMAT (e.g., +%H:%M) for a 'date'-style format".to_string(),
];
if let Some(field) = options.get_one::<String>(options::TIME_STYLE) {
//If both FULL_TIME and TIME_STYLE are present
//The one added last is dominant
if options.contains_id(options::FULL_TIME)
&& options.indices_of(options::FULL_TIME).unwrap().last()
> options.indices_of(options::TIME_STYLE).unwrap().last()
{
Ok(TimeStyle::FullIso)
} else {
match field.as_str() {
"full-iso" => Ok(TimeStyle::FullIso),
"long-iso" => Ok(TimeStyle::LongIso),
"iso" => Ok(TimeStyle::Iso),
"locale" => Ok(TimeStyle::Locale),
_ => match field.chars().next().unwrap() {
'+' => Ok(TimeStyle::Format(String::from(&field[1..]))),
_ => Err(LsError::TimeStyleParseError(
String::from(field),
possible_time_styles,
)),
},
}
}
} else if options.contains_id(options::FULL_TIME) {
Ok(TimeStyle::FullIso)
} else {
Ok(TimeStyle::Locale)
}
}
enum Dereference {
None,
DirArgs,
Expand Down Expand Up @@ -700,31 +751,7 @@ impl Config {
} else {
IndicatorStyle::None
};

let time_style = if let Some(field) = options.get_one::<String>(options::TIME_STYLE) {
//If both FULL_TIME and TIME_STYLE are present
//The one added last is dominant
if options.contains_id(options::FULL_TIME)
&& options.indices_of(options::FULL_TIME).unwrap().last()
> options.indices_of(options::TIME_STYLE).unwrap().last()
{
TimeStyle::FullIso
} else {
//Clap handles the env variable "TIME_STYLE"
match field.as_str() {
"full-iso" => TimeStyle::FullIso,
"long-iso" => TimeStyle::LongIso,
"iso" => TimeStyle::Iso,
"locale" => TimeStyle::Locale,
// below should never happen as clap already restricts the values.
_ => unreachable!("Invalid field for --time-style"),
}
}
} else if options.contains_id(options::FULL_TIME) {
TimeStyle::FullIso
} else {
TimeStyle::Locale
};
let time_style = parse_time_style(options)?;

let mut ignore_patterns: Vec<Pattern> = Vec::new();

Expand Down Expand Up @@ -1511,13 +1538,13 @@ pub fn uu_app<'a>() -> Command<'a> {
]),
)
.arg(
//This still needs support for posix-*, +FORMAT
//This still needs support for posix-*
Arg::new(options::TIME_STYLE)
.long(options::TIME_STYLE)
.help("time/date format with -l; see TIME_STYLE below")
.value_name("TIME_STYLE")
.env("TIME_STYLE")
.value_parser(["full-iso", "long-iso", "iso", "locale"])
.value_parser(NonEmptyStringValueParser::new())
.overrides_with_all(&[options::TIME_STYLE]),
)
.arg(
Expand Down Expand Up @@ -1549,7 +1576,7 @@ pub fn uu_app<'a>() -> Command<'a> {
.value_parser(ValueParser::os_string()),
)
.after_help(
"The TIME_STYLE argument can be full-iso, long-iso, iso. \
"The TIME_STYLE argument can be full-iso, long-iso, iso, locale or +FORMAT. FORMAT is interpreted like in date. \
Also the TIME_STYLE environment variable sets the default style to use.",
)
}
Expand Down Expand Up @@ -2519,7 +2546,7 @@ fn display_date(metadata: &Metadata, config: &Config) -> String {
//According to GNU a Gregorian year has 365.2425 * 24 * 60 * 60 == 31556952 seconds on the average.
let recent = time + chrono::Duration::seconds(31_556_952 / 2) > chrono::Local::now();

match config.time_style {
match &config.time_style {
TimeStyle::FullIso => time.format("%Y-%m-%d %H:%M:%S.%f %z"),
TimeStyle::LongIso => time.format("%Y-%m-%d %H:%M"),
TimeStyle::Iso => time.format(if recent { "%m-%d %H:%M" } else { "%Y-%m-%d " }),
Expand All @@ -2534,6 +2561,7 @@ fn display_date(metadata: &Metadata, config: &Config) -> String {

time.format(fmt)
}
TimeStyle::Format(e) => time.format(e),
}
.to_string()
}
Expand Down
31 changes: 29 additions & 2 deletions src/uu/rm/src/rm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,13 +303,36 @@ fn handle_dir(path: &Path, options: &Options) -> bool {
}
} else {
let mut dirs: VecDeque<DirEntry> = VecDeque::new();
// The Paths to not descend into. We need to this because WalkDir doesn't have a way, afaik, to not descend into a directory
// So we have to just ignore paths as they come up if they start with a path we aren't descending into
let mut not_descended: Vec<PathBuf> = Vec::new();

for entry in WalkDir::new(path) {
'outer: for entry in WalkDir::new(path) {
match entry {
Ok(entry) => {
if options.interactive == InteractiveMode::Always {
for not_descend in &not_descended {
if entry.path().starts_with(not_descend) {
// We don't need to continue the rest of code in this loop if we are in a directory we don't want to descend into
continue 'outer;
}
}
}
let file_type = entry.file_type();
if file_type.is_dir() {
dirs.push_back(entry);
// If we are in Interactive Mode Always and the directory isn't empty we ask if we should descend else we push this directory onto dirs vector
if options.interactive == InteractiveMode::Always
&& fs::read_dir(entry.path()).unwrap().count() != 0
{
// If we don't descend we push this directory onto our not_descended vector else we push this directory onto dirs vector
if prompt_descend(entry.path()) {
dirs.push_back(entry);
} else {
not_descended.push(entry.path().to_path_buf());
}
} else {
dirs.push_back(entry);
}
} else {
had_err = remove_file(entry.path(), options).bitor(had_err);
}
Expand Down Expand Up @@ -447,6 +470,10 @@ fn prompt_write_protected(path: &Path, is_dir: bool, options: &Options) -> bool
}
}

fn prompt_descend(path: &Path) -> bool {
prompt(&(format!("rm: descend into directory {}? ", path.quote())))
}

fn prompt_file(path: &Path, is_dir: bool) -> bool {
if is_dir {
prompt(&(format!("rm: remove directory {}? ", path.quote())))
Expand Down
28 changes: 28 additions & 0 deletions tests/by-util/test_cp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2108,3 +2108,31 @@ fn test_remove_destination_symbolic_link_loop() {
.no_stderr();
assert!(at.file_exists("loop"));
}

/// Test that copying a directory to itself is disallowed.
#[test]
fn test_copy_directory_to_itself_disallowed() {
let (at, mut ucmd) = at_and_ucmd!();
at.mkdir("d");
#[cfg(not(windows))]
let expected = "cp: cannot copy a directory, 'd', into itself, 'd/d'";
#[cfg(windows)]
let expected = "cp: cannot copy a directory, 'd', into itself, 'd\\d'";
ucmd.args(&["-R", "d", "d"]).fails().stderr_only(expected);
}

/// Test that copying a nested directory to itself is disallowed.
#[test]
fn test_copy_nested_directory_to_itself_disallowed() {
let (at, mut ucmd) = at_and_ucmd!();
at.mkdir("a");
at.mkdir("a/b");
at.mkdir("a/b/c");
#[cfg(not(windows))]
let expected = "cp: cannot copy a directory, 'a/b', into itself, 'a/b/c/b'";
#[cfg(windows)]
let expected = "cp: cannot copy a directory, 'a/b', into itself, 'a/b/c\\b'";
ucmd.args(&["-R", "a/b", "a/b/c"])
.fails()
.stderr_only(expected);
}
12 changes: 12 additions & 0 deletions tests/by-util/test_ls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1653,6 +1653,7 @@ fn test_ls_styles() {
let re_iso = Regex::new(r"[a-z-]* \d* \w* \w* \d* \d{2}-\d{2} \d{2}:\d{2} test\n").unwrap();
let re_locale =
Regex::new(r"[a-z-]* \d* \w* \w* \d* [A-Z][a-z]{2} ( |\d)\d \d{2}:\d{2} test\n").unwrap();
let re_custom_format = Regex::new(r"[a-z-]* \d* \w* \w* \d* \d{4}__\d{2} test\n").unwrap();

//full-iso
let result = scene
Expand All @@ -1675,6 +1676,17 @@ fn test_ls_styles() {
let result = scene.ucmd().arg("-l").arg("--time-style=locale").succeeds();
assert!(re_locale.is_match(result.stdout_str()));

//+FORMAT
let result = scene
.ucmd()
.arg("-l")
.arg("--time-style=+%Y__%M")
.succeeds();
assert!(re_custom_format.is_match(result.stdout_str()));

// Also fails due to not having full clap support for time_styles
scene.ucmd().arg("-l").arg("-time-style=invalid").fails();

//Overwrite options tests
let result = scene
.ucmd()
Expand Down
Loading

0 comments on commit 88fb43c

Please sign in to comment.