Skip to content

Commit

Permalink
bug: fix contents_first with root symlink
Browse files Browse the repository at this point in the history
This fixes two incorrect behaviors when `contents_first` is on and the
root path is a symlink:

  1. The root path was returned first in the iterator instead of last.
  2. Some subdirectories were returned out of order.

The issue was that root symlinks were returned immediately rather than
being pushed onto the `deferred_dirs` vec. That lead to `deferred_dirs`
and `depth` being out of sync, which lead to deferred directories being
processed one ascent too late.

TO DO:
  [ ] Remove FIXME
  [ ] Consider commenting changed code; it seems somewhat hard to follow
      to my sleep deprived brain.

Fixes: BurntSushi#163
  • Loading branch information
danielparks committed Aug 13, 2022
1 parent d7f6d53 commit 74f24c7
Showing 1 changed file with 18 additions and 14 deletions.
32 changes: 18 additions & 14 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -818,19 +818,14 @@ impl IntoIter {
&mut self,
mut dent: DirEntry,
) -> Option<Result<DirEntry>> {
if self.opts.follow_links && dent.file_type().is_symlink() {
let should_descend = if !dent.file_type().is_symlink() {
dent.is_dir()
} else if self.opts.follow_links {
dent = itry!(self.follow(dent));
}
let is_normal_dir = !dent.file_type().is_symlink() && dent.is_dir();
if is_normal_dir {
if self.opts.same_file_system && dent.depth() > 0 {
if itry!(self.is_same_file_system(&dent)) {
itry!(self.push(&dent));
}
} else {
itry!(self.push(&dent));
}
} else if dent.depth() == 0 && dent.file_type().is_symlink() {
// FIXME need !dent.file_type().is_symlink()? Seems like follow()
// should prevent this from being both a symlink and a dir.
!dent.file_type().is_symlink() && dent.is_dir()
} else if dent.depth() == 0 {
// As a special case, if we are processing a root entry, then we
// always follow it even if it's a symlink and follow_links is
// false. We are careful to not let this change the semantics of
Expand All @@ -841,11 +836,20 @@ impl IntoIter {
let md = itry!(fs::metadata(dent.path()).map_err(|err| {
Error::from_path(dent.depth(), dent.path().to_path_buf(), err)
}));
if md.file_type().is_dir() {
md.file_type().is_dir()
} else {
false
};
if should_descend {
if self.opts.same_file_system && dent.depth() > 0 {
if itry!(self.is_same_file_system(&dent)) {
itry!(self.push(&dent));
}
} else {
itry!(self.push(&dent));
}
}
if is_normal_dir && self.opts.contents_first {
if should_descend && self.opts.contents_first {
self.deferred_dirs.push(dent);
None
} else if self.skippable() {
Expand Down

0 comments on commit 74f24c7

Please sign in to comment.