From fe1ca8a7049d0e808c353ee694c984b47e0b9006 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 8 Jan 2024 12:33:34 +0100 Subject: [PATCH 1/3] Added LibrariesManager.Clone() / Auto-scan libraries on LibrariesManager.Build() --- commands/instances.go | 13 ++----- commands/internal/instances/instances.go | 7 +++- .../builder/internal/detector/detector.go | 26 ++++++------- .../librariesmanager/librariesmanager.go | 38 +++++++++++++------ .../librariesmanager/librariesmanager_test.go | 20 +++++++--- 5 files changed, 61 insertions(+), 43 deletions(-) diff --git a/commands/instances.go b/commands/instances.go index e6d9a2ebd68..de0f74bf674 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -389,16 +389,11 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro } } - lm := lmb.Build() + lm, libsLoadingWarnings := lmb.Build() _ = instances.SetLibraryManager(instance, lm) // should never fail - - { - lmi, release := lm.NewInstaller() - for _, status := range lmi.RescanLibraries() { - logrus.WithError(status.Err()).Warnf("Error loading library") - // TODO: report as warning: responseError(err) - } - release() + for _, status := range libsLoadingWarnings { + logrus.WithError(status.Err()).Warnf("Error loading library") + // TODO: report as warning: responseError(err) } // Refreshes the locale used, this will change the diff --git a/commands/internal/instances/instances.go b/commands/internal/instances/instances.go index a26a2dee1f8..028e82e6738 100644 --- a/commands/internal/instances/instances.go +++ b/commands/internal/instances/instances.go @@ -126,9 +126,12 @@ func Create(dataDir, packagesDir, downloadsDir *paths.Path, extraUserAgent ...st } tempDir := dataDir.Join("tmp") + pm := packagemanager.NewBuilder(dataDir, packagesDir, downloadsDir, tempDir, userAgent).Build() + lm, _ := librariesmanager.NewBuilder().Build() + instance := &coreInstance{ - pm: packagemanager.NewBuilder(dataDir, packagesDir, downloadsDir, tempDir, userAgent).Build(), - lm: librariesmanager.NewBuilder().Build(), + pm: pm, + lm: lm, li: librariesindex.EmptyIndex, } diff --git a/internal/arduino/builder/internal/detector/detector.go b/internal/arduino/builder/internal/detector/detector.go index 7d0a67f7cdd..d0436d5bf61 100644 --- a/internal/arduino/builder/internal/detector/detector.go +++ b/internal/arduino/builder/internal/detector/detector.go @@ -598,7 +598,7 @@ func LibrariesLoader( if useCachedLibrariesResolution { // Since we are using the cached libraries resolution // the library manager is not needed. - lm = librariesmanager.NewBuilder().Build() + lm, _ = librariesmanager.NewBuilder().Build() } if librariesManager == nil { lmb := librariesmanager.NewBuilder() @@ -646,21 +646,17 @@ func LibrariesLoader( }) } - lm = lmb.Build() - - { - lmi, release := lm.NewInstaller() - for _, status := range lmi.RescanLibraries() { - // With the refactoring of the initialization step of the CLI we changed how - // errors are returned when loading platforms and libraries, that meant returning a list of - // errors instead of a single one to enhance the experience for the user. - // I have no intention right now to start a refactoring of the legacy package too, so - // here's this shitty solution for now. - // When we're gonna refactor the legacy package this will be gone. - verboseOut.Write([]byte(status.Message())) - } - release() + newLm, libsLoadingWarnings := lmb.Build() + for _, status := range libsLoadingWarnings { + // With the refactoring of the initialization step of the CLI we changed how + // errors are returned when loading platforms and libraries, that meant returning a list of + // errors instead of a single one to enhance the experience for the user. + // I have no intention right now to start a refactoring of the legacy package too, so + // here's this shitty solution for now. + // When we're gonna refactor the legacy package this will be gone. + verboseOut.Write([]byte(status.Message())) } + lm = newLm } allLibs := lm.FindAllInstalled() diff --git a/internal/arduino/libraries/librariesmanager/librariesmanager.go b/internal/arduino/libraries/librariesmanager/librariesmanager.go index 64102cf46f9..c8ce9867d8d 100644 --- a/internal/arduino/libraries/librariesmanager/librariesmanager.go +++ b/internal/arduino/libraries/librariesmanager/librariesmanager.go @@ -65,6 +65,7 @@ type LibrariesDir struct { Location libraries.LibraryLocation PlatformRelease *cores.PlatformRelease IsSingleLibrary bool // true if Path points directly to a library instad of a dir of libraries + scanned bool } var tr = i18n.Tr @@ -95,14 +96,19 @@ func NewBuilder() *Builder { } } -// NewBuilder creates a Builder with the same configuration of this -// LibrariesManager. A "commit" function callback is returned: calling -// this function will write the new configuration into this LibrariesManager. -func (lm *LibrariesManager) NewBuilder() (*Builder, func()) { +// Clone creates a Builder starting with a copy of the same configuration +// of this LibrariesManager. At the moment of the Build() only the added +// libraries directories will be scanned, keeping the exising directories +// "cached" to optimize scan. If you need to do a full rescan you must use +// the RescanLibraries method of the Installer. +func (lm *LibrariesManager) Clone() *Builder { lmb := NewBuilder() - return lmb, func() { - lmb.BuildIntoExistingLibrariesManager(lm) + lmb.librariesDir = append(lmb.librariesDir, lm.librariesDir...) + for libName, libAlternatives := range lm.libraries { + // TODO: Maybe we should deep clone libAlternatives... + lmb.libraries[libName] = append(lmb.libraries[libName], libAlternatives...) } + return lmb } // NewExplorer returns a new Explorer. The returned function must be called @@ -120,10 +126,18 @@ func (lm *LibrariesManager) NewInstaller() (*Installer, func()) { } // Build builds a new LibrariesManager. -func (lmb *Builder) Build() *LibrariesManager { +func (lmb *Builder) Build() (*LibrariesManager, []*status.Status) { + var statuses []*status.Status res := &LibrariesManager{} + for _, dir := range res.librariesDir { + if !dir.scanned { + if errs := lmb.loadLibrariesFromDir(dir); len(errs) > 0 { + statuses = append(statuses, errs...) + } + } + } lmb.BuildIntoExistingLibrariesManager(res) - return res + return res, statuses } // BuildIntoExistingLibrariesManager will overwrite the given LibrariesManager instead @@ -184,9 +198,11 @@ func (lm *LibrariesManager) getLibrariesDir(installLocation libraries.LibraryLoc // loadLibrariesFromDir loads all libraries in the given directory. Returns // nil if the directory doesn't exists. -func (lmi *Installer) loadLibrariesFromDir(librariesDir *LibrariesDir) []*status.Status { +func (lm *LibrariesManager) loadLibrariesFromDir(librariesDir *LibrariesDir) []*status.Status { statuses := []*status.Status{} + librariesDir.scanned = true + var libDirs paths.PathList if librariesDir.IsSingleLibrary { libDirs.Add(librariesDir.Path) @@ -212,9 +228,9 @@ func (lmi *Installer) loadLibrariesFromDir(librariesDir *LibrariesDir) []*status continue } library.ContainerPlatform = librariesDir.PlatformRelease - alternatives := lmi.libraries[library.Name] + alternatives := lm.libraries[library.Name] alternatives.Add(library) - lmi.libraries[library.Name] = alternatives + lm.libraries[library.Name] = alternatives } return statuses diff --git a/internal/arduino/libraries/librariesmanager/librariesmanager_test.go b/internal/arduino/libraries/librariesmanager/librariesmanager_test.go index 2393868999e..0b7fd3a7857 100644 --- a/internal/arduino/libraries/librariesmanager/librariesmanager_test.go +++ b/internal/arduino/libraries/librariesmanager/librariesmanager_test.go @@ -21,17 +21,25 @@ import ( "github.com/stretchr/testify/require" ) -func Test_RescanLibrariesCallClear(t *testing.T) { +func TestLibrariesBuilderScanCloneRescan(t *testing.T) { lmb := NewBuilder() lmb.libraries["testLibA"] = libraries.List{} lmb.libraries["testLibB"] = libraries.List{} - lm := lmb.Build() + lm, warns := lmb.Build() + require.Empty(t, warns) + require.Len(t, lm.libraries, 2) + // Cloning should keep existing libraries + lm2, warns2 := lm.Clone().Build() + require.Empty(t, warns2) + require.Len(t, lm2.libraries, 2) + + // Full rescan should update libs { - lmi, release := lm.NewInstaller() - lmi.RescanLibraries() + lmi2, release := lm2.NewInstaller() + lmi2.RescanLibraries() release() } - - require.Len(t, lm.libraries, 0) + require.Len(t, lm.libraries, 2) // Ensure deep-coping worked as expected... + require.Len(t, lm2.libraries, 0) } From 645c398673f62a8ccbd316b988bbd332d7b5ad7a Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 8 Jan 2024 12:57:31 +0100 Subject: [PATCH 2/3] Ensure AddLibrariesDir do not share input parameters --- commands/instances.go | 8 ++++---- internal/arduino/builder/internal/detector/detector.go | 10 +++++----- .../libraries/librariesmanager/librariesmanager.go | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/commands/instances.go b/commands/instances.go index de0f74bf674..b265e51893d 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -307,7 +307,7 @@ 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 { - lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{ + lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ PlatformRelease: platformRelease, Path: platformRelease.GetLibrariesDir(), Location: libraries.PlatformBuiltIn, @@ -335,14 +335,14 @@ 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 { - lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{ + lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ Path: bundledLibsDir, Location: libraries.IDEBuiltIn, }) } // Add libraries directory from config file - lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{ + lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ Path: configuration.LibrariesDir(configuration.Settings), Location: libraries.User, }) @@ -382,7 +382,7 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro taskCallback(&rpc.TaskProgress{Completed: true}) } - lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{ + lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ Path: libRoot, Location: libraries.User, }) diff --git a/internal/arduino/builder/internal/detector/detector.go b/internal/arduino/builder/internal/detector/detector.go index d0436d5bf61..9285846fcf1 100644 --- a/internal/arduino/builder/internal/detector/detector.go +++ b/internal/arduino/builder/internal/detector/detector.go @@ -608,20 +608,20 @@ func LibrariesLoader( if err := builtInLibrariesFolders.ToAbs(); err != nil { return nil, nil, nil, err } - lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{ + lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ Path: builtInLibrariesFolders, Location: libraries.IDEBuiltIn, }) } if actualPlatform != targetPlatform { - lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{ + lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ PlatformRelease: actualPlatform, Path: actualPlatform.GetLibrariesDir(), Location: libraries.ReferencedPlatformBuiltIn, }) } - lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{ + lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ PlatformRelease: targetPlatform, Path: targetPlatform.GetLibrariesDir(), Location: libraries.PlatformBuiltIn, @@ -632,14 +632,14 @@ func LibrariesLoader( return nil, nil, nil, err } for _, folder := range librariesFolders { - lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{ + lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ Path: folder, Location: libraries.User, // XXX: Should be libraries.Unmanaged? }) } for _, dir := range libraryDirs { - lmb.AddLibrariesDir(&librariesmanager.LibrariesDir{ + lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ Path: dir, Location: libraries.Unmanaged, IsSingleLibrary: true, diff --git a/internal/arduino/libraries/librariesmanager/librariesmanager.go b/internal/arduino/libraries/librariesmanager/librariesmanager.go index c8ce9867d8d..e51930ea286 100644 --- a/internal/arduino/libraries/librariesmanager/librariesmanager.go +++ b/internal/arduino/libraries/librariesmanager/librariesmanager.go @@ -98,7 +98,7 @@ func NewBuilder() *Builder { // Clone creates a Builder starting with a copy of the same configuration // of this LibrariesManager. At the moment of the Build() only the added -// libraries directories will be scanned, keeping the exising directories +// libraries directories will be scanned, keeping the existing directories // "cached" to optimize scan. If you need to do a full rescan you must use // the RescanLibraries method of the Installer. func (lm *LibrariesManager) Clone() *Builder { @@ -152,7 +152,7 @@ func (lmb *Builder) BuildIntoExistingLibrariesManager(old *LibrariesManager) { // 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 (lmb *Builder) AddLibrariesDir(libDir *LibrariesDir) { +func (lmb *Builder) AddLibrariesDir(libDir LibrariesDir) { if libDir.Path == nil { return } @@ -165,7 +165,7 @@ func (lmb *Builder) AddLibrariesDir(libDir *LibrariesDir) { WithField("location", libDir.Location.String()). WithField("isSingleLibrary", libDir.IsSingleLibrary). Info("Adding libraries dir") - lmb.librariesDir = append(lmb.librariesDir, libDir) + lmb.librariesDir = append(lmb.librariesDir, &libDir) } // RescanLibraries reload all installed libraries in the system. From bcad9c6bde6bfa64a3504f6f6cc59ea1e84d7217 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 8 Jan 2024 14:16:47 +0100 Subject: [PATCH 3/3] Fixed wrong loop... ooops --- internal/arduino/libraries/librariesmanager/librariesmanager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/arduino/libraries/librariesmanager/librariesmanager.go b/internal/arduino/libraries/librariesmanager/librariesmanager.go index e51930ea286..123513b2d31 100644 --- a/internal/arduino/libraries/librariesmanager/librariesmanager.go +++ b/internal/arduino/libraries/librariesmanager/librariesmanager.go @@ -129,7 +129,7 @@ func (lm *LibrariesManager) NewInstaller() (*Installer, func()) { func (lmb *Builder) Build() (*LibrariesManager, []*status.Status) { var statuses []*status.Status res := &LibrariesManager{} - for _, dir := range res.librariesDir { + for _, dir := range lmb.librariesDir { if !dir.scanned { if errs := lmb.loadLibrariesFromDir(dir); len(errs) > 0 { statuses = append(statuses, errs...)