Merge branch 'master' into feat/more-error-details

Author: Bikappa

Diff for: arduino/cores/packagemanager/install_uninstall.go

@@ -16,6 +16,7 @@
 package packagemanager
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"runtime"
@@ -174,9 +175,13 @@ func (pme *Explorer) DownloadAndInstallPlatformAndTools(
 		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)})
+		stdout, stderr, err := pme.RunPostInstallScript(platformRelease.InstallDir)
+		skipEmptyMessageTaskProgressCB(taskCB)(&rpc.TaskProgress{Message: string(stdout), Completed: true})
+		skipEmptyMessageTaskProgressCB(taskCB)(&rpc.TaskProgress{Message: string(stderr), Completed: true})
+		if err != nil {
+			taskCB(&rpc.TaskProgress{Message: tr("WARNING cannot configure platform: %s", err), Completed: true})
 		}
+
 	} else {
 		log.Info("Skipping platform configuration.")
 		taskCB(&rpc.TaskProgress{Message: tr("Skipping platform configuration.")})
@@ -226,7 +231,7 @@ func (pme *Explorer) cacheInstalledJSON(platformRelease *cores.PlatformRelease)
 
 // RunPostInstallScript runs the post_install.sh (or post_install.bat) script for the
 // specified platformRelease or toolRelease.
-func (pme *Explorer) RunPostInstallScript(installDir *paths.Path) error {
+func (pme *Explorer) RunPostInstallScript(installDir *paths.Path) ([]byte, []byte, error) {
 	postInstallFilename := "post_install.sh"
 	if runtime.GOOS == "windows" {
 		postInstallFilename = "post_install.bat"
@@ -235,14 +240,16 @@ func (pme *Explorer) RunPostInstallScript(installDir *paths.Path) error {
 	if postInstall.Exist() && postInstall.IsNotDir() {
 		cmd, err := executils.NewProcessFromPath(pme.GetEnvVarsForSpawnedProcess(), postInstall)
 		if err != nil {
-			return err
+			return []byte{}, []byte{}, err
 		}
+		cmdStdout, cmdStderr := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{})
+		cmd.RedirectStdoutTo(cmdStdout)
+		cmd.RedirectStderrTo(cmdStderr)
 		cmd.SetDirFromPath(installDir)
-		if err := cmd.Run(); err != nil {
-			return err
-		}
+		err = cmd.Run()
+		return cmdStdout.Bytes(), cmdStderr.Bytes(), err
 	}
-	return nil
+	return []byte{}, []byte{}, nil
 }
 
 // IsManagedPlatformRelease returns true if the PlatforRelease is managed by the PackageManager
@@ -335,7 +342,10 @@ func (pme *Explorer) InstallTool(toolRelease *cores.ToolRelease, taskCB rpc.Task
 	if !skipPostInstall {
 		log.Info("Running tool post_install script")
 		taskCB(&rpc.TaskProgress{Message: tr("Configuring tool.")})
-		if err := pme.RunPostInstallScript(toolRelease.InstallDir); err != nil {
+		stdout, stderr, err := pme.RunPostInstallScript(toolRelease.InstallDir)
+		skipEmptyMessageTaskProgressCB(taskCB)(&rpc.TaskProgress{Message: string(stdout)})
+		skipEmptyMessageTaskProgressCB(taskCB)(&rpc.TaskProgress{Message: string(stderr)})
+		if err != nil {
 			taskCB(&rpc.TaskProgress{Message: tr("WARNING cannot configure tool: %s", err)})
 		}
 	} else {
@@ -412,3 +422,12 @@ func (pme *Explorer) IsToolRequired(toolRelease *cores.ToolRelease) bool {
 	}
 	return false
 }
+
+func skipEmptyMessageTaskProgressCB(taskCB rpc.TaskProgressCB) rpc.TaskProgressCB {
+	return func(msg *rpc.TaskProgress) {
+		if msg != nil && len(msg.Message) == 0 {
+			return
+		}
+		taskCB(msg)
+	}
+}

Diff for: arduino/cores/packagemanager/package_manager_test.go

@@ -19,6 +19,8 @@ import (
 	"fmt"
 	"net/url"
 	"os"
+	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/arduino/arduino-cli/arduino/cores"
@@ -639,3 +641,39 @@ func TestLegacyPackageConversionToPluggableDiscovery(t *testing.T) {
 		require.Equal(t, `"{network_cmd}" -address {upload.port.address} -port {upload.port.properties.port} -sketch "{build.path}/{build.project_name}.hex" -upload {upload.port.properties.endpoint_upload} -sync {upload.port.properties.endpoint_sync} -reset {upload.port.properties.endpoint_reset} -sync_exp {upload.port.properties.sync_return}`, platformProps.Get("tools.avrdude__pluggable_network.upload.pattern"))
 	}
 }
+
+func TestRunPostInstall(t *testing.T) {
+	pmb := packagemanager.NewBuilder(nil, nil, nil, nil, "test")
+	pm := pmb.Build()
+	pme, release := pm.NewExplorer()
+	defer release()
+
+	// prepare dummy post install script
+	dir := paths.New(t.TempDir())
+
+	var scriptPath *paths.Path
+	var err error
+	if runtime.GOOS == "windows" {
+		scriptPath = dir.Join("post_install.bat")
+
+		err = scriptPath.WriteFile([]byte(
+			`@echo off
+			echo sent in stdout
+			echo sent in stderr 1>&2`))
+	} else {
+		scriptPath = dir.Join("post_install.sh")
+		err = scriptPath.WriteFile([]byte(
+			`#!/bin/sh
+			echo "sent in stdout"
+			echo "sent in stderr" 1>&2`))
+	}
+	require.NoError(t, err)
+	err = os.Chmod(scriptPath.String(), 0777)
+	require.NoError(t, err)
+	stdout, stderr, err := pme.RunPostInstallScript(dir)
+	require.NoError(t, err)
+
+	// `HasPrefix` because windows seem to add a trailing space at the end
+	require.Equal(t, "sent in stdout", strings.Trim(string(stdout), "\n\r "))
+	require.Equal(t, "sent in stderr", strings.Trim(string(stderr), "\n\r "))
+}

Diff for: internal/cli/daemon/daemon.go

@@ -16,6 +16,7 @@
 package daemon
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"net"
@@ -173,5 +174,6 @@ func (r daemonResult) Data() interface{} {
 }
 
 func (r daemonResult) String() string {
-	return tr("Daemon is now listening on %s:%s", r.IP, r.Port)
+	j, _ := json.Marshal(r)
+	return fmt.Sprintln(tr("Daemon is now listening on %s:%s", r.IP, r.Port)) + fmt.Sprintln(string(j))
 }

Diff for: internal/integrationtest/compile_3/compile_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
 	"github.com/stretchr/testify/require"
+	"go.bug.st/testifyjson/requirejson"
 )
 
 func TestRuntimeToolPropertiesGeneration(t *testing.T) {
@@ -96,3 +97,22 @@ func TestCompileBuildPathInsideSketch(t *testing.T) {
 	_, _, err = cli.Run("compile", "-b", "arduino:avr:mega", "--build-path", "build-mega")
 	require.NoError(t, err)
 }
+
+func TestCompilerErrOutput(t *testing.T) {
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	// Run update-index with our test index
+	_, _, err := cli.Run("core", "install", "arduino:[email protected]")
+	require.NoError(t, err)
+
+	// prepare sketch
+	sketch, err := paths.New("testdata", "blink_with_wrong_cpp").Abs()
+	require.NoError(t, err)
+
+	// Run compile and catch err stream
+	out, _, err := cli.Run("compile", "-b", "arduino:avr:uno", "--format", "json", sketch.String())
+	require.Error(t, err)
+	compilerErr := requirejson.Parse(t, out).Query(".compiler_err")
+	compilerErr.MustContain(`"error"`)
+}

Diff for: internal/integrationtest/compile_3/testdata/blink_with_wrong_cpp/blink_with_wrong_cpp.ino

@@ -0,0 +1,2 @@
+void setup() {}
+void loop() {}

Diff for: internal/integrationtest/compile_3/testdata/blink_with_wrong_cpp/wrong.cpp

@@ -0,0 +1 @@
+void wrong() {

Diff for: legacy/builder/builder.go

@@ -152,7 +152,7 @@ func (s *Preprocess) Run(ctx *types.Context) error {
 	}
 
 	// Output arduino-preprocessed source
-	ctx.Stdout.Write([]byte(ctx.Source))
+	ctx.WriteStdout([]byte(ctx.Source))
 	return nil
 }
 

Diff for: legacy/builder/builder_utils/utils.go

@@ -190,7 +190,15 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath *paths.Path, source *p
 		ctx.CompilationDatabase.Add(source, command)
 	}
 	if !objIsUpToDate && !ctx.OnlyUpdateCompilationDatabase {
-		_, _, err = utils.ExecCommand(ctx, command, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
+		// Since this compile could be multithreaded, we first capture the command output
+		stdout, stderr, err := utils.ExecCommand(ctx, command, utils.Capture, utils.Capture)
+		// and transfer all at once at the end...
+		if ctx.Verbose {
+			ctx.WriteStdout(stdout)
+		}
+		ctx.WriteStderr(stderr)
+
+		// ...and then return the error
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}

Diff for: legacy/builder/container_find_includes.go

@@ -408,7 +408,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
 					return errors.New(tr("Internal error in cache"))
 				}
 			}
-			ctx.Stderr.Write(preproc_stderr)
+			ctx.WriteStderr(preproc_stderr)
 			return errors.WithStack(preproc_err)
 		}
 

Diff for: legacy/builder/preprocess_sketch.go

@@ -146,6 +146,6 @@ func (s *OutputCodeCompletions) Run(ctx *types.Context) error {
 		// we assume it is a json, let's make it compliant at least
 		ctx.CodeCompletions = "[]"
 	}
-	fmt.Fprintln(ctx.Stdout, ctx.CodeCompletions)
+	ctx.WriteStdout([]byte(ctx.CodeCompletions))
 	return nil
 }

Diff for: legacy/builder/types/context.go

@@ -249,3 +249,21 @@ func (ctx *Context) Warn(msg string) {
 	}
 	ctx.stdLock.Unlock()
 }
+
+func (ctx *Context) WriteStdout(data []byte) (int, error) {
+	ctx.stdLock.Lock()
+	defer ctx.stdLock.Unlock()
+	if ctx.Stdout == nil {
+		return os.Stdout.Write(data)
+	}
+	return ctx.Stdout.Write(data)
+}
+
+func (ctx *Context) WriteStderr(data []byte) (int, error) {
+	ctx.stdLock.Lock()
+	defer ctx.stdLock.Unlock()
+	if ctx.Stderr == nil {
+		return os.Stderr.Write(data)
+	}
+	return ctx.Stderr.Write(data)
+}

Diff for: legacy/builder/utils/utils.go

@@ -184,29 +184,30 @@ const (
 )
 
 func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) {
-	if ctx.Stdout == nil {
-		ctx.Stdout = os.Stdout
-	}
-	if ctx.Stderr == nil {
-		ctx.Stderr = os.Stderr
-	}
-
 	if ctx.Verbose {
 		ctx.Info(PrintableCommand(command.Args))
 	}
 
 	if stdout == Capture {
 		buffer := &bytes.Buffer{}
 		command.Stdout = buffer
-	} else if stdout == Show || stdout == ShowIfVerbose && ctx.Verbose {
-		command.Stdout = ctx.Stdout
+	} else if stdout == Show || (stdout == ShowIfVerbose && ctx.Verbose) {
+		if ctx.Stdout != nil {
+			command.Stdout = ctx.Stdout
+		} else {
+			command.Stdout = os.Stdout
+		}
 	}
 
 	if stderr == Capture {
 		buffer := &bytes.Buffer{}
 		command.Stderr = buffer
-	} else if stderr == Show || stderr == ShowIfVerbose && ctx.Verbose {
-		command.Stderr = ctx.Stderr
+	} else if stderr == Show || (stderr == ShowIfVerbose && ctx.Verbose) {
+		if ctx.Stderr != nil {
+			command.Stderr = ctx.Stderr
+		} else {
+			command.Stderr = os.Stderr
+		}
 	}
 
 	err := command.Start()