diff --git a/builder/virtualbox/common/output_config.go b/builder/virtualbox/common/output_config.go index f3427183c..7b5ddcd45 100644 --- a/builder/virtualbox/common/output_config.go +++ b/builder/virtualbox/common/output_config.go @@ -2,7 +2,6 @@ package common import ( "fmt" - "os" "github.com/mitchellh/packer/common" "github.com/mitchellh/packer/template/interpolate" @@ -17,13 +16,5 @@ func (c *OutputConfig) Prepare(ctx *interpolate.Context, pc *common.PackerConfig c.OutputDir = fmt.Sprintf("output-%s", pc.PackerBuildName) } - var errs []error - if !pc.PackerForce { - if _, err := os.Stat(c.OutputDir); err == nil { - errs = append(errs, fmt.Errorf( - "Output directory '%s' already exists. It must not exist.", c.OutputDir)) - } - } - - return errs + return nil } diff --git a/builder/virtualbox/common/output_config_test.go b/builder/virtualbox/common/output_config_test.go index 7fa039a16..a4d8e7999 100644 --- a/builder/virtualbox/common/output_config_test.go +++ b/builder/virtualbox/common/output_config_test.go @@ -39,27 +39,7 @@ func TestOutputConfigPrepare_exists(t *testing.T) { PackerForce: false, } errs := c.Prepare(testConfigTemplate(t), pc) - if len(errs) == 0 { - t.Fatal("should have errors") - } -} - -func TestOutputConfigPrepare_forceExists(t *testing.T) { - td, err := ioutil.TempDir("", "packer") - if err != nil { - t.Fatalf("err: %s", err) - } - defer os.RemoveAll(td) - - c := new(OutputConfig) - c.OutputDir = td - - pc := &common.PackerConfig{ - PackerBuildName: "foo", - PackerForce: true, - } - errs := c.Prepare(testConfigTemplate(t), pc) - if len(errs) > 0 { + if len(errs) != 0 { t.Fatal("should not have errors") } } diff --git a/builder/virtualbox/common/step_output_dir.go b/builder/virtualbox/common/step_output_dir.go index 209bbabe2..e01928b7a 100644 --- a/builder/virtualbox/common/step_output_dir.go +++ b/builder/virtualbox/common/step_output_dir.go @@ -22,7 +22,16 @@ type StepOutputDir struct { func (s *StepOutputDir) Run(state multistep.StateBag) multistep.StepAction { ui := state.Get("ui").(packer.Ui) - if _, err := os.Stat(s.Path); err == nil && s.Force { + if _, err := os.Stat(s.Path); err == nil { + if !s.Force { + err := fmt.Errorf( + "Output directory exists: %s\n\n"+ + "Use the force flag to delete it prior to building.", + s.Path) + state.Put("error", err) + return multistep.ActionHalt + } + ui.Say("Deleting previous output directory...") os.RemoveAll(s.Path) } diff --git a/builder/virtualbox/common/step_output_dir_test.go b/builder/virtualbox/common/step_output_dir_test.go index be485c278..77d1f855f 100644 --- a/builder/virtualbox/common/step_output_dir_test.go +++ b/builder/virtualbox/common/step_output_dir_test.go @@ -45,6 +45,30 @@ func TestStepOutputDir(t *testing.T) { } } +func TestStepOutputDir_exists(t *testing.T) { + state := testState(t) + step := testStepOutputDir(t) + + // Make the dir + if err := os.MkdirAll(step.Path, 0755); err != nil { + t.Fatalf("bad: %s", err) + } + + // Test the run + if action := step.Run(state); action != multistep.ActionHalt { + t.Fatalf("bad action: %#v", action) + } + if _, ok := state.GetOk("error"); !ok { + t.Fatal("should have error") + } + + // Test the cleanup + step.Cleanup(state) + if _, err := os.Stat(step.Path); err != nil { + t.Fatalf("err: %s", err) + } +} + func TestStepOutputDir_cancelled(t *testing.T) { state := testState(t) step := testStepOutputDir(t) diff --git a/builder/vmware/common/output_config.go b/builder/vmware/common/output_config.go index f3427183c..7b5ddcd45 100644 --- a/builder/vmware/common/output_config.go +++ b/builder/vmware/common/output_config.go @@ -2,7 +2,6 @@ package common import ( "fmt" - "os" "github.com/mitchellh/packer/common" "github.com/mitchellh/packer/template/interpolate" @@ -17,13 +16,5 @@ func (c *OutputConfig) Prepare(ctx *interpolate.Context, pc *common.PackerConfig c.OutputDir = fmt.Sprintf("output-%s", pc.PackerBuildName) } - var errs []error - if !pc.PackerForce { - if _, err := os.Stat(c.OutputDir); err == nil { - errs = append(errs, fmt.Errorf( - "Output directory '%s' already exists. It must not exist.", c.OutputDir)) - } - } - - return errs + return nil } diff --git a/builder/vmware/common/output_config_test.go b/builder/vmware/common/output_config_test.go index 7fa039a16..7de378d89 100644 --- a/builder/vmware/common/output_config_test.go +++ b/builder/vmware/common/output_config_test.go @@ -1,10 +1,9 @@ package common import ( - "github.com/mitchellh/packer/common" - "io/ioutil" - "os" "testing" + + "github.com/mitchellh/packer/common" ) func TestOutputConfigPrepare(t *testing.T) { @@ -23,43 +22,3 @@ func TestOutputConfigPrepare(t *testing.T) { t.Fatal("should have output dir") } } - -func TestOutputConfigPrepare_exists(t *testing.T) { - td, err := ioutil.TempDir("", "packer") - if err != nil { - t.Fatalf("err: %s", err) - } - defer os.RemoveAll(td) - - c := new(OutputConfig) - c.OutputDir = td - - pc := &common.PackerConfig{ - PackerBuildName: "foo", - PackerForce: false, - } - errs := c.Prepare(testConfigTemplate(t), pc) - if len(errs) == 0 { - t.Fatal("should have errors") - } -} - -func TestOutputConfigPrepare_forceExists(t *testing.T) { - td, err := ioutil.TempDir("", "packer") - if err != nil { - t.Fatalf("err: %s", err) - } - defer os.RemoveAll(td) - - c := new(OutputConfig) - c.OutputDir = td - - pc := &common.PackerConfig{ - PackerBuildName: "foo", - PackerForce: true, - } - errs := c.Prepare(testConfigTemplate(t), pc) - if len(errs) > 0 { - t.Fatal("should not have errors") - } -} diff --git a/builder/vmware/iso/builder_test.go b/builder/vmware/iso/builder_test.go index 60c6aeca0..13a9622f7 100644 --- a/builder/vmware/iso/builder_test.go +++ b/builder/vmware/iso/builder_test.go @@ -349,8 +349,8 @@ func TestBuilderPrepare_OutputDir(t *testing.T) { if len(warns) > 0 { t.Fatalf("bad: %#v", warns) } - if err == nil { - t.Fatal("should have error") + if err != nil { + t.Fatalf("err: %s", err) } // Test with a good one