FindFiles exits and returns no files when it can't open a directory

[jreisinger]

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.

[bitfield]

Thanks, @jreisinger! Yes, that seems a reasonable expectation.

[bitfield]

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 of filepath.Walk, wouldn't it? Then it would be easier to test (see Walking with filesystems).

[parkerduckworth]

@bitfield agree that using fs.WalkDir with fs.SkipDir would 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 denied to 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]

@parkerduckworth it's a tricky area, because one of the design goals for script is 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]

Ah, ok. Understood

…

On Mar 27, 2022, at 4:43 AM, John Arundel ***@***.***> wrote:  @parkerduckworth it's a tricky area, because one of the design goals for script is 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. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[bitfield]

Happy to accept PRs for this.

[parkerduckworth]

@bitfield great, just submitted one

[mahadzaryab1]

@bitfield what's the status of this? i'd be happy to pick it up if it still needs fixing

[bitfield]

@mahadzaryab1 I think this question is still open about the design—what's your view?

[mahadzaryab1]

@bitfield that sounds good to me! PR open for review at #217

[jreisinger]

As for switching from filepath.Walk to fs.Walkdir in #217: shouldn't be the only reason for doing so easier testing? I mean using fstest instead of creating folders and files in testdata. @bitfield do you think the switch still makes sense even though the tests haven't been changed?

[bitfield]

@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?