simplify the FunctionalOptions by removing the Options struct (#20)

* simplify the FunctionalOptions by removing the Options struct

* remove the atomic.Value for the entrypoint on extism

* fix up ctx handling, add more tests

* remove SetEntrypointName on extism

* harmonize compile logging

* remove the defaults being set in the NewCompiler, they should all be set in the applyDefaults method

* centralize logger setup in Compiler initialization

Author: robbyt

internal/helpers/logger.go

@@ -26,7 +26,7 @@ func SetupLogger(
 		handler = defaultHandler.WithGroup(vmName)
 		// Create a logger from the handler
 		defaultLogger := slog.New(handler)
-		defaultLogger.Warn("Handler is nil, using the default logger configuration.")
+		defaultLogger.Debug("Handler is nil, using the default logger configuration.")
 	}
 
 	var logger *slog.Logger

machines/extism/compiler/compiler.go

@@ -5,17 +5,15 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
-	"sync/atomic"
 
 	extismSDK "github.com/extism/go-sdk"
 	"github.com/robbyt/go-polyscript/execution/script"
-	"github.com/robbyt/go-polyscript/internal/helpers"
 	"github.com/robbyt/go-polyscript/machines/extism/compiler/internal/compile"
 )
 
 // Compiler implements the script.Compiler interface for WASM modules
 type Compiler struct {
-	entryPointName atomic.Value
+	entryPointName string
 	ctx            context.Context
 	options        *compile.Settings
 	logHandler     slog.Handler
@@ -24,53 +22,28 @@ type Compiler struct {
 
 // NewCompiler creates a new Extism WASM Compiler instance with the provided options.
 func NewCompiler(opts ...FunctionalOption) (*Compiler, error) {
-	// Initialize config with defaults
-	cfg := &Options{}
-	ApplyDefaults(cfg)
+	// Initialize the compiler with an empty struct
+	c := &Compiler{}
+
+	// Apply defaults
+	c.applyDefaults()
 
 	// Apply all options
 	for _, opt := range opts {
-		if err := opt(cfg); err != nil {
+		if err := opt(c); err != nil {
 			return nil, fmt.Errorf("error applying compiler option: %w", err)
 		}
 	}
 
 	// Validate the configuration
-	if err := Validate(cfg); err != nil {
+	if err := c.validate(); err != nil {
 		return nil, fmt.Errorf("invalid compiler configuration: %w", err)
 	}
 
-	var handler slog.Handler
-	var logger *slog.Logger
-
-	// Set up logging based on provided options
-	if cfg.Logger != nil {
-		// User provided a custom logger
-		logger = cfg.Logger
-		handler = logger.Handler()
-	} else {
-		// User provided a handler or we're using the default
-		handler, logger = helpers.SetupLogger(cfg.LogHandler, "extism", "Compiler")
-	}
-
-	// Set up entry point name in atomic.Value
-	var entryPointAtomicValue atomic.Value
-	entryPointAtomicValue.Store(cfg.EntryPoint)
+	// Finalize logger setup after all options have been applied
+	c.setupLogger()
 
-	// Create compile options from config
-	compileOpts := &compile.Settings{
-		EnableWASI:    cfg.EnableWASI,
-		RuntimeConfig: cfg.RuntimeConfig,
-		HostFunctions: cfg.HostFunctions,
-	}
-
-	return &Compiler{
-		entryPointName: entryPointAtomicValue,
-		ctx:            context.Background(),
-		options:        compileOpts,
-		logHandler:     handler,
-		logger:         logger,
-	}, nil
+	return c, nil
 }
 
 func (c *Compiler) String() string {
@@ -104,34 +77,30 @@ func (c *Compiler) Compile(scriptReader io.ReadCloser) (script.ExecutableContent
 
 	logger.Debug("Starting WASM compilation", "scriptLength", len(scriptBytes))
 
-	// Compile the WASM module using the CompileBytes function
+	// Compile the WASM module using the CompileBytes function from the internal compile package
 	plugin, err := compile.CompileBytes(c.ctx, scriptBytes, c.options)
 	if err != nil {
-		logger.Warn("WASM compilation failed", "error", err)
 		return nil, fmt.Errorf("%w: %w", ErrValidationFailed, err)
 	}
 
 	if plugin == nil {
-		logger.Error("Compilation returned nil plugin")
 		return nil, ErrBytecodeNil
 	}
 
 	// Create a temporary instance to verify the entry point exists
 	instance, err := plugin.Instance(c.ctx, extismSDK.PluginInstanceConfig{})
 	if err != nil {
-		logger.Error("Failed to create test instance", "error", err)
 		return nil, fmt.Errorf("%w: failed to create test instance: %w", ErrValidationFailed, err)
 	}
 	defer func() {
 		if err := instance.Close(c.ctx); err != nil {
-			c.logger.Warn("Failed to close Extism plugin instance in compiler", "error", err)
+			logger.Warn("Failed to close Extism plugin instance in compiler", "error", err)
 		}
 	}()
 
 	// Verify the entry point function exists
 	funcName := c.GetEntryPointName()
 	if !instance.FunctionExists(funcName) {
-		logger.Error("Entry point function not found", "function", funcName)
 		return nil, fmt.Errorf(
 			"%w: entry point function '%s' not found",
 			ErrValidationFailed,
@@ -142,20 +111,14 @@ func (c *Compiler) Compile(scriptReader io.ReadCloser) (script.ExecutableContent
 	// Create executable with the compiled plugin
 	executable := NewExecutable(scriptBytes, plugin, funcName)
 	if executable == nil {
-		logger.Warn("Failed to create Executable from WASM plugin")
 		return nil, ErrExecCreationFailed
 	}
 
-	logger.Debug("WASM compilation completed successfully")
+	logger.Debug("WASM compilation completed")
 	return executable, nil
 }
 
-// SetEntryPointName is a way to point the compiler at a different entrypoint in the wasm binary
-func (c *Compiler) SetEntryPointName(fName string) {
-	c.entryPointName.Store(fName)
-}
-
 // GetEntryPointName is a getter for the func name entrypoint
 func (c *Compiler) GetEntryPointName() string {
-	return c.entryPointName.Load().(string)
+	return c.entryPointName
 }

machines/extism/compiler/options.go

@@ -1,34 +1,27 @@
 package compiler
 
 import (
+	"context"
 	"fmt"
 	"log/slog"
 	"os"
 
 	extismSDK "github.com/extism/go-sdk"
+	"github.com/robbyt/go-polyscript/internal/helpers"
+	"github.com/robbyt/go-polyscript/machines/extism/compiler/internal/compile"
 	"github.com/tetratelabs/wazero"
 )
 
-// Options holds the configuration for the Extism compiler
-type Options struct {
-	EntryPoint    string
-	LogHandler    slog.Handler
-	Logger        *slog.Logger
-	EnableWASI    bool
-	RuntimeConfig wazero.RuntimeConfig
-	HostFunctions []extismSDK.HostFunction
-}
-
-// FunctionalOption is a function that configures a Options instance
-type FunctionalOption func(*Options) error
+// FunctionalOption is a function that configures a Compiler instance
+type FunctionalOption func(*Compiler) error
 
 // WithEntryPoint creates an option to set the entry point for Extism WASM modules
 func WithEntryPoint(entryPoint string) FunctionalOption {
-	return func(cfg *Options) error {
+	return func(c *Compiler) error {
 		if entryPoint == "" {
 			return fmt.Errorf("entry point cannot be empty")
 		}
-		cfg.EntryPoint = entryPoint
+		c.entryPointName = entryPoint
 		return nil
 	}
 }
@@ -37,13 +30,13 @@ func WithEntryPoint(entryPoint string) FunctionalOption {
 // This is the preferred option for logging configuration as it provides
 // more flexibility through the slog.Handler interface.
 func WithLogHandler(handler slog.Handler) FunctionalOption {
-	return func(cfg *Options) error {
+	return func(c *Compiler) error {
 		if handler == nil {
 			return fmt.Errorf("log handler cannot be nil")
 		}
-		cfg.LogHandler = handler
+		c.logHandler = handler
 		// Clear logger if handler is explicitly set
-		cfg.Logger = nil
+		c.logger = nil
 		return nil
 	}
 }
@@ -52,86 +45,133 @@ func WithLogHandler(handler slog.Handler) FunctionalOption {
 // This is less flexible than WithLogHandler but allows users to customize
 // their logging group configuration.
 func WithLogger(logger *slog.Logger) FunctionalOption {
-	return func(cfg *Options) error {
+	return func(c *Compiler) error {
 		if logger == nil {
 			return fmt.Errorf("logger cannot be nil")
 		}
-		cfg.Logger = logger
+		c.logger = logger
 		// Clear handler if logger is explicitly set
-		cfg.LogHandler = nil
+		c.logHandler = nil
 		return nil
 	}
 }
 
 // WithWASIEnabled creates an option to enable or disable WASI support
 func WithWASIEnabled(enabled bool) FunctionalOption {
-	return func(cfg *Options) error {
-		cfg.EnableWASI = enabled
+	return func(c *Compiler) error {
+		if c.options == nil {
+			c.options = &compile.Settings{}
+		}
+		c.options.EnableWASI = enabled
 		return nil
 	}
 }
 
 // WithRuntimeConfig creates an option to set a custom wazero runtime configuration
 func WithRuntimeConfig(config wazero.RuntimeConfig) FunctionalOption {
-	return func(cfg *Options) error {
+	return func(c *Compiler) error {
 		if config == nil {
 			return fmt.Errorf("runtime config cannot be nil")
 		}
-		cfg.RuntimeConfig = config
+		if c.options == nil {
+			c.options = &compile.Settings{}
+		}
+		c.options.RuntimeConfig = config
 		return nil
 	}
 }
 
 // WithHostFunctions creates an option to set additional host functions
 func WithHostFunctions(funcs []extismSDK.HostFunction) FunctionalOption {
-	return func(cfg *Options) error {
-		cfg.HostFunctions = funcs
+	return func(c *Compiler) error {
+		if c.options == nil {
+			c.options = &compile.Settings{}
+		}
+		c.options.HostFunctions = funcs
+		return nil
+	}
+}
+
+// WithContext creates an option to set a custom context for the Extism compiler.
+func WithContext(ctx context.Context) FunctionalOption {
+	return func(c *Compiler) error {
+		if ctx == nil {
+			return fmt.Errorf("context cannot be nil")
+		}
+		c.ctx = ctx
 		return nil
 	}
 }
 
-// ApplyDefaults sets the default values for a compilerConfig
-func ApplyDefaults(cfg *Options) {
+// applyDefaults sets the default values for a compiler
+func (c *Compiler) applyDefaults() {
 	// Default to stderr for logging if neither handler nor logger specified
-	if cfg.LogHandler == nil && cfg.Logger == nil {
-		cfg.LogHandler = slog.NewTextHandler(os.Stderr, nil)
+	if c.logHandler == nil && c.logger == nil {
+		c.logHandler = slog.NewTextHandler(os.Stderr, nil)
 	}
 
-	// Default entry point
-	if cfg.EntryPoint == "" {
-		cfg.EntryPoint = defaultEntryPoint
+	// Set default entry point
+	if c.entryPointName == "" {
+		c.entryPointName = defaultEntryPoint
 	}
 
-	// Default WASI setting
-	cfg.EnableWASI = true
+	// Initialize options struct if nil
+	if c.options == nil {
+		c.options = &compile.Settings{}
+	}
 
-	// Default runtime config
-	if cfg.RuntimeConfig == nil {
-		cfg.RuntimeConfig = wazero.NewRuntimeConfig()
+	// Set default runtime config if not already set
+	if c.options.RuntimeConfig == nil {
+		c.options.RuntimeConfig = wazero.NewRuntimeConfig()
 	}
 
-	// Default to empty host functions
-	if cfg.HostFunctions == nil {
-		cfg.HostFunctions = []extismSDK.HostFunction{}
+	// Set default host functions if not already set
+	if c.options.HostFunctions == nil {
+		c.options.HostFunctions = []extismSDK.HostFunction{}
+	}
+
+	// Default WASI to true (EnableWASI is a bool so we don't need to check if it's nil)
+	c.options.EnableWASI = true
+
+	// Default context
+	if c.ctx == nil {
+		c.ctx = context.Background()
 	}
 }
 
-// Validate checks if the configuration is valid
-func Validate(cfg *Options) error {
+// setupLogger configures the logger and handler based on the current state.
+// This is idempotent and can be called multiple times during initialization.
+func (c *Compiler) setupLogger() {
+	if c.logger != nil {
+		// When a logger is explicitly set, extract its handler
+		c.logHandler = c.logger.Handler()
+	} else {
+		// Otherwise use the handler (which might be default or custom) to create the logger
+		c.logHandler, c.logger = helpers.SetupLogger(c.logHandler, "extism", "Compiler")
+	}
+}
+
+// validate checks if the compiler configuration is valid
+func (c *Compiler) validate() error {
 	// Ensure we have either a logger or a handler
-	if cfg.LogHandler == nil && cfg.Logger == nil {
+	if c.logHandler == nil && c.logger == nil {
 		return fmt.Errorf("either log handler or logger must be specified")
 	}
 
 	// Entry point must be non-empty
-	if cfg.EntryPoint == "" {
+	if c.entryPointName == "" {
 		return fmt.Errorf("entry point must be specified")
 	}
 
 	// Runtime config cannot be nil
-	if cfg.RuntimeConfig == nil {
+	if c.options == nil || c.options.RuntimeConfig == nil {
 		return fmt.Errorf("runtime config cannot be nil")
 	}
 
+	// Context cannot be nil
+	if c.ctx == nil {
+		return fmt.Errorf("context cannot be nil")
+	}
+
 	return nil
 }