From 56ae0c7b9a6f122361446343260bccab44c74e0e Mon Sep 17 00:00:00 2001 From: Marc Johnson Date: Sat, 18 May 2024 15:30:06 -0400 Subject: [PATCH] [#9] fix: Clean will not remove file unless they're in the working directory --- build.go | 25 ++++++++++++++++--------- build_test.go | 14 ++++++-------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/build.go b/build.go index ac7ab7f..720f194 100644 --- a/build.go +++ b/build.go @@ -27,12 +27,17 @@ var ( // calls os.Exit(); this is to prevent callers from deceptively removing files // they shouldn't. func Clean(files []string) { + workingFS := os.DirFS(WorkingDir()) for _, file := range files { if containsBackDir(file) { fmt.Fprintf(os.Stderr, "file %q will not be removed, exiting the build\n", file) exit(1) } - fileSystem.Remove(filepath.Join(WorkingDir(), file)) + openFile, err := workingFS.Open(file) + if err == nil { + openFile.Close() + fileSystem.Remove(filepath.Join(WorkingDir(), file)) + } } } @@ -40,9 +45,7 @@ func containsBackDir(path string) bool { if !strings.Contains(path, "..") { return false } - if strings.Contains(path, "\\") { - path = strings.ReplaceAll(path, "\\", "/") - } + path = canonicalizePath(path) if path == ".." { return true } @@ -53,6 +56,13 @@ func containsBackDir(path string) bool { return containsBackDir(strings.TrimSuffix(dir, "/")) } +func canonicalizePath(path string) string { + if strings.Contains(path, "\\") { + return strings.ReplaceAll(path, "\\", "/") + } + return path +} + // Format runs the gofmt tool to repair the formatting of each source file; // returns false if the command fails func Format(a *goyek.A) bool { @@ -251,15 +261,12 @@ func allDirs(top string) ([]string, error) { if !topIsDir { return nil, fmt.Errorf("%q is not a directory", top) } - if strings.Contains(top, "\\") { - top = strings.ReplaceAll(top, "\\", "/") - } + top = canonicalizePath(top) entries, _ := afero.ReadDir(fileSystem, top) dirs := []string{top} for _, entry := range entries { if entry.IsDir() { - subDirectory := filepath.Join(top, entry.Name()) - subDirs, _ := allDirs(subDirectory) + subDirs, _ := allDirs(filepath.Join(top, entry.Name())) dirs = append(dirs, subDirs...) } } diff --git a/build_test.go b/build_test.go index eeebc63..be72325 100644 --- a/build_test.go +++ b/build_test.go @@ -16,17 +16,17 @@ import ( ) func TestClean(t *testing.T) { - originalFileSystem := fileSystem + // note: cannot use memory mapped filesystem; Clean relies on using the os + // filesystem to make sure all files are within the working directory originalWorkingDir := workingDir originalExit := exit + workingDir = "a/b/c" defer func() { - fileSystem = originalFileSystem workingDir = originalWorkingDir exit = originalExit + fileSystem.RemoveAll("a") }() - fileSystem = afero.NewMemMapFs() fileSystem.MkdirAll("a/b/c", 0o755) - workingDir = "a/b/c" afero.WriteFile(fileSystem, "a/b/c/myFile", []byte("foo"), 0o644) afero.WriteFile(fileSystem, "a/b/c/myOtherFile", []byte(""), 0o644) tests := map[string]struct { @@ -34,7 +34,7 @@ func TestClean(t *testing.T) { wantExitCalled bool }{ "no files": {files: nil}, - "no problems": {files: []string{"myFile", "myOtherFile", "myNonExistentFile"}}, + "no problems": {files: []string{"myFile", "myOtherFile", "c:\\myNonExistentFile"}}, "illegal path": {files: []string{"foo/../../bar"}, wantExitCalled: true}, } for name, tt := range tests { @@ -46,9 +46,7 @@ func TestClean(t *testing.T) { Clean(tt.files) for _, file := range tt.files { f := filepath.Join(workingDir, file) - if fileExists, err := afero.Exists(fileSystem, f); err != nil { - t.Errorf("Clean: something went wrong verifying the non-existence of %q: %v", f, err) - } else if fileExists { + if fileExists, _ := afero.Exists(fileSystem, f); fileExists { t.Errorf("Clean failed to delete %q", f) } }