diff --git a/cmd/cmd.go b/cmd/cmd.go index ae73d0417..c71f0de2e 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -22,7 +22,6 @@ import ( "github.com/ncw/rclone/fs" "github.com/ncw/rclone/fs/accounting" - "github.com/ncw/rclone/fs/config" "github.com/ncw/rclone/fs/config/configflags" "github.com/ncw/rclone/fs/config/flags" "github.com/ncw/rclone/fs/filter" @@ -373,9 +372,6 @@ func initConfig() { // Finish parsing any command line flags configflags.SetFlags() - // Load the rest of the config now we have started the logger - config.LoadConfig() - // Load filters var err error filter.Active, err = filter.NewFilter(&filterflags.Opt) diff --git a/cmd/version/version.go b/cmd/version/version.go index 143c597ee..2dd9cc3e8 100644 --- a/cmd/version/version.go +++ b/cmd/version/version.go @@ -6,10 +6,10 @@ import ( ) func init() { - cmd.Root.AddCommand(commandDefintion) + cmd.Root.AddCommand(commandDefinition) } -var commandDefintion = &cobra.Command{ +var commandDefinition = &cobra.Command{ Use: "version", Short: `Show the version number.`, Run: func(command *cobra.Command, args []string) { diff --git a/cmd/version/version_test.go b/cmd/version/version_test.go new file mode 100644 index 000000000..f9f6528d1 --- /dev/null +++ b/cmd/version/version_test.go @@ -0,0 +1,43 @@ +package version + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/ncw/rclone/cmd" + "github.com/ncw/rclone/fs/config" + "github.com/stretchr/testify/assert" +) + +func TestVersionWorksWithoutAccessibleConfigFile(t *testing.T) { + // create temp config file + tempFile, err := ioutil.TempFile("", "unreadable_config.conf") + assert.NoError(t, err) + path := tempFile.Name() + defer func() { + err := os.Remove(path) + assert.NoError(t, err) + }() + assert.NoError(t, tempFile.Close()) + assert.NoError(t, os.Chmod(path, 0000)) + // re-wire + oldOsStdout := os.Stdout + oldConfigPath := config.ConfigPath + config.ConfigPath = path + os.Stdout = nil + defer func() { + os.Stdout = oldOsStdout + config.ConfigPath = oldConfigPath + }() + + cmd.Root.SetArgs([]string{"version"}) + assert.NotPanics(t, func() { + assert.NoError(t, cmd.Root.Execute()) + }) + + cmd.Root.SetArgs([]string{"--version"}) + assert.NotPanics(t, func() { + assert.NoError(t, cmd.Root.Execute()) + }) +} diff --git a/fs/config/config.go b/fs/config/config.go index 6b02411dd..6aabe5094 100644 --- a/fs/config/config.go +++ b/fs/config/config.go @@ -60,8 +60,8 @@ const ( // Global var ( - // configData is the config file data structure - configData *goconfig.ConfigFile + // configFile is the global config data structure. Don't read it directly, use getConfigData() + configFile *goconfig.ConfigFile // ConfigPath points to the config file ConfigPath = makeConfigPath() @@ -81,6 +81,13 @@ func init() { fs.ConfigFileGet = FileGet } +func getConfigData() *goconfig.ConfigFile { + if configFile == nil { + LoadConfig() + } + return configFile +} + // Return the path to the configuration file func makeConfigPath() string { // Find user's home directory @@ -153,10 +160,10 @@ func makeConfigPath() string { func LoadConfig() { // Load configuration file. var err error - configData, err = loadConfigFile() + configFile, err = loadConfigFile() if err == errorConfigFileNotFound { fs.Logf(nil, "Config file %q not found - using defaults", ConfigPath) - configData, _ = goconfig.LoadFromReader(&bytes.Buffer{}) + configFile, _ = goconfig.LoadFromReader(&bytes.Buffer{}) } else if err != nil { log.Fatalf("Failed to load config file %q: %v", ConfigPath, err) } else { @@ -365,7 +372,7 @@ func saveConfig() error { }() var buf bytes.Buffer - err = goconfig.SaveConfigData(configData, &buf) + err = goconfig.SaveConfigData(getConfigData(), &buf) if err != nil { return errors.Errorf("Failed to save config file: %v", err) } @@ -456,7 +463,7 @@ func SaveConfig() { // disk first and overwrites the given value only. func SetValueAndSave(name, key, value string) (err error) { // Set the value in config in case we fail to reload it - configData.SetValue(name, key, value) + getConfigData().SetValue(name, key, value) // Reload the config file reloadedConfigFile, err := loadConfigFile() if err == errorConfigFileNotFound { @@ -471,7 +478,7 @@ func SetValueAndSave(name, key, value string) (err error) { return err } // Update the config file with the reloaded version - configData = reloadedConfigFile + configFile = reloadedConfigFile // Set the value in the reloaded version reloadedConfigFile.SetValue(name, key, value) // Save it again @@ -481,7 +488,7 @@ func SetValueAndSave(name, key, value string) (err error) { // ShowRemotes shows an overview of the config file func ShowRemotes() { - remotes := configData.GetSectionList() + remotes := getConfigData().GetSectionList() if len(remotes) == 0 { return } @@ -495,7 +502,7 @@ func ShowRemotes() { // ChooseRemote chooses a remote name func ChooseRemote() string { - remotes := configData.GetSectionList() + remotes := getConfigData().GetSectionList() sort.Strings(remotes) return Choose("remote", remotes, nil, false) } @@ -622,7 +629,7 @@ func ShowRemote(name string) { fmt.Printf("--------------------\n") fmt.Printf("[%s]\n", name) fs := MustFindByName(name) - for _, key := range configData.GetKeyList(name) { + for _, key := range getConfigData().GetKeyList(name) { isPassword := false for _, option := range fs.Options { if option.Name == key && option.IsPassword { @@ -649,7 +656,7 @@ func OkRemote(name string) bool { case 'e': return false case 'd': - configData.DeleteSection(name) + getConfigData().DeleteSection(name) return true default: fs.Errorf(nil, "Bad choice %c", i) @@ -736,7 +743,7 @@ func UpdateRemote(name string, keyValues []string) error { } // Set the config for i := 0; i < len(keyValues); i += 2 { - configData.SetValue(name, keyValues[i], keyValues[i+1]) + getConfigData().SetValue(name, keyValues[i], keyValues[i+1]) } RemoteConfig(name) ShowRemote(name) @@ -751,11 +758,11 @@ func CreateRemote(name string, provider string, keyValues []string) error { // Suppress Confirm fs.Config.AutoConfirm = true // Delete the old config if it exists - configData.DeleteSection(name) + getConfigData().DeleteSection(name) // Set the type - configData.SetValue(name, "type", provider) + getConfigData().SetValue(name, "type", provider) // Show this is automatically configured - configData.SetValue(name, ConfigAutomatic, "yes") + getConfigData().SetValue(name, ConfigAutomatic, "yes") // Set the remaining values return UpdateRemote(name, keyValues) } @@ -770,7 +777,7 @@ func PasswordRemote(name string, keyValues []string) error { fs.Config.AutoConfirm = true passwd := obscure.MustObscure(keyValues[1]) if passwd != "" { - configData.SetValue(name, keyValues[0], passwd) + getConfigData().SetValue(name, keyValues[0], passwd) RemoteConfig(name) ShowRemote(name) SaveConfig() @@ -830,10 +837,10 @@ func NewRemoteName() (name string) { // NewRemote make a new remote from its name func NewRemote(name string) { newType := ChooseOption(fsOption()) - configData.SetValue(name, "type", newType) + getConfigData().SetValue(name, "type", newType) fs := fs.MustFind(newType) for _, option := range fs.Options { - configData.SetValue(name, option.Name, ChooseOption(&option)) + getConfigData().SetValue(name, option.Name, ChooseOption(&option)) } RemoteConfig(name) if OkRemote(name) { @@ -855,7 +862,7 @@ func EditRemote(fs *fs.RegInfo, name string) { fmt.Printf("Edit? (y/n)>\n") if Confirm() { newValue := ChooseOption(&option) - configData.SetValue(name, key, newValue) + getConfigData().SetValue(name, key, newValue) } } if OkRemote(name) { @@ -868,7 +875,7 @@ func EditRemote(fs *fs.RegInfo, name string) { // DeleteRemote gets the user to delete a remote func DeleteRemote(name string) { - configData.DeleteSection(name) + getConfigData().DeleteSection(name) SaveConfig() } @@ -877,9 +884,9 @@ func DeleteRemote(name string) { func copyRemote(name string) string { newName := NewRemoteName() // Copy the keys - for _, key := range configData.GetKeyList(name) { - value := configData.MustValue(name, key, "") - configData.SetValue(newName, key, value) + for _, key := range getConfigData().GetKeyList(name) { + value := getConfigData().MustValue(name, key, "") + getConfigData().SetValue(newName, key, value) } return newName } @@ -889,7 +896,7 @@ func RenameRemote(name string) { fmt.Printf("Enter new name for %q remote.\n", name) newName := copyRemote(name) if name != newName { - configData.DeleteSection(name) + getConfigData().DeleteSection(name) SaveConfig() } } @@ -914,7 +921,7 @@ func ShowConfigLocation() { // ShowConfig prints the (unencrypted) config options func ShowConfig() { var buf bytes.Buffer - if err := goconfig.SaveConfigData(configData, &buf); err != nil { + if err := goconfig.SaveConfigData(getConfigData(), &buf); err != nil { log.Fatalf("Failed to serialize config: %v", err) } str := buf.String() @@ -927,7 +934,7 @@ func ShowConfig() { // EditConfig edits the config file interactively func EditConfig() { for { - haveRemotes := len(configData.GetSectionList()) != 0 + haveRemotes := len(getConfigData().GetSectionList()) != 0 what := []string{"eEdit existing remote", "nNew remote", "dDelete remote", "rRename remote", "cCopy remote", "sSet configuration password", "qQuit config"} if haveRemotes { fmt.Printf("Current remotes:\n\n") @@ -1023,10 +1030,10 @@ func Authorize(args []string) { defer DeleteRemote(name) // Indicate that we want fully automatic configuration. - configData.SetValue(name, ConfigAutomatic, "yes") + getConfigData().SetValue(name, ConfigAutomatic, "yes") if len(args) == 3 { - configData.SetValue(name, ConfigClientID, args[1]) - configData.SetValue(name, ConfigClientSecret, args[2]) + getConfigData().SetValue(name, ConfigClientID, args[1]) + getConfigData().SetValue(name, ConfigClientSecret, args[2]) } fs.Config(name) } @@ -1048,7 +1055,7 @@ func FileGet(section, key string, defaultVal ...string) string { if found { defaultVal = []string{newValue} } - return configData.MustValue(section, key, defaultVal...) + return getConfigData().MustValue(section, key, defaultVal...) } // FileGetBool gets the config key under section returning the @@ -1066,7 +1073,7 @@ func FileGetBool(section, key string, defaultVal ...bool) bool { defaultVal = []bool{newBool} } } - return configData.MustBool(section, key, defaultVal...) + return getConfigData().MustBool(section, key, defaultVal...) } // FileGetInt gets the config key under section returning the @@ -1084,20 +1091,20 @@ func FileGetInt(section, key string, defaultVal ...int) int { defaultVal = []int{newInt} } } - return configData.MustInt(section, key, defaultVal...) + return getConfigData().MustInt(section, key, defaultVal...) } // FileSet sets the key in section to value. It doesn't save // the config file. func FileSet(section, key, value string) { - configData.SetValue(section, key, value) + getConfigData().SetValue(section, key, value) } // FileDeleteKey deletes the config key in the config file. // It returns true if the key was deleted, // or returns false if the section or key didn't exist. func FileDeleteKey(section, key string) bool { - return configData.DeleteKey(section, key) + return getConfigData().DeleteKey(section, key) } var matchEnv = regexp.MustCompile(`^RCLONE_CONFIG_(.*?)_TYPE=.*$`) @@ -1105,7 +1112,7 @@ var matchEnv = regexp.MustCompile(`^RCLONE_CONFIG_(.*?)_TYPE=.*$`) // FileSections returns the sections in the config file // including any defined by environment variables. func FileSections() []string { - sections := configData.GetSectionList() + sections := getConfigData().GetSectionList() for _, item := range os.Environ() { matches := matchEnv.FindStringSubmatch(item) if len(matches) == 2 { @@ -1118,9 +1125,9 @@ func FileSections() []string { // Dump dumps all the config as a JSON file func Dump() error { dump := make(map[string]map[string]string) - for _, name := range configData.GetSectionList() { + for _, name := range getConfigData().GetSectionList() { params := make(map[string]string) - for _, key := range configData.GetKeyList(name) { + for _, key := range getConfigData().GetKeyList(name) { params[key] = FileGet(name, key) } dump[name] = params diff --git a/fs/config/config_test.go b/fs/config/config_test.go index fa63c918a..af858be16 100644 --- a/fs/config/config_test.go +++ b/fs/config/config_test.go @@ -27,22 +27,22 @@ func TestCRUD(t *testing.T) { oldOsStdout := os.Stdout oldConfigPath := ConfigPath oldConfig := fs.Config - oldConfigData := configData + oldConfigFile := configFile oldReadLine := ReadLine os.Stdout = nil ConfigPath = path fs.Config = &fs.ConfigInfo{} - configData = nil + configFile = nil defer func() { os.Stdout = oldOsStdout ConfigPath = oldConfigPath ReadLine = oldReadLine fs.Config = oldConfig - configData = oldConfigData + configFile = oldConfigFile }() LoadConfig() - assert.Equal(t, []string{}, configData.GetSectionList()) + assert.Equal(t, []string{}, getConfigData().GetSectionList()) // Fake a remote fs.Register(&fs.RegInfo{Name: "config_test_remote"}) @@ -59,25 +59,25 @@ func TestCRUD(t *testing.T) { } NewRemote("test") - assert.Equal(t, []string{"test"}, configData.GetSectionList()) + assert.Equal(t, []string{"test"}, configFile.GetSectionList()) // Reload the config file to workaround this bug // https://github.com/Unknwon/goconfig/issues/39 - configData, err = loadConfigFile() + configFile, err = loadConfigFile() require.NoError(t, err) // normal rename, test → asdf ReadLine = func() string { return "asdf" } RenameRemote("test") - assert.Equal(t, []string{"asdf"}, configData.GetSectionList()) + assert.Equal(t, []string{"asdf"}, configFile.GetSectionList()) // no-op rename, asdf → asdf RenameRemote("asdf") - assert.Equal(t, []string{"asdf"}, configData.GetSectionList()) + assert.Equal(t, []string{"asdf"}, configFile.GetSectionList()) // delete remote DeleteRemote("asdf") - assert.Equal(t, []string{}, configData.GetSectionList()) + assert.Equal(t, []string{}, configFile.GetSectionList()) } // Test some error cases