diff --git a/commands/core/install.go b/commands/core/install.go index dc8d761f696..e421a574261 100644 --- a/commands/core/install.go +++ b/commands/core/install.go @@ -98,19 +98,34 @@ func installPlatform(pm *packagemanager.PackageManager, for _, tool := range toolsToInstall { err := commands.InstallToolRelease(pm, tool, taskCB) if err != nil { - // TODO: handle error + return err } } - // Are we installing or upgrading? - platform := platformRelease.Platform - installed := pm.GetInstalledPlatformRelease(platform) + 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: "Installing " + platformRelease.String()}) } else { - log.Info("Updating platform " + installed.String()) - taskCB(&rpc.TaskProgress{Name: "Updating " + installed.String() + " with " + platformRelease.String()}) + // A platform with a different version is already installed + log.Info("Upgrading platform " + installed.String()) + taskCB(&rpc.TaskProgress{Name: "Upgrading " + installed.String() + " with " + platformRelease.String()}) + 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 fmt.Errorf("can't find dependencies for platform %s: %w", platformRef, err) + } } // Install @@ -126,8 +141,8 @@ func installPlatform(pm *packagemanager.PackageManager, // In case of error try to rollback if errUn != nil { - log.WithError(errUn).Error("Error updating platform.") - taskCB(&rpc.TaskProgress{Message: "Error updating platform: " + err.Error()}) + log.WithError(errUn).Error("Error upgrading platform.") + taskCB(&rpc.TaskProgress{Message: "Error upgrading platform: " + err.Error()}) // Rollback if err := pm.UninstallPlatform(platformRelease); err != nil { @@ -135,8 +150,16 @@ func installPlatform(pm *packagemanager.PackageManager, taskCB(&rpc.TaskProgress{Message: "Error rolling-back changes: " + err.Error()}) } - return fmt.Errorf("updating platform: %s", errUn) + return fmt.Errorf("upgrading platform: %s", errUn) } + + // Uninstall unused tools + for _, tool := range installedTools { + if !pm.IsToolRequired(tool) { + uninstallToolRelease(pm, tool, taskCB) + } + } + } // Perform post install diff --git a/commands/core/uninstall.go b/commands/core/uninstall.go index 036e52c1827..7695b3a5184 100644 --- a/commands/core/uninstall.go +++ b/commands/core/uninstall.go @@ -63,7 +63,7 @@ func PlatformUninstall(ctx context.Context, req *rpc.PlatformUninstallReq, taskC for _, tool := range tools { if !pm.IsToolRequired(tool) { - uninstallToolReleasse(pm, tool, taskCB) + uninstallToolRelease(pm, tool, taskCB) } } @@ -90,7 +90,7 @@ func uninstallPlatformRelease(pm *packagemanager.PackageManager, platformRelease return nil } -func uninstallToolReleasse(pm *packagemanager.PackageManager, toolRelease *cores.ToolRelease, taskCB commands.TaskProgressCB) error { +func uninstallToolRelease(pm *packagemanager.PackageManager, toolRelease *cores.ToolRelease, taskCB commands.TaskProgressCB) error { log := pm.Log.WithField("Tool", toolRelease) log.Info("Uninstalling tool") diff --git a/commands/core/upgrade.go b/commands/core/upgrade.go index 7b77f6e436b..45adc90a45c 100644 --- a/commands/core/upgrade.go +++ b/commands/core/upgrade.go @@ -64,7 +64,6 @@ func upgradePlatform(pm *packagemanager.PackageManager, platformRef *packagemana } // Search the latest version for all specified platforms - toInstallRefs := []*packagemanager.PlatformReference{} platform := pm.FindPlatform(platformRef) if platform == nil { return fmt.Errorf("platform %s not found", platformRef) @@ -78,17 +77,15 @@ func upgradePlatform(pm *packagemanager.PackageManager, platformRef *packagemana return ErrAlreadyLatest } platformRef.PlatformVersion = latest.Version - toInstallRefs = append(toInstallRefs, platformRef) - for _, platformRef := range toInstallRefs { - platform, tools, err := pm.FindPlatformReleaseDependencies(platformRef) - if err != nil { - return fmt.Errorf("platform %s is not installed", platformRef) - } - err = installPlatform(pm, platform, tools, downloadCB, taskCB, skipPostInstall) - if err != nil { - return err - } + platformRelease, tools, err := pm.FindPlatformReleaseDependencies(platformRef) + if err != nil { + return fmt.Errorf("platform %s is not installed", platformRef) + } + err = installPlatform(pm, platformRelease, tools, downloadCB, taskCB, skipPostInstall) + if err != nil { + return err } + return nil } diff --git a/commands/instances.go b/commands/instances.go index 89ec9e4bd90..31b7d7748b5 100644 --- a/commands/instances.go +++ b/commands/instances.go @@ -519,6 +519,17 @@ func Upgrade(ctx context.Context, req *rpc.UpgradeReq, downloadCB DownloadProgre } 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 err + } + + ref = &packagemanager.PlatformReference{ Package: latest.Platform.Package.Name, PlatformArchitecture: latest.Platform.Architecture, PlatformVersion: latest.Version, @@ -590,6 +601,24 @@ func Upgrade(ctx context.Context, req *rpc.UpgradeReq, downloadCB DownloadProgre } } + // 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: "Uninstalling " + toolRelease.String() + ", tool is no more required"}) + + if err := pm.UninstallTool(toolRelease); err != nil { + log.WithError(err).Error("Error uninstalling") + return err + } + + log.Info("Tool uninstalled") + taskCB(&rpc.TaskProgress{Message: toolRelease.String() + " uninstalled", Completed: true}) + } + } + // Perform post install if !req.SkipPostInstall { logrus.Info("Running post_install script") diff --git a/test/test_core.py b/test/test_core.py index e62aaa881c4..99a54f6fe5c 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -440,3 +440,37 @@ def test_core_list_updatable_all_flags(run_command, data_dir): assert expected_core_id in mapped assert "Arduino AVR Boards" == mapped[expected_core_id]["Name"] assert "1.8.3" == mapped[expected_core_id]["Latest"] + + +def test_core_upgrade_removes_unused_tools(run_command, data_dir): + assert run_command("update") + + # Installs a core + assert run_command("core install arduino:avr@1.8.2") + + # Verifies expected tool is installed + tool_path = Path(data_dir, "packages", "arduino", "tools", "avr-gcc", "7.3.0-atmel3.6.1-arduino5") + assert tool_path.exists() + + # Upgrades core + assert run_command("core upgrade arduino:avr") + + # Verifies tool is uninstalled since it's not used by newer core version + assert not tool_path.exists() + + +def test_core_install_removes_unused_tools(run_command, data_dir): + assert run_command("update") + + # Installs a core + assert run_command("core install arduino:avr@1.8.2") + + # Verifies expected tool is installed + tool_path = Path(data_dir, "packages", "arduino", "tools", "avr-gcc", "7.3.0-atmel3.6.1-arduino5") + assert tool_path.exists() + + # Installs newer version of already installed core + assert run_command("core install arduino:avr@1.8.3") + + # Verifies tool is uninstalled since it's not used by newer core version + assert not tool_path.exists() diff --git a/test/test_upgrade.py b/test/test_upgrade.py index f5c3d691e7f..094ef5508a8 100644 --- a/test/test_upgrade.py +++ b/test/test_upgrade.py @@ -65,3 +65,20 @@ def test_upgrade_using_library_with_invalid_version(run_command, data_dir): res = run_command("upgrade") assert res.ok assert "WiFi101" in res.stdout + + +def test_upgrade_unused_core_tools_are_removed(run_command, data_dir): + assert run_command("update") + + # Installs a core + assert run_command("core install arduino:avr@1.8.2") + + # Verifies expected tool is installed + tool_path = Path(data_dir, "packages", "arduino", "tools", "avr-gcc", "7.3.0-atmel3.6.1-arduino5") + assert tool_path.exists() + + # Upgrades everything + assert run_command("upgrade") + + # Verifies tool is uninstalled since it's not used by newer core version + assert not tool_path.exists()