From ae16aa41803537b5516f40f0423fb668d36b4b66 Mon Sep 17 00:00:00 2001 From: Matt Kocubinski Date: Wed, 30 Oct 2024 10:35:07 -0500 Subject: [PATCH 1/2] fix(server/v2): return ErrHelp (#22399) (cherry picked from commit 470e0859462b28a53adb411843539561d11d7bf5) # Conflicts: # server/v2/command_factory.go --- server/v2/command_factory.go | 185 ++++++++++++++++++++++++++++++ simapp/v2/simdv2/cmd/root_di.go | 6 + simapp/v2/simdv2/cmd/root_test.go | 23 ++++ 3 files changed, 214 insertions(+) create mode 100644 server/v2/command_factory.go diff --git a/server/v2/command_factory.go b/server/v2/command_factory.go new file mode 100644 index 000000000000..999f3565c22e --- /dev/null +++ b/server/v2/command_factory.go @@ -0,0 +1,185 @@ +package serverv2 + +import ( + "errors" + "io" + "os" + "path/filepath" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/spf13/viper" + + "cosmossdk.io/core/server" + "cosmossdk.io/log" +) + +// CommandFactory is a factory help create server/v2 root commands. +// For example usage see simapp/v2/cmd/root_di.go +type CommandFactory struct { + defaultHomeDir string + envPrefix string + configWriter ConfigWriter + loggerFactory func(server.ConfigMap, io.Writer) (log.Logger, error) + + logger log.Logger + // TODO remove this field + // this viper handle is kept because certain commands in server/v2 fetch a viper instance + // from the command context in order to read the config. + // After merging #22267 this is no longer required, and server.ConfigMap can be used instead. + // See issue #22388 + vipr *viper.Viper +} + +type CommandFactoryOption func(*CommandFactory) error + +// NewCommandFactory creates a new CommandFactory with the given options. +func NewCommandFactory(opts ...CommandFactoryOption) (*CommandFactory, error) { + f := &CommandFactory{} + for _, opt := range opts { + err := opt(f) + if err != nil { + return nil, err + } + } + return f, nil +} + +// WithEnvPrefix sets the environment variable prefix for the command factory. +func WithEnvPrefix(envPrefix string) CommandFactoryOption { + return func(f *CommandFactory) error { + f.envPrefix = envPrefix + return nil + } +} + +// WithStdDefaultHomeDir sets the server's default home directory `user home directory`/`defaultHomeBasename`. +func WithStdDefaultHomeDir(defaultHomeBasename string) CommandFactoryOption { + return func(f *CommandFactory) error { + // get the home directory from the environment variable + // to not clash with the $HOME system variable, when no prefix is set + // we check the NODE_HOME environment variable + homeDir, envHome := "", "HOME" + if len(f.envPrefix) > 0 { + homeDir = os.Getenv(f.envPrefix + "_" + envHome) + } else { + homeDir = os.Getenv("NODE_" + envHome) + } + if homeDir != "" { + f.defaultHomeDir = filepath.Clean(homeDir) + return nil + } + + // get user home directory + userHomeDir, err := os.UserHomeDir() + if err != nil { + return err + } + + f.defaultHomeDir = filepath.Join(userHomeDir, defaultHomeBasename) + return nil + } +} + +// WithDefaultHomeDir sets the server's default home directory. +func WithDefaultHomeDir(homedir string) CommandFactoryOption { + return func(f *CommandFactory) error { + f.defaultHomeDir = homedir + return nil + } +} + +// WithConfigWriter sets the config writer for the command factory. +// If set the config writer will be used to write TOML config files during ParseCommand invocations. +func WithConfigWriter(configWriter ConfigWriter) CommandFactoryOption { + return func(f *CommandFactory) error { + f.configWriter = configWriter + return nil + } +} + +// WithLoggerFactory sets the logger factory for the command factory. +func WithLoggerFactory(loggerFactory func(server.ConfigMap, io.Writer) (log.Logger, error)) CommandFactoryOption { + return func(f *CommandFactory) error { + f.loggerFactory = loggerFactory + return nil + } +} + +// enhanceCommand adds the following flags to the command: +// +// --log-level: The logging level (trace|debug|info|warn|error|fatal|panic|disabled or '*:,:') +// --log-format: The logging format (json|plain) +// --log-no-color: Disable colored logs +// --home: directory for config and data +// +// It also sets the environment variable prefix for the viper instance. +func (f *CommandFactory) enhanceCommand(cmd *cobra.Command) { + pflags := cmd.PersistentFlags() + pflags.String(FlagLogLevel, "info", "The logging level (trace|debug|info|warn|error|fatal|panic|disabled or '*:,:')") + pflags.String(FlagLogFormat, "plain", "The logging format (json|plain)") + pflags.Bool(FlagLogNoColor, false, "Disable colored logs") + pflags.StringP(FlagHome, "", f.defaultHomeDir, "directory for config and data") + viper.SetEnvPrefix(f.envPrefix) + viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_", "-", "_")) + viper.AutomaticEnv() +} + +// EnhanceRootCommand sets the viper and logger in the command context. +func (f *CommandFactory) EnhanceRootCommand(cmd *cobra.Command) { + f.enhanceCommand(cmd) + SetCmdServerContext(cmd, f.vipr, f.logger) +} + +// ParseCommand parses args against the input rootCmd CLI skeleton then returns the target subcommand, +// a fully realized config map, and a properly configured logger. +// If `WithConfigWriter` was set in the factory options, the config writer will be used to write the app.toml file. +// Internally a viper instance is created and used to bind the flags to the config map. +// Future invocations of EnhanceCommandContext will set the viper instance and logger in the command context. +func (f *CommandFactory) ParseCommand( + rootCmd *cobra.Command, + args []string, +) (*cobra.Command, server.ConfigMap, log.Logger, error) { + f.enhanceCommand(rootCmd) + cmd, _, err := rootCmd.Traverse(args) + if err != nil { + return nil, nil, nil, err + } + if err = cmd.ParseFlags(args); err != nil { + // help requested, return the command early + if errors.Is(err, pflag.ErrHelp) { + return cmd, nil, nil, err + } + return nil, nil, nil, err + } + home, err := cmd.Flags().GetString(FlagHome) + if err != nil { + return nil, nil, nil, err + } + configDir := filepath.Join(home, "config") + if f.configWriter != nil { + // create app.toml if it does not already exist + if _, err = os.Stat(filepath.Join(configDir, "app.toml")); os.IsNotExist(err) { + if err = f.configWriter.WriteConfig(configDir); err != nil { + return nil, nil, nil, err + } + } + } + f.vipr, err = ReadConfig(configDir) + if err != nil { + return nil, nil, nil, err + } + if err = f.vipr.BindPFlags(cmd.Flags()); err != nil { + return nil, nil, nil, err + } + + if f.loggerFactory != nil { + f.logger, err = f.loggerFactory(f.vipr.AllSettings(), cmd.OutOrStdout()) + if err != nil { + return nil, nil, nil, err + } + } + + return cmd, f.vipr.AllSettings(), f.logger, nil +} diff --git a/simapp/v2/simdv2/cmd/root_di.go b/simapp/v2/simdv2/cmd/root_di.go index 0bbae702becb..d1e3f62c5e12 100644 --- a/simapp/v2/simdv2/cmd/root_di.go +++ b/simapp/v2/simdv2/cmd/root_di.go @@ -1,7 +1,10 @@ package cmd import ( + "errors" + "github.com/spf13/cobra" + "github.com/spf13/pflag" autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" "cosmossdk.io/client/v2/autocli" @@ -38,6 +41,9 @@ func NewRootCmd[T transaction.Tx]( subCommand, configMap, logger, err := factory.ParseCommand(rootCommand, args) if err != nil { + if errors.Is(err, pflag.ErrHelp) { + return rootCommand, nil + } return nil, err } diff --git a/simapp/v2/simdv2/cmd/root_test.go b/simapp/v2/simdv2/cmd/root_test.go index 7f7d6a07d95a..687e8aa510f0 100644 --- a/simapp/v2/simdv2/cmd/root_test.go +++ b/simapp/v2/simdv2/cmd/root_test.go @@ -1,6 +1,7 @@ package cmd_test import ( + "bytes" "fmt" "testing" @@ -42,3 +43,25 @@ func TestHomeFlagRegistration(t *testing.T) { require.NoError(t, err) require.Equal(t, result, homeDir) } + +func TestHelpRequested(t *testing.T) { + argz := [][]string{ + {"query", "--help"}, + {"query", "tx", "-h"}, + {"--help"}, + {"start", "-h"}, + } + + for _, args := range argz { + rootCmd, err := cmd.NewRootCmd[transaction.Tx](args...) + require.NoError(t, err) + + var out bytes.Buffer + rootCmd.SetArgs(args) + rootCmd.SetOut(&out) + require.NoError(t, rootCmd.Execute()) + require.Contains(t, out.String(), args[0]) + require.Contains(t, out.String(), "--help") + require.Contains(t, out.String(), "Usage:") + } +} From 4d0fd7f800662b34d60deed11eadcf16ee9ce4a4 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 31 Oct 2024 09:19:53 +0100 Subject: [PATCH 2/2] Delete server/v2/command_factory.go --- server/v2/command_factory.go | 185 ----------------------------------- 1 file changed, 185 deletions(-) delete mode 100644 server/v2/command_factory.go diff --git a/server/v2/command_factory.go b/server/v2/command_factory.go deleted file mode 100644 index 999f3565c22e..000000000000 --- a/server/v2/command_factory.go +++ /dev/null @@ -1,185 +0,0 @@ -package serverv2 - -import ( - "errors" - "io" - "os" - "path/filepath" - "strings" - - "github.com/spf13/cobra" - "github.com/spf13/pflag" - "github.com/spf13/viper" - - "cosmossdk.io/core/server" - "cosmossdk.io/log" -) - -// CommandFactory is a factory help create server/v2 root commands. -// For example usage see simapp/v2/cmd/root_di.go -type CommandFactory struct { - defaultHomeDir string - envPrefix string - configWriter ConfigWriter - loggerFactory func(server.ConfigMap, io.Writer) (log.Logger, error) - - logger log.Logger - // TODO remove this field - // this viper handle is kept because certain commands in server/v2 fetch a viper instance - // from the command context in order to read the config. - // After merging #22267 this is no longer required, and server.ConfigMap can be used instead. - // See issue #22388 - vipr *viper.Viper -} - -type CommandFactoryOption func(*CommandFactory) error - -// NewCommandFactory creates a new CommandFactory with the given options. -func NewCommandFactory(opts ...CommandFactoryOption) (*CommandFactory, error) { - f := &CommandFactory{} - for _, opt := range opts { - err := opt(f) - if err != nil { - return nil, err - } - } - return f, nil -} - -// WithEnvPrefix sets the environment variable prefix for the command factory. -func WithEnvPrefix(envPrefix string) CommandFactoryOption { - return func(f *CommandFactory) error { - f.envPrefix = envPrefix - return nil - } -} - -// WithStdDefaultHomeDir sets the server's default home directory `user home directory`/`defaultHomeBasename`. -func WithStdDefaultHomeDir(defaultHomeBasename string) CommandFactoryOption { - return func(f *CommandFactory) error { - // get the home directory from the environment variable - // to not clash with the $HOME system variable, when no prefix is set - // we check the NODE_HOME environment variable - homeDir, envHome := "", "HOME" - if len(f.envPrefix) > 0 { - homeDir = os.Getenv(f.envPrefix + "_" + envHome) - } else { - homeDir = os.Getenv("NODE_" + envHome) - } - if homeDir != "" { - f.defaultHomeDir = filepath.Clean(homeDir) - return nil - } - - // get user home directory - userHomeDir, err := os.UserHomeDir() - if err != nil { - return err - } - - f.defaultHomeDir = filepath.Join(userHomeDir, defaultHomeBasename) - return nil - } -} - -// WithDefaultHomeDir sets the server's default home directory. -func WithDefaultHomeDir(homedir string) CommandFactoryOption { - return func(f *CommandFactory) error { - f.defaultHomeDir = homedir - return nil - } -} - -// WithConfigWriter sets the config writer for the command factory. -// If set the config writer will be used to write TOML config files during ParseCommand invocations. -func WithConfigWriter(configWriter ConfigWriter) CommandFactoryOption { - return func(f *CommandFactory) error { - f.configWriter = configWriter - return nil - } -} - -// WithLoggerFactory sets the logger factory for the command factory. -func WithLoggerFactory(loggerFactory func(server.ConfigMap, io.Writer) (log.Logger, error)) CommandFactoryOption { - return func(f *CommandFactory) error { - f.loggerFactory = loggerFactory - return nil - } -} - -// enhanceCommand adds the following flags to the command: -// -// --log-level: The logging level (trace|debug|info|warn|error|fatal|panic|disabled or '*:,:') -// --log-format: The logging format (json|plain) -// --log-no-color: Disable colored logs -// --home: directory for config and data -// -// It also sets the environment variable prefix for the viper instance. -func (f *CommandFactory) enhanceCommand(cmd *cobra.Command) { - pflags := cmd.PersistentFlags() - pflags.String(FlagLogLevel, "info", "The logging level (trace|debug|info|warn|error|fatal|panic|disabled or '*:,:')") - pflags.String(FlagLogFormat, "plain", "The logging format (json|plain)") - pflags.Bool(FlagLogNoColor, false, "Disable colored logs") - pflags.StringP(FlagHome, "", f.defaultHomeDir, "directory for config and data") - viper.SetEnvPrefix(f.envPrefix) - viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_", "-", "_")) - viper.AutomaticEnv() -} - -// EnhanceRootCommand sets the viper and logger in the command context. -func (f *CommandFactory) EnhanceRootCommand(cmd *cobra.Command) { - f.enhanceCommand(cmd) - SetCmdServerContext(cmd, f.vipr, f.logger) -} - -// ParseCommand parses args against the input rootCmd CLI skeleton then returns the target subcommand, -// a fully realized config map, and a properly configured logger. -// If `WithConfigWriter` was set in the factory options, the config writer will be used to write the app.toml file. -// Internally a viper instance is created and used to bind the flags to the config map. -// Future invocations of EnhanceCommandContext will set the viper instance and logger in the command context. -func (f *CommandFactory) ParseCommand( - rootCmd *cobra.Command, - args []string, -) (*cobra.Command, server.ConfigMap, log.Logger, error) { - f.enhanceCommand(rootCmd) - cmd, _, err := rootCmd.Traverse(args) - if err != nil { - return nil, nil, nil, err - } - if err = cmd.ParseFlags(args); err != nil { - // help requested, return the command early - if errors.Is(err, pflag.ErrHelp) { - return cmd, nil, nil, err - } - return nil, nil, nil, err - } - home, err := cmd.Flags().GetString(FlagHome) - if err != nil { - return nil, nil, nil, err - } - configDir := filepath.Join(home, "config") - if f.configWriter != nil { - // create app.toml if it does not already exist - if _, err = os.Stat(filepath.Join(configDir, "app.toml")); os.IsNotExist(err) { - if err = f.configWriter.WriteConfig(configDir); err != nil { - return nil, nil, nil, err - } - } - } - f.vipr, err = ReadConfig(configDir) - if err != nil { - return nil, nil, nil, err - } - if err = f.vipr.BindPFlags(cmd.Flags()); err != nil { - return nil, nil, nil, err - } - - if f.loggerFactory != nil { - f.logger, err = f.loggerFactory(f.vipr.AllSettings(), cmd.OutOrStdout()) - if err != nil { - return nil, nil, nil, err - } - } - - return cmd, f.vipr.AllSettings(), f.logger, nil -}