Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

walk: fd -L should include broken symlinks #497

Merged
merged 10 commits into from
Feb 28, 2020
4 changes: 2 additions & 2 deletions src/fshelper/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::io;
use std::os::unix::fs::PermissionsExt;
use std::path::{Path, PathBuf};

use ignore::DirEntry;
use crate::walk;

pub fn path_absolute_form(path: &Path) -> io::Result<PathBuf> {
if path.is_absolute() {
Expand Down Expand Up @@ -55,7 +55,7 @@ pub fn is_executable(_: &fs::Metadata) -> bool {
false
}

pub fn is_empty(entry: &DirEntry) -> bool {
pub fn is_empty(entry: &walk::DirEntry) -> bool {
if let Some(file_type) = entry.file_type() {
if file_type.is_dir() {
if let Ok(mut entries) = fs::read_dir(entry.path()) {
Expand Down
65 changes: 59 additions & 6 deletions src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ use crate::output;
use std::borrow::Cow;
use std::error::Error;
use std::ffi::OsStr;
use std::fs::{FileType, Metadata};
use std::io;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::process;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::mpsc::{channel, Receiver, Sender};
Expand Down Expand Up @@ -253,6 +254,36 @@ fn spawn_receiver(
})
}

pub enum DirEntry {
Normal(ignore::DirEntry),
BrokenSymlink(PathBuf),
}

impl DirEntry {
pub fn path(&self) -> &Path {
match self {
DirEntry::Normal(e) => e.path(),
DirEntry::BrokenSymlink(pathbuf) => pathbuf.as_path(),
}
}

pub fn file_type(&self) -> Option<FileType> {
match self {
DirEntry::Normal(e) => e.file_type(),
DirEntry::BrokenSymlink(pathbuf) => {
pathbuf.symlink_metadata().map(|m| m.file_type()).ok()
}
}
}

pub fn metadata(&self) -> Option<Metadata> {
match self {
DirEntry::Normal(e) => e.metadata().ok(),
DirEntry::BrokenSymlink(_) => None,
}
}
}

fn spawn_senders(
config: &Arc<FdOptions>,
wants_to_quit: &Arc<AtomicBool>,
Expand All @@ -272,17 +303,39 @@ fn spawn_senders(
}

let entry = match entry_o {
Ok(e) => e,
Ok(e) if e.depth() == 0 => {
// Skip the root directory entry.
return ignore::WalkState::Continue;
}
Ok(e) => DirEntry::Normal(e),
Err(ignore::Error::WithPath {
path,
err: inner_err,
}) => match inner_err.as_ref() {
ignore::Error::Io(io_error)
if io_error.kind() == io::ErrorKind::NotFound
&& path
.symlink_metadata()
.map_or(false, |m| m.file_type().is_symlink()) =>
{
DirEntry::BrokenSymlink(path.to_owned())
}
_ => {
tx_thread
.send(WorkerResult::Error(ignore::Error::WithPath {
path,
err: inner_err,
}))
.unwrap();
return ignore::WalkState::Continue;
}
},
Err(err) => {
tx_thread.send(WorkerResult::Error(err)).unwrap();
return ignore::WalkState::Continue;
}
};

if entry.depth() == 0 {
return ignore::WalkState::Continue;
}

// Check the name first, since it doesn't require metadata
let entry_path = entry.path();

Expand Down
19 changes: 19 additions & 0 deletions tests/testenv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,25 @@ impl TestEnv {
}
}

/// Create a broken symlink at the given path in the temp_dir.
pub fn create_broken_symlink<P: AsRef<Path>>(
&mut self,
link_path: P,
) -> Result<PathBuf, io::Error> {
let root = self.test_root();
let broken_symlink_link = root.join(link_path);
{
let temp_target_dir = TempDir::new("fd-tests-broken-symlink")?;
sharkdp marked this conversation as resolved.
Show resolved Hide resolved
let broken_symlink_target = temp_target_dir.path().join("broken_symlink_target");
fs::File::create(&broken_symlink_target)?;
#[cfg(unix)]
unix::fs::symlink(&broken_symlink_target, &broken_symlink_link)?;
#[cfg(windows)]
windows::fs::symlink_file(&broken_symlink_target, &broken_symlink_link)?;
}
Ok(broken_symlink_link)
}

/// Get the root directory for the tests.
pub fn test_root(&self) -> PathBuf {
self.temp_dir.path().to_path_buf()
Expand Down
32 changes: 30 additions & 2 deletions tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,32 @@ fn test_file_system_boundaries() {
);
}

#[test]
fn test_follow_broken_symlink() {
let mut te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
te.create_broken_symlink("broken_symlink")
.expect("Failed to create broken symlink.");

te.assert_output(
&["symlink"],
"broken_symlink
symlink",
);
te.assert_output(
&["--type", "symlink", "symlink"],
"broken_symlink
symlink",
);

te.assert_output(&["--type", "file", "symlink"], "");

te.assert_output(
&["--follow", "--type", "symlink", "symlink"],
"broken_symlink",
);
te.assert_output(&["--follow", "--type", "file", "symlink"], "");
}

/// Null separator (--print0)
#[test]
fn test_print0() {
Expand Down Expand Up @@ -878,7 +904,9 @@ fn test_extension() {
/// Symlink as search directory
#[test]
fn test_symlink_as_root() {
let te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
let mut te = TestEnv::new(DEFAULT_DIRS, DEFAULT_FILES);
te.create_broken_symlink("broken_symlink")
.expect("Failed to create broken symlink.");

// From: http://pubs.opengroup.org/onlinepubs/9699919799/functions/getcwd.html
// The getcwd() function shall place an absolute pathname of the current working directory in
Expand All @@ -899,6 +927,7 @@ fn test_symlink_as_root() {
&["", parent_parent],
&format!(
"{dir}/a.foo
{dir}/broken_symlink
{dir}/e1 e2
{dir}/one
{dir}/one/b.foo
Expand Down Expand Up @@ -990,7 +1019,6 @@ fn test_symlink_and_full_path_abs_path() {
),
);
}

/// Exclude patterns (--exclude)
#[test]
fn test_excludes() {
Expand Down