Skip to content

Commit

Permalink
chore: make using ModuleConfig paths more consistent (#1877)
Browse files Browse the repository at this point in the history
`.Abs()` will return a clone with all paths made absolute. This avoids a
bunch of manual and error prone joining.
  • Loading branch information
alecthomas authored Jun 26, 2024
1 parent 6bec1fb commit cc0c9e5
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 22 deletions.
11 changes: 5 additions & 6 deletions buildengine/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func buildModule(ctx context.Context, sch *schema.Schema, module Module, filesTr
ctx = log.ContextWithLogger(ctx, logger)

// clear the deploy directory before extracting schema
if err := os.RemoveAll(module.Config.AbsDeployDir()); err != nil {
if err := os.RemoveAll(module.Config.Abs().DeployDir); err != nil {
return fmt.Errorf("failed to clear errors: %w", err)
}

Expand All @@ -55,7 +55,7 @@ func buildModule(ctx context.Context, sch *schema.Schema, module Module, filesTr
errs = append(errs, err)
}
// read runtime-specific build errors from the build directory
errorList, err := loadProtoErrors(module.Config)
errorList, err := loadProtoErrors(module.Config.Abs())
if err != nil {
return fmt.Errorf("failed to read build errors for module: %w", err)
}
Expand All @@ -71,13 +71,12 @@ func buildModule(ctx context.Context, sch *schema.Schema, module Module, filesTr
return nil
}

func loadProtoErrors(config moduleconfig.ModuleConfig) (*schema.ErrorList, error) {
f := filepath.Join(config.AbsDeployDir(), config.Errors)
if _, err := os.Stat(f); errors.Is(err, os.ErrNotExist) {
func loadProtoErrors(config moduleconfig.AbsModuleConfig) (*schema.ErrorList, error) {
if _, err := os.Stat(config.Errors); errors.Is(err, os.ErrNotExist) {
return &schema.ErrorList{Errors: make([]*schema.Error, 0)}, nil
}

content, err := os.ReadFile(f)
content, err := os.ReadFile(config.Errors)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion buildengine/build_kotlin.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func SetPOMProperties(ctx context.Context, baseDir string) error {
}

func prepareFTLRoot(module Module) error {
buildDir := module.Config.AbsDeployDir()
buildDir := module.Config.Abs().DeployDir
if err := os.MkdirAll(buildDir, 0700); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion buildengine/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func assertBuildProtoErrors(msgs ...string) assertion {
t.Helper()
config, err := moduleconfig.LoadModuleConfig(bctx.moduleDir)
assert.NoError(t, err, "Error loading module config")
errorList, err := loadProtoErrors(config)
errorList, err := loadProtoErrors(config.Abs())
assert.NoError(t, err, "Error loading proto errors")

expected := make([]*schema.Error, 0, len(msgs))
Expand Down
20 changes: 9 additions & 11 deletions buildengine/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ func Deploy(ctx context.Context, module Module, replicas int32, waitForDeployOnl
ctx = log.ContextWithLogger(ctx, logger)
logger.Infof("Deploying module")

deployDir := module.Config.AbsDeployDir()
files, err := findFiles(deployDir, module.Config.Deploy)
moduleConfig := module.Config.Abs()
files, err := findFiles(moduleConfig)
if err != nil {
logger.Errorf(err, "failed to find files in %s", deployDir)
logger.Errorf(err, "failed to find files in %s", moduleConfig)
return err
}

filesByHash, err := hashFiles(deployDir, files)
filesByHash, err := hashFiles(moduleConfig.DeployDir, files)
if err != nil {
return err
}
Expand All @@ -57,7 +57,7 @@ func Deploy(ctx context.Context, module Module, replicas int32, waitForDeployOnl
return err
}

moduleSchema, err := loadProtoSchema(deployDir, module.Config, replicas)
moduleSchema, err := loadProtoSchema(moduleConfig, replicas)
if err != nil {
return fmt.Errorf("failed to load protobuf schema from %q: %w", module.Config.Schema, err)
}
Expand Down Expand Up @@ -130,9 +130,8 @@ func terminateModuleDeployment(ctx context.Context, client ftlv1connect.Controll
return err
}

func loadProtoSchema(deployDir string, config moduleconfig.ModuleConfig, replicas int32) (*schemapb.Module, error) {
schema := filepath.Join(deployDir, config.Schema)
content, err := os.ReadFile(schema)
func loadProtoSchema(config moduleconfig.AbsModuleConfig, replicas int32) (*schemapb.Module, error) {
content, err := os.ReadFile(config.Schema)
if err != nil {
return nil, err
}
Expand All @@ -155,10 +154,9 @@ func loadProtoSchema(deployDir string, config moduleconfig.ModuleConfig, replica
return module, nil
}

func findFiles(base string, files []string) ([]string, error) {
func findFiles(moduleConfig moduleconfig.AbsModuleConfig) ([]string, error) {
var out []string
for _, file := range files {
file = filepath.Join(base, file)
for _, file := range moduleConfig.Deploy {
info, err := os.Stat(file)
if err != nil {
return nil, err
Expand Down
47 changes: 46 additions & 1 deletion common/moduleconfig/moduleconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (

"github.com/BurntSushi/toml"
"golang.org/x/mod/modfile"

"github.com/TBD54566975/ftl/internal/slices"
)

// ModuleGoConfig is language-specific configuration for Go modules.
Expand All @@ -22,7 +24,7 @@ type ModuleKotlinConfig struct{}
//
// Module config files are currently TOML.
type ModuleConfig struct {
// Dir is the root of the module.
// Dir is the absolute path to the root of the module.
Dir string `toml:"-"`

Language string `toml:"language"`
Expand All @@ -45,6 +47,11 @@ type ModuleConfig struct {
Kotlin ModuleKotlinConfig `toml:"kotlin,optional"`
}

// AbsModuleConfig is a ModuleConfig with all paths made absolute.
//
// This is a type alias to prevent accidental use of the wrong type.
type AbsModuleConfig ModuleConfig

// LoadModuleConfig from a directory.
func LoadModuleConfig(dir string) (ModuleConfig, error) {
path := filepath.Join(dir, "ftl.toml")
Expand All @@ -60,6 +67,44 @@ func LoadModuleConfig(dir string) (ModuleConfig, error) {
return config, nil
}

func (c ModuleConfig) String() string {
return fmt.Sprintf("%s (%s)", c.Module, c.Dir)
}

// Abs creates a clone of ModuleConfig with all paths made absolute.
//
// This function will panic if any paths are not beneath the module directory.
// This should never happen under normal use, as LoadModuleConfig performs this
// validation separately. This is just a sanity check.
func (c ModuleConfig) Abs() AbsModuleConfig {
clone := c
clone.Dir = filepath.Clean(clone.Dir)
clone.DeployDir = filepath.Clean(filepath.Join(clone.Dir, clone.DeployDir))
if !strings.HasPrefix(clone.DeployDir, clone.Dir) {
panic(fmt.Sprintf("deploy-dir %q is not beneath module directory %q", clone.DeployDir, clone.Dir))
}
clone.Schema = filepath.Clean(filepath.Join(clone.DeployDir, clone.Schema))
if !strings.HasPrefix(clone.Schema, clone.DeployDir) {
panic(fmt.Sprintf("schema %q is not beneath deploy directory %q", clone.Schema, clone.DeployDir))
}
clone.Errors = filepath.Clean(filepath.Join(clone.DeployDir, clone.Errors))
if !strings.HasPrefix(clone.Errors, clone.DeployDir) {
panic(fmt.Sprintf("errors %q is not beneath deploy directory %q", clone.Errors, clone.DeployDir))
}
clone.Deploy = slices.Map(clone.Deploy, func(p string) string {
out := filepath.Clean(filepath.Join(clone.DeployDir, p))
if !strings.HasPrefix(out, clone.DeployDir) {
panic(fmt.Sprintf("deploy path %q is not beneath deploy directory %q", out, clone.DeployDir))
}
return out
})
// Watch paths are allowed to be outside the deploy directory.
clone.Watch = slices.Map(clone.Watch, func(p string) string {
return filepath.Clean(filepath.Join(clone.Dir, p))
})
return AbsModuleConfig(clone)
}

// AbsDeployDir returns the absolute path to the deploy directory.
func (c ModuleConfig) AbsDeployDir() string {
return filepath.Join(c.Dir, c.DeployDir)
Expand Down
4 changes: 2 additions & 2 deletions go-runtime/compile/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ func writeSchema(config moduleconfig.ModuleConfig, module *schema.Module) error
if err != nil {
return fmt.Errorf("failed to marshal schema: %w", err)
}
return os.WriteFile(filepath.Join(config.AbsDeployDir(), "schema.pb"), schemaBytes, 0600)
return os.WriteFile(config.Abs().Schema, schemaBytes, 0600)
}

func writeSchemaErrors(config moduleconfig.ModuleConfig, errors []*schema.Error) error {
Expand All @@ -550,7 +550,7 @@ func writeSchemaErrors(config moduleconfig.ModuleConfig, errors []*schema.Error)
if err != nil {
return fmt.Errorf("failed to marshal errors: %w", err)
}
return os.WriteFile(filepath.Join(config.AbsDeployDir(), config.Errors), elBytes, 0600)
return os.WriteFile(config.Abs().Errors, elBytes, 0600)
}

func getSumTypes(module *schema.Module, sch *schema.Schema, nativeNames NativeNames) []goSumType {
Expand Down

0 comments on commit cc0c9e5

Please sign in to comment.