Fix config init command reading existing config file

Author: silvanocerza

Diff for: cli/cli.go

@@ -99,15 +99,12 @@ func createCliCommandTree(cmd *cobra.Command) {
 
 	cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "Print the logs on the standard output.")
 	cmd.PersistentFlags().String("log-level", "", "Messages with this level and above will be logged. Valid levels are: trace, debug, info, warn, error, fatal, panic")
-	configuration.Settings.BindPFlag("logging.level", cmd.PersistentFlags().Lookup("log-level"))
 	cmd.PersistentFlags().String("log-file", "", "Path to the file where logs will be written.")
-	configuration.Settings.BindPFlag("logging.file", cmd.PersistentFlags().Lookup("log-file"))
 	cmd.PersistentFlags().String("log-format", "", "The output format for the logs, can be {text|json}.")
-	configuration.Settings.BindPFlag("logging.format", cmd.PersistentFlags().Lookup("log-format"))
 	cmd.PersistentFlags().StringVar(&outputFormat, "format", "text", "The output format, can be {text|json}.")
 	cmd.PersistentFlags().StringVar(&configFile, "config-file", "", "The custom config file (if not specified the default will be used).")
 	cmd.PersistentFlags().StringSlice("additional-urls", []string{}, "Comma-separated list of additional URLs for the Boards Manager.")
-	configuration.Settings.BindPFlag("board_manager.additional_urls", cmd.PersistentFlags().Lookup("additional-urls"))
+	configuration.BindFlags(cmd, configuration.Settings)
 }
 
 // convert the string passed to the `--log-level` option to the corresponding

Diff for: cli/config/init.go

@@ -17,16 +17,21 @@ package config
 
 import (
 	"os"
-	"path/filepath"
 
 	"github.com/arduino/arduino-cli/cli/errorcodes"
 	"github.com/arduino/arduino-cli/cli/feedback"
 	"github.com/arduino/arduino-cli/configuration"
+	"github.com/arduino/go-paths-helper"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
 )
 
-var destDir string
+var (
+	destDir   string
+	destFile  string
+	overwrite bool
+)
 
 const defaultFileName = "arduino-cli.yaml"
 
@@ -37,39 +42,71 @@ func initInitCommand() *cobra.Command {
 		Long:  "Creates or updates the configuration file in the data directory or custom directory with the current configuration settings.",
 		Example: "" +
 			"  # Writes current configuration to the configuration file in the data directory.\n" +
-			"  " + os.Args[0] + " config init",
+			"  " + os.Args[0] + " config init" +
+			"  " + os.Args[0] + " config init --dest-dir /home/user/MyDirectory" +
+			"  " + os.Args[0] + " config init --dest-file /home/user/MyDirectory/my_settings.yaml",
 		Args: cobra.NoArgs,
 		Run:  runInitCommand,
 	}
 	initCommand.Flags().StringVar(&destDir, "dest-dir", "", "Sets where to save the configuration file.")
+	initCommand.Flags().StringVar(&destFile, "dest-file", "", "Sets where to save the configuration file.")
+	initCommand.Flags().BoolVar(&overwrite, "overwrite", false, "Overwrite existing config file.")
 	return initCommand
 }
 
 func runInitCommand(cmd *cobra.Command, args []string) {
-	if destDir == "" {
+	if destFile != "" && destDir != "" {
+		feedback.Errorf("Can't use both --dest-file and --dest-dir flags at the same time.")
+		os.Exit(errorcodes.ErrGeneric)
+	}
+
+	var configFileAbsPath *paths.Path
+	var absPath *paths.Path
+	var err error
+
+	switch {
+	case destFile != "":
+		configFileAbsPath, err = paths.New(destFile).Abs()
+		if err != nil {
+			feedback.Errorf("Cannot find absolute path: %v", err)
+			os.Exit(errorcodes.ErrGeneric)
+		}
+
+		absPath = configFileAbsPath.Parent()
+	case destDir == "":
 		destDir = configuration.Settings.GetString("directories.Data")
+		fallthrough
+	default:
+		absPath, err = paths.New(destDir).Abs()
+		if err != nil {
+			feedback.Errorf("Cannot find absolute path: %v", err)
+			os.Exit(errorcodes.ErrGeneric)
+		}
+		configFileAbsPath = absPath.Join(defaultFileName)
 	}
 
-	absPath, err := filepath.Abs(destDir)
-	if err != nil {
-		feedback.Errorf("Cannot find absolute path: %v", err)
+	if !overwrite && configFileAbsPath.Exist() {
+		feedback.Error("Config file already exists, use --overwrite to discard the existing one.")
 		os.Exit(errorcodes.ErrGeneric)
 	}
-	configFileAbsPath := filepath.Join(absPath, defaultFileName)
 
 	logrus.Infof("Writing config file to: %s", absPath)
 
-	if err := os.MkdirAll(absPath, os.FileMode(0755)); err != nil {
+	if err := absPath.MkdirAll(); err != nil {
 		feedback.Errorf("Cannot create config file directory: %v", err)
 		os.Exit(errorcodes.ErrGeneric)
 	}
 
-	if err := configuration.Settings.WriteConfigAs(configFileAbsPath); err != nil {
+	newSettings := viper.New()
+	configuration.SetDefaults(newSettings)
+	configuration.BindFlags(cmd, newSettings)
+
+	if err := newSettings.WriteConfigAs(configFileAbsPath.String()); err != nil {
 		feedback.Errorf("Cannot create config file: %v", err)
 		os.Exit(errorcodes.ErrGeneric)
 	}
 
-	msg := "Config file written to: " + configFileAbsPath
+	msg := "Config file written to: " + configFileAbsPath.String()
 	logrus.Info(msg)
 	feedback.Print(msg)
 }

Diff for: configuration/configuration.go

@@ -25,6 +25,7 @@ import (
 	paths "github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-win32-utils"
 	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
 	jww "github.com/spf13/jwalterweatherman"
 	"github.com/spf13/viper"
 )
@@ -39,6 +40,8 @@ func Init(configPath string) *viper.Viper {
 	// Config file metadata
 	jww.SetStdoutThreshold(jww.LevelFatal)
 	settings := viper.New()
+	// Set default values for all the settings
+	SetDefaults(settings)
 
 	configDir := paths.New(configPath)
 	if configDir != nil && !configDir.IsDir() {
@@ -47,16 +50,6 @@ func Init(configPath string) *viper.Viper {
 		settings.SetConfigName("arduino-cli")
 	}
 
-	// Bind env vars
-	settings.SetEnvPrefix("ARDUINO")
-	settings.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
-	settings.AutomaticEnv()
-
-	// Bind env aliases to keep backward compatibility
-	settings.BindEnv("directories.User", "ARDUINO_SKETCHBOOK_DIR")
-	settings.BindEnv("directories.Downloads", "ARDUINO_DOWNLOADS_DIR")
-	settings.BindEnv("directories.Data", "ARDUINO_DATA_DIR")
-
 	if configPath == "" {
 		// Get default data path if none was provided
 		if configPath = settings.GetString("directories.Data"); configPath != "" {
@@ -69,20 +62,6 @@ func Init(configPath string) *viper.Viper {
 		settings.AddConfigPath(filepath.Dir(configPath))
 	}
 
-	// Early access directories.Data and directories.User in case
-	// those were set through env vars or cli flags
-	dataDir := settings.GetString("directories.Data")
-	if dataDir == "" {
-		dataDir = getDefaultArduinoDataDir()
-	}
-	userDir := settings.GetString("directories.User")
-	if userDir == "" {
-		userDir = getDefaultUserDir()
-	}
-
-	// Set default values for all the settings
-	setDefaults(settings, dataDir, userDir)
-
 	// Attempt to read config file
 	if err := settings.ReadInConfig(); err != nil {
 		// ConfigFileNotFoundError is acceptable, anything else
@@ -95,6 +74,14 @@ func Init(configPath string) *viper.Viper {
 	return settings
 }
 
+// BindFlags creates all the flags binding between the cobra Command and the instance of viper
+func BindFlags(cmd *cobra.Command, settings *viper.Viper) {
+	settings.BindPFlag("logging.level", cmd.Flag("log-level"))
+	settings.BindPFlag("logging.file", cmd.Flag("log-file"))
+	settings.BindPFlag("logging.format", cmd.Flag("log-format"))
+	settings.BindPFlag("board_manager.additional_urls", cmd.Flag("additional-urls"))
+}
+
 // getDefaultArduinoDataDir returns the full path to the default arduino folder
 func getDefaultArduinoDataDir() string {
 	userHomeDir, err := os.UserHomeDir()

Diff for: configuration/configuration_test.go

@@ -30,6 +30,12 @@ func tmpDirOrDie() string {
 	if err != nil {
 		panic(fmt.Sprintf("error creating tmp dir: %v", err))
 	}
+	// Symlinks are evaluated becase the temp folder on Mac OS is inside /var, it's not writable
+	// and is a symlink to /private/var, we want the full path so we do this
+	dir, err = filepath.EvalSymlinks(dir)
+	if err != nil {
+		panic(fmt.Sprintf("error evaluating tmp dir symlink: %v", err))
+	}
 	return dir
 }
 
@@ -73,6 +79,59 @@ func BenchmarkSearchConfigTree(b *testing.B) {
 }
 
 func TestInit(t *testing.T) {
-	settings := Init("")
+	tmp := tmpDirOrDie()
+	defer os.RemoveAll(tmp)
+	settings := Init(filepath.Join(tmp, "arduino-cli.yaml"))
 	require.NotNil(t, settings)
+
+	require.Equal(t, "info", settings.GetString("logging.level"))
+	require.Equal(t, "text", settings.GetString("logging.format"))
+
+	require.Empty(t, settings.GetStringSlice("board_manager.additional_urls"))
+
+	require.NotEmpty(t, settings.GetString("directories.Data"))
+	require.NotEmpty(t, settings.GetString("directories.Downloads"))
+	require.NotEmpty(t, settings.GetString("directories.User"))
+
+	require.Equal(t, "50051", settings.GetString("daemon.port"))
+
+	require.Equal(t, true, settings.GetBool("telemetry.enabled"))
+	require.Equal(t, ":9090", settings.GetString("telemetry.addr"))
+}
+
+func TestFindConfigFile(t *testing.T) {
+	configFile := FindConfigFile([]string{"--config-file"})
+	require.Equal(t, "", configFile)
+
+	configFile = FindConfigFile([]string{"--config-file", "some/path/to/config"})
+	require.Equal(t, "some/path/to/config", configFile)
+
+	configFile = FindConfigFile([]string{"--config-file", "some/path/to/config/arduino-cli.yaml"})
+	require.Equal(t, "some/path/to/config/arduino-cli.yaml", configFile)
+
+	configFile = FindConfigFile([]string{})
+	require.Equal(t, "", configFile)
+
+	// Create temporary directories
+	tmp := tmpDirOrDie()
+	defer os.RemoveAll(tmp)
+	target := filepath.Join(tmp, "foo", "bar", "baz")
+	os.MkdirAll(target, os.ModePerm)
+	require.Nil(t, os.Chdir(target))
+
+	// Create a config file
+	f, err := os.Create(filepath.Join(target, "..", "..", "arduino-cli.yaml"))
+	require.Nil(t, err)
+	f.Close()
+
+	configFile = FindConfigFile([]string{})
+	require.Equal(t, filepath.Join(tmp, "foo", "arduino-cli.yaml"), configFile)
+
+	// Create another config file
+	f, err = os.Create(filepath.Join(target, "arduino-cli.yaml"))
+	require.Nil(t, err)
+	f.Close()
+
+	configFile = FindConfigFile([]string{})
+	require.Equal(t, filepath.Join(target, "arduino-cli.yaml"), configFile)
 }

Diff for: configuration/defaults.go

@@ -17,11 +17,13 @@ package configuration
 
 import (
 	"path/filepath"
+	"strings"
 
 	"github.com/spf13/viper"
 )
 
-func setDefaults(settings *viper.Viper, dataDir, userDir string) {
+// SetDefaults sets the default values for certain keys
+func SetDefaults(settings *viper.Viper) {
 	// logging
 	settings.SetDefault("logging.level", "info")
 	settings.SetDefault("logging.format", "text")
@@ -30,14 +32,24 @@ func setDefaults(settings *viper.Viper, dataDir, userDir string) {
 	settings.SetDefault("board_manager.additional_urls", []string{})
 
 	// arduino directories
-	settings.SetDefault("directories.Data", dataDir)
-	settings.SetDefault("directories.Downloads", filepath.Join(dataDir, "staging"))
-	settings.SetDefault("directories.User", userDir)
+	settings.SetDefault("directories.Data", getDefaultArduinoDataDir())
+	settings.SetDefault("directories.Downloads", filepath.Join(getDefaultArduinoDataDir(), "staging"))
+	settings.SetDefault("directories.User", getDefaultUserDir())
 
 	// daemon settings
 	settings.SetDefault("daemon.port", "50051")
 
 	//telemetry settings
 	settings.SetDefault("telemetry.enabled", true)
 	settings.SetDefault("telemetry.addr", ":9090")
+
+	// Bind env vars
+	settings.SetEnvPrefix("ARDUINO")
+	settings.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
+	settings.AutomaticEnv()
+
+	// Bind env aliases to keep backward compatibility
+	settings.BindEnv("directories.User", "ARDUINO_SKETCHBOOK_DIR")
+	settings.BindEnv("directories.Downloads", "ARDUINO_DOWNLOADS_DIR")
+	settings.BindEnv("directories.Data", "ARDUINO_DATA_DIR")
 }