Fix update indexes running when not necessary (#1063)

* Libs and cores update is now correctly run only on first CLI execution

* Enhance error messages when updating indexes or local index files are missing

cli/instance/instance.go

@@ -17,10 +17,14 @@ package instance
 
 import (
 	"context"
+	"fmt"
+	"strings"
 
 	"github.com/arduino/arduino-cli/cli/output"
 	"github.com/arduino/arduino-cli/commands"
+	"github.com/arduino/arduino-cli/configuration"
 	rpc "github.com/arduino/arduino-cli/rpc/commands"
+	"github.com/arduino/go-paths-helper"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -51,9 +55,13 @@ func getInitResponse() (*rpc.InitResp, error) {
 		return nil, errors.Wrap(err, "creating instance")
 	}
 
-	// Init() succeeded but there were errors loading library indexes,
-	// let's rescan and try again
-	if resp.GetLibrariesIndexError() != "" {
+	// Gets the data directory to verify if library_index.json and package_index.json exist
+	dataDir := paths.New(configuration.Settings.GetString("directories.data"))
+
+	// The library_index.json file doesn't exists, that means the CLI is run for the first time
+	// so we proceed with the first update that downloads the file
+	libraryIndex := dataDir.Join("library_index.json")
+	if libraryIndex.NotExist() {
 		logrus.Warnf("There were errors loading the library index, trying again...")
 
 		// update all indexes
@@ -80,15 +88,11 @@ func getInitResponse() (*rpc.InitResp, error) {
 		resp.PlatformsIndexErrors = rescanResp.PlatformsIndexErrors
 	}
 
-	// Init() succeeded but there were errors loading platform indexes,
-	// let's rescan and try again
-	if resp.GetPlatformsIndexErrors() != nil {
-
-		// log each error
-		for _, err := range resp.GetPlatformsIndexErrors() {
-			logrus.Errorf("Error loading platform index: %v", err)
-		}
-
+	// The package_index.json file doesn't exists, that means the CLI is run for the first time,
+	// similarly to the library update we download that file and all the other package indexes
+	// from additional_urls
+	packageIndex := dataDir.Join("package_index.json")
+	if packageIndex.NotExist() {
 		// update platform index
 		_, err := commands.UpdateIndex(context.Background(),
 			&rpc.UpdateIndexReq{Instance: resp.GetInstance()}, output.ProgressBar())
@@ -124,8 +128,7 @@ func checkPlatformErrors(resp *rpc.InitResp) error {
 		for _, err := range resp.GetPlatformsIndexErrors() {
 			logrus.Errorf("Error loading platform index: %v", err)
 		}
-		// return
-		return errors.New("There were errors loading platform indexes")
+		return fmt.Errorf("error loading platform index: \n%v", strings.Join(resp.GetPlatformsIndexErrors(), "\n"))
 	}
 
 	return nil

commands/download.go

@@ -16,6 +16,7 @@
 package commands
 
 import (
+	"errors"
 	"time"
 
 	"github.com/arduino/arduino-cli/httpclient"
@@ -60,6 +61,10 @@ func Download(d *downloader.Downloader, label string, downloadCB DownloadProgres
 	if d.Error() != nil {
 		return d.Error()
 	}
+	// The URL is not reachable for some reason
+	if d.Resp.StatusCode >= 400 && d.Resp.StatusCode <= 599 {
+		return errors.New(d.Resp.Status)
+	}
 	downloadCB(&rpc.DownloadProgress{Completed: true})
 	return nil
 }

commands/instances.go

@@ -230,9 +230,9 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexReq, downloadCB Downlo
 			return nil, fmt.Errorf("downloading index %s: %s", URL, err)
 		}
 		coreIndexPath := indexpath.Join(path.Base(URL.Path))
-		Download(d, "Updating index: "+coreIndexPath.Base(), downloadCB)
-		if d.Error() != nil {
-			return nil, fmt.Errorf("downloading index %s: %s", URL, d.Error())
+		err = Download(d, "Updating index: "+coreIndexPath.Base(), downloadCB)
+		if err != nil {
+			return nil, fmt.Errorf("downloading index %s: %s", URL, err)
 		}
 
 		// Check for signature

go.mod

@@ -21,15 +21,12 @@ require (
 	github.com/h2non/filetype v1.0.8 // indirect
 	github.com/imjasonmiller/godice v0.1.2
 	github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8 // indirect
-	github.com/kr/pretty v0.2.0 // indirect
 	github.com/kr/text v0.2.0 // indirect
 	github.com/leonelquinteros/gotext v1.4.0
-	github.com/marcinbor85/gohex v0.0.0-20200531163658-baab2527a9a2
 	github.com/mattn/go-colorable v0.1.2
 	github.com/mattn/go-isatty v0.0.8
 	github.com/mattn/go-runewidth v0.0.9 // indirect
 	github.com/miekg/dns v1.0.5 // indirect
-	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
 	github.com/oleksandr/bonjour v0.0.0-20160508152359-5dcf00d8b228 // indirect
 	github.com/pkg/errors v0.9.1
 	github.com/pmylund/sortutil v0.0.0-20120526081524-abeda66eb583
@@ -40,11 +37,10 @@ require (
 	github.com/sirupsen/logrus v1.4.2
 	github.com/spf13/cobra v1.0.1-0.20200710201246-675ae5f5a98c
 	github.com/spf13/jwalterweatherman v1.0.0
-	github.com/spf13/pflag v1.0.3
 	github.com/spf13/viper v1.6.2
 	github.com/stretchr/testify v1.6.1
 	go.bug.st/cleanup v1.0.0
-	go.bug.st/downloader/v2 v2.0.1
+	go.bug.st/downloader/v2 v2.1.0
 	go.bug.st/relaxed-semver v0.0.0-20190922224835-391e10178d18
 	go.bug.st/serial v1.1.1
 	go.bug.st/serial.v1 v0.0.0-20180827123349-5f7892a7bb45 // indirect
@@ -53,7 +49,6 @@ require (
 	golang.org/x/text v0.3.2
 	google.golang.org/grpc v1.27.0
 	google.golang.org/protobuf v1.25.0
-	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
 	gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce // indirect
 	gopkg.in/src-d/go-git.v4 v4.13.1
 	gopkg.in/yaml.v2 v2.3.0

test/test_compile.py

@@ -538,3 +538,29 @@ def test_compile_with_export_binaries_config(run_command, data_dir, downloads_di
     assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.hex").exists()
     assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.with_bootloader.bin").exists()
     assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.with_bootloader.hex").exists()
+
+
+def test_compile_with_invalid_url(run_command, data_dir):
+    # Init the environment explicitly
+    run_command("core update-index")
+
+    # Download latest AVR
+    run_command("core install arduino:avr")
+
+    sketch_name = "CompileWithInvalidURL"
+    sketch_path = Path(data_dir, sketch_name)
+    fqbn = "arduino:avr:uno"
+
+    # Create a test sketch
+    assert run_command(f'sketch new "{sketch_path}"')
+
+    # Create settings with custom invalid URL
+    assert run_command("config init --dest-dir . --additional-urls https://example.com/package_example_index.json")
+
+    # Verifies compilation fails cause of missing local index file
+    res = run_command(f'compile -b {fqbn} "{sketch_path}"')
+    assert res.failed
+    lines = [l.strip() for l in res.stderr.splitlines()]
+    assert "Error creating instance: error loading platform index:" in lines
+    expected_index_file = Path(data_dir, "package_example_index.json")
+    assert f"loading json index file {expected_index_file}: " + f"open {expected_index_file}:" in lines[-1]

test/test_core.py

@@ -174,10 +174,30 @@ def test_core_search_no_args(run_command, httpserver):
     assert len(platforms) == num_platforms
 
 
-def test_core_updateindex_invalid_url(run_command):
-    url = "http://www.invalid-domain-asjkdakdhadjkh.com/package_example_index.json"
-    result = run_command("core update-index --additional-urls={}".format(url))
+def test_core_updateindex_url_not_found(run_command, httpserver):
+    assert run_command("core update-index")
+
+    # Brings up a local server to fake a failure
+    httpserver.expect_request("/test_index.json").respond_with_data(status=404)
+    url = httpserver.url_for("/test_index.json")
+
+    result = run_command(f"core update-index --additional-urls={url}")
+    assert result.failed
+    lines = [l.strip() for l in result.stderr.splitlines()]
+    assert f"Error updating index: downloading index {url}: 404 NOT FOUND" in lines
+
+
+def test_core_updateindex_internal_server_error(run_command, httpserver):
+    assert run_command("core update-index")
+
+    # Brings up a local server to fake a failure
+    httpserver.expect_request("/test_index.json").respond_with_data(status=500)
+    url = httpserver.url_for("/test_index.json")
+
+    result = run_command(f"core update-index --additional-urls={url}")
     assert result.failed
+    lines = [l.strip() for l in result.stderr.splitlines()]
+    assert f"Error updating index: downloading index {url}: 500 INTERNAL SERVER ERROR" in lines
 
 
 def test_core_install_without_updateindex(run_command):

test/test_update.py

@@ -47,3 +47,29 @@ def test_update_showing_outdated(run_command):
     assert "Updating index: library_index.json downloaded" in lines
     assert lines[-5].startswith("Arduino AVR Boards")
     assert lines[-2].startswith("USBHost")
+
+
+def test_update_with_url_not_found(run_command, httpserver):
+    assert run_command("update")
+
+    # Brings up a local server to fake a failure
+    httpserver.expect_request("/test_index.json").respond_with_data(status=404)
+    url = httpserver.url_for("/test_index.json")
+
+    res = run_command(f"update --additional-urls={url}")
+    assert res.failed
+    lines = [l.strip() for l in res.stderr.splitlines()]
+    assert f"Error updating core and libraries index: downloading index {url}: 404 NOT FOUND" in lines
+
+
+def test_update_with_url_internal_server_error(run_command, httpserver):
+    assert run_command("update")
+
+    # Brings up a local server to fake a failure
+    httpserver.expect_request("/test_index.json").respond_with_data(status=500)
+    url = httpserver.url_for("/test_index.json")
+
+    res = run_command(f"update --additional-urls={url}")
+    assert res.failed
+    lines = [l.strip() for l in res.stderr.splitlines()]
+    assert f"Error updating core and libraries index: downloading index {url}: 500 INTERNAL SERVER ERROR" in lines