Implement : -files0-from (#378) #509

Crypto-Darth · 2025-03-22T20:53:21Z

Implements : #378

Huge Thanks to @hanbings for the implementation guide.

Crypto-Darth · 2025-03-22T20:55:41Z

I will add more tests as soon as possible.

codecov · 2025-03-22T21:18:47Z

Codecov Report

Attention: Patch coverage is 78.94737% with 12 lines in your changes missing coverage. Please review.

Project coverage is 87.47%. Comparing base (cb83b6d) to head (f3ec65c).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/find/mod.rs	41.66%	5 Missing and 2 partials ⚠️
src/find/matchers/mod.rs	88.88%	0 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #509      +/-   ##
==========================================
- Coverage   87.55%   87.47%   -0.08%     
==========================================
  Files          31       31              
  Lines        6715     6770      +55     
  Branches      300      305       +5     
==========================================
+ Hits         5879     5922      +43     
- Misses        623      628       +5     
- Partials      213      220       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

src/find/mod.rs

Copilot

Pull Request Overview

This PR implements support for the "-files0-from" argument, allowing the program to read starting points from either stdin or a specified file, in line with GNU findutils behavior. Key changes include:

Adding the "from_file" field to the configuration structure.
Integrating logic in "parse_args" to handle "-files0-from" for both piped input and file-based input.
Adding tests in both the core module and integration tests along with an updated Cargo.toml dependency.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/find/mod.rs	Added support for "-files0-from" argument and file/stdi management.
tests/find_cmd_tests.rs	Introduced new tests validating "-files0-from" behavior under various conditions.
Cargo.toml	Added the "atty" dependency required for checking the STDIN state.

Comments suppressed due to low confidence (1)

src/find/mod.rs:168

[nitpick] The error message for an empty starting point in the file branch includes an extra phrase ("OR File is empty") compared to the STDIN branch. Consider unifying the messages to improve consistency.

return Err("Empty starting point detected in -files0-from input OR File is empty".into());

src/find/mod.rs

Crypto-Darth · 2025-03-24T06:11:46Z

I have migrated the tests so as to avoid using panic! ,works as intended.
Please review @sylvestre .

Thank you.

src/find/mod.rs

Crypto-Darth · 2025-04-01T07:50:04Z

@tavianator I'm still clueless on how should I go on to fix testcase 2 , my solution above won't work
Would request your suggestion.

src/find/matchers/mod.rs

tavianator · 2025-04-01T11:46:33Z

@tavianator I'm still clueless on how should I go on to fix testcase 2 , my solution above won't work Would request your suggestion.

So looking more at how GNU find works here, I think what you should do is: when you see -files0-from, save the argument in the Config, but don't open/read it yet. Then, after parsing everything, if you saved a -files0-from argument, go read it and add all the root paths. At that point you can also check if any other paths were already given, and give an error instead.

The reason I suggest this is that only the last -files0-from argument actually takes effect:

tavianator@graphene $ printf a > a
tavianator@graphene $ printf b > b
tavianator@graphene $ find -files0-from a -files0-from b
b

This is intentional by GNU find: https://savannah.gnu.org/bugs/?66965

Co-authored-by: Tavian Barnes <[email protected]>

…erand + Test remove

Crypto-Darth · 2025-04-01T14:34:27Z

So looking more at how GNU find works here, I think what you should do is: when you see -files0-from, save the argument in the Config, but don't open/read it yet. Then, after parsing everything, if you saved a -files0-from argument, go read it and add all the root paths. At that point you can also check if any other paths were already given, and give an error instead.

@tavianator ,
I think I found a better way by writing to the new paths directly via insert() rather than get_and_insert().
Therefore if there's more than 1 usage of -files0-from the last one will be considered.

Regarding the original Issue (Test 2) is now also solved.

Please review the latest commit .
Thank you.

tavianator · 2025-04-01T14:47:13Z

@tavianator ,
I think I found a better way by writing to the new paths directly via insert() rather than get_and_insert().
Therefore if there's more than 1 usage of -files0-from the last one will be considered.

That would almost work, but you also have to ensure that

$ find -files0-from - -files0-from file

doesn't read from stdin at all, so I think it's easiest to delay reading the file.

Crypto-Darth · 2025-04-01T19:09:46Z

@tavianator ,
I think I found a better way by writing to the new paths directly via insert() rather than get_and_insert().
Therefore if there's more than 1 usage of -files0-from the last one will be considered.

That would almost work, but you also have to ensure that
$ find -files0-from - -files0-from file
doesn't read from stdin at all, so I think it's easiest to delay reading the file.

To be really honest that is a very edge case , I don't see any user or a script using -files0-from twice but I understand the need for maximum compatibility hence, I have fixed it as per your suggestion in latest commit.
@tavianator

tavianator · 2025-04-01T19:57:47Z

src/find/matchers/mod.rs

+        .collect();
+    // empty starting point checker
+    if string_segments.iter().any(|s| s.is_empty()) {
+        println!("find: invalid zero-length file name");


This should be eprintln!(), and you should also set the exit code to 1

Any clue on how can I set the exit code to 1?
I don't see how current implementation allows me to set custom exit code without exiting in between of execution.

Seems like it would take more refactoring to make that work nicely. Feel free to defer it to a future PR

src/find/matchers/mod.rs

Co-authored-by: Tavian Barnes <[email protected]>

src/find/mod.rs

Co-authored-by: Tavian Barnes <[email protected]>

tavianator · 2025-04-01T21:08:26Z

This LGTM now except for those few nits. Sorry for so much back and forth, it's a surprisingly finicky feature!

Crypto-Darth · 2025-04-01T21:10:34Z

This LGTM now except for those few nits. Sorry for so much back and forth, it's a surprisingly finicky feature!

No worries :)
Really got to learn a lot.
Thanks a lot to you for your time and patience :)
Thanks to @sylvestre and @tertsdiepraam too for their time and reviews.

Crypto-Darth · 2025-04-02T07:39:21Z

@hanbings @sylvestre ,
Can we get this merged now if all good to go ? :)

hanbings · 2025-04-02T12:48:09Z

This is truly amazing work! Many thanks to all the contributors!

Crypto-Darth added 5 commits March 23, 2025 00:22

impliment : -files0-from

338481b

cargo fmt

a8dcfc4

update : documentation + minor fix

7e57ae3

tests : added 2 basic tests

20b0686

cargo fmt

ca225d0

hanbings linked an issue Mar 23, 2025 that may be closed by this pull request

Implement -files0-from #378

Closed

new : tests

07be798

Crypto-Darth marked this pull request as ready for review March 23, 2025 12:52

sylvestre requested a review from Copilot March 23, 2025 13:27

sylvestre reviewed Mar 23, 2025

View reviewed changes

src/find/mod.rs Outdated Show resolved Hide resolved

sylvestre reviewed Mar 23, 2025

View reviewed changes

src/find/mod.rs Outdated Show resolved Hide resolved

Copilot AI reviewed Mar 23, 2025

View reviewed changes

src/find/mod.rs Outdated Show resolved Hide resolved

Crypto-Darth added 2 commits March 24, 2025 11:29

Migrate: Tests

8f517b1

fix : spelling

74e47aa

Crypto-Darth requested a review from sylvestre March 24, 2025 06:11

fix: -ok and -okdir temporary block

29a1188

sylvestre reviewed Mar 25, 2025

View reviewed changes

src/find/mod.rs Outdated Show resolved Hide resolved

move : code block to function

b5444f8

sylvestre reviewed Mar 25, 2025

View reviewed changes

src/find/mod.rs Outdated Show resolved Hide resolved

update: match to if else

6e612a1

Crypto-Darth requested a review from sylvestre March 25, 2025 16:27

tavianator reviewed Mar 25, 2025

View reviewed changes

src/find/mod.rs Outdated Show resolved Hide resolved

Crypto-Darth added 2 commits March 26, 2025 01:03

fix : trim file if starting/ending is NUL + Test fix

51f1a73

fix : for files staring AND ending with ASCII NUL

da37be9

Crypto-Darth requested a review from tavianator March 25, 2025 19:53

tavianator reviewed Mar 25, 2025

View reviewed changes

src/find/mod.rs Outdated Show resolved Hide resolved

fix : check if file / pipe ends with NUL + Test fix

6b31453

Crypto-Darth added 2 commits April 1, 2025 01:20

fix : Error messages to help pass testcases.

7838f44

fix : tests + cargo fmt

b317a46

tavianator reviewed Apr 1, 2025

View reviewed changes

src/find/matchers/mod.rs Outdated Show resolved Hide resolved

tavianator reviewed Apr 1, 2025

View reviewed changes

src/find/matchers/mod.rs Outdated Show resolved Hide resolved

Crypto-Darth and others added 3 commits April 1, 2025 17:30

Update src/find/matchers/mod.rs

f26c0e0

Co-authored-by: Tavian Barnes <[email protected]>

Update src/find/matchers/mod.rs

b7a00f8

Co-authored-by: Tavian Barnes <[email protected]>

Fix : take last argument is same argument > 1 +error on extra file op…

6ea26ee

…erand + Test remove

Crypto-Darth requested a review from tavianator April 1, 2025 14:36

refactor entry point for parse_file0_args + Remove is_stdin from Config

22cfd63

tavianator reviewed Apr 1, 2025

View reviewed changes

println! to eprintln! + Test fix

ac70b59

tavianator reviewed Apr 1, 2025

View reviewed changes

src/find/matchers/mod.rs Outdated Show resolved Hide resolved

tavianator reviewed Apr 1, 2025

View reviewed changes

src/find/matchers/mod.rs Outdated Show resolved Hide resolved

tavianator reviewed Apr 1, 2025

View reviewed changes

src/find/matchers/mod.rs Outdated Show resolved Hide resolved

Update src/find/matchers/mod.rs

6cf2a99

Co-authored-by: Tavian Barnes <[email protected]>

tavianator reviewed Apr 1, 2025

View reviewed changes

src/find/mod.rs Outdated Show resolved Hide resolved

Crypto-Darth and others added 3 commits April 2, 2025 02:34

Update src/find/matchers/mod.rs

c1d3e45

Co-authored-by: Tavian Barnes <[email protected]>

Update src/find/mod.rs

d5853ca

Co-authored-by: Tavian Barnes <[email protected]>

minor changes

52e721f

Merge branch 'main' into main

f3ec65c

hanbings merged commit 1a40442 into uutils:main Apr 2, 2025
18 of 20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement : -files0-from (#378) #509

Implement : -files0-from (#378) #509

Crypto-Darth commented Mar 22, 2025

Crypto-Darth commented Mar 22, 2025

codecov bot commented Mar 22, 2025 •

edited

Loading

Copilot AI left a comment

Crypto-Darth commented Mar 24, 2025

Crypto-Darth commented Apr 1, 2025 •

edited

Loading

tavianator commented Apr 1, 2025

Crypto-Darth commented Apr 1, 2025

tavianator commented Apr 1, 2025

Crypto-Darth commented Apr 1, 2025 •

edited

Loading

tavianator Apr 1, 2025

Crypto-Darth Apr 1, 2025

tavianator Apr 1, 2025

Crypto-Darth Apr 1, 2025

tavianator commented Apr 1, 2025

Crypto-Darth commented Apr 1, 2025

Crypto-Darth commented Apr 2, 2025

hanbings commented Apr 2, 2025

Implement : -files0-from (#378) #509

Implement : -files0-from (#378) #509

Conversation

Crypto-Darth commented Mar 22, 2025

Crypto-Darth commented Mar 22, 2025

codecov bot commented Mar 22, 2025 • edited Loading

Codecov Report

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Crypto-Darth commented Mar 24, 2025

Crypto-Darth commented Apr 1, 2025 • edited Loading

tavianator commented Apr 1, 2025

Crypto-Darth commented Apr 1, 2025

tavianator commented Apr 1, 2025

Crypto-Darth commented Apr 1, 2025 • edited Loading

tavianator Apr 1, 2025

Choose a reason for hiding this comment

Crypto-Darth Apr 1, 2025

Choose a reason for hiding this comment

tavianator Apr 1, 2025

Choose a reason for hiding this comment

Crypto-Darth Apr 1, 2025

Choose a reason for hiding this comment

tavianator commented Apr 1, 2025

Crypto-Darth commented Apr 1, 2025

Crypto-Darth commented Apr 2, 2025

hanbings commented Apr 2, 2025

codecov bot commented Mar 22, 2025 •

edited

Loading

Crypto-Darth commented Apr 1, 2025 •

edited

Loading

Crypto-Darth commented Apr 1, 2025 •

edited

Loading