[skip-changelog] Refactoring of LibraryManager libraries directory handling. (#2477)

* Factored some library_index.json URL globals

* Removed unused parameter

* findLibraryIndexRelease requires libraries index instead of libraries manager

* LibrariesManager.AddLibrariesDir now accepts directly a LibraryDir struct

This is preparatory for the next commit

* AddLibrariesDir now can handle also single-lib directories

* Removed weird hack in the library detector

Previously the libraries detector had to load the single librares
provided via '--library' flag after a Rescan, because the library
manager wasn't able to handle a single folder library. Now it can so we
removed all the tricky machinery.

Author: cmaglie

Diff for: commands/instances.go

@@ -311,7 +311,11 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
 	for _, pack := range pme.GetPackages() {
 		for _, platform := range pack.Platforms {
 			if platformRelease := pme.GetInstalledPlatformRelease(platform); platformRelease != nil {
-				lm.AddPlatformReleaseLibrariesDir(platformRelease, libraries.PlatformBuiltIn)
+				lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
+					PlatformRelease: platformRelease,
+					Path:            platformRelease.GetLibrariesDir(),
+					Location:        libraries.PlatformBuiltIn,
+				})
 			}
 		}
 	}
@@ -324,11 +328,17 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
 	if profile == nil {
 		// Add directories of libraries bundled with IDE
 		if bundledLibsDir := configuration.IDEBuiltinLibrariesDir(configuration.Settings); bundledLibsDir != nil {
-			lm.AddLibrariesDir(bundledLibsDir, libraries.IDEBuiltIn)
+			lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
+				Path:     bundledLibsDir,
+				Location: libraries.IDEBuiltIn,
+			})
 		}
 
 		// Add libraries directory from config file
-		lm.AddLibrariesDir(configuration.LibrariesDir(configuration.Settings), libraries.User)
+		lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
+			Path:     configuration.LibrariesDir(configuration.Settings),
+			Location: libraries.User,
+		})
 	} else {
 		// Load libraries required for profile
 		for _, libraryRef := range profile.Libraries {
@@ -368,7 +378,10 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
 				taskCallback(&rpc.TaskProgress{Completed: true})
 			}
 
-			lm.AddLibrariesDir(libRoot, libraries.User)
+			lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
+				Path:     libRoot,
+				Location: libraries.User,
+			})
 		}
 	}
 
@@ -412,11 +425,7 @@ func UpdateLibrariesIndex(ctx context.Context, req *rpc.UpdateLibrariesIndexRequ
 	}
 	defer tmp.RemoveAll()
 
-	indexResource := resources.IndexResource{
-		URL:                          librariesmanager.LibraryIndexWithSignatureArchiveURL,
-		EnforceSignatureVerification: true,
-	}
-	if err := indexResource.Download(lm.IndexFile.Parent(), downloadCB); err != nil {
+	if err := globals.LibrariesIndexResource.Download(lm.IndexFile.Parent(), downloadCB); err != nil {
 		return err
 	}
 

Diff for: commands/lib/download.go

@@ -42,7 +42,7 @@ func LibraryDownload(ctx context.Context, req *rpc.LibraryDownloadRequest, downl
 
 	logrus.Info("Preparing download")
 
-	lib, err := findLibraryIndexRelease(lm, req)
+	lib, err := findLibraryIndexRelease(lm.Index, req)
 	if err != nil {
 		return nil, err
 	}

Diff for: commands/lib/install.go

@@ -72,7 +72,7 @@ func LibraryInstall(ctx context.Context, req *rpc.LibraryInstallRequest, downloa
 	// Find the libReleasesToInstall to install
 	libReleasesToInstall := map[*librariesindex.Release]*librariesmanager.LibraryInstallPlan{}
 	for _, lib := range toInstall {
-		libRelease, err := findLibraryIndexRelease(lm, &rpc.LibraryInstallRequest{
+		libRelease, err := findLibraryIndexRelease(lm.Index, &rpc.LibraryInstallRequest{
 			Name:    lib.GetName(),
 			Version: lib.GetVersionRequired(),
 		})

Diff for: commands/lib/resolve_deps.go

@@ -36,7 +36,7 @@ func LibraryResolveDependencies(ctx context.Context, req *rpc.LibraryResolveDepe
 	}
 
 	// Search the requested lib
-	reqLibRelease, err := findLibraryIndexRelease(lm, req)
+	reqLibRelease, err := findLibraryIndexRelease(lm.Index, req)
 	if err != nil {
 		return nil, err
 	}

Diff for: commands/lib/uninstall.go

@@ -32,7 +32,7 @@ func LibraryUninstall(ctx context.Context, req *rpc.LibraryUninstallRequest, tas
 		return err
 	}
 
-	ref, err := createLibIndexReference(lm, req)
+	ref, err := createLibIndexReference(req)
 	if err != nil {
 		return &cmderrors.InvalidLibraryError{Cause: err}
 	}

Diff for: commands/lib/utils.go

@@ -19,15 +19,14 @@ import (
 	"github.com/arduino/arduino-cli/commands"
 	"github.com/arduino/arduino-cli/commands/cmderrors"
 	"github.com/arduino/arduino-cli/internal/arduino/libraries/librariesindex"
-	"github.com/arduino/arduino-cli/internal/arduino/libraries/librariesmanager"
 )
 
 type libraryReferencer interface {
 	commands.Versioned
 	GetName() string
 }
 
-func createLibIndexReference(lm *librariesmanager.LibrariesManager, req libraryReferencer) (*librariesindex.Reference, error) {
+func createLibIndexReference(req libraryReferencer) (*librariesindex.Reference, error) {
 	version, err := commands.ParseVersion(req)
 	if err != nil {
 		return nil, &cmderrors.InvalidVersionError{Cause: err}
@@ -36,12 +35,12 @@ func createLibIndexReference(lm *librariesmanager.LibrariesManager, req libraryR
 	return &librariesindex.Reference{Name: req.GetName(), Version: version}, nil
 }
 
-func findLibraryIndexRelease(lm *librariesmanager.LibrariesManager, req libraryReferencer) (*librariesindex.Release, error) {
-	ref, err := createLibIndexReference(lm, req)
+func findLibraryIndexRelease(li *librariesindex.Index, req libraryReferencer) (*librariesindex.Release, error) {
+	ref, err := createLibIndexReference(req)
 	if err != nil {
 		return nil, err
 	}
-	lib := lm.Index.FindRelease(ref)
+	lib := li.FindRelease(ref)
 	if lib == nil {
 		return nil, &cmderrors.LibraryNotFoundError{Library: ref.String()}
 	}

Diff for: internal/arduino/builder/internal/detector/detector.go

@@ -608,20 +608,42 @@ func LibrariesLoader(
 			if err := builtInLibrariesFolders.ToAbs(); err != nil {
 				return nil, nil, nil, err
 			}
-			lm.AddLibrariesDir(builtInLibrariesFolders, libraries.IDEBuiltIn)
+			lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
+				Path:     builtInLibrariesFolders,
+				Location: libraries.IDEBuiltIn,
+			})
 		}
 
 		if actualPlatform != targetPlatform {
-			lm.AddPlatformReleaseLibrariesDir(actualPlatform, libraries.ReferencedPlatformBuiltIn)
+			lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
+				PlatformRelease: actualPlatform,
+				Path:            actualPlatform.GetLibrariesDir(),
+				Location:        libraries.ReferencedPlatformBuiltIn,
+			})
 		}
-		lm.AddPlatformReleaseLibrariesDir(targetPlatform, libraries.PlatformBuiltIn)
+		lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
+			PlatformRelease: targetPlatform,
+			Path:            targetPlatform.GetLibrariesDir(),
+			Location:        libraries.PlatformBuiltIn,
+		})
 
 		librariesFolders := otherLibrariesDirs
 		if err := librariesFolders.ToAbs(); err != nil {
 			return nil, nil, nil, err
 		}
 		for _, folder := range librariesFolders {
-			lm.AddLibrariesDir(folder, libraries.User)
+			lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
+				Path:     folder,
+				Location: libraries.User, // XXX: Should be libraries.Unmanaged?
+			})
+		}
+
+		for _, dir := range libraryDirs {
+			lm.AddLibrariesDir(&librariesmanager.LibrariesDir{
+				Path:            dir,
+				Location:        libraries.Unmanaged,
+				IsSingleLibrary: true,
+			})
 		}
 
 		for _, status := range lm.RescanLibraries() {
@@ -633,13 +655,6 @@ func LibrariesLoader(
 			// When we're gonna refactor the legacy package this will be gone.
 			verboseOut.Write([]byte(status.Message()))
 		}
-
-		for _, dir := range libraryDirs {
-			// Libraries specified this way have top priority
-			if err := lm.LoadLibraryFromDir(dir, libraries.Unmanaged); err != nil {
-				return nil, nil, nil, err
-			}
-		}
 	}
 
 	resolver := librariesresolver.NewCppResolver()

Diff for: internal/arduino/globals/globals.go

@@ -15,6 +15,12 @@
 
 package globals
 
+import (
+	"net/url"
+
+	"github.com/arduino/arduino-cli/internal/arduino/resources"
+)
+
 var (
 	// MainFileValidExtension is the extension that must be used for files in new sketches
 	MainFileValidExtension = ".ino"
@@ -63,4 +69,13 @@ var (
 
 	// DefaultIndexURL is the default index url
 	DefaultIndexURL = "https://downloads.arduino.cc/packages/package_index.tar.bz2"
+
+	// LibrariesIndexURL is the URL where to get the libraries index.
+	LibrariesIndexURL, _ = url.Parse("https://downloads.arduino.cc/libraries/library_index.tar.bz2")
+
+	// LibrariesIndexResource is the IndexResource to get the libraries index.
+	LibrariesIndexResource = resources.IndexResource{
+		URL:                          LibrariesIndexURL,
+		EnforceSignatureVerification: true,
+	}
 )

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

@@ -49,6 +49,7 @@ type LibrariesDir struct {
 	Path            *paths.Path
 	Location        libraries.LibraryLocation
 	PlatformRelease *cores.PlatformRelease
+	IsSingleLibrary bool // true if Path points directly to a library instad of a dir of libraries
 }
 
 var tr = i18n.Tr
@@ -102,46 +103,28 @@ func (lm *LibrariesManager) LoadIndex() error {
 // AddLibrariesDir adds path to the list of directories
 // to scan when searching for libraries. If a path is already
 // in the list it is ignored.
-func (lm *LibrariesManager) AddLibrariesDir(path *paths.Path, location libraries.LibraryLocation) {
-	for _, dir := range lm.LibrariesDir {
-		if dir.Path.EquivalentTo(path) {
-			return
-		}
-	}
-	logrus.WithField("dir", path).WithField("location", location.String()).Info("Adding libraries dir")
-	lm.LibrariesDir = append(lm.LibrariesDir, &LibrariesDir{
-		Path:     path,
-		Location: location,
-	})
-}
-
-// AddPlatformReleaseLibrariesDir add the libraries directory in the
-// specified PlatformRelease to the list of directories to scan when
-// searching for libraries.
-func (lm *LibrariesManager) AddPlatformReleaseLibrariesDir(plaftormRelease *cores.PlatformRelease, location libraries.LibraryLocation) {
-	path := plaftormRelease.GetLibrariesDir()
-	if path == nil {
+func (lm *LibrariesManager) AddLibrariesDir(libDir *LibrariesDir) {
+	if libDir.Path == nil {
 		return
 	}
 	for _, dir := range lm.LibrariesDir {
-		if dir.Path.EquivalentTo(path) {
+		if dir.Path.EquivalentTo(libDir.Path) {
 			return
 		}
 	}
-	logrus.WithField("dir", path).WithField("location", location.String()).Info("Adding libraries dir")
-	lm.LibrariesDir = append(lm.LibrariesDir, &LibrariesDir{
-		Path:            path,
-		Location:        location,
-		PlatformRelease: plaftormRelease,
-	})
+	logrus.WithField("dir", libDir.Path).
+		WithField("location", libDir.Location.String()).
+		WithField("isSingleLibrary", libDir.IsSingleLibrary).
+		Info("Adding libraries dir")
+	lm.LibrariesDir = append(lm.LibrariesDir, libDir)
 }
 
 // RescanLibraries reload all installed libraries in the system.
 func (lm *LibrariesManager) RescanLibraries() []*status.Status {
 	lm.clearLibraries()
 	statuses := []*status.Status{}
 	for _, dir := range lm.LibrariesDir {
-		if errs := lm.LoadLibrariesFromDir(dir); len(errs) > 0 {
+		if errs := lm.loadLibrariesFromDir(dir); len(errs) > 0 {
 			statuses = append(statuses, errs...)
 		}
 	}
@@ -164,22 +147,29 @@ func (lm *LibrariesManager) getLibrariesDir(installLocation libraries.LibraryLoc
 	}
 }
 
-// LoadLibrariesFromDir loads all libraries in the given directory. Returns
+// loadLibrariesFromDir loads all libraries in the given directory. Returns
 // nil if the directory doesn't exists.
-func (lm *LibrariesManager) LoadLibrariesFromDir(librariesDir *LibrariesDir) []*status.Status {
+func (lm *LibrariesManager) loadLibrariesFromDir(librariesDir *LibrariesDir) []*status.Status {
 	statuses := []*status.Status{}
-	subDirs, err := librariesDir.Path.ReadDir()
-	if os.IsNotExist(err) {
-		return statuses
-	}
-	if err != nil {
-		s := status.Newf(codes.FailedPrecondition, tr("reading dir %[1]s: %[2]s"), librariesDir.Path, err)
-		return append(statuses, s)
+
+	var libDirs paths.PathList
+	if librariesDir.IsSingleLibrary {
+		libDirs.Add(librariesDir.Path)
+	} else {
+		d, err := librariesDir.Path.ReadDir()
+		if os.IsNotExist(err) {
+			return statuses
+		}
+		if err != nil {
+			s := status.Newf(codes.FailedPrecondition, tr("reading dir %[1]s: %[2]s"), librariesDir.Path, err)
+			return append(statuses, s)
+		}
+		d.FilterDirs()
+		d.FilterOutHiddenFiles()
+		libDirs = d
 	}
-	subDirs.FilterDirs()
-	subDirs.FilterOutHiddenFiles()
 
-	for _, subDir := range subDirs {
+	for _, subDir := range libDirs {
 		library, err := libraries.Load(subDir, librariesDir.Location)
 		if err != nil {
 			s := status.Newf(codes.Internal, tr("loading library from %[1]s: %[2]s"), subDir, err)
@@ -195,25 +185,6 @@ func (lm *LibrariesManager) LoadLibrariesFromDir(librariesDir *LibrariesDir) []*
 	return statuses
 }
 
-// LoadLibraryFromDir loads one single library from the libRootDir.
-// libRootDir must point to the root of a valid library.
-// An error is returned if the path doesn't exist or loading of the library fails.
-func (lm *LibrariesManager) LoadLibraryFromDir(libRootDir *paths.Path, location libraries.LibraryLocation) error {
-	if libRootDir.NotExist() {
-		return fmt.Errorf(tr("library path does not exist: %s"), libRootDir)
-	}
-
-	library, err := libraries.Load(libRootDir, location)
-	if err != nil {
-		return fmt.Errorf(tr("loading library from %[1]s: %[2]s"), libRootDir, err)
-	}
-
-	alternatives := lm.Libraries[library.Name]
-	alternatives.Add(library)
-	lm.Libraries[library.Name] = alternatives
-	return nil
-}
-
 // FindByReference return the installed libraries matching the Reference
 // name and version or, if the version is nil, the libraries installed
 // in the installLocation.