From 08644bf573e35f678989c46b318922bc11c00657 Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 23 Apr 2024 19:46:15 -0700 Subject: [PATCH] fix: watch for changes in replace paths --- buildengine/discover_test.go | 8 +- buildengine/filehash.go | 64 +++++++---- buildengine/filehash_test.go | 53 +++++++++ .../testdata/projects/highgoversion/go.mod | 2 +- buildengine/watch.go | 2 +- common/moduleconfig/moduleconfig.go | 103 +++++++++++++++++- common/moduleconfig/moduleconfig_test.go | 20 ++++ common/moduleconfig/testdata/imports.go | 11 ++ 8 files changed, 233 insertions(+), 30 deletions(-) create mode 100644 buildengine/filehash_test.go create mode 100644 common/moduleconfig/moduleconfig_test.go create mode 100644 common/moduleconfig/testdata/imports.go diff --git a/buildengine/discover_test.go b/buildengine/discover_test.go index ab943f864e..0cc92d8c9d 100644 --- a/buildengine/discover_test.go +++ b/buildengine/discover_test.go @@ -25,7 +25,7 @@ func TestDiscoverModules(t *testing.T) { DeployDir: "_ftl", Schema: "schema.pb", Errors: "errors.pb", - Watch: []string{"**/*.go", "go.mod", "go.sum"}, + Watch: []string{"**/*.go", "go.mod", "go.sum", "../../../../go-runtime/ftl/**/*.go"}, }, }, Module{ @@ -38,7 +38,7 @@ func TestDiscoverModules(t *testing.T) { DeployDir: "_ftl", Schema: "schema.pb", Errors: "errors.pb", - Watch: []string{"**/*.go", "go.mod", "go.sum"}, + Watch: []string{"**/*.go", "go.mod", "go.sum", "../../../../go-runtime/ftl/**/*.go"}, }, }, Module{ @@ -117,7 +117,7 @@ func TestDiscoverModules(t *testing.T) { DeployDir: "_ftl", Schema: "schema.pb", Errors: "errors.pb", - Watch: []string{"**/*.go", "go.mod", "go.sum"}, + Watch: []string{"**/*.go", "go.mod", "go.sum", "../../../../go-runtime/ftl/**/*.go"}, }, }, Module{ @@ -130,7 +130,7 @@ func TestDiscoverModules(t *testing.T) { DeployDir: "_ftl", Schema: "schema.pb", Errors: "errors.pb", - Watch: []string{"**/*.go", "go.mod", "go.sum"}, + Watch: []string{"**/*.go", "go.mod", "go.sum", "../../../../go-runtime/ftl/**/*.go"}, }, }, ExternalLibrary{ diff --git a/buildengine/filehash.go b/buildengine/filehash.go index 34a9320514..d046b3102e 100644 --- a/buildengine/filehash.go +++ b/buildengine/filehash.go @@ -67,32 +67,35 @@ func ComputeFileHashes(project Project) (FileHashes, error) { config := project.Config() fileHashes := make(FileHashes) - err := WalkDir(config.Dir, func(srcPath string, entry fs.DirEntry) error { - if entry.IsDir() { + rootDirs := computeRootDirs(config.Dir, config.Watch) + + for _, rootDir := range rootDirs { + err := WalkDir(rootDir, func(srcPath string, entry fs.DirEntry) error { + if entry.IsDir() { + return nil + } + hash, matched, err := ComputeFileHash(rootDir, srcPath, config.Watch) + if err != nil { + return err + } + if !matched { + return nil + } + fileHashes[srcPath] = hash return nil - } - hash, matched, err := ComputeFileHash(project, srcPath) + }) + if err != nil { - return err + return nil, err } - if !matched { - return nil - } - fileHashes[srcPath] = hash - return nil - }) - if err != nil { - return nil, err } - return fileHashes, err + return fileHashes, nil } -func ComputeFileHash(project Project, srcPath string) (hash []byte, matched bool, err error) { - config := project.Config() - - for _, pattern := range config.Watch { - relativePath, err := filepath.Rel(config.Dir, srcPath) +func ComputeFileHash(baseDir, srcPath string, watch []string) (hash []byte, matched bool, err error) { + for _, pattern := range watch { + relativePath, err := filepath.Rel(baseDir, srcPath) if err != nil { return nil, false, err } @@ -122,3 +125,26 @@ func ComputeFileHash(project Project, srcPath string) (hash []byte, matched bool } return nil, false, nil } + +// computeRootDirs computes the unique root directories for the given baseDir and patterns. +func computeRootDirs(baseDir string, patterns []string) []string { + uniqueRoots := make(map[string]struct{}) + uniqueRoots[baseDir] = struct{}{} + + for _, pattern := range patterns { + fullPath := filepath.Join(baseDir, pattern) + dirPath, _ := doublestar.SplitPattern(fullPath) + cleanedPath := filepath.Clean(dirPath) + + if _, err := os.Stat(cleanedPath); err == nil { + uniqueRoots[cleanedPath] = struct{}{} + } + } + + roots := make([]string, 0, len(uniqueRoots)) + for root := range uniqueRoots { + roots = append(roots, root) + } + + return roots +} diff --git a/buildengine/filehash_test.go b/buildengine/filehash_test.go new file mode 100644 index 0000000000..1d0a8e956a --- /dev/null +++ b/buildengine/filehash_test.go @@ -0,0 +1,53 @@ +package buildengine + +import ( + "os" + "path/filepath" + "testing" + + "github.com/alecthomas/assert/v2" +) + +func TestComputeRootDirs(t *testing.T) { + dir := t.TempDir() + libDir := filepath.Join(dir, "libs", "test") + + err := os.MkdirAll(libDir, 0700) + assert.NoError(t, err) + + tests := []struct { + name string + baseDir string + patterns []string + roots []string + }{ + { + name: "Simple Patterns", + baseDir: filepath.Join(dir, "modules", "test"), + patterns: []string{ + "**/*.go", + "go.mod", + "go.sum", + }, + roots: []string{filepath.Join(dir, "modules", "test")}, + }, + { + name: "Upward Traversal", + baseDir: filepath.Join(dir, "modules", "test"), + patterns: []string{ + "../../libs/test/**/*.go", + }, + roots: []string{ + filepath.Join(dir, "modules", "test"), + filepath.Join(dir, "libs", "test"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + roots := computeRootDirs(tt.baseDir, tt.patterns) + assert.Compare(t, roots, tt.roots) + }) + } +} diff --git a/buildengine/testdata/projects/highgoversion/go.mod b/buildengine/testdata/projects/highgoversion/go.mod index a6183e21eb..f8f5d9d31b 100644 --- a/buildengine/testdata/projects/highgoversion/go.mod +++ b/buildengine/testdata/projects/highgoversion/go.mod @@ -43,4 +43,4 @@ require ( google.golang.org/protobuf v1.33.0 // indirect ) -replace github.com/TBD54566975/ftl => /Users/dli/Development/ftl +replace github.com/TBD54566975/ftl => ../../../.. diff --git a/buildengine/watch.go b/buildengine/watch.go index 4ad26c1a9c..f44bea62aa 100644 --- a/buildengine/watch.go +++ b/buildengine/watch.go @@ -210,7 +210,7 @@ func (t *modifyFilesTransaction) ModifiedFiles(paths ...string) error { } for _, path := range paths { - hash, matched, err := ComputeFileHash(projectHashes.Project, path) + hash, matched, err := ComputeFileHash(projectHashes.Project.Config().Dir, path, projectHashes.Project.Config().Watch) if err != nil { return err } diff --git a/common/moduleconfig/moduleconfig.go b/common/moduleconfig/moduleconfig.go index 512eb8751f..b7a6aa1e2a 100644 --- a/common/moduleconfig/moduleconfig.go +++ b/common/moduleconfig/moduleconfig.go @@ -2,10 +2,14 @@ package moduleconfig import ( "fmt" + "go/parser" + "go/token" + "os" "path/filepath" "strings" "github.com/BurntSushi/toml" + "golang.org/x/mod/modfile" ) // ModuleGoConfig is language-specific configuration for Go modules. @@ -95,6 +99,11 @@ func setConfigDefaults(moduleDir string, config *ModuleConfig) error { } if len(config.Watch) == 0 { config.Watch = []string{"**/*.go", "go.mod", "go.sum"} + watches, err := replacementWatches(moduleDir, config.DeployDir) + if err != nil { + return err + } + config.Watch = append(config.Watch, watches...) } } @@ -107,11 +116,7 @@ func setConfigDefaults(moduleDir string, config *ModuleConfig) error { return fmt.Errorf("deploy files must be relative to the module directory") } } - for _, watch := range config.Watch { - if !isBeneath(moduleDir, watch) { - return fmt.Errorf("watch files must be relative to the module directory") - } - } + return nil } @@ -119,3 +124,91 @@ func isBeneath(moduleDir, path string) bool { resolved := filepath.Clean(filepath.Join(moduleDir, path)) return strings.HasPrefix(resolved, strings.TrimSuffix(moduleDir, "/")+"/") } + +func replacementWatches(moduleDir, deployDir string) ([]string, error) { + goModPath := filepath.Join(moduleDir, "go.mod") + goModBytes, err := os.ReadFile(goModPath) + if err != nil { + return nil, fmt.Errorf("failed to read %s: %w", goModPath, err) + } + goModFile, err := modfile.Parse(goModPath, goModBytes, nil) + if err != nil { + return nil, fmt.Errorf("failed to parse %s: %w", goModPath, err) + } + + replacements := make(map[string]string) + for _, r := range goModFile.Replace { + replacements[r.Old.Path] = r.New.Path + if strings.HasPrefix(r.New.Path, ".") { + relPath, err := filepath.Rel(filepath.Dir(goModPath), filepath.Join(filepath.Dir(goModPath), r.New.Path)) + if err != nil { + return nil, err + } + replacements[r.Old.Path] = relPath + } + } + + files, err := findReplacedImports(moduleDir, deployDir, replacements) + if err != nil { + return nil, err + } + + uniquePatterns := make(map[string]struct{}) + for _, file := range files { + pattern := filepath.Join(file, "**/*.go") + uniquePatterns[pattern] = struct{}{} + } + + patterns := make([]string, 0, len(uniquePatterns)) + for pattern := range uniquePatterns { + patterns = append(patterns, pattern) + } + + return patterns, nil +} + +// findReplacedImports finds Go files with imports that are specified in the replacements. +func findReplacedImports(moduleDir, deployDir string, replacements map[string]string) ([]string, error) { + var libPaths []string + + err := filepath.WalkDir(moduleDir, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() && !strings.Contains(path, deployDir) && strings.HasSuffix(path, ".go") { + imports, err := parseImports(path) + if err != nil { + return err + } + + for _, imp := range imports { + for oldPath, newPath := range replacements { + if strings.HasPrefix(imp, oldPath) { + resolvedPath := filepath.Join(newPath, strings.TrimPrefix(imp, oldPath)) + libPaths = append(libPaths, resolvedPath) + break // Only add the library path once for each import match + } + } + } + } + return nil + }) + + return libPaths, err +} + +func parseImports(filePath string) ([]string, error) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, filePath, nil, parser.ImportsOnly) + if err != nil { + return nil, err + } + + var imports []string + for _, imp := range file.Imports { + // Trim the quotes from the import path value + trimmedPath := strings.Trim(imp.Path.Value, `"`) + imports = append(imports, trimmedPath) + } + return imports, nil +} diff --git a/common/moduleconfig/moduleconfig_test.go b/common/moduleconfig/moduleconfig_test.go new file mode 100644 index 0000000000..2daa1a0260 --- /dev/null +++ b/common/moduleconfig/moduleconfig_test.go @@ -0,0 +1,20 @@ +package moduleconfig + +import ( + "path/filepath" + "reflect" + "testing" +) + +func TestParseImportsFromTestData(t *testing.T) { + testFilePath := filepath.Join("testdata", "imports.go") + expectedImports := []string{"fmt", "os"} + imports, err := parseImports(testFilePath) + if err != nil { + t.Fatalf("Failed to parse imports: %v", err) + } + + if !reflect.DeepEqual(imports, expectedImports) { + t.Errorf("parseImports() got = %v, want %v", imports, expectedImports) + } +} diff --git a/common/moduleconfig/testdata/imports.go b/common/moduleconfig/testdata/imports.go new file mode 100644 index 0000000000..d9197ac716 --- /dev/null +++ b/common/moduleconfig/testdata/imports.go @@ -0,0 +1,11 @@ +package main + +import ( + "fmt" + "os" +) + +func main() { + fmt.Println("Hello, world!") + os.Exit(0) +}