Implement `-ignore_readdir_race` and `-noignore_readdir_race`.
This PR will pass the test about race conditions in BFS test. Our implementation does not seem to cause race conditions, so only add Config related to -ignore_readdir_race and -noignore_readdir_race without making other logical changes.
Closes #377
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 66.29%. Comparing base (
5be00c8) to head (2184e7c).
Additional details and impacted files
@@ Coverage Diff @@
## main #411 +/- ##
==========================================
+ Coverage 66.25% 66.29% +0.03%
==========================================
Files 34 34
Lines 4004 4017 +13
Branches 911 911
==========================================
+ Hits 2653 2663 +10
- Misses 996 998 +2
- Partials 355 356 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Commit f4bb537b5511117023412c09cf7ffa57a25e74b2 has GNU testsuite comparison:
Run GNU findutils tests: GNU tests summary = TOTAL: 702 / PASS: 445 / FAIL: 254 / ERROR: 2
Run GNU findutils tests: Changes from main: PASS +0 / FAIL +0 / ERROR +0 / SKIP +0
Run GNU findutils tests: Gnu tests No changes
Run BFS tests: Changes from main: PASS +2 / SKIP +0 / FAIL -2
Run BFS tests: BFS tests summary = TOTAL: 288 / PASS: 192 / SKIP: 1 / FAIL: 95
could you please document the fact that this option doesn't do anything ?
could you please document the fact that this option doesn't do anything ?
Sorry for the late reply.
In short, taking the test https://github.com/tavianator/bfs/blob/main/tests/gnu/ignore_readdir_race.sh as an example, during the execution of -exec, our main thread will always be blocked by the -exec executor, so there will be no race condition.
But I guess this problem may not appear until https://github.com/uutils/findutils/issues/6 is implemented or https://github.com/uutils/findutils/issues/366 is fixed.
Wirte up
I usually start by checking the GNU tests and the BFS tests for this parameter.
The test project for -ignore_readdir_race is at https://github.com/tavianator/bfs/blob/main/tests/gnu/ignore_readdir_race.sh
ignore_readdir_race.sh
cd "$TEST"
"$XTOUCH" foo bar
# -links 1 forces a stat() call, which will fail for the second file
invoke_bfs . -mindepth 1 -ignore_readdir_race -links 1 -exec "$TESTS/remove-sibling.sh" {} \;
remove-sibling.sh
#!/usr/bin/env bash
for file in "${1%/*}"/*; do
if [ "$file" != "$1" ]; then
rm "$file"
exit $?
fi
done
exit 1
I then ran the script on my local machine:
/tmp/test_dir$ touch foo bar
# and copy remove-sibling.sh
/tmp/test_dir$ ls
foo bar remove-sibling.sh
# GNU find
/tmp/test_dir$ find . -mindepth 1 -links 1 -exec "./remove-sibling.sh" {} \;
find: ‘./bar’: No such file or directory
/tmp/test_dir$ ls
remove-sibling.sh
# uutils/find
/tmp/test_dir$ touch foo bar
/tmp/test_dir$ ls
foo bar remove-sibling.sh
/tmp/test_dir$ ... findutils/target/debug/find . -mindepth 1 -links 1 -exec "./remove-sibling.sh" {} \;
/tmp/test_dir$ ls
remove-sibling.sh
Then I realized that our implementation didn't seem to have this problem in the same test. OK, so let's look at the code.
First we jump here from the main function, then turns to the parser parse_args which parses the arguments.
https://github.com/uutils/findutils/blob/main/src/find/mod.rs#L176
fn do_find<'a>(args: &[&str], deps: &'a dyn Dependencies<'a>) -> Result<u64, Box<dyn Error>> {
let paths_and_matcher = parse_args(args)?;
...
}
After getting all the matchers that meet the parameters, use process_dir to traverse the directory.
https://github.com/uutils/findutils/blob/main/src/find/mod.rs#L189
found_count += process_dir(
&path,
&paths_and_matcher.config,
deps,
&*paths_and_matcher.matcher,
&mut quit,
);
Here, matcher.matches is executed once for each directory entry.
So far, everything has been executed linearly, and no parallel processing has occurred.
https://github.com/uutils/findutils/blob/main/src/find/mod.rs#L159
while let Some(result) = it.next() {
match result {
Err(err) => {
uucore::error::set_exit_code(1);
writeln!(&mut stderr(), "Error: {dir}: {err}").unwrap()
}
Ok(entry) => {
let mut matcher_io = matchers::MatcherIO::new(deps);
if matcher.matches(&entry, &mut matcher_io) {
found_count += 1;
}
...
}
}
}
Finally, there is the part of exec.rs that executes the instructions.
When executing a command, we must wait for the command to complete before returning a result of whether the execution is completed (of course, an error message is output on the screen here.)
https://github.com/uutils/findutils/blob/main/src/find/matchers/exec.rs#L87
...
match command.status() {
Ok(status) => status.success(),
Err(e) => {
writeln!(&mut stderr(), "Failed to run {}: {}", self.executable, e).unwrap();
false
}
}
...
Therefore, in the case of -exec, we always complete all work in sequence within one thread, and there will be no situation where instructions are not completed but uutils/findutils is still traversing.
oh, sorry, i just meant "in the code. explaining why it isn't doing anything" :)
oh, sorry, i just meant "in the code. explaining why it isn't doing anything" :)
Ok, I've submitted the description in my latest commit.
Commit 74136375caf31aaee155c1a1abe487d821a96b74 has GNU testsuite comparison:
Run GNU findutils tests: GNU tests summary = TOTAL: 702 / PASS: 445 / FAIL: 254 / ERROR: 2
Run GNU findutils tests: Changes from main: PASS +0 / FAIL +0 / ERROR +0 / SKIP +0
Run GNU findutils tests: Gnu tests No changes
Run BFS tests: Changes from main: PASS +2 / SKIP +0 / FAIL -2
Run BFS tests: BFS tests summary = TOTAL: 288 / PASS: 192 / SKIP: 1 / FAIL: 95
Commit c9a36d00dcb5188c8b9c0261b7a9c4ad08abcfe7 has GNU testsuite comparison:
Run BFS tests: Changes from main: PASS +2 / SKIP +0 / FAIL -2
Run BFS tests: BFS tests summary = TOTAL: 288 / PASS: 192 / SKIP: 1 / FAIL: 95
Run GNU findutils tests: GNU tests summary = TOTAL: 702 / PASS: 445 / FAIL: 254 / ERROR: 2
Run GNU findutils tests: Changes from main: PASS +0 / FAIL +0 / ERROR +0 / SKIP +0
Run GNU findutils tests: Gnu tests No changes
Commit 9250adc7898307b49124ffb6811ee905bba93cef has GNU testsuite comparison:
Run GNU findutils tests: GNU tests summary = TOTAL: 702 / PASS: 445 / FAIL: 254 / ERROR: 2
Run GNU findutils tests: Changes from main: PASS +0 / FAIL +0 / ERROR +0 / SKIP +0
Run GNU findutils tests: Gnu tests No changes
Run BFS tests: Changes from main: PASS +2 / SKIP +0 / FAIL -2
Run BFS tests: BFS tests summary = TOTAL: 288 / PASS: 192 / SKIP: 1 / FAIL: 95
needs rebase
Commit 2184e7c3a0d495237fbced271af49cf53a57f1b2 has GNU testsuite comparison:
Run BFS tests: Changes from main: PASS +2 / SKIP +0 / FAIL -2
Run BFS tests: BFS tests summary = TOTAL: 288 / PASS: 195 / SKIP: 1 / FAIL: 92
Run GNU findutils tests: GNU tests summary = TOTAL: 702 / PASS: 445 / FAIL: 254 / ERROR: 2
Run GNU findutils tests: Changes from main: PASS +0 / FAIL +0 / ERROR +0 / SKIP +0
Run GNU findutils tests: Gnu tests No changes
needs rebasing
@hanbings do you have an update on this ? thanks :)
@hanbings do you have an update on this ? thanks :)
There is nothing new here yet, and completing this PR requires completing the global error handling associated with this feature (which is https://github.com/uutils/findutils/issues/15). Before that I would like to complete https://github.com/uutils/findutils/pull/369 first. :)
@hanbings any plan to finish it ? thanks :)