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.

Sign up for GitHub

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

Jump to bottom

Implement -ignore_readdir_race and -noignore_readdir_race. #411

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Implement -ignore_readdir_race and -noignore_readdir_race. #411

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0e17be1
Added -ignore_readdir_race and -noignore_readdir_race support.
hanbings Jul 4, 2024
f4bb537
Added tests.
hanbings Jul 4, 2024
7413637
Add description of why parameter has no effect.
hanbings Jul 6, 2024
c9a36d0
Run cargo fmt.
hanbings Jul 6, 2024
9250adc
Add description of not changing behavior in parser.
hanbings Jul 8, 2024
c935271
Merge branch 'main' into implement-377
hanbings Jul 31, 2024
2184e7c
Resolving Conflicts.
hanbings Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/find/matchers/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,20 @@ fn build_matcher_tree(

return Ok((i, top_level_matcher.build()));
}
// In our implementation, including the `-exec` parameter,
// it is always run in a single thread.
// Therefore, there is no race condition for now.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "race condition" that this flag is talking about is another command simultaneously modifying the directory while find is reading it, something like

tavianator@graphene $ touch {1..10000}
tavianator@graphene $ find -size 1 & rm ./*
[1] 65991
find: ‘./10000’: No such file or directory
[1]  + 65991 exit 1     find -size 1
tavianator@graphene $ touch {1..10000}
tavianator@graphene $ find -ignore_readdir_race -size 1 & rm ./*
[1] 66034
[1]  + 66034 done       find -ignore_readdir_race -size 1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hanbings ping ? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I forgot about this. The race condition does happen when deleting a large number of files, and I'm rewriting a piece of code that implements this feature.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terribly sorry, I did misunderstand the meaning of readdir_race.

I rechecked the code and read the GNU documentation. The -ignore_readdir_race parameter requires a way to suppress file not found errors globally in the software, so I need to complete #15 to centralize the error handling logic before I can return here.

I will finish them as soon as possible. :)

// and we currently only add the corresponding fields in Config.
//
// Related: https:/uutils/findutils/pull/411#issuecomment-2210638686
"-ignore_readdir_race" => {
config.ignore_readdir_race = true;
None
}
"-noignore_readdir_race" => {
config.ignore_readdir_race = false;
None
}
"-d" | "-depth" => {
// TODO add warning if it appears after actual testing criterion
config.depth_first = true;
Expand Down
33 changes: 33 additions & 0 deletions src/find/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub struct Config {
sorted_output: bool,
help_requested: bool,
version_requested: bool,
ignore_readdir_race: bool,
hanbings marked this conversation as resolved.
Show resolved Hide resolved
}

impl Default for Config {
Expand All @@ -33,6 +34,7 @@ impl Default for Config {
sorted_output: false,
help_requested: false,
version_requested: false,
ignore_readdir_race: false,
}
}
}
Expand Down Expand Up @@ -1219,4 +1221,35 @@ mod tests {

assert_eq!(rc, 0);
}

#[test]
#[cfg(unix)]
fn test_ignore_readdir_race() {
use crate::find::tests::FakeDependencies;

let deps = FakeDependencies::new();
let rc = find_main(
&["find", "./test_data/simple/subdir", "-ignore_readdir_race"],
&deps,
);

assert_eq!(rc, 0);
}

#[test]
fn test_noignore_readdir_race() {
use crate::find::tests::FakeDependencies;

let deps = FakeDependencies::new();
let rc = find_main(
&[
"find",
"./test_data/simple/subdir",
"-noignore_readdir_race",
],
&deps,
);

assert_eq!(rc, 0);
}
}
24 changes: 24 additions & 0 deletions tests/find_cmd_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -881,3 +881,27 @@ fn find_samefile() {
.stdout(predicate::str::is_empty())
.stderr(predicate::str::contains("not-exist-file"));
}

#[test]
#[serial(working_dir)]
fn find_ignore_readdir_race() {
Command::cargo_bin("find")
.expect("found binary")
.args(["./test_data/simple/subdir", "-ignore_readdir_race"])
.assert()
.success()
.stdout(predicate::str::contains("./test_data/simple/subdir"))
.stderr(predicate::str::is_empty());
}

#[test]
#[serial(working_dir)]
fn find_noignore_readdir_race() {
Command::cargo_bin("find")
.expect("found binary")
.args(["./test_data/simple/subdir", "-noignore_readdir_race"])
.assert()
.success()
.stdout(predicate::str::contains("./test_data/simple/subdir"))
.stderr(predicate::str::is_empty());
}
Loading