From 14c568a9d29ed8c9f843e131ca0dedf3200198d3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 5 May 2013 14:47:17 -0700 Subject: [PATCH] Get rid of BuilderFactory --- packer/build.go | 16 --------- packer/build_test.go | 12 ------- packer/environment.go | 70 ++++++++++++++++++-------------------- packer/environment_test.go | 36 ++++---------------- packer/template.go | 4 +-- packer/template_test.go | 4 +-- 6 files changed, 45 insertions(+), 97 deletions(-) diff --git a/packer/build.go b/packer/build.go index c17facfc1..5bbb7483d 100644 --- a/packer/build.go +++ b/packer/build.go @@ -33,22 +33,6 @@ type Builder interface { Run(build Build, ui Ui) } -// This factory is responsible for returning Builders for the given name. -// -// CreateBuilder is called with the string name of a builder and should -// return a Builder implementation by that name or nil if no builder can be -// found. -type BuilderFactory interface { - CreateBuilder(name string) Builder -} - -// This implements BuilderFactory to return nil for every builder. -type NilBuilderFactory byte - -func (NilBuilderFactory) CreateBuilder(name string) Builder { - return nil -} - // Prepare prepares the build by doing some initialization for the builder // and any hooks. This _must_ be called prior to Run. func (b *coreBuild) Prepare() { diff --git a/packer/build_test.go b/packer/build_test.go index 08161b3d5..cc0a63970 100644 --- a/packer/build_test.go +++ b/packer/build_test.go @@ -5,14 +5,6 @@ import ( "testing" ) -type hashBuilderFactory struct { - builderMap map[string]Builder -} - -func (bf *hashBuilderFactory) CreateBuilder(name string) Builder { - return bf.builderMap[name] -} - type TestBuilder struct { prepareCalled bool prepareConfig interface{} @@ -44,10 +36,6 @@ func testBuilder() *TestBuilder { return &TestBuilder{} } -func testBuildFactory(builderMap map[string]Builder) BuilderFactory { - return &hashBuilderFactory{builderMap} -} - func TestBuild_Prepare(t *testing.T) { assert := asserts.NewTestingAsserts(t, true) diff --git a/packer/environment.go b/packer/environment.go index 73f5de17c..5f6e7681d 100644 --- a/packer/environment.go +++ b/packer/environment.go @@ -9,13 +9,16 @@ import ( "strings" ) +type BuilderFunc func(name string) Builder + +type CommandFunc func(name string) Command + // The environment interface provides access to the configuration and // state of a single Packer run. // // It allows for things such as executing CLI commands, getting the // list of available builders, and more. type Environment interface { - BuilderFactory() BuilderFactory Cli(args []string) int Ui() Ui } @@ -23,15 +26,17 @@ type Environment interface { // An implementation of an Environment that represents the Packer core // environment. type coreEnvironment struct { - builderFactory BuilderFactory - command map[string]Command + builderFunc BuilderFunc + commands []string + commandFunc CommandFunc ui Ui } // This struct configures new environments. type EnvironmentConfig struct { - BuilderFactory BuilderFactory - Command map[string]Command + BuilderFunc BuilderFunc + CommandFunc CommandFunc + Commands []string Ui Ui } @@ -39,8 +44,9 @@ type EnvironmentConfig struct { // be used to create a new enviroment with NewEnvironment with sane defaults. func DefaultEnvironmentConfig() *EnvironmentConfig { config := &EnvironmentConfig{} - config.BuilderFactory = new(NilBuilderFactory) - config.Command = make(map[string]Command) + config.BuilderFunc = func(string) Builder { return nil } + config.CommandFunc = func(string) Command { return nil } + config.Commands = make([]string, 0) config.Ui = &ReaderWriterUi{os.Stdin, os.Stdout} return config } @@ -53,28 +59,15 @@ func NewEnvironment(config *EnvironmentConfig) (resultEnv Environment, err error } env := &coreEnvironment{} - env.builderFactory = config.BuilderFactory - env.command = make(map[string]Command) + env.builderFunc = config.BuilderFunc + env.commandFunc = config.CommandFunc + env.commands = config.Commands env.ui = config.Ui - for k, v := range config.Command { - env.command[k] = v - } - - // TODO: Should "version" be allowed to be overriden? - if _, ok := env.command["version"]; !ok { - env.command["version"] = new(versionCommand) - } - resultEnv = env return } -// Returns the BuilderFactory associated with this Environment. -func (e *coreEnvironment) BuilderFactory() BuilderFactory { - return e.builderFactory -} - // Executes a command as if it was typed on the command-line interface. // The return value is the exit code of the command. func (e *coreEnvironment) Cli(args []string) int { @@ -83,16 +76,23 @@ func (e *coreEnvironment) Cli(args []string) int { return 1 } - command, ok := e.command[args[0]] - if !ok { - // The command was not found. In this case, let's go through - // the arguments and see if the user is requesting the version. + version := args[0] == "version" + if !version { for _, arg := range args { if arg == "--version" || arg == "-v" { - command = e.command["version"] + version = true break } } + } + + var command Command + if version { + command = new(versionCommand) + } + + if command == nil { + command = e.commandFunc(args[0]) // If we still don't have a command, show the help. if command == nil { @@ -108,25 +108,23 @@ func (e *coreEnvironment) Cli(args []string) int { func (e *coreEnvironment) printHelp() { // Created a sorted slice of the map keys and record the longest // command name so we can better format the output later. - commandKeys := make([]string, len(e.command)) i := 0 maxKeyLen := 0 - for key, _ := range e.command { - commandKeys[i] = key - if len(key) > maxKeyLen { - maxKeyLen = len(key) + for _, command := range e.commands { + if len(command) > maxKeyLen { + maxKeyLen = len(command) } i++ } // Sort the keys - sort.Strings(commandKeys) + sort.Strings(e.commands) e.ui.Say("usage: packer [--version] [--help] []\n\n") e.ui.Say("Available commands are:\n") - for _, key := range commandKeys { - command := e.command[key] + for _, key := range e.commands { + command := e.commandFunc(key) // Pad the key with spaces so that they're all the same width key = fmt.Sprintf("%v%v", key, strings.Repeat(" ", maxKeyLen-len(key))) diff --git a/packer/environment_test.go b/packer/environment_test.go index 303bec327..0a28cc988 100644 --- a/packer/environment_test.go +++ b/packer/environment_test.go @@ -3,14 +3,13 @@ package packer import ( "bytes" "cgl.tideland.biz/asserts" - "encoding/gob" "os" "strings" "testing" ) func testEnvironment() Environment { - config := &EnvironmentConfig{} + config := DefaultEnvironmentConfig() config.Ui = &ReaderWriterUi{ new(bytes.Buffer), new(bytes.Buffer), @@ -24,25 +23,11 @@ func testEnvironment() Environment { return env } -// This is just a sanity test to prove that our coreEnvironment can't -// encode or decode using gobs. That is fine, and expected, but I just like -// to be sure. -func TestEnvironment_CoreEnvironmentCantEncode(t *testing.T) { - assert := asserts.NewTestingAsserts(t, true) - - env := testEnvironment() - - b := new(bytes.Buffer) - e := gob.NewEncoder(b) - err := e.Encode(env) - assert.NotNil(err, "encoding should fail") -} - -func TestEnvironment_DefaultConfig_Command(t *testing.T) { +func TestEnvironment_DefaultConfig_Commands(t *testing.T) { assert := asserts.NewTestingAsserts(t, true) config := DefaultEnvironmentConfig() - assert.NotNil(config.Command, "default Command should not be nil") + assert.Empty(config.Commands, "should have no commands") } func TestEnvironment_DefaultConfig_Ui(t *testing.T) { @@ -65,23 +50,16 @@ func TestNewEnvironment_NoConfig(t *testing.T) { assert.NotNil(err, "should be an error") } -func TestEnvironment_GetBuilderFactory(t *testing.T) { - assert := asserts.NewTestingAsserts(t, true) - - config := DefaultEnvironmentConfig() - env, _ := NewEnvironment(config) - - assert.Equal(env.BuilderFactory(), config.BuilderFactory, "should match factories") -} - func TestEnvironment_Cli_CallsRun(t *testing.T) { assert := asserts.NewTestingAsserts(t, true) command := &TestCommand{} + commands := make(map[string]Command) + commands["foo"] = command config := &EnvironmentConfig{} - config.Command = make(map[string]Command) - config.Command["foo"] = command + config.Commands = []string{"foo"} + config.CommandFunc = func(n string) Command { return commands[n] } env, _ := NewEnvironment(config) assert.Equal(env.Cli([]string{"foo", "bar", "baz"}), 0, "runs foo command") diff --git a/packer/template.go b/packer/template.go index 4cea19817..7877d502e 100644 --- a/packer/template.go +++ b/packer/template.go @@ -95,14 +95,14 @@ func (t *Template) BuildNames() []string { // // If the build does not exist as part of this template, an error is // returned. -func (t *Template) Build(name string, bf BuilderFactory) (b Build, err error) { +func (t *Template) Build(name string, bf BuilderFunc) (b Build, err error) { builderConfig, ok := t.Builders[name] if !ok { err = fmt.Errorf("No such build found in template: %s", name) return } - builder := bf.CreateBuilder(builderConfig.builderName) + builder := bf(builderConfig.builderName) if builder == nil { err = fmt.Errorf("Builder could not be found: %s", builderConfig.builderName) return diff --git a/packer/template_test.go b/packer/template_test.go index f49b99d72..3f19044d2 100644 --- a/packer/template_test.go +++ b/packer/template_test.go @@ -157,7 +157,7 @@ func TestTemplate_BuildUnknownBuilder(t *testing.T) { template, err := ParseTemplate([]byte(data)) assert.Nil(err, "should not error") - builderFactory := testBuildFactory(map[string]Builder{}) + builderFactory := func(string) Builder { return nil } build, err := template.Build("test1", builderFactory) assert.Nil(build, "build should be nil") assert.NotNil(err, "should have error") @@ -191,7 +191,7 @@ func TestTemplate_Build(t *testing.T) { "test-builder": builder, } - builderFactory := testBuildFactory(builderMap) + builderFactory := func(n string) Builder { return builderMap[n] } // Get the build, verifying we can get it without issue, but also // that the proper builder was looked up and used for the build.