From c7163b776f756a0ca25e9bebf872eeb1e7bf404a Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 8 Aug 2022 19:06:29 +0200 Subject: [PATCH 01/16] [skip-changelog] [breaking] Downloaders/Installers refactor (#1809) * Made PackageManager.TempDir private * Merged InstallToolRelease function into the proper packagemanager method * Moved uninstallToolRelease into the proper packagamanger method * Moved uninstallPlatformRelease into the proper packagamanger method * Moved downloadToolRelease into the proper packagamanger method * Merged downloadTool with the proper packagamanger method * Merged downloadPlatform with the proper packagamanger method * Moved installPlatform into a packagamanger method * Moved upgradePlatform into a packagamanger method * Made PackageManager.Log field private * Removed the massive code duplication in the 'upgrade' command * Removed the massive code duplication in the 'outdated' command * Updated docs * Update arduino/cores/packagemanager/install_uninstall.go Co-authored-by: per1234 * Update arduino/cores/packagemanager/install_uninstall.go Co-authored-by: per1234 Co-authored-by: per1234 --- arduino/cores/packagemanager/download.go | 16 +- .../cores/packagemanager/install_uninstall.go | 222 ++++++++++++- arduino/cores/packagemanager/loader.go | 38 +-- .../cores/packagemanager/package_manager.go | 22 +- arduino/cores/packagemanager/profiles.go | 4 +- cli/outdated/outdated.go | 4 +- cli/update/update.go | 3 +- cli/upgrade/upgrade.go | 4 +- commands/bundled_tools.go | 56 ---- commands/core/download.go | 31 +- commands/core/install.go | 115 +------ commands/core/uninstall.go | 38 +-- commands/core/upgrade.go | 34 +- commands/daemon/daemon.go | 6 +- commands/instances.go | 307 +----------------- commands/outdated/outdated.go | 48 +++ commands/upgrade/upgrade.go | 59 ++++ docs/UPGRADING.md | 50 +++ 18 files changed, 432 insertions(+), 625 deletions(-) delete mode 100644 commands/bundled_tools.go create mode 100644 commands/outdated/outdated.go create mode 100644 commands/upgrade/upgrade.go diff --git a/arduino/cores/packagemanager/download.go b/arduino/cores/packagemanager/download.go index 90ace62b9b6..6dfda64fbe6 100644 --- a/arduino/cores/packagemanager/download.go +++ b/arduino/cores/packagemanager/download.go @@ -21,6 +21,7 @@ import ( "github.com/arduino/arduino-cli/arduino" "github.com/arduino/arduino-cli/arduino/cores" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" + "github.com/pkg/errors" "go.bug.st/downloader/v2" semver "go.bug.st/relaxed-semver" ) @@ -118,13 +119,20 @@ func (pm *PackageManager) FindPlatformReleaseDependencies(item *PlatformReferenc } // DownloadToolRelease downloads a ToolRelease. If the tool is already downloaded a nil Downloader -// is returned. -func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *downloader.Config, label string, progressCB rpc.DownloadProgressCB) error { +// is returned. Uses the given downloader configuration for download, or the default config if nil. +func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *downloader.Config, progressCB rpc.DownloadProgressCB) error { resource := tool.GetCompatibleFlavour() if resource == nil { - return fmt.Errorf(tr("tool not available for your OS")) + return &arduino.FailedDownloadError{ + Message: tr("Error downloading tool %s", tool), + Cause: errors.New(tr("no versions available for the current OS", tool))} } - return resource.Download(pm.DownloadDir, config, label, progressCB) + if err := resource.Download(pm.DownloadDir, config, tool.String(), progressCB); err != nil { + return &arduino.FailedDownloadError{ + Message: tr("Error downloading tool %s", tool), + Cause: err} + } + return nil } // DownloadPlatformRelease downloads a PlatformRelease. If the platform is already downloaded a diff --git a/arduino/cores/packagemanager/install_uninstall.go b/arduino/cores/packagemanager/install_uninstall.go index 17ed7ca7f98..001c4885700 100644 --- a/arduino/cores/packagemanager/install_uninstall.go +++ b/arduino/cores/packagemanager/install_uninstall.go @@ -20,13 +20,170 @@ import ( "fmt" "runtime" + "github.com/arduino/arduino-cli/arduino" "github.com/arduino/arduino-cli/arduino/cores" "github.com/arduino/arduino-cli/arduino/cores/packageindex" "github.com/arduino/arduino-cli/executils" + rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/arduino/go-paths-helper" "github.com/pkg/errors" ) +// DownloadAndInstallPlatformUpgrades runs a full installation process to upgrade the given platform. +// This method takes care of downloading missing archives, upgrading platforms and tools, and +// removing the previously installed platform/tools that are no longer needed after the upgrade. +func (pm *PackageManager) DownloadAndInstallPlatformUpgrades( + platformRef *PlatformReference, + downloadCB rpc.DownloadProgressCB, + taskCB rpc.TaskProgressCB, + skipPostInstall bool, +) error { + if platformRef.PlatformVersion != nil { + return &arduino.InvalidArgumentError{Message: tr("Upgrade doesn't accept parameters with version")} + } + + // Search the latest version for all specified platforms + platform := pm.FindPlatform(platformRef) + if platform == nil { + return &arduino.PlatformNotFoundError{Platform: platformRef.String()} + } + installed := pm.GetInstalledPlatformRelease(platform) + if installed == nil { + return &arduino.PlatformNotFoundError{Platform: platformRef.String()} + } + latest := platform.GetLatestRelease() + if !latest.Version.GreaterThan(installed.Version) { + return &arduino.PlatformAlreadyAtTheLatestVersionError{} + } + platformRef.PlatformVersion = latest.Version + + platformRelease, tools, err := pm.FindPlatformReleaseDependencies(platformRef) + if err != nil { + return &arduino.PlatformNotFoundError{Platform: platformRef.String()} + } + if err := pm.DownloadAndInstallPlatformAndTools(platformRelease, tools, downloadCB, taskCB, skipPostInstall); err != nil { + return err + } + + return nil +} + +// DownloadAndInstallPlatformAndTools runs a full installation process for the given platform and tools. +// This method takes care of downloading missing archives, installing/upgrading platforms and tools, and +// removing the previously installed platform/tools that are no longer needed after the upgrade. +func (pm *PackageManager) DownloadAndInstallPlatformAndTools( + platformRelease *cores.PlatformRelease, requiredTools []*cores.ToolRelease, + downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB, + skipPostInstall bool) error { + log := pm.log.WithField("platform", platformRelease) + + // Prerequisite checks before install + toolsToInstall := []*cores.ToolRelease{} + for _, tool := range requiredTools { + if tool.IsInstalled() { + log.WithField("tool", tool).Warn("Tool already installed") + taskCB(&rpc.TaskProgress{Name: tr("Tool %s already installed", tool), Completed: true}) + } else { + toolsToInstall = append(toolsToInstall, tool) + } + } + + // Package download + taskCB(&rpc.TaskProgress{Name: tr("Downloading packages")}) + for _, tool := range toolsToInstall { + if err := pm.DownloadToolRelease(tool, nil, downloadCB); err != nil { + return err + } + } + if err := pm.DownloadPlatformRelease(platformRelease, nil, downloadCB); err != nil { + return err + } + taskCB(&rpc.TaskProgress{Completed: true}) + + // Install tools first + for _, tool := range toolsToInstall { + if err := pm.InstallTool(tool, taskCB); err != nil { + return err + } + } + + installed := pm.GetInstalledPlatformRelease(platformRelease.Platform) + installedTools := []*cores.ToolRelease{} + if installed == nil { + // No version of this platform is installed + log.Info("Installing platform") + taskCB(&rpc.TaskProgress{Name: tr("Installing platform %s", platformRelease)}) + } else { + // A platform with a different version is already installed + log.Info("Replacing platform " + installed.String()) + taskCB(&rpc.TaskProgress{Name: tr("Replacing platform %[1]s with %[2]s", installed, platformRelease)}) + platformRef := &PlatformReference{ + Package: platformRelease.Platform.Package.Name, + PlatformArchitecture: platformRelease.Platform.Architecture, + PlatformVersion: installed.Version, + } + + // Get a list of tools used by the currently installed platform version. + // This must be done so tools used by the currently installed version are + // removed if not used also by the newly installed version. + var err error + _, installedTools, err = pm.FindPlatformReleaseDependencies(platformRef) + if err != nil { + return &arduino.NotFoundError{Message: tr("Can't find dependencies for platform %s", platformRef), Cause: err} + } + } + + // Install + if err := pm.InstallPlatform(platformRelease); err != nil { + log.WithError(err).Error("Cannot install platform") + return &arduino.FailedInstallError{Message: tr("Cannot install platform"), Cause: err} + } + + // If upgrading remove previous release + if installed != nil { + uninstallErr := pm.UninstallPlatform(installed, taskCB) + + // In case of error try to rollback + if uninstallErr != nil { + log.WithError(uninstallErr).Error("Error upgrading platform.") + taskCB(&rpc.TaskProgress{Message: tr("Error upgrading platform: %s", uninstallErr)}) + + // Rollback + if err := pm.UninstallPlatform(platformRelease, taskCB); err != nil { + log.WithError(err).Error("Error rolling-back changes.") + taskCB(&rpc.TaskProgress{Message: tr("Error rolling-back changes: %s", err)}) + } + + return &arduino.FailedInstallError{Message: tr("Cannot upgrade platform"), Cause: uninstallErr} + } + + // Uninstall unused tools + for _, tool := range installedTools { + taskCB(&rpc.TaskProgress{Name: tr("Uninstalling %s, tool is no more required", tool)}) + if !pm.IsToolRequired(tool) { + pm.UninstallTool(tool, taskCB) + } + } + + } + + // Perform post install + if !skipPostInstall { + log.Info("Running post_install script") + taskCB(&rpc.TaskProgress{Message: tr("Configuring platform.")}) + if err := pm.RunPostInstallScript(platformRelease); err != nil { + taskCB(&rpc.TaskProgress{Message: tr("WARNING cannot configure platform: %s", err)}) + } + } else { + log.Info("Skipping platform configuration.") + taskCB(&rpc.TaskProgress{Message: tr("Skipping platform configuration.")}) + } + + log.Info("Platform installed") + taskCB(&rpc.TaskProgress{Message: tr("Platform %s installed", platformRelease), Completed: true}) + return nil +} + // InstallPlatform installs a specific release of a platform. func (pm *PackageManager) InstallPlatform(platformRelease *cores.PlatformRelease) error { destDir := pm.PackagesDir.Join( @@ -39,7 +196,7 @@ func (pm *PackageManager) InstallPlatform(platformRelease *cores.PlatformRelease // InstallPlatformInDirectory installs a specific release of a platform in a specific directory. func (pm *PackageManager) InstallPlatformInDirectory(platformRelease *cores.PlatformRelease, destDir *paths.Path) error { - if err := platformRelease.Resource.Install(pm.DownloadDir, pm.TempDir, destDir); err != nil { + if err := platformRelease.Resource.Install(pm.DownloadDir, pm.tempDir, destDir); err != nil { return errors.Errorf(tr("installing platform %[1]s: %[2]s"), platformRelease, err) } if d, err := destDir.Abs(); err == nil { @@ -109,25 +266,51 @@ func (pm *PackageManager) IsManagedPlatformRelease(platformRelease *cores.Platfo } // UninstallPlatform remove a PlatformRelease. -func (pm *PackageManager) UninstallPlatform(platformRelease *cores.PlatformRelease) error { +func (pm *PackageManager) UninstallPlatform(platformRelease *cores.PlatformRelease, taskCB rpc.TaskProgressCB) error { + log := pm.log.WithField("platform", platformRelease) + + log.Info("Uninstalling platform") + taskCB(&rpc.TaskProgress{Name: tr("Uninstalling %s", platformRelease)}) + if platformRelease.InstallDir == nil { - return fmt.Errorf(tr("platform not installed")) + err := fmt.Errorf(tr("platform not installed")) + log.WithError(err).Error("Error uninstalling") + return &arduino.FailedUninstallError{Message: err.Error()} } // Safety measure if !pm.IsManagedPlatformRelease(platformRelease) { - return fmt.Errorf(tr("%s is not managed by package manager"), platformRelease) + err := fmt.Errorf(tr("%s is not managed by package manager"), platformRelease) + log.WithError(err).Error("Error uninstalling") + return &arduino.FailedUninstallError{Message: err.Error()} } if err := platformRelease.InstallDir.RemoveAll(); err != nil { - return fmt.Errorf(tr("removing platform files: %s"), err) + err = fmt.Errorf(tr("removing platform files: %s"), err) + log.WithError(err).Error("Error uninstalling") + return &arduino.FailedUninstallError{Message: err.Error()} } + platformRelease.InstallDir = nil + + log.Info("Platform uninstalled") + taskCB(&rpc.TaskProgress{Message: tr("Platform %s uninstalled", platformRelease), Completed: true}) return nil } // InstallTool installs a specific release of a tool. -func (pm *PackageManager) InstallTool(toolRelease *cores.ToolRelease) error { +func (pm *PackageManager) InstallTool(toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB) error { + log := pm.log.WithField("Tool", toolRelease) + + if toolRelease.IsInstalled() { + log.Warn("Tool already installed") + taskCB(&rpc.TaskProgress{Name: tr("Tool %s already installed", toolRelease), Completed: true}) + return nil + } + + log.Info("Installing tool") + taskCB(&rpc.TaskProgress{Name: tr("Installing %s", toolRelease)}) + toolResource := toolRelease.GetCompatibleFlavour() if toolResource == nil { return fmt.Errorf(tr("no compatible version of %s tools found for the current os"), toolRelease.Tool.Name) @@ -137,7 +320,15 @@ func (pm *PackageManager) InstallTool(toolRelease *cores.ToolRelease) error { "tools", toolRelease.Tool.Name, toolRelease.Version.String()) - return toolResource.Install(pm.DownloadDir, pm.TempDir, destDir) + err := toolResource.Install(pm.DownloadDir, pm.tempDir, destDir) + if err != nil { + log.WithError(err).Warn("Cannot install tool") + return &arduino.FailedInstallError{Message: tr("Cannot install tool %s", toolRelease), Cause: err} + } + log.Info("Tool installed") + taskCB(&rpc.TaskProgress{Message: tr("%s installed", toolRelease), Completed: true}) + + return nil } // IsManagedToolRelease returns true if the ToolRelease is managed by the PackageManager @@ -161,20 +352,31 @@ func (pm *PackageManager) IsManagedToolRelease(toolRelease *cores.ToolRelease) b } // UninstallTool remove a ToolRelease. -func (pm *PackageManager) UninstallTool(toolRelease *cores.ToolRelease) error { +func (pm *PackageManager) UninstallTool(toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB) error { + log := pm.log.WithField("Tool", toolRelease) + log.Info("Uninstalling tool") + if toolRelease.InstallDir == nil { return fmt.Errorf(tr("tool not installed")) } // Safety measure if !pm.IsManagedToolRelease(toolRelease) { - return fmt.Errorf(tr("tool %s is not managed by package manager"), toolRelease) + err := &arduino.FailedUninstallError{Message: tr("tool %s is not managed by package manager", toolRelease)} + log.WithError(err).Error("Error uninstalling") + return err } if err := toolRelease.InstallDir.RemoveAll(); err != nil { - return fmt.Errorf(tr("removing tool files: %s"), err) + err = &arduino.FailedUninstallError{Message: err.Error()} + log.WithError(err).Error("Error uninstalling") + return err } + toolRelease.InstallDir = nil + + log.Info("Tool uninstalled") + taskCB(&rpc.TaskProgress{Message: tr("Tool %s uninstalled", toolRelease), Completed: true}) return nil } diff --git a/arduino/cores/packagemanager/loader.go b/arduino/cores/packagemanager/loader.go index e517fcfd5af..b48fac48f1b 100644 --- a/arduino/cores/packagemanager/loader.go +++ b/arduino/cores/packagemanager/loader.go @@ -55,7 +55,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 { var merr []error - pm.Log.Infof("Loading hardware from: %s", path) + pm.log.Infof("Loading hardware from: %s", path) if err := path.ToAbs(); err != nil { return append(merr, fmt.Errorf("%s: %w", tr("finding absolute path of %s", path), err)) } @@ -76,9 +76,9 @@ func (pm *PackageManager) LoadHardwareFromDirectory(path *paths.Path) []error { // "Global" platform.txt used to overwrite all installed platforms. // For more info: https://arduino.github.io/arduino-cli/latest/platform-specification/#global-platformtxt if globalPlatformTxt := path.Join("platform.txt"); globalPlatformTxt.Exist() { - pm.Log.Infof("Loading custom platform properties: %s", globalPlatformTxt) + pm.log.Infof("Loading custom platform properties: %s", globalPlatformTxt) if p, err := properties.LoadFromPath(globalPlatformTxt); err != nil { - pm.Log.WithError(err).Errorf("Error loading properties.") + pm.log.WithError(err).Errorf("Error loading properties.") } else { pm.CustomGlobalProperties.Merge(p) } @@ -89,7 +89,7 @@ func (pm *PackageManager) LoadHardwareFromDirectory(path *paths.Path) []error { // Skip tools, they're not packages and don't contain Platforms if packager == "tools" { - pm.Log.Infof("Excluding directory: %s", packagerPath) + pm.log.Infof("Excluding directory: %s", packagerPath) continue } @@ -127,7 +127,7 @@ func (pm *PackageManager) LoadHardwareFromDirectory(path *paths.Path) []error { // - PACKAGER/tools/TOOL-NAME/TOOL-VERSION/... (ex: arduino/tools/bossac/1.7.0/...) toolsSubdirPath := packagerPath.Join("tools") if toolsSubdirPath.IsDir() { - pm.Log.Infof("Checking existence of 'tools' path: %s", toolsSubdirPath) + pm.log.Infof("Checking existence of 'tools' path: %s", toolsSubdirPath) merr = append(merr, pm.LoadToolsFromPackageDir(targetPackage, toolsSubdirPath)...) } // If the Package does not contain Platforms or Tools we remove it since does not contain anything valuable @@ -143,7 +143,7 @@ func (pm *PackageManager) LoadHardwareFromDirectory(path *paths.Path) []error { // 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 { - pm.Log.Infof("Loading package %s from: %s", targetPackage.Name, packageDir) + pm.log.Infof("Loading package %s from: %s", targetPackage.Name, packageDir) var merr []error @@ -220,11 +220,11 @@ func (pm *PackageManager) loadPlatform(targetPackage *cores.Package, architectur tmp := cores.NewPackages() index.MergeIntoPackages(tmp) if tmpPackage := tmp.GetOrCreatePackage(targetPackage.Name); tmpPackage == nil { - pm.Log.Warnf("Can't determine bundle platform version for %s", targetPackage.Name) + pm.log.Warnf("Can't determine bundle platform version for %s", targetPackage.Name) } else if tmpPlatform := tmpPackage.GetOrCreatePlatform(architecture); tmpPlatform == nil { - pm.Log.Warnf("Can't determine bundle platform version for %s:%s", targetPackage.Name, architecture) + pm.log.Warnf("Can't determine bundle platform version for %s:%s", targetPackage.Name, architecture) } else if tmpPlatformRelease := tmpPlatform.GetLatestRelease(); tmpPlatformRelease == nil { - pm.Log.Warnf("Can't determine bundle platform version for %s:%s, no valid release found", targetPackage.Name, architecture) + pm.log.Warnf("Can't determine bundle platform version for %s:%s, no valid release found", targetPackage.Name, architecture) } else { version = tmpPlatformRelease.Version } @@ -239,12 +239,12 @@ func (pm *PackageManager) loadPlatform(targetPackage *cores.Package, architectur release := platform.GetOrCreateRelease(version) release.IsIDEBundled = isIDEBundled if isIDEBundled { - pm.Log.Infof("Package is built-in") + pm.log.Infof("Package is built-in") } if err := pm.loadPlatformRelease(release, platformPath); err != nil { return fmt.Errorf("%s: %w", tr("loading platform release %s", release), err) } - pm.Log.WithField("platform", release).Infof("Loaded platform") + pm.log.WithField("platform", release).Infof("Loaded platform") } else { // case: ARCHITECTURE/VERSION/boards.txt @@ -272,7 +272,7 @@ func (pm *PackageManager) loadPlatform(targetPackage *cores.Package, architectur if err := pm.loadPlatformRelease(release, versionDir); err != nil { return fmt.Errorf("%s: %w", tr("loading platform release %s", release), err) } - pm.Log.WithField("platform", release).Infof("Loaded platform") + pm.log.WithField("platform", release).Infof("Loaded platform") } } @@ -357,7 +357,7 @@ func (pm *PackageManager) loadPlatformRelease(platform *cores.PlatformRelease, p if len(split) != 2 { return fmt.Errorf(tr("invalid pluggable monitor reference: %s"), ref) } - pm.Log.WithField("protocol", protocol).WithField("tool", ref).Info("Adding monitor tool") + pm.log.WithField("protocol", protocol).WithField("tool", ref).Info("Adding monitor tool") platform.Monitors[protocol] = &cores.MonitorDependency{ Packager: split[0], Name: split[1], @@ -367,7 +367,7 @@ func (pm *PackageManager) loadPlatformRelease(platform *cores.PlatformRelease, p // Support for pluggable monitors in debugging/development environments platform.MonitorsDevRecipes = map[string]string{} for protocol, recipe := range platform.Properties.SubTree("pluggable_monitor.pattern").AsMap() { - pm.Log.WithField("protocol", protocol).WithField("recipe", recipe).Info("Adding monitor recipe") + pm.log.WithField("protocol", protocol).WithField("recipe", recipe).Info("Adding monitor recipe") platform.MonitorsDevRecipes[protocol] = recipe } @@ -592,7 +592,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 { - pm.Log.Infof("Loading tools from dir: %s", toolsPath) + pm.log.Infof("Loading tools from dir: %s", toolsPath) var merr []error @@ -637,7 +637,7 @@ func (pm *PackageManager) loadToolReleaseFromDirectory(tool *cores.Tool, version } else { toolRelease := tool.GetOrCreateRelease(version) toolRelease.InstallDir = absToolReleasePath - pm.Log.WithField("tool", toolRelease).Infof("Loaded tool") + pm.log.WithField("tool", toolRelease).Infof("Loaded tool") return nil } } @@ -655,7 +655,7 @@ func (pm *PackageManager) LoadToolsFromBundleDirectories(dirs paths.PathList) [] // LoadToolsFromBundleDirectory FIXMEDOC func (pm *PackageManager) LoadToolsFromBundleDirectory(toolsPath *paths.Path) error { - pm.Log.Infof("Loading tools from bundle dir: %s", toolsPath) + 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 // then the all the tools are available in the same directory, mixed together, and their @@ -689,7 +689,7 @@ func (pm *PackageManager) LoadToolsFromBundleDirectory(toolsPath *paths.Path) er if builtinToolsVersionsTxtPath != "" { // If builtin_tools_versions.txt is found create tools based on the info // contained in that file - pm.Log.Infof("Found builtin_tools_versions.txt") + pm.log.Infof("Found builtin_tools_versions.txt") toolPath, err := paths.New(builtinToolsVersionsTxtPath).Parent().Abs() if err != nil { return fmt.Errorf(tr("getting parent dir of %[1]s: %[2]s"), builtinToolsVersionsTxtPath, err) @@ -708,7 +708,7 @@ func (pm *PackageManager) LoadToolsFromBundleDirectory(toolsPath *paths.Path) er version := semver.ParseRelaxed(toolVersion) release := tool.GetOrCreateRelease(version) release.InstallDir = toolPath - pm.Log.WithField("tool", release).Infof("Loaded tool") + pm.log.WithField("tool", release).Infof("Loaded tool") } } } else { diff --git a/arduino/cores/packagemanager/package_manager.go b/arduino/cores/packagemanager/package_manager.go index dd98284b17f..401c2407dfb 100644 --- a/arduino/cores/packagemanager/package_manager.go +++ b/arduino/cores/packagemanager/package_manager.go @@ -38,12 +38,12 @@ import ( // The manager also keeps track of the status of the Packages (their Platform Releases, actually) // installed in the system. type PackageManager struct { - Log logrus.FieldLogger + log logrus.FieldLogger Packages cores.Packages IndexDir *paths.Path PackagesDir *paths.Path DownloadDir *paths.Path - TempDir *paths.Path + tempDir *paths.Path CustomGlobalProperties *properties.Map profile *sketch.Profile discoveryManager *discoverymanager.DiscoveryManager @@ -55,12 +55,12 @@ 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(), + log: logrus.StandardLogger(), Packages: cores.NewPackages(), IndexDir: indexDir, PackagesDir: packagesDir, DownloadDir: downloadDir, - TempDir: tempDir, + tempDir: tempDir, CustomGlobalProperties: properties.NewMap(), discoveryManager: discoverymanager.New(), userAgent: userAgent, @@ -386,7 +386,7 @@ func (pm *PackageManager) GetInstalledPlatformRelease(platform *cores.Platform) } debug := func(msg string, pl *cores.PlatformRelease) { - pm.Log.WithField("bundle", pl.IsIDEBundled). + pm.log.WithField("bundle", pl.IsIDEBundled). WithField("version", pl.Version). WithField("managed", pm.IsManagedPlatformRelease(pl)). Debugf("%s: %s", msg, pl) @@ -465,7 +465,7 @@ func (pm *PackageManager) InstalledBoards() []*cores.Board { // FindToolsRequiredFromPlatformRelease returns a list of ToolReleases needed by the specified PlatformRelease. // If a ToolRelease is not found return an error func (pm *PackageManager) FindToolsRequiredFromPlatformRelease(platform *cores.PlatformRelease) ([]*cores.ToolRelease, error) { - pm.Log.Infof("Searching tools required for platform %s", platform) + pm.log.Infof("Searching tools required for platform %s", platform) // maps "PACKAGER:TOOL" => ToolRelease foundTools := map[string]*cores.ToolRelease{} @@ -483,7 +483,7 @@ func (pm *PackageManager) FindToolsRequiredFromPlatformRelease(platform *cores.P requiredTools := []*cores.ToolRelease{} platform.ToolDependencies.Sort() for _, toolDep := range platform.ToolDependencies { - pm.Log.WithField("tool", toolDep).Infof("Required tool") + pm.log.WithField("tool", toolDep).Infof("Required tool") tool := pm.FindToolDependency(toolDep) if tool == nil { return nil, fmt.Errorf(tr("tool release not found: %s"), toolDep) @@ -494,7 +494,7 @@ func (pm *PackageManager) FindToolsRequiredFromPlatformRelease(platform *cores.P platform.DiscoveryDependencies.Sort() for _, discoveryDep := range platform.DiscoveryDependencies { - pm.Log.WithField("discovery", discoveryDep).Infof("Required discovery") + pm.log.WithField("discovery", discoveryDep).Infof("Required discovery") tool := pm.FindDiscoveryDependency(discoveryDep) if tool == nil { return nil, fmt.Errorf(tr("discovery release not found: %s"), discoveryDep) @@ -505,7 +505,7 @@ func (pm *PackageManager) FindToolsRequiredFromPlatformRelease(platform *cores.P platform.MonitorDependencies.Sort() for _, monitorDep := range platform.MonitorDependencies { - pm.Log.WithField("monitor", monitorDep).Infof("Required monitor") + pm.log.WithField("monitor", monitorDep).Infof("Required monitor") tool := pm.FindMonitorDependency(monitorDep) if tool == nil { return nil, fmt.Errorf(tr("monitor release not found: %s"), monitorDep) @@ -537,7 +537,7 @@ func (pm *PackageManager) GetTool(toolID string) *cores.Tool { // FindToolsRequiredForBoard FIXMEDOC func (pm *PackageManager) FindToolsRequiredForBoard(board *cores.Board) ([]*cores.ToolRelease, error) { - pm.Log.Infof("Searching tools required for board %s", board) + pm.log.Infof("Searching tools required for board %s", board) // core := board.Properties["build.core"] platform := board.PlatformRelease @@ -560,7 +560,7 @@ func (pm *PackageManager) FindToolsRequiredForBoard(board *cores.Board) ([]*core requiredTools := []*cores.ToolRelease{} platform.ToolDependencies.Sort() for _, toolDep := range platform.ToolDependencies { - pm.Log.WithField("tool", toolDep).Infof("Required tool") + pm.log.WithField("tool", toolDep).Infof("Required tool") tool := pm.FindToolDependency(toolDep) if tool == nil { return nil, fmt.Errorf(tr("tool release not found: %s"), toolDep) diff --git a/arduino/cores/packagemanager/profiles.go b/arduino/cores/packagemanager/profiles.go index abe74a36860..a0be13ab946 100644 --- a/arduino/cores/packagemanager/profiles.go +++ b/arduino/cores/packagemanager/profiles.go @@ -86,8 +86,8 @@ func (pm *PackageManager) loadProfilePlatform(platformRef *sketch.ProfilePlatfor func (pm *PackageManager) installMissingProfilePlatform(platformRef *sketch.ProfilePlatformReference, destDir *paths.Path, downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) error { // Instantiate a temporary package manager only for platform installation - _ = pm.TempDir.MkdirAll() - tmp, err := paths.MkTempDir(pm.TempDir.String(), "") + _ = pm.tempDir.MkdirAll() + tmp, err := paths.MkTempDir(pm.tempDir.String(), "") if err != nil { return fmt.Errorf("installing missing platform: could not create temp dir %s", err) } diff --git a/cli/outdated/outdated.go b/cli/outdated/outdated.go index 15f91edbae2..7938cc9c91e 100644 --- a/cli/outdated/outdated.go +++ b/cli/outdated/outdated.go @@ -21,7 +21,7 @@ import ( "github.com/arduino/arduino-cli/cli/feedback" "github.com/arduino/arduino-cli/cli/instance" - "github.com/arduino/arduino-cli/commands" + "github.com/arduino/arduino-cli/commands/outdated" "github.com/arduino/arduino-cli/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/arduino/arduino-cli/table" @@ -50,7 +50,7 @@ func runOutdatedCommand(cmd *cobra.Command, args []string) { inst := instance.CreateAndInit() logrus.Info("Executing `arduino-cli outdated`") - outdatedResp, err := commands.Outdated(context.Background(), &rpc.OutdatedRequest{ + outdatedResp, err := outdated.Outdated(context.Background(), &rpc.OutdatedRequest{ Instance: inst, }) if err != nil { diff --git a/cli/update/update.go b/cli/update/update.go index 8ce69ae75d7..b6fed03f1a5 100644 --- a/cli/update/update.go +++ b/cli/update/update.go @@ -24,6 +24,7 @@ import ( "github.com/arduino/arduino-cli/cli/instance" "github.com/arduino/arduino-cli/cli/output" "github.com/arduino/arduino-cli/commands" + "github.com/arduino/arduino-cli/commands/outdated" "github.com/arduino/arduino-cli/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/arduino/arduino-cli/table" @@ -70,7 +71,7 @@ func runUpdateCommand(cmd *cobra.Command, args []string) { feedback.Errorf(tr("Error initializing instance: %v"), err) } - outdatedResp, err := commands.Outdated(context.Background(), &rpc.OutdatedRequest{ + outdatedResp, err := outdated.Outdated(context.Background(), &rpc.OutdatedRequest{ Instance: inst, }) if err != nil { diff --git a/cli/upgrade/upgrade.go b/cli/upgrade/upgrade.go index 5134774dbb4..bbe001ff72e 100644 --- a/cli/upgrade/upgrade.go +++ b/cli/upgrade/upgrade.go @@ -23,7 +23,7 @@ import ( "github.com/arduino/arduino-cli/cli/feedback" "github.com/arduino/arduino-cli/cli/instance" "github.com/arduino/arduino-cli/cli/output" - "github.com/arduino/arduino-cli/commands" + "github.com/arduino/arduino-cli/commands/upgrade" "github.com/arduino/arduino-cli/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/sirupsen/logrus" @@ -54,7 +54,7 @@ func runUpgradeCommand(cmd *cobra.Command, args []string) { inst := instance.CreateAndInit() logrus.Info("Executing `arduino-cli upgrade`") - err := commands.Upgrade(context.Background(), &rpc.UpgradeRequest{ + err := upgrade.Upgrade(context.Background(), &rpc.UpgradeRequest{ Instance: inst, SkipPostInstall: postInstallFlags.DetectSkipPostInstallValue(), }, output.NewDownloadProgressBarCB(), output.TaskProgress()) diff --git a/commands/bundled_tools.go b/commands/bundled_tools.go deleted file mode 100644 index e55af7ae21a..00000000000 --- a/commands/bundled_tools.go +++ /dev/null @@ -1,56 +0,0 @@ -// This file is part of arduino-cli. -// -// Copyright 2020 ARDUINO SA (http://www.arduino.cc/) -// -// This software is released under the GNU General Public License version 3, -// which covers the main part of arduino-cli. -// The terms of this license can be found at: -// https://www.gnu.org/licenses/gpl-3.0.en.html -// -// You can be released from the requirements of the above licenses by purchasing -// a commercial license. Buying such a license is mandatory if you want to -// modify or otherwise use the software for commercial activities involving the -// Arduino software without disclosing the source code of your own applications. -// To purchase a commercial license, send an email to license@arduino.cc. - -package commands - -import ( - "github.com/arduino/arduino-cli/arduino" - "github.com/arduino/arduino-cli/arduino/cores" - "github.com/arduino/arduino-cli/arduino/cores/packagemanager" - "github.com/arduino/arduino-cli/arduino/httpclient" - rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" -) - -// DownloadToolRelease downloads a ToolRelease -func DownloadToolRelease(pm *packagemanager.PackageManager, toolRelease *cores.ToolRelease, downloadCB rpc.DownloadProgressCB) error { - config, err := httpclient.GetDownloaderConfig() - if err != nil { - return err - } - return pm.DownloadToolRelease(toolRelease, config, toolRelease.String(), downloadCB) -} - -// InstallToolRelease installs a ToolRelease -func InstallToolRelease(pm *packagemanager.PackageManager, toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB) error { - log := pm.Log.WithField("Tool", toolRelease) - - if toolRelease.IsInstalled() { - log.Warn("Tool already installed") - taskCB(&rpc.TaskProgress{Name: tr("Tool %s already installed", toolRelease), Completed: true}) - return nil - } - - log.Info("Installing tool") - taskCB(&rpc.TaskProgress{Name: tr("Installing %s", toolRelease)}) - err := pm.InstallTool(toolRelease) - if err != nil { - log.WithError(err).Warn("Cannot install tool") - return &arduino.FailedInstallError{Message: tr("Cannot install tool %s", toolRelease), Cause: err} - } - log.Info("Tool installed") - taskCB(&rpc.TaskProgress{Message: tr("%s installed", toolRelease), Completed: true}) - - return nil -} diff --git a/commands/core/download.go b/commands/core/download.go index b1ef76395df..4da671ed837 100644 --- a/commands/core/download.go +++ b/commands/core/download.go @@ -17,12 +17,9 @@ package core import ( "context" - "errors" "github.com/arduino/arduino-cli/arduino" - "github.com/arduino/arduino-cli/arduino/cores" "github.com/arduino/arduino-cli/arduino/cores/packagemanager" - "github.com/arduino/arduino-cli/arduino/httpclient" "github.com/arduino/arduino-cli/commands" "github.com/arduino/arduino-cli/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" @@ -52,39 +49,15 @@ func PlatformDownload(ctx context.Context, req *rpc.PlatformDownloadRequest, dow return nil, &arduino.PlatformNotFoundError{Platform: ref.String(), Cause: err} } - if err := downloadPlatform(pm, platform, downloadCB); err != nil { + if err := pm.DownloadPlatformRelease(platform, nil, downloadCB); err != nil { return nil, err } for _, tool := range tools { - if err := downloadTool(pm, tool, downloadCB); err != nil { + if err := pm.DownloadToolRelease(tool, nil, downloadCB); err != nil { return nil, err } } return &rpc.PlatformDownloadResponse{}, nil } - -func downloadPlatform(pm *packagemanager.PackageManager, platformRelease *cores.PlatformRelease, downloadCB rpc.DownloadProgressCB) error { - // Download platform - config, err := httpclient.GetDownloaderConfig() - if err != nil { - return &arduino.FailedDownloadError{Message: tr("Error downloading platform %s", platformRelease), Cause: err} - } - return pm.DownloadPlatformRelease(platformRelease, config, downloadCB) -} - -func downloadTool(pm *packagemanager.PackageManager, tool *cores.ToolRelease, downloadCB rpc.DownloadProgressCB) error { - // Check if tool has a flavor available for the current OS - if tool.GetCompatibleFlavour() == nil { - return &arduino.FailedDownloadError{ - Message: tr("Error downloading tool %s", tool), - Cause: errors.New(tr("no versions available for the current OS", tool))} - } - - if err := commands.DownloadToolRelease(pm, tool, downloadCB); err != nil { - return &arduino.FailedDownloadError{Message: tr("Error downloading tool %s", tool), Cause: err} - } - - return nil -} diff --git a/commands/core/install.go b/commands/core/install.go index 42c4db2935d..21c9f0c9c46 100644 --- a/commands/core/install.go +++ b/commands/core/install.go @@ -20,7 +20,6 @@ import ( "fmt" "github.com/arduino/arduino-cli/arduino" - "github.com/arduino/arduino-cli/arduino/cores" "github.com/arduino/arduino-cli/arduino/cores/packagemanager" "github.com/arduino/arduino-cli/commands" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" @@ -64,7 +63,7 @@ func PlatformInstall(ctx context.Context, req *rpc.PlatformInstallRequest, } } - if err := installPlatform(pm, platformRelease, tools, downloadCB, taskCB, req.GetSkipPostInstall()); err != nil { + if err := pm.DownloadAndInstallPlatformAndTools(platformRelease, tools, downloadCB, taskCB, req.GetSkipPostInstall()); err != nil { return nil, err } @@ -74,115 +73,3 @@ func PlatformInstall(ctx context.Context, req *rpc.PlatformInstallRequest, return &rpc.PlatformInstallResponse{}, nil } - -func installPlatform(pm *packagemanager.PackageManager, - platformRelease *cores.PlatformRelease, requiredTools []*cores.ToolRelease, - downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB, - skipPostInstall bool) error { - log := pm.Log.WithField("platform", platformRelease) - - // Prerequisite checks before install - toolsToInstall := []*cores.ToolRelease{} - for _, tool := range requiredTools { - if tool.IsInstalled() { - log.WithField("tool", tool).Warn("Tool already installed") - taskCB(&rpc.TaskProgress{Name: tr("Tool %s already installed", tool), Completed: true}) - } else { - toolsToInstall = append(toolsToInstall, tool) - } - } - - // Package download - taskCB(&rpc.TaskProgress{Name: tr("Downloading packages")}) - for _, tool := range toolsToInstall { - if err := downloadTool(pm, tool, downloadCB); err != nil { - return err - } - } - if err := downloadPlatform(pm, platformRelease, downloadCB); err != nil { - return err - } - taskCB(&rpc.TaskProgress{Completed: true}) - - // Install tools first - for _, tool := range toolsToInstall { - if err := commands.InstallToolRelease(pm, tool, taskCB); err != nil { - return err - } - } - - installed := pm.GetInstalledPlatformRelease(platformRelease.Platform) - installedTools := []*cores.ToolRelease{} - if installed == nil { - // No version of this platform is installed - log.Info("Installing platform") - taskCB(&rpc.TaskProgress{Name: tr("Installing platform %s", platformRelease)}) - } else { - // A platform with a different version is already installed - log.Info("Replacing platform " + installed.String()) - taskCB(&rpc.TaskProgress{Name: tr("Replacing platform %[1]s with %[2]s", installed, platformRelease)}) - platformRef := &packagemanager.PlatformReference{ - Package: platformRelease.Platform.Package.Name, - PlatformArchitecture: platformRelease.Platform.Architecture, - PlatformVersion: installed.Version, - } - - // Get a list of tools used by the currently installed platform version. - // This must be done so tools used by the currently installed version are - // removed if not used also by the newly installed version. - var err error - _, installedTools, err = pm.FindPlatformReleaseDependencies(platformRef) - if err != nil { - return &arduino.NotFoundError{Message: tr("Can't find dependencies for platform %s", platformRef), Cause: err} - } - } - - // Install - if err := pm.InstallPlatform(platformRelease); err != nil { - log.WithError(err).Error("Cannot install platform") - return &arduino.FailedInstallError{Message: tr("Cannot install platform"), Cause: err} - } - - // If upgrading remove previous release - if installed != nil { - uninstallErr := pm.UninstallPlatform(installed) - - // In case of error try to rollback - if uninstallErr != nil { - log.WithError(uninstallErr).Error("Error upgrading platform.") - taskCB(&rpc.TaskProgress{Message: tr("Error upgrading platform: %s", uninstallErr)}) - - // Rollback - if err := pm.UninstallPlatform(platformRelease); err != nil { - log.WithError(err).Error("Error rolling-back changes.") - taskCB(&rpc.TaskProgress{Message: tr("Error rolling-back changes: %s", err)}) - } - - return &arduino.FailedInstallError{Message: tr("Cannot upgrade platform"), Cause: uninstallErr} - } - - // Uninstall unused tools - for _, tool := range installedTools { - if !pm.IsToolRequired(tool) { - uninstallToolRelease(pm, tool, taskCB) - } - } - - } - - // Perform post install - if !skipPostInstall { - log.Info("Running post_install script") - taskCB(&rpc.TaskProgress{Message: tr("Configuring platform.")}) - if err := pm.RunPostInstallScript(platformRelease); err != nil { - taskCB(&rpc.TaskProgress{Message: tr("WARNING cannot configure platform: %s", err)}) - } - } else { - log.Info("Skipping platform configuration.") - taskCB(&rpc.TaskProgress{Message: tr("Skipping platform configuration.")}) - } - - log.Info("Platform installed") - taskCB(&rpc.TaskProgress{Message: tr("Platform %s installed", platformRelease), Completed: true}) - return nil -} diff --git a/commands/core/uninstall.go b/commands/core/uninstall.go index bbd9a0347d5..b5127e3cd37 100644 --- a/commands/core/uninstall.go +++ b/commands/core/uninstall.go @@ -19,7 +19,6 @@ import ( "context" "github.com/arduino/arduino-cli/arduino" - "github.com/arduino/arduino-cli/arduino/cores" "github.com/arduino/arduino-cli/arduino/cores/packagemanager" "github.com/arduino/arduino-cli/commands" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" @@ -53,13 +52,14 @@ func PlatformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, t return nil, &arduino.NotFoundError{Message: tr("Can't find dependencies for platform %s", ref), Cause: err} } - if err := uninstallPlatformRelease(pm, platform, taskCB); err != nil { + if err := pm.UninstallPlatform(platform, taskCB); err != nil { return nil, err } for _, tool := range tools { if !pm.IsToolRequired(tool) { - uninstallToolRelease(pm, tool, taskCB) + taskCB(&rpc.TaskProgress{Name: tr("Uninstalling %s, tool is no more required", tool)}) + pm.UninstallTool(tool, taskCB) } } @@ -69,35 +69,3 @@ func PlatformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, t return &rpc.PlatformUninstallResponse{}, nil } - -func uninstallPlatformRelease(pm *packagemanager.PackageManager, platformRelease *cores.PlatformRelease, taskCB rpc.TaskProgressCB) error { - log := pm.Log.WithField("platform", platformRelease) - - log.Info("Uninstalling platform") - taskCB(&rpc.TaskProgress{Name: tr("Uninstalling %s", platformRelease)}) - - if err := pm.UninstallPlatform(platformRelease); err != nil { - log.WithError(err).Error("Error uninstalling") - return &arduino.FailedUninstallError{Message: tr("Error uninstalling platform %s", platformRelease), Cause: err} - } - - log.Info("Platform uninstalled") - taskCB(&rpc.TaskProgress{Message: tr("Platform %s uninstalled", platformRelease), Completed: true}) - return nil -} - -func uninstallToolRelease(pm *packagemanager.PackageManager, toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB) error { - log := pm.Log.WithField("Tool", toolRelease) - - log.Info("Uninstalling tool") - taskCB(&rpc.TaskProgress{Name: tr("Uninstalling %s, tool is no more required", toolRelease)}) - - if err := pm.UninstallTool(toolRelease); err != nil { - log.WithError(err).Error("Error uninstalling") - return &arduino.FailedUninstallError{Message: tr("Error uninstalling tool %s", toolRelease), Cause: err} - } - - log.Info("Tool uninstalled") - taskCB(&rpc.TaskProgress{Message: tr("Tool %s uninstalled", toolRelease), Completed: true}) - return nil -} diff --git a/commands/core/upgrade.go b/commands/core/upgrade.go index 20c88e5707b..6f55ab2367e 100644 --- a/commands/core/upgrade.go +++ b/commands/core/upgrade.go @@ -38,7 +38,7 @@ func PlatformUpgrade(ctx context.Context, req *rpc.PlatformUpgradeRequest, Package: req.PlatformPackage, PlatformArchitecture: req.Architecture, } - if err := upgradePlatform(pm, ref, downloadCB, taskCB, req.GetSkipPostInstall()); err != nil { + if err := pm.DownloadAndInstallPlatformUpgrades(ref, downloadCB, taskCB, req.GetSkipPostInstall()); err != nil { return nil, err } @@ -48,35 +48,3 @@ func PlatformUpgrade(ctx context.Context, req *rpc.PlatformUpgradeRequest, return &rpc.PlatformUpgradeResponse{}, nil } - -func upgradePlatform(pm *packagemanager.PackageManager, platformRef *packagemanager.PlatformReference, - downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB, skipPostInstall bool) error { - if platformRef.PlatformVersion != nil { - return &arduino.InvalidArgumentError{Message: tr("Upgrade doesn't accept parameters with version")} - } - - // Search the latest version for all specified platforms - platform := pm.FindPlatform(platformRef) - if platform == nil { - return &arduino.PlatformNotFoundError{Platform: platformRef.String()} - } - installed := pm.GetInstalledPlatformRelease(platform) - if installed == nil { - return &arduino.PlatformNotFoundError{Platform: platformRef.String()} - } - latest := platform.GetLatestRelease() - if !latest.Version.GreaterThan(installed.Version) { - return &arduino.PlatformAlreadyAtTheLatestVersionError{} - } - platformRef.PlatformVersion = latest.Version - - platformRelease, tools, err := pm.FindPlatformReleaseDependencies(platformRef) - if err != nil { - return &arduino.PlatformNotFoundError{Platform: platformRef.String()} - } - if err := installPlatform(pm, platformRelease, tools, downloadCB, taskCB, skipPostInstall); err != nil { - return err - } - - return nil -} diff --git a/commands/daemon/daemon.go b/commands/daemon/daemon.go index 4e84da07324..a82656bbe70 100644 --- a/commands/daemon/daemon.go +++ b/commands/daemon/daemon.go @@ -29,7 +29,9 @@ import ( "github.com/arduino/arduino-cli/commands/core" "github.com/arduino/arduino-cli/commands/lib" "github.com/arduino/arduino-cli/commands/monitor" + "github.com/arduino/arduino-cli/commands/outdated" "github.com/arduino/arduino-cli/commands/sketch" + "github.com/arduino/arduino-cli/commands/upgrade" "github.com/arduino/arduino-cli/commands/upload" "github.com/arduino/arduino-cli/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" @@ -202,13 +204,13 @@ func (s *ArduinoCoreServerImpl) UpdateCoreLibrariesIndex(req *rpc.UpdateCoreLibr // Outdated FIXMEDOC func (s *ArduinoCoreServerImpl) Outdated(ctx context.Context, req *rpc.OutdatedRequest) (*rpc.OutdatedResponse, error) { - resp, err := commands.Outdated(ctx, req) + resp, err := outdated.Outdated(ctx, req) return resp, convertErrorToRPCStatus(err) } // Upgrade FIXMEDOC func (s *ArduinoCoreServerImpl) Upgrade(req *rpc.UpgradeRequest, stream rpc.ArduinoCoreService_UpgradeServer) error { - err := commands.Upgrade(stream.Context(), req, + err := upgrade.Upgrade(stream.Context(), req, func(p *rpc.DownloadProgress) { stream.Send(&rpc.UpgradeResponse{ Progress: p, diff --git a/commands/instances.go b/commands/instances.go index 1b52fafd040..05b6f99f5db 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -17,7 +17,6 @@ package commands import ( "context" - "errors" "fmt" "net/url" "os" @@ -28,7 +27,6 @@ import ( "github.com/arduino/arduino-cli/arduino/cores/packageindex" "github.com/arduino/arduino-cli/arduino/cores/packagemanager" "github.com/arduino/arduino-cli/arduino/globals" - "github.com/arduino/arduino-cli/arduino/httpclient" "github.com/arduino/arduino-cli/arduino/libraries" "github.com/arduino/arduino-cli/arduino/libraries/librariesindex" "github.com/arduino/arduino-cli/arduino/libraries/librariesmanager" @@ -95,11 +93,11 @@ func (instance *CoreInstance) installToolIfMissing(tool *cores.ToolRelease, down return false, nil } taskCB(&rpc.TaskProgress{Name: tr("Downloading missing tool %s", tool)}) - if err := DownloadToolRelease(instance.PackageManager, tool, downloadCB); err != nil { + if err := instance.PackageManager.DownloadToolRelease(tool, nil, downloadCB); err != nil { return false, fmt.Errorf(tr("downloading %[1]s tool: %[2]s"), tool, err) } taskCB(&rpc.TaskProgress{Completed: true}) - if err := InstallToolRelease(instance.PackageManager, tool, taskCB); err != nil { + if err := instance.PackageManager.InstallTool(tool, taskCB); err != nil { return false, fmt.Errorf(tr("installing %[1]s tool: %[2]s"), tool, err) } return true, nil @@ -524,307 +522,6 @@ func UpdateCoreLibrariesIndex(ctx context.Context, req *rpc.UpdateCoreLibrariesI return nil } -// Outdated returns a list struct containing both Core and Libraries that can be updated -func Outdated(ctx context.Context, req *rpc.OutdatedRequest) (*rpc.OutdatedResponse, error) { - id := req.GetInstance().GetId() - - lm := GetLibraryManager(id) - if lm == nil { - return nil, &arduino.InvalidInstanceError{} - } - - outdatedLibraries := []*rpc.InstalledLibrary{} - for _, libAlternatives := range lm.Libraries { - for _, library := range libAlternatives.Alternatives { - if library.Location != libraries.User { - continue - } - available := lm.Index.FindLibraryUpdate(library) - if available == nil { - continue - } - - outdatedLibraries = append(outdatedLibraries, &rpc.InstalledLibrary{ - Library: getOutputLibrary(library), - Release: getOutputRelease(available), - }) - } - } - - pm := GetPackageManager(id) - if pm == nil { - return nil, &arduino.InvalidInstanceError{} - } - - outdatedPlatforms := []*rpc.Platform{} - for _, targetPackage := range pm.Packages { - for _, installed := range targetPackage.Platforms { - if installedRelease := pm.GetInstalledPlatformRelease(installed); installedRelease != nil { - latest := installed.GetLatestRelease() - if latest == nil || latest == installedRelease { - continue - } - rpcPlatform := PlatformReleaseToRPC(latest) - rpcPlatform.Installed = installedRelease.Version.String() - - outdatedPlatforms = append( - outdatedPlatforms, - rpcPlatform, - ) - } - } - } - - return &rpc.OutdatedResponse{ - OutdatedLibraries: outdatedLibraries, - OutdatedPlatforms: outdatedPlatforms, - }, nil -} - -func getOutputLibrary(lib *libraries.Library) *rpc.Library { - insdir := "" - if lib.InstallDir != nil { - insdir = lib.InstallDir.String() - } - srcdir := "" - if lib.SourceDir != nil { - srcdir = lib.SourceDir.String() - } - utldir := "" - if lib.UtilityDir != nil { - utldir = lib.UtilityDir.String() - } - cntplat := "" - if lib.ContainerPlatform != nil { - cntplat = lib.ContainerPlatform.String() - } - - return &rpc.Library{ - Name: lib.Name, - Author: lib.Author, - Maintainer: lib.Maintainer, - Sentence: lib.Sentence, - Paragraph: lib.Paragraph, - Website: lib.Website, - Category: lib.Category, - Architectures: lib.Architectures, - Types: lib.Types, - InstallDir: insdir, - SourceDir: srcdir, - UtilityDir: utldir, - Location: lib.Location.ToRPCLibraryLocation(), - ContainerPlatform: cntplat, - Layout: lib.Layout.ToRPCLibraryLayout(), - RealName: lib.RealName, - DotALinkage: lib.DotALinkage, - Precompiled: lib.Precompiled, - LdFlags: lib.LDflags, - IsLegacy: lib.IsLegacy, - Version: lib.Version.String(), - License: lib.License, - } -} - -func getOutputRelease(lib *librariesindex.Release) *rpc.LibraryRelease { - if lib != nil { - return &rpc.LibraryRelease{ - Author: lib.Author, - Version: lib.Version.String(), - Maintainer: lib.Maintainer, - Sentence: lib.Sentence, - Paragraph: lib.Paragraph, - Website: lib.Website, - Category: lib.Category, - Architectures: lib.Architectures, - Types: lib.Types, - } - } - return &rpc.LibraryRelease{} -} - -// Upgrade downloads and installs outdated Cores and Libraries -func Upgrade(ctx context.Context, req *rpc.UpgradeRequest, downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) error { - downloaderConfig, err := httpclient.GetDownloaderConfig() - if err != nil { - return err - } - - lm := GetLibraryManager(req.Instance.Id) - if lm == nil { - return &arduino.InvalidInstanceError{} - } - - for _, libAlternatives := range lm.Libraries { - for _, library := range libAlternatives.Alternatives { - if library.Location != libraries.User { - continue - } - available := lm.Index.FindLibraryUpdate(library) - if available == nil { - continue - } - - // Downloads latest library release - taskCB(&rpc.TaskProgress{Name: tr("Downloading %s", available)}) - if err := available.Resource.Download(lm.DownloadsDir, downloaderConfig, available.String(), downloadCB); err != nil { - return &arduino.FailedDownloadError{Message: tr("Error downloading library"), Cause: err} - } - - // Installs downloaded library - taskCB(&rpc.TaskProgress{Name: tr("Installing %s", available)}) - libPath, libReplaced, err := lm.InstallPrerequisiteCheck(available) - if errors.Is(err, librariesmanager.ErrAlreadyInstalled) { - taskCB(&rpc.TaskProgress{Message: tr("Already installed %s", available), Completed: true}) - continue - } else if err != nil { - return &arduino.FailedLibraryInstallError{Cause: err} - } - - if libReplaced != nil { - taskCB(&rpc.TaskProgress{Message: tr("Replacing %[1]s with %[2]s", libReplaced, available)}) - } - - if err := lm.Install(available, libPath); err != nil { - return &arduino.FailedLibraryInstallError{Cause: err} - } - - taskCB(&rpc.TaskProgress{Message: tr("Installed %s", available), Completed: true}) - } - } - - pm := GetPackageManager(req.Instance.Id) - if pm == nil { - return &arduino.InvalidInstanceError{} - } - - for _, targetPackage := range pm.Packages { - for _, installed := range targetPackage.Platforms { - if installedRelease := pm.GetInstalledPlatformRelease(installed); installedRelease != nil { - latest := installed.GetLatestRelease() - if latest == nil || latest == installedRelease { - continue - } - - ref := &packagemanager.PlatformReference{ - Package: installedRelease.Platform.Package.Name, - PlatformArchitecture: installedRelease.Platform.Architecture, - PlatformVersion: installedRelease.Version, - } - // Get list of installed tools needed by the currently installed version - _, installedTools, err := pm.FindPlatformReleaseDependencies(ref) - if err != nil { - return &arduino.NotFoundError{Message: tr("Can't find dependencies for platform %s", ref), Cause: err} - } - - ref = &packagemanager.PlatformReference{ - Package: latest.Platform.Package.Name, - PlatformArchitecture: latest.Platform.Architecture, - PlatformVersion: latest.Version, - } - - taskCB(&rpc.TaskProgress{Name: tr("Downloading %s", latest)}) - _, tools, err := pm.FindPlatformReleaseDependencies(ref) - if err != nil { - return &arduino.NotFoundError{Message: tr("Can't find dependencies for platform %s", ref), Cause: err} - } - - toolsToInstall := []*cores.ToolRelease{} - for _, tool := range tools { - if tool.IsInstalled() { - logrus.WithField("tool", tool).Warn("Tool already installed") - taskCB(&rpc.TaskProgress{Name: tr("Tool %s already installed", tool), Completed: true}) - } else { - toolsToInstall = append(toolsToInstall, tool) - } - } - - // Downloads platform tools - for _, tool := range toolsToInstall { - if err := DownloadToolRelease(pm, tool, downloadCB); err != nil { - taskCB(&rpc.TaskProgress{Message: tr("Error downloading tool %s", tool)}) - return &arduino.FailedDownloadError{Message: tr("Error downloading tool %s", tool), Cause: err} - } - } - - // Downloads platform - if err := pm.DownloadPlatformRelease(latest, downloaderConfig, downloadCB); err != nil { - return &arduino.FailedDownloadError{Message: tr("Error downloading platform %s", latest), Cause: err} - } - - logrus.Info("Updating platform " + installed.String()) - taskCB(&rpc.TaskProgress{Name: tr("Updating platform %s", latest)}) - - // Installs tools - for _, tool := range toolsToInstall { - if err := InstallToolRelease(pm, tool, taskCB); err != nil { - msg := tr("Error installing tool %s", tool) - taskCB(&rpc.TaskProgress{Message: msg}) - return &arduino.FailedInstallError{Message: msg, Cause: err} - } - } - - // Installs platform - err = pm.InstallPlatform(latest) - if err != nil { - logrus.WithError(err).Error("Cannot install platform") - msg := tr("Error installing platform %s", latest) - taskCB(&rpc.TaskProgress{Message: msg}) - return &arduino.FailedInstallError{Message: msg, Cause: err} - } - - // Uninstall previously installed release - err = pm.UninstallPlatform(installedRelease) - - // In case uninstall fails tries to rollback - if err != nil { - logrus.WithError(err).Error("Error updating platform.") - taskCB(&rpc.TaskProgress{Message: tr("Error upgrading platform: %s", err)}) - - // Rollback - if err := pm.UninstallPlatform(latest); err != nil { - logrus.WithError(err).Error("Error rolling-back changes.") - msg := tr("Error rolling-back changes") - taskCB(&rpc.TaskProgress{Message: fmt.Sprintf("%s: %s", msg, err)}) - return &arduino.FailedInstallError{Message: msg, Cause: err} - } - } - - // Uninstall unused tools - for _, toolRelease := range installedTools { - if !pm.IsToolRequired(toolRelease) { - log := pm.Log.WithField("Tool", toolRelease) - - log.Info("Uninstalling tool") - taskCB(&rpc.TaskProgress{Name: tr("Uninstalling %s: tool is no more required", toolRelease)}) - - if err := pm.UninstallTool(toolRelease); err != nil { - log.WithError(err).Error("Error uninstalling") - return &arduino.FailedInstallError{Message: tr("Error uninstalling tool %s", toolRelease), Cause: err} - } - - log.Info("Tool uninstalled") - taskCB(&rpc.TaskProgress{Message: tr("%s uninstalled", toolRelease), Completed: true}) - } - } - - // Perform post install - if !req.SkipPostInstall { - logrus.Info("Running post_install script") - taskCB(&rpc.TaskProgress{Message: tr("Configuring platform")}) - if err := pm.RunPostInstallScript(latest); err != nil { - taskCB(&rpc.TaskProgress{Message: tr("WARNING: cannot run post install: %s", err)}) - } - } else { - logrus.Info("Skipping platform configuration (post_install run).") - taskCB(&rpc.TaskProgress{Message: tr("Skipping platform configuration")}) - } - } - } - } - - return nil -} - // LoadSketch collects and returns all files composing a sketch func LoadSketch(ctx context.Context, req *rpc.LoadSketchRequest) (*rpc.LoadSketchResponse, error) { // TODO: This should be a ToRpc function for the Sketch struct diff --git a/commands/outdated/outdated.go b/commands/outdated/outdated.go new file mode 100644 index 00000000000..3bcf3503bde --- /dev/null +++ b/commands/outdated/outdated.go @@ -0,0 +1,48 @@ +// This file is part of arduino-cli. +// +// Copyright 2022 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package outdated + +import ( + "context" + + "github.com/arduino/arduino-cli/commands/core" + "github.com/arduino/arduino-cli/commands/lib" + rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" +) + +// Outdated returns a list struct containing both Core and Libraries that can be updated +func Outdated(ctx context.Context, req *rpc.OutdatedRequest) (*rpc.OutdatedResponse, error) { + libraryListResponse, err := lib.LibraryList(ctx, &rpc.LibraryListRequest{ + Instance: req.GetInstance(), + Updatable: true, + }) + if err != nil { + return nil, err + } + + getPlatformsResp, err := core.GetPlatforms(&rpc.PlatformListRequest{ + Instance: req.GetInstance(), + UpdatableOnly: true, + }) + if err != nil { + return nil, err + } + + return &rpc.OutdatedResponse{ + OutdatedLibraries: libraryListResponse.GetInstalledLibraries(), + OutdatedPlatforms: getPlatformsResp, + }, nil +} diff --git a/commands/upgrade/upgrade.go b/commands/upgrade/upgrade.go new file mode 100644 index 00000000000..e30d09f2a27 --- /dev/null +++ b/commands/upgrade/upgrade.go @@ -0,0 +1,59 @@ +// This file is part of arduino-cli. +// +// Copyright 2022 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package upgrade + +import ( + "context" + "strings" + + "github.com/arduino/arduino-cli/commands/core" + "github.com/arduino/arduino-cli/commands/lib" + "github.com/arduino/arduino-cli/commands/outdated" + rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" +) + +// Upgrade downloads and installs outdated Cores and Libraries +func Upgrade(ctx context.Context, req *rpc.UpgradeRequest, downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) error { + outdatedResp, err := outdated.Outdated(ctx, &rpc.OutdatedRequest{Instance: req.GetInstance()}) + if err != nil { + return err + } + + for _, libToUpgrade := range outdatedResp.GetOutdatedLibraries() { + err := lib.LibraryInstall(ctx, &rpc.LibraryInstallRequest{ + Instance: req.GetInstance(), + Name: libToUpgrade.GetLibrary().GetName(), + }, downloadCB, taskCB) + if err != nil { + return err + } + } + + for _, platformToUpgrade := range outdatedResp.GetOutdatedPlatforms() { + split := strings.Split(platformToUpgrade.GetId(), ":") + _, err := core.PlatformUpgrade(ctx, &rpc.PlatformUpgradeRequest{ + Instance: req.GetInstance(), + PlatformPackage: split[0], + Architecture: split[1], + SkipPostInstall: req.GetSkipPostInstall(), + }, downloadCB, taskCB) + if err != nil { + return err + } + } + + return nil +} diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 34c00a402ac..80f7f28264e 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -2,6 +2,56 @@ Here you can find a list of migration guides to handle breaking changes between releases of the CLI. +## 0.26.0 + +### `github.com/arduino/arduino-cli/commands.DownloadToolRelease`, and `InstallToolRelease` functions have been removed + +This functionality was duplicated and already available via `PackageManager` methods. + +### `github.com/arduino/arduino-cli/commands.Outdated` and `Upgrade` functions have been moved + +- `github.com/arduino/arduino-cli/commands.Outdated` is now `github.com/arduino/arduino-cli/commands/outdated.Outdated` +- `github.com/arduino/arduino-cli/commands.Upgrade` is now `github.com/arduino/arduino-cli/commands/upgrade.Upgrade` + +Old code must change the imports accordingly. + +### `github.com/arduino-cli/arduino/cores/packagemanager.PackageManager` methods and fields change + +- The `PackageManager.Log` and `TempDir` fields are now private. + +- The `PackageManager.DownloadToolRelease` method has no more the `label` parameter: + + ```go + func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *downloader.Config, label string, progressCB rpc.DownloadProgressCB) error { + ``` + + has been changed to: + + ```go + func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *downloader.Config, progressCB rpc.DownloadProgressCB) error { + ``` + + Old code should remove the `label` parameter. + +- The `PackageManager.UninstallPlatform`, `PackageManager.InstallTool`, and `PackageManager.UninstallTool` methods now + requires a `github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1.TaskProgressCB` + + ```go + func (pm *PackageManager) UninstallPlatform(platformRelease *cores.PlatformRelease) error { + func (pm *PackageManager) InstallTool(toolRelease *cores.ToolRelease) error { + func (pm *PackageManager) UninstallTool(toolRelease *cores.ToolRelease) error { + ``` + + have been changed to: + + ```go + func (pm *PackageManager) UninstallPlatform(platformRelease *cores.PlatformRelease, taskCB rpc.TaskProgressCB) error { + func (pm *PackageManager) InstallTool(toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB) error { + func (pm *PackageManager) UninstallTool(toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB) error { + ``` + + If you're not interested in getting the task events you can pass an empty callback function. + ## 0.25.0 ### go-lang function `github.com/arduino/arduino-cli/arduino/utils.FeedStreamTo` has been changed From 9075512834694cd322865d05fa45fd0c2d93a112 Mon Sep 17 00:00:00 2001 From: per1234 Date: Tue, 9 Aug 2022 01:34:51 -0700 Subject: [PATCH 02/16] [skip changelog] Remove spurious argument from error message (#1827) `github.com/arduino/arduino-cli/i18n.Tr` provides a `fmt.Printf`-style interface, where the first argument may serve as a template filled by subsequent arguments according to its format verbs. This call contains a subsequent argument, but no verb in the format string to use it. This generates some cryptic content in the error message. For example: Error during install: Error downloading tool arduino:bossac@1.7.0-arduino3: no versions available for the current OS%!(EXTRA *cores.ToolRelease=arduino:bossac@1.7.0-arduino3) The tool name is already provided in the correctly configured `Message` field of the error, so there is no need to include it in the `Cause` field as well, so the spurious argument can be removed entirely rather than the alternative fix of adjusting the format string argument. I suspect the spurious argument was simply the result of a copy/paste error. --- arduino/cores/packagemanager/download.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arduino/cores/packagemanager/download.go b/arduino/cores/packagemanager/download.go index 6dfda64fbe6..a45241d9d4f 100644 --- a/arduino/cores/packagemanager/download.go +++ b/arduino/cores/packagemanager/download.go @@ -125,7 +125,7 @@ func (pm *PackageManager) DownloadToolRelease(tool *cores.ToolRelease, config *d if resource == nil { return &arduino.FailedDownloadError{ Message: tr("Error downloading tool %s", tool), - Cause: errors.New(tr("no versions available for the current OS", tool))} + Cause: errors.New(tr("no versions available for the current OS"))} } if err := resource.Download(pm.DownloadDir, config, tool.String(), progressCB); err != nil { return &arduino.FailedDownloadError{ From 9cc1eef95f29837e108684d1865161fdf70802b3 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 13 Jul 2022 14:53:38 +0200 Subject: [PATCH 03/16] Improved streaming of pluggable-discoveries events (WIP) Now the DiscoveryManager is able to start the discoveries and add/remove them in a thread-safe way. Also the watchers may connect and disconnect seamlessly at any time, the incoming events from the discovery are broadcasted correctly to each active watcher. This refactoring dramatically simplifies the DiscoveryManager design. --- .../discoverymanager/discoverymanager.go | 297 +++++++----------- cli/arguments/port.go | 25 +- cli/board/list.go | 3 +- commands/board/list.go | 109 ++----- commands/daemon/daemon.go | 27 +- 5 files changed, 149 insertions(+), 312 deletions(-) diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index b18fbf46bbd..e6857e77de0 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -28,8 +28,12 @@ import ( // DiscoveryManager is required to handle multiple pluggable-discovery that // may be shared across platforms type DiscoveryManager struct { - discoveriesMutex sync.Mutex - discoveries map[string]*discovery.PluggableDiscovery + discoveriesMutex sync.Mutex + discoveries map[string]*discovery.PluggableDiscovery + discoveriesRunning bool + feed chan *discovery.Event + watchersMutex sync.Mutex + watchers map[*PortWatcher]bool } var tr = i18n.Tr @@ -38,15 +42,20 @@ var tr = i18n.Tr func New() *DiscoveryManager { return &DiscoveryManager{ discoveries: map[string]*discovery.PluggableDiscovery{}, + watchers: map[*PortWatcher]bool{}, + feed: make(chan *discovery.Event, 50), } } // Clear resets the DiscoveryManager to its initial state func (dm *DiscoveryManager) Clear() { - dm.QuitAll() dm.discoveriesMutex.Lock() - defer dm.discoveriesMutex.Unlock() + for _, d := range dm.discoveries { + d.Quit() + logrus.Infof("Closed and removed discovery %s", d.GetID()) + } dm.discoveries = map[string]*discovery.PluggableDiscovery{} + dm.discoveriesMutex.Unlock() } // IDs returns the list of discoveries' ids in this DiscoveryManager @@ -60,233 +69,137 @@ func (dm *DiscoveryManager) IDs() []string { return ids } -// Add adds a discovery to the list of managed discoveries -func (dm *DiscoveryManager) Add(disc *discovery.PluggableDiscovery) error { - id := disc.GetID() +// Start starts all the discoveries in this DiscoveryManager. +// If the discoveries are already running, this function does nothing. +func (dm *DiscoveryManager) Start() { dm.discoveriesMutex.Lock() defer dm.discoveriesMutex.Unlock() - if _, has := dm.discoveries[id]; has { - return errors.Errorf(tr("pluggable discovery already added: %s"), id) + if dm.discoveriesRunning { + return } - dm.discoveries[id] = disc - return nil -} -// remove quits and deletes the discovery with specified id -// from the discoveries managed by this DiscoveryManager -func (dm *DiscoveryManager) remove(id string) { - dm.discoveriesMutex.Lock() - d := dm.discoveries[id] - delete(dm.discoveries, id) - dm.discoveriesMutex.Unlock() - d.Quit() - logrus.Infof("Closed and removed discovery %s", id) -} + go dm.feeder() -// parallelize runs function f concurrently for each discovery. -// Returns a list of errors returned by each call of f. -func (dm *DiscoveryManager) parallelize(f func(d *discovery.PluggableDiscovery) error) []error { var wg sync.WaitGroup - errChan := make(chan error) - dm.discoveriesMutex.Lock() - discoveries := []*discovery.PluggableDiscovery{} for _, d := range dm.discoveries { - discoveries = append(discoveries, d) - } - dm.discoveriesMutex.Unlock() - for _, d := range discoveries { wg.Add(1) go func(d *discovery.PluggableDiscovery) { - defer wg.Done() - if err := f(d); err != nil { - errChan <- err - } + dm.startDiscovery(d) + wg.Done() }(d) } + wg.Wait() + dm.discoveriesRunning = true +} - // Wait in a goroutine to collect eventual errors running a discovery. - // When all goroutines that are calling discoveries are done close the errors chan. - go func() { - wg.Wait() - close(errChan) - }() +// Add adds a discovery to the list of managed discoveries +func (dm *DiscoveryManager) Add(d *discovery.PluggableDiscovery) error { + dm.discoveriesMutex.Lock() + defer dm.discoveriesMutex.Unlock() - errs := []error{} - for err := range errChan { - errs = append(errs, err) + id := d.GetID() + if _, has := dm.discoveries[id]; has { + return errors.Errorf(tr("pluggable discovery already added: %s"), id) } - return errs -} - -// RunAll the discoveries for this DiscoveryManager, -// returns an error for each discovery failing to run -func (dm *DiscoveryManager) RunAll() []error { - return dm.parallelize(func(d *discovery.PluggableDiscovery) error { - if d.State() != discovery.Dead { - // This discovery is already alive, nothing to do - return nil - } + dm.discoveries[id] = d - if err := d.Run(); err != nil { - dm.remove(d.GetID()) - return fmt.Errorf(tr("discovery %[1]s process not started: %[2]w"), d.GetID(), err) - } - return nil - }) + if dm.discoveriesRunning { + dm.startDiscovery(d) + } + return nil } -// StartAll the discoveries for this DiscoveryManager, -// returns an error for each discovery failing to start -func (dm *DiscoveryManager) StartAll() []error { - return dm.parallelize(func(d *discovery.PluggableDiscovery) error { - state := d.State() - if state != discovery.Idling { - // Already started - return nil - } - if err := d.Start(); err != nil { - dm.remove(d.GetID()) - return fmt.Errorf(tr("starting discovery %[1]s: %[2]w"), d.GetID(), err) - } - return nil - }) +// PortWatcher is a watcher for all discovery events (port connection/disconnection) +type PortWatcher struct { + closeCB func() + feed chan *discovery.Event } -// StartSyncAll the discoveries for this DiscoveryManager, -// returns an error for each discovery failing to start syncing -func (dm *DiscoveryManager) StartSyncAll() (<-chan *discovery.Event, []error) { - eventSink := make(chan *discovery.Event, 5) - var wg sync.WaitGroup - errs := dm.parallelize(func(d *discovery.PluggableDiscovery) error { - state := d.State() - if state != discovery.Idling || state == discovery.Syncing { - // Already syncing - return nil - } - - eventCh, err := d.StartSync(5) - if err != nil { - dm.remove(d.GetID()) - return fmt.Errorf(tr("start syncing discovery %[1]s: %[2]w"), d.GetID(), err) - } +// Feed returns the feed of events coming from the discoveries +func (pw *PortWatcher) Feed() <-chan *discovery.Event { + return pw.feed +} - wg.Add(1) - go func() { - for ev := range eventCh { - eventSink <- ev - } - wg.Done() - }() - return nil - }) - go func() { - wg.Wait() - eventSink <- &discovery.Event{Type: "quit"} - close(eventSink) - }() - return eventSink, errs +// Close closes the PortWatcher +func (pw *PortWatcher) Close() { + pw.closeCB() } -// StopAll the discoveries for this DiscoveryManager, -// returns an error for each discovery failing to stop -func (dm *DiscoveryManager) StopAll() []error { - return dm.parallelize(func(d *discovery.PluggableDiscovery) error { - state := d.State() - if state != discovery.Syncing && state != discovery.Running { - // Not running nor syncing, nothing to stop - return nil - } +// Watch starts a watcher for all discovery events (port connection/disconnection). +// The watcher must be closed when it is no longer needed with the Close method. +func (dm *DiscoveryManager) Watch() (*PortWatcher, error) { + dm.Start() - if err := d.Stop(); err != nil { - dm.remove(d.GetID()) - return fmt.Errorf(tr("stopping discovery %[1]s: %[2]w"), d.GetID(), err) - } - return nil - }) + watcher := &PortWatcher{ + feed: make(chan *discovery.Event), + } + watcher.closeCB = func() { + dm.watchersMutex.Lock() + delete(dm.watchers, watcher) + dm.watchersMutex.Unlock() + close(watcher.feed) + } + dm.watchersMutex.Lock() + dm.watchers[watcher] = true + dm.watchersMutex.Unlock() + return watcher, nil } -// QuitAll quits all the discoveries managed by this DiscoveryManager. -// Returns an error for each discovery that fails quitting -func (dm *DiscoveryManager) QuitAll() []error { - errs := dm.parallelize(func(d *discovery.PluggableDiscovery) error { - if d.State() == discovery.Dead { - // Stop! Stop! It's already dead! - return nil +func (dm *DiscoveryManager) startDiscovery(d *discovery.PluggableDiscovery) (discErr error) { + defer func() { + if discErr != nil { + logrus.Errorf("Discovery %s failed to run: %s", d.GetID(), discErr) } + }() - d.Quit() - return nil - }) - return errs -} - -// List returns a list of available ports detected from all discoveries -// and a list of errors for those discoveries that returned one. -func (dm *DiscoveryManager) List() ([]*discovery.Port, []error) { - var wg sync.WaitGroup - // Use this struct to avoid the need of two separate - // channels for ports and errors. - type listMsg struct { - Err error - Port *discovery.Port + if err := d.Run(); err != nil { + return fmt.Errorf(tr("discovery %[1]s process not started: %[2]w"), d.GetID(), err) } - msgChan := make(chan listMsg) - dm.discoveriesMutex.Lock() - discoveries := []*discovery.PluggableDiscovery{} - for _, d := range dm.discoveries { - discoveries = append(discoveries, d) - } - dm.discoveriesMutex.Unlock() - for _, d := range discoveries { - wg.Add(1) - go func(d *discovery.PluggableDiscovery) { - defer wg.Done() - if d.State() != discovery.Running { - // Discovery is not running, it won't return anything - return - } - ports, err := d.List() - if err != nil { - msgChan <- listMsg{Err: fmt.Errorf(tr("listing ports from discovery %[1]s: %[2]w"), d.GetID(), err)} - } - for _, p := range ports { - msgChan <- listMsg{Port: p} - } - }(d) + eventCh, err := d.StartSync(5) + if err != nil { + return fmt.Errorf("%s: %s", tr("starting discovery %s", d.GetID()), err) } go func() { - // Close the channel only after all goroutines are done - wg.Wait() - close(msgChan) + for ev := range eventCh { + dm.feed <- ev + } }() + return nil +} - ports := []*discovery.Port{} - errs := []error{} - for msg := range msgChan { - if msg.Err != nil { - errs = append(errs, msg.Err) - } else { - ports = append(ports, msg.Port) +func (dm *DiscoveryManager) feeder() { + // Feed all watchers with data coming from the discoveries + for ev := range dm.feed { + dm.watchersMutex.Lock() + for watcher := range dm.watchers { + select { + case watcher.feed <- ev: + // OK + default: + // If the watcher is not able to process event fast enough + // remove the watcher from the list of watchers + go watcher.Close() + } } + dm.cacheEvent(ev) + dm.watchersMutex.Unlock() } - return ports, errs } -// ListCachedPorts return the current list of ports detected from all discoveries -func (dm *DiscoveryManager) ListCachedPorts() []*discovery.Port { - res := []*discovery.Port{} +func (dm *DiscoveryManager) cacheEvent(ev *discovery.Event) { + // XXX: TODO +} + +// List return the current list of ports detected from all discoveries +func (dm *DiscoveryManager) List() []*discovery.Port { + dm.Start() + + // XXX: Cache ports and return them dm.discoveriesMutex.Lock() - discoveries := []*discovery.PluggableDiscovery{} + defer dm.discoveriesMutex.Unlock() + res := []*discovery.Port{} for _, d := range dm.discoveries { - discoveries = append(discoveries, d) - } - dm.discoveriesMutex.Unlock() - for _, d := range discoveries { - if d.State() != discovery.Syncing { - // Discovery is not syncing - continue - } res = append(res, d.ListCachedPorts()...) } return res diff --git a/cli/arguments/port.go b/cli/arguments/port.go index 12e7e28f5c7..d77215dcf58 100644 --- a/cli/arguments/port.go +++ b/cli/arguments/port.go @@ -106,31 +106,16 @@ func (p *Port) GetPort(instance *rpc.Instance, sk *sketch.Sketch) (*discovery.Po return nil, errors.New("invalid instance") } dm := pm.DiscoveryManager() - if errs := dm.RunAll(); len(errs) == len(dm.IDs()) { - // All discoveries failed to run, we can't do anything - return nil, fmt.Errorf("%v", errs) - } else if len(errs) > 0 { - // If only some discoveries failed to run just tell the user and go on - for _, err := range errs { - feedback.Error(err) - } - } - eventChan, errs := dm.StartSyncAll() - if len(errs) > 0 { - return nil, fmt.Errorf("%v", errs) + watcher, err := dm.Watch() + if err != nil { + return nil, err } - - defer func() { - // Quit all discoveries at the end. - if errs := dm.QuitAll(); len(errs) > 0 { - logrus.Errorf("quitting discoveries when getting port metadata: %v", errs) - } - }() + defer watcher.Close() deadline := time.After(p.timeout.Get()) for { select { - case portEvent := <-eventChan: + case portEvent := <-watcher.Feed(): if portEvent.Type != "add" { continue } diff --git a/cli/board/list.go b/cli/board/list.go index 83f4e4418cd..b162e2a6f79 100644 --- a/cli/board/list.go +++ b/cli/board/list.go @@ -75,11 +75,12 @@ func runListCommand(cmd *cobra.Command, args []string) { } func watchList(cmd *cobra.Command, inst *rpc.Instance) { - eventsChan, err := board.Watch(inst.Id, nil) + eventsChan, closeCB, err := board.Watch(inst.Id) if err != nil { feedback.Errorf(tr("Error detecting boards: %v"), err) os.Exit(errorcodes.ErrNetwork) } + defer closeCB() // This is done to avoid printing the header each time a new event is received if feedback.GetFormat() == feedback.Text { diff --git a/commands/board/list.go b/commands/board/list.go index c2498889c10..84e7149b88f 100644 --- a/commands/board/list.go +++ b/commands/board/list.go @@ -16,6 +16,7 @@ package board import ( + "context" "encoding/json" "fmt" "io/ioutil" @@ -183,22 +184,11 @@ func List(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, e error) { } dm := pm.DiscoveryManager() - if errs := dm.RunAll(); len(errs) > 0 { - return nil, &arduino.UnavailableError{Message: tr("Error starting board discoveries"), Cause: fmt.Errorf("%v", errs)} - } - if errs := dm.StartAll(); len(errs) > 0 { - return nil, &arduino.UnavailableError{Message: tr("Error starting board discoveries"), Cause: fmt.Errorf("%v", errs)} - } - defer func() { - if errs := dm.StopAll(); len(errs) > 0 { - logrus.Error(errs) - } - }() + dm.Start() time.Sleep(time.Duration(req.GetTimeout()) * time.Millisecond) retVal := []*rpc.DetectedPort{} - ports, errs := pm.DiscoveryManager().List() - for _, port := range ports { + for _, port := range dm.List() { boards, err := identify(pm, port) if err != nil { return nil, err @@ -212,92 +202,49 @@ func List(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, e error) { } retVal = append(retVal, b) } - if len(errs) > 0 { - return retVal, &arduino.UnavailableError{Message: tr("Error getting board list"), Cause: fmt.Errorf("%v", errs)} - } return retVal, nil } // Watch returns a channel that receives boards connection and disconnection events. -// The discovery process can be interrupted by sending a message to the interrupt channel. -func Watch(instanceID int32, interrupt <-chan bool) (<-chan *rpc.BoardListWatchResponse, error) { +// It also returns a callback function that must be used to stop and dispose the watch. +func Watch(instanceID int32) (<-chan *rpc.BoardListWatchResponse, func(), error) { pm := commands.GetPackageManager(instanceID) dm := pm.DiscoveryManager() - runErrs := dm.RunAll() - if len(runErrs) == len(dm.IDs()) { - // All discoveries failed to run, we can't do anything - return nil, &arduino.UnavailableError{Message: tr("Error starting board discoveries"), Cause: fmt.Errorf("%v", runErrs)} + watcher, err := dm.Watch() + if err != nil { + return nil, nil, err } - eventsChan, errs := dm.StartSyncAll() - if len(runErrs) > 0 { - errs = append(runErrs, errs...) - } + ctx, cancel := context.WithCancel(context.Background()) + go func() { + <-ctx.Done() + watcher.Close() + }() outChan := make(chan *rpc.BoardListWatchResponse) - go func() { defer close(outChan) - for _, err := range errs { - outChan <- &rpc.BoardListWatchResponse{ - EventType: "error", - Error: err.Error(), + for event := range watcher.Feed() { + port := &rpc.DetectedPort{ + Port: event.Port.ToRPC(), } - } - for { - select { - case event := <-eventsChan: - if event.Type == "quit" { - // The discovery manager has closed its event channel because it's - // quitting all the discovery processes that are running, this - // means that the events channel we're listening from won't receive any - // more events. - // Handling this case is necessary when the board watcher is running and - // the instance being used is reinitialized since that quits all the - // discovery processes and reset the discovery manager. That would leave - // this goroutine listening forever on a "dead" channel and might even - // cause panics. - // This message avoid all this issues. - // It will be the client's task restarting the board watcher if necessary, - // this host won't attempt restarting it. - outChan <- &rpc.BoardListWatchResponse{ - EventType: event.Type, - } - return - } - - port := &rpc.DetectedPort{ - Port: event.Port.ToRPC(), - } - boardsError := "" - if event.Type == "add" { - boards, err := identify(pm, event.Port) - if err != nil { - boardsError = err.Error() - } - port.MatchingBoards = boards + boardsError := "" + if event.Type == "add" { + boards, err := identify(pm, event.Port) + if err != nil { + boardsError = err.Error() } - outChan <- &rpc.BoardListWatchResponse{ - EventType: event.Type, - Port: port, - Error: boardsError, - } - case <-interrupt: - for _, err := range dm.StopAll() { - // Discoveries that return errors have their process - // closed and are removed from the list of discoveries - // in the manager - outChan <- &rpc.BoardListWatchResponse{ - EventType: "error", - Error: tr("stopping discoveries: %s", err), - } - } - return + port.MatchingBoards = boards + } + outChan <- &rpc.BoardListWatchResponse{ + EventType: event.Type, + Port: port, + Error: boardsError, } } }() - return outChan, nil + return outChan, cancel, nil } diff --git a/commands/daemon/daemon.go b/commands/daemon/daemon.go index a82656bbe70..03045c87a78 100644 --- a/commands/daemon/daemon.go +++ b/commands/daemon/daemon.go @@ -36,9 +36,7 @@ import ( "github.com/arduino/arduino-cli/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/sirupsen/logrus" - "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" - "google.golang.org/grpc/status" ) // ArduinoCoreServerImpl FIXMEDOC @@ -109,42 +107,35 @@ func (s *ArduinoCoreServerImpl) BoardListWatch(stream rpc.ArduinoCoreService_Boa return err } - interrupt := make(chan bool, 1) + eventsChan, closeWatcher, err := board.Watch(msg.Instance.Id) + if err != nil { + return convertErrorToRPCStatus(err) + } + go func() { - defer close(interrupt) + defer closeWatcher() for { msg, err := stream.Recv() // Handle client closing the stream and eventual errors if err == io.EOF { logrus.Info("boards watcher stream closed") - interrupt <- true - return - } else if st, ok := status.FromError(err); ok && st.Code() == codes.Canceled { - logrus.Info("boards watcher interrupted by host") return - } else if err != nil { + } + if err != nil { logrus.Infof("interrupting boards watcher: %v", err) - interrupt <- true return } // Message received, does the client want to interrupt? if msg != nil && msg.Interrupt { logrus.Info("boards watcher interrupted by client") - interrupt <- msg.Interrupt return } } }() - eventsChan, err := board.Watch(msg.Instance.Id, interrupt) - if err != nil { - return convertErrorToRPCStatus(err) - } - for event := range eventsChan { - err = stream.Send(event) - if err != nil { + if err := stream.Send(event); err != nil { logrus.Infof("sending board watch message: %v", err) } } From ec8e53ed4478696f5be9d4c289ef756fd67e84fe Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 13 Jul 2022 16:14:28 +0200 Subject: [PATCH 04/16] Added discovery id in discovery.Event struct --- arduino/discovery/discovery.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 40067cc3e43..37c7f750c0e 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -121,8 +121,9 @@ func (p *Port) String() string { // Event is a pluggable discovery event type Event struct { - Type string - Port *Port + Type string + Port *Port + DiscoveryID string } // New create and connect to the given pluggable discovery @@ -178,7 +179,7 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis disc.statusMutex.Lock() disc.cachedPorts[msg.Port.Address+"|"+msg.Port.Protocol] = msg.Port if disc.eventChan != nil { - disc.eventChan <- &Event{"add", msg.Port} + disc.eventChan <- &Event{"add", msg.Port, disc.GetID()} } disc.statusMutex.Unlock() } else if msg.EventType == "remove" { @@ -189,7 +190,7 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis disc.statusMutex.Lock() delete(disc.cachedPorts, msg.Port.Address+"|"+msg.Port.Protocol) if disc.eventChan != nil { - disc.eventChan <- &Event{"remove", msg.Port} + disc.eventChan <- &Event{"remove", msg.Port, disc.GetID()} } disc.statusMutex.Unlock() } else { From 76ceeccd791a594b08716802f80bf62e2d0086a4 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 13 Jul 2022 16:15:52 +0200 Subject: [PATCH 05/16] Cache active ports and transmit them when a new watcher connects --- .../discoverymanager/discoverymanager.go | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index e6857e77de0..f5036502101 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -34,6 +34,7 @@ type DiscoveryManager struct { feed chan *discovery.Event watchersMutex sync.Mutex watchers map[*PortWatcher]bool + watchersCache map[string]map[string]*discovery.Event } var tr = i18n.Tr @@ -41,9 +42,10 @@ var tr = i18n.Tr // New creates a new DiscoveryManager func New() *DiscoveryManager { return &DiscoveryManager{ - discoveries: map[string]*discovery.PluggableDiscovery{}, - watchers: map[*PortWatcher]bool{}, - feed: make(chan *discovery.Event, 50), + discoveries: map[string]*discovery.PluggableDiscovery{}, + watchers: map[*PortWatcher]bool{}, + feed: make(chan *discovery.Event, 50), + watchersCache: map[string]map[string]*discovery.Event{}, } } @@ -139,9 +141,16 @@ func (dm *DiscoveryManager) Watch() (*PortWatcher, error) { dm.watchersMutex.Unlock() close(watcher.feed) } - dm.watchersMutex.Lock() - dm.watchers[watcher] = true - dm.watchersMutex.Unlock() + go func() { + dm.watchersMutex.Lock() + for _, cache := range dm.watchersCache { + for _, ev := range cache { + watcher.feed <- ev + } + } + dm.watchers[watcher] = true + dm.watchersMutex.Unlock() + }() return watcher, nil } @@ -188,19 +197,35 @@ func (dm *DiscoveryManager) feeder() { } func (dm *DiscoveryManager) cacheEvent(ev *discovery.Event) { - // XXX: TODO + cache := dm.watchersCache[ev.DiscoveryID] + if cache == nil { + cache = map[string]*discovery.Event{} + dm.watchersCache[ev.DiscoveryID] = cache + } + + eventID := ev.Port.Address + "|" + ev.Port.Protocol + switch ev.Type { + case "add": + cache[eventID] = ev + case "remove": + delete(cache, eventID) + default: + logrus.Errorf("Unhandled event from discovery: %s", ev.Type) + return + } } // List return the current list of ports detected from all discoveries func (dm *DiscoveryManager) List() []*discovery.Port { dm.Start() - // XXX: Cache ports and return them - dm.discoveriesMutex.Lock() - defer dm.discoveriesMutex.Unlock() res := []*discovery.Port{} - for _, d := range dm.discoveries { - res = append(res, d.ListCachedPorts()...) + dm.watchersMutex.Lock() + defer dm.watchersMutex.Unlock() + for _, cache := range dm.watchersCache { + for _, ev := range cache { + res = append(res, ev.Port) + } } return res } From d72db2f1cab173d35582e1bc43cdeca9018cfd15 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 13 Jul 2022 23:48:06 +0200 Subject: [PATCH 06/16] Correctly handle discovery cleanup --- arduino/discovery/discovery.go | 24 +++++++++---------- .../discoverymanager/discoverymanager.go | 14 +++++++---- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 37c7f750c0e..4f77a6290ed 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -277,10 +277,7 @@ func (disc *PluggableDiscovery) killProcess() error { } disc.statusMutex.Lock() defer disc.statusMutex.Unlock() - if disc.eventChan != nil { - close(disc.eventChan) - disc.eventChan = nil - } + disc.stopSync() disc.state = Dead logrus.Infof("killed discovery %s process", disc.id) return nil @@ -367,13 +364,18 @@ func (disc *PluggableDiscovery) Stop() error { } disc.statusMutex.Lock() defer disc.statusMutex.Unlock() - disc.cachedPorts = map[string]*Port{} + disc.stopSync() + disc.state = Idling + return nil +} + +func (disc *PluggableDiscovery) stopSync() { if disc.eventChan != nil { + disc.eventChan <- &Event{"stop", nil, disc.GetID()} close(disc.eventChan) disc.eventChan = nil + disc.cachedPorts = map[string]*Port{} } - disc.state = Idling - return nil } // Quit terminates the discovery. No more commands can be accepted by the discovery. @@ -427,12 +429,8 @@ func (disc *PluggableDiscovery) StartSync(size int) (<-chan *Event, error) { disc.statusMutex.Lock() defer disc.statusMutex.Unlock() disc.state = Syncing - disc.cachedPorts = map[string]*Port{} - if disc.eventChan != nil { - // In case there is already an existing event channel in use we close it - // before creating a new one. - close(disc.eventChan) - } + // In case there is already an existing event channel in use we close it before creating a new one. + disc.stopSync() c := make(chan *Event, size) disc.eventChan = c return c, nil diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index f5036502101..8636654d5aa 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -52,12 +52,15 @@ func New() *DiscoveryManager { // Clear resets the DiscoveryManager to its initial state func (dm *DiscoveryManager) Clear() { dm.discoveriesMutex.Lock() - for _, d := range dm.discoveries { - d.Quit() - logrus.Infof("Closed and removed discovery %s", d.GetID()) + defer dm.discoveriesMutex.Unlock() + + if dm.discoveriesRunning { + for _, d := range dm.discoveries { + d.Quit() + logrus.Infof("Closed and removed discovery %s", d.GetID()) + } } dm.discoveries = map[string]*discovery.PluggableDiscovery{} - dm.discoveriesMutex.Unlock() } // IDs returns the list of discoveries' ids in this DiscoveryManager @@ -209,6 +212,9 @@ func (dm *DiscoveryManager) cacheEvent(ev *discovery.Event) { cache[eventID] = ev case "remove": delete(cache, eventID) + case "quit": + // Remove all the events for this discovery + delete(dm.watchersCache, ev.DiscoveryID) default: logrus.Errorf("Unhandled event from discovery: %s", ev.Type) return From 907468348c5a60b69aa1b943ea3e8c1d01aeed51 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 14 Jul 2022 00:10:50 +0200 Subject: [PATCH 07/16] Fixed wrong test --- .../cores/packagemanager/package_manager_test.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/arduino/cores/packagemanager/package_manager_test.go b/arduino/cores/packagemanager/package_manager_test.go index ec12a6070b9..9cb1509dca0 100644 --- a/arduino/cores/packagemanager/package_manager_test.go +++ b/arduino/cores/packagemanager/package_manager_test.go @@ -329,16 +329,14 @@ func TestPackageManagerClear(t *testing.T) { packageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test") packageManager.LoadHardwareFromDirectory(customHardware) - // Creates another PackageManager but don't load the hardware - emptyPackageManager := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware, "test") + // Check that the hardware is loaded + require.NotEmpty(t, packageManager.Packages) - // Verifies they're not equal - require.NotEqual(t, packageManager, emptyPackageManager) - - // Clear the first PackageManager that contains loaded hardware + // Clear the package manager packageManager.Clear() - // Verifies both PackageManagers are now equal - require.Equal(t, packageManager, emptyPackageManager) + + // Check that the hardware is cleared + require.Empty(t, packageManager.Packages) } func TestFindToolsRequiredFromPlatformRelease(t *testing.T) { From 851957ddf990fab66a037ccee84f2a0aeef377a4 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 15 Jul 2022 01:32:34 +0200 Subject: [PATCH 08/16] Correctly handle discovery cleanup and re-add --- arduino/discovery/discovery.go | 5 +- .../discoverymanager/discoverymanager.go | 55 ++++++++++--------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 4f77a6290ed..ead70adecc6 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -371,10 +371,13 @@ func (disc *PluggableDiscovery) Stop() error { func (disc *PluggableDiscovery) stopSync() { if disc.eventChan != nil { + for _, port := range disc.cachedPorts { + disc.eventChan <- &Event{"remove", port, disc.GetID()} + } + disc.cachedPorts = map[string]*Port{} disc.eventChan <- &Event{"stop", nil, disc.GetID()} close(disc.eventChan) disc.eventChan = nil - disc.cachedPorts = map[string]*Port{} } } diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index 8636654d5aa..bc418f215c4 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -18,6 +18,7 @@ package discoverymanager import ( "fmt" "sync" + "time" "github.com/arduino/arduino-cli/arduino/discovery" "github.com/arduino/arduino-cli/i18n" @@ -83,7 +84,12 @@ func (dm *DiscoveryManager) Start() { return } - go dm.feeder() + go func() { + // Feed all watchers with data coming from the discoveries + for ev := range dm.feed { + dm.feedEvent(ev) + } + }() var wg sync.WaitGroup for _, d := range dm.discoveries { @@ -136,13 +142,13 @@ func (dm *DiscoveryManager) Watch() (*PortWatcher, error) { dm.Start() watcher := &PortWatcher{ - feed: make(chan *discovery.Event), + feed: make(chan *discovery.Event, 10), } watcher.closeCB = func() { dm.watchersMutex.Lock() delete(dm.watchers, watcher) - dm.watchersMutex.Unlock() close(watcher.feed) + dm.watchersMutex.Unlock() } go func() { dm.watchersMutex.Lock() @@ -180,44 +186,43 @@ func (dm *DiscoveryManager) startDiscovery(d *discovery.PluggableDiscovery) (dis return nil } -func (dm *DiscoveryManager) feeder() { - // Feed all watchers with data coming from the discoveries - for ev := range dm.feed { - dm.watchersMutex.Lock() - for watcher := range dm.watchers { - select { - case watcher.feed <- ev: - // OK - default: - // If the watcher is not able to process event fast enough - // remove the watcher from the list of watchers - go watcher.Close() - } +func (dm *DiscoveryManager) feedEvent(ev *discovery.Event) { + dm.watchersMutex.Lock() + defer dm.watchersMutex.Unlock() + + if ev.Type == "stop" { + // Remove all the cached events for the terminating discovery + delete(dm.watchersCache, ev.DiscoveryID) + return + } + + // Send the event to all watchers + for watcher := range dm.watchers { + select { + case watcher.feed <- ev: + // OK + case <-time.After(time.Millisecond * 500): + // If the watcher is not able to process event fast enough + // remove the watcher from the list of watchers + logrus.Info("Watcher is not able to process events fast enough, removing it from the list of watchers") + delete(dm.watchers, watcher) } - dm.cacheEvent(ev) - dm.watchersMutex.Unlock() } -} -func (dm *DiscoveryManager) cacheEvent(ev *discovery.Event) { + // Cache the event for the discovery cache := dm.watchersCache[ev.DiscoveryID] if cache == nil { cache = map[string]*discovery.Event{} dm.watchersCache[ev.DiscoveryID] = cache } - eventID := ev.Port.Address + "|" + ev.Port.Protocol switch ev.Type { case "add": cache[eventID] = ev case "remove": delete(cache, eventID) - case "quit": - // Remove all the events for this discovery - delete(dm.watchersCache, ev.DiscoveryID) default: logrus.Errorf("Unhandled event from discovery: %s", ev.Type) - return } } From b1f6d0edba7fe6a98fea39903f52ca7d2db1c337 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 20 Jul 2022 16:35:20 +0200 Subject: [PATCH 09/16] Added some doc comments in the source code --- arduino/discovery/discovery.go | 2 ++ .../discoverymanager/discoverymanager.go | 24 ++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index ead70adecc6..62b813f5f02 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -371,6 +371,8 @@ func (disc *PluggableDiscovery) Stop() error { func (disc *PluggableDiscovery) stopSync() { if disc.eventChan != nil { + // When stopping sync send a batch of "remove" events for + // all the active ports. for _, port := range disc.cachedPorts { disc.eventChan <- &Event{"remove", port, disc.GetID()} } diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index bc418f215c4..381f2d11207 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -26,16 +26,20 @@ import ( "github.com/sirupsen/logrus" ) -// DiscoveryManager is required to handle multiple pluggable-discovery that -// may be shared across platforms +// DiscoveryManager manages the many-to-many communication between all pluggable +// discoveries and all watchers. Each PluggableDiscovery, once started, will +// produce a sequence of "events". These events will be broadcasted to all +// listening Watcher. +// The DiscoveryManager will not start the discoveries until the Start method +// is called. type DiscoveryManager struct { discoveriesMutex sync.Mutex - discoveries map[string]*discovery.PluggableDiscovery - discoveriesRunning bool - feed chan *discovery.Event + discoveries map[string]*discovery.PluggableDiscovery // all registered PluggableDiscovery + discoveriesRunning bool // set to true once discoveries are started + feed chan *discovery.Event // all events will pass through this channel watchersMutex sync.Mutex - watchers map[*PortWatcher]bool - watchersCache map[string]map[string]*discovery.Event + watchers map[*PortWatcher]bool // all registered Watcher + watchersCache map[string]map[string]*discovery.Event // this is a cache of all active ports } var tr = i18n.Tr @@ -85,7 +89,7 @@ func (dm *DiscoveryManager) Start() { } go func() { - // Feed all watchers with data coming from the discoveries + // Send all events coming from the feed channel to all active watchers for ev := range dm.feed { dm.feedEvent(ev) } @@ -152,11 +156,13 @@ func (dm *DiscoveryManager) Watch() (*PortWatcher, error) { } go func() { dm.watchersMutex.Lock() + // When a watcher is started, send all the current active ports first... for _, cache := range dm.watchersCache { for _, ev := range cache { watcher.feed <- ev } } + // ...and after that add the watcher to the list of watchers receiving events dm.watchers[watcher] = true dm.watchersMutex.Unlock() }() @@ -165,6 +171,7 @@ func (dm *DiscoveryManager) Watch() (*PortWatcher, error) { func (dm *DiscoveryManager) startDiscovery(d *discovery.PluggableDiscovery) (discErr error) { defer func() { + // If this function returns an error log it if discErr != nil { logrus.Errorf("Discovery %s failed to run: %s", d.GetID(), discErr) } @@ -179,6 +186,7 @@ func (dm *DiscoveryManager) startDiscovery(d *discovery.PluggableDiscovery) (dis } go func() { + // Transfer all incoming events from this discovery to the feed channel for ev := range eventCh { dm.feed <- ev } From 32aa13882481615cd88c80985c2f5f2be1bbfb87 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 20 Jul 2022 16:41:22 +0200 Subject: [PATCH 10/16] Move Unlock under defer --- arduino/discovery/discoverymanager/discoverymanager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index 381f2d11207..54c9ce77176 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -150,9 +150,9 @@ func (dm *DiscoveryManager) Watch() (*PortWatcher, error) { } watcher.closeCB = func() { dm.watchersMutex.Lock() + defer dm.watchersMutex.Unlock() delete(dm.watchers, watcher) close(watcher.feed) - dm.watchersMutex.Unlock() } go func() { dm.watchersMutex.Lock() From 5330736ddc3b3ae827d03af8dae612559304ff08 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 4 Aug 2022 14:26:28 +0200 Subject: [PATCH 11/16] Factored subrotuine into a function it will be useful in the next commits. --- .../discoverymanager/discoverymanager.go | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index 54c9ce77176..3ef6d3f58aa 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -198,24 +198,28 @@ func (dm *DiscoveryManager) feedEvent(ev *discovery.Event) { dm.watchersMutex.Lock() defer dm.watchersMutex.Unlock() + sendToAllWatchers := func(ev *discovery.Event) { + // Send the event to all watchers + for watcher := range dm.watchers { + select { + case watcher.feed <- ev: + // OK + case <-time.After(time.Millisecond * 500): + // If the watcher is not able to process event fast enough + // remove the watcher from the list of watchers + logrus.Info("Watcher is not able to process events fast enough, removing it from the list of watchers") + delete(dm.watchers, watcher) + } + } + } + if ev.Type == "stop" { // Remove all the cached events for the terminating discovery delete(dm.watchersCache, ev.DiscoveryID) return } - // Send the event to all watchers - for watcher := range dm.watchers { - select { - case watcher.feed <- ev: - // OK - case <-time.After(time.Millisecond * 500): - // If the watcher is not able to process event fast enough - // remove the watcher from the list of watchers - logrus.Info("Watcher is not able to process events fast enough, removing it from the list of watchers") - delete(dm.watchers, watcher) - } - } + sendToAllWatchers(ev) // Cache the event for the discovery cache := dm.watchersCache[ev.DiscoveryID] From 983353457242cd0b5680109489a5324dd68f1117 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 4 Aug 2022 14:34:18 +0200 Subject: [PATCH 12/16] Do not cache ports in the DiscoveryClient there is already a cache in the DiscoveryManager there is no need to duplicate it. --- arduino/discovery/discovery.go | 23 ------------------- .../discoverymanager/discoverymanager.go | 17 +++++++++++++- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 62b813f5f02..64e45ec7424 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -57,7 +57,6 @@ type PluggableDiscovery struct { incomingMessagesError error state int eventChan chan<- *Event - cachedPorts map[string]*Port } type discoveryMessage struct { @@ -132,7 +131,6 @@ func New(id string, args ...string) *PluggableDiscovery { id: id, processArgs: args, state: Dead, - cachedPorts: map[string]*Port{}, } } @@ -177,7 +175,6 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis return } disc.statusMutex.Lock() - disc.cachedPorts[msg.Port.Address+"|"+msg.Port.Protocol] = msg.Port if disc.eventChan != nil { disc.eventChan <- &Event{"add", msg.Port, disc.GetID()} } @@ -188,7 +185,6 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis return } disc.statusMutex.Lock() - delete(disc.cachedPorts, msg.Port.Address+"|"+msg.Port.Protocol) if disc.eventChan != nil { disc.eventChan <- &Event{"remove", msg.Port, disc.GetID()} } @@ -371,12 +367,6 @@ func (disc *PluggableDiscovery) Stop() error { func (disc *PluggableDiscovery) stopSync() { if disc.eventChan != nil { - // When stopping sync send a batch of "remove" events for - // all the active ports. - for _, port := range disc.cachedPorts { - disc.eventChan <- &Event{"remove", port, disc.GetID()} - } - disc.cachedPorts = map[string]*Port{} disc.eventChan <- &Event{"stop", nil, disc.GetID()} close(disc.eventChan) disc.eventChan = nil @@ -440,16 +430,3 @@ func (disc *PluggableDiscovery) StartSync(size int) (<-chan *Event, error) { disc.eventChan = c return c, nil } - -// ListCachedPorts returns a list of the available ports. The list is a cache of all the -// add/remove events happened from the StartSync call and it will not consume any -// resource from the underliying discovery. -func (disc *PluggableDiscovery) ListCachedPorts() []*Port { - disc.statusMutex.Lock() - defer disc.statusMutex.Unlock() - res := []*Port{} - for _, port := range disc.cachedPorts { - res = append(res, port) - } - return res -} diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index 3ef6d3f58aa..ccbd433038e 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -214,7 +214,22 @@ func (dm *DiscoveryManager) feedEvent(ev *discovery.Event) { } if ev.Type == "stop" { - // Remove all the cached events for the terminating discovery + // Send remove events for all the cached ports of the terminating discovery + cache := dm.watchersCache[ev.DiscoveryID] + for _, addEv := range cache { + removeEv := &discovery.Event{ + Type: "remove", + Port: &discovery.Port{ + Address: addEv.Port.Address, + AddressLabel: addEv.Port.AddressLabel, + Protocol: addEv.Port.Protocol, + ProtocolLabel: addEv.Port.ProtocolLabel}, + DiscoveryID: addEv.DiscoveryID, + } + sendToAllWatchers(removeEv) + } + + // Remove the cache for the terminating discovery delete(dm.watchersCache, ev.DiscoveryID) return } From 2e9eb5f5a50302e5c8880cc3065def70828be0ec Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 8 Aug 2022 17:20:01 +0200 Subject: [PATCH 13/16] Discovery: eventChan must be protected by mutex when doing START_SYNC otherwise the discovery may send some events before the eventChan is setup (and those events will be lost) --- arduino/discovery/discovery.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 64e45ec7424..9def18596e7 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -407,6 +407,9 @@ func (disc *PluggableDiscovery) List() ([]*Port, error) { // The event channel must be consumed as quickly as possible since it may block the // discovery if it becomes full. The channel size is configurable. func (disc *PluggableDiscovery) StartSync(size int) (<-chan *Event, error) { + disc.statusMutex.Lock() + defer disc.statusMutex.Unlock() + if err := disc.sendCommand("START_SYNC\n"); err != nil { return nil, err } @@ -421,8 +424,6 @@ func (disc *PluggableDiscovery) StartSync(size int) (<-chan *Event, error) { return nil, errors.Errorf(tr("communication out of sync, expected '%[1]s', received '%[2]s'"), "OK", msg.Message) } - disc.statusMutex.Lock() - defer disc.statusMutex.Unlock() disc.state = Syncing // In case there is already an existing event channel in use we close it before creating a new one. disc.stopSync() From 285452f3a51550935b0809824035b27bc09b8a16 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 8 Aug 2022 17:21:40 +0200 Subject: [PATCH 14/16] Increased error level for logging watchers that lags --- arduino/discovery/discoverymanager/discoverymanager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index ccbd433038e..40928544948 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -207,7 +207,7 @@ func (dm *DiscoveryManager) feedEvent(ev *discovery.Event) { case <-time.After(time.Millisecond * 500): // If the watcher is not able to process event fast enough // remove the watcher from the list of watchers - logrus.Info("Watcher is not able to process events fast enough, removing it from the list of watchers") + logrus.Error("Watcher is not able to process events fast enough, removing it from the list of watchers") delete(dm.watchers, watcher) } } From 1be67efa8aaab35a7da648af8bd9ca31ed46539d Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 8 Aug 2022 20:00:51 +0200 Subject: [PATCH 15/16] Updated discvoery_client to the latest API --- arduino/discovery/discovery_client/go.mod | 2 +- arduino/discovery/discovery_client/main.go | 73 ++++++++++------------ 2 files changed, 34 insertions(+), 41 deletions(-) diff --git a/arduino/discovery/discovery_client/go.mod b/arduino/discovery/discovery_client/go.mod index 840449b3c36..54950bcd7c9 100644 --- a/arduino/discovery/discovery_client/go.mod +++ b/arduino/discovery/discovery_client/go.mod @@ -7,6 +7,7 @@ replace github.com/arduino/arduino-cli => ../../.. require ( github.com/arduino/arduino-cli v0.0.0-00010101000000-000000000000 github.com/gizak/termui/v3 v3.1.0 + github.com/sirupsen/logrus v1.4.2 ) require ( @@ -19,7 +20,6 @@ require ( github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7 // indirect github.com/nsf/termbox-go v0.0.0-20190121233118-02980233997d // indirect github.com/pkg/errors v0.9.1 // indirect - github.com/sirupsen/logrus v1.4.2 // indirect golang.org/x/net v0.0.0-20210505024714-0287a6fb4125 // indirect golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf // indirect golang.org/x/text v0.3.6 // indirect diff --git a/arduino/discovery/discovery_client/main.go b/arduino/discovery/discovery_client/main.go index 2f033a604a3..8306da7462e 100644 --- a/arduino/discovery/discovery_client/main.go +++ b/arduino/discovery/discovery_client/main.go @@ -21,36 +21,28 @@ import ( "log" "os" "sort" - "time" "github.com/arduino/arduino-cli/arduino/discovery" + "github.com/arduino/arduino-cli/arduino/discovery/discoverymanager" ui "github.com/gizak/termui/v3" "github.com/gizak/termui/v3/widgets" + "github.com/sirupsen/logrus" ) func main() { - discoveries := []*discovery.PluggableDiscovery{} - discEvent := make(chan *discovery.Event) + logrus.SetLevel(logrus.ErrorLevel) + dm := discoverymanager.New() for _, discCmd := range os.Args[1:] { - disc := discovery.New("", discCmd) - if err := disc.Run(); err != nil { - log.Fatal("Error starting discovery:", err) - } - if err := disc.Start(); err != nil { - log.Fatal("Error starting discovery:", err) - } - eventChan, err := disc.StartSync(10) - if err != nil { - log.Fatal("Error starting discovery:", err) - } - go func() { - for msg := range eventChan { - discEvent <- msg - } - }() - discoveries = append(discoveries, disc) + disc := discovery.New(discCmd, discCmd) + dm.Add(disc) } + dm.Start() + activePorts := map[string]*discovery.Port{} + watcher, err := dm.Watch() + if err != nil { + log.Fatalf("failed to start discvoeries: %v", err) + } if err := ui.Init(); err != nil { log.Fatalf("failed to initialize termui: %v", err) } @@ -66,15 +58,20 @@ func main() { updateList := func() { rows := []string{} rows = append(rows, "Available ports list:") - for _, disc := range discoveries { - for i, port := range disc.ListCachedPorts() { - rows = append(rows, fmt.Sprintf(" [%04d] Address: %s", i, port.AddressLabel)) - rows = append(rows, fmt.Sprintf(" Protocol: %s", port.ProtocolLabel)) - keys := port.Properties.Keys() - sort.Strings(keys) - for _, k := range keys { - rows = append(rows, fmt.Sprintf(" %s=%s", k, port.Properties.Get(k))) - } + + ids := sort.StringSlice{} + for id := range activePorts { + ids = append(ids, id) + } + ids.Sort() + for _, id := range ids { + port := activePorts[id] + rows = append(rows, fmt.Sprintf("> Address: %s", port.AddressLabel)) + rows = append(rows, fmt.Sprintf(" Protocol: %s", port.ProtocolLabel)) + keys := port.Properties.Keys() + sort.Strings(keys) + for _, k := range keys { + rows = append(rows, fmt.Sprintf(" %s=%s", k, port.Properties.Get(k))) } } l.Rows = rows @@ -123,20 +120,16 @@ out: previousKey = e.ID } - case <-discEvent: + case ev := <-watcher.Feed(): + if ev.Type == "add" { + activePorts[ev.Port.Address+"|"+ev.Port.Protocol] = ev.Port + } + if ev.Type == "remove" { + delete(activePorts, ev.Port.Address+"|"+ev.Port.Protocol) + } updateList() } ui.Render(l) } - - for _, disc := range discoveries { - disc.Quit() - fmt.Println("Discovery QUITed") - for disc.State() == discovery.Alive { - time.Sleep(time.Millisecond) - } - fmt.Println("Discovery correctly terminated") - } - } From 52499a9792e8a6561a4a74171ef35e9c3cfffcbb Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 8 Aug 2022 23:32:41 +0200 Subject: [PATCH 16/16] Report discovery start errors --- .../discoverymanager/discoverymanager.go | 15 ++++++++++++--- cli/arguments/completion.go | 2 +- cli/arguments/port.go | 2 +- cli/board/list.go | 5 ++++- commands/board/list.go | 10 +++++----- commands/daemon/daemon.go | 2 +- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/arduino/discovery/discoverymanager/discoverymanager.go b/arduino/discovery/discoverymanager/discoverymanager.go index 40928544948..5cf3f2d3aa1 100644 --- a/arduino/discovery/discoverymanager/discoverymanager.go +++ b/arduino/discovery/discoverymanager/discoverymanager.go @@ -81,11 +81,11 @@ func (dm *DiscoveryManager) IDs() []string { // Start starts all the discoveries in this DiscoveryManager. // If the discoveries are already running, this function does nothing. -func (dm *DiscoveryManager) Start() { +func (dm *DiscoveryManager) Start() []error { dm.discoveriesMutex.Lock() defer dm.discoveriesMutex.Unlock() if dm.discoveriesRunning { - return + return nil } go func() { @@ -95,16 +95,25 @@ func (dm *DiscoveryManager) Start() { } }() + errs := []error{} + var errsLock sync.Mutex + var wg sync.WaitGroup for _, d := range dm.discoveries { wg.Add(1) go func(d *discovery.PluggableDiscovery) { - dm.startDiscovery(d) + if err := dm.startDiscovery(d); err != nil { + errsLock.Lock() + errs = append(errs, err) + errsLock.Unlock() + } wg.Done() }(d) } wg.Wait() dm.discoveriesRunning = true + + return errs } // Add adds a discovery to the list of managed discoveries diff --git a/cli/arguments/completion.go b/cli/arguments/completion.go index 34674e8e57b..32ab99f407b 100644 --- a/cli/arguments/completion.go +++ b/cli/arguments/completion.go @@ -178,7 +178,7 @@ func GetInstallableLibs() []string { func GetConnectedBoards() []string { inst := instance.CreateAndInit() - list, _ := board.List(&rpc.BoardListRequest{ + list, _, _ := board.List(&rpc.BoardListRequest{ Instance: inst, }) var res []string diff --git a/cli/arguments/port.go b/cli/arguments/port.go index d77215dcf58..1e054e713b4 100644 --- a/cli/arguments/port.go +++ b/cli/arguments/port.go @@ -146,7 +146,7 @@ func (p *Port) GetSearchTimeout() time.Duration { // discovered Port object together with the FQBN. If the port does not match // exactly 1 board, func (p *Port) DetectFQBN(inst *rpc.Instance) (string, *rpc.Port) { - detectedPorts, err := board.List(&rpc.BoardListRequest{ + detectedPorts, _, err := board.List(&rpc.BoardListRequest{ Instance: inst, Timeout: p.timeout.Get().Milliseconds(), }) diff --git a/cli/board/list.go b/cli/board/list.go index b162e2a6f79..fe82973847a 100644 --- a/cli/board/list.go +++ b/cli/board/list.go @@ -64,13 +64,16 @@ func runListCommand(cmd *cobra.Command, args []string) { os.Exit(0) } - ports, err := board.List(&rpc.BoardListRequest{ + ports, discvoeryErrors, err := board.List(&rpc.BoardListRequest{ Instance: inst, Timeout: timeoutArg.Get().Milliseconds(), }) if err != nil { feedback.Errorf(tr("Error detecting boards: %v"), err) } + for _, err := range discvoeryErrors { + feedback.Errorf(tr("Error starting discovery: %v"), err) + } feedback.PrintResult(result{ports}) } diff --git a/commands/board/list.go b/commands/board/list.go index 84e7149b88f..fe9b4afeb09 100644 --- a/commands/board/list.go +++ b/commands/board/list.go @@ -177,21 +177,21 @@ func identify(pm *packagemanager.PackageManager, port *discovery.Port) ([]*rpc.B // List returns a list of boards found by the loaded discoveries. // In case of errors partial results from discoveries that didn't fail // are returned. -func List(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, e error) { +func List(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, discoveryStartErrors []error, e error) { pm := commands.GetPackageManager(req.GetInstance().Id) if pm == nil { - return nil, &arduino.InvalidInstanceError{} + return nil, nil, &arduino.InvalidInstanceError{} } dm := pm.DiscoveryManager() - dm.Start() + discoveryStartErrors = dm.Start() time.Sleep(time.Duration(req.GetTimeout()) * time.Millisecond) retVal := []*rpc.DetectedPort{} for _, port := range dm.List() { boards, err := identify(pm, port) if err != nil { - return nil, err + return nil, discoveryStartErrors, err } // boards slice can be empty at this point if neither the cores nor the @@ -202,7 +202,7 @@ func List(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, e error) { } retVal = append(retVal, b) } - return retVal, nil + return retVal, discoveryStartErrors, nil } // Watch returns a channel that receives boards connection and disconnection events. diff --git a/commands/daemon/daemon.go b/commands/daemon/daemon.go index 03045c87a78..96ec5daed6e 100644 --- a/commands/daemon/daemon.go +++ b/commands/daemon/daemon.go @@ -67,7 +67,7 @@ func (s *ArduinoCoreServerImpl) BoardDetails(ctx context.Context, req *rpc.Board // BoardList FIXMEDOC func (s *ArduinoCoreServerImpl) BoardList(ctx context.Context, req *rpc.BoardListRequest) (*rpc.BoardListResponse, error) { - ports, err := board.List(req) + ports, _, err := board.List(req) if err != nil { return nil, convertErrorToRPCStatus(err) }