The original approach of running version, but that seems to bypass wrappedMain entirely

errors

While working on this change it was found that prefixing an error
message with the ErrorPrefix string would trigger a copyOutput function
that would copy any outputted string to Stderr, until a new ErrorPrefix
or Outprefix string is encountered in the output. During background runs of
Packer an error message with the ErrorPrefix was being outputted which
was causing all output, including Stdout, to be written to Stderr.

This change updates the logic to only override the Stdout logging
for non-recoverable errors. The idea being that any non-recoverable
error should bypass panicwrap so that user know an error occurred.
All other errors should follow the same behavior that we had prior to
Packer v1.7.1.

Closes #10855

CHANGELOG.md

@@ -1,3 +1,5 @@
+## 1.7.2 (Upcoming)
+
 ## 1.7.1 (March 31, 2021)
 
 ### NOTES:

command/exec_test.go

@@ -95,6 +95,8 @@ func TestHelperProcess(*testing.T) {
 		os.Exit((&BuildCommand{Meta: commandMeta()}).Run(args))
 	case "hcl2_upgrade":
 		os.Exit((&HCL2UpgradeCommand{Meta: commandMeta()}).Run(args))
+	case "version":
+		os.Exit((&VersionCommand{Meta: commandMeta()}).Run(args))
 	default:
 		fmt.Fprintf(os.Stderr, "Unknown command %q\n", cmd)
 		os.Exit(2)

command/version_test.go

@@ -1,11 +1,37 @@
 package command
 
 import (
+	"fmt"
 	"testing"
 
+	"github.com/hashicorp/packer/version"
 	"github.com/mitchellh/cli"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestVersionCommand_implements(t *testing.T) {
 	var _ cli.Command = &VersionCommand{}
 }
+
+func Test_version(t *testing.T) {
+	tc := []struct {
+		command  []string
+		env      []string
+		expected string
+	}{
+		{[]string{"version"}, nil, fmt.Sprintf("Packer v%s", version.FormattedVersion()) + "\n"},
+		{[]string{"version", "&"}, nil, fmt.Sprintf("Packer v%s", version.FormattedVersion()) + "\n"},
+	}
+
+	for _, tc := range tc {
+		t.Run(fmt.Sprintf("packer %s", tc.command), func(t *testing.T) {
+			p := helperCommand(t, tc.command...)
+			bs, err := p.Output()
+			fmt.Println(err)
+			if err != nil {
+				t.Fatalf("%v: %s", err, bs)
+			}
+			assert.Equal(t, tc.expected, string(bs))
+		})
+	}
+}

main.go

@@ -152,6 +152,10 @@ func wrappedMain() int {
 	// passed into commands like `packer build`
 	config, err := loadConfig()
 	if err != nil {
+		// Writing to Stdout here so that the error message bypasses panicwrap. By using the
+		// ErrorPrefix this output will be redirected to Stderr by the copyOutput func.
+		// TODO: nywilken need to revisit this setup to better output errors to Stderr, and output to Stdout
+		// without panicwrap
 		fmt.Fprintf(os.Stdout, "%s Error loading configuration: \n\n%s\n", ErrorPrefix, err)
 		return 1
 	}
@@ -166,6 +170,10 @@ func wrappedMain() int {
 
 	cacheDir, err := packersdk.CachePath()
 	if err != nil {
+		// Writing to Stdout here so that the error message bypasses panicwrap. By using the
+		// ErrorPrefix this output will be redirected to Stderr by the copyOutput func.
+		// TODO: nywilken need to revisit this setup to better output errors to Stderr, and output to Stdout
+		// without panicwrap
 		fmt.Fprintf(os.Stdout, "%s Error preparing cache directory: \n\n%s\n", ErrorPrefix, err)
 		return 1
 	}
@@ -187,7 +195,8 @@ func wrappedMain() int {
 		// Set this so that we don't get colored output in our machine-
 		// readable UI.
 		if err := os.Setenv("PACKER_NO_COLOR", "1"); err != nil {
-			fmt.Fprintf(os.Stdout, "%s Packer failed to initialize UI: %s\n", ErrorPrefix, err)
+			// Outputting error using Ui here to conform to the machine readable format.
+			ui.Error(fmt.Sprintf("Packer failed to initialize UI: %s\n", err))
 			return 1
 		}
 	} else {
@@ -202,13 +211,16 @@ func wrappedMain() int {
 			currentPID := os.Getpid()
 			backgrounded, err := checkProcess(currentPID)
 			if err != nil {
-				fmt.Fprintf(os.Stdout, "%s cannot determine if process is in "+
-					"background: %s\n", ErrorPrefix, err)
+				// Writing to Stderr will ensure that the output gets captured by panicwrap.
+				// This error message and any other message writing to Stderr after this point will only show up with PACKER_LOG=1
+				// TODO: nywilken need to revisit this setup to better output errors to Stderr, and output to Stdout without panicwrap.
+				fmt.Fprintf(os.Stderr, "%s cannot determine if process is in background: %s\n", ErrorPrefix, err)
 			}
+
 			if backgrounded {
-				fmt.Fprintf(os.Stdout, "%s Running in background, not using a TTY\n", ErrorPrefix)
+				fmt.Fprintf(os.Stderr, "%s Running in background, not using a TTY\n", ErrorPrefix)
 			} else if TTY, err := openTTY(); err != nil {
-				fmt.Fprintf(os.Stdout, "%s No tty available: %s\n", ErrorPrefix, err)
+				fmt.Fprintf(os.Stderr, "%s No tty available: %s\n", ErrorPrefix, err)
 			} else {
 				basicUi.TTY = TTY
 				basicUi.PB = &packer.UiProgressBar{}
@@ -246,6 +258,10 @@ func wrappedMain() int {
 	}
 
 	if err != nil {
+		// Writing to Stdout here so that the error message bypasses panicwrap. By using the
+		// ErrorPrefix this output will be redirected to Stderr by the copyOutput func.
+		// TODO: nywilken need to revisit this setup to better output errors to Stderr, and output to Stdout
+		// without panicwrap
 		fmt.Fprintf(os.Stdout, "%s Error executing CLI: %s\n", ErrorPrefix, err)
 		return 1
 	}
@@ -454,3 +470,5 @@ func init() {
 	// Seed the random number generator
 	rand.Seed(time.Now().UTC().UnixNano())
 }
+
+var backgroundCheckFn func(int) (bool, error)

main_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"math/rand"
+	"os"
 	"reflect"
 	"strings"
 	"testing"
@@ -67,3 +68,11 @@ func TestRandom(t *testing.T) {
 		t.Fatal("math.rand is not seeded properly")
 	}
 }
+
+func ExampleWrappedMain() {
+	os.Setenv("PACKER_WRAP_COOKIE", "49C22B1A-3A93-4C98-97FA-E07D18C787B5")
+	backgroundCheckFn = func(_ int) (bool, error) { return true, nil }
+	os.Args = []string{"packer", "version"}
+	wrappedMain()
+	//Output: Packer v1.7.2-dev
+}

version/version.go

@@ -9,12 +9,12 @@ import (
 var GitCommit string
 
 // The main version number that is being run at the moment.
-const Version = "1.7.1"
+const Version = "1.7.2"
 
 // A pre-release marker for the version. If this is "" (empty string)
 // then it means that it is a final release. Otherwise, this is a pre-release
 // such as "dev" (in development), "beta", "rc1", etc.
-const VersionPrerelease = ""
+const VersionPrerelease = "dev"
 
 var PackerVersion *pluginVersion.PluginVersion