Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Sep 21, 2022
Merged
24 changes: 24 additions & 0 deletions arduino/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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())
}
4 changes: 2 additions & 2 deletions arduino/libraries/libraries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion arduino/libraries/librariesindex/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions arduino/libraries/librariesindex/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand Down
28 changes: 28 additions & 0 deletions arduino/libraries/librarieslist.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package libraries

import (
"sort"

semver "go.bug.st/relaxed-semver"
)

// List is a list of Libraries
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
71 changes: 41 additions & 30 deletions arduino/libraries/librariesmanager/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -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"))
}

Expand All @@ -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)
}
Expand All @@ -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).
Expand Down