Skip to content

Commit f57bcae

Browse files
committed
Fix config init command reading existing config file
1 parent ea72158 commit f57bcae

File tree

5 files changed

+281
-19
lines changed

5 files changed

+281
-19
lines changed

Diff for: cli/cli.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,12 @@ func createCliCommandTree(cmd *cobra.Command) {
9999

100100
cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "Print the logs on the standard output.")
101101
cmd.PersistentFlags().String("log-level", "", "Messages with this level and above will be logged. Valid levels are: trace, debug, info, warn, error, fatal, panic")
102-
configuration.Settings.BindPFlag("logging.level", cmd.PersistentFlags().Lookup("log-level"))
103102
cmd.PersistentFlags().String("log-file", "", "Path to the file where logs will be written.")
104-
configuration.Settings.BindPFlag("logging.file", cmd.PersistentFlags().Lookup("log-file"))
105103
cmd.PersistentFlags().String("log-format", "", "The output format for the logs, can be {text|json}.")
106-
configuration.Settings.BindPFlag("logging.format", cmd.PersistentFlags().Lookup("log-format"))
107104
cmd.PersistentFlags().StringVar(&outputFormat, "format", "text", "The output format, can be {text|json}.")
108105
cmd.PersistentFlags().StringVar(&configFile, "config-file", "", "The custom config file (if not specified the default will be used).")
109106
cmd.PersistentFlags().StringSlice("additional-urls", []string{}, "Comma-separated list of additional URLs for the Boards Manager.")
110-
configuration.Settings.BindPFlag("board_manager.additional_urls", cmd.PersistentFlags().Lookup("additional-urls"))
107+
configuration.BindFlags(cmd, configuration.Settings)
111108
}
112109

113110
// convert the string passed to the `--log-level` option to the corresponding

Diff for: cli/config/init.go

+46-11
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,20 @@ package config
1717

1818
import (
1919
"os"
20-
"path/filepath"
2120

2221
"github.com/arduino/arduino-cli/cli/errorcodes"
2322
"github.com/arduino/arduino-cli/cli/feedback"
2423
"github.com/arduino/arduino-cli/configuration"
24+
"github.com/arduino/go-paths-helper"
2525
"github.com/sirupsen/logrus"
2626
"github.com/spf13/cobra"
2727
)
2828

29-
var destDir string
29+
var (
30+
destDir string
31+
destFile string
32+
overwrite bool
33+
)
3034

3135
const defaultFileName = "arduino-cli.yaml"
3236

@@ -37,39 +41,70 @@ func initInitCommand() *cobra.Command {
3741
Long: "Creates or updates the configuration file in the data directory or custom directory with the current configuration settings.",
3842
Example: "" +
3943
" # Writes current configuration to the configuration file in the data directory.\n" +
40-
" " + os.Args[0] + " config init",
44+
" " + os.Args[0] + " config init" +
45+
" " + os.Args[0] + " config init --dest-dir /home/user/MyDirectory" +
46+
" " + os.Args[0] + " config init --dest-file /home/user/MyDirectory/my_settings.yaml",
4147
Args: cobra.NoArgs,
4248
Run: runInitCommand,
4349
}
4450
initCommand.Flags().StringVar(&destDir, "dest-dir", "", "Sets where to save the configuration file.")
51+
initCommand.Flags().StringVar(&destFile, "dest-file", "", "Sets where to save the configuration file.")
52+
initCommand.Flags().BoolVar(&overwrite, "overwrite", false, "Overwrite existing config file.")
4553
return initCommand
4654
}
4755

4856
func runInitCommand(cmd *cobra.Command, args []string) {
49-
if destDir == "" {
57+
if destFile != "" && destDir != "" {
58+
feedback.Errorf("Can't use both --dest-file and --dest-dir flags at the same time.")
59+
os.Exit(errorcodes.ErrGeneric)
60+
}
61+
62+
var configFileAbsPath *paths.Path
63+
var absPath *paths.Path
64+
var err error
65+
66+
switch {
67+
case destFile != "":
68+
configFileAbsPath, err = paths.New(destFile).Abs()
69+
if err != nil {
70+
feedback.Errorf("Cannot find absolute path: %v", err)
71+
os.Exit(errorcodes.ErrGeneric)
72+
}
73+
74+
absPath = configFileAbsPath.Parent()
75+
case destDir == "":
5076
destDir = configuration.Settings.GetString("directories.Data")
77+
fallthrough
78+
default:
79+
absPath, err = paths.New(destDir).Abs()
80+
if err != nil {
81+
feedback.Errorf("Cannot find absolute path: %v", err)
82+
os.Exit(errorcodes.ErrGeneric)
83+
}
84+
configFileAbsPath = absPath.Join(defaultFileName)
5185
}
5286

53-
absPath, err := filepath.Abs(destDir)
54-
if err != nil {
55-
feedback.Errorf("Cannot find absolute path: %v", err)
87+
if !overwrite && configFileAbsPath.Exist() {
88+
feedback.Error("Config file already exists, use --overwrite to discard the existing one.")
5689
os.Exit(errorcodes.ErrGeneric)
5790
}
58-
configFileAbsPath := filepath.Join(absPath, defaultFileName)
5991

6092
logrus.Infof("Writing config file to: %s", absPath)
6193

62-
if err := os.MkdirAll(absPath, os.FileMode(0755)); err != nil {
94+
if err := absPath.MkdirAll(); err != nil {
6395
feedback.Errorf("Cannot create config file directory: %v", err)
6496
os.Exit(errorcodes.ErrGeneric)
6597
}
6698

67-
if err := configuration.Settings.WriteConfigAs(configFileAbsPath); err != nil {
99+
newSettings := configuration.Init(configFileAbsPath.String())
100+
configuration.BindFlags(cmd, newSettings)
101+
102+
if err := newSettings.WriteConfigAs(configFileAbsPath.String()); err != nil {
68103
feedback.Errorf("Cannot create config file: %v", err)
69104
os.Exit(errorcodes.ErrGeneric)
70105
}
71106

72-
msg := "Config file written to: " + configFileAbsPath
107+
msg := "Config file written to: " + configFileAbsPath.String()
73108
logrus.Info(msg)
74109
feedback.Print(msg)
75110
}

Diff for: configuration/configuration.go

+9
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
paths "github.com/arduino/go-paths-helper"
2626
"github.com/arduino/go-win32-utils"
2727
"github.com/sirupsen/logrus"
28+
"github.com/spf13/cobra"
2829
jww "github.com/spf13/jwalterweatherman"
2930
"github.com/spf13/viper"
3031
)
@@ -95,6 +96,14 @@ func Init(configPath string) *viper.Viper {
9596
return settings
9697
}
9798

99+
// BindFlags creates all the flags binding between the cobra Command and the instance of viper
100+
func BindFlags(cmd *cobra.Command, settings *viper.Viper) {
101+
settings.BindPFlag("logging.level", cmd.Flag("log-level"))
102+
settings.BindPFlag("logging.file", cmd.Flag("log-file"))
103+
settings.BindPFlag("logging.format", cmd.Flag("log-format"))
104+
settings.BindPFlag("board_manager.additional_urls", cmd.Flag("additional-urls"))
105+
}
106+
98107
// getDefaultArduinoDataDir returns the full path to the default arduino folder
99108
func getDefaultArduinoDataDir() string {
100109
userHomeDir, err := os.UserHomeDir()

Diff for: configuration/configuration_test.go

+60-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ func tmpDirOrDie() string {
3030
if err != nil {
3131
panic(fmt.Sprintf("error creating tmp dir: %v", err))
3232
}
33+
// Symlinks are evaluated becase the temp folder on Mac OS is inside /var, it's not writable
34+
// and is a symlink to /private/var, we want the full path so we do this
35+
dir, err = filepath.EvalSymlinks(dir)
36+
if err != nil {
37+
panic(fmt.Sprintf("error evaluating tmp dir symlink: %v", err))
38+
}
3339
return dir
3440
}
3541

@@ -73,6 +79,59 @@ func BenchmarkSearchConfigTree(b *testing.B) {
7379
}
7480

7581
func TestInit(t *testing.T) {
76-
settings := Init("")
82+
tmp := tmpDirOrDie()
83+
defer os.RemoveAll(tmp)
84+
settings := Init(filepath.Join(tmp, "arduino-cli.yaml"))
7785
require.NotNil(t, settings)
86+
87+
require.Equal(t, "info", settings.GetString("logging.level"))
88+
require.Equal(t, "text", settings.GetString("logging.format"))
89+
90+
require.Empty(t, settings.GetStringSlice("board_manager.additional_urls"))
91+
92+
require.NotEmpty(t, settings.GetString("directories.Data"))
93+
require.NotEmpty(t, settings.GetString("directories.Downloads"))
94+
require.NotEmpty(t, settings.GetString("directories.User"))
95+
96+
require.Equal(t, "50051", settings.GetString("daemon.port"))
97+
98+
require.Equal(t, true, settings.GetBool("telemetry.enabled"))
99+
require.Equal(t, ":9090", settings.GetString("telemetry.addr"))
100+
}
101+
102+
func TestFindConfigFile(t *testing.T) {
103+
configFile := FindConfigFile([]string{"--config-file"})
104+
require.Equal(t, "", configFile)
105+
106+
configFile = FindConfigFile([]string{"--config-file", "some/path/to/config"})
107+
require.Equal(t, "some/path/to/config", configFile)
108+
109+
configFile = FindConfigFile([]string{"--config-file", "some/path/to/config/arduino-cli.yaml"})
110+
require.Equal(t, "some/path/to/config/arduino-cli.yaml", configFile)
111+
112+
configFile = FindConfigFile([]string{})
113+
require.Equal(t, "", configFile)
114+
115+
// Create temporary directories
116+
tmp := tmpDirOrDie()
117+
defer os.RemoveAll(tmp)
118+
target := filepath.Join(tmp, "foo", "bar", "baz")
119+
os.MkdirAll(target, os.ModePerm)
120+
require.Nil(t, os.Chdir(target))
121+
122+
// Create a config file
123+
f, err := os.Create(filepath.Join(target, "..", "..", "arduino-cli.yaml"))
124+
require.Nil(t, err)
125+
f.Close()
126+
127+
configFile = FindConfigFile([]string{})
128+
require.Equal(t, filepath.Join(tmp, "foo", "arduino-cli.yaml"), configFile)
129+
130+
// Create another config file
131+
f, err = os.Create(filepath.Join(target, "arduino-cli.yaml"))
132+
require.Nil(t, err)
133+
f.Close()
134+
135+
configFile = FindConfigFile([]string{})
136+
require.Equal(t, filepath.Join(target, "arduino-cli.yaml"), configFile)
78137
}

Diff for: test/test_config.py

+165-3
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,178 @@
1313
# software without disclosing the source code of your own applications. To purchase
1414
# a commercial license, send an email to [email protected].
1515
from pathlib import Path
16+
import json
17+
import yaml
1618

1719

1820
def test_init(run_command, data_dir, working_dir):
1921
result = run_command("config init")
22+
assert "" == result.stderr
2023
assert result.ok
2124
assert data_dir in result.stdout
2225

2326

24-
def test_init_dest(run_command, working_dir):
25-
dest = str(Path(working_dir) / "config" / "test")
27+
def test_init_with_existing_custom_config(run_command, data_dir, working_dir, downloads_dir):
28+
result = run_command("config init --additional-urls https://example.com")
29+
assert result.ok
30+
assert data_dir in result.stdout
31+
32+
config_file = open(Path(data_dir) / "arduino-cli.yaml", "r")
33+
configs = yaml.load(config_file.read(), Loader=yaml.FullLoader)
34+
config_file.close()
35+
assert ["https://example.com"] == configs["board_manager"]["additional_urls"]
36+
assert "50051" == configs["daemon"]["port"]
37+
assert data_dir == configs["directories"]["data"]
38+
assert downloads_dir == configs["directories"]["downloads"]
39+
assert data_dir == configs["directories"]["user"]
40+
assert "" == configs["logging"]["file"]
41+
assert "text" == configs["logging"]["format"]
42+
assert "info" == configs["logging"]["level"]
43+
assert ":9090" == configs["telemetry"]["addr"]
44+
assert configs["telemetry"]["enabled"]
45+
46+
config_file_path = Path(working_dir) / "config" / "test" / "config.yaml"
47+
assert not config_file_path.exists()
48+
result = run_command(f'config init --dest-file "{config_file_path}"')
49+
assert result.ok
50+
assert str(config_file_path) in result.stdout
51+
52+
config_file = open(config_file_path, "r")
53+
configs = yaml.load(config_file.read(), Loader=yaml.FullLoader)
54+
config_file.close()
55+
assert [] == configs["board_manager"]["additional_urls"]
56+
assert "50051" == configs["daemon"]["port"]
57+
assert data_dir == configs["directories"]["data"]
58+
assert downloads_dir == configs["directories"]["downloads"]
59+
assert data_dir == configs["directories"]["user"]
60+
assert "" == configs["logging"]["file"]
61+
assert "text" == configs["logging"]["format"]
62+
assert "info" == configs["logging"]["level"]
63+
assert ":9090" == configs["telemetry"]["addr"]
64+
assert configs["telemetry"]["enabled"]
65+
66+
67+
def test_init_dest_absolute_path(run_command, working_dir):
68+
dest = Path(working_dir) / "config" / "test"
69+
expected_config_file = dest / "arduino-cli.yaml"
70+
assert not expected_config_file.exists()
71+
result = run_command(f'config init --dest-dir "{dest}"')
72+
assert result.ok
73+
assert str(expected_config_file) in result.stdout
74+
assert expected_config_file.exists()
75+
76+
77+
def test_init_dest_relative_path(run_command, working_dir):
78+
dest = Path(working_dir) / "config" / "test"
79+
expected_config_file = dest / "arduino-cli.yaml"
80+
assert not expected_config_file.exists()
81+
result = run_command('config init --dest-dir "config/test"')
82+
assert result.ok
83+
assert str(expected_config_file) in result.stdout
84+
assert expected_config_file.exists()
85+
86+
87+
def test_init_dest_flag_with_overwrite_flag(run_command, working_dir):
88+
dest = Path(working_dir) / "config" / "test"
89+
90+
expected_config_file = dest / "arduino-cli.yaml"
91+
assert not expected_config_file.exists()
92+
93+
result = run_command(f'config init --dest-dir "{dest}"')
94+
assert result.ok
95+
assert expected_config_file.exists()
96+
2697
result = run_command(f'config init --dest-dir "{dest}"')
98+
assert result.failed
99+
assert "Config file already exists, use --overwrite to discard the existing one." in result.stderr
100+
101+
result = run_command(f'config init --dest-dir "{dest}" --overwrite')
102+
assert result.ok
103+
assert str(expected_config_file) in result.stdout
104+
105+
106+
def test_init_dest_and_config_file_flags(run_command, working_dir):
107+
result = run_command('config init --dest-file "some_other_path" --dest-dir "some_path"')
108+
assert result.failed
109+
assert "Can't use both --dest-file and --dest-dir flags at the same time." in result.stderr
110+
111+
112+
def test_init_config_file_flag_absolute_path(run_command, working_dir):
113+
config_file = Path(working_dir) / "config" / "test" / "config.yaml"
114+
assert not config_file.exists()
115+
result = run_command(f'config init --dest-file "{config_file}"')
116+
assert result.ok
117+
assert str(config_file) in result.stdout
118+
assert config_file.exists()
119+
120+
121+
def test_init_config_file_flag_relative_path(run_command, working_dir):
122+
config_file = Path(working_dir) / "config.yaml"
123+
assert not config_file.exists()
124+
result = run_command('config init --dest-file "config.yaml"')
125+
assert result.ok
126+
assert str(config_file) in result.stdout
127+
assert config_file.exists()
128+
129+
130+
def test_init_config_file_flag_with_overwrite_flag(run_command, working_dir):
131+
config_file = Path(working_dir) / "config" / "test" / "config.yaml"
132+
assert not config_file.exists()
133+
134+
result = run_command(f'config init --dest-file "{config_file}"')
135+
assert result.ok
136+
assert config_file.exists()
137+
138+
result = run_command(f'config init --dest-file "{config_file}"')
139+
assert result.failed
140+
assert "Config file already exists, use --overwrite to discard the existing one." in result.stderr
141+
142+
result = run_command(f'config init --dest-file "{config_file}" --overwrite')
143+
assert result.ok
144+
assert str(config_file) in result.stdout
145+
146+
147+
def test_dump(run_command, data_dir, working_dir):
148+
# Create a config file first
149+
config_file = Path(working_dir) / "config" / "test" / "config.yaml"
150+
assert not config_file.exists()
151+
result = run_command(f'config init --dest-file "{config_file}"')
152+
assert result.ok
153+
assert config_file.exists()
154+
155+
result = run_command(f'config dump --config-file "{config_file}" --format json')
156+
assert result.ok
157+
settings_json = json.loads(result.stdout)
158+
assert [] == settings_json["board_manager"]["additional_urls"]
159+
160+
result = run_command('config init --additional-urls "https://example.com"')
161+
assert result.ok
162+
config_file = Path(data_dir) / "arduino-cli.yaml"
163+
assert str(config_file) in result.stdout
164+
assert config_file.exists()
165+
166+
result = run_command("config dump --format json")
167+
assert result.ok
168+
settings_json = json.loads(result.stdout)
169+
assert ["https://example.com"] == settings_json["board_manager"]["additional_urls"]
170+
171+
172+
def test_dump_with_config_file_flag(run_command, working_dir):
173+
# Create a config file first
174+
config_file = Path(working_dir) / "config" / "test" / "config.yaml"
175+
assert not config_file.exists()
176+
result = run_command(f'config init --dest-file "{config_file}" --additional-urls=https://example.com')
177+
assert result.ok
178+
assert config_file.exists()
179+
180+
result = run_command(f'config dump --config-file "{config_file}" --format json')
181+
assert result.ok
182+
settings_json = json.loads(result.stdout)
183+
assert ["https://example.com"] == settings_json["board_manager"]["additional_urls"]
184+
185+
result = run_command(
186+
f'config dump --config-file "{config_file}" --additional-urls=https://another-url.com --format json'
187+
)
27188
assert result.ok
28-
assert dest in result.stdout
189+
settings_json = json.loads(result.stdout)
190+
assert ["https://another-url.com"] == settings_json["board_manager"]["additional_urls"]

0 commit comments

Comments
 (0)