From d0c126210f68c47cf58be77eb8e545cebb13ce88 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 2 Nov 2024 16:19:41 -0400 Subject: [PATCH 1/9] Switch To Using fs.WalkDir --- script.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/script.go b/script.go index 2d39bdb..383146c 100644 --- a/script.go +++ b/script.go @@ -10,6 +10,7 @@ import ( "fmt" "hash" "io" + "io/fs" "math" "net/http" "os" @@ -92,12 +93,12 @@ func File(path string) *Pipe { // test/2.txt func FindFiles(dir string) *Pipe { var paths []string - err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + err := fs.WalkDir(os.DirFS(dir), ".", func(path string, d fs.DirEntry, err error) error { if err != nil { return err } - if !info.IsDir() { - paths = append(paths, path) + if !d.IsDir() { + paths = append(paths, filepath.Join(dir, path)) } return nil }) From 9511d86856a38a8319ca6c15bc149ad945965d4a Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 2 Nov 2024 16:20:55 -0400 Subject: [PATCH 2/9] Return Error Only If There Are No Paths Found --- script.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script.go b/script.go index 383146c..3891b58 100644 --- a/script.go +++ b/script.go @@ -102,7 +102,7 @@ func FindFiles(dir string) *Pipe { } return nil }) - if err != nil { + if err != nil && len(paths) == 0 { return NewPipe().WithError(err) } return Slice(paths) From 28365c1becb9ce2cae44ea10dea170b155f3f2c4 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 10 Nov 2024 15:32:49 -0500 Subject: [PATCH 3/9] Add Unit Test For Case When There Is An Error --- script_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/script_test.go b/script_test.go index a3f9124..802691b 100644 --- a/script_test.go +++ b/script_test.go @@ -1286,6 +1286,39 @@ func TestFindFiles_InNonexistentPathReturnsError(t *testing.T) { } } +func TestFindFiles_DoesNotErrorWhenSubDirectoryIsNotReadable(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + fileAPath := filepath.Join(tmpDir, "file_a.txt") + if err := os.WriteFile(fileAPath, []byte("hello world!"), os.ModePerm); err != nil { + t.Fatal(err) + } + restrictedDirPath := filepath.Join(tmpDir, "restricted_dir") + if err := os.Mkdir(restrictedDirPath, os.ModePerm); err != nil { + t.Fatal(err) + } + fileBPath := filepath.Join(restrictedDirPath, "file_b.txt") + if err := os.WriteFile(fileBPath, []byte("hello again!"), os.ModePerm); err != nil { + t.Fatal(err) + } + if err := os.Chmod(restrictedDirPath, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(restrictedDirPath, os.ModePerm) }) + p := script.FindFiles(tmpDir) + if p.Error() != nil { + t.Fatal(p.Error()) + } + want := fileAPath + "\n" + got, err := p.String() + if err != nil { + t.Fatal(err) + } + if !cmp.Equal(want, got) { + t.Fatal(cmp.Diff(want, got)) + } +} + func TestIfExists_ProducesErrorPlusNoOutputForNonexistentFile(t *testing.T) { t.Parallel() want := "" From 9f2b623dd9f5d4b68f5bf4594d7b104cd8088df9 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Tue, 12 Nov 2024 07:10:15 -0500 Subject: [PATCH 4/9] Address Feedback --- script_test.go | 33 --------------------------------- script_unix_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/script_test.go b/script_test.go index 802691b..a3f9124 100644 --- a/script_test.go +++ b/script_test.go @@ -1286,39 +1286,6 @@ func TestFindFiles_InNonexistentPathReturnsError(t *testing.T) { } } -func TestFindFiles_DoesNotErrorWhenSubDirectoryIsNotReadable(t *testing.T) { - t.Parallel() - tmpDir := t.TempDir() - fileAPath := filepath.Join(tmpDir, "file_a.txt") - if err := os.WriteFile(fileAPath, []byte("hello world!"), os.ModePerm); err != nil { - t.Fatal(err) - } - restrictedDirPath := filepath.Join(tmpDir, "restricted_dir") - if err := os.Mkdir(restrictedDirPath, os.ModePerm); err != nil { - t.Fatal(err) - } - fileBPath := filepath.Join(restrictedDirPath, "file_b.txt") - if err := os.WriteFile(fileBPath, []byte("hello again!"), os.ModePerm); err != nil { - t.Fatal(err) - } - if err := os.Chmod(restrictedDirPath, 0o000); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { os.Chmod(restrictedDirPath, os.ModePerm) }) - p := script.FindFiles(tmpDir) - if p.Error() != nil { - t.Fatal(p.Error()) - } - want := fileAPath + "\n" - got, err := p.String() - if err != nil { - t.Fatal(err) - } - if !cmp.Equal(want, got) { - t.Fatal(cmp.Diff(want, got)) - } -} - func TestIfExists_ProducesErrorPlusNoOutputForNonexistentFile(t *testing.T) { t.Parallel() want := "" diff --git a/script_unix_test.go b/script_unix_test.go index 40a98c8..5f397a6 100644 --- a/script_unix_test.go +++ b/script_unix_test.go @@ -3,6 +3,8 @@ package script_test import ( + "os" + "path/filepath" "testing" "github.com/bitfield/script" @@ -106,6 +108,38 @@ func TestExecPipesDataToExternalCommandAndGetsExpectedOutput(t *testing.T) { } } +func TestFindFiles_DoesNotErrorWhenSubDirectoryIsNotReadable(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + fileAPath := filepath.Join(tmpDir, "file_a.txt") + if err := os.WriteFile(fileAPath, []byte("hello world!"), os.ModePerm); err != nil { + t.Fatal(err) + } + restrictedDirPath := filepath.Join(tmpDir, "restricted_dir") + if err := os.Mkdir(restrictedDirPath, os.ModePerm); err != nil { + t.Fatal(err) + } + fileBPath := filepath.Join(restrictedDirPath, "file_b.txt") + if err := os.WriteFile(fileBPath, []byte("hello again!"), os.ModePerm); err != nil { + t.Fatal(err) + } + if err := os.Chmod(restrictedDirPath, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(restrictedDirPath, os.ModePerm) }) + got, err := script.FindFiles(tmpDir).String() + if err != nil { + t.Fatal(err) + } + want := fileAPath + "\n" + if err != nil { + t.Fatal(err) + } + if !cmp.Equal(want, got) { + t.Fatal(cmp.Diff(want, got)) + } +} + func ExampleExec_ok() { script.Exec("echo Hello, world!").Stdout() // Output: From fa20e7ac50cee5fec42b56e3c2f79d4c3a67a2de Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Wed, 13 Nov 2024 07:00:56 -0500 Subject: [PATCH 5/9] Skip Directory If Permission Error --- script.go | 3 +++ script_unix_test.go | 14 +++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/script.go b/script.go index 3891b58..6c03ed7 100644 --- a/script.go +++ b/script.go @@ -95,6 +95,9 @@ func FindFiles(dir string) *Pipe { var paths []string err := fs.WalkDir(os.DirFS(dir), ".", func(path string, d fs.DirEntry, err error) error { if err != nil { + if os.IsPermission(err) { + return fs.SkipDir + } return err } if !d.IsDir() { diff --git a/script_unix_test.go b/script_unix_test.go index 5f397a6..4e536ee 100644 --- a/script_unix_test.go +++ b/script_unix_test.go @@ -111,16 +111,16 @@ func TestExecPipesDataToExternalCommandAndGetsExpectedOutput(t *testing.T) { func TestFindFiles_DoesNotErrorWhenSubDirectoryIsNotReadable(t *testing.T) { t.Parallel() tmpDir := t.TempDir() - fileAPath := filepath.Join(tmpDir, "file_a.txt") - if err := os.WriteFile(fileAPath, []byte("hello world!"), os.ModePerm); err != nil { + restrictedDirPath := filepath.Join(tmpDir, "a_restricted_dir") + if err := os.Mkdir(restrictedDirPath, os.ModePerm); err != nil { t.Fatal(err) } - restrictedDirPath := filepath.Join(tmpDir, "restricted_dir") - if err := os.Mkdir(restrictedDirPath, os.ModePerm); err != nil { + fileAPath := filepath.Join(restrictedDirPath, "file_a.txt") + if err := os.WriteFile(fileAPath, []byte("hello again!"), os.ModePerm); err != nil { t.Fatal(err) } - fileBPath := filepath.Join(restrictedDirPath, "file_b.txt") - if err := os.WriteFile(fileBPath, []byte("hello again!"), os.ModePerm); err != nil { + fileBPath := filepath.Join(tmpDir, "file_b.txt") + if err := os.WriteFile(fileBPath, []byte("hello world!"), os.ModePerm); err != nil { t.Fatal(err) } if err := os.Chmod(restrictedDirPath, 0o000); err != nil { @@ -131,7 +131,7 @@ func TestFindFiles_DoesNotErrorWhenSubDirectoryIsNotReadable(t *testing.T) { if err != nil { t.Fatal(err) } - want := fileAPath + "\n" + want := fileBPath + "\n" if err != nil { t.Fatal(err) } From 0b74dea31b15d6a597cf175c3f5ad4bdb8f6dac6 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sat, 16 Nov 2024 15:10:30 -0500 Subject: [PATCH 6/9] Remove Unncessary File --- script_unix_test.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/script_unix_test.go b/script_unix_test.go index 4e536ee..71e2fae 100644 --- a/script_unix_test.go +++ b/script_unix_test.go @@ -112,26 +112,18 @@ func TestFindFiles_DoesNotErrorWhenSubDirectoryIsNotReadable(t *testing.T) { t.Parallel() tmpDir := t.TempDir() restrictedDirPath := filepath.Join(tmpDir, "a_restricted_dir") - if err := os.Mkdir(restrictedDirPath, os.ModePerm); err != nil { + if err := os.Mkdir(restrictedDirPath, 0o000); err != nil { t.Fatal(err) } - fileAPath := filepath.Join(restrictedDirPath, "file_a.txt") - if err := os.WriteFile(fileAPath, []byte("hello again!"), os.ModePerm); err != nil { + fileAPath := filepath.Join(tmpDir, "file_a.txt") + if err := os.WriteFile(fileAPath, []byte("hello world!"), os.ModePerm); err != nil { t.Fatal(err) } - fileBPath := filepath.Join(tmpDir, "file_b.txt") - if err := os.WriteFile(fileBPath, []byte("hello world!"), os.ModePerm); err != nil { - t.Fatal(err) - } - if err := os.Chmod(restrictedDirPath, 0o000); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { os.Chmod(restrictedDirPath, os.ModePerm) }) got, err := script.FindFiles(tmpDir).String() if err != nil { t.Fatal(err) } - want := fileBPath + "\n" + want := fileAPath + "\n" if err != nil { t.Fatal(err) } From 69e60551db8c93ae9b454e0191b630a0269e7795 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 17 Nov 2024 15:12:10 -0500 Subject: [PATCH 7/9] Make WalkDir Func a Closure Signed-off-by: Mahad Zaryab --- script.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/script.go b/script.go index 6c03ed7..96aacf5 100644 --- a/script.go +++ b/script.go @@ -93,20 +93,19 @@ func File(path string) *Pipe { // test/2.txt func FindFiles(dir string) *Pipe { var paths []string - err := fs.WalkDir(os.DirFS(dir), ".", func(path string, d fs.DirEntry, err error) error { + var innerErr error + fs.WalkDir(os.DirFS(dir), ".", func(path string, d fs.DirEntry, err error) error { if err != nil { - if os.IsPermission(err) { - return fs.SkipDir - } - return err + innerErr = err + return fs.SkipDir } if !d.IsDir() { paths = append(paths, filepath.Join(dir, path)) } return nil }) - if err != nil && len(paths) == 0 { - return NewPipe().WithError(err) + if innerErr != nil && len(paths) == 0 { + return NewPipe().WithError(innerErr) } return Slice(paths) } From 8200a7fc820a9be742279adee7b43da4480fba1a Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Thu, 28 Nov 2024 12:08:01 -0500 Subject: [PATCH 8/9] Add Documentation --- script.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/script.go b/script.go index 96aacf5..c3b6302 100644 --- a/script.go +++ b/script.go @@ -77,8 +77,12 @@ func File(path string) *Pipe { } // FindFiles creates a pipe listing all the files in the directory dir and its -// subdirectories recursively, one per line, like Unix find(1). If dir doesn't -// exist or can't be read, the pipe's error status will be set. +// subdirectories recursively, one per line, like Unix find(1). +// If an error occurs while reading the directory, the behaviour is as follows: +// - If no files have been found (the directory is empty or inaccessible), the +// resulting Pipe will have its error status set to the encountered error. +// - If at least one file is found, the error is ignored, and the resulting Pipe will +// contain the successfully found file paths. // // Each line of the output consists of a slash-separated path, starting with // the initial directory. For example, if the directory looks like this: From f8d98738c6d1a3b9937c2db4d95fbe18329aea7b Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Fri, 29 Nov 2024 10:22:39 -0500 Subject: [PATCH 9/9] Make Documentation More Concise --- script.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/script.go b/script.go index c3b6302..1fd40bc 100644 --- a/script.go +++ b/script.go @@ -78,11 +78,8 @@ func File(path string) *Pipe { // FindFiles creates a pipe listing all the files in the directory dir and its // subdirectories recursively, one per line, like Unix find(1). -// If an error occurs while reading the directory, the behaviour is as follows: -// - If no files have been found (the directory is empty or inaccessible), the -// resulting Pipe will have its error status set to the encountered error. -// - If at least one file is found, the error is ignored, and the resulting Pipe will -// contain the successfully found file paths. +// Errors are ignored unless no files are found (in which case the pipe's error +// status will be set to the last error encountered). // // Each line of the output consists of a slash-separated path, starting with // the initial directory. For example, if the directory looks like this: