Removed state-altering methods from PackageManager

They have been moved into a Builder object that has the ability to build
a new PackageManager. This allows to clearly separate subrotuines that
actually change the status of the PackageManager from subroutines that
just need to query it.

Author: cmaglie

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

@@ -33,7 +33,7 @@ import (
 )
 
 // LoadHardware read all plaforms from the configured paths
-func (pm *PackageManager) LoadHardware() []error {
+func (pm *Builder) LoadHardware() []error {
 	hardwareDirs := configuration.HardwareDirectories(configuration.Settings)
 	merr := pm.LoadHardwareFromDirectories(hardwareDirs)
 
@@ -44,7 +44,7 @@ func (pm *PackageManager) LoadHardware() []error {
 }
 
 // LoadHardwareFromDirectories load plaforms from a set of directories
-func (pm *PackageManager) LoadHardwareFromDirectories(hardwarePaths paths.PathList) []error {
+func (pm *Builder) LoadHardwareFromDirectories(hardwarePaths paths.PathList) []error {
 	var merr []error
 	for _, path := range hardwarePaths {
 		merr = append(merr, pm.LoadHardwareFromDirectory(path)...)
@@ -53,7 +53,7 @@ func (pm *PackageManager) LoadHardwareFromDirectories(hardwarePaths paths.PathLi
 }
 
 // LoadHardwareFromDirectory read a plaform from the path passed as parameter
-func (pm *PackageManager) LoadHardwareFromDirectory(path *paths.Path) []error {
+func (pm *Builder) LoadHardwareFromDirectory(path *paths.Path) []error {
 	var merr []error
 	pm.log.Infof("Loading hardware from: %s", path)
 	if err := path.ToAbs(); err != nil {
@@ -142,7 +142,7 @@ func (pm *PackageManager) LoadHardwareFromDirectory(path *paths.Path) []error {
 // loadPlatforms load plaftorms from the specified directory assuming that they belongs
 // to the targetPackage object passed as parameter.
 // A list of gRPC Status error is returned for each Platform failed to load.
-func (pm *PackageManager) loadPlatforms(targetPackage *cores.Package, packageDir *paths.Path) []error {
+func (pm *Builder) loadPlatforms(targetPackage *cores.Package, packageDir *paths.Path) []error {
 	pm.log.Infof("Loading package %s from: %s", targetPackage.Name, packageDir)
 
 	var merr []error
@@ -174,7 +174,7 @@ func (pm *PackageManager) loadPlatforms(targetPackage *cores.Package, packageDir
 // loadPlatform loads a single platform and all its installed releases given a platformPath.
 // platformPath must be a directory.
 // Returns a gRPC Status error in case of failures.
-func (pm *PackageManager) loadPlatform(targetPackage *cores.Package, architecture string, platformPath *paths.Path) error {
+func (pm *Builder) loadPlatform(targetPackage *cores.Package, architecture string, platformPath *paths.Path) error {
 	// This is not a platform
 	if platformPath.IsNotDir() {
 		return errors.New(tr("path is not a platform directory: %s", platformPath))
@@ -279,7 +279,7 @@ func (pm *PackageManager) loadPlatform(targetPackage *cores.Package, architectur
 	return nil
 }
 
-func (pm *PackageManager) loadPlatformRelease(platform *cores.PlatformRelease, path *paths.Path) error {
+func (pm *Builder) loadPlatformRelease(platform *cores.PlatformRelease, path *paths.Path) error {
 	platform.InstallDir = path
 
 	// Some useful paths
@@ -440,14 +440,14 @@ func convertLegacyNetworkPatternToPluggableDiscovery(props *properties.Map, newT
 	return res
 }
 
-func (pm *PackageManager) loadProgrammer(programmerProperties *properties.Map) *cores.Programmer {
+func (pm *Builder) loadProgrammer(programmerProperties *properties.Map) *cores.Programmer {
 	return &cores.Programmer{
 		Name:       programmerProperties.Get("name"),
 		Properties: programmerProperties,
 	}
 }
 
-func (pm *PackageManager) loadBoards(platform *cores.PlatformRelease) error {
+func (pm *Builder) loadBoards(platform *cores.PlatformRelease) error {
 	if platform.InstallDir == nil {
 		return fmt.Errorf(tr("platform not installed"))
 	}
@@ -591,7 +591,7 @@ func convertUploadToolsToPluggableDiscovery(props *properties.Map) {
 
 // LoadToolsFromPackageDir loads a set of tools from the given toolsPath. The tools will be loaded
 // in the given *Package.
-func (pm *PackageManager) LoadToolsFromPackageDir(targetPackage *cores.Package, toolsPath *paths.Path) []error {
+func (pm *Builder) LoadToolsFromPackageDir(targetPackage *cores.Package, toolsPath *paths.Path) []error {
 	pm.log.Infof("Loading tools from dir: %s", toolsPath)
 
 	var merr []error
@@ -612,7 +612,7 @@ func (pm *PackageManager) LoadToolsFromPackageDir(targetPackage *cores.Package,
 	return merr
 }
 
-func (pm *PackageManager) loadToolReleasesFromTool(tool *cores.Tool, toolPath *paths.Path) error {
+func (pm *Builder) loadToolReleasesFromTool(tool *cores.Tool, toolPath *paths.Path) error {
 	toolVersions, err := toolPath.ReadDir()
 	if err != nil {
 		return err
@@ -629,7 +629,7 @@ func (pm *PackageManager) loadToolReleasesFromTool(tool *cores.Tool, toolPath *p
 	return nil
 }
 
-func (pm *PackageManager) loadToolReleaseFromDirectory(tool *cores.Tool, version *semver.RelaxedVersion, toolReleasePath *paths.Path) error {
+func (pm *Builder) loadToolReleaseFromDirectory(tool *cores.Tool, version *semver.RelaxedVersion, toolReleasePath *paths.Path) error {
 	if absToolReleasePath, err := toolReleasePath.Abs(); err != nil {
 		return errors.New(tr("error opening %s", absToolReleasePath))
 	} else if !absToolReleasePath.IsDir() {
@@ -643,7 +643,7 @@ func (pm *PackageManager) loadToolReleaseFromDirectory(tool *cores.Tool, version
 }
 
 // LoadToolsFromBundleDirectories FIXMEDOC
-func (pm *PackageManager) LoadToolsFromBundleDirectories(dirs paths.PathList) []error {
+func (pm *Builder) LoadToolsFromBundleDirectories(dirs paths.PathList) []error {
 	var merr []error
 	for _, dir := range dirs {
 		if err := pm.LoadToolsFromBundleDirectory(dir); err != nil {
@@ -654,7 +654,7 @@ func (pm *PackageManager) LoadToolsFromBundleDirectories(dirs paths.PathList) []
 }
 
 // LoadToolsFromBundleDirectory FIXMEDOC
-func (pm *PackageManager) LoadToolsFromBundleDirectory(toolsPath *paths.Path) error {
+func (pm *Builder) LoadToolsFromBundleDirectory(toolsPath *paths.Path) error {
 	pm.log.Infof("Loading tools from bundle dir: %s", toolsPath)
 
 	// We scan toolsPath content to find a "builtin_tools_versions.txt", if such file exists

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

@@ -111,8 +111,8 @@ func TestLoadDiscoveries(t *testing.T) {
 	fakePath := paths.New("fake-path")
 
 	createTestPackageManager := func() *PackageManager {
-		packageManager := NewPackageManager(fakePath, fakePath, fakePath, fakePath, "test")
-		pack := packageManager.Packages.GetOrCreatePackage("arduino")
+		pmb := NewBuilder(fakePath, fakePath, fakePath, fakePath, "test")
+		pack := pmb.Packages.GetOrCreatePackage("arduino")
 		// ble-discovery tool
 		tool := pack.GetOrCreateTool("ble-discovery")
 		toolRelease := tool.GetOrCreateRelease(semver.ParseRelaxed("1.0.0"))
@@ -131,71 +131,71 @@ func TestLoadDiscoveries(t *testing.T) {
 		release := platform.GetOrCreateRelease(semver.MustParse("1.0.0"))
 		release.InstallDir = fakePath
 
-		return packageManager
+		return pmb.Build()
 	}
 
-	packageManager := createTestPackageManager()
-	release := packageManager.Packages["arduino"].Platforms["avr"].Releases["1.0.0"]
+	pm := createTestPackageManager()
+	release := pm.Packages["arduino"].Platforms["avr"].Releases["1.0.0"]
 	release.Properties = properties.NewFromHashmap(map[string]string{
 		"pluggable_discovery.required": "arduino:ble-discovery",
 	})
 
-	err := packageManager.LoadDiscoveries()
+	err := pm.LoadDiscoveries()
 	require.Len(t, err, 2)
 	require.Equal(t, err[0].Error(), "discovery builtin:serial-discovery not found")
 	require.Equal(t, err[1].Error(), "discovery builtin:mdns-discovery not found")
-	discoveries := packageManager.DiscoveryManager().IDs()
+	discoveries := pm.DiscoveryManager().IDs()
 	require.Len(t, discoveries, 1)
 	require.Contains(t, discoveries, "arduino:ble-discovery")
 
-	packageManager = createTestPackageManager()
-	release = packageManager.Packages["arduino"].Platforms["avr"].Releases["1.0.0"]
+	pm = createTestPackageManager()
+	release = pm.Packages["arduino"].Platforms["avr"].Releases["1.0.0"]
 	release.Properties = properties.NewFromHashmap(map[string]string{
 		"pluggable_discovery.required.0": "arduino:ble-discovery",
 		"pluggable_discovery.required.1": "arduino:serial-discovery",
 	})
 
-	err = packageManager.LoadDiscoveries()
+	err = pm.LoadDiscoveries()
 	require.Len(t, err, 2)
 	require.Equal(t, err[0].Error(), "discovery builtin:serial-discovery not found")
 	require.Equal(t, err[1].Error(), "discovery builtin:mdns-discovery not found")
-	discoveries = packageManager.DiscoveryManager().IDs()
+	discoveries = pm.DiscoveryManager().IDs()
 	require.Len(t, discoveries, 2)
 	require.Contains(t, discoveries, "arduino:ble-discovery")
 	require.Contains(t, discoveries, "arduino:serial-discovery")
 
-	packageManager = createTestPackageManager()
-	release = packageManager.Packages["arduino"].Platforms["avr"].Releases["1.0.0"]
+	pm = createTestPackageManager()
+	release = pm.Packages["arduino"].Platforms["avr"].Releases["1.0.0"]
 	release.Properties = properties.NewFromHashmap(map[string]string{
 		"pluggable_discovery.required.0":     "arduino:ble-discovery",
 		"pluggable_discovery.required.1":     "arduino:serial-discovery",
 		"pluggable_discovery.teensy.pattern": "\"{runtime.tools.teensy_ports.path}/hardware/tools/teensy_ports\" -J2",
 	})
 
-	err = packageManager.LoadDiscoveries()
+	err = pm.LoadDiscoveries()
 	require.Len(t, err, 2)
 	require.Equal(t, err[0].Error(), "discovery builtin:serial-discovery not found")
 	require.Equal(t, err[1].Error(), "discovery builtin:mdns-discovery not found")
-	discoveries = packageManager.DiscoveryManager().IDs()
+	discoveries = pm.DiscoveryManager().IDs()
 	require.Len(t, discoveries, 3)
 	require.Contains(t, discoveries, "arduino:ble-discovery")
 	require.Contains(t, discoveries, "arduino:serial-discovery")
 	require.Contains(t, discoveries, "teensy")
 
-	packageManager = createTestPackageManager()
-	release = packageManager.Packages["arduino"].Platforms["avr"].Releases["1.0.0"]
+	pm = createTestPackageManager()
+	release = pm.Packages["arduino"].Platforms["avr"].Releases["1.0.0"]
 	release.Properties = properties.NewFromHashmap(map[string]string{
 		"pluggable_discovery.required":       "arduino:some-discovery",
 		"pluggable_discovery.required.0":     "arduino:ble-discovery",
 		"pluggable_discovery.required.1":     "arduino:serial-discovery",
 		"pluggable_discovery.teensy.pattern": "\"{runtime.tools.teensy_ports.path}/hardware/tools/teensy_ports\" -J2",
 	})
 
-	err = packageManager.LoadDiscoveries()
+	err = pm.LoadDiscoveries()
 	require.Len(t, err, 2)
 	require.Equal(t, err[0].Error(), "discovery builtin:serial-discovery not found")
 	require.Equal(t, err[1].Error(), "discovery builtin:mdns-discovery not found")
-	discoveries = packageManager.DiscoveryManager().IDs()
+	discoveries = pm.DiscoveryManager().IDs()
 	require.Len(t, discoveries, 3)
 	require.Contains(t, discoveries, "arduino:ble-discovery")
 	require.Contains(t, discoveries, "arduino:serial-discovery")

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

@@ -50,20 +50,62 @@ type PackageManager struct {
 	userAgent              string
 }
 
+// Builder is used to create a new PackageManager. The builder
+// has methods to load patforms and tools to actually build the PackageManager.
+// Once the PackageManager is built, it cannot be changed anymore.
+type Builder struct {
+	*PackageManager
+}
+
 var tr = i18n.Tr
 
-// NewPackageManager returns a new instance of the PackageManager
-func NewPackageManager(indexDir, packagesDir, downloadDir, tempDir *paths.Path, userAgent string) *PackageManager {
-	return &PackageManager{
-		log:                    logrus.StandardLogger(),
-		Packages:               cores.NewPackages(),
-		IndexDir:               indexDir,
-		PackagesDir:            packagesDir,
-		DownloadDir:            downloadDir,
-		tempDir:                tempDir,
-		CustomGlobalProperties: properties.NewMap(),
-		discoveryManager:       discoverymanager.New(),
-		userAgent:              userAgent,
+// NewBuilder returns a new Builder
+func NewBuilder(indexDir, packagesDir, downloadDir, tempDir *paths.Path, userAgent string) *Builder {
+	return &Builder{
+		PackageManager: &PackageManager{
+			log:                    logrus.StandardLogger(),
+			Packages:               cores.NewPackages(),
+			IndexDir:               indexDir,
+			PackagesDir:            packagesDir,
+			DownloadDir:            downloadDir,
+			tempDir:                tempDir,
+			CustomGlobalProperties: properties.NewMap(),
+			discoveryManager:       discoverymanager.New(),
+			userAgent:              userAgent,
+		},
+	}
+}
+
+// BuildIntoExistingPackageManager will overwrite the give PackageManager instead
+// of building a new one.
+func (pmb *Builder) BuildIntoExistingPackageManager(old *PackageManager) {
+	old.log = pmb.log
+	old.Packages = pmb.Packages
+	old.IndexDir = pmb.IndexDir
+	old.PackagesDir = pmb.PackagesDir
+	old.DownloadDir = pmb.DownloadDir
+	old.tempDir = pmb.tempDir
+	old.CustomGlobalProperties = pmb.CustomGlobalProperties
+	old.profile = pmb.profile
+	old.discoveryManager = pmb.discoveryManager
+	old.userAgent = pmb.userAgent
+}
+
+// Build builds a new PackageManager.
+func (pmb *Builder) Build() *PackageManager {
+	res := &PackageManager{}
+	pmb.BuildIntoExistingPackageManager(res)
+	return res
+}
+
+// NewBuilder creates a Builder with the same configuration
+// of this PackageManager. A "commit" function callback is returned: calling
+// this function will make the builder write the new configuration into this
+// PackageManager.
+func (pm *PackageManager) NewBuilder() (*Builder, func()) {
+	pmb := NewBuilder(pm.IndexDir, pm.PackagesDir, pm.DownloadDir, pm.tempDir, pm.userAgent)
+	return pmb, func() {
+		pmb.BuildIntoExistingPackageManager(pm)
 	}
 }
 
@@ -83,13 +125,6 @@ func (pm *PackageManager) GetEnvVarsForSpawnedProcess() []string {
 	}
 }
 
-// Clear resets the PackageManager to its initial state
-func (pm *PackageManager) Clear() {
-	pm.Packages = cores.NewPackages()
-	pm.CustomGlobalProperties = properties.NewMap()
-	pm.discoveryManager.Clear()
-}
-
 // DiscoveryManager returns the DiscoveryManager in use by this PackageManager
 func (pm *PackageManager) DiscoveryManager() *discoverymanager.DiscoveryManager {
 	return pm.discoveryManager
@@ -243,12 +278,12 @@ func (pm *PackageManager) ResolveFQBN(fqbn *cores.FQBN) (
 }
 
 // LoadPackageIndex loads a package index by looking up the local cached file from the specified URL
-func (pm *PackageManager) LoadPackageIndex(URL *url.URL) error {
+func (pmb *Builder) LoadPackageIndex(URL *url.URL) error {
 	indexFileName := path.Base(URL.Path)
 	if strings.HasSuffix(indexFileName, ".tar.bz2") {
 		indexFileName = strings.TrimSuffix(indexFileName, ".tar.bz2") + ".json"
 	}
-	indexPath := pm.IndexDir.Join(indexFileName)
+	indexPath := pmb.IndexDir.Join(indexFileName)
 	index, err := packageindex.LoadIndex(indexPath)
 	if err != nil {
 		return fmt.Errorf(tr("loading json index file %[1]s: %[2]s"), indexPath, err)
@@ -258,18 +293,18 @@ func (pm *PackageManager) LoadPackageIndex(URL *url.URL) error {
 		p.URL = URL.String()
 	}
 
-	index.MergeIntoPackages(pm.Packages)
+	index.MergeIntoPackages(pmb.Packages)
 	return nil
 }
 
 // LoadPackageIndexFromFile load a package index from the specified file
-func (pm *PackageManager) LoadPackageIndexFromFile(indexPath *paths.Path) (*packageindex.Index, error) {
+func (pmb *Builder) LoadPackageIndexFromFile(indexPath *paths.Path) (*packageindex.Index, error) {
 	index, err := packageindex.LoadIndex(indexPath)
 	if err != nil {
 		return nil, fmt.Errorf(tr("loading json index file %[1]s: %[2]s"), indexPath, err)
 	}
 
-	index.MergeIntoPackages(pm.Packages)
+	index.MergeIntoPackages(pmb.Packages)
 	return index, nil
 }