libs/cli: clean up package (#7806)

cmd/tendermint/commands/completion.go

@@ -0,0 +1,46 @@
+package commands
+
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+)
+
+// NewCompletionCmd returns a cobra.Command that generates bash and zsh
+// completion scripts for the given root command. If hidden is true, the
+// command will not show up in the root command's list of available commands.
+func NewCompletionCmd(rootCmd *cobra.Command, hidden bool) *cobra.Command {
+	flagZsh := "zsh"
+	cmd := &cobra.Command{
+		Use:   "completion",
+		Short: "Generate shell completion scripts",
+		Long: fmt.Sprintf(`Generate Bash and Zsh completion scripts and print them to STDOUT.
+
+Once saved to file, a completion script can be loaded in the shell's
+current session as shown:
+
+   $ . <(%s completion)
+
+To configure your bash shell to load completions for each session add to
+your $HOME/.bashrc or $HOME/.profile the following instruction:
+
+   . <(%s completion)
+`, rootCmd.Use, rootCmd.Use),
+		RunE: func(cmd *cobra.Command, _ []string) error {
+			zsh, err := cmd.Flags().GetBool(flagZsh)
+			if err != nil {
+				return err
+			}
+			if zsh {
+				return rootCmd.GenZshCompletion(cmd.OutOrStdout())
+			}
+			return rootCmd.GenBashCompletion(cmd.OutOrStdout())
+		},
+		Hidden: hidden,
+		Args:   cobra.NoArgs,
+	}
+
+	cmd.Flags().Bool(flagZsh, false, "Generate Zsh completion script")
+
+	return cmd
+}

cmd/tendermint/commands/root_test.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -17,6 +18,17 @@ import (
 	tmos "github.com/tendermint/tendermint/libs/os"
 )
 
+// writeConfigVals writes a toml file with the given values.
+// It returns an error if writing was impossible.
+func writeConfigVals(dir string, vals map[string]string) error {
+	data := ""
+	for k, v := range vals {
+		data += fmt.Sprintf("%s = \"%s\"\n", k, v)
+	}
+	cfile := filepath.Join(dir, "config.toml")
+	return os.WriteFile(cfile, []byte(data), 0600)
+}
+
 // clearConfig clears env vars, the given root dir, and resets viper.
 func clearConfig(t *testing.T, dir string) *cfg.Config {
 	t.Helper()
@@ -41,7 +53,7 @@ func testRootCmd(conf *cfg.Config) *cobra.Command {
 	return cmd
 }
 
-func testSetup(t *testing.T, conf *cfg.Config, args []string, env map[string]string) error {
+func testSetup(ctx context.Context, t *testing.T, conf *cfg.Config, args []string, env map[string]string) error {
 	t.Helper()
 
 	cmd := testRootCmd(conf)
@@ -49,7 +61,7 @@ func testSetup(t *testing.T, conf *cfg.Config, args []string, env map[string]str
 
 	// run with the args and env
 	args = append([]string{cmd.Use}, args...)
-	return cli.RunWithArgs(cmd, args, env)
+	return cli.RunWithArgs(ctx, cmd, args, env)
 }
 
 func TestRootHome(t *testing.T) {
@@ -65,11 +77,14 @@ func TestRootHome(t *testing.T) {
 		{nil, map[string]string{"TMHOME": newRoot}, newRoot},
 	}
 
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	for i, tc := range cases {
 		t.Run(fmt.Sprint(i), func(t *testing.T) {
 			conf := clearConfig(t, tc.root)
 
-			err := testSetup(t, conf, tc.args, tc.env)
+			err := testSetup(ctx, t, conf, tc.args, tc.env)
 			require.NoError(t, err)
 
 			require.Equal(t, tc.root, conf.RootDir)
@@ -99,11 +114,14 @@ func TestRootFlagsEnv(t *testing.T) {
 		{nil, map[string]string{"TM_LOG_LEVEL": "debug"}, "debug"},       // right env
 	}
 
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	for i, tc := range cases {
 		t.Run(fmt.Sprint(i), func(t *testing.T) {
 			conf := clearConfig(t, defaultDir)
 
-			err := testSetup(t, conf, tc.args, tc.env)
+			err := testSetup(ctx, t, conf, tc.args, tc.env)
 			require.NoError(t, err)
 
 			assert.Equal(t, tc.logLevel, conf.LogLevel)
@@ -113,6 +131,9 @@ func TestRootFlagsEnv(t *testing.T) {
 }
 
 func TestRootConfig(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	// write non-default config
 	nonDefaultLogLvl := "debug"
 	cvals := map[string]string{
@@ -142,14 +163,14 @@ func TestRootConfig(t *testing.T) {
 
 			// write the non-defaults to a different path
 			// TODO: support writing sub configs so we can test that too
-			err = WriteConfigVals(configFilePath, cvals)
+			err = writeConfigVals(configFilePath, cvals)
 			require.NoError(t, err)
 
 			cmd := testRootCmd(conf)
 
 			// run with the args and env
 			tc.args = append([]string{cmd.Use}, tc.args...)
-			err = cli.RunWithArgs(cmd, tc.args, tc.env)
+			err = cli.RunWithArgs(ctx, cmd, tc.args, tc.env)
 			require.NoError(t, err)
 
 			require.Equal(t, tc.logLvl, conf.LogLevel)

cmd/tendermint/main.go

@@ -44,7 +44,7 @@ func main() {
 		commands.MakeRollbackStateCommand(conf),
 		commands.MakeKeyMigrateCommand(conf, logger),
 		debug.DebugCmd,
-		cli.NewCompletionCmd(rcmd, true),
+		commands.NewCompletionCmd(rcmd, true),
 	)
 
 	// NOTE:
@@ -60,7 +60,7 @@ func main() {
 	// Create & start node
 	rcmd.AddCommand(commands.NewRunNodeCmd(nodeFunc, conf, logger))
 
-	if err := rcmd.ExecuteContext(ctx); err != nil {
+	if err := cli.RunWithTrace(ctx, rcmd); err != nil {
 		panic(err)
 	}
 }

libs/cli/helper.go

@@ -1,29 +1,20 @@
 package cli
 
 import (
-	"bytes"
+	"context"
 	"fmt"
-	"io"
 	"os"
-	"path/filepath"
+	"runtime"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
 )
 
-// WriteConfigVals writes a toml file with the given values.
-// It returns an error if writing was impossible.
-func WriteConfigVals(dir string, vals map[string]string) error {
-	data := ""
-	for k, v := range vals {
-		data += fmt.Sprintf("%s = \"%s\"\n", k, v)
-	}
-	cfile := filepath.Join(dir, "config.toml")
-	return os.WriteFile(cfile, []byte(data), 0600)
-}
-
 // RunWithArgs executes the given command with the specified command line args
 // and environmental variables set. It returns any error returned from cmd.Execute()
-func RunWithArgs(cmd Executable, args []string, env map[string]string) error {
+//
+// This is only used in testing.
+func RunWithArgs(ctx context.Context, cmd *cobra.Command, args []string, env map[string]string) error {
 	oargs := os.Args
 	oenv := map[string]string{}
 	// defer returns the environment back to normal
@@ -46,85 +37,24 @@ func RunWithArgs(cmd Executable, args []string, env map[string]string) error {
 	}
 
 	// and finally run the command
-	return cmd.Execute()
+	return RunWithTrace(ctx, cmd)
 }
 
-// RunCaptureWithArgs executes the given command with the specified command
-// line args and environmental variables set. It returns string fields
-// representing output written to stdout and stderr, additionally any error
-// from cmd.Execute() is also returned
-func RunCaptureWithArgs(cmd Executable, args []string, env map[string]string) (stdout, stderr string, err error) {
-	oldout, olderr := os.Stdout, os.Stderr // keep backup of the real stdout
-	rOut, wOut, _ := os.Pipe()
-	rErr, wErr, _ := os.Pipe()
-	os.Stdout, os.Stderr = wOut, wErr
-	defer func() {
-		os.Stdout, os.Stderr = oldout, olderr // restoring the real stdout
-	}()
-
-	// copy the output in a separate goroutine so printing can't block indefinitely
-	copyStd := func(reader *os.File) *(chan string) {
-		stdC := make(chan string)
-		go func() {
-			var buf bytes.Buffer
-			// io.Copy will end when we call reader.Close() below
-			io.Copy(&buf, reader) //nolint:errcheck //ignore error
-			select {
-			case <-cmd.Context().Done():
-			case stdC <- buf.String():
-			}
-		}()
-		return &stdC
-	}
-	outC := copyStd(rOut)
-	errC := copyStd(rErr)
-
-	// now run the command
-	err = RunWithArgs(cmd, args, env)
-
-	// and grab the stdout to return
-	wOut.Close()
-	wErr.Close()
-	stdout = <-*outC
-	stderr = <-*errC
-	return stdout, stderr, err
-}
-
-// NewCompletionCmd returns a cobra.Command that generates bash and zsh
-// completion scripts for the given root command. If hidden is true, the
-// command will not show up in the root command's list of available commands.
-func NewCompletionCmd(rootCmd *cobra.Command, hidden bool) *cobra.Command {
-	flagZsh := "zsh"
-	cmd := &cobra.Command{
-		Use:   "completion",
-		Short: "Generate shell completion scripts",
-		Long: fmt.Sprintf(`Generate Bash and Zsh completion scripts and print them to STDOUT.
-
-Once saved to file, a completion script can be loaded in the shell's
-current session as shown:
-
-   $ . <(%s completion)
-
-To configure your bash shell to load completions for each session add to
-your $HOME/.bashrc or $HOME/.profile the following instruction:
+func RunWithTrace(ctx context.Context, cmd *cobra.Command) error {
+	cmd.SilenceUsage = true
+	cmd.SilenceErrors = true
+
+	if err := cmd.ExecuteContext(ctx); err != nil {
+		if viper.GetBool(TraceFlag) {
+			const size = 64 << 10
+			buf := make([]byte, size)
+			buf = buf[:runtime.Stack(buf, false)]
+			fmt.Fprintf(os.Stderr, "ERROR: %v\n%s\n", err, buf)
+		} else {
+			fmt.Fprintf(os.Stderr, "ERROR: %v\n", err)
+		}
 
-   . <(%s completion)
-`, rootCmd.Use, rootCmd.Use),
-		RunE: func(cmd *cobra.Command, _ []string) error {
-			zsh, err := cmd.Flags().GetBool(flagZsh)
-			if err != nil {
-				return err
-			}
-			if zsh {
-				return rootCmd.GenZshCompletion(cmd.OutOrStdout())
-			}
-			return rootCmd.GenBashCompletion(cmd.OutOrStdout())
-		},
-		Hidden: hidden,
-		Args:   cobra.NoArgs,
+		return err
 	}
-
-	cmd.Flags().Bool(flagZsh, false, "Generate Zsh completion script")
-
-	return cmd
+	return nil
 }

libs/cli/setup.go

@@ -1,11 +1,8 @@
 package cli
 
 import (
-	"context"
-	"fmt"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -13,52 +10,27 @@ import (
 )
 
 const (
-	HomeFlag     = "home"
-	TraceFlag    = "trace"
-	OutputFlag   = "output"
-	EncodingFlag = "encoding"
+	HomeFlag   = "home"
+	TraceFlag  = "trace"
+	OutputFlag = "output" // used in the cli
 )
 
-// Executable is the minimal interface to *corba.Command, so we can
-// wrap if desired before the test
-type Executable interface {
-	Execute() error
-	Context() context.Context
-}
-
 // PrepareBaseCmd is meant for tendermint and other servers
-func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defaultHome string) Executor {
+func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defaultHome string) *cobra.Command {
+	// the primary caller of this command is in the SDK and
+	// returning the cobra.Command object avoids breaking that
+	// code. In the long term, the SDK could avoid this entirely.
 	cobra.OnInitialize(func() { InitEnv(envPrefix) })
 	cmd.PersistentFlags().StringP(HomeFlag, "", defaultHome, "directory for config and data")
 	cmd.PersistentFlags().Bool(TraceFlag, false, "print out full stack trace on errors")
 	cmd.PersistentPreRunE = concatCobraCmdFuncs(BindFlagsLoadViper, cmd.PersistentPreRunE)
-	return Executor{cmd, os.Exit}
-}
-
-// PrepareMainCmd is meant for client side libs that want some more flags
-//
-// This adds --encoding (hex, btc, base64) and --output (text, json) to
-// the command.  These only really make sense in interactive commands.
-func PrepareMainCmd(cmd *cobra.Command, envPrefix, defaultHome string) Executor {
-	cmd.PersistentFlags().StringP(EncodingFlag, "e", "hex", "Binary encoding (hex|b64|btc)")
-	cmd.PersistentFlags().StringP(OutputFlag, "o", "text", "Output format (text|json)")
-	cmd.PersistentPreRunE = concatCobraCmdFuncs(validateOutput, cmd.PersistentPreRunE)
-	return PrepareBaseCmd(cmd, envPrefix, defaultHome)
+	return cmd
 }
 
 // InitEnv sets to use ENV variables if set.
 func InitEnv(prefix string) {
-	copyEnvVars(prefix)
-
-	// env variables with TM prefix (eg. TM_ROOT)
-	viper.SetEnvPrefix(prefix)
-	viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_", "-", "_"))
-	viper.AutomaticEnv()
-}
-
-// This copies all variables like TMROOT to TM_ROOT,
-// so we can support both formats for the user
-func copyEnvVars(prefix string) {
+	// This copies all variables like TMROOT to TM_ROOT,
+	// so we can support both formats for the user
 	prefix = strings.ToUpper(prefix)
 	ps := prefix + "_"
 	for _, e := range os.Environ() {
@@ -71,42 +43,11 @@ func copyEnvVars(prefix string) {
 			}
 		}
 	}
-}
-
-// Executor wraps the cobra Command with a nicer Execute method
-type Executor struct {
-	*cobra.Command
-	Exit func(int) // this is os.Exit by default, override in tests
-}
 
-type ExitCoder interface {
-	ExitCode() int
-}
-
-// execute adds all child commands to the root command sets flags appropriately.
-// This is called by main.main(). It only needs to happen once to the rootCmd.
-func (e Executor) Execute() error {
-	e.SilenceUsage = true
-	e.SilenceErrors = true
-	err := e.Command.Execute()
-	if err != nil {
-		if viper.GetBool(TraceFlag) {
-			const size = 64 << 10
-			buf := make([]byte, size)
-			buf = buf[:runtime.Stack(buf, false)]
-			fmt.Fprintf(os.Stderr, "ERROR: %v\n%s\n", err, buf)
-		} else {
-			fmt.Fprintf(os.Stderr, "ERROR: %v\n", err)
-		}
-
-		// return error code 1 by default, can override it with a special error type
-		exitCode := 1
-		if ec, ok := err.(ExitCoder); ok {
-			exitCode = ec.ExitCode()
-		}
-		e.Exit(exitCode)
-	}
-	return err
+	// env variables with TM prefix (eg. TM_ROOT)
+	viper.SetEnvPrefix(prefix)
+	viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_", "-", "_"))
+	viper.AutomaticEnv()
 }
 
 type cobraCmdFunc func(cmd *cobra.Command, args []string) error
@@ -149,14 +90,3 @@ func BindFlagsLoadViper(cmd *cobra.Command, args []string) error {
 	}
 	return nil
 }
-
-func validateOutput(cmd *cobra.Command, args []string) error {
-	// validate output format
-	output := viper.GetString(OutputFlag)
-	switch output {
-	case "text", "json":
-	default:
-		return fmt.Errorf("unsupported output format: %s", output)
-	}
-	return nil
-}

libs/cli/setup_test.go

@@ -1,8 +1,12 @@
 package cli
 
 import (
+	"bytes"
+	"context"
 	"fmt"
+	"io"
 	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
@@ -14,6 +18,9 @@ import (
 )
 
 func TestSetupEnv(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	cases := []struct {
 		args     []string
 		env      map[string]string
@@ -44,11 +51,10 @@ func TestSetupEnv(t *testing.T) {
 		}
 		demo.Flags().String("foobar", "", "Some test value from config")
 		cmd := PrepareBaseCmd(demo, "DEMO", "/qwerty/asdfgh") // some missing dir..
-		cmd.Exit = func(int) {}
 
 		viper.Reset()
 		args := append([]string{cmd.Use}, tc.args...)
-		err := RunWithArgs(cmd, args, tc.env)
+		err := RunWithArgs(ctx, cmd, args, tc.env)
 		require.NoError(t, err, i)
 		assert.Equal(t, tc.expected, foo, i)
 	}
@@ -61,12 +67,27 @@ func tempDir(t *testing.T) string {
 	return cdir
 }
 
+// writeConfigVals writes a toml file with the given values.
+// It returns an error if writing was impossible.
+func writeConfigVals(dir string, vals map[string]string) error {
+	lines := make([]string, 0, len(vals))
+	for k, v := range vals {
+		lines = append(lines, fmt.Sprintf("%s = %q", k, v))
+	}
+	data := strings.Join(lines, "\n")
+	cfile := filepath.Join(dir, "config.toml")
+	return os.WriteFile(cfile, []byte(data), 0600)
+}
+
 func TestSetupConfig(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	// we pre-create two config files we can refer to in the rest of
 	// the test cases.
 	cval1 := "fubble"
 	conf1 := tempDir(t)
-	err := WriteConfigVals(conf1, map[string]string{"boo": cval1})
+	err := writeConfigVals(conf1, map[string]string{"boo": cval1})
 	require.NoError(t, err)
 
 	cases := []struct {
@@ -103,11 +124,10 @@ func TestSetupConfig(t *testing.T) {
 		boo.Flags().String("boo", "", "Some test value from config")
 		boo.Flags().String("two-words", "", "Check out env handling -")
 		cmd := PrepareBaseCmd(boo, "RD", "/qwerty/asdfgh") // some missing dir...
-		cmd.Exit = func(int) {}
 
 		viper.Reset()
 		args := append([]string{cmd.Use}, tc.args...)
-		err := RunWithArgs(cmd, args, tc.env)
+		err := RunWithArgs(ctx, cmd, args, tc.env)
 		require.NoError(t, err, i)
 		assert.Equal(t, tc.expected, foo, i)
 		assert.Equal(t, tc.expectedTwo, two, i)
@@ -121,15 +141,18 @@ type DemoConfig struct {
 }
 
 func TestSetupUnmarshal(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	// we pre-create two config files we can refer to in the rest of
 	// the test cases.
 	cval1, cval2 := "someone", "else"
 	conf1 := tempDir(t)
-	err := WriteConfigVals(conf1, map[string]string{"name": cval1})
+	err := writeConfigVals(conf1, map[string]string{"name": cval1})
 	require.NoError(t, err)
 	// even with some ignored fields, should be no problem
 	conf2 := tempDir(t)
-	err = WriteConfigVals(conf2, map[string]string{"name": cval2, "foo": "bar"})
+	err = writeConfigVals(conf2, map[string]string{"name": cval2, "foo": "bar"})
 	require.NoError(t, err)
 
 	// unused is not declared on a flag and remains from base
@@ -182,17 +205,19 @@ func TestSetupUnmarshal(t *testing.T) {
 		// from the default config here
 		marsh.Flags().Int("age", base.Age, "Some test value from config")
 		cmd := PrepareBaseCmd(marsh, "MR", "/qwerty/asdfgh") // some missing dir...
-		cmd.Exit = func(int) {}
 
 		viper.Reset()
 		args := append([]string{cmd.Use}, tc.args...)
-		err := RunWithArgs(cmd, args, tc.env)
+		err := RunWithArgs(ctx, cmd, args, tc.env)
 		require.NoError(t, err, i)
 		assert.Equal(t, tc.expected, cfg, i)
 	}
 }
 
 func TestSetupTrace(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	cases := []struct {
 		args     []string
 		env      map[string]string
@@ -215,18 +240,16 @@ func TestSetupTrace(t *testing.T) {
 			},
 		}
 		cmd := PrepareBaseCmd(trace, "DBG", "/qwerty/asdfgh") // some missing dir..
-		cmd.Exit = func(int) {}
 
 		viper.Reset()
 		args := append([]string{cmd.Use}, tc.args...)
-		stdout, stderr, err := RunCaptureWithArgs(cmd, args, tc.env)
+		stdout, stderr, err := runCaptureWithArgs(ctx, cmd, args, tc.env)
 		require.Error(t, err, i)
 		require.Equal(t, "", stdout, i)
 		require.NotEqual(t, "", stderr, i)
 		msg := strings.Split(stderr, "\n")
 		desired := fmt.Sprintf("ERROR: %s", tc.expected)
-		assert.Equal(t, desired, msg[0], i)
-		t.Log(msg)
+		assert.Equal(t, desired, msg[0], i, msg)
 		if tc.long && assert.True(t, len(msg) > 2, i) {
 			// the next line starts the stack trace...
 			assert.Contains(t, stderr, "TestSetupTrace", i)
@@ -234,3 +257,44 @@ func TestSetupTrace(t *testing.T) {
 		}
 	}
 }
+
+// runCaptureWithArgs executes the given command with the specified command
+// line args and environmental variables set. It returns string fields
+// representing output written to stdout and stderr, additionally any error
+// from cmd.Execute() is also returned
+func runCaptureWithArgs(ctx context.Context, cmd *cobra.Command, args []string, env map[string]string) (stdout, stderr string, err error) {
+	oldout, olderr := os.Stdout, os.Stderr // keep backup of the real stdout
+	rOut, wOut, _ := os.Pipe()
+	rErr, wErr, _ := os.Pipe()
+	os.Stdout, os.Stderr = wOut, wErr
+	defer func() {
+		os.Stdout, os.Stderr = oldout, olderr // restoring the real stdout
+	}()
+
+	// copy the output in a separate goroutine so printing can't block indefinitely
+	copyStd := func(reader *os.File) *(chan string) {
+		stdC := make(chan string)
+		go func() {
+			var buf bytes.Buffer
+			// io.Copy will end when we call reader.Close() below
+			io.Copy(&buf, reader) //nolint:errcheck //ignore error
+			select {
+			case <-cmd.Context().Done():
+			case stdC <- buf.String():
+			}
+		}()
+		return &stdC
+	}
+	outC := copyStd(rOut)
+	errC := copyStd(rErr)
+
+	// now run the command
+	err = RunWithArgs(ctx, cmd, args, env)
+
+	// and grab the stdout to return
+	wOut.Close()
+	wErr.Close()
+	stdout = <-*outC
+	stderr = <-*errC
+	return stdout, stderr, err
+}