FindFiles exits and returns no files when it can't open a directory #99
Description
This program
package main
import (
"log"
"os"
"github.com/bitfield/script"
)
func main() {
h, err := os.UserHomeDir()
if err != nil {
log.Fatal(err)
}
_, err = script.FindFiles(h).Stdout()
if err != nil {
log.Fatal(err)
}
}
yields:
$ go build
$ ./script
2022/03/12 11:51:51 open /Users/user/.Trash: operation not permitted
I expected behavior similar to find $HOME -type f, i.e. to return files from directories it can open.
Metadata
Metadata
Assignees
Labels
No labels
Projects
Milestone
Relationships
Development
Participants
Activity
bitfield commentedon Mar 12, 2022
Thanks, @jreisinger! Yes, that seems a reasonable expectation.
[-]FindFiles exits and prints no files when it can't open a directory[/-][+]FindFiles exits and returns no files when it can't open a directory[/+]bitfield commentedon Mar 15, 2022
It seems like FindFiles should just ignore any errors encountered by
filepath.Walk. I think while we're working on that, it would make sense to update FindFiles to use fs.WalkDir instead offilepath.Walk, wouldn't it? Then it would be easier to test (see Walking with filesystems).parkerduckworth commentedon Mar 27, 2022
@bitfield agree that using
fs.WalkDirwithfs.SkipDirwould provide a nice DX here.Thinking that we'd still want to include the permission errors in the output though (rather than completely swallowing), as this is more aligned with the default behavior of
find?Maybe checking to see if the fs.WalkDirFunc returns
fs.ErrPermission, and if so, append<path-to-file>: permission deniedto filenames. Otherwise if any other err type is encountered, it should be piped out WithError.The developer could always just Reject lines containing
"permission denied"from the resulting pipe of a successful find.bitfield commentedon Mar 27, 2022
@parkerduckworth it's a tricky area, because one of the design goals for
scriptis to just get out of your way so that you can cook up quick hacks, and in the event of errors it should just "do the right thing", without blocking your task.parkerduckworth commentedon Mar 27, 2022
bitfield commentedon Jun 2, 2022
Happy to accept PRs for this.
parkerduckworth commentedon Jun 2, 2022
@bitfield great, just submitted one
mahadzaryab1 commentedon Oct 19, 2024
@bitfield what's the status of this? i'd be happy to pick it up if it still needs fixing
bitfield commentedon Oct 20, 2024
@mahadzaryab1 I think this question is still open about the design—what's your view?
FindFilesto ignore errors #217mahadzaryab1 commentedon Nov 2, 2024
@bitfield that sounds good to me! PR open for review at #217
jreisinger commentedon Nov 5, 2024
As for switching from
filepath.Walktofs.Walkdirin #217: shouldn't be the only reason for doing so easier testing? I mean usingfstestinstead of creating folders and files intestdata. @bitfield do you think the switch still makes sense even though the tests haven't been changed?bitfield commentedon Nov 5, 2024
@jreisinger as far as I know, there's no difference, hence no specific reason to change. It's just that if we were writing this function today, we would of course use the FS-based approach. And we are writing it today, in a sense, so I'm fine with the change, unless you think there's a good reason not to make it?
3 remaining items