compile --show-properties now correctly output the result in text and JSON format (#2035)

* Added function to create buffering streams

* Moved the direct output of build properties to commands.Compile

* BuildProperties are now returned as part of Compile response

* Build properties are now displayed alone if running in text mode

* Fixed wrong error report

* Added integration test

* Small improvement on result output

Author: cmaglie

Diff for: commands/compile/compile.go

@@ -19,6 +19,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"sort"
 	"strings"
 
 	"github.com/arduino/arduino-cli/arduino"
@@ -185,17 +186,32 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 		}
 	}()
 
-	// if --preprocess or --show-properties were passed, we can stop here
+	defer func() {
+		buildProperties := builderCtx.BuildProperties
+		if buildProperties == nil {
+			return
+		}
+		keys := buildProperties.Keys()
+		sort.Strings(keys)
+		for _, key := range keys {
+			r.BuildProperties = append(r.BuildProperties, key+"="+buildProperties.Get(key))
+		}
+	}()
+
 	if req.GetShowProperties() {
-		compileErr := builder.RunParseHardwareAndDumpBuildProperties(builderCtx)
+		// Just get build properties and exit
+		compileErr := builder.RunParseHardware(builderCtx)
 		if compileErr != nil {
-			compileErr = &arduino.CompileFailedError{Message: err.Error()}
+			compileErr = &arduino.CompileFailedError{Message: compileErr.Error()}
 		}
 		return r, compileErr
-	} else if req.GetPreprocess() {
+	}
+
+	if req.GetPreprocess() {
+		// Just output preprocessed source code and exit
 		compileErr := builder.RunPreprocess(builderCtx)
 		if compileErr != nil {
-			compileErr = &arduino.CompileFailedError{Message: err.Error()}
+			compileErr = &arduino.CompileFailedError{Message: compileErr.Error()}
 		}
 		return r, compileErr
 	}

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

@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"strings"
 
@@ -187,6 +188,14 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 		overrides = o.Overrides
 	}
 
+	var stdOut, stdErr io.Writer
+	var stdIORes func() *feedback.OutputStreamsResult
+	if showProperties {
+		stdOut, stdErr, stdIORes = feedback.NewBufferedStreams()
+	} else {
+		stdOut, stdErr, stdIORes = feedback.OutputStreams()
+	}
+
 	compileRequest := &rpc.CompileRequest{
 		Instance:                      inst,
 		Fqbn:                          fqbn,
@@ -212,9 +221,7 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 		EncryptKey:                    encryptKey,
 		SkipLibrariesDiscovery:        skipLibrariesDiscovery,
 	}
-	stdOut, stdErr, stdIORes := feedback.OutputStreams()
 	compileRes, compileError := compile.Compile(context.Background(), compileRequest, stdOut, stdErr, nil)
-
 	if compileError == nil && uploadAfterCompile {
 		userFieldRes, err := upload.SupportedUserFields(context.Background(), &rpc.SupportedUserFieldsRequest{
 			Instance: inst,
@@ -306,11 +313,12 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 
 	stdIO := stdIORes()
 	res := &compileResult{
-		CompilerOut:   stdIO.Stdout,
-		CompilerErr:   stdIO.Stderr,
-		BuilderResult: compileRes,
-		ProfileOut:    profileOut,
-		Success:       compileError == nil,
+		CompilerOut:        stdIO.Stdout,
+		CompilerErr:        stdIO.Stderr,
+		BuilderResult:      compileRes,
+		ProfileOut:         profileOut,
+		Success:            compileError == nil,
+		showOnlyProperties: showProperties,
 	}
 
 	if compileError != nil {
@@ -355,13 +363,19 @@ type compileResult struct {
 	Success       bool                 `json:"success"`
 	ProfileOut    string               `json:"profile_out,omitempty"`
 	Error         string               `json:"error,omitempty"`
+
+	showOnlyProperties bool
 }
 
 func (r *compileResult) Data() interface{} {
 	return r
 }
 
 func (r *compileResult) String() string {
+	if r.showOnlyProperties {
+		return strings.Join(r.BuilderResult.BuildProperties, fmt.Sprintln())
+	}
+
 	titleColor := color.New(color.FgHiGreen)
 	nameColor := color.New(color.FgHiYellow)
 	pathColor := color.New(color.FgHiBlack)

Diff for: internal/cli/feedback/stdio.go

@@ -16,6 +16,7 @@
 package feedback
 
 import (
+	"bytes"
 	"errors"
 	"io"
 )
@@ -56,6 +57,24 @@ func OutputStreams() (io.Writer, io.Writer, func() *OutputStreamsResult) {
 	return feedbackOut, feedbackErr, getOutputStreamResult
 }
 
+// NewBufferedStreams returns a pair of io.Writer to buffer the command output.
+// The returned writers will accumulate the output until the command
+// execution is completed. The io.Writes will not affect other feedback streams.
+//
+// This function returns also a callback that must be called when the
+// command execution is completed, it will return an *OutputStreamsResult
+// object that can be used as a Result or to retrieve the accumulated output
+// to embed it in another object.
+func NewBufferedStreams() (io.Writer, io.Writer, func() *OutputStreamsResult) {
+	out, err := &bytes.Buffer{}, &bytes.Buffer{}
+	return out, err, func() *OutputStreamsResult {
+		return &OutputStreamsResult{
+			Stdout: out.String(),
+			Stderr: err.String(),
+		}
+	}
+}
+
 func getOutputStreamResult() *OutputStreamsResult {
 	return &OutputStreamsResult{
 		Stdout: bufferOut.String(),

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

@@ -0,0 +1,63 @@
+// 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 [email protected].
+
+package compile_test
+
+import (
+	"testing"
+
+	"github.com/arduino/arduino-cli/internal/integrationtest"
+	"github.com/arduino/go-properties-orderedmap"
+	"github.com/stretchr/testify/require"
+	"go.bug.st/testifyjson/requirejson"
+)
+
+func TestCompileShowProperties(t *testing.T) {
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	_, _, err := cli.Run("core", "update-index")
+	require.NoError(t, err)
+	_, _, err = cli.Run("core", "install", "arduino:avr")
+	require.NoError(t, err)
+
+	bareMinimum := cli.CopySketch("bare_minimum")
+
+	// Test --show-properties output is clean
+	stdout, stderr, err := cli.Run("compile", "--fqbn", "arduino:avr:uno", "-v", "--show-properties", bareMinimum.String())
+	require.NoError(t, err)
+	_, err = properties.LoadFromBytes(stdout)
+	require.NoError(t, err, "Output must be a clean property list")
+	require.Empty(t, stderr)
+
+	// Test --show-properties --format JSON output is clean
+	stdout, stderr, err = cli.Run("compile", "--fqbn", "arduino:avr:uno", "-v", "--show-properties", "--format", "json", bareMinimum.String())
+	require.NoError(t, err)
+	requirejson.Parse(t, stdout, "Output must be a valid JSON")
+	require.Empty(t, stderr)
+
+	// Test --show-properties output is clean, with a wrong FQBN
+	stdout, stderr, err = cli.Run("compile", "--fqbn", "arduino:avr:unoa", "-v", "--show-properties", bareMinimum.String())
+	require.Error(t, err)
+	_, err = properties.LoadFromBytes(stdout)
+	require.NoError(t, err, "Output must be a clean property list")
+	require.NotEmpty(t, stderr)
+
+	// Test --show-properties --format JSON output is clean, with a wrong FQBN
+	stdout, stderr, err = cli.Run("compile", "--fqbn", "arduino:avr:unoa", "-v", "--show-properties", "--format", "json", bareMinimum.String())
+	require.Error(t, err)
+	requirejson.Parse(t, stdout, "Output must be a valid JSON")
+	require.Empty(t, stderr)
+}

Diff for: legacy/builder/builder.go

@@ -162,22 +162,6 @@ func (s *Preprocess) Run(ctx *types.Context) error {
 	return nil
 }
 
-type ParseHardwareAndDumpBuildProperties struct{}
-
-func (s *ParseHardwareAndDumpBuildProperties) Run(ctx *types.Context) error {
-	if ctx.BuildPath == nil {
-		ctx.BuildPath = sketch.GenBuildPath(ctx.SketchLocation)
-	}
-
-	commands := []types.Command{
-		&ContainerSetupHardwareToolsLibsSketchAndProps{},
-
-		&DumpBuildProperties{},
-	}
-
-	return runCommands(ctx, commands)
-}
-
 func runCommands(ctx *types.Context, commands []types.Command) error {
 	ctx.Progress.AddSubSteps(len(commands))
 	defer ctx.Progress.RemoveSubSteps()
@@ -203,9 +187,14 @@ func RunBuilder(ctx *types.Context) error {
 	return command.Run(ctx)
 }
 
-func RunParseHardwareAndDumpBuildProperties(ctx *types.Context) error {
-	command := ParseHardwareAndDumpBuildProperties{}
-	return command.Run(ctx)
+func RunParseHardware(ctx *types.Context) error {
+	if ctx.BuildPath == nil {
+		ctx.BuildPath = sketch.GenBuildPath(ctx.SketchLocation)
+	}
+	commands := []types.Command{
+		&ContainerSetupHardwareToolsLibsSketchAndProps{},
+	}
+	return runCommands(ctx, commands)
 }
 
 func RunPreprocess(ctx *types.Context) error {

Diff for: legacy/builder/dump_build_properties.go

@@ -32,7 +32,7 @@ message CompileRequest {
   string fqbn = 2;
   // The path where the sketch is stored.
   string sketch_path = 3;
-  // Show all build preferences used instead of compiling.
+  // Just get the build properties and do not run the full compile.
   bool show_properties = 4;
   // Print preprocessed code to stdout instead of compiling.
   bool preprocess = 5;
@@ -112,6 +112,8 @@ message CompileResponse {
   InstalledPlatformReference build_platform = 7;
   // Completions reports of the compilation process (stream)
   TaskProgress progress = 8;
+  // Build properties used for compiling
+  repeated string build_properties = 9;
 }
 
 message ExecutableSectionSize {