coreutils icon indicating copy to clipboard operation
coreutils copied to clipboard

`rm`: implement iterator interface

Open tertsdiepraam opened this issue 2 years ago • 6 comments

Part of https://github.com/nushell/nushell/issues/10447#issuecomment-1741883171

This path makes an iterator interface for rm. The iterator is the Remover type, which yields an UResult<()> for every file. nushell can choose to how to print the errors.

To achieve that, I had to make every function return errors instead of printing them. This in turn lead to some big refactors for remove_file and remove_dir. I wish there was a better way to write an iterator like this, but I think it turned out alright. Maybe we can clean it up more in the future.

Oh and walkdir was only getting in the way. So I removed that too.

tertsdiepraam avatar Oct 01 '23 16:10 tertsdiepraam

GNU testsuite comparison:

Congrats! The gnu test tests/cksum/b2sum is no longer failing!
Congrats! The gnu test tests/cp/link-deref is no longer failing!
Congrats! The gnu test tests/install/basic-1 is no longer failing!
Congrats! The gnu test tests/misc/nohup is no longer failing!
Congrats! The gnu test tests/mv/mv-special-1 is no longer failing!
Congrats! The gnu test tests/mv/trailing-slash is no longer failing!
Congrats! The gnu test tests/rm/ir-1 is no longer failing!
Congrats! The gnu test tests/split/filter is no longer failing!
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/cycle. tests/rm/cycle is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/empty-name. tests/rm/empty-name is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/f-1. tests/rm/f-1 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/fail-eacces. tests/rm/fail-eacces is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/ignorable. tests/rm/ignorable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/inaccessible. tests/rm/inaccessible is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/rm/rm1
GNU test failed: tests/rm/unread2. tests/rm/unread2 is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/chmod/usage. tests/chmod/usage is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/cksum/b2sum. tests/cksum/b2sum is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/cp/link-deref. tests/cp/link-deref is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/cp/reflink-auto. tests/cp/reflink-auto is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/install/basic-1. tests/install/basic-1 is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/ln/misc. tests/ln/misc is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/misc/nohup. tests/misc/nohup is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/backup-is-src. tests/mv/backup-is-src is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/into-self. tests/mv/into-self is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/mv-special-1. tests/mv/mv-special-1 is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/trailing-slash. tests/mv/trailing-slash is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/rm/interactive-always. tests/rm/interactive-always is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/rm/interactive-once. tests/rm/interactive-once is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/rm/r-root. tests/rm/r-root is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/split/filter. tests/split/filter is passing on 'main'. Maybe you have to rebase?

github-actions[bot] avatar Oct 01 '23 17:10 github-actions[bot]

Looks like I got some tests to fix 😄

tertsdiepraam avatar Oct 01 '23 17:10 tertsdiepraam

GNU testsuite comparison:

Congrats! The gnu test tests/cksum/b2sum is no longer failing!
Congrats! The gnu test tests/cp/link-deref is no longer failing!
Congrats! The gnu test tests/install/basic-1 is no longer failing!
Congrats! The gnu test tests/misc/nohup is no longer failing!
Congrats! The gnu test tests/mv/mv-special-1 is no longer failing!
Congrats! The gnu test tests/mv/trailing-slash is no longer failing!
Congrats! The gnu test tests/rm/ir-1 is no longer failing!
Congrats! The gnu test tests/split/filter is no longer failing!
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/cycle. tests/rm/cycle is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/f-1. tests/rm/f-1 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/fail-eacces. tests/rm/fail-eacces is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/ignorable. tests/rm/ignorable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/inaccessible. tests/rm/inaccessible is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/unread2. tests/rm/unread2 is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/chmod/usage. tests/chmod/usage is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/cksum/b2sum. tests/cksum/b2sum is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/cp/link-deref. tests/cp/link-deref is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/cp/reflink-auto. tests/cp/reflink-auto is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/install/basic-1. tests/install/basic-1 is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/ln/misc. tests/ln/misc is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/misc/nohup. tests/misc/nohup is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/backup-is-src. tests/mv/backup-is-src is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/into-self. tests/mv/into-self is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/mv-special-1. tests/mv/mv-special-1 is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/trailing-slash. tests/mv/trailing-slash is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/rm/interactive-always. tests/rm/interactive-always is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/rm/interactive-once. tests/rm/interactive-once is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/rm/r-root. tests/rm/r-root is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/split/filter. tests/split/filter is passing on 'main'. Maybe you have to rebase?

github-actions[bot] avatar Oct 01 '23 21:10 github-actions[bot]

GNU testsuite comparison:

Congrats! The gnu test tests/cksum/b2sum is no longer failing!
Congrats! The gnu test tests/cp/link-deref is no longer failing!
Congrats! The gnu test tests/install/basic-1 is no longer failing!
Congrats! The gnu test tests/misc/nohup is no longer failing!
Congrats! The gnu test tests/mv/mv-special-1 is no longer failing!
Congrats! The gnu test tests/mv/trailing-slash is no longer failing!
Congrats! The gnu test tests/rm/ir-1 is no longer failing!
Congrats! The gnu test tests/split/filter is no longer failing!
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/cycle. tests/rm/cycle is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/f-1. tests/rm/f-1 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/fail-eacces. tests/rm/fail-eacces is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/ignorable. tests/rm/ignorable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/inaccessible. tests/rm/inaccessible is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/unread2. tests/rm/unread2 is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/chmod/usage. tests/chmod/usage is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/cksum/b2sum. tests/cksum/b2sum is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/cp/link-deref. tests/cp/link-deref is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/cp/reflink-auto. tests/cp/reflink-auto is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/install/basic-1. tests/install/basic-1 is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/ln/misc. tests/ln/misc is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/misc/nohup. tests/misc/nohup is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/backup-is-src. tests/mv/backup-is-src is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/into-self. tests/mv/into-self is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/mv-special-1. tests/mv/mv-special-1 is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/mv/trailing-slash. tests/mv/trailing-slash is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/rm/interactive-always. tests/rm/interactive-always is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/rm/interactive-once. tests/rm/interactive-once is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/rm/r-root. tests/rm/r-root is passing on 'main'. Maybe you have to rebase?
GNU test error: tests/split/filter. tests/split/filter is passing on 'main'. Maybe you have to rebase?

github-actions[bot] avatar Oct 02 '23 10:10 github-actions[bot]

GNU testsuite comparison:

Congrats! The gnu test tests/rm/ir-1 is no longer failing!
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/inaccessible. tests/rm/inaccessible is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/unread2. tests/rm/unread2 is passing on 'main'. Maybe you have to rebase?

github-actions[bot] avatar Oct 06 '23 12:10 github-actions[bot]

@tertsdiepraam sorry but it needs some rebasing

sylvestre avatar Dec 25 '23 12:12 sylvestre

any updates here?

fdncred avatar Feb 23 '24 19:02 fdncred

@fdncred Oof, it's been a while. I might have some ideas to bring this forward. An iterator may not be the right approach here. The code was getting too complicated.

But, I think we have missed the easy solution here: calling remove multiple times with a single file. I think that's much easier for us to build and for nushell to use. If I recall correctly, that should be enough for nushells's usecase.

tertsdiepraam avatar Feb 25 '24 10:02 tertsdiepraam

I'm gonna go with a 2 step plan here:

  1. Take some of the refactors I did and extract them into a separate PR.
  2. And then re-evaluate the error handling. Either with a channel or an iterator or a callback kind of thing.

tertsdiepraam avatar Feb 25 '24 12:02 tertsdiepraam