Factored out output package import from release package (WIP 1/2)

There is still need to factor a function to convert a
result.DownloadResult to output.ProcessResult.

Author: cmaglie

commands/core/download.go

@@ -93,7 +93,25 @@ func downloadToolsArchives(tools []*cores.ToolRelease, results *output.CoreProce
 		downloads[tool.Tool.Name+"@"+tool.Version] = tool.GetCompatibleFlavour()
 	}
 	logrus.Info("Downloading tools")
-	releases.ParallelDownload(downloads, false, "Downloaded", &results.Tools, commands.GenerateDownloadProgressFormatter())
+	downloadRes := releases.ParallelDownload(downloads, false, commands.GenerateDownloadProgressFormatter())
+
+	for name, res := range downloadRes {
+		path, err := downloads[name].ArchivePath()
+		if err != nil {
+			// FIXME: do something!!
+			logrus.Error("Could not determine library archive path:", err)
+		}
+		status := ""
+		if res.Error == nil {
+			status = "Downloaded"
+		}
+		results.Tools = append(results.Tools, output.ProcessResult{
+			ItemName: name,
+			Path:     path,
+			Error:    res.Error.Error(),
+			Status:   status,
+		})
+	}
 }
 
 func downloadPlatformArchives(platforms []*cores.PlatformRelease, results *output.CoreProcessResults) {
@@ -102,5 +120,23 @@ func downloadPlatformArchives(platforms []*cores.PlatformRelease, results *outpu
 		downloads[platform.Platform.Package.Name+":"+platform.Platform.Name+"@"+platform.Version] = platform.Resource
 	}
 	logrus.Info("Downloading cores")
-	releases.ParallelDownload(downloads, false, "Downloaded", &results.Cores, commands.GenerateDownloadProgressFormatter())
+	downloadRes := releases.ParallelDownload(downloads, false, commands.GenerateDownloadProgressFormatter())
+
+	for name, res := range downloadRes {
+		path, err := downloads[name].ArchivePath()
+		if err != nil {
+			// FIXME: do something!!
+			logrus.Error("Could not determine library archive path:", err)
+		}
+		status := ""
+		if res.Error == nil {
+			status = "Downloaded"
+		}
+		results.Cores = append(results.Cores, output.ProcessResult{
+			ItemName: name,
+			Path:     path,
+			Error:    res.Error.Error(),
+			Status:   status,
+		})
+	}
 }

commands/lib/download.go

@@ -74,7 +74,25 @@ func runDownloadCommand(cmd *cobra.Command, args []string) {
 	}
 
 	logrus.Info("Downloading")
-	releases.ParallelDownload(libsToDownload, false, "Downloaded", &outputResults.Libraries, commands.GenerateDownloadProgressFormatter())
+	downloadRes := releases.ParallelDownload(libsToDownload, false, commands.GenerateDownloadProgressFormatter())
+	// "Downloaded", &outputResults.Libraries
+	for name, res := range downloadRes {
+		path, err := libsToDownload[name].ArchivePath()
+		if err != nil {
+			// FIXME: do something!!
+			logrus.Error("Could not determine library archive path:", err)
+		}
+		status := ""
+		if res.Error == nil {
+			status = "Installed"
+		}
+		outputResults.Libraries = append(outputResults.Libraries, output.ProcessResult{
+			ItemName: name,
+			Path:     path,
+			Error:    res.Error.Error(),
+			Status:   status,
+		})
+	}
 	logrus.Info("Download finished")
 	formatter.Print(outputResults)
 	logrus.Info("Done")

commands/lib/install.go

@@ -73,7 +73,25 @@ func runInstallCommand(cmd *cobra.Command, args []string) {
 	}
 
 	logrus.Info("Downloading")
-	releases.ParallelDownload(libsToDownload, false, "Installed", &outputResults.Libraries, commands.GenerateDownloadProgressFormatter())
+	downloadRes := releases.ParallelDownload(libsToDownload, false, commands.GenerateDownloadProgressFormatter())
+	// "Installed", &outputResults.Libraries,
+	for name, res := range downloadRes {
+		path, err := libsToDownload[name].ArchivePath()
+		if err != nil {
+			// FIXME: do something!!
+			logrus.Error("Could not determine library archive path:", err)
+		}
+		status := ""
+		if res.Error == nil {
+			status = "Installed"
+		}
+		outputResults.Libraries = append(outputResults.Libraries, output.ProcessResult{
+			ItemName: name,
+			Path:     path,
+			Error:    res.Error.Error(),
+			Status:   status,
+		})
+	}
 	logrus.Info("Download finished")
 
 	logrus.Info("Installing")

common/releases/helpers.go

@@ -35,7 +35,6 @@ import (
 	"os"
 
 	"github.com/bcmi-labs/arduino-cli/common"
-	"github.com/bcmi-labs/arduino-cli/common/formatter/output"
 	"github.com/bcmi-labs/arduino-cli/task"
 	"github.com/sirupsen/logrus"
 )
@@ -118,28 +117,25 @@ type ParallelDownloadProgressHandler interface {
 //   OkStatus is used to tell the overlying process result ("Downloaded", "Installed", etc...)
 //	 An optional progressHandler can be passed in order to be notified of the status of the download.
 //   DOES NOT RETURN since will append results to the provided refResults; use refResults.Results() to get them.
-func ParallelDownload(items map[string]*DownloadResource, forced bool, OkStatus string, refResults *[]output.ProcessResult,
-	progressHandler ParallelDownloadProgressHandler) {
+func ParallelDownload(items map[string]*DownloadResource, forced bool, progressHandler ParallelDownloadProgressHandler) map[string]*DownloadResult {
 
 	// TODO (l.biava): Future improvements envision this utility as an object (say a Builder)
 	// to simplify the passing of all those parameters, the progress handling closure, the outputResults
 	// internally populated, etc.
 
-	itemC := len(items)
-	tasks := make(map[string]task.Wrapper, itemC)
-	paths := make(map[string]string, itemC)
+	tasks := map[string]task.Wrapper{}
+	res := map[string]*DownloadResult{}
 
-	logrus.Info(fmt.Sprintf("Initiating parallel download of %d tasks", itemC))
+	logrus.Info(fmt.Sprintf("Initiating parallel download of %d resources", len(items)))
 
 	for itemName, item := range items {
 		cached := IsCached(item)
 		releaseNotNil := item != nil
 		if forced || releaseNotNil && (!cached || checkLocalArchive(item) != nil) {
 			// Notify the progress handler of the new task
 			if progressHandler != nil {
-				progressHandler.OnNewDownloadTask(itemName, int64(item.Size))
+				progressHandler.OnNewDownloadTask(itemName, item.Size)
 			}
-			paths[itemName], _ = item.ArchivePath() // if the release exists the archivepath always exists
 
 			// Forward the per-file progress handler, if available
 			// WARNING: This is using a closure on itemName!
@@ -154,13 +150,8 @@ func ParallelDownload(items map[string]*DownloadResource, forced bool, OkStatus
 
 			tasks[itemName] = downloadTask(item, getProgressHandler(itemName))
 		} else if !forced && releaseNotNil && cached {
-			// Consider OK
-			path, _ := item.ArchivePath()
-			*refResults = append(*refResults, output.ProcessResult{
-				ItemName: itemName,
-				Status:   OkStatus,
-				Path:     path,
-			})
+			// Item already downloaded
+			res[itemName] = &DownloadResult{AlreadyDownloaded: true}
 		}
 	}
 
@@ -177,19 +168,14 @@ func ParallelDownload(items map[string]*DownloadResource, forced bool, OkStatus
 
 		for name, result := range results {
 			if result.Error != nil {
-				*refResults = append(*refResults, output.ProcessResult{
-					ItemName: name,
-					Error:    result.Error.Error(),
-				})
+				res[name] = &DownloadResult{Error: result.Error}
 			} else {
-				*refResults = append(*refResults, output.ProcessResult{
-					ItemName: name,
-					Status:   OkStatus,
-					Path:     paths[name],
-				})
+				res[name] = &DownloadResult{}
 			}
 		}
 	}
+
+	return res
 }
 
 // OpenLocalArchiveForDownload open local archive if present

common/releases/structs.go

@@ -52,3 +52,14 @@ func (r *DownloadResource) ArchivePath() (string, error) {
 	}
 	return filepath.Join(staging, r.ArchiveFileName), nil
 }
+
+// DownloadResult contains the result of a download
+type DownloadResult struct {
+	// Error is nil if the download is successful otherwise
+	// it contains the reason for the download failure
+	Error error
+
+	// AlreadyDownloaded is true if a cache file is found
+	// and, consequently, the download has not been executed
+	AlreadyDownloaded bool
+}