[breaking] Add post install script support for tools (#2075)

* Add post install script support for tools

* Add TestCoreInstallRunsToolPostInstallScript to core_test.go

* Document changes in UPGRADING.md

Author: MatteoPologruto

arduino/cores/packagemanager/install_uninstall.go

@@ -102,7 +102,7 @@ func (pme *Explorer) DownloadAndInstallPlatformAndTools(
 
 	// Install tools first
 	for _, tool := range toolsToInstall {
-		if err := pme.InstallTool(tool, taskCB); err != nil {
+		if err := pme.InstallTool(tool, taskCB, skipPostInstall); err != nil {
 			return err
 		}
 	}
@@ -171,7 +171,10 @@ func (pme *Explorer) DownloadAndInstallPlatformAndTools(
 	if !skipPostInstall {
 		log.Info("Running post_install script")
 		taskCB(&rpc.TaskProgress{Message: tr("Configuring platform.")})
-		if err := pme.RunPostInstallScript(platformRelease); err != nil {
+		if !platformRelease.IsInstalled() {
+			return errors.New(tr("platform not installed"))
+		}
+		if err := pme.RunPostInstallScript(platformRelease.InstallDir); err != nil {
 			taskCB(&rpc.TaskProgress{Message: tr("WARNING cannot configure platform: %s", err)})
 		}
 	} else {
@@ -222,22 +225,19 @@ func (pme *Explorer) cacheInstalledJSON(platformRelease *cores.PlatformRelease)
 }
 
 // RunPostInstallScript runs the post_install.sh (or post_install.bat) script for the
-// specified platformRelease.
-func (pme *Explorer) RunPostInstallScript(platformRelease *cores.PlatformRelease) error {
-	if !platformRelease.IsInstalled() {
-		return errors.New(tr("platform not installed"))
-	}
+// specified platformRelease or toolRelease.
+func (pme *Explorer) RunPostInstallScript(installDir *paths.Path) error {
 	postInstallFilename := "post_install.sh"
 	if runtime.GOOS == "windows" {
 		postInstallFilename = "post_install.bat"
 	}
-	postInstall := platformRelease.InstallDir.Join(postInstallFilename)
+	postInstall := installDir.Join(postInstallFilename)
 	if postInstall.Exist() && postInstall.IsNotDir() {
 		cmd, err := executils.NewProcessFromPath(pme.GetEnvVarsForSpawnedProcess(), postInstall)
 		if err != nil {
 			return err
 		}
-		cmd.SetDirFromPath(platformRelease.InstallDir)
+		cmd.SetDirFromPath(installDir)
 		if err := cmd.Run(); err != nil {
 			return err
 		}
@@ -299,7 +299,7 @@ func (pme *Explorer) UninstallPlatform(platformRelease *cores.PlatformRelease, t
 }
 
 // InstallTool installs a specific release of a tool.
-func (pme *Explorer) InstallTool(toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB) error {
+func (pme *Explorer) InstallTool(toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB, skipPostInstall bool) error {
 	log := pme.log.WithField("Tool", toolRelease)
 
 	if toolRelease.IsInstalled() {
@@ -325,6 +325,22 @@ func (pme *Explorer) InstallTool(toolRelease *cores.ToolRelease, taskCB rpc.Task
 		log.WithError(err).Warn("Cannot install tool")
 		return &arduino.FailedInstallError{Message: tr("Cannot install tool %s", toolRelease), Cause: err}
 	}
+	if d, err := destDir.Abs(); err == nil {
+		toolRelease.InstallDir = d
+	} else {
+		return err
+	}
+	// Perform post install
+	if !skipPostInstall {
+		log.Info("Running tool post_install script")
+		taskCB(&rpc.TaskProgress{Message: tr("Configuring tool.")})
+		if err := pme.RunPostInstallScript(toolRelease.InstallDir); err != nil {
+			taskCB(&rpc.TaskProgress{Message: tr("WARNING cannot configure tool: %s", err)})
+		}
+	} else {
+		log.Info("Skipping tool configuration.")
+		taskCB(&rpc.TaskProgress{Message: tr("Skipping tool configuration.")})
+	}
 	log.Info("Tool installed")
 	taskCB(&rpc.TaskProgress{Message: tr("%s installed", toolRelease), Completed: true})
 

commands/instances.go

@@ -138,7 +138,7 @@ func installTool(pm *packagemanager.PackageManager, tool *cores.ToolRelease, dow
 		return fmt.Errorf(tr("downloading %[1]s tool: %[2]s"), tool, err)
 	}
 	taskCB(&rpc.TaskProgress{Completed: true})
-	if err := pme.InstallTool(tool, taskCB); err != nil {
+	if err := pme.InstallTool(tool, taskCB, true); err != nil {
 		return fmt.Errorf(tr("installing %[1]s tool: %[2]s"), tool, err)
 	}
 	return nil

docs/UPGRADING.md

@@ -2,6 +2,31 @@
 
 Here you can find a list of migration guides to handle breaking changes between releases of the CLI.
 
+## 0.31.0
+
+### Added `post_install` script support for tools
+
+The `post_install` script now runs when a tool is correctly installed and the CLI is in "interactive" mode. This
+behavior can be [configured](https://arduino.github.io/arduino-cli/0.30/commands/arduino-cli_core_install/#options).
+
+### golang API: methods in `github.com/arduino/arduino-cli/arduino/cores/packagemanager` changed signature
+
+The following methods in `github.com/arduino/arduino-cli/arduino/cores/packagemanager`:
+
+```go
+func (pme *Explorer) InstallTool(toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB) error { ... }
+func (pme *Explorer) RunPostInstallScript(platformRelease *cores.PlatformRelease) error { ... }
+```
+
+have changed. `InstallTool` requires the new `skipPostInstall` parameter, which must be set to `true` to skip the post
+install script. `RunPostInstallScript` does not require a `*cores.PlatformRelease` parameter but requires a
+`*paths.Path` parameter:
+
+```go
+func (pme *Explorer) InstallTool(toolRelease *cores.ToolRelease, taskCB rpc.TaskProgressCB, skipPostInstall bool) error {...}
+func (pme *Explorer) RunPostInstallScript(installDir *paths.Path) error { ... }
+```
+
 ## 0.30.0
 
 ### Sketch name validation

internal/integrationtest/core/core_test.go

@@ -992,3 +992,18 @@ func TestCoreInstallCreatesInstalledJson(t *testing.T) {
 	sortedExpected := requirejson.Parse(t, expectedInstalledJson).Query("walk(if type == \"array\" then sort else . end)").String()
 	require.JSONEq(t, sortedExpected, sortedInstalled)
 }
+
+func TestCoreInstallRunsToolPostInstallScript(t *testing.T) {
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	url := "http://drazzy.com/package_drazzy.com_index.json"
+
+	_, _, err := cli.Run("core", "update-index", "--additional-urls", url)
+	require.NoError(t, err)
+
+	// Checks that the post_install script is correctly skipped on the CI
+	stdout, _, err := cli.Run("core", "install", "ATTinyCore:avr", "--verbose", "--additional-urls", url)
+	require.NoError(t, err)
+	require.Contains(t, string(stdout), "Skipping tool configuration.")
+}