Skip to content

Commit

Permalink
fix: infinite loop caused by build updating files (#1274)
Browse files Browse the repository at this point in the history
fixes: #1202

Changes:
- file watch service now allows transactions that pause file update
logic on modules, and allows specific file changes to be explicitly
allowed (ie does not count as an update that triggers another build)
- building a go module uses a transaction to handle `go mod tidy`
commands
  • Loading branch information
matt2e authored Apr 22, 2024
1 parent 50b2138 commit 4ded60b
Show file tree
Hide file tree
Showing 12 changed files with 387 additions and 95 deletions.
8 changes: 4 additions & 4 deletions buildengine/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ import (

// Build a project in the given directory given the schema and project config.
// For a module, this will build the module. For an external library, this will build stubs for imported modules.
func Build(ctx context.Context, sch *schema.Schema, project Project) error {
func Build(ctx context.Context, sch *schema.Schema, project Project, filesTransaction ModifyFilesTransaction) error {
switch project := project.(type) {
case Module:
return buildModule(ctx, sch, project)
return buildModule(ctx, sch, project, filesTransaction)
case ExternalLibrary:
return buildExternalLibrary(ctx, sch, project)
default:
panic(fmt.Sprintf("unsupported project type: %T", project))
}
}

func buildModule(ctx context.Context, sch *schema.Schema, module Module) error {
func buildModule(ctx context.Context, sch *schema.Schema, module Module, filesTransaction ModifyFilesTransaction) error {
logger := log.FromContext(ctx).Scope(module.Module)
ctx = log.ContextWithLogger(ctx, logger)

Expand All @@ -43,7 +43,7 @@ func buildModule(ctx context.Context, sch *schema.Schema, module Module) error {
var err error
switch module.Language {
case "go":
err = buildGoModule(ctx, sch, module)
err = buildGoModule(ctx, sch, module, filesTransaction)
case "kotlin":
err = buildKotlinModule(ctx, sch, module)
default:
Expand Down
4 changes: 2 additions & 2 deletions buildengine/build_go.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"github.com/TBD54566975/ftl/go-runtime/compile"
)

func buildGoModule(ctx context.Context, sch *schema.Schema, module Module) error {
if err := compile.Build(ctx, module.Dir, sch); err != nil {
func buildGoModule(ctx context.Context, sch *schema.Schema, module Module, transaction ModifyFilesTransaction) error {
if err := compile.Build(ctx, module.Dir, sch, transaction); err != nil {
return fmt.Errorf("failed to build module %q: %w", module.Config().Key, err)
}
return nil
Expand Down
16 changes: 15 additions & 1 deletion buildengine/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ type buildContext struct {

type assertion func(t testing.TB, bctx buildContext) error

type mockModifyFilesTransaction struct{}

func (t *mockModifyFilesTransaction) Begin() error {
return nil
}

func (t *mockModifyFilesTransaction) ModifiedFiles(paths ...string) error {
return nil
}

func (t *mockModifyFilesTransaction) End() error {
return nil
}

func testBuild(
t *testing.T,
bctx buildContext,
Expand All @@ -33,7 +47,7 @@ func testBuild(
assert.NoError(t, err, "Error getting absolute path for module directory")
module, err := LoadModule(abs)
assert.NoError(t, err)
err = Build(ctx, bctx.sch, module)
err = Build(ctx, bctx.sch, module, &mockModifyFilesTransaction{})
if len(expectedBuildErrMsg) > 0 {
assert.Error(t, err)
assert.Contains(t, err.Error(), expectedBuildErrMsg)
Expand Down
2 changes: 1 addition & 1 deletion buildengine/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestDeploy(t *testing.T) {
assert.NoError(t, err)

// Build first to make sure the files are there.
err = Build(ctx, sch, module)
err = Build(ctx, sch, module, &mockModifyFilesTransaction{})
assert.NoError(t, err)

sum, err := sha256.SumFile(modulePath + "/_ftl/main")
Expand Down
17 changes: 11 additions & 6 deletions buildengine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type Engine struct {
projectMetas *xsync.MapOf[ProjectKey, projectMeta]
moduleDirs []string
externalDirs []string
watcher *Watcher
controllerSchema *xsync.MapOf[string, *schema.Module]
schemaChanges *pubsub.Topic[schemaChange]
cancel func()
Expand Down Expand Up @@ -88,6 +89,7 @@ func New(ctx context.Context, client ftlv1connect.ControllerServiceClient, modul
moduleDirs: moduleDirs,
externalDirs: externalDirs,
projectMetas: xsync.NewMapOf[ProjectKey, projectMeta](),
watcher: NewWatcher(),
controllerSchema: xsync.NewMapOf[string, *schema.Module](),
schemaChanges: pubsub.New[schemaChange](),
parallelism: runtime.NumCPU(),
Expand Down Expand Up @@ -242,13 +244,16 @@ func (e *Engine) watchForModuleChanges(ctx context.Context, period time.Duration
defer e.schemaChanges.Unsubscribe(schemaChanges)

watchEvents := make(chan WatchEvent, 128)
watch := Watch(ctx, period, e.moduleDirs, e.externalDirs)
watch.Subscribe(watchEvents)
defer watch.Unsubscribe(watchEvents)
defer watch.Close()
topic, err := e.watcher.Watch(ctx, period, e.moduleDirs, e.externalDirs)
if err != nil {
return err
}
topic.Subscribe(watchEvents)
defer topic.Unsubscribe(watchEvents)
defer topic.Close()

// Build and deploy all modules first.
err := e.buildAndDeploy(ctx, 1, true)
err = e.buildAndDeploy(ctx, 1, true)
if err != nil {
logger.Errorf(err, "initial deploy failed")
} else {
Expand Down Expand Up @@ -554,7 +559,7 @@ func (e *Engine) build(ctx context.Context, key ProjectKey, builtModules map[str
if e.listener != nil {
e.listener.OnBuildStarted(meta.project)
}
err := Build(ctx, sch, meta.project)
err := Build(ctx, sch, meta.project, e.watcher.GetTransaction(meta.project.Config().Dir))
if err != nil {
return err
}
Expand Down
77 changes: 46 additions & 31 deletions buildengine/filehash.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,47 +63,62 @@ func CompareFileHashes(oldFiles, newFiles FileHashes) (FileChangeType, string, b

// ComputeFileHashes computes the SHA256 hash of all (non-git-ignored) files in
// the given directory.
func ComputeFileHashes(module Project) (FileHashes, error) {
config := module.Config()
func ComputeFileHashes(project Project) (FileHashes, error) {
config := project.Config()

fileHashes := make(FileHashes)
err := WalkDir(config.Dir, func(srcPath string, entry fs.DirEntry) error {
for _, pattern := range config.Watch {
relativePath, err := filepath.Rel(config.Dir, srcPath)
if err != nil {
return err
}
if entry.IsDir() {
return nil
}
hash, matched, err := ComputeFileHash(project, srcPath)
if err != nil {
return err
}
if !matched {
return nil
}
fileHashes[srcPath] = hash
return nil
})
if err != nil {
return nil, err
}

return fileHashes, err
}

match, err := doublestar.PathMatch(pattern, relativePath)
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)
if err != nil {
return nil, false, err
}
match, err := doublestar.PathMatch(pattern, relativePath)
if err != nil {
return nil, false, err
}
if match {
file, err := os.Open(srcPath)
if err != nil {
return err
return nil, false, err
}

if match && !entry.IsDir() {
file, err := os.Open(srcPath)
if err != nil {
return err
}

hasher := sha256.New()
if _, err := io.Copy(hasher, file); err != nil {
_ = file.Close()
return err
}
hasher := sha256.New()
if _, err := io.Copy(hasher, file); err != nil {
_ = file.Close()
return nil, false, err
}

fileHashes[srcPath] = hasher.Sum(nil)
hash := hasher.Sum(nil)

if err := file.Close(); err != nil {
return err
}
if err := file.Close(); err != nil {
return nil, false, err
}
return hash, true, nil
}

return nil
})
if err != nil {
return nil, err
}

return fileHashes, err
return nil, false, nil
}
12 changes: 6 additions & 6 deletions buildengine/testdata/projects/another/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 4ded60b

Please sign in to comment.