cmd/cue: fail with non-zero exit status

Previously, if a command failed cue would still return a 0 exit status.

This also now passes through stderr and stdout to the system
by default.

Made the error stubbing less hacky.

Fixes #30

Change-Id: Ib6dc3733b557bfe817789e14631fd9da09d7b031
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/1844
Reviewed-by: Marcel van Lohuizen <mpvl@google.com>

cmd/cue/cmd/cmd_test.go

@@ -15,6 +15,7 @@
 package cmd
 
 import (
+	"os"
 	"testing"
 
 	"cuelang.org/go/cue/errors"
@@ -27,12 +28,17 @@ func TestCmd(t *testing.T) {
 		"run",
 		"run_list",
 		"baddisplay",
+		"errcode",
 		"http",
 	}
+	defer func() {
+		stdout = os.Stdout
+		stderr = os.Stderr
+	}()
 	for _, name := range testCases {
 		run := func(cmd *cobra.Command, args []string) error {
-			testOut = cmd.OutOrStdout()
-			defer func() { testOut = nil }()
+			stdout = cmd.OutOrStdout()
+			stderr = cmd.OutOrStderr()
 
 			tools := buildTools(rootCmd, args)
 			cmd, err := addCustom(rootCmd, "command", name, tools)
@@ -41,7 +47,7 @@ func TestCmd(t *testing.T) {
 			}
 			err = executeTasks("command", name, tools)
 			if err != nil {
-				errors.Print(testOut, err)
+				errors.Print(stdout, err)
 			}
 			return nil
 		}

cmd/cue/cmd/custom.go

@@ -33,6 +33,7 @@ import (
 	"cuelang.org/go/cue"
 	"github.com/spf13/cobra"
 	"golang.org/x/sync/errgroup"
+	"golang.org/x/xerrors"
 )
 
 const (
@@ -48,6 +49,12 @@ func lookupString(obj cue.Value, key string) string {
 	return str
 }
 
+// Variables used for testing.
+var (
+	stdout io.Writer = os.Stdout
+	stderr io.Writer = os.Stderr
+)
+
 func addCustom(parent *cobra.Command, typ, name string, tools *cue.Instance) (*cobra.Command, error) {
 	if tools == nil {
 		return nil, errors.New("no commands defined")
@@ -68,6 +75,7 @@ func addCustom(parent *cobra.Command, typ, name string, tools *cue.Instance) (*c
 		Short: lookupString(o, "short"),
 		Long:  lookupString(o, "long"),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			// TODO:
 			// - parse flags and env vars
 			// - constrain current config with config section
 
@@ -111,10 +119,9 @@ func (k *taskKey) lookupTasks(root *cue.Instance) cue.Value {
 }
 
 func doTasks(cmd *cobra.Command, typ, command string, root *cue.Instance) error {
-	if err := executeTasks(typ, command, root); err != nil {
-		exitIfErr(cmd, root, err, true)
-	}
-	return nil
+	err := executeTasks(typ, command, root)
+	exitIfErr(cmd, root, err, true)
+	return err
 }
 
 // executeTasks runs user-defined tasks as part of a user-defined command.
@@ -293,19 +300,12 @@ func newPrintCmd(v cue.Value) (Runner, error) {
 	return &printCmd{}, nil
 }
 
-// TODO: get rid of this hack
-var testOut io.Writer
-
 func (c *printCmd) Run(ctx context.Context, v cue.Value) (res interface{}, err error) {
 	str, err := v.Lookup("text").String()
 	if err != nil {
 		return nil, err
 	}
-	if testOut != nil {
-		fmt.Fprintln(testOut, str)
-	} else {
-		fmt.Println(str)
-	}
+	fmt.Fprintln(stdout, str)
 	return nil, nil
 }
 
@@ -319,12 +319,14 @@ func (c *execCmd) Run(ctx context.Context, v cue.Value) (res interface{}, err er
 	// TODO: set environment variables, if defined.
 	var bin string
 	var args []string
+	doc := ""
 	switch v := v.Lookup("cmd"); v.Kind() {
 	case cue.StringKind:
 		str, _ := v.String()
 		if str == "" {
 			return cue.Value{}, errors.New("empty command")
 		}
+		doc = str
 		list := strings.Fields(str)
 		bin = list[0]
 		for _, s := range list[1:] {
@@ -340,12 +342,14 @@ func (c *execCmd) Run(ctx context.Context, v cue.Value) (res interface{}, err er
 		if err != nil {
 			return cue.Value{}, err
 		}
+		doc += bin
 		for list.Next() {
 			str, err := list.Value().String()
 			if err != nil {
 				return cue.Value{}, err
 			}
 			args = append(args, str)
+			doc += " " + str
 		}
 	}
 
@@ -356,35 +360,33 @@ func (c *execCmd) Run(ctx context.Context, v cue.Value) (res interface{}, err er
 			return nil, fmt.Errorf("cue: %v", err)
 		}
 	}
-	captureOut := !v.Lookup("stdout").IsNull()
+	captureOut := v.Lookup("stdout").Exists()
 	if !captureOut {
-		cmd.Stdout = os.Stdout
+		cmd.Stdout = stdout
 	}
-	captureErr := !v.Lookup("stderr").IsNull()
-	if captureErr {
-		cmd.Stderr = os.Stderr
+	captureErr := v.Lookup("stderr").Exists()
+	if !captureErr {
+		cmd.Stderr = stderr
 	}
 
 	update := map[string]interface{}{}
-	var stdout, stderr []byte
 	if captureOut {
+		var stdout []byte
 		stdout, err = cmd.Output()
 		update["stdout"] = string(stdout)
 	} else {
 		err = cmd.Run()
 	}
 	update["success"] = err == nil
 	if err != nil {
-		if exit, ok := err.(*exec.ExitError); ok && captureErr {
-			stderr = exit.Stderr
+		if exit := (*exec.ExitError)(nil); xerrors.As(err, &exit) && captureErr {
+			update["stderr"] = string(exit.Stderr)
 		} else {
-			return nil, fmt.Errorf("cue: %v", err)
+			update = nil
 		}
+		err = fmt.Errorf("command %q failed: %v", doc, err)
 	}
-	if captureErr {
-		update["stderr"] = string(stderr)
-	}
-	return update, nil
+	return update, err
 }
 
 type httpCmd struct{}

cmd/cue/cmd/testdata/tasks/cmd_baddisplay.out

@@ -1,3 +1,3 @@
 not of right kind (number vs string):
-    $CWD/testdata/tasks/task_tool.cue:23:9
+    $CWD/testdata/tasks/task_tool.cue:29:9
 

cmd/cue/cmd/testdata/tasks/cmd_errcode.out

@@ -0,0 +1 @@
+command "ls --badflags" failed: exit status 1

cmd/cue/cmd/testdata/tasks/task_tool.cue

@@ -8,12 +8,18 @@ command run_list: runBase & {
 	task echo cmd: ["echo", message]
 }
 
+command errcode: {
+	task bad: {
+		kind:   "exec"
+		cmd:    "ls --badflags"
+		stderr: string // suppress error message
+	}}
+
 // TODO: capture stdout and stderr for tests.
 command runRedirect: {
 	task echo: {
-		kind:   "exec"
-		cmd:    "echo \(message)"
-		stdout: null // should be automatic
+		kind: "exec"
+		cmd:  "echo \(message)"
 	}
 }
 

go.mod

@@ -14,4 +14,5 @@ require (
 	golang.org/x/exp/errors v0.0.0-20181221233300-b68661188fbf
 	golang.org/x/sync v0.0.0-20181108010431-42b317875d0f
 	golang.org/x/tools v0.0.0-20181210225255-6a3e9aa2ab77
+	golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373
 )

go.sum

@@ -59,6 +59,8 @@ golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/tools v0.0.0-20181210225255-6a3e9aa2ab77 h1:s+6psEFi3o1QryeA/qyvUoVaHMCQkYVvZ0i2ZolwSJc=
 golang.org/x/tools v0.0.0-20181210225255-6a3e9aa2ab77/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
+golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373 h1:PPwnA7z1Pjf7XYaBP9GL1VAMZmcIWyFz7QCMSIIa3Bg=
+golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=