fix: prevent discovery of modules while scaffolding modules (#4046)

fixes #4039 `watch` can now be configured to only discover modules while it has acquisition of a file lock. This allows coordination with `ftl new` so that half scaffolded modules are not detected too quickly, causing build errors.
block · Jan 15, 2025 · 35df745 · 35df745
1 parent c014877
commit 35df745
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 7 deletions.
diff --git a/frontend/cli/cmd_new.go b/frontend/cli/cmd_new.go
@@ -7,12 +7,14 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"time"
 
 	"github.com/alecthomas/kong"
 
 	"github.com/block/ftl/common/schema"
 	"github.com/block/ftl/internal"
 	"github.com/block/ftl/internal/buildengine/languageplugin"
+	"github.com/block/ftl/internal/flock"
 	"github.com/block/ftl/internal/log"
 	"github.com/block/ftl/internal/moduleconfig"
 	"github.com/block/ftl/internal/projectconfig"
@@ -58,6 +60,12 @@ func (i newCmd) Run(ctx context.Context, ktctx *kong.Context, config projectconf
 		return err
 	}
 
+	release, err := flock.Acquire(ctx, config.WatchModulesLockPath(), 30*time.Second)
+	if err != nil {
+		return fmt.Errorf("could not acquire file lock: %w", err)
+	}
+	defer release() //nolint:errcheck
+
 	err = plugin.CreateModule(ctx, config, moduleConfig, flags)
 	if err != nil {
 		return err

diff --git a/go-runtime/goplugin/service.go b/go-runtime/goplugin/service.go
@@ -238,7 +238,7 @@ func (s *Service) Build(ctx context.Context, req *connect.Request[langpb.BuildRe
 		return err
 	}
 
-	watcher := watch.NewWatcher(watchPatterns...)
+	watcher := watch.NewWatcher(optional.None[string](), watchPatterns...)
 
 	ongoingState := &compile.OngoingState{}
 

diff --git a/internal/buildengine/engine.go b/internal/buildengine/engine.go
@@ -145,7 +145,7 @@ func New(
 		projectConfig:    projectConfig,
 		moduleDirs:       moduleDirs,
 		moduleMetas:      xsync.NewMapOf[string, moduleMeta](),
-		watcher:          watch.NewWatcher("ftl.toml"),
+		watcher:          watch.NewWatcher(optional.Some(projectConfig.WatchModulesLockPath()), "ftl.toml"),
 		controllerSchema: xsync.NewMapOf[string, *schema.Module](),
 		schemaChanges:    pubsub.New[schemaeventsource.Event](),
 		pluginEvents:     make(chan languageplugin.PluginEvent, 128),

diff --git a/internal/projectconfig/projectconfig.go b/internal/projectconfig/projectconfig.go
@@ -218,3 +218,8 @@ func (c Config) SchemaPath(module string) string {
 func (c Config) SQLCGenFTLPath() string {
 	return filepath.Join(c.Root(), ".ftl", "resources", "sqlc-gen-ftl.wasm")
 }
+
+// WatchModulesLockPath returns the path to the lock file used to prevent scaffolding new modules while discovering modules.
+func (c Config) WatchModulesLockPath() string {
+	return filepath.Join(c.Root(), ".ftl", "modules.lock")
+}
diff --git a/internal/watch/watch.go b/internal/watch/watch.go
@@ -8,9 +8,11 @@ import (
 	"sync"
 	"time"
 
+	"github.com/alecthomas/types/optional"
 	"github.com/alecthomas/types/pubsub"
 
 	"github.com/block/ftl/common/slices"
+	"github.com/block/ftl/internal/flock"
 	"github.com/block/ftl/internal/log"
 	"github.com/block/ftl/internal/maps"
 	"github.com/block/ftl/internal/moduleconfig"
@@ -63,6 +65,8 @@ type moduleHashes struct {
 type Watcher struct {
 	isWatching bool
 
+	// lock path ensures no modules are scaffolded while a Watcher walks over the files
+	lockPath optional.Option[string]
 	// patterns are relative to each module found
 	patterns []string
 
@@ -72,10 +76,11 @@ type Watcher struct {
 	moduleTransactions map[string][]*modifyFilesTransaction
 }
 
-func NewWatcher(patterns ...string) *Watcher {
+func NewWatcher(lockPath optional.Option[string], patterns ...string) *Watcher {
 	svc := &Watcher{
 		existingModules:    map[string]moduleHashes{},
 		moduleTransactions: map[string][]*modifyFilesTransaction{},
+		lockPath:           lockPath,
 		patterns:           patterns,
 	}
 
@@ -125,11 +130,25 @@ func (w *Watcher) Watch(ctx context.Context, period time.Duration, moduleDirs []
 				return
 			}
 
+			var flockRelease func() error
+			if path, ok := w.lockPath.Get(); ok {
+				var err error
+				flockRelease, err = flock.Acquire(ctx, path, period)
+				if err != nil {
+					logger.Debugf("error acquiring modules lock to discover modules: %v", err)
+					continue
+				}
+			} else {
+				flockRelease = func() error { return nil }
+			}
 			modules, err := DiscoverModules(ctx, moduleDirs)
 			if err != nil {
 				logger.Tracef("error discovering modules: %v", err)
 				continue
 			}
+			if err := flockRelease(); err != nil {
+				logger.Debugf("error releasing modules lock after discovering modules: %v", err)
+			}
 
 			modulesByDir := maps.FromSlice(modules, func(config moduleconfig.UnvalidatedModuleConfig) (string, moduleconfig.UnvalidatedModuleConfig) {
 				return config.Dir, config

diff --git a/internal/watch/watch_integration_test.go b/internal/watch/watch_integration_test.go
@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/alecthomas/assert/v2"
+	"github.com/alecthomas/types/optional"
 	"github.com/alecthomas/types/pubsub"
 
 	in "github.com/block/ftl/internal/integration"
@@ -23,7 +24,7 @@ func TestWatch(t *testing.T) {
 	var topic *pubsub.Topic[WatchEvent]
 	var one, two moduleconfig.UnvalidatedModuleConfig
 
-	w := NewWatcher("**/*.go", "go.mod", "go.sum")
+	w := NewWatcher(optional.None[string](), "**/*.go", "go.mod", "go.sum")
 	in.Run(t,
 		func(tb testing.TB, ic in.TestContext) {
 			events, topic = startWatching(ic, t, w, ic.WorkingDir())
@@ -68,7 +69,7 @@ func TestWatchWithBuildModifyingFiles(t *testing.T) {
 	var events chan WatchEvent
 	var topic *pubsub.Topic[WatchEvent]
 	var transaction ModifyFilesTransaction
-	w := NewWatcher("**/*.go", "go.mod", "go.sum")
+	w := NewWatcher(optional.None[string](), "**/*.go", "go.mod", "go.sum")
 
 	in.Run(t,
 		func(tb testing.TB, ic in.TestContext) {
@@ -104,7 +105,7 @@ func TestWatchWithBuildAndUserModifyingFiles(t *testing.T) {
 	var events chan WatchEvent
 	var topic *pubsub.Topic[WatchEvent]
 	var transaction ModifyFilesTransaction
-	w := NewWatcher("**/*.go", "go.mod", "go.sum")
+	w := NewWatcher(optional.None[string](), "**/*.go", "go.mod", "go.sum")
 
 	in.Run(t,
 		func(tb testing.TB, ic in.TestContext) {

diff --git a/jvm-runtime/plugin/common/jvmcommon.go b/jvm-runtime/plugin/common/jvmcommon.go
@@ -18,6 +18,7 @@ import (
 
 	"connectrpc.com/connect"
 	"github.com/alecthomas/atomic"
+	"github.com/alecthomas/types/optional"
 	"github.com/alecthomas/types/pubsub"
 	"github.com/beevik/etree"
 	"github.com/block/scaffolder"
@@ -222,7 +223,7 @@ func (s *Service) runDevMode(ctx context.Context, req *connect.Request[langpb.Bu
 	if err != nil {
 		return err
 	}
-	watcher := watch.NewWatcher(watchPatterns...)
+	watcher := watch.NewWatcher(optional.None[string](), watchPatterns...)
 	fileEvents := make(chan watch.WatchEventModuleChanged, 32)
 	if err := watchFiles(ctx, watcher, buildCtx, fileEvents); err != nil {
 		return err