Move URL parsing in utility function

Author: silvanocerza

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

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

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,25 +657,19 @@ 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
 			}
 
 			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())
 				}