From 16cd87bf44f8212b7521e7c6f68ee9f254d32627 Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Thu, 3 Dec 2020 10:12:50 +0100 Subject: [PATCH 1/3] Add support for file:// schema to set local additional URLs --- cli/cli.go | 1 - cli/config/validate.go | 25 +++++----- commands/daemon/testdata/arduino-cli.yml | 1 - commands/instances.go | 59 +++++++++++++----------- configuration/configuration.go | 1 - configuration/configuration_test.go | 1 - configuration/defaults.go | 1 - docs/configuration.md | 23 --------- docs/getting-started.md | 15 +++--- test/test_core.py | 7 +++ 10 files changed, 59 insertions(+), 75 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index 617173f8324..ca711c159fc 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -104,7 +104,6 @@ func createCliCommandTree(cmd *cobra.Command) { 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.") - cmd.PersistentFlags().StringSlice("additional-paths", []string{}, "Comma-separated list of additional file paths for the Boards Manager.") configuration.BindFlags(cmd, configuration.Settings) } diff --git a/cli/config/validate.go b/cli/config/validate.go index b1a02b7267d..d448ed32955 100644 --- a/cli/config/validate.go +++ b/cli/config/validate.go @@ -21,19 +21,18 @@ import ( ) var validMap = map[string]reflect.Kind{ - "board_manager.additional_urls": reflect.Slice, - "board_manager.additional_paths": reflect.Slice, - "daemon.port": reflect.String, - "directories.data": reflect.String, - "directories.downloads": reflect.String, - "directories.user": reflect.String, - "library.enable_unsafe_install": reflect.Bool, - "logging.file": reflect.String, - "logging.format": reflect.String, - "logging.level": reflect.String, - "sketch.always_export_binaries": reflect.Bool, - "telemetry.addr": reflect.String, - "telemetry.enabled": reflect.Bool, + "board_manager.additional_urls": reflect.Slice, + "daemon.port": reflect.String, + "directories.data": reflect.String, + "directories.downloads": reflect.String, + "directories.user": reflect.String, + "library.enable_unsafe_install": reflect.Bool, + "logging.file": reflect.String, + "logging.format": reflect.String, + "logging.level": reflect.String, + "sketch.always_export_binaries": reflect.Bool, + "telemetry.addr": reflect.String, + "telemetry.enabled": reflect.Bool, } func typeOf(key string) (reflect.Kind, error) { diff --git a/commands/daemon/testdata/arduino-cli.yml b/commands/daemon/testdata/arduino-cli.yml index 29e8e6d4344..61fb5f655eb 100644 --- a/commands/daemon/testdata/arduino-cli.yml +++ b/commands/daemon/testdata/arduino-cli.yml @@ -2,7 +2,6 @@ board_manager: additional_urls: - http://foobar.com - http://example.com - additional_paths: [] daemon: port: "50051" diff --git a/commands/instances.go b/commands/instances.go index 91af60b523b..94a17de463e 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -202,24 +202,6 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexReq, downloadCB Downlo indexpath := paths.New(configuration.Settings.GetString("directories.Data")) - for _, x := range configuration.Settings.GetStringSlice("board_manager.additional_paths") { - logrus.Info("JSON PATH: ", x) - - pathJSON, _ := paths.New(x).Abs() - - if _, err := packageindex.LoadIndexNoSign(pathJSON); err != nil { - return nil, fmt.Errorf("invalid package index in %s: %s", pathJSON, err) - } - - fi, _ := os.Stat(x) - downloadCB(&rpc.DownloadProgress{ - File: "Updating index: " + pathJSON.Base(), - TotalSize: fi.Size(), - }) - downloadCB(&rpc.DownloadProgress{Completed: true}) - - } - urls := []string{globals.DefaultIndexURL} urls = append(urls, configuration.Settings.GetStringSlice("board_manager.additional_urls")...) for _, u := range urls { @@ -230,6 +212,26 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexReq, downloadCB Downlo continue } + if URL.Scheme == "file" { + path := paths.New(URL.Path) + pathJSON, err := path.Abs() + if err != nil { + return nil, fmt.Errorf("can't get absolute path of %v: %w", path, err) + } + + if _, err := packageindex.LoadIndexNoSign(pathJSON); err != nil { + return nil, fmt.Errorf("invalid package index in %s: %s", pathJSON, err) + } + + fi, _ := os.Stat(URL.Path) + downloadCB(&rpc.DownloadProgress{ + File: "Updating index: " + pathJSON.Base(), + TotalSize: fi.Size(), + }) + downloadCB(&rpc.DownloadProgress{Completed: true}) + continue + } + logrus.WithField("url", URL).Print("Updating index") var tmp *paths.Path @@ -665,16 +667,21 @@ func createInstance(ctx context.Context, getLibOnly bool) (*createInstanceResult continue } - if err := res.Pm.LoadPackageIndex(URL); err != nil { - res.PlatformIndexErrors = append(res.PlatformIndexErrors, err.Error()) - } - } + if URL.Scheme == "file" { + path := paths.New(URL.Path) + pathJSON, err := path.Abs() + if err != nil { + return nil, fmt.Errorf("can't get absolute path of %v: %w", path, err) + } - for _, x := range configuration.Settings.GetStringSlice("board_manager.additional_paths") { - pathJSON, _ := paths.New(x).Abs() + _, err = res.Pm.LoadPackageIndexFromFile(pathJSON) + if err != nil { + res.PlatformIndexErrors = append(res.PlatformIndexErrors, err.Error()) + } + continue + } - _, err := res.Pm.LoadPackageIndexFromFile(pathJSON) - if err != nil { + if err := res.Pm.LoadPackageIndex(URL); err != nil { res.PlatformIndexErrors = append(res.PlatformIndexErrors, err.Error()) } } diff --git a/configuration/configuration.go b/configuration/configuration.go index 841bd42ea76..01f424c586d 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -76,7 +76,6 @@ func BindFlags(cmd *cobra.Command, settings *viper.Viper) { 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")) - settings.BindPFlag("board_manager.additional_paths", cmd.Flag("additional-paths")) } // getDefaultArduinoDataDir returns the full path to the default arduino folder diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go index 8bd86ab7479..45ee9261535 100644 --- a/configuration/configuration_test.go +++ b/configuration/configuration_test.go @@ -89,7 +89,6 @@ func TestInit(t *testing.T) { require.Equal(t, "text", settings.GetString("logging.format")) require.Empty(t, settings.GetStringSlice("board_manager.additional_urls")) - require.Empty(t, settings.GetStringSlice("board_manager.additional_paths")) require.NotEmpty(t, settings.GetString("directories.Data")) require.NotEmpty(t, settings.GetString("directories.Downloads")) diff --git a/configuration/defaults.go b/configuration/defaults.go index d07e9e2169d..58eae4921a7 100644 --- a/configuration/defaults.go +++ b/configuration/defaults.go @@ -33,7 +33,6 @@ func SetDefaults(settings *viper.Viper) { // Boards Manager settings.SetDefault("board_manager.additional_urls", []string{}) - settings.SetDefault("board_manager.additional_paths", []string{}) // arduino directories settings.SetDefault("directories.Data", getDefaultArduinoDataDir()) diff --git a/docs/configuration.md b/docs/configuration.md index ab47f3147de..2b1e67c6a81 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -2,8 +2,6 @@ - `board_manager` - `additional_urls` - the URLs to any additional Boards Manager package index files needed for your boards platforms. - - `additional_paths` - the absolute file paths to any additional Boards Manager package index files needed for your - boards platforms. - `daemon` - options related to running Arduino CLI as a [gRPC] server. - `port` - TCP port used for gRPC client connections. - `directories` - directories used by Arduino CLI. @@ -68,12 +66,6 @@ Setting an additional Boards Manager URL using the `ARDUINO_BOARD_MANAGER_ADDITI $ export ARDUINO_BOARD_MANAGER_ADDITIONAL_URLS=https://downloads.arduino.cc/packages/package_staging_index.json ``` -Setting an additional Boards Manager file using the `ARDUINO_BOARD_MANAGER_ADDITIONAL_PATHS` environment variable: - -```sh -$ export ARDUINO_BOARD_MANAGER_ADDITIONAL_PATHS=/path/to/your/package_staging_index.json -``` - ### Configuration file [`arduino-cli config init`][arduino-cli config init] creates or updates a configuration file with the current @@ -136,21 +128,6 @@ Doing the same using a TOML format file: additional_urls = [ "https://downloads.arduino.cc/packages/package_staging_index.json" ] ``` -Setting an additional Boards Manager File using a YAML format configuration file: - -```yaml -board_manager: - additional_paths: - - /path/to/your/package_staging_index.json -``` - -Doing the same using a TOML format file: - -```toml -[board_manager] -additional_paths = [ "/path/to/your/package_staging_index.json" ] -``` - [grpc]: https://grpc.io [sketchbook directory]: sketch-specification.md#sketchbook [arduino cli lib install]: commands/arduino-cli_lib_install.md diff --git a/docs/getting-started.md b/docs/getting-started.md index 5d0f6bd4947..a5dd529af28 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -183,12 +183,12 @@ For example, to add the NRF52832 core, edit the configuration file and change th ```yaml board_manager: - additional_paths: - - /absolute/path/to/your/package_nrf52832_index.json + additional_urls: + - https://arduino.esp8266.com/stable/package_esp8266com_index.json + - file:///absolute/path/to/your/package_nrf52832_index.json ``` -From now on, commands supporting custom cores will automatically use the additional URL and additional paths from the -configuration file: +From now on, commands supporting custom cores will automatically use the additional URL from the configuration file: ```sh $ arduino-cli core update-index @@ -214,14 +214,13 @@ ID Version Name esp8266:esp8266 2.5.2 esp8266 ``` -The same applies to the additional package index file provided by file paths. Use the `--additional-paths` option, that -has to be specified every time and for every command that operates on a 3rd party platform core, for example: +The same applies to the additional package index file provided by file paths: ```sh -$ arduino-cli core update-index --additional-paths /absolute/path/to/your/package_esp8266com_index.json +$ arduino-cli core update-index --additional-urls file:///absolute/path/to/your/package_esp8266com_index.json Updating index: package_esp8266com_index.json downloaded -$ arduino-cli core search esp8266 --additional-paths /absolute/path/to/your/package_esp8266com_index.json +$ arduino-cli core search esp8266 --additional-urls file:///absolute/path/to/your/package_esp8266com_index.json ID Version Name esp8266:esp8266 2.5.2 esp8266 ``` diff --git a/test/test_core.py b/test/test_core.py index c7d107bdeae..d365c5badc6 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -353,3 +353,10 @@ def ordered(obj): return obj assert ordered(installed_json) == ordered(expected_installed_json) + + +def test_core_update_with_local_url(run_command): + test_index = Path(__file__).parent / "testdata" / "test_index.json" + res = run_command(f'core update-index --additional-urls="file://{test_index}"') + assert res.ok + assert "Updating index: test_index.json downloaded" in res.stdout From 7edf79e7d3b24f91b340c9e0c436d9c878a080db Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Thu, 3 Dec 2020 14:34:09 +0100 Subject: [PATCH 2/3] Fix local path parsing on Windows --- commands/instances.go | 17 ++++++++++++++--- test/test_core.py | 5 ++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/commands/instances.go b/commands/instances.go index 94a17de463e..0d403477637 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -23,6 +23,7 @@ import ( "net/url" "os" "path" + "runtime" "github.com/arduino/arduino-cli/arduino/builder" "github.com/arduino/arduino-cli/arduino/cores" @@ -212,8 +213,15 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexReq, downloadCB Downlo continue } + logrus.WithField("url", URL).Print("Updating index") + if URL.Scheme == "file" { path := paths.New(URL.Path) + if runtime.GOOS == "windows" { + // Parsed local file URLs on Windows are returned with a leading / + // so we remove it + path = paths.New(URL.Path[1:]) + } pathJSON, err := path.Abs() if err != nil { return nil, fmt.Errorf("can't get absolute path of %v: %w", path, err) @@ -223,7 +231,7 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexReq, downloadCB Downlo return nil, fmt.Errorf("invalid package index in %s: %s", pathJSON, err) } - fi, _ := os.Stat(URL.Path) + fi, _ := os.Stat(path.String()) downloadCB(&rpc.DownloadProgress{ File: "Updating index: " + pathJSON.Base(), TotalSize: fi.Size(), @@ -232,8 +240,6 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexReq, downloadCB Downlo continue } - logrus.WithField("url", URL).Print("Updating index") - var tmp *paths.Path if tmpFile, err := ioutil.TempFile("", ""); err != nil { return nil, fmt.Errorf("creating temp file for index download: %s", err) @@ -669,6 +675,11 @@ func createInstance(ctx context.Context, getLibOnly bool) (*createInstanceResult if URL.Scheme == "file" { path := paths.New(URL.Path) + if runtime.GOOS == "windows" { + // Parsed local file URLs on Windows are returned with a leading / + // so we remove it + path = paths.New(URL.Path[1:]) + } pathJSON, err := path.Abs() if err != nil { return nil, fmt.Errorf("can't get absolute path of %v: %w", path, err) diff --git a/test/test_core.py b/test/test_core.py index d365c5badc6..e2813ddad62 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -356,7 +356,10 @@ def ordered(obj): def test_core_update_with_local_url(run_command): - test_index = Path(__file__).parent / "testdata" / "test_index.json" + test_index = str(Path(__file__).parent / "testdata" / "test_index.json") + if platform.system() == "Windows": + test_index = f"/{test_index}".replace("\\", "/") + res = run_command(f'core update-index --additional-urls="file://{test_index}"') assert res.ok assert "Updating index: test_index.json downloaded" in res.stdout From 73024f9feac9211cd8e5f10ea64b8dece94391aa Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Thu, 3 Dec 2020 17:14:08 +0100 Subject: [PATCH 3/3] Move URL parsing in utility function --- arduino/utils/url.go | 35 +++++++++++++++++++++++++++ arduino/utils/url_test.go | 51 +++++++++++++++++++++++++++++++++++++++ commands/instances.go | 30 ++++++----------------- 3 files changed, 93 insertions(+), 23 deletions(-) create mode 100644 arduino/utils/url.go create mode 100644 arduino/utils/url_test.go diff --git a/arduino/utils/url.go b/arduino/utils/url.go new file mode 100644 index 00000000000..a4f6fa44f0d --- /dev/null +++ b/arduino/utils/url.go @@ -0,0 +1,35 @@ +// This file is part of arduino-cli. +// +// Copyright 2020 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package utils + +import ( + "net/url" + "runtime" +) + +// URLParse parses a raw URL string and handles local files URLs depending on the platform +func URLParse(rawURL string) (*url.URL, error) { + URL, err := url.Parse(rawURL) + if err != nil { + return nil, err + } + if URL.Scheme == "file" && runtime.GOOS == "windows" { + // Parsed local file URLs on Windows are returned with a leading / + // so we remove it + URL.Path = URL.Path[1:] + } + return URL, nil +} diff --git a/arduino/utils/url_test.go b/arduino/utils/url_test.go new file mode 100644 index 00000000000..e7874a30cdd --- /dev/null +++ b/arduino/utils/url_test.go @@ -0,0 +1,51 @@ +// This file is part of arduino-cli. +// +// Copyright 2020 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package utils + +import ( + "fmt" + "runtime" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestURLParse(t *testing.T) { + type test struct { + URL string + ExpectedHost string + ExpectedPath string + Skip bool + } + onWindows := runtime.GOOS == "windows" + tests := []test{ + {"https://example.com", "example.com", "", false}, + {"https://example.com/some/path", "example.com", "/some/path", false}, + {"file:///home/user/some/path", "", "/home/user/some/path", onWindows}, + {"file:///C:/Users/me/some/path", "", "C:/Users/me/some/path", !onWindows}, + } + + for i, test := range tests { + t.Run(fmt.Sprintf("URLParseTest%02d", i), func(t *testing.T) { + if test.Skip { + t.Skip("Skipped") + } + res, err := URLParse(test.URL) + require.NoError(t, err) + require.Equal(t, test.ExpectedPath, res.Path) + }) + } +} diff --git a/commands/instances.go b/commands/instances.go index 0d403477637..304bc33b5e6 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -23,7 +23,6 @@ import ( "net/url" "os" "path" - "runtime" "github.com/arduino/arduino-cli/arduino/builder" "github.com/arduino/arduino-cli/arduino/cores" @@ -33,6 +32,7 @@ import ( "github.com/arduino/arduino-cli/arduino/libraries/librariesindex" "github.com/arduino/arduino-cli/arduino/libraries/librariesmanager" "github.com/arduino/arduino-cli/arduino/security" + "github.com/arduino/arduino-cli/arduino/utils" "github.com/arduino/arduino-cli/cli/globals" "github.com/arduino/arduino-cli/configuration" rpc "github.com/arduino/arduino-cli/rpc/commands" @@ -207,7 +207,7 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexReq, downloadCB Downlo urls = append(urls, configuration.Settings.GetStringSlice("board_manager.additional_urls")...) for _, u := range urls { logrus.Info("URL: ", u) - URL, err := url.Parse(u) + URL, err := utils.URLParse(u) if err != nil { logrus.Warnf("unable to parse additional URL: %s", u) continue @@ -217,23 +217,13 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexReq, downloadCB Downlo if URL.Scheme == "file" { path := paths.New(URL.Path) - if runtime.GOOS == "windows" { - // Parsed local file URLs on Windows are returned with a leading / - // so we remove it - path = paths.New(URL.Path[1:]) - } - pathJSON, err := path.Abs() - if err != nil { - return nil, fmt.Errorf("can't get absolute path of %v: %w", path, err) - } - - if _, err := packageindex.LoadIndexNoSign(pathJSON); err != nil { - return nil, fmt.Errorf("invalid package index in %s: %s", pathJSON, err) + if _, err := packageindex.LoadIndexNoSign(path); err != nil { + return nil, fmt.Errorf("invalid package index in %s: %s", path, err) } fi, _ := os.Stat(path.String()) downloadCB(&rpc.DownloadProgress{ - File: "Updating index: " + pathJSON.Base(), + File: "Updating index: " + path.Base(), TotalSize: fi.Size(), }) downloadCB(&rpc.DownloadProgress{Completed: true}) @@ -667,7 +657,7 @@ func createInstance(ctx context.Context, getLibOnly bool) (*createInstanceResult urls := []string{globals.DefaultIndexURL} urls = append(urls, configuration.Settings.GetStringSlice("board_manager.additional_urls")...) for _, u := range urls { - URL, err := url.Parse(u) + URL, err := utils.URLParse(u) if err != nil { logrus.Warnf("Unable to parse index URL: %s, skip...", u) continue @@ -675,17 +665,11 @@ func createInstance(ctx context.Context, getLibOnly bool) (*createInstanceResult if URL.Scheme == "file" { path := paths.New(URL.Path) - if runtime.GOOS == "windows" { - // Parsed local file URLs on Windows are returned with a leading / - // so we remove it - path = paths.New(URL.Path[1:]) - } - pathJSON, err := path.Abs() if err != nil { return nil, fmt.Errorf("can't get absolute path of %v: %w", path, err) } - _, err = res.Pm.LoadPackageIndexFromFile(pathJSON) + _, err = res.Pm.LoadPackageIndexFromFile(path) if err != nil { res.PlatformIndexErrors = append(res.PlatformIndexErrors, err.Error()) }