From 92f41b81013973873de757ff907c36709e414477 Mon Sep 17 00:00:00 2001 From: David Peter Date: Sun, 8 Aug 2021 13:24:56 +0200 Subject: [PATCH 1/2] Fix directory-existence check on Windows This fixes a bug on Windows where `fd` could not be used on ram disks and encrypted folders. closes #752 --- Cargo.lock | 12 ++++++++++++ Cargo.toml | 1 + src/filesystem.rs | 10 ++++++---- src/main.rs | 8 ++++---- 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6dcf8821d..dede4b7ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,5 +1,7 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +version = 3 + [[package]] name = "aho-corasick" version = "0.7.18" @@ -166,6 +168,7 @@ dependencies = [ "lazy_static", "libc", "lscolors", + "normpath", "num_cpus", "regex", "regex-syntax", @@ -330,6 +333,15 @@ dependencies = [ "libc", ] +[[package]] +name = "normpath" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "27e6e8f70e9fbbe3752d330d769e3424f24b9458ce266df93a3b456902fd696a" +dependencies = [ + "winapi", +] + [[package]] name = "num_cpus" version = "1.13.0" diff --git a/Cargo.toml b/Cargo.toml index 7c9322475..a4d479d79 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,6 +47,7 @@ lscolors = "0.7" globset = "0.4" anyhow = "1.0" dirs-next = "2.0" +normpath = "0.3" [dependencies.clap] version = "2.31.2" diff --git a/src/filesystem.rs b/src/filesystem.rs index 19fc0e643..6680ae5c7 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -7,6 +7,8 @@ use std::io; use std::os::unix::fs::{FileTypeExt, PermissionsExt}; use std::path::{Path, PathBuf}; +use normpath::PathExt; + use crate::walk; pub fn path_absolute_form(path: &Path) -> io::Result { @@ -33,10 +35,10 @@ pub fn absolute_path(path: &Path) -> io::Result { Ok(path_buf) } -// Path::is_dir() is not guaranteed to be intuitively correct for "." and ".." -// See: https://github.com/rust-lang/rust/issues/45302 -pub fn is_dir(path: &Path) -> bool { - path.is_dir() && (path.file_name().is_some() || path.canonicalize().is_ok()) +pub fn is_existing_directory(path: &Path) -> bool { + // Note: we do not use `.exists()` here, as `.` always exists, even if + // the CWD has been deleted. + path.is_dir() && (path.file_name().is_some() || path.normalize().is_ok()) } #[cfg(any(unix, target_os = "redox"))] diff --git a/src/main.rs b/src/main.rs index 8a315a0b6..5221d957b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -55,7 +55,7 @@ fn run() -> Result { // Set the current working directory of the process if let Some(base_directory) = matches.value_of_os("base-directory") { let base_directory = Path::new(base_directory); - if !filesystem::is_dir(base_directory) { + if !filesystem::is_existing_directory(base_directory) { return Err(anyhow!( "The '--base-directory' path '{}' is not a directory.", base_directory.to_string_lossy() @@ -70,7 +70,7 @@ fn run() -> Result { } let current_directory = Path::new("."); - if !filesystem::is_dir(current_directory) { + if !filesystem::is_existing_directory(current_directory) { return Err(anyhow!( "Could not retrieve current directory (has it been deleted?)." )); @@ -95,7 +95,7 @@ fn run() -> Result { let mut directories = vec![]; for path in paths { let path_buffer = PathBuf::from(path); - if filesystem::is_dir(&path_buffer) { + if filesystem::is_existing_directory(&path_buffer) { directories.push(path_buffer); } else { print_error(format!( @@ -130,7 +130,7 @@ fn run() -> Result { // Detect if the user accidentally supplied a path instead of a search pattern if !matches.is_present("full-path") && pattern.contains(std::path::MAIN_SEPARATOR) - && filesystem::is_dir(Path::new(pattern)) + && Path::new(pattern).is_dir() { return Err(anyhow!( "The search pattern '{pattern}' contains a path-separation character ('{sep}') \ From 5efd2d97d42de74f87d3a5da4159e88e4c677228 Mon Sep 17 00:00:00 2001 From: David Peter Date: Sun, 8 Aug 2021 14:13:32 +0200 Subject: [PATCH 2/2] Attempt to fix #365 --- src/main.rs | 3 ++- tests/tests.rs | 30 +++++++++++++++++++----------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/main.rs b/src/main.rs index 5221d957b..8654688ad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,6 +21,7 @@ use atty::Stream; use globset::GlobBuilder; use lscolors::LsColors; use regex::bytes::{RegexBuilder, RegexSetBuilder}; +use normpath::PathExt; use crate::error::print_error; use crate::exec::CommandTemplate; @@ -120,7 +121,7 @@ fn run() -> Result { .iter() .map(|path_buffer| { path_buffer - .canonicalize() + .normalize() .and_then(|pb| filesystem::absolute_path(pb.as_path())) .unwrap() }) diff --git a/tests/tests.rs b/tests/tests.rs index 783420ad6..7fe2c7068 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -5,6 +5,7 @@ use std::io::Write; use std::path::Path; use std::time::{Duration, SystemTime}; +use normpath::PathExt; use regex::escape; use crate::testenv::TestEnv; @@ -26,8 +27,9 @@ static DEFAULT_FILES: &[&str] = &[ fn get_absolute_root_path(env: &TestEnv) -> String { let path = env .test_root() - .canonicalize() + .normalize() .expect("absolute path") + .as_path() .to_str() .expect("string") .to_string(); @@ -1090,16 +1092,19 @@ fn test_symlink_as_root() { fn test_symlink_and_absolute_path() { let (te, abs_path) = get_test_env_with_abs_path(DEFAULT_DIRS, DEFAULT_FILES); + let expected_path = if cfg!(windows) { "symlink" } else { "one/two" }; + te.assert_output_subdirectory( "symlink", &["--absolute-path"], &format!( - "{abs_path}/one/two/c.foo - {abs_path}/one/two/C.Foo2 - {abs_path}/one/two/three - {abs_path}/one/two/three/d.foo - {abs_path}/one/two/three/directory_foo", - abs_path = &abs_path + "{abs_path}/{expected_path}/c.foo + {abs_path}/{expected_path}/C.Foo2 + {abs_path}/{expected_path}/three + {abs_path}/{expected_path}/three/d.foo + {abs_path}/{expected_path}/three/directory_foo", + abs_path = &abs_path, + expected_path = expected_path ), ); } @@ -1127,6 +1132,8 @@ fn test_symlink_and_full_path() { let root = te.system_root(); let prefix = escape(&root.to_string_lossy()); + let expected_path = if cfg!(windows) { "symlink" } else { "one/two" }; + te.assert_output_subdirectory( "symlink", &[ @@ -1135,10 +1142,11 @@ fn test_symlink_and_full_path() { &format!("^{prefix}.*three", prefix = prefix), ], &format!( - "{abs_path}/one/two/three - {abs_path}/one/two/three/d.foo - {abs_path}/one/two/three/directory_foo", - abs_path = &abs_path + "{abs_path}/{expected_path}/three + {abs_path}/{expected_path}/three/d.foo + {abs_path}/{expected_path}/three/directory_foo", + abs_path = &abs_path, + expected_path = expected_path ), ); }