Fixed compiler output capture in JSON mode (#2078)

* Added integration test for #1698

* Added helper functions to handle concurrent writes to ctx.Stdout/Stderr

* Do not alter ctx.Stderr/Stdout in ExecCommand

Some instances of ctx.Stdout may now contains nil since it's no more
altered during ExecCommand.

* Handle multi-threaded compile streams

Author: cmaglie

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"`)
+}

internal/integrationtest/compile_3/testdata/blink_with_wrong_cpp/blink_with_wrong_cpp.ino

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

internal/integrationtest/compile_3/testdata/blink_with_wrong_cpp/wrong.cpp

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

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
 }
 

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)
 		}

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)
 		}
 

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
 }

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)
+}

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()