Skip to content

simplify the FunctionalOptions by removing the Options struct #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/helpers/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
67 changes: 15 additions & 52 deletions machines/extism/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
132 changes: 86 additions & 46 deletions machines/extism/compiler/options.go
Original file line number Diff line number Diff line change
@@ -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
}
}
Expand All @@ -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
}
}
Expand All @@ -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
}
Loading