[breaking] Remove auto detection of Arduino IDE built-in libraries and tools / Allow gRPC install of built-in libraries (#1817)

* Removed IDE-bundle autodetection

* Do not return bundled tools folder with HardwarePaths

* Created configuration functions to obtain data dir and downloads dir

* Bundled libraries configuration is now considere a single directory

* Allow library installation on bundled libs dir via gRPC

* Added integration test for library install in bundled directory

* Fixed nil pointer exception in build options extraction

* Updated docs

* Removed useless custom env bindings

* Applied code review changes

Co-authored-by: per1234 <[email protected]>

* Set builtin libraries dir by default when running as a daemon

* Added test for regression

See #1817 (review)

* Fixed regression: 'lib uninstall' must operate only on user dir

Co-authored-by: per1234 <[email protected]>

Author: cmaglie

Diff for: arduino/cores/packagemanager/loader.go

@@ -38,7 +38,7 @@ func (pm *Builder) LoadHardware() []error {
 	hardwareDirs := configuration.HardwareDirectories(configuration.Settings)
 	merr := pm.LoadHardwareFromDirectories(hardwareDirs)
 
-	bundleToolDirs := configuration.BundleToolsDirectories(configuration.Settings)
+	bundleToolDirs := configuration.BuiltinToolsDirectories(configuration.Settings)
 	merr = append(merr, pm.LoadToolsFromBundleDirectories(bundleToolDirs)...)
 
 	return merr

Diff for: arduino/cores/packagemanager/package_manager_test.go

@@ -224,7 +224,7 @@ func TestFindToolsRequiredForBoard(t *testing.T) {
 	pmb := packagemanager.NewBuilder(
 		dataDir1,
 		configuration.PackagesDir(configuration.Settings),
-		paths.New(configuration.Settings.GetString("directories.Downloads")),
+		configuration.DownloadsDir(configuration.Settings),
 		dataDir1,
 		"test",
 	)

Diff for: arduino/libraries/libraries_location.go

@@ -93,7 +93,7 @@ func (d *LibraryLocation) UnmarshalJSON(b []byte) error {
 func (d *LibraryLocation) ToRPCLibraryLocation() rpc.LibraryLocation {
 	switch *d {
 	case IDEBuiltIn:
-		return rpc.LibraryLocation_LIBRARY_LOCATION_IDE_BUILTIN
+		return rpc.LibraryLocation_LIBRARY_LOCATION_BUILTIN
 	case PlatformBuiltIn:
 		return rpc.LibraryLocation_LIBRARY_LOCATION_PLATFORM_BUILTIN
 	case ReferencedPlatformBuiltIn:
@@ -110,7 +110,7 @@ func (d *LibraryLocation) ToRPCLibraryLocation() rpc.LibraryLocation {
 // FromRPCLibraryLocation converts a rpc.LibraryLocation to a LibraryLocation
 func FromRPCLibraryLocation(l rpc.LibraryLocation) LibraryLocation {
 	switch l {
-	case rpc.LibraryLocation_LIBRARY_LOCATION_IDE_BUILTIN:
+	case rpc.LibraryLocation_LIBRARY_LOCATION_BUILTIN:
 		return IDEBuiltIn
 	case rpc.LibraryLocation_LIBRARY_LOCATION_PLATFORM_BUILTIN:
 		return PlatformBuiltIn
@@ -124,3 +124,15 @@ func FromRPCLibraryLocation(l rpc.LibraryLocation) LibraryLocation {
 		panic(fmt.Sprintf("invalid rpc.LibraryLocation value %d", l))
 	}
 }
+
+// FromRPCLibraryInstallLocation converts a rpc.LibraryInstallLocation to a LibraryLocation
+func FromRPCLibraryInstallLocation(l rpc.LibraryInstallLocation) LibraryLocation {
+	switch l {
+	case rpc.LibraryInstallLocation_LIBRARY_INSTALL_LOCATION_BUILTIN:
+		return IDEBuiltIn
+	case rpc.LibraryInstallLocation_LIBRARY_INSTALL_LOCATION_USER:
+		return User
+	default:
+		panic(fmt.Sprintf("invalid rpc.LibraryInstallLocation value %d", l))
+	}
+}

Diff for: arduino/libraries/librariesmanager/install.go

@@ -48,13 +48,13 @@ var (
 // InstallPrerequisiteCheck performs prequisite checks to install a library. It returns the
 // install path, where the library should be installed and the possible library that is already
 // installed on the same folder and it's going to be replaced by the new one.
-func (lm *LibrariesManager) InstallPrerequisiteCheck(indexLibrary *librariesindex.Release) (*paths.Path, *libraries.Library, error) {
+func (lm *LibrariesManager) InstallPrerequisiteCheck(indexLibrary *librariesindex.Release, installLocation libraries.LibraryLocation) (*paths.Path, *libraries.Library, error) {
 	saneName := utils.SanitizeName(indexLibrary.Library.Name)
 
 	var replaced *libraries.Library
 	if installedLibs, have := lm.Libraries[saneName]; have {
 		for _, installedLib := range installedLibs.Alternatives {
-			if installedLib.Location != libraries.User {
+			if installedLib.Location != installLocation {
 				continue
 			}
 			if installedLib.Version != nil && installedLib.Version.Equal(indexLibrary.Version) {
@@ -64,9 +64,12 @@ func (lm *LibrariesManager) InstallPrerequisiteCheck(indexLibrary *librariesinde
 		}
 	}
 
-	libsDir := lm.getUserLibrariesDir()
+	libsDir := lm.getLibrariesDir(installLocation)
 	if libsDir == nil {
-		return nil, nil, fmt.Errorf(tr("User directory not set"))
+		if installLocation == libraries.User {
+			return nil, nil, fmt.Errorf(tr("User directory not set"))
+		}
+		return nil, nil, fmt.Errorf(tr("Builtin libraries directory not set"))
 	}
 
 	libPath := libsDir.Join(saneName)
@@ -79,8 +82,8 @@ func (lm *LibrariesManager) InstallPrerequisiteCheck(indexLibrary *librariesinde
 }
 
 // Install installs a library on the specified path.
-func (lm *LibrariesManager) Install(indexLibrary *librariesindex.Release, libPath *paths.Path) error {
-	libsDir := lm.getUserLibrariesDir()
+func (lm *LibrariesManager) Install(indexLibrary *librariesindex.Release, libPath *paths.Path, installLocation libraries.LibraryLocation) error {
+	libsDir := lm.getLibrariesDir(installLocation)
 	if libsDir == nil {
 		return fmt.Errorf(tr("User directory not set"))
 	}
@@ -100,9 +103,9 @@ func (lm *LibrariesManager) Uninstall(lib *libraries.Library) error {
 	return nil
 }
 
-//InstallZipLib  installs a Zip library on the specified path.
+// InstallZipLib installs a Zip library on the specified path.
 func (lm *LibrariesManager) InstallZipLib(ctx context.Context, archivePath string, overwrite bool) error {
-	libsDir := lm.getUserLibrariesDir()
+	libsDir := lm.getLibrariesDir(libraries.User)
 	if libsDir == nil {
 		return fmt.Errorf(tr("User directory not set"))
 	}
@@ -184,9 +187,9 @@ func (lm *LibrariesManager) InstallZipLib(ctx context.Context, archivePath strin
 	return nil
 }
 
-//InstallGitLib  installs a library hosted on a git repository on the specified path.
+// InstallGitLib installs a library hosted on a git repository on the specified path.
 func (lm *LibrariesManager) InstallGitLib(gitURL string, overwrite bool) error {
-	libsDir := lm.getUserLibrariesDir()
+	libsDir := lm.getLibrariesDir(libraries.User)
 	if libsDir == nil {
 		return fmt.Errorf(tr("User directory not set"))
 	}

Diff for: arduino/libraries/librariesmanager/librariesmanager.go

@@ -78,9 +78,9 @@ func (alts *LibraryAlternatives) Remove(library *libraries.Library) {
 }
 
 // FindVersion returns the library mathching the provided version or nil if not found
-func (alts *LibraryAlternatives) FindVersion(version *semver.Version) *libraries.Library {
+func (alts *LibraryAlternatives) FindVersion(version *semver.Version, installLocation libraries.LibraryLocation) *libraries.Library {
 	for _, lib := range alts.Alternatives {
-		if lib.Version.Equal(version) {
+		if lib.Version.Equal(version) && lib.Location == installLocation {
 			return lib
 		}
 	}
@@ -118,6 +118,7 @@ func NewLibraryManager(indexDir *paths.Path, downloadsDir *paths.Path) *Librarie
 // LoadIndex reads a library_index.json from a file and returns
 // the corresponding Index structure.
 func (lm *LibrariesManager) LoadIndex() error {
+	logrus.WithField("index", lm.IndexFile).Info("Loading libraries index file")
 	index, err := librariesindex.LoadIndex(lm.IndexFile)
 	if err != nil {
 		lm.Index = librariesindex.EmptyIndex
@@ -175,9 +176,9 @@ func (lm *LibrariesManager) RescanLibraries() []*status.Status {
 	return statuses
 }
 
-func (lm *LibrariesManager) getUserLibrariesDir() *paths.Path {
+func (lm *LibrariesManager) getLibrariesDir(installLocation libraries.LibraryLocation) *paths.Path {
 	for _, dir := range lm.LibrariesDir {
-		if dir.Location == libraries.User {
+		if dir.Location == installLocation {
 			return dir.Path
 		}
 	}
@@ -244,7 +245,7 @@ func (lm *LibrariesManager) LoadLibraryFromDir(libRootDir *paths.Path, location
 // FindByReference return the installed library matching the Reference
 // name and version or, if the version is nil, the library installed
 // in the User folder.
-func (lm *LibrariesManager) FindByReference(libRef *librariesindex.Reference) *libraries.Library {
+func (lm *LibrariesManager) FindByReference(libRef *librariesindex.Reference, installLocation libraries.LibraryLocation) *libraries.Library {
 	saneName := utils.SanitizeName(libRef.Name)
 	alternatives, have := lm.Libraries[saneName]
 	if !have {
@@ -253,11 +254,11 @@ func (lm *LibrariesManager) FindByReference(libRef *librariesindex.Reference) *l
 	// TODO: Move "search into user" into another method...
 	if libRef.Version == nil {
 		for _, candidate := range alternatives.Alternatives {
-			if candidate.Location == libraries.User {
+			if candidate.Location == installLocation {
 				return candidate
 			}
 		}
 		return nil
 	}
-	return alternatives.FindVersion(libRef.Version)
+	return alternatives.FindVersion(libRef.Version, installLocation)
 }

Diff for: cli/cache/clean.go

@@ -40,8 +40,8 @@ func initCleanCommand() *cobra.Command {
 func runCleanCommand(cmd *cobra.Command, args []string) {
 	logrus.Info("Executing `arduino-cli cache clean`")
 
-	cachePath := configuration.Settings.GetString("directories.Downloads")
-	err := os.RemoveAll(cachePath)
+	cachePath := configuration.DownloadsDir(configuration.Settings)
+	err := cachePath.RemoveAll()
 	if err != nil {
 		feedback.Errorf(tr("Error cleaning caches: %v"), err)
 		os.Exit(errorcodes.ErrGeneric)

Diff for: cli/cli.go

@@ -158,7 +158,7 @@ func preRun(cmd *cobra.Command, args []string) {
 	configFile := configuration.Settings.ConfigFileUsed()
 
 	// initialize inventory
-	err := inventory.Init(configuration.Settings.GetString("directories.Data"))
+	err := inventory.Init(configuration.DataDir(configuration.Settings).String())
 	if err != nil {
 		feedback.Errorf("Error: %v", err)
 		os.Exit(errorcodes.ErrBadArgument)

Diff for: cli/config/validate.go

@@ -30,6 +30,8 @@ var validMap = map[string]reflect.Kind{
 	"directories.data":              reflect.String,
 	"directories.downloads":         reflect.String,
 	"directories.user":              reflect.String,
+	"directories.builtin.tools":     reflect.String,
+	"directories.builtin.libraries": reflect.String,
 	"library.enable_unsafe_install": reflect.Bool,
 	"logging.file":                  reflect.String,
 	"logging.format":                reflect.String,

Diff for: cli/core/search.go

@@ -34,7 +34,6 @@ import (
 	"github.com/arduino/arduino-cli/configuration"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/arduino-cli/table"
-	"github.com/arduino/go-paths-helper"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
@@ -130,7 +129,7 @@ func (sr searchResults) String() string {
 // of 24 hours is used.
 // Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
 func indexesNeedUpdating(duration string) bool {
-	indexpath := paths.New(configuration.Settings.GetString("directories.Data"))
+	indexpath := configuration.DataDir(configuration.Settings)
 
 	now := time.Now()
 	modTimeThreshold, err := time.ParseDuration(duration)

Diff for: cli/daemon/daemon.go

@@ -72,6 +72,9 @@ func NewCommand() *cobra.Command {
 func runDaemonCommand(cmd *cobra.Command, args []string) {
 	logrus.Info("Executing `arduino-cli daemon`")
 
+	// Bundled libraries support is enabled by default when running as a daemon
+	configuration.Settings.SetDefault("directories.builtin.Libraries", configuration.GetDefaultBuiltinLibrariesDir())
+
 	port := configuration.Settings.GetString("daemon.port")
 	gRPCOptions := []grpc.ServerOption{}
 	if debugFile != "" {

Diff for: cli/instance/instance.go

@@ -125,7 +125,7 @@ func InitWithProfile(instance *rpc.Instance, profileName string, sketchPath *pat
 // 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 := paths.New(configuration.Settings.GetString("directories.data"))
+	dataDir := configuration.DataDir(configuration.Settings)
 
 	libraryIndex := dataDir.Join("library_index.json")
 	packageIndex := dataDir.Join("package_index.json")

Diff for: commands/compile/compile.go

@@ -19,8 +19,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"path/filepath"
-	"sort"
 	"strings"
 
 	"github.com/arduino/arduino-cli/arduino"
@@ -35,7 +33,6 @@ import (
 	"github.com/arduino/arduino-cli/legacy/builder/types"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	paths "github.com/arduino/go-paths-helper"
-	properties "github.com/arduino/go-properties-orderedmap"
 	"github.com/sirupsen/logrus"
 )
 
@@ -123,7 +120,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 
 	// FIXME: This will be redundant when arduino-builder will be part of the cli
 	builderCtx.HardwareDirs = configuration.HardwareDirectories(configuration.Settings)
-	builderCtx.BuiltInToolsDirs = configuration.BundleToolsDirectories(configuration.Settings)
+	builderCtx.BuiltInToolsDirs = configuration.BuiltinToolsDirectories(configuration.Settings)
 
 	// FIXME: This will be redundant when arduino-builder will be part of the cli
 	builderCtx.OtherLibrariesDirs = paths.NewPathList(req.GetLibraries()...)
@@ -164,29 +161,11 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 		}
 	}
 
+	builderCtx.BuiltInLibrariesDirs = configuration.IDEBuiltinLibrariesDir(configuration.Settings)
+
 	// Will be deprecated.
 	builderCtx.ArduinoAPIVersion = "10607"
 
-	// Check if Arduino IDE is installed and get it's libraries location.
-	// TODO: Remove?
-	dataDir := paths.New(configuration.Settings.GetString("directories.Data"))
-	preferencesTxt := dataDir.Join("preferences.txt")
-	ideProperties, err := properties.LoadFromPath(preferencesTxt)
-	if err == nil {
-		lastIdeSubProperties := ideProperties.SubTree("last").SubTree("ide")
-		// Preferences can contain records from previous IDE versions. Find the latest one.
-		var pathVariants []string
-		for k := range lastIdeSubProperties.AsMap() {
-			if strings.HasSuffix(k, ".hardwarepath") {
-				pathVariants = append(pathVariants, k)
-			}
-		}
-		sort.Strings(pathVariants)
-		ideHardwarePath := lastIdeSubProperties.Get(pathVariants[len(pathVariants)-1])
-		ideLibrariesPath := filepath.Join(filepath.Dir(ideHardwarePath), "libraries")
-		builderCtx.BuiltInLibrariesDirs = paths.NewPathList(ideLibrariesPath)
-	}
-
 	builderCtx.Stdout = outStream
 	builderCtx.Stderr = errStream
 	builderCtx.Clean = req.GetClean()

Diff for: commands/instances.go

@@ -149,7 +149,7 @@ func Create(req *rpc.CreateRequest, extraUserAgent ...string) (*rpc.CreateRespon
 	instance := &CoreInstance{}
 
 	// Setup downloads directory
-	downloadsDir := paths.New(configuration.Settings.GetString("directories.Downloads"))
+	downloadsDir := configuration.DownloadsDir(configuration.Settings)
 	if downloadsDir.NotExist() {
 		err := downloadsDir.MkdirAll()
 		if err != nil {
@@ -158,7 +158,7 @@ func Create(req *rpc.CreateRequest, extraUserAgent ...string) (*rpc.CreateRespon
 	}
 
 	// Setup data directory
-	dataDir := paths.New(configuration.Settings.GetString("directories.Data"))
+	dataDir := configuration.DataDir(configuration.Settings)
 	packagesDir := configuration.PackagesDir(configuration.Settings)
 	if packagesDir.NotExist() {
 		err := packagesDir.MkdirAll()
@@ -389,7 +389,7 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
 
 	if profile == nil {
 		// Add directories of libraries bundled with IDE
-		if bundledLibsDir := configuration.IDEBundledLibrariesDir(configuration.Settings); bundledLibsDir != nil {
+		if bundledLibsDir := configuration.IDEBuiltinLibrariesDir(configuration.Settings); bundledLibsDir != nil {
 			lm.AddLibrariesDir(bundledLibsDir, libraries.IDEBuiltIn)
 		}
 
@@ -495,7 +495,7 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexRequest, downloadCB rp
 		return nil, &arduino.InvalidInstanceError{}
 	}
 
-	indexpath := paths.New(configuration.Settings.GetString("directories.Data"))
+	indexpath := configuration.DataDir(configuration.Settings)
 
 	urls := []string{globals.DefaultIndexURL}
 	urls = append(urls, configuration.Settings.GetStringSlice("board_manager.additional_urls")...)

Diff for: commands/lib/install.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"github.com/arduino/arduino-cli/arduino"
+	"github.com/arduino/arduino-cli/arduino/libraries"
 	"github.com/arduino/arduino-cli/arduino/libraries/librariesindex"
 	"github.com/arduino/arduino-cli/arduino/libraries/librariesmanager"
 	"github.com/arduino/arduino-cli/commands"
@@ -30,13 +31,13 @@ import (
 
 // LibraryInstall FIXMEDOC
 func LibraryInstall(ctx context.Context, req *rpc.LibraryInstallRequest, downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) error {
-
 	lm := commands.GetLibraryManager(req)
 	if lm == nil {
 		return &arduino.InvalidInstanceError{}
 	}
 
 	toInstall := map[string]*rpc.LibraryDependencyStatus{}
+	installLocation := libraries.FromRPCLibraryInstallLocation(req.GetInstallLocation())
 	if req.NoDeps {
 		toInstall[req.Name] = &rpc.LibraryDependencyStatus{
 			Name:            req.Name,
@@ -81,7 +82,7 @@ func LibraryInstall(ctx context.Context, req *rpc.LibraryInstallRequest, downloa
 	// Check if any of the libraries to install is already installed and remove it from the list
 	j := 0
 	for i, libRelease := range libReleasesToInstall {
-		_, libReplaced, err := lm.InstallPrerequisiteCheck(libRelease)
+		_, libReplaced, err := lm.InstallPrerequisiteCheck(libRelease, installLocation)
 		if errors.Is(err, librariesmanager.ErrAlreadyInstalled) {
 			taskCB(&rpc.TaskProgress{Message: tr("Already installed %s", libRelease), Completed: true})
 		} else if err != nil {
@@ -104,7 +105,7 @@ func LibraryInstall(ctx context.Context, req *rpc.LibraryInstallRequest, downloa
 			return err
 		}
 
-		if err := installLibrary(lm, libRelease, taskCB); err != nil {
+		if err := installLibrary(lm, libRelease, installLocation, taskCB); err != nil {
 			if errors.Is(err, librariesmanager.ErrAlreadyInstalled) {
 				continue
 			} else {
@@ -123,10 +124,10 @@ func LibraryInstall(ctx context.Context, req *rpc.LibraryInstallRequest, downloa
 	return nil
 }
 
-func installLibrary(lm *librariesmanager.LibrariesManager, libRelease *librariesindex.Release, taskCB rpc.TaskProgressCB) error {
+func installLibrary(lm *librariesmanager.LibrariesManager, libRelease *librariesindex.Release, installLocation libraries.LibraryLocation, taskCB rpc.TaskProgressCB) error {
 	taskCB(&rpc.TaskProgress{Name: tr("Installing %s", libRelease)})
 	logrus.WithField("library", libRelease).Info("Installing library")
-	libPath, libReplaced, err := lm.InstallPrerequisiteCheck(libRelease)
+	libPath, libReplaced, err := lm.InstallPrerequisiteCheck(libRelease, installLocation)
 	if errors.Is(err, librariesmanager.ErrAlreadyInstalled) {
 		taskCB(&rpc.TaskProgress{Message: tr("Already installed %s", libRelease), Completed: true})
 		return err
@@ -140,7 +141,7 @@ func installLibrary(lm *librariesmanager.LibrariesManager, libRelease *libraries
 		taskCB(&rpc.TaskProgress{Message: tr("Replacing %[1]s with %[2]s", libReplaced, libRelease)})
 	}
 
-	if err := lm.Install(libRelease, libPath); err != nil {
+	if err := lm.Install(libRelease, libPath, installLocation); err != nil {
 		return &arduino.FailedLibraryInstallError{Cause: err}
 	}