Automatically download indexes, if missing, in gRPC `Init` call (#2119)

* Created core.PlatformList implementaion follow gRPC naming

Previously it was named GetPlatforms with a different return type than
the one defined in gRPC API .proto files.

* Added test for #1529

* Perform first-update automatically in gRPC Init

* Made cli.instance.Create function private

* Extract function to compute index file name

* Auto-download 3rd party indexes as part of the Init

arduino/resources/index.go

@@ -37,6 +37,15 @@ type IndexResource struct {
 	SignatureURL *url.URL
 }
 
+// IndexFileName returns the index file name as it is saved in data dir (package_xxx_index.json).
+func (res *IndexResource) IndexFileName() string {
+	filename := path.Base(res.URL.Path) // == package_index.json[.gz] || packacge_index.tar.bz2
+	if i := strings.Index(filename, "."); i != -1 {
+		filename = filename[:i]
+	}
+	return filename + ".json"
+}
+
 // Download will download the index and possibly check the signature using the Arduino's public key.
 // If the file is in .gz format it will be unpacked first.
 func (res *IndexResource) Download(destDir *paths.Path, downloadCB rpc.DownloadProgressCB) error {
@@ -53,9 +62,10 @@ func (res *IndexResource) Download(destDir *paths.Path, downloadCB rpc.DownloadP
 	defer tmp.RemoveAll()
 
 	// Download index file
-	indexFileName := path.Base(res.URL.Path) // == package_index.json[.gz]
-	tmpIndexPath := tmp.Join(indexFileName)
-	if err := httpclient.DownloadFile(tmpIndexPath, res.URL.String(), "", tr("Downloading index: %s", indexFileName), downloadCB, nil, downloader.NoResume); err != nil {
+	downloadFileName := path.Base(res.URL.Path) // == package_index.json[.gz] || package_index.tar.bz2
+	indexFileName := res.IndexFileName()        // == package_index.json
+	tmpIndexPath := tmp.Join(downloadFileName)
+	if err := httpclient.DownloadFile(tmpIndexPath, res.URL.String(), "", tr("Downloading index: %s", downloadFileName), downloadCB, nil, downloader.NoResume); err != nil {
 		return &arduino.FailedDownloadError{Message: tr("Error downloading index '%s'", res.URL), Cause: err}
 	}
 
@@ -63,8 +73,7 @@ func (res *IndexResource) Download(destDir *paths.Path, downloadCB rpc.DownloadP
 	hasSignature := false
 
 	// Expand the index if it is compressed
-	if strings.HasSuffix(indexFileName, ".tar.bz2") {
-		indexFileName = strings.TrimSuffix(indexFileName, ".tar.bz2") + ".json" // == package_index.json
+	if strings.HasSuffix(downloadFileName, ".tar.bz2") {
 		signatureFileName := indexFileName + ".sig"
 		signaturePath = destDir.Join(signatureFileName)
 
@@ -95,8 +104,7 @@ func (res *IndexResource) Download(destDir *paths.Path, downloadCB rpc.DownloadP
 		} else {
 			logrus.Infof("No signature %s found in package index archive %s", signatureFileName, tmpArchivePath.Base())
 		}
-	} else if strings.HasSuffix(indexFileName, ".gz") {
-		indexFileName = strings.TrimSuffix(indexFileName, ".gz") // == package_index.json
+	} else if strings.HasSuffix(downloadFileName, ".gz") {
 		tmpUnzippedIndexPath := tmp.Join(indexFileName)
 		if err := paths.GUnzip(tmpIndexPath, tmpUnzippedIndexPath); err != nil {
 			return &arduino.PermissionDeniedError{Message: tr("Error extracting %s", indexFileName), Cause: err}

commands/core/list.go

@@ -25,9 +25,9 @@ import (
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 )
 
-// GetPlatforms returns a list of installed platforms, optionally filtered by
+// PlatformList returns a list of installed platforms, optionally filtered by
 // those requiring an update.
-func GetPlatforms(req *rpc.PlatformListRequest) ([]*rpc.Platform, error) {
+func PlatformList(req *rpc.PlatformListRequest) (*rpc.PlatformListResponse, error) {
 	pme, release := commands.GetPackageManagerExplorer(req)
 	if pme == nil {
 		return nil, &arduino.InvalidInstanceError{}
@@ -85,5 +85,5 @@ func GetPlatforms(req *rpc.PlatformListRequest) ([]*rpc.Platform, error) {
 		}
 		return false
 	})
-	return res, nil
+	return &rpc.PlatformListResponse{InstalledPlatforms: res}, nil
 }

commands/daemon/daemon.go

@@ -284,11 +284,8 @@ func (s *ArduinoCoreServerImpl) PlatformSearch(ctx context.Context, req *rpc.Pla
 
 // PlatformList FIXMEDOC
 func (s *ArduinoCoreServerImpl) PlatformList(ctx context.Context, req *rpc.PlatformListRequest) (*rpc.PlatformListResponse, error) {
-	platforms, err := core.GetPlatforms(req)
-	if err != nil {
-		return nil, convertErrorToRPCStatus(err)
-	}
-	return &rpc.PlatformListResponse{InstalledPlatforms: platforms}, nil
+	platforms, err := core.PlatformList(req)
+	return platforms, convertErrorToRPCStatus(err)
 }
 
 // Upload FIXMEDOC

commands/instances.go

@@ -262,20 +262,11 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
 		})
 	}
 
-	{
-		// We need to rebuild the PackageManager currently in use by this instance
-		// in case this is not the first Init on this instances, that might happen
-		// after reinitializing an instance after installing or uninstalling a core.
-		// If this is not done the information of the uninstall core is kept in memory,
-		// even if it should not.
-		pmb, commitPackageManager := instance.pm.NewBuilder()
-
-		// Load packages index
-		urls := []string{globals.DefaultIndexURL}
-		if profile == nil {
-			urls = append(urls, configuration.Settings.GetStringSlice("board_manager.additional_urls")...)
-		}
-		for _, u := range urls {
+	// Perform first-update of indexes if needed
+	defaultIndexURL, _ := utils.URLParse(globals.DefaultIndexURL)
+	allPackageIndexUrls := []*url.URL{defaultIndexURL}
+	if profile == nil {
+		for _, u := range configuration.Settings.GetStringSlice("board_manager.additional_urls") {
 			URL, err := utils.URLParse(u)
 			if err != nil {
 				e := &arduino.InitFailedError{
@@ -286,7 +277,21 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
 				responseError(e.ToRPCStatus())
 				continue
 			}
+			allPackageIndexUrls = append(allPackageIndexUrls, URL)
+		}
+	}
+	firstUpdate(context.Background(), req.GetInstance(), downloadCallback, allPackageIndexUrls)
 
+	{
+		// We need to rebuild the PackageManager currently in use by this instance
+		// in case this is not the first Init on this instances, that might happen
+		// after reinitializing an instance after installing or uninstalling a core.
+		// If this is not done the information of the uninstall core is kept in memory,
+		// even if it should not.
+		pmb, commitPackageManager := instance.pm.NewBuilder()
+
+		// Load packages index
+		for _, URL := range allPackageIndexUrls {
 			if URL.Scheme == "file" {
 				_, err := pmb.LoadPackageIndexFromFile(paths.New(URL.Path))
 				if err != nil {
@@ -595,3 +600,41 @@ func LoadSketch(ctx context.Context, req *rpc.LoadSketchRequest) (*rpc.LoadSketc
 		RootFolderFiles:  rootFolderFiles,
 	}, nil
 }
+
+// firstUpdate downloads libraries and packages indexes if they don't exist.
+// This ideally is only executed the first time the CLI is run.
+func firstUpdate(ctx context.Context, instance *rpc.Instance, downloadCb func(msg *rpc.DownloadProgress), externalPackageIndexes []*url.URL) error {
+	// Gets the data directory to verify if library_index.json and package_index.json exist
+	dataDir := configuration.DataDir(configuration.Settings)
+	libraryIndex := dataDir.Join("library_index.json")
+
+	if libraryIndex.NotExist() {
+		// 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 {
+			return err
+		}
+	}
+
+	for _, URL := range externalPackageIndexes {
+		if URL.Scheme == "file" {
+			continue
+		}
+		packageIndexFileName := (&resources.IndexResource{URL: URL}).IndexFileName()
+		packageIndexFile := dataDir.Join(packageIndexFileName)
+		if packageIndexFile.NotExist() {
+			// The index file doesn't exists, that means the CLI is run for the first time,
+			// or the 3rd party package index URL has just been added. Similarly to the
+			// 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 {
+				return err
+			}
+			break
+		}
+	}
+
+	return nil
+}

docs/UPGRADING.md

@@ -118,6 +118,40 @@ has been removed as well.
 
 That method was outdated and must not be used.
 
+### golang API: method `github.com/arduino/arduino-cli/commands/core/GetPlatforms` renamed
+
+The following method in `github.com/arduino/arduino-cli/commands/core`:
+
+```go
+func GetPlatforms(req *rpc.PlatformListRequest) ([]*rpc.Platform, error) { ... }
+```
+
+has been changed to:
+
+```go
+func PlatformList(req *rpc.PlatformListRequest) (*rpc.PlatformListResponse, error) { ... }
+```
+
+now it better follows the gRPC API interface. Old code like the following:
+
+```go
+platforms, _ := core.GetPlatforms(&rpc.PlatformListRequest{Instance: inst})
+for _, i := range platforms {
+    ...
+}
+```
+
+must be changed as follows:
+
+```go
+// Use PlatformList function instead of GetPlatforms
+platforms, _ := core.PlatformList(&rpc.PlatformListRequest{Instance: inst})
+// Access installed platforms through the .InstalledPlatforms field
+for _, i := range platforms.InstalledPlatforms {
+    ...
+}
+```
+
 ## 0.31.0
 
 ### Added `post_install` script support for tools

internal/cli/arguments/completion.go

@@ -124,14 +124,14 @@ func GetInstalledProgrammers() []string {
 func GetUninstallableCores() []string {
 	inst := instance.CreateAndInit()
 
-	platforms, _ := core.GetPlatforms(&rpc.PlatformListRequest{
+	platforms, _ := core.PlatformList(&rpc.PlatformListRequest{
 		Instance:      inst,
 		UpdatableOnly: false,
 		All:           false,
 	})
 	var res []string
 	// transform the data structure for the completion
-	for _, i := range platforms {
+	for _, i := range platforms.InstalledPlatforms {
 		res = append(res, i.Id+"\t"+i.Name)
 	}
 	return res

internal/cli/arguments/reference.go

@@ -92,15 +92,15 @@ func ParseReference(arg string) (*Reference, error) {
 	ret.Architecture = toks[1]
 
 	// Now that we have the required informations in `ret` we can
-	// try to use core.GetPlatforms to optimize what the user typed
+	// try to use core.PlatformList to optimize what the user typed
 	// (by replacing the PackageName and Architecture in ret with the content of core.GetPlatform())
-	platforms, _ := core.GetPlatforms(&rpc.PlatformListRequest{
+	platforms, _ := core.PlatformList(&rpc.PlatformListRequest{
 		Instance:      instance.CreateAndInit(),
 		UpdatableOnly: false,
 		All:           true, // this is true because we want also the installable platforms
 	})
 	foundPlatforms := []string{}
-	for _, platform := range platforms {
+	for _, platform := range platforms.InstalledPlatforms {
 		platformID := platform.GetId()
 		platformUser := ret.PackageName + ":" + ret.Architecture
 		// At first we check if the platform the user is searching for matches an available one,

internal/cli/core/list.go

@@ -60,7 +60,7 @@ func List(inst *rpc.Instance, all bool, updatableOnly bool) {
 
 // GetList returns a list of installed platforms.
 func GetList(inst *rpc.Instance, all bool, updatableOnly bool) []*rpc.Platform {
-	platforms, err := core.GetPlatforms(&rpc.PlatformListRequest{
+	platforms, err := core.PlatformList(&rpc.PlatformListRequest{
 		Instance:      inst,
 		UpdatableOnly: updatableOnly,
 		All:           all,
@@ -68,7 +68,7 @@ func GetList(inst *rpc.Instance, all bool, updatableOnly bool) []*rpc.Platform {
 	if err != nil {
 		feedback.Fatal(tr("Error listing platforms: %v", err), feedback.ErrGeneric)
 	}
-	return platforms
+	return platforms.InstalledPlatforms
 }
 
 // output from this command requires special formatting, let's create a dedicated

internal/cli/core/search.go

@@ -58,20 +58,16 @@ func initSearchCommand() *cobra.Command {
 const indexUpdateInterval = "24h"
 
 func runSearchCommand(cmd *cobra.Command, args []string) {
-	inst, status := instance.Create()
-	if status != nil {
-		feedback.Fatal(tr("Error creating instance: %v", status), feedback.ErrGeneric)
-	}
+	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)
 		}
+		instance.Init(inst)
 	}
 
-	instance.Init(inst)
-
 	arguments := strings.ToLower(strings.Join(args, " "))
 	logrus.Infof("Executing `arduino-cli core search` with args: '%s'", arguments)
 

internal/cli/core/update_index.go

@@ -40,7 +40,7 @@ func initUpdateIndexCommand() *cobra.Command {
 }
 
 func runUpdateIndexCommand(cmd *cobra.Command, args []string) {
-	inst := instance.CreateInstanceAndRunFirstUpdate()
+	inst := instance.CreateAndInit()
 	logrus.Info("Executing `arduino-cli core update-index`")
 	UpdateIndex(inst)
 }

internal/cli/core/upgrade.go

@@ -60,7 +60,7 @@ func runUpgradeCommand(args []string, skipPostInstall bool) {
 func Upgrade(inst *rpc.Instance, args []string, skipPostInstall bool) {
 	// if no platform was passed, upgrade allthethings
 	if len(args) == 0 {
-		targets, err := core.GetPlatforms(&rpc.PlatformListRequest{
+		targets, err := core.PlatformList(&rpc.PlatformListRequest{
 			Instance:      inst,
 			UpdatableOnly: true,
 		})
@@ -68,12 +68,12 @@ func Upgrade(inst *rpc.Instance, args []string, skipPostInstall bool) {
 			feedback.Fatal(tr("Error retrieving core list: %v", err), feedback.ErrGeneric)
 		}
 
-		if len(targets) == 0 {
+		if len(targets.InstalledPlatforms) == 0 {
 			feedback.Print(tr("All the cores are already at the latest version"))
 			return
 		}
 
-		for _, t := range targets {
+		for _, t := range targets.InstalledPlatforms {
 			args = append(args, t.Id)
 		}
 	}

internal/cli/instance/instance.go

@@ -16,10 +16,7 @@
 package instance
 
 import (
-	"context"
-
 	"github.com/arduino/arduino-cli/commands"
-	"github.com/arduino/arduino-cli/configuration"
 	"github.com/arduino/arduino-cli/i18n"
 	"github.com/arduino/arduino-cli/internal/cli/feedback"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
@@ -41,7 +38,7 @@ func CreateAndInit() *rpc.Instance {
 // If Create fails the CLI prints an error and exits since to execute further operations a valid Instance is mandatory.
 // If Init returns errors they're printed only.
 func CreateAndInitWithProfile(profileName string, sketchPath *paths.Path) (*rpc.Instance, *rpc.Profile) {
-	instance, err := Create()
+	instance, err := create()
 	if err != nil {
 		feedback.Fatal(tr("Error creating instance: %v", err), feedback.ErrGeneric)
 	}
@@ -49,8 +46,8 @@ func CreateAndInitWithProfile(profileName string, sketchPath *paths.Path) (*rpc.
 	return instance, profile
 }
 
-// Create and return a new Instance.
-func Create() (*rpc.Instance, error) {
+// create and return a new Instance.
+func create() (*rpc.Instance, error) {
 	res, err := commands.Create(&rpc.CreateRequest{})
 	if err != nil {
 		return nil, err
@@ -71,12 +68,6 @@ func Init(instance *rpc.Instance) {
 // In case of loading failures return a list of errors for each platform or library that we failed to load.
 // Required Package and library indexes files are automatically downloaded.
 func InitWithProfile(instance *rpc.Instance, profileName string, sketchPath *paths.Path) *rpc.Profile {
-	// In case the CLI is executed for the first time
-	if err := FirstUpdate(instance); err != nil {
-		feedback.Warning(tr("Error initializing instance: %v", err))
-		return nil
-	}
-
 	downloadCallback := feedback.ProgressBar()
 	taskCallback := feedback.TaskProgress()
 
@@ -110,70 +101,3 @@ func InitWithProfile(instance *rpc.Instance, profileName string, sketchPath *pat
 
 	return profile
 }
-
-// FirstUpdate downloads libraries and packages indexes if they don't exist.
-// This ideally is only executed the first time the CLI is run.
-func FirstUpdate(instance *rpc.Instance) error {
-	// Gets the data directory to verify if library_index.json and package_index.json exist
-	dataDir := configuration.DataDir(configuration.Settings)
-
-	libraryIndex := dataDir.Join("library_index.json")
-	packageIndex := dataDir.Join("package_index.json")
-
-	if libraryIndex.Exist() && packageIndex.Exist() {
-		return nil
-	}
-
-	// 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
-	if libraryIndex.NotExist() {
-		err := commands.UpdateLibrariesIndex(context.Background(),
-			&rpc.UpdateLibrariesIndexRequest{
-				Instance: instance,
-			},
-			feedback.ProgressBar(),
-		)
-		if err != nil {
-			return 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
-	if packageIndex.NotExist() {
-		err := commands.UpdateIndex(context.Background(),
-			&rpc.UpdateIndexRequest{
-				Instance:                   instance,
-				IgnoreCustomPackageIndexes: true,
-			},
-			feedback.ProgressBar())
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
-// CreateInstanceAndRunFirstUpdate creates an instance and runs `FirstUpdate`.
-// It is mandatory for all `update-index` commands to call this
-func CreateInstanceAndRunFirstUpdate() *rpc.Instance {
-	// We don't initialize any CoreInstance when updating indexes since we don't need to.
-	// Also meaningless errors might be returned when calling this command with --additional-urls
-	// since the CLI would be searching for a corresponding file for the additional urls set
-	// as argument but none would be obviously found.
-	inst, status := Create()
-	if status != nil {
-		feedback.Fatal(tr("Error creating instance: %v", status), feedback.ErrGeneric)
-	}
-
-	// In case this is the first time the CLI is run we need to update indexes
-	// to make it work correctly, we must do this explicitly in this command since
-	// we must use instance.Create instead of instance.CreateAndInit for the
-	// reason stated above.
-	if err := FirstUpdate(inst); err != nil {
-		feedback.Fatal(tr("Error updating indexes: %v", err), feedback.ErrGeneric)
-	}
-	return inst
-}

internal/cli/lib/search.go

@@ -55,12 +55,9 @@ func initSearchCommand() *cobra.Command {
 const indexUpdateInterval = 60 * time.Minute
 
 func runSearchCommand(args []string, namesOnly bool, omitReleasesDetails bool) {
-	inst, status := instance.Create()
-	logrus.Info("Executing `arduino-cli lib search`")
+	inst := instance.CreateAndInit()
 
-	if status != nil {
-		feedback.Fatal(tr("Error creating instance: %v", status), feedback.ErrGeneric)
-	}
+	logrus.Info("Executing `arduino-cli lib search`")
 
 	if indexNeedsUpdating(indexUpdateInterval) {
 		if err := commands.UpdateLibrariesIndex(
@@ -70,10 +67,9 @@ func runSearchCommand(args []string, namesOnly bool, omitReleasesDetails bool) {
 		); err != nil {
 			feedback.Fatal(tr("Error updating library index: %v", err), feedback.ErrGeneric)
 		}
+		instance.Init(inst)
 	}
 
-	instance.Init(inst)
-
 	searchResp, err := lib.LibrarySearch(context.Background(), &rpc.LibrarySearchRequest{
 		Instance:            inst,
 		SearchArgs:          strings.Join(args, " "),

internal/cli/lib/update_index.go

@@ -40,7 +40,7 @@ func initUpdateIndexCommand() *cobra.Command {
 }
 
 func runUpdateIndexCommand(cmd *cobra.Command, args []string) {
-	inst := instance.CreateInstanceAndRunFirstUpdate()
+	inst := instance.CreateAndInit()
 	logrus.Info("Executing `arduino-cli lib update-index`")
 	UpdateIndex(inst)
 }

internal/cli/update/update.go

@@ -47,7 +47,7 @@ func NewCommand() *cobra.Command {
 }
 
 func runUpdateCommand(showOutdated bool) {
-	inst := instance.CreateInstanceAndRunFirstUpdate()
+	inst := instance.CreateAndInit()
 	logrus.Info("Executing `arduino-cli update`")
 	lib.UpdateIndex(inst)
 	core.UpdateIndex(inst)

internal/integrationtest/arduino-cli.go

@@ -470,3 +470,11 @@ func (inst *ArduinoCLIInstance) PlatformUpgrade(ctx context.Context, packager, a
 	logCallf(">>> PlatformUpgrade(%v:%v)\n", packager, arch)
 	return installCl, err
 }
+
+// PlatformList calls the "PlatformList" gRPC method.
+func (inst *ArduinoCLIInstance) PlatformList(ctx context.Context) (*commands.PlatformListResponse, error) {
+	req := &commands.PlatformListRequest{Instance: inst.instance}
+	logCallf(">>> PlatformList(%+v)\n", req)
+	resp, err := inst.cli.daemonClient.PlatformList(ctx, req)
+	return resp, err
+}

internal/integrationtest/daemon/daemon_test.go

@@ -83,6 +83,23 @@ func TestArduinoCliDaemon(t *testing.T) {
 	testWatcher()
 }
 
+func TestDaemonAutoUpdateIndexOnFirstInit(t *testing.T) {
+	// https://github.com/arduino/arduino-cli/issues/1529
+
+	env, cli := createEnvForDaemon(t)
+	defer env.CleanUp()
+
+	grpcInst := cli.Create()
+	require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
+		fmt.Printf("INIT> %v\n", ir.GetMessage())
+	}))
+
+	_, err := grpcInst.PlatformList(context.Background())
+	require.NoError(t, err)
+
+	require.FileExists(t, cli.DataDir().Join("package_index.json").String())
+}
+
 // createEnvForDaemon performs the minimum required operations to start the arduino-cli daemon.
 // It returns a testsuite.Environment and an ArduinoCLI client to perform the integration tests.
 // The Environment must be disposed by calling the CleanUp method via defer.
@@ -94,9 +111,6 @@ func createEnvForDaemon(t *testing.T) (*integrationtest.Environment, *integratio
 		UseSharedStagingFolder: true,
 	})
 
-	_, _, err := cli.Run("core", "update-index")
-	require.NoError(t, err)
-
 	_ = cli.StartDaemon(false)
 	return env, cli
 }