more refactoring

command/build.go

@@ -13,7 +13,6 @@ import (
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclparse"
 	"github.com/hashicorp/packer/hcl2template"
-	"github.com/hashicorp/packer/helper/enumflag"
 	"github.com/hashicorp/packer/packer"
 	"github.com/hashicorp/packer/template"
 	"golang.org/x/sync/semaphore"
@@ -37,34 +36,15 @@ func (c *BuildCommand) Run(args []string) int {
 	return c.RunContext(buildCtx, cfg)
 }
 
-// Config is the command-configuration parsed from the command line.
+func (c *BuildCommand) ParseArgs(args []string) (*BuildArgs, int) {
-type Config struct {
+	var cfg *BuildArgs
-	Color, Debug, Force, Timestamp bool
-	ParallelBuilds                 int64
-	OnError                        string
-	Path                           string
-}
-
-func (c *BuildCommand) ParseArgs(args []string) (Config, int) {
-	var cfg Config
-	var parallel bool
 	flags := c.Meta.FlagSet("build", FlagSetBuildFilter|FlagSetVars)
 	flags.Usage = func() { c.Ui.Say(c.Help()) }
-	flags.BoolVar(&cfg.Color, "color", true, "")
+	cfg.AddFlagSets(flags)
-	flags.BoolVar(&cfg.Debug, "debug", false, "")
-	flags.BoolVar(&cfg.Force, "force", false, "")
-	flags.BoolVar(&cfg.Timestamp, "timestamp-ui", false, "")
-	flagOnError := enumflag.New(&cfg.OnError, "cleanup", "abort", "ask")
-	flags.Var(flagOnError, "on-error", "")
-	flags.BoolVar(&parallel, "parallel", true, "")
-	flags.Int64Var(&cfg.ParallelBuilds, "parallel-builds", 0, "")
 	if err := flags.Parse(args); err != nil {
 		return cfg, 1
 	}
 
-	if parallel == false && cfg.ParallelBuilds == 0 {
-		cfg.ParallelBuilds = 1
-	}
 	if cfg.ParallelBuilds < 1 {
 		cfg.ParallelBuilds = math.MaxInt64
 	}
@@ -78,7 +58,7 @@ func (c *BuildCommand) ParseArgs(args []string) (Config, int) {
 	return cfg, 0
 }
 
-func (m *Meta) GetConfigFromHCL(path string) (BuildStarter, int) {
+func (m *Meta) GetConfigFromHCL(path string) (packer.BuildGetter, int) {
 	parser := &hcl2template.Parser{
 		Parser:                hclparse.NewParser(),
 		BuilderSchemas:        m.CoreConfig.Components.BuilderStore,
@@ -87,74 +67,50 @@ func (m *Meta) GetConfigFromHCL(path string) (BuildStarter, int) {
 	}
 
 	cfg, diags := parser.Parse(path, m.varFiles, m.flagVars)
-	{
+	return cfg, writeDiags(m.Ui, parser.Files(), diags)
-		// write HCL errors/diagnostics if any.
+}
-		b := bytes.NewBuffer(nil)
-		err := hcl.NewDiagnosticTextWriter(b, parser.Files(), 80, false).WriteDiagnostics(diags)
-		if err != nil {
-			m.Ui.Error("could not write diagnostic: " + err.Error())
-			return nil, 1
-		}
-		if b.Len() != 0 {
-			m.Ui.Message(b.String())
-		}
-	}
-	ret := 0
-	if diags.HasErrors() {
-		ret = 1
-	}
 
-	return func(opts buildStarterOptions) ([]packer.Build, int) {
+func writeDiags(ui packer.Ui, files map[string]*hcl.File, diags hcl.Diagnostics) int {
-		builds, diags := cfg.GetBuilds(opts.only, opts.except)
+	// write HCL errors/diagnostics if any.
-		{
+	b := bytes.NewBuffer(nil)
-			// write HCL errors/diagnostics if any.
+	err := hcl.NewDiagnosticTextWriter(b, files, 80, false).WriteDiagnostics(diags)
-			b := bytes.NewBuffer(nil)
+	if err != nil {
-			err := hcl.NewDiagnosticTextWriter(b, parser.Files(), 80, false).WriteDiagnostics(diags)
+		ui.Error("could not write diagnostic: " + err.Error())
-			if err != nil {
+		return 1
-				m.Ui.Error("could not write diagnostic: " + err.Error())
+	}
-				return nil, 1
+	if b.Len() != 0 {
-			}
-			if b.Len() != 0 {
-				m.Ui.Message(b.String())
-			}
-		}
 		if diags.HasErrors() {
-			ret = 1
+			ui.Error(b.String())
+			return 1
 		}
-
+		ui.Say(b.String())
-		return builds, ret
+	}
-	}, ret
+	return 0
 }
 
-// GetBuilds will start all packer plugins ( builder, provisioner and
+func (m *Meta) GetConfig(path ...string) (packer.BuildGetter, int) {
-// post-processor ) referenced in the config. These plugins will be in a
+	cfgType, err := ConfigType(path...)
-// waiting to execute mode. Upon error a non nil error will be returned.
+	if err != nil {
-type BuildStarter func(buildStarterOptions) ([]packer.Build, int)
+		m.Ui.Error(fmt.Sprintf("could not tell config type: %s", err))
-
-type buildStarterOptions struct {
-	except, only []string
-}
-
-func (m *Meta) GetConfig(path string) (BuildStarter, int) {
-	isHCLLoaded, err := isHCLLoaded(path)
-	if path != "-" && err != nil {
-		m.Ui.Error(fmt.Sprintf("could not tell whether %s is hcl enabled: %s", path, err))
 		return nil, 1
 	}
-	if isHCLLoaded {
-		return m.GetConfigFromHCL(path)
-	}
 
-	// TODO: uncomment once we've polished HCL a bit more.
+	switch cfgType {
-	// c.Ui.Say(`Legacy JSON Configuration Will Be Used.
+	case "hcl":
-	// The template will be parsed in the legacy configuration style. This style
+		// TODO(azr): allow to pass a slice of files here.
-	// will continue to work but users are encouraged to move to the new style.
+		return m.GetConfigFromHCL(path[0])
-	// See: https://packer.io/guides/hcl
+	default:
-	// `)
+		// TODO: uncomment once we've polished HCL a bit more.
-	return m.GetConfigFromJSON(path)
+		// c.Ui.Say(`Legacy JSON Configuration Will Be Used.
+		// The template will be parsed in the legacy configuration style. This style
+		// will continue to work but users are encouraged to move to the new style.
+		// See: https://packer.io/guides/hcl
+		// `)
+		return m.GetConfigFromJSON(path[0])
+	}
 }
 
-func (m *Meta) GetConfigFromJSON(path string) (BuildStarter, int) {
+func (m *Meta) GetConfigFromJSON(path string) (packer.BuildGetter, int) {
 	// Parse the template
 	tpl, err := template.ParseFile(path)
 	if err != nil {
@@ -169,38 +125,24 @@ func (m *Meta) GetConfigFromJSON(path string) (BuildStarter, int) {
 		m.Ui.Error(err.Error())
 		ret = 1
 	}
-	return func(opts buildStarterOptions) ([]packer.Build, int) {
+	return core, ret
-		ret := 0
-		buildNames := core.BuildNames(opts.only, opts.except)
-		builds := make([]packer.Build, 0, len(buildNames))
-		for _, n := range buildNames {
-			b, err := core.Build(n)
-			if err != nil {
-				m.Ui.Error(fmt.Sprintf(
-					"Failed to initialize build '%s': %s",
-					n, err))
-				ret = 1
-				continue
-			}
-
-			builds = append(builds, b)
-		}
-		return builds, ret
-	}, ret
 }
 
-func (c *BuildCommand) RunContext(buildCtx context.Context, cfg Config) int {
+func (c *BuildCommand) RunContext(buildCtx context.Context, cfg *BuildArgs) int {
-
 	packerStarter, ret := c.GetConfig(cfg.Path)
 	if ret != 0 {
 		return ret
 	}
 
-	builds, ret := packerStarter(buildStarterOptions{
+	builds, diags := packerStarter.GetBuilds(packer.GetBuildsOptions{
-		except: c.CoreConfig.Except,
+		Only:   cfg.Only,
-		only:   c.CoreConfig.Only,
+		Except: cfg.Except,
 	})
 
+	if ret := writeDiags(c.Ui, nil, diags); ret != 0 {
+		return ret
+	}
+
 	if cfg.Debug {
 		c.Ui.Say("Debug mode enabled. Builds will not be parallelized.")
 	}
@@ -230,7 +172,7 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cfg Config) int {
 			}
 		}
 		// Now add timestamps if requested
-		if cfg.Timestamp {
+		if cfg.TimestampUi {
 			ui = &packer.TimestampedUi{
 				Ui: ui,
 			}

command/build_test.go

@@ -630,13 +630,13 @@ func TestBuildCommand_ParseArgs(t *testing.T) {
 	tests := []struct {
 		fields       fields
 		args         args
-		wantCfg      Config
+		wantCfg      BuildArgs
 		wantExitCode int
 	}{
 		{fields{defaultMeta},
 			args{[]string{"file.json"}},
-			Config{
+			BuildArgs{
-				Path:           "file.json",
+				MetaArgs:       MetaArgs{Path: "file.json"},
 				ParallelBuilds: math.MaxInt64,
 				Color:          true,
 			},
@@ -644,8 +644,8 @@ func TestBuildCommand_ParseArgs(t *testing.T) {
 		},
 		{fields{defaultMeta},
 			args{[]string{"-parallel=true", "file.json"}},
-			Config{
+			BuildArgs{
-				Path:           "file.json",
+				MetaArgs:       MetaArgs{Path: "file.json"},
 				ParallelBuilds: math.MaxInt64,
 				Color:          true,
 			},
@@ -653,8 +653,8 @@ func TestBuildCommand_ParseArgs(t *testing.T) {
 		},
 		{fields{defaultMeta},
 			args{[]string{"-parallel=false", "file.json"}},
-			Config{
+			BuildArgs{
-				Path:           "file.json",
+				MetaArgs:       MetaArgs{Path: "file.json"},
 				ParallelBuilds: 1,
 				Color:          true,
 			},
@@ -662,8 +662,8 @@ func TestBuildCommand_ParseArgs(t *testing.T) {
 		},
 		{fields{defaultMeta},
 			args{[]string{"-parallel-builds=5", "file.json"}},
-			Config{
+			BuildArgs{
-				Path:           "file.json",
+				MetaArgs:       MetaArgs{Path: "file.json"},
 				ParallelBuilds: 5,
 				Color:          true,
 			},
@@ -671,8 +671,8 @@ func TestBuildCommand_ParseArgs(t *testing.T) {
 		},
 		{fields{defaultMeta},
 			args{[]string{"-parallel=false", "-parallel-builds=5", "otherfile.json"}},
-			Config{
+			BuildArgs{
-				Path:           "otherfile.json",
+				MetaArgs:       MetaArgs{Path: "otherfile.json"},
 				ParallelBuilds: 5,
 				Color:          true,
 			},

command/cli.go

@@ -3,7 +3,6 @@ package command
 import (
 	"flag"
 	"fmt"
-	"math"
 	"strings"
 
 	"github.com/hashicorp/packer/helper/enumflag"
@@ -12,25 +11,20 @@ import (
 	"github.com/hashicorp/packer/packer"
 )
 
-// NewMetaArgs parses cli args and put possible values
-func (ma *MetaArgs) AddFlagSets(fs *flag.FlagSet) {
-	fs.Var((*sliceflag.StringFlag)(&ma.Only), "only", "")
-	fs.Var((*sliceflag.StringFlag)(&ma.Except), "except", "")
-	fs.Var((*kvflag.Flag)(&ma.Vars), "var", "")
-	fs.Var((*kvflag.StringSlice)(&ma.VarFiles), "var-file", "")
-}
-
 // ConfigType tells what type of config we should use, it can return values
 // like "hcl" or "json".
 // Make sure Args was correctly set before.
-func (ma *MetaArgs) ConfigType() (string, error) {
+func ConfigType(args ...string) (string, error) {
-	switch len(ma.Args) {
+	switch len(args) {
 	// TODO(azr): in the future, I want to allow passing multiple arguments to
 	// merge HCL confs together; but this will probably need an RFC first.
-	// TODO(azr): To allow piping HCL2 confs (when args is "-"), we probably
-	// will need to add a setting that says "this is an HCL config".
 	case 1:
-		name := ma.Args[0]
+		name := args[0]
+		if name == "-" {
+			// TODO(azr): To allow piping HCL2 confs (when args is "-"), we probably
+			// will need to add a setting that says "this is an HCL config".
+			return "json", nil
+		}
 		if strings.HasSuffix(name, ".pkr.hcl") ||
 			strings.HasSuffix(name, ".pkr.json") {
 			return "hcl", nil
@@ -41,13 +35,21 @@ func (ma *MetaArgs) ConfigType() (string, error) {
 		}
 		return "json", err
 	default:
-		return "", fmt.Errorf("packer only takes on argument: %q", ma.Args)
+		return "", fmt.Errorf("packer only takes one argument: %q", args)
 	}
 }
 
+// NewMetaArgs parses cli args and put possible values
+func (ma *MetaArgs) AddFlagSets(fs *flag.FlagSet) {
+	fs.Var((*sliceflag.StringFlag)(&ma.Only), "only", "")
+	fs.Var((*sliceflag.StringFlag)(&ma.Except), "except", "")
+	fs.Var((*kvflag.Flag)(&ma.Vars), "var", "")
+	fs.Var((*kvflag.StringSlice)(&ma.VarFiles), "var-file", "")
+}
+
 // MetaArgs defines commonalities between all comands
 type MetaArgs struct {
-	Args         []string
+	Path         string
 	Only, Except []string
 	Vars         map[string]string
 	VarFiles     []string
@@ -69,23 +71,6 @@ func (ba *BuildArgs) AddFlagSets(flags *flag.FlagSet) {
 	ba.MetaArgs.AddFlagSets(flags)
 }
 
-func (ba *BuildArgs) ParseArgvs(args []string) int {
-	flags := flag.NewFlagSet("build", flag.ContinueOnError)
-	// flags.Usage = func() { ba.Ui.Say(ba.Help()) }
-	ba.AddFlagSets(flags)
-	err := flags.Parse(args)
-	if err != nil {
-		return 1
-	}
-
-	if ba.ParallelBuilds < 1 {
-		ba.ParallelBuilds = math.MaxInt64
-	}
-
-	ba.Args = flags.Args()
-	return 0
-}
-
 // BuildArgs represents a parsed cli line for a `packer build`
 type BuildArgs struct {
 	MetaArgs
@@ -94,19 +79,6 @@ type BuildArgs struct {
 	OnError                                           string
 }
 
-func (ca *ConsoleArgs) ParseArgvs(args []string) int {
-	flags := flag.NewFlagSet("console", flag.ContinueOnError)
-	// flags.Usage = func() { ca.Ui.Say(ca.Help()) }
-	ca.AddFlagSets(flags)
-	err := flags.Parse(args)
-	if err != nil {
-		return 1
-	}
-
-	ca.Args = flags.Args()
-	return 0
-}
-
 // ConsoleArgs represents a parsed cli line for a `packer console`
 type ConsoleArgs struct{ MetaArgs }
 
@@ -116,19 +88,6 @@ func (fa *FixArgs) AddFlagSets(flags *flag.FlagSet) {
 	fa.MetaArgs.AddFlagSets(flags)
 }
 
-func (fa *FixArgs) ParseArgvs(args []string) int {
-	flags := flag.NewFlagSet("fix", flag.ContinueOnError)
-	// flags.Usage = func() { fa.Ui.Say(fa.Help()) }
-	fa.AddFlagSets(flags)
-	err := flags.Parse(args)
-	if err != nil {
-		return 1
-	}
-
-	fa.Args = flags.Args()
-	return 0
-}
-
 // FixArgs represents a parsed cli line for a `packer fix`
 type FixArgs struct {
 	MetaArgs
@@ -141,19 +100,6 @@ func (va *ValidateArgs) AddFlagSets(flags *flag.FlagSet) {
 	va.MetaArgs.AddFlagSets(flags)
 }
 
-func (va *ValidateArgs) ParseArgvs(args []string) int {
-	flags := flag.NewFlagSet("validate", flag.ContinueOnError)
-	// flags.Usage = func() { va.Ui.Say(va.Help()) }
-	va.AddFlagSets(flags)
-	err := flags.Parse(args)
-	if err != nil {
-		return 1
-	}
-
-	va.Args = flags.Args()
-	return 0
-}
-
 // ValidateArgs represents a parsed cli line for a `packer validate`
 type ValidateArgs struct {
 	MetaArgs

command/command.go

@@ -1,13 +1 @@
 package command
-
-import "context"
-
-// PackerInterface is the interface to use packer; it represents ways users can
-// use Packer. A call returns a int that will be the exit code of Packer,
-// everything else is up to the implementer.
-type PackerInterface interface {
-	Build(ctx context.Context, args *BuildArgs) int
-	Console(ctx context.Context, args *ConsoleArgs) int
-	Fix(ctx context.Context, args *FixArgs) int
-	Validate(ctx context.Context, args *ValidateArgs) int
-}

hcl2template/types.packer_config.go

@@ -12,6 +12,8 @@ import (
 // PackerConfig represents a loaded Packer HCL config. It will contain
 // references to all possible blocks of the allowed configuration.
 type PackerConfig struct {
+	parser *Parser
+
 	// Directory where the config files are defined
 	Basedir string
 
@@ -254,7 +256,7 @@ func (cfg *PackerConfig) getCoreBuildPostProcessors(source *SourceBlock, blocks
 // GetBuilds returns a list of packer Build based on the HCL2 parsed build
 // blocks. All Builders, Provisioners and Post Processors will be started and
 // configured.
-func (cfg *PackerConfig) GetBuilds(only, except []string) ([]packer.Build, hcl.Diagnostics) {
+func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packer.Build, hcl.Diagnostics) {
 	res := []packer.Build{}
 	var diags hcl.Diagnostics
 
@@ -274,8 +276,8 @@ func (cfg *PackerConfig) GetBuilds(only, except []string) ([]packer.Build, hcl.D
 			buildName := fmt.Sprintf("%s.%s", src.Type, src.Name)
 
 			// -only
-			if len(only) > 0 {
+			if len(opts.Only) > 0 {
-				onlyGlobs, diags := convertFilterOption(only, "only")
+				onlyGlobs, diags := convertFilterOption(opts.Only, "only")
 				if diags.HasErrors() {
 					return nil, diags
 				}
@@ -292,8 +294,8 @@ func (cfg *PackerConfig) GetBuilds(only, except []string) ([]packer.Build, hcl.D
 			}
 
 			// -except
-			if len(except) > 0 {
+			if len(opts.Except) > 0 {
-				exceptGlobs, diags := convertFilterOption(except, "except")
+				exceptGlobs, diags := convertFilterOption(opts.Except, "except")
 				if diags.HasErrors() {
 					return nil, diags
 				}

packer/core.go

@@ -11,6 +11,7 @@ import (
 
 	multierror "github.com/hashicorp/go-multierror"
 	version "github.com/hashicorp/go-version"
+	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/packer/template"
 	"github.com/hashicorp/packer/template/interpolate"
 )
@@ -198,6 +199,25 @@ func (c *Core) generateCoreBuildProvisioner(rawP *template.Provisioner, rawName
 	return cbp, nil
 }
 
+func (c *Core) GetBuilds(opts GetBuildsOptions) ([]Build, hcl.Diagnostics) {
+	buildNames := c.BuildNames(opts.Only, opts.Except)
+	builds := []Build{}
+	diags := hcl.Diagnostics{}
+	for _, n := range buildNames {
+		b, err := c.Build(n)
+		if err != nil {
+			diags = append(diags, &hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  fmt.Sprintf("Failed to initialize build %q", n),
+				Detail:   err.Error(),
+			})
+			continue
+		}
+		builds = append(builds, b)
+	}
+	return builds, diags
+}
+
 // Build returns the Build object for the given name.
 func (c *Core) Build(n string) (Build, error) {
 	// Setup the builder

packer/new_stuff.go

@@ -0,0 +1,33 @@
+package packer
+
+import "github.com/hashicorp/hcl/v2"
+
+type GetBuildsOptions struct {
+	// Get builds except the ones that match with except and with only the ones
+	// that match with. When those are empty everything matches.
+	Except, Only []string
+}
+
+type BuildGetter interface {
+	// GetBuilds return all possible builds for a config. It also starts them.
+	// TODO(azr): rename to builder starter ?
+	GetBuilds(GetBuildsOptions) ([]Build, hcl.Diagnostics)
+}
+
+//go:generate enumer -type FixMode
+type FixConfigMode int
+
+const (
+	Stdout  FixConfigMode = 0
+	Inplace FixConfigMode = 1
+	Diff    FixConfigMode = 2
+)
+
+type FixConfigOptions struct {
+	DiffOnly bool
+}
+
+type OtherInterfaceyMacOtherInterfaceFace interface {
+	// FixConfig will output the config in a fixed manner.
+	FixConfig(FixConfigOptions) hcl.Diagnostics
+}