From f2d8496136d872accb0485851dff773000fe4128 Mon Sep 17 00:00:00 2001 From: Grzegorz Dlugoszewski Date: Thu, 4 Jun 2020 11:38:01 +0200 Subject: [PATCH] Refactor config provider using viper --- cmd/git-get/main.go | 11 +++- go.mod | 1 + pkg/config.go | 119 ++++++++++++++++++---------------------- pkg/config_test.go | 130 ++++++++++++++++++++++++-------------------- pkg/list.go | 5 +- pkg/url.go | 3 +- pkg/url_test.go | 3 + 7 files changed, 142 insertions(+), 130 deletions(-) diff --git a/cmd/git-get/main.go b/cmd/git-get/main.go index cbf446f..7b7be8c 100644 --- a/cmd/git-get/main.go +++ b/cmd/git-get/main.go @@ -6,6 +6,7 @@ import ( "os" "github.com/spf13/cobra" + "github.com/spf13/viper" ) var ( @@ -23,11 +24,17 @@ var cmd = &cobra.Command{ } var list bool +var reposRoot string func init() { - pkg.LoadConf() + // pkg.LoadConf() cmd.PersistentFlags().BoolVarP(&list, "list", "l", false, "Lists all repositories inside git-get root") + cmd.PersistentFlags().StringVarP(&reposRoot, "reposRoot", "r", "", "repos root") + viper.BindPFlag("reposRoot", cmd.PersistentFlags().Lookup("reposRoot")) + + pkg.InitConfig() + } func Run(cmd *cobra.Command, args []string) { @@ -45,7 +52,7 @@ func Run(cmd *cobra.Command, args []string) { url, err := pkg.ParseURL(args[0]) exitIfError(err) - _, err = pkg.CloneRepo(url, pkg.Cfg.ReposRoot(), false) + _, err = pkg.CloneRepo(url, viper.GetString(pkg.KeyReposRoot), false) exitIfError(err) } diff --git a/go.mod b/go.mod index b7bdf87..e695f69 100644 --- a/go.mod +++ b/go.mod @@ -9,5 +9,6 @@ require ( github.com/mitchellh/go-homedir v1.1.0 github.com/pkg/errors v0.9.1 github.com/spf13/cobra v1.0.0 + github.com/spf13/viper v1.4.0 github.com/xlab/treeprint v1.0.0 ) diff --git a/pkg/config.go b/pkg/config.go index c83eff0..07cb927 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -1,98 +1,85 @@ package pkg import ( - "os" "path" "strings" "github.com/go-git/go-git/v5/config" plumbing "github.com/go-git/go-git/v5/plumbing/format/config" "github.com/mitchellh/go-homedir" + "github.com/spf13/viper" ) const ( - CfgSection = "gitget" - CfgReposRoot = "reposRoot" - CfgDefaultHost = "defaultHost" - - EnvReposRoot = "GITGET_REPOSROOT" - EnvDefaultHost = "GITGET_DEFAULTHOST" - - DefaultReposRootSubpath = "repositories" - DefaultHost = "github.com" + GitgetPrefix = "gitget" + KeyReposRoot = "reposRoot" + DefReposRoot = "repositories" + KeyDefaultHost = "defaultHost" + DefDefaultHost = "github.com" ) -var Cfg *Conf - -// Conf provides methods for accessing configuration values -// Values are looked up in the following order: env variable, gitignore file, default value -type Conf struct { - gitconfig *config.Config +// gitconfig provides methods for looking up configiration values inside .gitconfig file +type gitconfig struct { + *config.Config } -func LoadConf() { - // We don't care if loading gitconfig file throws an error - // When gitconfig is nil, getters will just return the default values - gitconfig, _ := config.LoadConfig(config.GlobalScope) +// InitConfig initializes viper config registry. Values are looked up in the following order: cli flag, env variable, gitconfig file, default value +// Viper doesn't support gitconfig file format so it can't find missing values there automatically. They need to be specified in setMissingValues func. +func InitConfig() { + viper.SetEnvPrefix(strings.ToUpper(GitgetPrefix)) + viper.AutomaticEnv() - Cfg = &Conf{ - gitconfig: gitconfig, + cfg := loadGitconfig() + setMissingValues(cfg) +} + +// loadGitconfig loads configuration from a gitconfig file. +// We ignore errors when gitconfig file can't be found, opened or parsed. In those cases viper will provide default config values. +func loadGitconfig() *gitconfig { + // TODO: load system scope + cfg, _ := config.LoadConfig(config.GlobalScope) + + return &gitconfig{ + Config: cfg, } } -func (c *Conf) ReposRoot() string { - defReposRoot := path.Join(home(), DefaultReposRootSubpath) - - reposRoot := os.Getenv(EnvReposRoot) - if reposRoot != "" { - return reposRoot +// setMissingValues checks if config values are provided by flags or env vars. If not, it tries loading them from gitconfig file. +// If that fails, the default values are used. +func setMissingValues(cfg *gitconfig) { + if !viper.IsSet(KeyReposRoot) { + viper.Set(KeyReposRoot, cfg.get(KeyReposRoot, path.Join(home(), DefReposRoot))) } - if c == nil || c.gitconfig == nil { - return defReposRoot + if !viper.IsSet(KeyDefaultHost) { + viper.Set(KeyDefaultHost, cfg.get(KeyDefaultHost, DefDefaultHost)) + } +} + +// get looks up the value for a given key in gitconfig file. +// It returns the default value when gitconfig is missing, or it doesn't contain a gitget section, +// or if the section is empty, or if it doesn't contain a valid value for the key. +func (c *gitconfig) get(key string, def string) string { + if c == nil || c.Config == nil { + return def } - gitget := c.findConfigSection(CfgSection) + gitget := c.findGitconfigSection(GitgetPrefix) if gitget == nil { - return defReposRoot + return def } - reposRoot = gitget.Option(CfgReposRoot) - reposRoot = strings.TrimSpace(reposRoot) - if reposRoot == "" { - return defReposRoot + opt := gitget.Option(key) + if strings.TrimSpace(opt) == "" { + return def } - return reposRoot + return opt } -func (c *Conf) DefaultHost() string { - defaultHost := os.Getenv(EnvDefaultHost) - if defaultHost != "" { - return defaultHost - } - - if c == nil || c.gitconfig == nil { - return DefaultHost - } - - gitget := c.findConfigSection(CfgSection) - if gitget == nil { - return DefaultHost - } - - defaultHost = gitget.Option(CfgDefaultHost) - defaultHost = strings.TrimSpace(defaultHost) - if defaultHost == "" { - return DefaultHost - } - - return defaultHost -} - -func (c *Conf) findConfigSection(name string) *plumbing.Section { - for _, s := range c.gitconfig.Raw.Sections { - if s.Name == name { +func (c *gitconfig) findGitconfigSection(name string) *plumbing.Section { + for _, s := range c.Raw.Sections { + if strings.ToLower(s.Name) == strings.ToLower(name) { return s } } @@ -100,9 +87,9 @@ func (c *Conf) findConfigSection(name string) *plumbing.Section { return nil } -// home returns path to a home directory or empty string if can't be found +// home returns path to a home directory or empty string if can't be found. // Using empty string means that in the unlikely situation where home dir can't be found -// and there's no reposRoot specified in the global config, the current dir will be used as repos root. +// and there's no reposRoot specified by any of the config methods, the current dir will be used as repos root. func home() string { home, err := homedir.Dir() if err != nil { diff --git a/pkg/config_test.go b/pkg/config_test.go index c985d50..6cbfcb7 100644 --- a/pkg/config_test.go +++ b/pkg/config_test.go @@ -3,141 +3,153 @@ package pkg import ( "os" "path" + "strings" "testing" "github.com/go-git/go-git/v5/config" + "github.com/spf13/viper" ) -func newConfigWithFullGitconfig() *Conf { - gitconfig := config.NewConfig() +const ( + EnvDefaultHost = "GITGET_DEFAULTHOST" + EnvReposRoot = "GITGET_REPOSROOT" +) - gitget := gitconfig.Raw.Section(CfgSection) - gitget.AddOption(CfgReposRoot, "file.root") - gitget.AddOption(CfgDefaultHost, "file.host") +func newConfigWithFullGitconfig() *gitconfig { + cfg := config.NewConfig() - return &Conf{ - gitconfig: gitconfig, + gitget := cfg.Raw.Section(GitgetPrefix) + gitget.AddOption(KeyReposRoot, "file.root") + gitget.AddOption(KeyDefaultHost, "file.host") + + return &gitconfig{ + Config: cfg, } } -func newConfigWithEmptyGitgetSection() *Conf { - gitconfig := config.NewConfig() +func newConfigWithEmptyGitgetSection() *gitconfig { + cfg := config.NewConfig() - _ = gitconfig.Raw.Section(CfgSection) + _ = cfg.Raw.Section(GitgetPrefix) - return &Conf{ - gitconfig: gitconfig, + return &gitconfig{ + Config: cfg, } } -func newConfigWithEmptyValues() *Conf { - gitconfig := config.NewConfig() +func newConfigWithEmptyValues() *gitconfig { + cfg := config.NewConfig() - gitget := gitconfig.Raw.Section(CfgSection) - gitget.AddOption(CfgReposRoot, "") - gitget.AddOption(CfgDefaultHost, " ") + gitget := cfg.Raw.Section(GitgetPrefix) + gitget.AddOption(KeyReposRoot, "") + gitget.AddOption(KeyDefaultHost, " ") - return &Conf{ - gitconfig: gitconfig, + return &gitconfig{ + Config: cfg, } } -func newConfigWithoutGitgetSection() *Conf { - gitconfig := config.NewConfig() +func newConfigWithoutGitgetSection() *gitconfig { + cfg := config.NewConfig() - return &Conf{ - gitconfig: gitconfig, + return &gitconfig{ + Config: cfg, } } -func newConfigWithEmptyGitconfig() *Conf { - return &Conf{ - gitconfig: nil, +func newConfigWithEmptyGitconfig() *gitconfig { + return &gitconfig{ + Config: nil, } } -func newConfigWithEnvVars() *Conf { +func newConfigWithEnvVars() *gitconfig { _ = os.Setenv(EnvDefaultHost, "env.host") _ = os.Setenv(EnvReposRoot, "env.root") - return &Conf{ - gitconfig: nil, + return &gitconfig{ + Config: nil, } } -func newConfigWithGitconfigAndEnvVars() *Conf { - gitconfig := config.NewConfig() +func newConfigWithGitconfigAndEnvVars() *gitconfig { + cfg := config.NewConfig() - gitget := gitconfig.Raw.Section(CfgSection) - gitget.AddOption(CfgReposRoot, "file.root") - gitget.AddOption(CfgDefaultHost, "file.host") + gitget := cfg.Raw.Section(GitgetPrefix) + gitget.AddOption(KeyReposRoot, "file.root") + gitget.AddOption(KeyDefaultHost, "file.host") _ = os.Setenv(EnvDefaultHost, "env.host") _ = os.Setenv(EnvReposRoot, "env.root") - return &Conf{ - gitconfig: gitconfig, + return &gitconfig{ + Config: cfg, } } -func newConfigWithEmptySectionAndEnvVars() *Conf { - gitconfig := config.NewConfig() +func newConfigWithEmptySectionAndEnvVars() *gitconfig { + cfg := config.NewConfig() - _ = gitconfig.Raw.Section(CfgSection) + _ = cfg.Raw.Section(GitgetPrefix) _ = os.Setenv(EnvDefaultHost, "env.host") _ = os.Setenv(EnvReposRoot, "env.root") - return &Conf{ - gitconfig: gitconfig, + return &gitconfig{ + Config: cfg, } } -func newConfigWithMixed() *Conf { - gitconfig := config.NewConfig() +func newConfigWithMixed() *gitconfig { + cfg := config.NewConfig() - gitget := gitconfig.Raw.Section(CfgSection) - gitget.AddOption(CfgReposRoot, "file.root") - gitget.AddOption(CfgDefaultHost, "file.host") + gitget := cfg.Raw.Section(GitgetPrefix) + gitget.AddOption(KeyReposRoot, "file.root") + gitget.AddOption(KeyDefaultHost, "file.host") _ = os.Setenv(EnvDefaultHost, "env.host") - return &Conf{ - gitconfig: gitconfig, + return &gitconfig{ + Config: cfg, } } func TestConfig(t *testing.T) { - defReposRoot := path.Join(home(), DefaultReposRootSubpath) + defReposRoot := path.Join(home(), DefReposRoot) var tests = []struct { - makeConfig func() *Conf + makeConfig func() *gitconfig wantReposRoot string wantDefaultHost string }{ {newConfigWithFullGitconfig, "file.root", "file.host"}, - {newConfigWithoutGitgetSection, defReposRoot, DefaultHost}, - {newConfigWithEmptyGitconfig, defReposRoot, DefaultHost}, + {newConfigWithoutGitgetSection, defReposRoot, DefDefaultHost}, + {newConfigWithEmptyGitconfig, defReposRoot, DefDefaultHost}, {newConfigWithEnvVars, "env.root", "env.host"}, {newConfigWithGitconfigAndEnvVars, "env.root", "env.host"}, {newConfigWithEmptySectionAndEnvVars, "env.root", "env.host"}, - {newConfigWithEmptyGitgetSection, defReposRoot, DefaultHost}, - {newConfigWithEmptyValues, defReposRoot, DefaultHost}, + {newConfigWithEmptyGitgetSection, defReposRoot, DefDefaultHost}, + {newConfigWithEmptyValues, defReposRoot, DefDefaultHost}, {newConfigWithMixed, "file.root", "env.host"}, } for _, test := range tests { + viper.SetEnvPrefix(strings.ToUpper(GitgetPrefix)) + viper.AutomaticEnv() + cfg := test.makeConfig() + setMissingValues(cfg) - if cfg.ReposRoot() != test.wantReposRoot { - t.Errorf("Wrong reposRoot value, got: %+v; want: %+v", cfg.ReposRoot(), test.wantReposRoot) + if viper.GetString(KeyDefaultHost) != test.wantDefaultHost { + t.Errorf("Wrong %s value, got: %s; want: %s", KeyDefaultHost, viper.GetString(KeyDefaultHost), test.wantDefaultHost) } - if cfg.DefaultHost() != test.wantDefaultHost { - t.Errorf("Wrong defaultHost value, got: %+v; want: %+v", cfg.DefaultHost(), test.wantDefaultHost) + if viper.GetString(KeyReposRoot) != test.wantReposRoot { + t.Errorf("Wrong %s value, got: %s; want: %s", KeyReposRoot, viper.GetString(KeyReposRoot), test.wantReposRoot) } - // Unset env variables after each test so they don't affect other tests + // Unset env variables and reset viper registry after each test + viper.Reset() err := os.Unsetenv(EnvDefaultHost) checkFatal(t, err) err = os.Unsetenv(EnvReposRoot) diff --git a/pkg/list.go b/pkg/list.go index 9aa7b71..d0ed708 100644 --- a/pkg/list.go +++ b/pkg/list.go @@ -8,6 +8,7 @@ import ( "sort" "strings" + "github.com/spf13/viper" "github.com/xlab/treeprint" "github.com/go-git/go-git/v5" @@ -24,7 +25,7 @@ var repos []string func FindRepos() ([]string, error) { repos = []string{} - root := Cfg.ReposRoot() + root := viper.GetString(KeyReposRoot) if _, err := os.Stat(root); err != nil { return nil, fmt.Errorf("Repos root %s does not exist or can't be accessed", root) @@ -105,7 +106,7 @@ func OpenAll(paths []string) ([]*Repo, error) { } func PrintRepos(repos []*Repo) { - root := Cfg.ReposRoot() + root := viper.GetString(KeyReposRoot) seg := make([][]string, len(repos)) diff --git a/pkg/url.go b/pkg/url.go index 1f27680..d7332a1 100644 --- a/pkg/url.go +++ b/pkg/url.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/pkg/errors" + "github.com/spf13/viper" ) // scpSyntax matches the SCP-like addresses used by the ssh protocol (eg, [user@]host.xz:path/to/repo.git/). @@ -45,7 +46,7 @@ func ParseURL(rawURL string) (url *urlpkg.URL, err error) { // Default to configured defaultHost when host is empty if url.Host == "" { - url.Host = Cfg.DefaultHost() + url.Host = viper.GetString(KeyDefaultHost) } // Default to https when scheme is empty diff --git a/pkg/url_test.go b/pkg/url_test.go index e6ea5bf..fcd1090 100644 --- a/pkg/url_test.go +++ b/pkg/url_test.go @@ -48,6 +48,9 @@ func TestURLParse(t *testing.T) { {"file://local/grdl/git-get", "local/grdl/git-get"}, } + // We need to init config first so the default values are correctly loaded + InitConfig() + for _, test := range tests { url, err := ParseURL(test.in) if err != nil {