Get rid of BuilderFactory

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() {

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)
 

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] <command> [<args>]\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)))

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")

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

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.