From 459b44144b38b83f479ed509570d1fffd25f7bb4 Mon Sep 17 00:00:00 2001 From: Alec Thomas Date: Fri, 23 Feb 2024 10:11:45 +1100 Subject: [PATCH] refactor: move logic from cmd/ftl/cmd_build* into buildengine (#974) This is just a pure move, with no logic changes. --------- Co-authored-by: github-actions[bot] --- .gitignore | 1 + buildengine/build.go | 30 ++++++++++++++++--- .../build_go.go | 11 +++---- .../build_kotlin.go | 18 ++++++----- buildengine/deps.go | 8 +++-- buildengine/discover.go | 8 +++-- buildengine/discover_test.go | 4 ++- buildengine/filehash.go | 4 ++- buildengine/topological_test.go | 22 +++++++------- buildengine/watch.go | 3 +- buildengine/watch_test.go | 3 +- cmd/ftl/cmd_build.go | 28 ++++------------- cmd/ftl/cmd_deploy.go | 8 ++--- cmd/ftl/cmd_dev.go | 5 ++-- cmd/ftl/cmd_init.go | 3 +- .../moduleconfig}/moduleconfig.go | 2 +- go-runtime/compile/build.go | 4 +-- go-runtime/encoding/encoding_test.go | 4 +-- 18 files changed, 95 insertions(+), 71 deletions(-) rename cmd/ftl/cmd_build_go.go => buildengine/build_go.go (62%) rename cmd/ftl/cmd_build_kotlin.go => buildengine/build_kotlin.go (69%) rename {buildengine => common/moduleconfig}/moduleconfig.go (99%) diff --git a/.gitignore b/.gitignore index 812e5ba22f..0dec62bdf6 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,4 @@ dist/ examples/**/go.work examples/**/go.work.sum **/_ftl +buildengine/.gitignore diff --git a/buildengine/build.go b/buildengine/build.go index 8b61e96eeb..a038f65bed 100644 --- a/buildengine/build.go +++ b/buildengine/build.go @@ -2,17 +2,39 @@ package buildengine import ( "context" + "fmt" - "github.com/TBD54566975/ftl/backend/schema" + "github.com/TBD54566975/ftl/common/moduleconfig" ) // A Module is a ModuleConfig with its dependencies populated. type Module struct { - ModuleConfig + moduleconfig.ModuleConfig Dependencies []string } +// LoadModule loads a module from the given directory. +// +// A [Module] includes the module configuration as well as its dependencies +// extracted from source code. +func LoadModule(dir string) (Module, error) { + config, err := moduleconfig.LoadModuleConfig(dir) + if err != nil { + return Module{}, err + } + return UpdateDependencies(config) +} + // Build a module in the given directory given the schema and module config. -func Build(ctx context.Context, schema *schema.Schema, config Module) error { - panic("not implemented") +func Build(ctx context.Context /*schema *schema.Schema, */, module Module) error { + switch module.Language { + case "go": + return buildGo(ctx, module) + + case "kotlin": + return buildKotlin(ctx, module) + + default: + return fmt.Errorf("unknown language %q", module.Language) + } } diff --git a/cmd/ftl/cmd_build_go.go b/buildengine/build_go.go similarity index 62% rename from cmd/ftl/cmd_build_go.go rename to buildengine/build_go.go index a8884d6b4c..7a15f18f08 100644 --- a/cmd/ftl/cmd_build_go.go +++ b/buildengine/build_go.go @@ -1,4 +1,4 @@ -package main +package buildengine import ( "context" @@ -10,20 +10,21 @@ import ( "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect" "github.com/TBD54566975/ftl/backend/schema" "github.com/TBD54566975/ftl/go-runtime/compile" + "github.com/TBD54566975/ftl/internal/rpc" ) -func (b *buildCmd) buildGo(ctx context.Context, client ftlv1connect.ControllerServiceClient) error { +func buildGo(ctx context.Context, module Module) error { + client := rpc.ClientFromContext[ftlv1connect.ControllerServiceClient](ctx) resp, err := client.GetSchema(ctx, connect.NewRequest(&ftlv1.GetSchemaRequest{})) if err != nil { return err } sch, err := schema.FromProto(resp.Msg.Schema) if err != nil { - return err + return fmt.Errorf("failed to convert schema from proto: %w", err) } - if err := compile.Build(ctx, b.ModuleDir, sch); err != nil { + if err := compile.Build(ctx, module.Dir, sch); err != nil { return fmt.Errorf("failed to build module: %w", err) } - return nil } diff --git a/cmd/ftl/cmd_build_kotlin.go b/buildengine/build_kotlin.go similarity index 69% rename from cmd/ftl/cmd_build_kotlin.go rename to buildengine/build_kotlin.go index 2358e0e1fd..81cb3f1aa6 100644 --- a/cmd/ftl/cmd_build_kotlin.go +++ b/buildengine/build_kotlin.go @@ -1,4 +1,4 @@ -package main +package buildengine import ( "context" @@ -9,20 +9,19 @@ import ( "github.com/beevik/etree" "github.com/TBD54566975/ftl" - "github.com/TBD54566975/ftl/buildengine" "github.com/TBD54566975/ftl/internal/exec" "github.com/TBD54566975/ftl/internal/log" ) -func (b *buildCmd) buildKotlin(ctx context.Context, config buildengine.ModuleConfig) error { +func buildKotlin(ctx context.Context, module Module) error { logger := log.FromContext(ctx) - if err := setPomProperties(logger, filepath.Join(b.ModuleDir, "..")); err != nil { - return fmt.Errorf("unable to update ftl.version in %s: %w", b.ModuleDir, err) + if err := SetPOMProperties(ctx, filepath.Join(module.Dir, "..")); err != nil { + return fmt.Errorf("unable to update ftl.version in %s: %w", module.Dir, err) } - logger.Debugf("Using build command '%s'", config.Build) - err := exec.Command(ctx, log.Debug, b.ModuleDir, "bash", "-c", config.Build).RunBuffered(ctx) + logger.Debugf("Using build command '%s'", module.Build) + err := exec.Command(ctx, log.Debug, module.Dir, "bash", "-c", module.Build).RunBuffered(ctx) if err != nil { return fmt.Errorf("failed to build module: %w", err) } @@ -30,7 +29,10 @@ func (b *buildCmd) buildKotlin(ctx context.Context, config buildengine.ModuleCon return nil } -func setPomProperties(logger *log.Logger, baseDir string) error { +// SetPOMProperties updates the ftl.version and ftlEndpoint properties in the +// pom.xml file in the given base directory. +func SetPOMProperties(ctx context.Context, baseDir string) error { + logger := log.FromContext(ctx) ftlVersion := ftl.Version if ftlVersion == "dev" { ftlVersion = "1.0-SNAPSHOT" diff --git a/buildengine/deps.go b/buildengine/deps.go index f03de132e0..9e5dcd0137 100644 --- a/buildengine/deps.go +++ b/buildengine/deps.go @@ -15,10 +15,12 @@ import ( "golang.design/x/reflect" "golang.org/x/exp/maps" + + "github.com/TBD54566975/ftl/common/moduleconfig" ) // UpdateAllDependencies calls UpdateDependencies on each module in the list. -func UpdateAllDependencies(modules []ModuleConfig) ([]Module, error) { +func UpdateAllDependencies(modules []moduleconfig.ModuleConfig) ([]Module, error) { out := []Module{} for _, module := range modules { updated, err := UpdateDependencies(module) @@ -32,7 +34,7 @@ func UpdateAllDependencies(modules []ModuleConfig) ([]Module, error) { // UpdateDependencies finds the dependencies for an FTL module and returns a // Module with those dependencies populated. -func UpdateDependencies(config ModuleConfig) (Module, error) { +func UpdateDependencies(config moduleconfig.ModuleConfig) (Module, error) { dependencies, err := extractDependencies(config) if err != nil { return Module{}, err @@ -41,7 +43,7 @@ func UpdateDependencies(config ModuleConfig) (Module, error) { return Module{ModuleConfig: out, Dependencies: dependencies}, nil } -func extractDependencies(config ModuleConfig) ([]string, error) { +func extractDependencies(config moduleconfig.ModuleConfig) ([]string, error) { switch config.Language { case "go": return extractGoFTLImports(config.Module, config.Dir) diff --git a/buildengine/discover.go b/buildengine/discover.go index e7c9b07d06..6e42fdd4a1 100644 --- a/buildengine/discover.go +++ b/buildengine/discover.go @@ -5,12 +5,14 @@ import ( "os" "path/filepath" "sort" + + "github.com/TBD54566975/ftl/common/moduleconfig" ) // DiscoverModules recursively loads all modules under the given directories. // // If no directories are provided, the current working directory is used. -func DiscoverModules(dirs ...string) ([]ModuleConfig, error) { +func DiscoverModules(dirs ...string) ([]moduleconfig.ModuleConfig, error) { if len(dirs) == 0 { cwd, err := os.Getwd() if err != nil { @@ -18,14 +20,14 @@ func DiscoverModules(dirs ...string) ([]ModuleConfig, error) { } dirs = []string{cwd} } - out := []ModuleConfig{} + out := []moduleconfig.ModuleConfig{} for _, dir := range dirs { err := WalkDir(dir, func(path string, d fs.DirEntry) error { if filepath.Base(path) != "ftl.toml" { return nil } moduleDir := filepath.Dir(path) - config, err := LoadModuleConfig(moduleDir) + config, err := moduleconfig.LoadModuleConfig(moduleDir) if err != nil { return err } diff --git a/buildengine/discover_test.go b/buildengine/discover_test.go index 7e7adebaf2..776c6dd95f 100644 --- a/buildengine/discover_test.go +++ b/buildengine/discover_test.go @@ -4,12 +4,14 @@ import ( "testing" "github.com/alecthomas/assert/v2" + + "github.com/TBD54566975/ftl/common/moduleconfig" ) func TestDiscoverModules(t *testing.T) { modules, err := DiscoverModules("testdata/modules") assert.NoError(t, err) - expected := []ModuleConfig{ + expected := []moduleconfig.ModuleConfig{ { Dir: "testdata/modules/alpha", Language: "go", diff --git a/buildengine/filehash.go b/buildengine/filehash.go index 71525f013a..8c3374ff98 100644 --- a/buildengine/filehash.go +++ b/buildengine/filehash.go @@ -9,6 +9,8 @@ import ( "path/filepath" "github.com/bmatcuk/doublestar/v4" + + "github.com/TBD54566975/ftl/common/moduleconfig" ) type FileChangeType rune @@ -63,7 +65,7 @@ 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(config ModuleConfig) (FileHashes, error) { +func ComputeFileHashes(config moduleconfig.ModuleConfig) (FileHashes, error) { fileHashes := make(FileHashes) err := WalkDir(config.Dir, func(srcPath string, entry fs.DirEntry) error { for _, pattern := range config.Watch { diff --git a/buildengine/topological_test.go b/buildengine/topological_test.go index 6b0c971637..8033e50764 100644 --- a/buildengine/topological_test.go +++ b/buildengine/topological_test.go @@ -4,27 +4,29 @@ import ( "testing" "github.com/alecthomas/assert/v2" + + "github.com/TBD54566975/ftl/common/moduleconfig" ) func TestBuildOrder(t *testing.T) { modules := []Module{ { - ModuleConfig: ModuleConfig{Module: "alpha"}, + ModuleConfig: moduleconfig.ModuleConfig{Module: "alpha"}, Dependencies: []string{"beta", "gamma"}, }, { - ModuleConfig: ModuleConfig{Module: "beta"}, + ModuleConfig: moduleconfig.ModuleConfig{Module: "beta"}, Dependencies: []string{"kappa"}, }, { - ModuleConfig: ModuleConfig{Module: "gamma"}, + ModuleConfig: moduleconfig.ModuleConfig{Module: "gamma"}, Dependencies: []string{"kappa"}, }, { - ModuleConfig: ModuleConfig{Module: "kappa"}, + ModuleConfig: moduleconfig.ModuleConfig{Module: "kappa"}, }, { - ModuleConfig: ModuleConfig{Module: "delta"}, + ModuleConfig: moduleconfig.ModuleConfig{Module: "delta"}, }, } @@ -33,15 +35,15 @@ func TestBuildOrder(t *testing.T) { expected := [][]Module{ { - {ModuleConfig: ModuleConfig{Module: "delta"}}, - {ModuleConfig: ModuleConfig{Module: "kappa"}}, + {ModuleConfig: moduleconfig.ModuleConfig{Module: "delta"}}, + {ModuleConfig: moduleconfig.ModuleConfig{Module: "kappa"}}, }, { - {ModuleConfig: ModuleConfig{Module: "beta"}, Dependencies: []string{"kappa"}}, - {ModuleConfig: ModuleConfig{Module: "gamma"}, Dependencies: []string{"kappa"}}, + {ModuleConfig: moduleconfig.ModuleConfig{Module: "beta"}, Dependencies: []string{"kappa"}}, + {ModuleConfig: moduleconfig.ModuleConfig{Module: "gamma"}, Dependencies: []string{"kappa"}}, }, { - {ModuleConfig: ModuleConfig{Module: "alpha"}, Dependencies: []string{"beta", "gamma"}}, + {ModuleConfig: moduleconfig.ModuleConfig{Module: "alpha"}, Dependencies: []string{"beta", "gamma"}}, }, } assert.Equal(t, expected, graph) diff --git a/buildengine/watch.go b/buildengine/watch.go index 9cb3c23d90..ce6e71360b 100644 --- a/buildengine/watch.go +++ b/buildengine/watch.go @@ -6,6 +6,7 @@ import ( "github.com/alecthomas/types/pubsub" + "github.com/TBD54566975/ftl/common/moduleconfig" "github.com/TBD54566975/ftl/internal/log" "github.com/TBD54566975/ftl/internal/maps" ) @@ -60,7 +61,7 @@ func Watch(ctx context.Context, period time.Duration, dirs ...string) *pubsub.To logger.Tracef("error discovering modules: %v", err) continue } - moduleConfigsByDir := maps.FromSlice(moduleConfigs, func(config ModuleConfig) (string, ModuleConfig) { + moduleConfigsByDir := maps.FromSlice(moduleConfigs, func(config moduleconfig.ModuleConfig) (string, moduleconfig.ModuleConfig) { return config.Module, config }) diff --git a/buildengine/watch_test.go b/buildengine/watch_test.go index 5a704fbb50..c868848b06 100644 --- a/buildengine/watch_test.go +++ b/buildengine/watch_test.go @@ -1,4 +1,4 @@ -package buildengine +package buildengine_test import ( "context" @@ -10,6 +10,7 @@ import ( "github.com/alecthomas/assert/v2" + . "github.com/TBD54566975/ftl/buildengine" "github.com/TBD54566975/ftl/internal/log" ) diff --git a/cmd/ftl/cmd_build.go b/cmd/ftl/cmd_build.go index 6b63159e90..f3a7cc9541 100644 --- a/cmd/ftl/cmd_build.go +++ b/cmd/ftl/cmd_build.go @@ -2,10 +2,8 @@ package main import ( "context" - "fmt" "time" - "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect" "github.com/TBD54566975/ftl/buildengine" "github.com/TBD54566975/ftl/internal/log" ) @@ -14,36 +12,22 @@ type buildCmd struct { ModuleDir string `arg:"" help:"Directory containing ftl.toml" type:"existingdir" default:"."` } -func (b *buildCmd) Run(ctx context.Context, client ftlv1connect.ControllerServiceClient) error { +func (b *buildCmd) Run(ctx context.Context) error { logger := log.FromContext(ctx) startTime := time.Now() - // Load the TOML file. - var err error - config, err := buildengine.LoadModuleConfig(b.ModuleDir) + + module, err := buildengine.LoadModule(b.ModuleDir) if err != nil { return err } + logger.Infof("Building %s module '%s'", module.Language, module.Module) - logger.Infof("Building %s module '%s'", config.Language, config.Module) - - switch config.Language { - case "kotlin": - err = b.buildKotlin(ctx, config) - - case "go": - err = b.buildGo(ctx, client) - - default: - return fmt.Errorf("unable to build, unknown language %q", config.Language) - } - + err = buildengine.Build(ctx, module) if err != nil { return err } - duration := time.Since(startTime) - formattedDuration := fmt.Sprintf("%.2fs", duration.Seconds()) - logger.Infof("Successfully built module '%s' in %s", config.Module, formattedDuration) + logger.Infof("Successfully built module '%s' in %.2fs", module.Module, time.Since(startTime).Seconds()) return nil } diff --git a/cmd/ftl/cmd_deploy.go b/cmd/ftl/cmd_deploy.go index f53b039098..d6993647a0 100644 --- a/cmd/ftl/cmd_deploy.go +++ b/cmd/ftl/cmd_deploy.go @@ -16,7 +16,7 @@ import ( ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1" "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect" schemapb "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/schema" - "github.com/TBD54566975/ftl/buildengine" + "github.com/TBD54566975/ftl/common/moduleconfig" "github.com/TBD54566975/ftl/internal/log" "github.com/TBD54566975/ftl/internal/sha256" "github.com/TBD54566975/ftl/internal/slices" @@ -32,7 +32,7 @@ func (d *deployCmd) Run(ctx context.Context, client ftlv1connect.ControllerServi logger := log.FromContext(ctx) // Load the TOML file. - config, err := buildengine.LoadModuleConfig(d.ModuleDir) + config, err := moduleconfig.LoadModuleConfig(d.ModuleDir) if err != nil { return err } @@ -42,7 +42,7 @@ func (d *deployCmd) Run(ctx context.Context, client ftlv1connect.ControllerServi } build := buildCmd{ModuleDir: d.ModuleDir} - err = build.Run(ctx, client) + err = build.Run(ctx) if err != nil { return err } @@ -145,7 +145,7 @@ func (d *deployCmd) checkReadiness(ctx context.Context, client ftlv1connect.Cont } } } -func (d *deployCmd) loadProtoSchema(deployDir string, config buildengine.ModuleConfig) (*schemapb.Module, error) { +func (d *deployCmd) loadProtoSchema(deployDir string, config moduleconfig.ModuleConfig) (*schemapb.Module, error) { schema := filepath.Join(deployDir, config.Schema) content, err := os.ReadFile(schema) if err != nil { diff --git a/cmd/ftl/cmd_dev.go b/cmd/ftl/cmd_dev.go index 7f534f5711..4c6a8b5926 100644 --- a/cmd/ftl/cmd_dev.go +++ b/cmd/ftl/cmd_dev.go @@ -14,6 +14,7 @@ import ( "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect" "github.com/TBD54566975/ftl/backend/schema" "github.com/TBD54566975/ftl/buildengine" + "github.com/TBD54566975/ftl/common/moduleconfig" "github.com/TBD54566975/ftl/internal/log" ) @@ -127,7 +128,7 @@ func (d *devCmd) Run(ctx context.Context, client ftlv1connect.ControllerServiceC for dir := range modules { currentModule := modules[dir] - config, err := buildengine.LoadModuleConfig(dir) + config, err := moduleconfig.LoadModuleConfig(dir) if err != nil { return err } @@ -261,7 +262,7 @@ func (d *devCmd) addOrRemoveModules(tomls []string, modules moduleMap) error { for _, toml := range tomls { dir := filepath.Dir(toml) if _, ok := modules[dir]; !ok { - config, err := buildengine.LoadModuleConfig(dir) + config, err := moduleconfig.LoadModuleConfig(dir) if err != nil { return err } diff --git a/cmd/ftl/cmd_init.go b/cmd/ftl/cmd_init.go index c30c338394..4461d42a3f 100644 --- a/cmd/ftl/cmd_init.go +++ b/cmd/ftl/cmd_init.go @@ -16,6 +16,7 @@ import ( "github.com/TBD54566975/ftl/backend/schema" "github.com/TBD54566975/ftl/backend/schema/strcase" + "github.com/TBD54566975/ftl/buildengine" goruntime "github.com/TBD54566975/ftl/go-runtime" "github.com/TBD54566975/ftl/internal" "github.com/TBD54566975/ftl/internal/exec" @@ -90,7 +91,7 @@ func (i initKotlinCmd) Run(ctx context.Context, parent *initCmd) error { return err } - return setPomProperties(log.FromContext(ctx), i.Dir) + return buildengine.SetPOMProperties(ctx, i.Dir) } func updatePom(pomFile, name string) error { diff --git a/buildengine/moduleconfig.go b/common/moduleconfig/moduleconfig.go similarity index 99% rename from buildengine/moduleconfig.go rename to common/moduleconfig/moduleconfig.go index b0a2a2e95d..034829ca2e 100644 --- a/buildengine/moduleconfig.go +++ b/common/moduleconfig/moduleconfig.go @@ -1,4 +1,4 @@ -package buildengine +package moduleconfig import ( "fmt" diff --git a/go-runtime/compile/build.go b/go-runtime/compile/build.go index e775b937dd..006135e2b0 100644 --- a/go-runtime/compile/build.go +++ b/go-runtime/compile/build.go @@ -17,7 +17,7 @@ import ( "github.com/TBD54566975/ftl" "github.com/TBD54566975/ftl/backend/schema" "github.com/TBD54566975/ftl/backend/schema/strcase" - "github.com/TBD54566975/ftl/buildengine" + "github.com/TBD54566975/ftl/common/moduleconfig" "github.com/TBD54566975/ftl/internal" "github.com/TBD54566975/ftl/internal/exec" "github.com/TBD54566975/ftl/internal/log" @@ -67,7 +67,7 @@ func Build(ctx context.Context, moduleDir string, sch *schema.Schema) error { ftlVersion = ftl.Version } - config, err := buildengine.LoadModuleConfig(moduleDir) + config, err := moduleconfig.LoadModuleConfig(moduleDir) if err != nil { return fmt.Errorf("failed to load module config: %w", err) } diff --git a/go-runtime/encoding/encoding_test.go b/go-runtime/encoding/encoding_test.go index f713307117..860a7e479a 100644 --- a/go-runtime/encoding/encoding_test.go +++ b/go-runtime/encoding/encoding_test.go @@ -3,10 +3,9 @@ package encoding_test import ( "testing" - "github.com/alecthomas/assert/v2" - . "github.com/TBD54566975/ftl/go-runtime/encoding" "github.com/TBD54566975/ftl/go-runtime/ftl" + "github.com/alecthomas/assert/v2" ) func TestMarshal(t *testing.T) { @@ -33,6 +32,7 @@ func TestMarshal(t *testing.T) { {name: "OptionPtr", input: struct{ Option *ftl.Option[int] }{&somePtr}, expected: `{"option":42}`}, {name: "OptionStruct", input: struct{ Option ftl.Option[inner] }{ftl.Some(inner{"foo"})}, expected: `{"option":{"fooBar":"foo"}}`}, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { actual, err := Marshal(tt.input)