Started refactoring/rationalization of 'core' commands

Author: cmaglie

arduino/cores/packagemanager/download.go

@@ -19,10 +19,8 @@ package packagemanager
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/arduino/arduino-cli/arduino/cores"
-	"github.com/arduino/arduino-cli/common/formatter/output"
 	"github.com/cavaliercoder/grab"
 	"go.bug.st/relaxed-semver"
 )
@@ -65,58 +63,37 @@ func (pm *PackageManager) FindPlatformRelease(ref *PlatformReference) *cores.Pla
 	return platformRelease
 }
 
-// FindItemsToDownload takes a set of PlatformReference and returns a set of items to download and
+// FindPlatformReleaseDependencies takes a PlatformReference and returns a set of items to download and
 // a set of outputs for non existing platforms.
-func (pm *PackageManager) FindItemsToDownload(items []PlatformReference) (
-	[]*cores.PlatformRelease, []*cores.ToolRelease, error) {
-
-	retPlatforms := []*cores.PlatformRelease{}
-	retTools := []*cores.ToolRelease{}
-	added := map[string]bool{}
-
-	for _, item := range items {
-		targetPackage, exists := pm.packages.Packages[item.Package]
-		if !exists {
-			return nil, nil, fmt.Errorf("package %s not found", item.Package)
-		}
-		platform, exists := targetPackage.Platforms[item.PlatformArchitecture]
-		if !exists {
-			return nil, nil, fmt.Errorf("platform %s not found in package %s", item.PlatformArchitecture, targetPackage.String())
-		}
+func (pm *PackageManager) FindPlatformReleaseDependencies(item *PlatformReference) (*cores.PlatformRelease, []*cores.ToolRelease, error) {
+	targetPackage, exists := pm.packages.Packages[item.Package]
+	if !exists {
+		return nil, nil, fmt.Errorf("package %s not found", item.Package)
+	}
+	platform, exists := targetPackage.Platforms[item.PlatformArchitecture]
+	if !exists {
+		return nil, nil, fmt.Errorf("platform %s not found in package %s", item.PlatformArchitecture, targetPackage.String())
+	}
 
-		if added[platform.String()] {
-			continue
+	var release *cores.PlatformRelease
+	if item.PlatformVersion != nil {
+		release = platform.GetRelease(item.PlatformVersion)
+		if release == nil {
+			return nil, nil, fmt.Errorf("required version %s not found for platform %s", item.PlatformVersion, platform.String())
 		}
-		added[platform.String()] = true
-
-		var release *cores.PlatformRelease
-		if item.PlatformVersion != nil {
-			release = platform.GetRelease(item.PlatformVersion)
-			if release == nil {
-				return nil, nil, fmt.Errorf("required version %s not found for platform %s", item.PlatformVersion, platform.String())
-			}
-		} else {
-			release = platform.GetLatestRelease()
-			if release == nil {
-				return nil, nil, fmt.Errorf("platform %s has no available releases", platform.String())
-			}
+	} else {
+		release = platform.GetLatestRelease()
+		if release == nil {
+			return nil, nil, fmt.Errorf("platform %s has no available releases", platform.String())
 		}
-		retPlatforms = append(retPlatforms, release)
+	}
 
-		// replaces "latest" with latest version too
-		toolDeps, err := pm.packages.GetDepsOfPlatformRelease(release)
-		if err != nil {
-			return nil, nil, fmt.Errorf("getting tool dependencies for platform %s: %s", release.String(), err)
-		}
-		for _, tool := range toolDeps {
-			if added[tool.String()] {
-				continue
-			}
-			added[tool.String()] = true
-			retTools = append(retTools, tool)
-		}
+	// replaces "latest" with latest version too
+	toolDeps, err := pm.packages.GetDepsOfPlatformRelease(release)
+	if err != nil {
+		return nil, nil, fmt.Errorf("getting tool dependencies for platform %s: %s", release.String(), err)
 	}
-	return retPlatforms, retTools, nil
+	return release, toolDeps, nil
 }
 
 // DownloadToolRelease downloads a ToolRelease. If the tool is already downloaded a nil Response
@@ -134,79 +111,3 @@ func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease) (*grab.Re
 func (pm *PackageManager) DownloadPlatformRelease(platform *cores.PlatformRelease) (*grab.Response, error) {
 	return platform.Resource.Download(pm.DownloadDir)
 }
-
-// FIXME: Make more generic and decouple the error print logic (that list should not exists;
-// rather a failure @ the first package)
-
-func (pm *PackageManager) InstallToolReleases(toolReleases []*cores.ToolRelease,
-	result *output.CoreProcessResults) error {
-
-	for _, toolRelease := range toolReleases {
-		pm.Log.WithField("Package", toolRelease.Tool.Package.Name).
-			WithField("Name", toolRelease.Tool.Name).
-			WithField("Version", toolRelease.Version).
-			Info("Installing tool")
-
-		err := pm.InstallTool(toolRelease)
-		var processResult output.ProcessResult
-		if err != nil {
-			if os.IsExist(err) {
-				pm.Log.WithError(err).Warnf("Cannot install tool `%s`, it is already installed", toolRelease.Tool.Name)
-				processResult = output.ProcessResult{
-					Status: "Already Installed",
-				}
-			} else {
-				pm.Log.WithError(err).Warnf("Cannot install tool `%s`", toolRelease.Tool.Name)
-				processResult = output.ProcessResult{
-					Error: err.Error(),
-				}
-			}
-		} else {
-			pm.Log.Info("Adding installed tool to final result")
-			processResult = output.ProcessResult{
-				Status: "Installed",
-			}
-		}
-		name := toolRelease.String()
-		processResult.ItemName = name
-		result.Tools[name] = processResult
-	}
-	return nil
-}
-
-func (pm *PackageManager) InstallPlatformReleases(platformReleases []*cores.PlatformRelease,
-	outputResults *output.CoreProcessResults) error {
-
-	for _, platformRelease := range platformReleases {
-		pm.Log.WithField("Package", platformRelease.Platform.Package.Name).
-			WithField("Name", platformRelease.Platform.Name).
-			WithField("Version", platformRelease.Version).
-			Info("Installing platform")
-
-		err := pm.InstallPlatform(platformRelease)
-		var result output.ProcessResult
-		if err != nil {
-			if os.IsExist(err) {
-				pm.Log.WithError(err).Warnf("Cannot install platform `%s`, it is already installed", platformRelease.Platform.Name)
-				result = output.ProcessResult{
-					Status: "Already Installed",
-				}
-			} else {
-				pm.Log.WithError(err).Warnf("Cannot install platform `%s`", platformRelease.Platform.Name)
-				result = output.ProcessResult{
-					Error: err.Error(),
-				}
-			}
-		} else {
-			pm.Log.Info("Adding installed platform to final result")
-
-			result = output.ProcessResult{
-				Status: "Installed",
-			}
-		}
-		name := platformRelease.String()
-		result.ItemName = name
-		outputResults.Cores[name] = result
-	}
-	return nil
-}

commands/commands_test.go

@@ -312,7 +312,7 @@ func detectLatestAVRCore(t *testing.T) string {
 	return latest.String()
 }
 
-func TestCoreDownload(t *testing.T) {
+func TestCoreCommands(t *testing.T) {
 	defer makeTempDataDir(t)()
 	defer makeTempSketchbookDir(t)()
 
@@ -358,7 +358,7 @@ func TestCoreDownload(t *testing.T) {
 	// Install avr
 	exitCode, d = executeWithArgs(t, "core", "install", "arduino:avr")
 	require.Zero(t, exitCode, "exit code")
-	require.Contains(t, string(d), AVR+" - Installed")
+	require.Contains(t, string(d), AVR+" installed")
 
 	exitCode, d = executeWithArgs(t, "core", "list")
 	require.Zero(t, exitCode, "exit code")

commands/compile/compile.go

@@ -29,8 +29,8 @@ import (
 	"github.com/arduino/arduino-cli/arduino/cores"
 	"github.com/arduino/arduino-cli/arduino/cores/packagemanager"
 	"github.com/arduino/arduino-cli/commands"
+	"github.com/arduino/arduino-cli/commands/core"
 	"github.com/arduino/arduino-cli/common/formatter"
-	"github.com/arduino/arduino-cli/common/formatter/output"
 	"github.com/arduino/arduino-cli/configs"
 	"github.com/arduino/go-paths-helper"
 	properties "github.com/arduino/go-properties-map"
@@ -108,24 +108,9 @@ func run(cmd *cobra.Command, args []string) {
 	loadBuiltinCtagsMetadata(pm)
 	ctags, err := getBuiltinCtagsTool(pm)
 	if !ctags.IsInstalled() {
-		formatter.Print("Downloading missing tool: " + ctags.String())
-		resp, err := pm.DownloadToolRelease(ctags)
-		if err != nil {
-			formatter.PrintError(err, "Error downloading ctags")
-			os.Exit(commands.ErrNetwork)
-		}
-		formatter.DownloadProgressBar(resp, ctags.String())
-		if resp.Err() != nil {
-			formatter.PrintError(resp.Err(), "Error downloading ctags")
-			os.Exit(commands.ErrNetwork)
-		}
-		formatter.Print("Installing " + ctags.String())
-		res := &output.CoreProcessResults{Tools: map[string]output.ProcessResult{}}
-		if err := pm.InstallToolReleases([]*cores.ToolRelease{ctags}, res); err != nil {
-			formatter.PrintError(err, "Error installing ctags")
-			formatter.PrintErrorMessage("Missing ctags tool.")
-			os.Exit(commands.ErrCoreConfig)
-		}
+		formatter.Print("Downloading and installing missing tool: " + ctags.String())
+		core.DownloadToolRelease(pm, ctags)
+		core.InstallToolRelease(pm, ctags)
 
 		if err := pm.LoadHardware(commands.Config); err != nil {
 			formatter.PrintError(err, "Could not load hardware packages.")

commands/core/args.go

@@ -29,8 +29,8 @@ import (
 )
 
 // parsePlatformReferenceArgs parses a sequence of "packager:arch@version" tokens and returns a platformReference slice.
-func parsePlatformReferenceArgs(args []string) []packagemanager.PlatformReference {
-	ret := []packagemanager.PlatformReference{}
+func parsePlatformReferenceArgs(args []string) []*packagemanager.PlatformReference {
+	ret := []*packagemanager.PlatformReference{}
 
 	for _, arg := range args {
 		var version *semver.Version
@@ -48,7 +48,7 @@ func parsePlatformReferenceArgs(args []string) []packagemanager.PlatformReferenc
 			formatter.PrintErrorMessage(fmt.Sprintf("'%s' is an invalid item (does not match the syntax 'PACKAGER:ARCH[@VERSION]')", arg))
 			os.Exit(commands.ErrBadArgument)
 		}
-		ret = append(ret, packagemanager.PlatformReference{
+		ret = append(ret, &packagemanager.PlatformReference{
 			Package:              split[0],
 			PlatformArchitecture: split[1],
 			PlatformVersion:      version,

commands/core/download.go

@@ -20,6 +20,7 @@ package core
 import (
 	"os"
 
+	"github.com/arduino/arduino-cli/arduino/cores"
 	"github.com/arduino/arduino-cli/arduino/cores/packagemanager"
 	"github.com/arduino/arduino-cli/commands"
 	"github.com/arduino/arduino-cli/common/formatter"
@@ -45,13 +46,15 @@ func initDownloadCommand() *cobra.Command {
 func runDownloadCommand(cmd *cobra.Command, args []string) {
 	logrus.Info("Executing `arduino core download`")
 
-	pm := commands.InitPackageManager()
 	platformsRefs := parsePlatformReferenceArgs(args)
-	downloadPlatforms(pm, platformsRefs)
+	pm := commands.InitPackageManager()
+	for _, platformRef := range platformsRefs {
+		downloadPlatformByRef(pm, platformRef)
+	}
 }
 
-func downloadPlatforms(pm *packagemanager.PackageManager, platformsRefs []packagemanager.PlatformReference) {
-	platforms, tools, err := pm.FindItemsToDownload(platformsRefs)
+func downloadPlatformByRef(pm *packagemanager.PackageManager, platformsRef *packagemanager.PlatformReference) {
+	platform, tools, err := pm.FindPlatformReleaseDependencies(platformsRef)
 	if err != nil {
 		formatter.PrintError(err, "Could not determine platform dependencies")
 		os.Exit(commands.ErrBadCall)
@@ -66,22 +69,24 @@ func downloadPlatforms(pm *packagemanager.PackageManager, platformsRefs []packag
 	}
 
 	// Download tools
-	formatter.Print("Downloading tools...")
 	for _, tool := range tools {
-		resp, err := pm.DownloadToolRelease(tool)
-		download(resp, err, tool.String())
+		DownloadToolRelease(pm, tool)
 	}
 
 	// Download cores
-	formatter.Print("Downloading cores...")
-	for _, platform := range platforms {
-		resp, err := pm.DownloadPlatformRelease(platform)
-		download(resp, err, platform.String())
-	}
+	resp, err := pm.DownloadPlatformRelease(platform)
+	download(resp, err, platform.String())
 
 	logrus.Info("Done")
 }
 
+// DownloadToolRelease downloads a ToolRelease
+func DownloadToolRelease(pm *packagemanager.PackageManager, toolRelease *cores.ToolRelease) {
+	formatter.Print("Downloading " + toolRelease.String() + "...")
+	resp, err := pm.DownloadToolRelease(toolRelease)
+	download(resp, err, toolRelease.String())
+}
+
 func download(resp *grab.Response, err error, label string) {
 	if err != nil {
 		formatter.PrintError(err, "Error downloading "+label)