[breaking] Fix behaviour of lib commands when a library is installed multiple times (#1878)

* Removed LibraryAlternatives object in favor of libraries.List

* Use RealName instead of (dir) Name as key in librarymanager.Library map

* Refactored some librarymanager functions to query libraries list

* Simplified librariesmanager.Install

It does not require installLocation since it can be obtained with
libPath.Parent()

* Added checks for multiple library installation

* Added test to check that the cli is able to handle multiple lib installations

* Renamed fields in Library structure: Name->CanonicalName; RealName->Name

* Use Name instead of CanonicalName in LibraryList

This fixes also `lib list` and `lib examples`.

* Use `Name` field where appropriate instead of `CanonicalName`

* Updated documentation

* Update arduino/libraries/librarieslist.go

Co-authored-by: Umberto Baldi <34278123+umbynos@users.noreply.github.com>

* Improved integration test

Co-authored-by: Umberto Baldi <34278123+umbynos@users.noreply.github.com>

Author: cmaglie

arduino/errors.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/arduino/arduino-cli/i18n"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	"github.com/arduino/go-paths-helper"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -810,3 +811,26 @@ func (e *MultiplePlatformsError) Error() string {
 func (e *MultiplePlatformsError) ToRPCStatus() *status.Status {
 	return status.New(codes.InvalidArgument, e.Error())
 }
+
+// MultipleLibraryInstallDetected is returned when the user request an
+// operation on a library but multiple libraries with the same name
+// (in library.properties) are detected.
+type MultipleLibraryInstallDetected struct {
+	LibName string
+	LibsDir paths.PathList
+	Message string
+}
+
+func (e *MultipleLibraryInstallDetected) Error() string {
+	res := tr("The library %s has multiple installations:", e.LibName) + "\n"
+	for _, lib := range e.LibsDir {
+		res += fmt.Sprintf("- %s\n", lib)
+	}
+	res += e.Message
+	return res
+}
+
+// ToRPCStatus converts the error into a *status.Status
+func (e *MultipleLibraryInstallDetected) ToRPCStatus() *status.Status {
+	return status.New(codes.InvalidArgument, e.Error())
+}

arduino/libraries/libraries.go

@@ -63,12 +63,12 @@ type Library struct {
 	Types []string `json:"types,omitempty"`
 
 	InstallDir             *paths.Path
+	CanonicalName          string
 	SourceDir              *paths.Path
 	UtilityDir             *paths.Path
 	Location               LibraryLocation
 	ContainerPlatform      *cores.PlatformRelease `json:""`
 	Layout                 LibraryLayout
-	RealName               string
 	DotALinkage            bool
 	Precompiled            bool
 	PrecompiledWithSources bool
@@ -132,7 +132,7 @@ func (library *Library) ToRPCLibrary() (*rpc.Library, error) {
 		Location:          library.Location.ToRPCLibraryLocation(),
 		ContainerPlatform: platformOrEmpty(library.ContainerPlatform),
 		Layout:            library.Layout.ToRPCLibraryLayout(),
-		RealName:          library.RealName,
+		RealName:          library.Name,
 		DotALinkage:       library.DotALinkage,
 		Precompiled:       library.Precompiled,
 		LdFlags:           library.LDflags,

arduino/libraries/librariesindex/index.go

@@ -125,7 +125,7 @@ func (idx *Index) FindRelease(ref *Reference) *Release {
 // FindIndexedLibrary search an indexed library that matches the provided
 // installed library or nil if not found
 func (idx *Index) FindIndexedLibrary(lib *libraries.Library) *Library {
-	return idx.Libraries[lib.RealName]
+	return idx.Libraries[lib.Name]
 }
 
 // FindLibraryUpdate check if an installed library may be updated using

arduino/libraries/librariesindex/index_test.go

@@ -72,22 +72,22 @@ func TestIndexer(t *testing.T) {
 	})
 	require.Nil(t, rtcInexistent)
 
-	rtc := index.FindIndexedLibrary(&libraries.Library{RealName: "RTCZero"})
+	rtc := index.FindIndexedLibrary(&libraries.Library{Name: "RTCZero"})
 	require.NotNil(t, rtc)
 	require.Equal(t, "RTCZero", rtc.Name)
 
-	rtcUpdate := index.FindLibraryUpdate(&libraries.Library{RealName: "RTCZero", Version: semver.MustParse("1.0.0")})
+	rtcUpdate := index.FindLibraryUpdate(&libraries.Library{Name: "RTCZero", Version: semver.MustParse("1.0.0")})
 	require.NotNil(t, rtcUpdate)
 	require.Equal(t, "RTCZero@1.6.0", rtcUpdate.String())
 
-	rtcUpdateNoVersion := index.FindLibraryUpdate(&libraries.Library{RealName: "RTCZero", Version: nil})
+	rtcUpdateNoVersion := index.FindLibraryUpdate(&libraries.Library{Name: "RTCZero", Version: nil})
 	require.NotNil(t, rtcUpdateNoVersion)
 	require.Equal(t, "RTCZero@1.6.0", rtcUpdateNoVersion.String())
 
-	rtcNoUpdate := index.FindLibraryUpdate(&libraries.Library{RealName: "RTCZero", Version: semver.MustParse("3.0.0")})
+	rtcNoUpdate := index.FindLibraryUpdate(&libraries.Library{Name: "RTCZero", Version: semver.MustParse("3.0.0")})
 	require.Nil(t, rtcNoUpdate)
 
-	rtcInexistent2 := index.FindLibraryUpdate(&libraries.Library{RealName: "RTCZero-blah", Version: semver.MustParse("1.0.0")})
+	rtcInexistent2 := index.FindLibraryUpdate(&libraries.Library{Name: "RTCZero-blah", Version: semver.MustParse("1.0.0")})
 	require.Nil(t, rtcInexistent2)
 
 	resolve1 := index.ResolveDependencies(alp.Releases["1.2.1"])

arduino/libraries/librarieslist.go

@@ -17,6 +17,8 @@ package libraries
 
 import (
 	"sort"
+
+	semver "go.bug.st/relaxed-semver"
 )
 
 // List is a list of Libraries
@@ -39,6 +41,16 @@ func (list *List) Add(libs ...*Library) {
 	}
 }
 
+// Remove removes the library from the list
+func (list *List) Remove(library *Library) {
+	for i, lib := range *list {
+		if lib == library {
+			*list = append((*list)[:i], (*list)[i+1:]...)
+			return
+		}
+	}
+}
+
 // FindByName returns the first library in the list that match
 // the specified name or nil if not found
 func (list *List) FindByName(name string) *Library {
@@ -50,6 +62,22 @@ func (list *List) FindByName(name string) *Library {
 	return nil
 }
 
+// FilterByVersionAndInstallLocation returns the libraries matching the provided version and install location. If version
+// is nil all version are matched.
+func (list *List) FilterByVersionAndInstallLocation(version *semver.Version, installLocation LibraryLocation) List {
+	var found List
+	for _, lib := range *list {
+		if lib.Location != installLocation {
+			continue
+		}
+		if version != nil && !lib.Version.Equal(version) {
+			continue
+		}
+		found.Add(lib)
+	}
+	return found
+}
+
 // SortByName sorts the libraries by name
 func (list *List) SortByName() {
 	sort.Slice(*list, func(i, j int) bool {

arduino/libraries/librariesmanager/install.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"strings"
 
+	"github.com/arduino/arduino-cli/arduino"
 	"github.com/arduino/arduino-cli/arduino/globals"
 	"github.com/arduino/arduino-cli/arduino/libraries"
 	"github.com/arduino/arduino-cli/arduino/libraries/librariesindex"
@@ -49,45 +50,51 @@ var (
 // 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, installLocation libraries.LibraryLocation) (*paths.Path, *libraries.Library, error) {
-	saneName := utils.SanitizeName(indexLibrary.Library.Name)
+	installDir := lm.getLibrariesDir(installLocation)
+	if installDir == nil {
+		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"))
+	}
 
-	var replaced *libraries.Library
-	if installedLibs, have := lm.Libraries[saneName]; have {
-		for _, installedLib := range installedLibs.Alternatives {
-			if installedLib.Location != installLocation {
-				continue
-			}
-			if installedLib.Version != nil && installedLib.Version.Equal(indexLibrary.Version) {
-				return installedLib.InstallDir, nil, ErrAlreadyInstalled
-			}
-			replaced = installedLib
+	name := indexLibrary.Library.Name
+	libs := lm.FindByReference(&librariesindex.Reference{Name: name}, installLocation)
+	for _, lib := range libs {
+		if lib.Version != nil && lib.Version.Equal(indexLibrary.Version) {
+			return lib.InstallDir, nil, ErrAlreadyInstalled
 		}
 	}
 
-	libsDir := lm.getLibrariesDir(installLocation)
-	if libsDir == nil {
-		if installLocation == libraries.User {
-			return nil, nil, fmt.Errorf(tr("User directory not set"))
+	if len(libs) > 1 {
+		libsDir := paths.NewPathList()
+		for _, lib := range libs {
+			libsDir.Add(lib.InstallDir)
+		}
+		return nil, nil, &arduino.MultipleLibraryInstallDetected{
+			LibName: name,
+			LibsDir: libsDir,
+			Message: tr("Automatic library install can't be performed in this case, please manually remove all duplicates and retry."),
 		}
-		return nil, nil, fmt.Errorf(tr("Builtin libraries directory not set"))
 	}
 
-	libPath := libsDir.Join(saneName)
-	if replaced != nil && replaced.InstallDir.EquivalentTo(libPath) {
+	var replaced *libraries.Library
+	if len(libs) == 1 {
+		replaced = libs[0]
+	}
 
+	libPath := installDir.Join(utils.SanitizeName(indexLibrary.Library.Name))
+	if replaced != nil && replaced.InstallDir.EquivalentTo(libPath) {
+		return libPath, replaced, nil
 	} else if libPath.IsDir() {
 		return nil, nil, fmt.Errorf(tr("destination dir %s already exists, cannot install"), libPath)
 	}
 	return libPath, replaced, nil
 }
 
 // Install installs a library on the specified path.
-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"))
-	}
-	return indexLibrary.Resource.Install(lm.DownloadsDir, libsDir, libPath)
+func (lm *LibrariesManager) Install(indexLibrary *librariesindex.Release, libPath *paths.Path) error {
+	return indexLibrary.Resource.Install(lm.DownloadsDir, libPath.Parent(), libPath)
 }
 
 // Uninstall removes a Library
@@ -99,7 +106,9 @@ func (lm *LibrariesManager) Uninstall(lib *libraries.Library) error {
 		return fmt.Errorf(tr("removing lib directory: %s"), err)
 	}
 
-	lm.Libraries[lib.Name].Remove(lib)
+	alternatives := lm.Libraries[lib.Name]
+	alternatives.Remove(lib)
+	lm.Libraries[lib.Name] = alternatives
 	return nil
 }
 
@@ -189,8 +198,8 @@ func (lm *LibrariesManager) InstallZipLib(ctx context.Context, archivePath strin
 
 // InstallGitLib installs a library hosted on a git repository on the specified path.
 func (lm *LibrariesManager) InstallGitLib(gitURL string, overwrite bool) error {
-	libsDir := lm.getLibrariesDir(libraries.User)
-	if libsDir == nil {
+	installDir := lm.getLibrariesDir(libraries.User)
+	if installDir == nil {
 		return fmt.Errorf(tr("User directory not set"))
 	}
 
@@ -202,10 +211,9 @@ func (lm *LibrariesManager) InstallGitLib(gitURL string, overwrite bool) error {
 		return err
 	}
 
-	installPath := libsDir.Join(libraryName)
-
 	// Deletes libraries folder if already installed
-	if _, ok := lm.Libraries[libraryName]; ok {
+	installPath := installDir.Join(libraryName)
+	if installPath.IsDir() {
 		if !overwrite {
 			return fmt.Errorf(tr("library %s already installed"), libraryName)
 		}
@@ -215,6 +223,9 @@ func (lm *LibrariesManager) InstallGitLib(gitURL string, overwrite bool) error {
 			Trace("Deleting library")
 		installPath.RemoveAll()
 	}
+	if installPath.Exist() {
+		return fmt.Errorf(tr("could not create directory %s: a file with the same name exists!", installPath))
+	}
 
 	logrus.
 		WithField("library name", libraryName).