Refactor path handling (fixes #113)

* Fix path check
* Fix full path matching
* Allow more simple driver names in Windows tests
* Factor out special is_dir() check for "." and ".."
This commit is contained in:
J.W 2017-10-19 02:04:34 +08:00 committed by David Peter
parent 37def9bb83
commit 8d85debc12
9 changed files with 112 additions and 126 deletions

View File

@ -9,7 +9,6 @@ use super::TokenizedCommand;
/// be executed, and this process will continue until the receiver's sender has closed.
pub fn job(
rx: Arc<Mutex<Receiver<PathBuf>>>,
base: Arc<Option<PathBuf>>,
cmd: Arc<TokenizedCommand>,
out_perm: Arc<Mutex<()>>,
) {
@ -23,12 +22,7 @@ pub fn job(
// Obtain the next path from the receiver, else if the channel
// has closed, exit from the loop
let value: PathBuf = match lock.recv() {
Ok(value) => {
match *base {
Some(ref base) => base.join(&value),
None => value,
}
}
Ok(value) => value,
Err(_) => break,
};

View File

@ -109,6 +109,8 @@ impl TokenizedCommand {
input: &Path,
out_perm: Arc<Mutex<()>>,
) -> CommandTicket<'a> {
let input = input.strip_prefix(".").unwrap_or(input);
for token in &self.tokens {
match *token {
Token::Basename => *command += basename(&input.to_string_lossy()),

View File

@ -1,50 +1,18 @@
use std::env::current_dir;
use std::path::{Path, PathBuf};
use std::io;
/// Get a relative path with respect to a certain base path.
/// See: https://stackoverflow.com/a/39343127/704831
pub fn path_relative_from(path: &Path, base: &Path) -> Option<PathBuf> {
use std::path::Component;
if path.is_absolute() != base.is_absolute() {
if path.is_absolute() {
Some(PathBuf::from(path))
} else {
Some(PathBuf::from(base.join(path)))
}
pub fn path_absolute_form(path: &Path) -> io::Result<PathBuf> {
if path.is_absolute() {
Ok(path.to_path_buf())
} else {
let mut ita = path.components();
let mut itb = base.components();
let mut comps: Vec<Component> = vec![];
loop {
match (ita.next(), itb.next()) {
(None, None) => break,
(Some(a), None) => {
comps.push(a);
comps.extend(ita.by_ref());
break;
}
(None, _) => comps.push(Component::ParentDir),
(Some(a), Some(b)) if comps.is_empty() && a == b => (),
(Some(a), Some(b)) if b == Component::CurDir => comps.push(a),
(Some(_), Some(b)) if b == Component::ParentDir => return None,
(Some(a), Some(_)) => {
comps.push(Component::ParentDir);
for _ in itb {
comps.push(Component::ParentDir);
}
comps.push(a);
comps.extend(ita.by_ref());
break;
}
}
}
Some(comps.iter().map(|c| c.as_os_str()).collect())
let path = path.strip_prefix(".").unwrap_or(path);
current_dir().map(|path_buf| path_buf.join(path))
}
}
pub fn absolute_path(path: &Path) -> io::Result<PathBuf> {
let path_buf = path.canonicalize()?;
let path_buf = path_absolute_form(path)?;
#[cfg(windows)]
let path_buf = Path::new(path_buf.as_path().to_string_lossy().trim_left_matches(
@ -53,3 +21,13 @@ pub fn absolute_path(path: &Path) -> io::Result<PathBuf> {
Ok(path_buf)
}
// Path::is_dir() is not guarandteed to be intuitively correct for "." and ".."
// See: https://github.com/rust-lang/rust/issues/45302
pub fn is_dir(path: &Path) -> bool {
if path.file_name().is_some() {
path.is_dir()
} else {
path.is_dir() && path.canonicalize().is_ok()
}
}

View File

@ -7,13 +7,6 @@ use lscolors::LsColors;
use walk::FileType;
use regex_syntax::{Expr, ExprBuilder};
/// Root directory
#[cfg(unix)]
pub static ROOT_DIR: &'static str = "/";
#[cfg(windows)]
pub static ROOT_DIR: &'static str = "";
/// Defines how to display search result paths.
#[derive(PartialEq)]
pub enum PathDisplay {

View File

@ -29,7 +29,7 @@ use atty::Stream;
use regex::RegexBuilder;
use exec::TokenizedCommand;
use internal::{error, pattern_has_uppercase_char, FdOptions, PathDisplay, ROOT_DIR};
use internal::{error, pattern_has_uppercase_char, FdOptions, PathDisplay};
use lscolors::LsColors;
use walk::FileType;
@ -42,8 +42,7 @@ fn main() {
// Get the current working directory
let current_dir = Path::new(".");
// .is_dir() is not guarandteed to be intuitively correct for "." and ".."
if let Err(_) = current_dir.canonicalize() {
if !fshelper::is_dir(&current_dir) {
error("Error: could not get current directory.");
}
@ -52,7 +51,7 @@ fn main() {
Some(path) => PathBuf::from(path),
None => current_dir.to_path_buf(),
};
if let Err(_) = root_dir_buf.canonicalize() {
if !fshelper::is_dir(&root_dir_buf) {
error(&format!(
"Error: '{}' is not a directory.",
root_dir_buf.to_string_lossy()
@ -132,21 +131,11 @@ fn main() {
command,
};
// If base_dir is ROOT_DIR, then root_dir must be absolute.
// Otherwise root_dir/entry cannot be turned into an existing relative path from base_dir.
//
// We utilize ROOT_DIR to avoid resolving the components of root_dir.
let base_dir_buf = match config.path_display {
PathDisplay::Relative => current_dir.to_path_buf(),
PathDisplay::Absolute => PathBuf::from(ROOT_DIR),
};
let base_dir = base_dir_buf.as_path();
match RegexBuilder::new(pattern)
.case_insensitive(!config.case_sensitive)
.dot_matches_new_line(true)
.build() {
Ok(re) => walk::scan(root_dir, Arc::new(re), base_dir, Arc::new(config)),
Ok(re) => walk::scan(root_dir, Arc::new(re), Arc::new(config)),
Err(err) => error(err.description()),
}
}

View File

@ -1,4 +1,4 @@
use internal::{FdOptions, PathDisplay};
use internal::FdOptions;
use lscolors::LsColors;
use std::{fs, process};
@ -10,23 +10,13 @@ use std::os::unix::fs::PermissionsExt;
use ansi_term;
pub fn print_entry(base: &Path, entry: &PathBuf, config: &FdOptions) {
let path_full = if !entry.as_os_str().is_empty() {
base.join(entry)
} else {
base.to_path_buf()
};
let path_to_print = if config.path_display == PathDisplay::Absolute {
&path_full
} else {
entry
};
pub fn print_entry(entry: &PathBuf, config: &FdOptions) {
let path = entry.strip_prefix(".").unwrap_or(entry);
let r = if let Some(ref ls_colors) = config.ls_colors {
print_entry_colorized(base, path_to_print, config, ls_colors)
print_entry_colorized(path, config, ls_colors)
} else {
print_entry_uncolorized(path_to_print, config)
print_entry_uncolorized(path, config)
};
if r.is_err() {
@ -35,12 +25,7 @@ pub fn print_entry(base: &Path, entry: &PathBuf, config: &FdOptions) {
}
}
fn print_entry_colorized(
base: &Path,
path: &Path,
config: &FdOptions,
ls_colors: &LsColors,
) -> io::Result<()> {
fn print_entry_colorized(path: &Path, config: &FdOptions, ls_colors: &LsColors) -> io::Result<()> {
let default_style = ansi_term::Style::default();
let stdout = io::stdout();
@ -50,7 +35,7 @@ fn print_entry_colorized(
let mut separator = String::new();
// Full path to the current component.
let mut component_path = base.to_path_buf();
let mut component_path = PathBuf::new();
// Traverse the path and colorize each component
for component in path.components() {

View File

@ -1,6 +1,6 @@
use exec::{self, TokenizedCommand};
use fshelper;
use internal::{error, FdOptions, PathDisplay};
use internal::{error, FdOptions};
use output;
use std::path::Path;
@ -36,7 +36,7 @@ pub enum FileType {
/// If the `--exec` argument was supplied, this will create a thread pool for executing
/// jobs in parallel from a given command line and the discovered paths. Otherwise, each
/// path will simply be written to standard output.
pub fn scan(root: &Path, pattern: Arc<Regex>, base: &Path, config: Arc<FdOptions>) {
pub fn scan(root: &Path, pattern: Arc<Regex>, config: Arc<FdOptions>) {
let (tx, rx) = channel();
let threads = config.threads;
@ -54,8 +54,6 @@ pub fn scan(root: &Path, pattern: Arc<Regex>, base: &Path, config: Arc<FdOptions
// Spawn the thread that receives all results through the channel.
let rx_config = Arc::clone(&config);
let rx_base = base.to_owned();
let is_absolute = config.path_display == PathDisplay::Absolute;
let receiver_thread = thread::spawn(move || {
// This will be set to `Some` if the `--exec` argument was supplied.
if let Some(ref cmd) = rx_config.command {
@ -63,8 +61,6 @@ pub fn scan(root: &Path, pattern: Arc<Regex>, base: &Path, config: Arc<FdOptions
let out_perm = Arc::new(Mutex::new(()));
let base = Arc::new(if is_absolute { Some(rx_base) } else { None });
// This is safe because `cmd` will exist beyond the end of this scope.
// It's required to tell Rust that it's safe to share across threads.
let cmd = unsafe { Arc::from_raw(cmd as *const TokenizedCommand) };
@ -74,11 +70,10 @@ pub fn scan(root: &Path, pattern: Arc<Regex>, base: &Path, config: Arc<FdOptions
for _ in 0..threads {
let rx = shared_rx.clone();
let cmd = cmd.clone();
let base = base.clone();
let out_perm = out_perm.clone();
// Spawn a job thread that will listen for and execute inputs.
let handle = thread::spawn(move || exec::job(rx, base, cmd, out_perm));
let handle = thread::spawn(move || exec::job(rx, cmd, out_perm));
// Push the handle of the spawned thread into the vector for later joining.
handles.push(handle);
@ -110,7 +105,7 @@ pub fn scan(root: &Path, pattern: Arc<Regex>, base: &Path, config: Arc<FdOptions
if time::Instant::now() - start > max_buffer_time {
// Flush the buffer
for v in &buffer {
output::print_entry(&rx_base, v, &rx_config);
output::print_entry(&v, &rx_config);
}
buffer.clear();
@ -119,7 +114,7 @@ pub fn scan(root: &Path, pattern: Arc<Regex>, base: &Path, config: Arc<FdOptions
}
}
ReceiverMode::Streaming => {
output::print_entry(&rx_base, &value, &rx_config);
output::print_entry(&value, &rx_config);
}
}
}
@ -129,7 +124,7 @@ pub fn scan(root: &Path, pattern: Arc<Regex>, base: &Path, config: Arc<FdOptions
if !buffer.is_empty() {
buffer.sort();
for value in buffer {
output::print_entry(&rx_base, &value, &rx_config);
output::print_entry(&value, &rx_config);
}
}
}
@ -137,7 +132,6 @@ pub fn scan(root: &Path, pattern: Arc<Regex>, base: &Path, config: Arc<FdOptions
// Spawn the sender threads.
walker.run(|| {
let base = base.to_owned();
let config = Arc::clone(&config);
let pattern = Arc::clone(&pattern);
let tx_thread = tx.clone();
@ -186,21 +180,19 @@ pub fn scan(root: &Path, pattern: Arc<Regex>, base: &Path, config: Arc<FdOptions
}
let search_str_o = if config.search_full_path {
Some(entry_path.to_string_lossy())
match fshelper::path_absolute_form(&entry_path) {
Ok(path_abs_buf) => Some(path_abs_buf.to_string_lossy().into_owned().into()),
Err(_) => error("Error: unable to get full path."),
}
} else {
entry_path.file_name().map(|f| f.to_string_lossy())
};
if let Some(search_str) = search_str_o {
pattern.find(&*search_str).map(|_| {
let path_rel_buf = match fshelper::path_relative_from(entry_path, &*base) {
Some(p) => p,
None => error("Error: could not get relative path for directory entry."),
};
if pattern.is_match(&*search_str) {
// TODO: take care of the unwrap call
tx_thread.send(path_rel_buf.to_owned()).unwrap()
});
tx_thread.send(entry_path.to_owned()).unwrap()
}
}
ignore::WalkState::Continue

View File

@ -136,10 +136,16 @@ impl TestEnv {
}
/// Get the root directory for the tests.
pub fn root(&self) -> PathBuf {
pub fn test_root(&self) -> PathBuf {
self.temp_dir.path().to_path_buf()
}
/// Get the root directory of the file system.
pub fn system_root(&self) -> PathBuf {
let mut components = self.temp_dir.path().components();
PathBuf::from(components.next().expect("root directory").as_os_str())
}
/// Assert that calling *fd* with the specified arguments produces the expected output.
pub fn assert_output(&self, args: &[&str], expected: &str) {
self.assert_output_subdirectory(".", args, expected)

View File

@ -1,11 +1,14 @@
//! Integration tests for the CLI interface of fd.
extern crate regex;
mod testenv;
use testenv::TestEnv;
use regex::escape;
fn get_absolute_root_path(env: &TestEnv) -> String {
let path = env.root()
let path = env.test_root()
.canonicalize()
.expect("absolute path")
.to_str()
@ -177,8 +180,14 @@ fn test_case_insensitive() {
fn test_full_path() {
let te = TestEnv::new();
let root = te.system_root();
let prefix = escape(&root.to_string_lossy());
te.assert_output(
&["--full-path", "three.*foo"],
&[
"--full-path",
&format!("^{prefix}.*three.*foo$", prefix = prefix),
],
"one/two/three/d.foo
one/two/three/directory_foo",
);
@ -343,7 +352,7 @@ fn test_absolute_path() {
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo
{abs_path}/symlink",
abs_path = abs_path
abs_path = &abs_path
),
);
@ -356,7 +365,7 @@ fn test_absolute_path() {
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo",
abs_path = abs_path
abs_path = &abs_path
),
);
@ -369,7 +378,7 @@ fn test_absolute_path() {
{abs_path}/one/two/C.Foo2
{abs_path}/one/two/three/d.foo
{abs_path}/one/two/three/directory_foo",
abs_path = abs_path
abs_path = &abs_path
),
);
}
@ -435,9 +444,13 @@ fn test_symlink() {
// the array pointed to by buf, and return buf. The pathname shall contain no components that
// are dot or dot-dot, or are symbolic links.
//
// Symlinks on Unix are aliases to real paths, only has one redirection.
// Key points:
// 1. The path of the current working directory of a Unix process cannot contain symlinks.
// 2. The path of the current working directory of a Windows process can contain symlinks.
//
// Symlinks on Windows can refer to symlinks, and are resolved after logical step "..".
// More:
// 1. On Windows, symlinks are resolved after the ".." component.
// 2. On Unix, symlinks are resolved immediately as encountered.
let parent_parent = if cfg!(windows) { ".." } else { "../.." };
te.assert_output_subdirectory(
@ -462,12 +475,13 @@ fn test_symlink() {
"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}/{dir}/c.foo
{abs_path}/{dir}/C.Foo2
{abs_path}/{dir}/three
{abs_path}/{dir}/three/d.foo
{abs_path}/{dir}/three/directory_foo",
dir = if cfg!(windows) { "symlink" } else { "one/two" },
abs_path = &abs_path
),
);
@ -479,7 +493,40 @@ fn test_symlink() {
{abs_path}/symlink/three
{abs_path}/symlink/three/d.foo
{abs_path}/symlink/three/directory_foo",
abs_path = abs_path
abs_path = &abs_path
),
);
let root = te.system_root();
let prefix = escape(&root.to_string_lossy());
te.assert_output_subdirectory(
"symlink",
&[
"--absolute-path",
"--full-path",
&format!("^{prefix}.*three", prefix = prefix),
],
&format!(
"{abs_path}/{dir}/three
{abs_path}/{dir}/three/d.foo
{abs_path}/{dir}/three/directory_foo",
dir = if cfg!(windows) { "symlink" } else { "one/two" },
abs_path = &abs_path
),
);
te.assert_output(
&[
"--full-path",
&format!("^{prefix}.*symlink.*three", prefix = prefix),
&format!("{abs_path}/symlink", abs_path = abs_path),
],
&format!(
"{abs_path}/symlink/three
{abs_path}/symlink/three/d.foo
{abs_path}/symlink/three/directory_foo",
abs_path = &abs_path
),
);
}