cmd/cue: use stderr and non-zero exit for errors
Exit code is determined from returning an error or if
anything was printed to stderr.
Closes #101
Closes #100
Change-Id: I980798586d8a23d17a32c52d62da49376e7165ef
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/3102
Reviewed-by: Marcel van Lohuizen <mpvl@google.com>
diff --git a/cmd/cue/cmd/add.go b/cmd/cue/cmd/add.go
index a4b2b3e..4eba6e2 100644
--- a/cmd/cue/cmd/add.go
+++ b/cmd/cue/cmd/add.go
@@ -195,7 +195,7 @@
func restoreOriginals(cmd *Command, originals []originalFile) {
for _, fo := range originals {
if err := fo.restore(); err != nil {
- fmt.Fprintln(cmd.OutOrStderr(), "Error restoring file: ", err)
+ fmt.Fprintln(cmd.Stderr(), "Error restoring file: ", err)
}
}
}
diff --git a/cmd/cue/cmd/cmd.go b/cmd/cue/cmd/cmd.go
index 9e5fb0c..0f928ab 100644
--- a/cmd/cue/cmd/cmd.go
+++ b/cmd/cue/cmd/cmd.go
@@ -211,12 +211,13 @@
`,
RunE: mkRunE(c, func(cmd *Command, args []string) error {
+ w := cmd.Stderr()
if len(args) == 0 {
- fmt.Println("cmd must be run as one of its subcommands")
+ fmt.Fprintln(w, "cmd must be run as one of its subcommands")
} else {
- fmt.Printf("cmd must be run as one of its subcommands: unknown subcommand %q\n", args[0])
+ fmt.Fprintf(w, "cmd must be run as one of its subcommands: unknown subcommand %q\n", args[0])
}
- fmt.Println("Run 'cue help cmd' for known subcommands.")
+ fmt.Fprintln(w, "Run 'cue help cmd' for known subcommands.")
os.Exit(1) // TODO: get rid of this
return nil
}),
diff --git a/cmd/cue/cmd/cmd_test.go b/cmd/cue/cmd/cmd_test.go
index d09c41f..d69e005 100644
--- a/cmd/cue/cmd/cmd_test.go
+++ b/cmd/cue/cmd/cmd_test.go
@@ -41,7 +41,7 @@
c := newRootCmd()
run := func(cmd *cobra.Command, args []string) error {
stdout = cmd.OutOrStdout()
- stderr = cmd.OutOrStderr()
+ stderr = c.Stderr()
tools, _ := buildTools(c, args)
cmd, err := addCustom(c, c.root, "command", name, tools)
diff --git a/cmd/cue/cmd/common.go b/cmd/cue/cmd/common.go
index 2bf9f37..0678480 100644
--- a/cmd/cue/cmd/common.go
+++ b/cmd/cue/cmd/common.go
@@ -74,7 +74,7 @@
})
b := w.Bytes()
- _, _ = cmd.OutOrStderr().Write(b)
+ _, _ = cmd.Stderr().Write(b)
if fatal {
exit()
}
@@ -89,7 +89,6 @@
}
func loadFromArgs(cmd *Command, args []string) []*build.Instance {
- log.SetOutput(cmd.OutOrStderr())
binst := load.Instances(args, nil)
if len(binst) == 0 {
return nil
diff --git a/cmd/cue/cmd/common_test.go b/cmd/cue/cmd/common_test.go
index 58333d9..8348106 100644
--- a/cmd/cue/cmd/common_test.go
+++ b/cmd/cue/cmd/common_test.go
@@ -51,7 +51,6 @@
func runCommand(t *testing.T, cmd *cobra.Command, name string, args ...string) {
t.Helper()
- log.SetFlags(0)
const dir = "./testdata"
diff --git a/cmd/cue/cmd/get.go b/cmd/cue/cmd/get.go
index 499e62c..e562f78 100644
--- a/cmd/cue/cmd/get.go
+++ b/cmd/cue/cmd/get.go
@@ -36,12 +36,13 @@
per language and are documented in the respective subcommands.
`,
RunE: mkRunE(c, func(cmd *Command, args []string) error {
+ stderr := cmd.Stderr()
if len(args) == 0 {
- fmt.Println("get must be run as one of its subcommands")
+ fmt.Fprintln(stderr, "get must be run as one of its subcommands")
} else {
- fmt.Printf("get must be run as one of its subcommands: unknown subcommand %q\n", args[0])
+ fmt.Fprintf(stderr, "get must be run as one of its subcommands: unknown subcommand %q\n", args[0])
}
- fmt.Println("Run 'cue help get' for known subcommands.")
+ fmt.Fprintln(stderr, "Run 'cue help get' for known subcommands.")
os.Exit(1) // TODO: get rid of this
return nil
}),
diff --git a/cmd/cue/cmd/get_go.go b/cmd/cue/cmd/get_go.go
index 411cf71..bedc4e8 100644
--- a/cmd/cue/cmd/get_go.go
+++ b/cmd/cue/cmd/get_go.go
@@ -337,7 +337,7 @@
e := extractor{
cmd: cmd,
- stderr: cmd.OutOrStderr(),
+ stderr: cmd.Stderr(),
pkgs: pkgs,
orig: map[types.Type]*ast.StructType{},
}
@@ -474,8 +474,9 @@
b, err := format.Source(w.Bytes())
if err != nil {
_ = ioutil.WriteFile(filepath.Join(dir, file), w.Bytes(), 0644)
- fmt.Println(w.String())
- fmt.Println(dir, file)
+ stderr := e.cmd.Stderr()
+ fmt.Fprintln(stderr, w.String())
+ fmt.Fprintln(stderr, dir, file)
return err
}
err = ioutil.WriteFile(filepath.Join(dir, file), b, 0644)
@@ -894,7 +895,7 @@
case *types.Map:
if b, ok := x.Key().Underlying().(*types.Basic); !ok || b.Kind() != types.String {
- log.Panicf("unsupported map key type %T", x.Key())
+ panic(fmt.Sprintf("unsupported map key type %T", x.Key()))
}
fmt.Fprintf(e.w, "{ <_>: ")
e.printType(x.Elem())
diff --git a/cmd/cue/cmd/get_go_test.go b/cmd/cue/cmd/get_go_test.go
index 509be39..56092e5 100644
--- a/cmd/cue/cmd/get_go_test.go
+++ b/cmd/cue/cmd/get_go_test.go
@@ -42,7 +42,7 @@
cmd.SetArgs([]string{"./testdata/code/go/..."})
err = cmd.Execute()
if err != nil {
- log.Fatal(err)
+ t.Fatal(err)
}
// Packages will generate differently in modules versus GOPATH. Search
diff --git a/cmd/cue/cmd/import.go b/cmd/cue/cmd/import.go
index 612f64d..adb58c2 100644
--- a/cmd/cue/cmd/import.go
+++ b/cmd/cue/cmd/import.go
@@ -251,8 +251,6 @@
}
func runImport(cmd *Command, args []string) error {
- log.SetOutput(cmd.OutOrStderr())
-
var group errgroup.Group
pkgFlag := flagPackage.String(cmd)
@@ -391,7 +389,10 @@
case os.IsNotExist(err):
case err == nil:
if !flagForce.Bool(cmd) {
- log.Printf("skipping file %q: already exists", cueFile)
+ // TODO: mimic old behavior: write to stderr, but do not exit
+ // with error code. Consider what is best to do here.
+ stderr := cmd.Command.OutOrStderr()
+ fmt.Fprintf(stderr, "skipping file %q: already exists\n", cueFile)
return nil
}
default:
diff --git a/cmd/cue/cmd/root.go b/cmd/cue/cmd/root.go
index 433ffe4..71c9760 100644
--- a/cmd/cue/cmd/root.go
+++ b/cmd/cue/cmd/root.go
@@ -18,7 +18,6 @@
"context"
"fmt"
"io"
- logger "log"
"os"
"strings"
@@ -39,8 +38,6 @@
// TODO: documentation of concepts
// tasks the key element for cmd, serve, and fix
-var log = logger.New(os.Stderr, "", logger.Lshortfile)
-
type runFunction func(cmd *Command, args []string) error
func mkRunE(c *Command, f runFunction) func(*cobra.Command, []string) error {
@@ -125,8 +122,37 @@
// Subcommands
cmd *cobra.Command
+
+ hasErr bool
}
+type errWriter Command
+
+func (w *errWriter) Write(b []byte) (int, error) {
+ c := (*Command)(w)
+ c.hasErr = true
+ return c.Command.OutOrStderr().Write(b)
+}
+
+// Hint: search for uses of OutOrStderr other than the one here to see
+// which output does not trigger a non-zero exit code. os.Stderr may never
+// be used directly.
+
+// Stderr returns a writer that should be used for error messages.
+func (c *Command) Stderr() io.Writer {
+ return (*errWriter)(c)
+}
+
+// TODO: add something similar for Stdout. The output model of Cobra isn't
+// entirely clear, and such a change seems non-trivial.
+
+// Consider overriding these methods from Cobra using OutOrStdErr.
+// We don't use them currently, but may be safer to block. Having them
+// will encourage their usage, and the naming is inconsistent with other CUE APIs.
+// PrintErrf(format string, args ...interface{})
+// PrintErrln(args ...interface{})
+// PrintErr(args ...interface{})
+
func (c *Command) SetOutput(w io.Writer) {
c.root.SetOutput(w)
}
@@ -136,8 +162,10 @@
stdin = r
}
+// ErrPrintedError indicates error messages have been printed to stderr.
+var ErrPrintedError = errors.New("terminating because of errors")
+
func (c *Command) Run(ctx context.Context) (err error) {
- log.SetFlags(0)
// Three categories of commands:
// - normal
// - user defined
@@ -145,7 +173,13 @@
// For the latter two, we need to use the default loading.
defer recoverError(&err)
- return c.root.Execute()
+ if err := c.root.Execute(); err != nil {
+ return err
+ }
+ if c.hasErr {
+ return ErrPrintedError
+ }
+ return nil
}
func recoverError(err *error) {
@@ -199,8 +233,9 @@
}
_, err = addCustom(cmd, rootCmd, commandSection, args[0], tools)
if err != nil {
- fmt.Printf("command %s %q is not defined\n", commandSection, args[0])
- fmt.Println("Run 'cue help' to show available commands.")
+ stderr := cmd.Stderr()
+ fmt.Fprintf(stderr, "command %s %q is not defined\n", commandSection, args[0])
+ fmt.Println(stderr, "Run 'cue help' to show available commands.")
os.Exit(1)
}
return cmd, nil
diff --git a/cmd/cue/cmd/vet.go b/cmd/cue/cmd/vet.go
index 6e5e49a..06c184e 100644
--- a/cmd/cue/cmd/vet.go
+++ b/cmd/cue/cmd/vet.go
@@ -119,7 +119,7 @@
cue.Optional(true),
cue.Hidden(true),
}
- w := cmd.OutOrStderr()
+ w := cmd.Stderr()
err := inst.Value().Validate(append(opt, cue.Concrete(concrete))...)
if err != nil && !hasFlag {
err = inst.Value().Validate(append(opt, cue.Concrete(false))...)
diff --git a/cmd/cue/main.go b/cmd/cue/main.go
index d319567..77c34bb 100644
--- a/cmd/cue/main.go
+++ b/cmd/cue/main.go
@@ -25,7 +25,9 @@
func main() {
err := cmd.Main(context.Background(), os.Args[1:])
if err != nil {
- fmt.Println(err)
+ if err != cmd.ErrPrintedError {
+ fmt.Println(err)
+ }
os.Exit(1)
}
}
