config: load config file only on first access (closes #1659, closes #2096) (#2147)

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)

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) {

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())
+	})
+}

fs/config/config.go

@@ -60,8 +60,8 @@ const (
 
 // Global
 var (
-	// configData is the config file data structure
+	// configFile is the global config data structure. Don't read it directly, use getConfigData()
-	configData *goconfig.ConfigFile
+	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) {
+	for _, key := range getConfigData().GetKeyList(name) {
-		value := configData.MustValue(name, key, "")
+		value := getConfigData().MustValue(name, key, "")
-		configData.SetValue(newName, key, value)
+		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])
+		getConfigData().SetValue(name, ConfigClientID, args[1])
-		configData.SetValue(name, ConfigClientSecret, args[2])
+		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

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