From b92bd8f6a8d10553d27fdff8cf6d7205c83904f3 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 2 May 2017 17:01:50 +0200 Subject: [PATCH 01/14] Separate out PrepareBaseCmd, try to set env vars --- cli/setup.go | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 cli/setup.go diff --git a/cli/setup.go b/cli/setup.go new file mode 100644 index 000000000..be5735d92 --- /dev/null +++ b/cli/setup.go @@ -0,0 +1,136 @@ +package cli + +import ( + "fmt" + "os" + "strings" + + "github.com/pkg/errors" + "github.com/spf13/cobra" + "github.com/spf13/viper" + data "github.com/tendermint/go-wire/data" + "github.com/tendermint/go-wire/data/base58" +) + +const ( + RootFlag = "root" + OutputFlag = "output" + EncodingFlag = "encoding" +) + +// PrepareBaseCmd is meant for tendermint and other servers +func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defautRoot string) func() { + cobra.OnInitialize(func() { initEnv(envPrefix) }) + cmd.PersistentFlags().StringP(RootFlag, "r", defautRoot, "root directory for config and data") + cmd.PersistentPreRunE = multiE(bindFlags, cmd.PersistentPreRunE) + return func() { execute(cmd) } +} + +// PrepareMainCmd is meant for client side libs that want some more flags +func PrepareMainCmd(cmd *cobra.Command, envPrefix, defautRoot string) func() { + cmd.PersistentFlags().StringP(EncodingFlag, "e", "hex", "Binary encoding (hex|b64|btc)") + cmd.PersistentFlags().StringP(OutputFlag, "o", "text", "Output format (text|json)") + cmd.PersistentPreRunE = multiE(setEncoding, validateOutput, cmd.PersistentPreRunE) + return PrepareBaseCmd(cmd, envPrefix, defautRoot) +} + +// 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) { + prefix = strings.ToUpper(prefix) + ps := prefix + "_" + for _, e := range os.Environ() { + kv := strings.SplitN(e, "=", 1) + k, v := kv[0], kv[1] + if strings.HasPrefix(k, prefix) && !strings.HasPrefix(k, ps) { + k2 := strings.Replace(k, prefix, ps, 1) + os.Setenv(k2, v) + } + } +} + +// 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 execute(cmd *cobra.Command) { + // TODO: this can do something cooler with debug and log-levels + if err := cmd.Execute(); err != nil { + fmt.Println(err) + os.Exit(-1) + } +} + +type wrapE func(cmd *cobra.Command, args []string) error + +func multiE(fs ...wrapE) wrapE { + return func(cmd *cobra.Command, args []string) error { + for _, f := range fs { + if f != nil { + if err := f(cmd, args); err != nil { + return err + } + } + } + return nil + } +} + +func bindFlags(cmd *cobra.Command, args []string) error { + // cmd.Flags() includes flags from this command and all persistent flags from the parent + if err := viper.BindPFlags(cmd.Flags()); err != nil { + return err + } + + // rootDir is command line flag, env variable, or default $HOME/.tlc + rootDir := viper.GetString("root") + viper.SetConfigName("config") // name of config file (without extension) + viper.AddConfigPath(rootDir) // search root directory + + // If a config file is found, read it in. + if err := viper.ReadInConfig(); err == nil { + // stderr, so if we redirect output to json file, this doesn't appear + // fmt.Fprintln(os.Stderr, "Using config file:", viper.ConfigFileUsed()) + } else if _, ok := err.(viper.ConfigFileNotFoundError); !ok { + // we ignore not found error, only parse error + // stderr, so if we redirect output to json file, this doesn't appear + fmt.Fprintf(os.Stderr, "%#v", err) + } + return nil +} + +// setEncoding reads the encoding flag +func setEncoding(cmd *cobra.Command, args []string) error { + // validate and set encoding + enc := viper.GetString("encoding") + switch enc { + case "hex": + data.Encoder = data.HexEncoder + case "b64": + data.Encoder = data.B64Encoder + case "btc": + data.Encoder = base58.BTCEncoder + default: + return errors.Errorf("Unsupported encoding: %s", enc) + } + return nil +} + +func validateOutput(cmd *cobra.Command, args []string) error { + // validate output format + output := viper.GetString(OutputFlag) + switch output { + case "text", "json": + default: + return errors.Errorf("Unsupported output format: %s", output) + } + return nil +} From d4ab9679d71c8fc174284696d15930cb799fa24f Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 2 May 2017 17:16:22 +0200 Subject: [PATCH 02/14] Fix up error in copyEnv --- cli/setup.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cli/setup.go b/cli/setup.go index be5735d92..5a7218a80 100644 --- a/cli/setup.go +++ b/cli/setup.go @@ -50,11 +50,13 @@ func copyEnvVars(prefix string) { prefix = strings.ToUpper(prefix) ps := prefix + "_" for _, e := range os.Environ() { - kv := strings.SplitN(e, "=", 1) - k, v := kv[0], kv[1] - if strings.HasPrefix(k, prefix) && !strings.HasPrefix(k, ps) { - k2 := strings.Replace(k, prefix, ps, 1) - os.Setenv(k2, v) + kv := strings.SplitN(e, "=", 2) + if len(kv) == 2 { + k, v := kv[0], kv[1] + if strings.HasPrefix(k, prefix) && !strings.HasPrefix(k, ps) { + k2 := strings.Replace(k, prefix, ps, 1) + os.Setenv(k2, v) + } } } } From a95a60cb0bece614db9c0d16faade4aaad0dfab5 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 2 May 2017 14:50:24 -0400 Subject: [PATCH 03/14] cli: support --root and --home --- cli/setup.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cli/setup.go b/cli/setup.go index 5a7218a80..d7a11e069 100644 --- a/cli/setup.go +++ b/cli/setup.go @@ -14,6 +14,7 @@ import ( const ( RootFlag = "root" + HomeFlag = "home" OutputFlag = "output" EncodingFlag = "encoding" ) @@ -21,7 +22,8 @@ const ( // PrepareBaseCmd is meant for tendermint and other servers func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defautRoot string) func() { cobra.OnInitialize(func() { initEnv(envPrefix) }) - cmd.PersistentFlags().StringP(RootFlag, "r", defautRoot, "root directory for config and data") + cmd.PersistentFlags().StringP(RootFlag, "r", defautRoot, "DEPRECATED. Use --home") + cmd.PersistentFlags().StringP(HomeFlag, "h", defautRoot, "root directory for config and data") cmd.PersistentPreRunE = multiE(bindFlags, cmd.PersistentPreRunE) return func() { execute(cmd) } } @@ -93,7 +95,11 @@ func bindFlags(cmd *cobra.Command, args []string) error { } // rootDir is command line flag, env variable, or default $HOME/.tlc - rootDir := viper.GetString("root") + // NOTE: we support both --root and --home for now, but eventually only --home + rootDir := viper.GetString(HomeFlag) + if !viper.IsSet(HomeFlag) && viper.IsSet(RootFlag) { + rootDir = viper.GetString(RootFlag) + } viper.SetConfigName("config") // name of config file (without extension) viper.AddConfigPath(rootDir) // search root directory From 7becd35126765a5cad3018c1efc3922cd2f1a0d2 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 2 May 2017 14:57:32 -0400 Subject: [PATCH 04/14] cli: more descriptive naming --- cli/setup.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cli/setup.go b/cli/setup.go index d7a11e069..14801ee52 100644 --- a/cli/setup.go +++ b/cli/setup.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/viper" + data "github.com/tendermint/go-wire/data" "github.com/tendermint/go-wire/data/base58" ) @@ -24,7 +25,7 @@ func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defautRoot string) func() { cobra.OnInitialize(func() { initEnv(envPrefix) }) cmd.PersistentFlags().StringP(RootFlag, "r", defautRoot, "DEPRECATED. Use --home") cmd.PersistentFlags().StringP(HomeFlag, "h", defautRoot, "root directory for config and data") - cmd.PersistentPreRunE = multiE(bindFlags, cmd.PersistentPreRunE) + cmd.PersistentPreRunE = concatCobraCmdFuncs(bindFlagsLoadViper, cmd.PersistentPreRunE) return func() { execute(cmd) } } @@ -32,7 +33,7 @@ func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defautRoot string) func() { func PrepareMainCmd(cmd *cobra.Command, envPrefix, defautRoot string) func() { cmd.PersistentFlags().StringP(EncodingFlag, "e", "hex", "Binary encoding (hex|b64|btc)") cmd.PersistentFlags().StringP(OutputFlag, "o", "text", "Output format (text|json)") - cmd.PersistentPreRunE = multiE(setEncoding, validateOutput, cmd.PersistentPreRunE) + cmd.PersistentPreRunE = concatCobraCmdFuncs(setEncoding, validateOutput, cmd.PersistentPreRunE) return PrepareBaseCmd(cmd, envPrefix, defautRoot) } @@ -73,9 +74,10 @@ func execute(cmd *cobra.Command) { } } -type wrapE func(cmd *cobra.Command, args []string) error +type cobraCmdFunc func(cmd *cobra.Command, args []string) error -func multiE(fs ...wrapE) wrapE { +// Returns a single function that calls each argument function in sequence +func concatCobraCmdFuncs(fs ...cobraCmdFunc) cobraCmdFunc { return func(cmd *cobra.Command, args []string) error { for _, f := range fs { if f != nil { @@ -88,7 +90,8 @@ func multiE(fs ...wrapE) wrapE { } } -func bindFlags(cmd *cobra.Command, args []string) error { +// Bind all flags and read the config into viper +func bindFlagsLoadViper(cmd *cobra.Command, args []string) error { // cmd.Flags() includes flags from this command and all persistent flags from the parent if err := viper.BindPFlags(cmd.Flags()); err != nil { return err From 435fd0ece75acd97910f7e617525bc31839730bc Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 3 May 2017 09:25:04 +0200 Subject: [PATCH 05/14] Add clarifying comments as requested by Rigel --- cli/setup.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cli/setup.go b/cli/setup.go index 14801ee52..8120449c7 100644 --- a/cli/setup.go +++ b/cli/setup.go @@ -30,6 +30,9 @@ func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defautRoot string) func() { } // 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, defautRoot string) func() { cmd.PersistentFlags().StringP(EncodingFlag, "e", "hex", "Binary encoding (hex|b64|btc)") cmd.PersistentFlags().StringP(OutputFlag, "o", "text", "Output format (text|json)") @@ -77,6 +80,7 @@ func execute(cmd *cobra.Command) { type cobraCmdFunc func(cmd *cobra.Command, args []string) error // Returns a single function that calls each argument function in sequence +// RunE, PreRunE, PersistentPreRunE, etc. all have this same signature func concatCobraCmdFuncs(fs ...cobraCmdFunc) cobraCmdFunc { return func(cmd *cobra.Command, args []string) error { for _, f := range fs { From 62427adbec0814203bfcc93ee20e40092b5f6f3c Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 3 May 2017 10:02:21 +0200 Subject: [PATCH 06/14] First basic test case on setup functionality --- cli/setup_test.go | 122 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 cli/setup_test.go diff --git a/cli/setup_test.go b/cli/setup_test.go new file mode 100644 index 000000000..4e16bdfe2 --- /dev/null +++ b/cli/setup_test.go @@ -0,0 +1,122 @@ +package cli + +import ( + "bytes" + "io" + "os" + "strconv" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Executable is the minimal interface to *corba.Command, so we can +// wrap if desired before the test +type Executable interface { + Execute() error +} + +func TestSetupEnv(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + cases := []struct { + args []string + env map[string]string + expected string + }{ + {nil, nil, ""}, + {[]string{"--foobar", "bang!"}, nil, "bang!"}, + // make sure reset is good + {nil, nil, ""}, + // test both variants of the prefix + {nil, map[string]string{"DEMO_FOOBAR": "good"}, "good"}, + {nil, map[string]string{"DEMOFOOBAR": "silly"}, "silly"}, + // and that cli overrides env... + {[]string{"--foobar", "important"}, + map[string]string{"DEMO_FOOBAR": "ignored"}, "important"}, + } + + for idx, tc := range cases { + i := strconv.Itoa(idx) + // test command that store value of foobar in local variable + var foo string + cmd := &cobra.Command{ + Use: "demo", + RunE: func(cmd *cobra.Command, args []string) error { + foo = viper.GetString("foobar") + return nil + }, + } + cmd.Flags().String("foobar", "", "Some test value from config") + PrepareBaseCmd(cmd, "DEMO", "/qwerty/asdfgh") // some missing dir.. + + viper.Reset() + args := append([]string{cmd.Use}, tc.args...) + err := runWithArgs(cmd, args, tc.env) + require.Nil(err, i) + assert.Equal(tc.expected, foo, i) + } +} + +// 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 { + oargs := os.Args + oenv := map[string]string{} + // defer returns the environment back to normal + defer func() { + os.Args = oargs + for k, v := range oenv { + os.Setenv(k, v) + } + }() + + // set the args and env how we want them + os.Args = args + for k, v := range env { + // backup old value if there, to restore at end + ov := os.Getenv(k) + if ov != "" { + oenv[k] = ov + } + err := os.Setenv(k, v) + if err != nil { + return err + } + } + + // and finally run the command + return cmd.Execute() +} + +// runCaptureWithArgs executes the given command with the specified command line args +// and environmental variables set. It returns whatever was writen to +// stdout along with any error returned from cmd.Execute() +func runCaptureWithArgs(cmd Executable, args []string, env map[string]string) (output string, err error) { + old := os.Stdout // keep backup of the real stdout + r, w, _ := os.Pipe() + os.Stdout = w + defer func() { + os.Stdout = old // restoring the real stdout + }() + + outC := make(chan string) + // copy the output in a separate goroutine so printing can't block indefinitely + go func() { + var buf bytes.Buffer + // io.Copy will end when we call w.Close() below + io.Copy(&buf, r) + outC <- buf.String() + }() + + // now run the command + err = runWithArgs(cmd, args, env) + + // and grab the stdout to return + w.Close() + output = <-outC + return output, err +} From 5637a7885430b695dc06fb9d9b7f034a2a0361b2 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 3 May 2017 10:23:58 +0200 Subject: [PATCH 07/14] Test setting config file as root --- cli/setup_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 4 deletions(-) diff --git a/cli/setup_test.go b/cli/setup_test.go index 4e16bdfe2..fb8e5655c 100644 --- a/cli/setup_test.go +++ b/cli/setup_test.go @@ -2,8 +2,11 @@ package cli import ( "bytes" + "fmt" "io" + "io/ioutil" "os" + "path/filepath" "strconv" "testing" @@ -61,6 +64,69 @@ func TestSetupEnv(t *testing.T) { } } +func writeConfig(vals map[string]string) (string, error) { + cdir, err := ioutil.TempDir("", "test-cli") + if err != nil { + return "", err + } + data := "" + for k, v := range vals { + data = data + fmt.Sprintf("%s = \"%s\"\n", k, v) + } + cfile := filepath.Join(cdir, "config.toml") + err = ioutil.WriteFile(cfile, []byte(data), 0666) + return cdir, err +} + +func TestSetupConfig(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + // we pre-create two config files we can refer to in the rest of + // the test cases. + cval1, cval2 := "fubble", "wubble" + conf1, err := writeConfig(map[string]string{"boo": cval1}) + require.Nil(err) + // even with some ignored fields, should be no problem + conf2, err := writeConfig(map[string]string{"boo": cval2, "foo": "bar"}) + require.Nil(err) + + cases := []struct { + args []string + env map[string]string + expected string + }{ + {nil, nil, ""}, + // setting on the command line + {[]string{"--boo", "haha"}, nil, "haha"}, + {[]string{"--root", conf1}, nil, cval1}, + // test both variants of the prefix + {nil, map[string]string{"RD_BOO": "bang"}, "bang"}, + {nil, map[string]string{"RD_ROOT": conf1}, cval1}, + {nil, map[string]string{"RDROOT": conf2}, cval2}, + } + + for idx, tc := range cases { + i := strconv.Itoa(idx) + // test command that store value of foobar in local variable + var foo string + cmd := &cobra.Command{ + Use: "reader", + RunE: func(cmd *cobra.Command, args []string) error { + foo = viper.GetString("boo") + return nil + }, + } + cmd.Flags().String("boo", "", "Some test value from config") + PrepareBaseCmd(cmd, "RD", "/qwerty/asdfgh") // some missing dir... + + viper.Reset() + args := append([]string{cmd.Use}, tc.args...) + err := runWithArgs(cmd, args, tc.env) + require.Nil(err, i) + assert.Equal(tc.expected, foo, i) + } +} + // 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 { @@ -78,10 +144,7 @@ func runWithArgs(cmd Executable, args []string, env map[string]string) error { os.Args = args for k, v := range env { // backup old value if there, to restore at end - ov := os.Getenv(k) - if ov != "" { - oenv[k] = ov - } + oenv[k] = os.Getenv(k) err := os.Setenv(k, v) if err != nil { return err From d05b8131a32c72139bf187834478c09889aa6b30 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 3 May 2017 10:30:50 +0200 Subject: [PATCH 08/14] Updated glide with cobra/viper, fixed Makefile typo --- Makefile | 2 +- glide.lock | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++---- glide.yaml | 3 +++ 3 files changed, 68 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 49acb091a..cd1a57346 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONEY: all test install get_vendor_deps ensure_tools +.PHONY: all test install get_vendor_deps ensure_tools GOTOOLS = \ github.com/Masterminds/glide diff --git a/glide.lock b/glide.lock index 003e3d618..b7d4fb8ff 100644 --- a/glide.lock +++ b/glide.lock @@ -1,16 +1,65 @@ -hash: 47e715510d6b57cff8dc4750b6b9d89a41469a8330a7a8bea1c044b2ac61e581 -updated: 2017-04-21T16:04:25.798163098-04:00 +hash: a28817fffc1bfbba980a957b7782a84ea574fb73d5dfb01730f7e304c9dee630 +updated: 2017-05-03T10:27:41.060683376+02:00 imports: +- name: github.com/fsnotify/fsnotify + version: 4da3e2cfbabc9f751898f250b49f2439785783a1 +- name: github.com/go-kit/kit + version: 0873e56b0faeae3a1d661b10d629135508ea5504 + subpackages: + - log + - log/level + - log/term +- name: github.com/go-logfmt/logfmt + version: 390ab7935ee28ec6b286364bba9b4dd6410cb3d5 - name: github.com/go-stack/stack version: 100eb0c0a9c5b306ca2fb4f165df21d80ada4b82 - name: github.com/golang/snappy - version: d9eb7a3d35ec988b8585d4a0068e462c27d28380 + version: 553a641470496b2327abcac10b36396bd98e45c9 +- name: github.com/hashicorp/hcl + version: 630949a3c5fa3c613328e1b8256052cbc2327c9b + subpackages: + - hcl/ast + - hcl/parser + - hcl/scanner + - hcl/strconv + - hcl/token + - json/parser + - json/scanner + - json/token +- name: github.com/inconshreveable/mousetrap + version: 76626ae9c91c4f2a10f34cad8ce83ea42c93bb75 - name: github.com/jmhodges/levigo version: c42d9e0ca023e2198120196f842701bb4c55d7b9 +- name: github.com/kr/logfmt + version: b84e30acd515aadc4b783ad4ff83aff3299bdfe0 +- name: github.com/magiconair/properties + version: 51463bfca2576e06c62a8504b5c0f06d61312647 - name: github.com/mattn/go-colorable version: d228849504861217f796da67fae4f6e347643f15 - name: github.com/mattn/go-isatty version: 30a891c33c7cde7b02a981314b4228ec99380cca +- name: github.com/mitchellh/mapstructure + version: 53818660ed4955e899c0bcafa97299a388bd7c8e +- name: github.com/pelletier/go-buffruneio + version: c37440a7cf42ac63b919c752ca73a85067e05992 +- name: github.com/pelletier/go-toml + version: 13d49d4606eb801b8f01ae542b4afc4c6ee3d84a +- name: github.com/pkg/errors + version: bfd5150e4e41705ded2129ec33379de1cb90b513 +- name: github.com/spf13/afero + version: 9be650865eab0c12963d8753212f4f9c66cdcf12 + subpackages: + - mem +- name: github.com/spf13/cast + version: acbeb36b902d72a7a4c18e8f3241075e7ab763e4 +- name: github.com/spf13/cobra + version: fcd0c5a1df88f5d6784cb4feead962c3f3d0b66c +- name: github.com/spf13/jwalterweatherman + version: fa7ca7e836cf3a8bb4ebf799f472c12d7e903d66 +- name: github.com/spf13/pflag + version: 9ff6c6923cfffbcd502984b8e0c80539a94968b7 +- name: github.com/spf13/viper + version: 5d46e70da8c0b6f812e0b170b7a985753b5c63cb - name: github.com/syndtr/goleveldb version: 23851d93a2292dcc56e71a18ec9e0624d84a0f65 subpackages: @@ -27,7 +76,10 @@ imports: - leveldb/table - leveldb/util - name: github.com/tendermint/go-wire - version: 4325edc613ad1e9286c8bb770ed40ad3fe647e6c + version: 334005c236d19c632fb5f073f9de3b0fab6a522b + subpackages: + - data + - data/base58 - name: github.com/tendermint/log15 version: ae0f3d6450da9eac7074b439c8e1c3cabf0d5ce6 subpackages: @@ -40,6 +92,13 @@ imports: version: d75a52659825e75fff6158388dddc6a5b04f9ba5 subpackages: - unix +- name: golang.org/x/text + version: f4b4367115ec2de254587813edaa901bc1c723a8 + subpackages: + - transform + - unicode/norm +- name: gopkg.in/yaml.v2 + version: cd8b52f8269e0feb286dfeef29f8fe4d5b397e0b testImports: - name: github.com/davecgh/go-spew version: 6d212800a42e8ab5c146b8ace3490ee17e5225f9 @@ -53,3 +112,4 @@ testImports: version: 69483b4bd14f5845b5a1e55bca19e954e827f1d0 subpackages: - assert + - require diff --git a/glide.yaml b/glide.yaml index a4c5dd2b6..b0f22d1a7 100644 --- a/glide.yaml +++ b/glide.yaml @@ -11,6 +11,9 @@ import: - package: golang.org/x/crypto subpackages: - ripemd160 +- package: github.com/go-logfmt/logfmt +- package: github.com/spf13/cobra +- package: github.com/spf13/viper testImport: - package: github.com/stretchr/testify subpackages: From ef3b9610a17f4d87d5cb03bfe9d69f1d416e1ce3 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 3 May 2017 10:37:25 +0200 Subject: [PATCH 09/14] Fixed up the --home flag, ebuchman check this out --- cli/setup.go | 8 ++++++-- cli/setup_test.go | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cli/setup.go b/cli/setup.go index 8120449c7..b6f006583 100644 --- a/cli/setup.go +++ b/cli/setup.go @@ -24,7 +24,8 @@ const ( func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defautRoot string) func() { cobra.OnInitialize(func() { initEnv(envPrefix) }) cmd.PersistentFlags().StringP(RootFlag, "r", defautRoot, "DEPRECATED. Use --home") - cmd.PersistentFlags().StringP(HomeFlag, "h", defautRoot, "root directory for config and data") + // -h is already reserved for --help as part of the cobra framework + cmd.PersistentFlags().String(HomeFlag, "", "root directory for config and data") cmd.PersistentPreRunE = concatCobraCmdFuncs(bindFlagsLoadViper, cmd.PersistentPreRunE) return func() { execute(cmd) } } @@ -104,7 +105,10 @@ func bindFlagsLoadViper(cmd *cobra.Command, args []string) error { // rootDir is command line flag, env variable, or default $HOME/.tlc // NOTE: we support both --root and --home for now, but eventually only --home rootDir := viper.GetString(HomeFlag) - if !viper.IsSet(HomeFlag) && viper.IsSet(RootFlag) { + // @ebuchman: viper.IsSet doesn't do what you think... + // Even a default of "" on the pflag marks it as set, + // simply by fact of having a pflag. + if rootDir == "" { rootDir = viper.GetString(RootFlag) } viper.SetConfigName("config") // name of config file (without extension) diff --git a/cli/setup_test.go b/cli/setup_test.go index fb8e5655c..47170fe21 100644 --- a/cli/setup_test.go +++ b/cli/setup_test.go @@ -103,6 +103,9 @@ func TestSetupConfig(t *testing.T) { {nil, map[string]string{"RD_BOO": "bang"}, "bang"}, {nil, map[string]string{"RD_ROOT": conf1}, cval1}, {nil, map[string]string{"RDROOT": conf2}, cval2}, + {nil, map[string]string{"RDHOME": conf1}, cval1}, + // and when both are set??? HOME wins every time! + {[]string{"--root", conf1}, map[string]string{"RDHOME": conf2}, cval2}, } for idx, tc := range cases { From 8efeeb5f38e8647dcb1f162f3f1e58500c02f0ed Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 3 May 2017 11:25:07 +0200 Subject: [PATCH 10/14] Add --debug flag to return full stack trace on error --- cli/setup.go | 39 +++++++++++++++++++++++------- cli/setup_test.go | 61 +++++++++++++++++++++++++++++++++++++---------- 2 files changed, 80 insertions(+), 20 deletions(-) diff --git a/cli/setup.go b/cli/setup.go index b6f006583..e55baf902 100644 --- a/cli/setup.go +++ b/cli/setup.go @@ -16,25 +16,36 @@ import ( const ( RootFlag = "root" HomeFlag = "home" + DebugFlag = "debug" OutputFlag = "output" EncodingFlag = "encoding" ) +// Executable is the minimal interface to *corba.Command, so we can +// wrap if desired before the test +type Executable interface { + Execute() error +} + // PrepareBaseCmd is meant for tendermint and other servers -func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defautRoot string) func() { +func PrepareBaseCmd(cmd *cobra.Command, envPrefix, defautRoot string) Executor { cobra.OnInitialize(func() { initEnv(envPrefix) }) cmd.PersistentFlags().StringP(RootFlag, "r", defautRoot, "DEPRECATED. Use --home") // -h is already reserved for --help as part of the cobra framework + // do you want to try something else?? + // also, default must be empty, so we can detect this unset and fall back + // to --root / TM_ROOT / TMROOT cmd.PersistentFlags().String(HomeFlag, "", "root directory for config and data") + cmd.PersistentFlags().Bool(DebugFlag, false, "print out full stack trace on errors") cmd.PersistentPreRunE = concatCobraCmdFuncs(bindFlagsLoadViper, cmd.PersistentPreRunE) - return func() { execute(cmd) } + return Executor{cmd} } // 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, defautRoot string) func() { +func PrepareMainCmd(cmd *cobra.Command, envPrefix, defautRoot 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(setEncoding, validateOutput, cmd.PersistentPreRunE) @@ -68,14 +79,26 @@ func copyEnvVars(prefix string) { } } +// Executor wraps the cobra Command with a nicer Execute method +type Executor struct { + *cobra.Command +} + // 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 execute(cmd *cobra.Command) { - // TODO: this can do something cooler with debug and log-levels - if err := cmd.Execute(); err != nil { - fmt.Println(err) - os.Exit(-1) +func (e Executor) Execute() error { + e.SilenceUsage = true + e.SilenceErrors = true + err := e.Command.Execute() + if err != nil { + // TODO: something cooler with log-levels + if viper.GetBool(DebugFlag) { + fmt.Printf("ERROR: %+v\n", err) + } else { + fmt.Println("ERROR:", err.Error()) + } } + return err } type cobraCmdFunc func(cmd *cobra.Command, args []string) error diff --git a/cli/setup_test.go b/cli/setup_test.go index 47170fe21..d8e37d3a4 100644 --- a/cli/setup_test.go +++ b/cli/setup_test.go @@ -8,20 +8,16 @@ import ( "os" "path/filepath" "strconv" + "strings" "testing" + "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// Executable is the minimal interface to *corba.Command, so we can -// wrap if desired before the test -type Executable interface { - Execute() error -} - func TestSetupEnv(t *testing.T) { assert, require := assert.New(t), require.New(t) @@ -46,15 +42,15 @@ func TestSetupEnv(t *testing.T) { i := strconv.Itoa(idx) // test command that store value of foobar in local variable var foo string - cmd := &cobra.Command{ + demo := &cobra.Command{ Use: "demo", RunE: func(cmd *cobra.Command, args []string) error { foo = viper.GetString("foobar") return nil }, } - cmd.Flags().String("foobar", "", "Some test value from config") - PrepareBaseCmd(cmd, "DEMO", "/qwerty/asdfgh") // some missing dir.. + demo.Flags().String("foobar", "", "Some test value from config") + cmd := PrepareBaseCmd(demo, "DEMO", "/qwerty/asdfgh") // some missing dir.. viper.Reset() args := append([]string{cmd.Use}, tc.args...) @@ -112,15 +108,15 @@ func TestSetupConfig(t *testing.T) { i := strconv.Itoa(idx) // test command that store value of foobar in local variable var foo string - cmd := &cobra.Command{ + boo := &cobra.Command{ Use: "reader", RunE: func(cmd *cobra.Command, args []string) error { foo = viper.GetString("boo") return nil }, } - cmd.Flags().String("boo", "", "Some test value from config") - PrepareBaseCmd(cmd, "RD", "/qwerty/asdfgh") // some missing dir... + boo.Flags().String("boo", "", "Some test value from config") + cmd := PrepareBaseCmd(boo, "RD", "/qwerty/asdfgh") // some missing dir... viper.Reset() args := append([]string{cmd.Use}, tc.args...) @@ -130,6 +126,47 @@ func TestSetupConfig(t *testing.T) { } } +func TestSetupDebug(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + cases := []struct { + args []string + env map[string]string + long bool + expected string + }{ + {nil, nil, false, "Debug flag = false"}, + {[]string{"--debug"}, nil, true, "Debug flag = true"}, + {[]string{"--no-such-flag"}, nil, false, "unknown flag: --no-such-flag"}, + {nil, map[string]string{"DBG_DEBUG": "true"}, true, "Debug flag = true"}, + } + + for idx, tc := range cases { + i := strconv.Itoa(idx) + // test command that store value of foobar in local variable + debug := &cobra.Command{ + Use: "debug", + RunE: func(cmd *cobra.Command, args []string) error { + return errors.Errorf("Debug flag = %t", viper.GetBool(DebugFlag)) + }, + } + cmd := PrepareBaseCmd(debug, "DBG", "/qwerty/asdfgh") // some missing dir.. + + viper.Reset() + args := append([]string{cmd.Use}, tc.args...) + out, err := runCaptureWithArgs(cmd, args, tc.env) + require.NotNil(err, i) + msg := strings.Split(out, "\n") + desired := fmt.Sprintf("ERROR: %s", tc.expected) + assert.Equal(desired, msg[0], i) + if tc.long && assert.True(len(msg) > 2, i) { + // the next line starts the stack trace... + assert.Contains(msg[1], "TestSetupDebug", i) + assert.Contains(msg[2], "setup_test.go", i) + } + } +} + // 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 { From ee45dbdc8b7947e95f403b736711ff8000f2927d Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Thu, 4 May 2017 19:16:58 +0200 Subject: [PATCH 11/14] Test how unmarshall plays with flags/env/config/default struct --- cli/setup_test.go | 77 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/cli/setup_test.go b/cli/setup_test.go index d8e37d3a4..169b14004 100644 --- a/cli/setup_test.go +++ b/cli/setup_test.go @@ -126,6 +126,83 @@ func TestSetupConfig(t *testing.T) { } } +type DemoConfig struct { + Name string `mapstructure:"name"` + Age int `mapstructure:"age"` + Unused int `mapstructure:"unused"` +} + +func TestSetupUnmarshal(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + // we pre-create two config files we can refer to in the rest of + // the test cases. + cval1, cval2 := "someone", "else" + conf1, err := writeConfig(map[string]string{"name": cval1}) + require.Nil(err) + // even with some ignored fields, should be no problem + conf2, err := writeConfig(map[string]string{"name": cval2, "foo": "bar"}) + require.Nil(err) + + // unused is not declared on a flag and remains from base + base := DemoConfig{ + Name: "default", + Age: 42, + Unused: -7, + } + c := func(name string, age int) DemoConfig { + r := base + // anything set on the flags as a default is used over + // the default config object + r.Name = "from-flag" + if name != "" { + r.Name = name + } + if age != 0 { + r.Age = age + } + return r + } + + cases := []struct { + args []string + env map[string]string + expected DemoConfig + }{ + {nil, nil, c("", 0)}, + // setting on the command line + {[]string{"--name", "haha"}, nil, c("haha", 0)}, + {[]string{"--root", conf1}, nil, c(cval1, 0)}, + // test both variants of the prefix + {nil, map[string]string{"MR_AGE": "56"}, c("", 56)}, + {nil, map[string]string{"MR_ROOT": conf1}, c(cval1, 0)}, + {[]string{"--age", "17"}, map[string]string{"MRHOME": conf2}, c(cval2, 17)}, + } + + for idx, tc := range cases { + i := strconv.Itoa(idx) + // test command that store value of foobar in local variable + cfg := base + marsh := &cobra.Command{ + Use: "marsh", + RunE: func(cmd *cobra.Command, args []string) error { + return viper.Unmarshal(&cfg) + }, + } + marsh.Flags().String("name", "from-flag", "Some test value from config") + // if we want a flag to use the proper default, then copy it + // 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... + + viper.Reset() + args := append([]string{cmd.Use}, tc.args...) + err := runWithArgs(cmd, args, tc.env) + require.Nil(err, i) + assert.Equal(tc.expected, cfg, i) + } +} + func TestSetupDebug(t *testing.T) { assert, require := assert.New(t), require.New(t) From 3585a542a0e07d0e9d396b2f809c2826c8536437 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Fri, 5 May 2017 00:48:23 -0400 Subject: [PATCH 12/14] cli: viper.Set(HomeFlag, rootDir) --- cli/setup.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/setup.go b/cli/setup.go index e55baf902..7a4a2098e 100644 --- a/cli/setup.go +++ b/cli/setup.go @@ -127,12 +127,12 @@ func bindFlagsLoadViper(cmd *cobra.Command, args []string) error { // rootDir is command line flag, env variable, or default $HOME/.tlc // NOTE: we support both --root and --home for now, but eventually only --home + // Also ensure we set the correct rootDir under HomeFlag so we dont need to + // repeat this logic elsewhere. rootDir := viper.GetString(HomeFlag) - // @ebuchman: viper.IsSet doesn't do what you think... - // Even a default of "" on the pflag marks it as set, - // simply by fact of having a pflag. if rootDir == "" { rootDir = viper.GetString(RootFlag) + viper.Set(HomeFlag, rootDir) } viper.SetConfigName("config") // name of config file (without extension) viper.AddConfigPath(rootDir) // search root directory From d0132b0fffc2eb79c7a129a2faeae08cc0e5fcad Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 5 May 2017 14:46:04 +0200 Subject: [PATCH 13/14] Moved helper functions into non-test code for reuse elsewhere --- .gitignore | 1 + cli/helper.go | 64 ++++++++++++++++++++++++++++++++++++++++++++ cli/setup_test.go | 68 +++-------------------------------------------- 3 files changed, 69 insertions(+), 64 deletions(-) create mode 100644 cli/helper.go diff --git a/.gitignore b/.gitignore index 381931381..f37225baa 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ *.swp *.swo +vendor diff --git a/cli/helper.go b/cli/helper.go new file mode 100644 index 000000000..b0662c78d --- /dev/null +++ b/cli/helper.go @@ -0,0 +1,64 @@ +package cli + +import ( + "bytes" + "io" + "os" +) + +// 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 { + oargs := os.Args + oenv := map[string]string{} + // defer returns the environment back to normal + defer func() { + os.Args = oargs + for k, v := range oenv { + os.Setenv(k, v) + } + }() + + // set the args and env how we want them + os.Args = args + for k, v := range env { + // backup old value if there, to restore at end + oenv[k] = os.Getenv(k) + err := os.Setenv(k, v) + if err != nil { + return err + } + } + + // and finally run the command + return cmd.Execute() +} + +// RunCaptureWithArgs executes the given command with the specified command line args +// and environmental variables set. It returns whatever was writen to +// stdout along with any error returned from cmd.Execute() +func RunCaptureWithArgs(cmd Executable, args []string, env map[string]string) (output string, err error) { + old := os.Stdout // keep backup of the real stdout + r, w, _ := os.Pipe() + os.Stdout = w + defer func() { + os.Stdout = old // restoring the real stdout + }() + + outC := make(chan string) + // copy the output in a separate goroutine so printing can't block indefinitely + go func() { + var buf bytes.Buffer + // io.Copy will end when we call w.Close() below + io.Copy(&buf, r) + outC <- buf.String() + }() + + // now run the command + err = RunWithArgs(cmd, args, env) + + // and grab the stdout to return + w.Close() + output = <-outC + return output, err +} diff --git a/cli/setup_test.go b/cli/setup_test.go index 169b14004..34877209d 100644 --- a/cli/setup_test.go +++ b/cli/setup_test.go @@ -1,11 +1,8 @@ package cli import ( - "bytes" "fmt" - "io" "io/ioutil" - "os" "path/filepath" "strconv" "strings" @@ -54,7 +51,7 @@ func TestSetupEnv(t *testing.T) { viper.Reset() args := append([]string{cmd.Use}, tc.args...) - err := runWithArgs(cmd, args, tc.env) + err := RunWithArgs(cmd, args, tc.env) require.Nil(err, i) assert.Equal(tc.expected, foo, i) } @@ -120,7 +117,7 @@ func TestSetupConfig(t *testing.T) { viper.Reset() args := append([]string{cmd.Use}, tc.args...) - err := runWithArgs(cmd, args, tc.env) + err := RunWithArgs(cmd, args, tc.env) require.Nil(err, i) assert.Equal(tc.expected, foo, i) } @@ -197,7 +194,7 @@ func TestSetupUnmarshal(t *testing.T) { viper.Reset() args := append([]string{cmd.Use}, tc.args...) - err := runWithArgs(cmd, args, tc.env) + err := RunWithArgs(cmd, args, tc.env) require.Nil(err, i) assert.Equal(tc.expected, cfg, i) } @@ -231,7 +228,7 @@ func TestSetupDebug(t *testing.T) { viper.Reset() args := append([]string{cmd.Use}, tc.args...) - out, err := runCaptureWithArgs(cmd, args, tc.env) + out, err := RunCaptureWithArgs(cmd, args, tc.env) require.NotNil(err, i) msg := strings.Split(out, "\n") desired := fmt.Sprintf("ERROR: %s", tc.expected) @@ -243,60 +240,3 @@ func TestSetupDebug(t *testing.T) { } } } - -// 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 { - oargs := os.Args - oenv := map[string]string{} - // defer returns the environment back to normal - defer func() { - os.Args = oargs - for k, v := range oenv { - os.Setenv(k, v) - } - }() - - // set the args and env how we want them - os.Args = args - for k, v := range env { - // backup old value if there, to restore at end - oenv[k] = os.Getenv(k) - err := os.Setenv(k, v) - if err != nil { - return err - } - } - - // and finally run the command - return cmd.Execute() -} - -// runCaptureWithArgs executes the given command with the specified command line args -// and environmental variables set. It returns whatever was writen to -// stdout along with any error returned from cmd.Execute() -func runCaptureWithArgs(cmd Executable, args []string, env map[string]string) (output string, err error) { - old := os.Stdout // keep backup of the real stdout - r, w, _ := os.Pipe() - os.Stdout = w - defer func() { - os.Stdout = old // restoring the real stdout - }() - - outC := make(chan string) - // copy the output in a separate goroutine so printing can't block indefinitely - go func() { - var buf bytes.Buffer - // io.Copy will end when we call w.Close() below - io.Copy(&buf, r) - outC <- buf.String() - }() - - // now run the command - err = runWithArgs(cmd, args, env) - - // and grab the stdout to return - w.Close() - output = <-outC - return output, err -} From 2f02ed18e9b706467c9474d024a25a0b7a9c0e97 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 5 May 2017 14:58:53 +0200 Subject: [PATCH 14/14] One more helper function for cli tests... --- cli/helper.go | 20 ++++++++++++++++++++ cli/setup_test.go | 24 ++++-------------------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/cli/helper.go b/cli/helper.go index b0662c78d..79654bc34 100644 --- a/cli/helper.go +++ b/cli/helper.go @@ -2,10 +2,30 @@ package cli import ( "bytes" + "fmt" "io" + "io/ioutil" "os" + "path/filepath" ) +// WriteDemoConfig writes a toml file with the given values. +// It returns the RootDir the config.toml file is stored in, +// or an error if writing was impossible +func WriteDemoConfig(vals map[string]string) (string, error) { + cdir, err := ioutil.TempDir("", "test-cli") + if err != nil { + return "", err + } + data := "" + for k, v := range vals { + data = data + fmt.Sprintf("%s = \"%s\"\n", k, v) + } + cfile := filepath.Join(cdir, "config.toml") + err = ioutil.WriteFile(cfile, []byte(data), 0666) + return cdir, err +} + // 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 { diff --git a/cli/setup_test.go b/cli/setup_test.go index 34877209d..6396b769e 100644 --- a/cli/setup_test.go +++ b/cli/setup_test.go @@ -2,8 +2,6 @@ package cli import ( "fmt" - "io/ioutil" - "path/filepath" "strconv" "strings" "testing" @@ -57,30 +55,16 @@ func TestSetupEnv(t *testing.T) { } } -func writeConfig(vals map[string]string) (string, error) { - cdir, err := ioutil.TempDir("", "test-cli") - if err != nil { - return "", err - } - data := "" - for k, v := range vals { - data = data + fmt.Sprintf("%s = \"%s\"\n", k, v) - } - cfile := filepath.Join(cdir, "config.toml") - err = ioutil.WriteFile(cfile, []byte(data), 0666) - return cdir, err -} - func TestSetupConfig(t *testing.T) { assert, require := assert.New(t), require.New(t) // we pre-create two config files we can refer to in the rest of // the test cases. cval1, cval2 := "fubble", "wubble" - conf1, err := writeConfig(map[string]string{"boo": cval1}) + conf1, err := WriteDemoConfig(map[string]string{"boo": cval1}) require.Nil(err) // even with some ignored fields, should be no problem - conf2, err := writeConfig(map[string]string{"boo": cval2, "foo": "bar"}) + conf2, err := WriteDemoConfig(map[string]string{"boo": cval2, "foo": "bar"}) require.Nil(err) cases := []struct { @@ -135,10 +119,10 @@ func TestSetupUnmarshal(t *testing.T) { // we pre-create two config files we can refer to in the rest of // the test cases. cval1, cval2 := "someone", "else" - conf1, err := writeConfig(map[string]string{"name": cval1}) + conf1, err := WriteDemoConfig(map[string]string{"name": cval1}) require.Nil(err) // even with some ignored fields, should be no problem - conf2, err := writeConfig(map[string]string{"name": cval2, "foo": "bar"}) + conf2, err := WriteDemoConfig(map[string]string{"name": cval2, "foo": "bar"}) require.Nil(err) // unused is not declared on a flag and remains from base