[breaking] gRPC UpdateIndex and UpdateLibrariesIndex improvements (#2569)

* gRPC UpdateIndex and UpdateLibrariesIndex improvements

The two calls now have the update_if_older_than_secs field that allows
to avoid updating the index if it has been already updated.

Also the response is more explicit with oneof(..) clause and the status
of each update (in case of multiple index update) is returned in the response.

* Do not make any output in case of skipped/already-up-to-date

* Added json output to 'core update index'

* Added json output to 'lib update index'

* Removed unused function

* Workaround for Windows paths in URI

Author: cmaglie

Diff for: commands/daemon/daemon.go

@@ -121,18 +121,36 @@ func (s *ArduinoCoreServerImpl) Destroy(ctx context.Context, req *rpc.DestroyReq
 // UpdateIndex FIXMEDOC
 func (s *ArduinoCoreServerImpl) UpdateIndex(req *rpc.UpdateIndexRequest, stream rpc.ArduinoCoreService_UpdateIndexServer) error {
 	syncSend := NewSynchronizedSend(stream.Send)
-	err := commands.UpdateIndex(stream.Context(), req,
-		func(p *rpc.DownloadProgress) { syncSend.Send(&rpc.UpdateIndexResponse{DownloadProgress: p}) },
+	res, err := commands.UpdateIndex(stream.Context(), req,
+		func(p *rpc.DownloadProgress) {
+			syncSend.Send(&rpc.UpdateIndexResponse{
+				Message: &rpc.UpdateIndexResponse_DownloadProgress{DownloadProgress: p},
+			})
+		},
 	)
+	if res != nil {
+		syncSend.Send(&rpc.UpdateIndexResponse{
+			Message: &rpc.UpdateIndexResponse_Result_{Result: res},
+		})
+	}
 	return convertErrorToRPCStatus(err)
 }
 
 // UpdateLibrariesIndex FIXMEDOC
 func (s *ArduinoCoreServerImpl) UpdateLibrariesIndex(req *rpc.UpdateLibrariesIndexRequest, stream rpc.ArduinoCoreService_UpdateLibrariesIndexServer) error {
 	syncSend := NewSynchronizedSend(stream.Send)
-	err := commands.UpdateLibrariesIndex(stream.Context(), req,
-		func(p *rpc.DownloadProgress) { syncSend.Send(&rpc.UpdateLibrariesIndexResponse{DownloadProgress: p}) },
+	res, err := commands.UpdateLibrariesIndex(stream.Context(), req,
+		func(p *rpc.DownloadProgress) {
+			syncSend.Send(&rpc.UpdateLibrariesIndexResponse{
+				Message: &rpc.UpdateLibrariesIndexResponse_DownloadProgress{DownloadProgress: p},
+			})
+		},
 	)
+	if res != nil {
+		syncSend.Send(&rpc.UpdateLibrariesIndexResponse{
+			Message: &rpc.UpdateLibrariesIndexResponse_Result_{Result: res},
+		})
+	}
 	return convertErrorToRPCStatus(err)
 }
 

Diff for: commands/instances.go

@@ -20,7 +20,9 @@ import (
 	"fmt"
 	"net/url"
 	"path/filepath"
+	"runtime"
 	"strings"
+	"time"
 
 	"github.com/arduino/arduino-cli/commands/cmderrors"
 	"github.com/arduino/arduino-cli/commands/internal/instances"
@@ -406,30 +408,59 @@ func Destroy(ctx context.Context, req *rpc.DestroyRequest) (*rpc.DestroyResponse
 }
 
 // UpdateLibrariesIndex updates the library_index.json
-func UpdateLibrariesIndex(ctx context.Context, req *rpc.UpdateLibrariesIndexRequest, downloadCB rpc.DownloadProgressCB) error {
+func UpdateLibrariesIndex(ctx context.Context, req *rpc.UpdateLibrariesIndexRequest, downloadCB rpc.DownloadProgressCB) (*rpc.UpdateLibrariesIndexResponse_Result, error) {
 	logrus.Info("Updating libraries index")
+
 	pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
 	if err != nil {
-		return err
+		return nil, err
 	}
 	indexDir := pme.IndexDir
 	release()
 
+	index := globals.LibrariesIndexResource
+	result := func(status rpc.IndexUpdateReport_Status) *rpc.UpdateLibrariesIndexResponse_Result {
+		return &rpc.UpdateLibrariesIndexResponse_Result{
+			LibrariesIndex: &rpc.IndexUpdateReport{
+				IndexUrl: globals.LibrariesIndexResource.URL.String(),
+				Status:   status,
+			},
+		}
+	}
+
+	// Create the index directory if it doesn't exist
 	if err := indexDir.MkdirAll(); err != nil {
-		return &cmderrors.PermissionDeniedError{Message: tr("Could not create index directory"), Cause: err}
+		return result(rpc.IndexUpdateReport_STATUS_FAILED), &cmderrors.PermissionDeniedError{Message: tr("Could not create index directory"), Cause: err}
+	}
+
+	// Check if the index file is already up-to-date
+	indexFileName, _ := index.IndexFileName()
+	if info, err := indexDir.Join(indexFileName).Stat(); err == nil {
+		ageSecs := int64(time.Since(info.ModTime()).Seconds())
+		if ageSecs < req.GetUpdateIfOlderThanSecs() {
+			return result(rpc.IndexUpdateReport_STATUS_ALREADY_UP_TO_DATE), nil
+		}
 	}
 
+	// Perform index update
 	if err := globals.LibrariesIndexResource.Download(indexDir, downloadCB); err != nil {
-		return err
+		return nil, err
 	}
 
-	return nil
+	return result(rpc.IndexUpdateReport_STATUS_UPDATED), nil
 }
 
 // UpdateIndex FIXMEDOC
-func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexRequest, downloadCB rpc.DownloadProgressCB) error {
+func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexRequest, downloadCB rpc.DownloadProgressCB) (*rpc.UpdateIndexResponse_Result, error) {
 	if !instances.IsValid(req.GetInstance()) {
-		return &cmderrors.InvalidInstanceError{}
+		return nil, &cmderrors.InvalidInstanceError{}
+	}
+
+	report := func(indexURL *url.URL, status rpc.IndexUpdateReport_Status) *rpc.IndexUpdateReport {
+		return &rpc.IndexUpdateReport{
+			IndexUrl: indexURL.String(),
+			Status:   status,
+		}
 	}
 
 	indexpath := configuration.DataDir(configuration.Settings)
@@ -440,46 +471,75 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexRequest, downloadCB rp
 	}
 
 	failed := false
+	result := &rpc.UpdateIndexResponse_Result{}
 	for _, u := range urls {
-		URL, err := utils.URLParse(u)
+		URL, err := url.Parse(u)
 		if err != nil {
 			logrus.Warnf("unable to parse additional URL: %s", u)
 			msg := fmt.Sprintf("%s: %v", tr("Unable to parse URL"), err)
 			downloadCB.Start(u, tr("Downloading index: %s", u))
 			downloadCB.End(false, msg)
 			failed = true
+			result.UpdatedIndexes = append(result.UpdatedIndexes, report(URL, rpc.IndexUpdateReport_STATUS_FAILED))
 			continue
 		}
 
 		logrus.WithField("url", URL).Print("Updating index")
 
 		if URL.Scheme == "file" {
-			downloadCB.Start(u, tr("Downloading index: %s", filepath.Base(URL.Path)))
 			path := paths.New(URL.Path)
+			if URL.Scheme == "file" && runtime.GOOS == "windows" && len(URL.Path) > 1 {
+				// https://github.com/golang/go/issues/32456
+				// Parsed local file URLs on Windows are returned with a leading / so we remove it
+				path = paths.New(URL.Path[1:])
+			}
 			if _, err := packageindex.LoadIndexNoSign(path); err != nil {
 				msg := fmt.Sprintf("%s: %v", tr("Invalid package index in %s", path), err)
+				downloadCB.Start(u, tr("Downloading index: %s", filepath.Base(URL.Path)))
 				downloadCB.End(false, msg)
 				failed = true
+				result.UpdatedIndexes = append(result.UpdatedIndexes, report(URL, rpc.IndexUpdateReport_STATUS_FAILED))
 			} else {
-				downloadCB.End(true, "")
+				result.UpdatedIndexes = append(result.UpdatedIndexes, report(URL, rpc.IndexUpdateReport_STATUS_SKIPPED))
 			}
 			continue
 		}
 
+		// Check if the index is up-to-date
 		indexResource := resources.IndexResource{URL: URL}
+		indexFileName, err := indexResource.IndexFileName()
+		if err != nil {
+			downloadCB.Start(u, tr("Downloading index: %s", filepath.Base(URL.Path)))
+			downloadCB.End(false, tr("Invalid index URL: %s", err))
+			failed = true
+			result.UpdatedIndexes = append(result.UpdatedIndexes, report(URL, rpc.IndexUpdateReport_STATUS_FAILED))
+			continue
+		}
+		indexFile := indexpath.Join(indexFileName)
+		if info, err := indexFile.Stat(); err == nil {
+			ageSecs := int64(time.Since(info.ModTime()).Seconds())
+			if ageSecs < req.GetUpdateIfOlderThanSecs() {
+				result.UpdatedIndexes = append(result.UpdatedIndexes, report(URL, rpc.IndexUpdateReport_STATUS_ALREADY_UP_TO_DATE))
+				continue
+			}
+		}
+
 		if strings.HasSuffix(URL.Host, "arduino.cc") && strings.HasSuffix(URL.Path, ".json") {
 			indexResource.SignatureURL, _ = url.Parse(u) // should not fail because we already parsed it
 			indexResource.SignatureURL.Path += ".sig"
 		}
 		if err := indexResource.Download(indexpath, downloadCB); err != nil {
 			failed = true
+			result.UpdatedIndexes = append(result.UpdatedIndexes, report(URL, rpc.IndexUpdateReport_STATUS_FAILED))
+		} else {
+			result.UpdatedIndexes = append(result.UpdatedIndexes, report(URL, rpc.IndexUpdateReport_STATUS_UPDATED))
 		}
 	}
 
 	if failed {
-		return &cmderrors.FailedDownloadError{Message: tr("Some indexes could not be updated.")}
+		return result, &cmderrors.FailedDownloadError{Message: tr("Some indexes could not be updated.")}
 	}
-	return nil
+	return result, nil
 }
 
 // firstUpdate downloads libraries and packages indexes if they don't exist.
@@ -493,7 +553,7 @@ func firstUpdate(ctx context.Context, instance *rpc.Instance, downloadCb func(ms
 		// 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
 		req := &rpc.UpdateLibrariesIndexRequest{Instance: instance}
-		if err := UpdateLibrariesIndex(ctx, req, downloadCb); err != nil {
+		if _, err := UpdateLibrariesIndex(ctx, req, downloadCb); err != nil {
 			return err
 		}
 	}
@@ -515,7 +575,7 @@ func firstUpdate(ctx context.Context, instance *rpc.Instance, downloadCb func(ms
 			// library update we download that file and all the other package indexes from
 			// additional_urls
 			req := &rpc.UpdateIndexRequest{Instance: instance}
-			if err := UpdateIndex(ctx, req, downloadCb); err != nil {
+			if _, err := UpdateIndex(ctx, req, downloadCb); err != nil {
 				return err
 			}
 			break

Diff for: docs/UPGRADING.md

@@ -4,6 +4,76 @@ Here you can find a list of migration guides to handle breaking changes between
 
 ## 0.36.0
 
+### The gRPC `cc.arduino.cli.commands.v1.UpdateIndexResponse` and `UpdateLibrariesIndexResponse` have changed.
+
+The responses coming from the update index commands:
+
+```proto
+message UpdateIndexResponse {
+  // Progress of the package index download.
+  DownloadProgress download_progress = 1;
+}
+
+message UpdateLibrariesIndexResponse {
+  // Progress of the libraries index download.
+  DownloadProgress download_progress = 1;
+}
+```
+
+are now more explicit and contains details about the result of the operation:
+
+```proto
+message UpdateIndexResponse {
+  message Result {
+    // The result of the packages index update.
+    repeated IndexUpdateReport updated_indexes = 1;
+  }
+  oneof message {
+    // Progress of the package index download.
+    DownloadProgress download_progress = 1;
+    // The result of the index update.
+    Result result = 2;
+  }
+}
+
+message UpdateLibrariesIndexResponse {
+  message Result {
+    // The result of the libraries index update.
+    IndexUpdateReport libraries_index = 1;
+  }
+  oneof message {
+    // Progress of the libraries index download.
+    DownloadProgress download_progress = 1;
+    // The result of the index update.
+    Result result = 2;
+  }
+}
+```
+
+The `IndexUpdateReport` message contains details for each index update operation performed:
+
+```proto
+message IndexUpdateReport {
+  enum Status {
+    // The status of the index update is unspecified.
+    STATUS_UNSPECIFIED = 0;
+    // The index has been successfully updated.
+    STATUS_UPDATED = 1;
+    // The index was already up to date.
+    STATUS_ALREADY_UP_TO_DATE = 2;
+    // The index update failed.
+    STATUS_FAILED = 3;
+    // The index update was skipped.
+    STATUS_SKIPPED = 4;
+  }
+
+  // The URL of the index that was updated.
+  string index_url = 1;
+  // The result of the index update.
+  Status status = 2;
+}
+```
+
 ### The gRPC `cc.arduino.cli.commands.v1.Profile` message has been removed in favor of `SketchProfile`
 
 The message `Profile` has been replaced with `SketchProfile` in the `InitResponse.profile` field:

Diff for: internal/cli/core/search.go

@@ -19,15 +19,11 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"path"
 	"strings"
 	"time"
 
 	"github.com/arduino/arduino-cli/commands"
 	"github.com/arduino/arduino-cli/commands/core"
-	"github.com/arduino/arduino-cli/internal/arduino/globals"
-	"github.com/arduino/arduino-cli/internal/arduino/utils"
-	"github.com/arduino/arduino-cli/internal/cli/configuration"
 	"github.com/arduino/arduino-cli/internal/cli/feedback"
 	"github.com/arduino/arduino-cli/internal/cli/feedback/result"
 	"github.com/arduino/arduino-cli/internal/cli/feedback/table"
@@ -55,17 +51,24 @@ func initSearchCommand() *cobra.Command {
 }
 
 // indexUpdateInterval specifies the time threshold over which indexes are updated
-const indexUpdateInterval = "24h"
+const indexUpdateInterval = 24 * time.Hour
 
 func runSearchCommand(cmd *cobra.Command, args []string, allVersions bool) {
 	inst := instance.CreateAndInit()
 
-	if indexesNeedUpdating(indexUpdateInterval) {
-		err := commands.UpdateIndex(context.Background(), &rpc.UpdateIndexRequest{Instance: inst}, feedback.ProgressBar())
-		if err != nil {
-			feedback.FatalError(err, feedback.ErrGeneric)
+	res, err := commands.UpdateIndex(
+		context.Background(),
+		&rpc.UpdateIndexRequest{Instance: inst, UpdateIfOlderThanSecs: int64(indexUpdateInterval.Seconds())},
+		feedback.ProgressBar())
+	if err != nil {
+		feedback.FatalError(err, feedback.ErrGeneric)
+	}
+	for _, idxRes := range res.GetUpdatedIndexes() {
+		if idxRes.GetStatus() == rpc.IndexUpdateReport_STATUS_UPDATED {
+			// At least one index has been updated, reinitialize the instance
+			instance.Init(inst)
+			break
 		}
-		instance.Init(inst)
 	}
 
 	arguments := strings.ToLower(strings.Join(args, " "))
@@ -134,58 +137,3 @@ func (sr searchResults) String() string {
 	}
 	return t.Render()
 }
-
-// indexesNeedUpdating returns whether one or more index files need updating.
-// A duration string must be provided to calculate the time threshold
-// used to update the indexes, if the duration is not valid a default
-// of 24 hours is used.
-// Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
-func indexesNeedUpdating(duration string) bool {
-	indexpath := configuration.DataDir(configuration.Settings)
-
-	now := time.Now()
-	modTimeThreshold, err := time.ParseDuration(duration)
-	if err != nil {
-		feedback.Fatal(tr("Invalid timeout: %s", err), feedback.ErrBadArgument)
-	}
-
-	urls := []string{globals.DefaultIndexURL}
-	urls = append(urls, configuration.Settings.GetStringSlice("board_manager.additional_urls")...)
-	for _, u := range urls {
-		URL, err := utils.URLParse(u)
-		if err != nil {
-			continue
-		}
-
-		if URL.Scheme == "file" {
-			// No need to update local files
-			continue
-		}
-
-		// should handle:
-		// - package_index.json
-		// - package_index.json.sig
-		// - package_index.json.gz
-		// - package_index.tar.bz2
-		indexFileName := path.Base(URL.Path)
-		indexFileName = strings.TrimSuffix(indexFileName, ".tar.bz2")
-		indexFileName = strings.TrimSuffix(indexFileName, ".gz")
-		indexFileName = strings.TrimSuffix(indexFileName, ".sig")
-		indexFileName = strings.TrimSuffix(indexFileName, ".json")
-		// and obtain package_index.json as result
-		coreIndexPath := indexpath.Join(indexFileName + ".json")
-		if coreIndexPath.NotExist() {
-			return true
-		}
-
-		info, err := coreIndexPath.Stat()
-		if err != nil {
-			return true
-		}
-
-		if now.After(info.ModTime().Add(modTimeThreshold)) {
-			return true
-		}
-	}
-	return false
-}