bugfix: compile ... --dump-profile now produces a clean output (only the profile) (#2852)

* Output a clean profile with --dump-profile

* Use bytes.NewBuffer for creation of bytes buffers

* Added integration test

Author: cmaglie

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

@@ -211,7 +211,9 @@ func runCompileCommand(cmd *cobra.Command, args []string, srv rpc.ArduinoCoreSer
 
 	var stdOut, stdErr io.Writer
 	var stdIORes func() *feedback.OutputStreamsResult
-	if showProperties != arguments.ShowPropertiesDisabled {
+	if showProperties != arguments.ShowPropertiesDisabled || dumpProfile {
+		// When dumping profile or showing properties, we buffer the output
+		// to avoid mixing compilation output with profile/properties output
 		stdOut, stdErr, stdIORes = feedback.NewBufferedStreams()
 	} else {
 		stdOut, stdErr, stdIORes = feedback.OutputStreams()
@@ -312,60 +314,69 @@ func runCompileCommand(cmd *cobra.Command, args []string, srv rpc.ArduinoCoreSer
 		}
 	}
 
+	successful := (compileError == nil)
 	profileOut := ""
-	if dumpProfile && compileError == nil {
-		// Output profile
-
-		libs := ""
-		hasVendoredLibs := false
-		for _, lib := range builderRes.GetUsedLibraries() {
-			if lib.GetLocation() != rpc.LibraryLocation_LIBRARY_LOCATION_USER && lib.GetLocation() != rpc.LibraryLocation_LIBRARY_LOCATION_UNMANAGED {
-				continue
+	stdIO := stdIORes()
+
+	if dumpProfile {
+		if successful {
+			// Output profile
+
+			libs := ""
+			hasVendoredLibs := false
+			for _, lib := range builderRes.GetUsedLibraries() {
+				if lib.GetLocation() != rpc.LibraryLocation_LIBRARY_LOCATION_USER && lib.GetLocation() != rpc.LibraryLocation_LIBRARY_LOCATION_UNMANAGED {
+					continue
+				}
+				if lib.GetVersion() == "" {
+					hasVendoredLibs = true
+					continue
+				}
+				libs += fmt.Sprintln("      - " + lib.GetName() + " (" + lib.GetVersion() + ")")
 			}
-			if lib.GetVersion() == "" {
-				hasVendoredLibs = true
-				continue
+			if hasVendoredLibs {
+				msg := "\n"
+				msg += i18n.Tr("WARNING: The sketch is compiled using one or more custom libraries.") + "\n"
+				msg += i18n.Tr("Currently, Build Profiles only support libraries available through Arduino Library Manager.")
+				feedback.Warning(msg)
 			}
-			libs += fmt.Sprintln("      - " + lib.GetName() + " (" + lib.GetVersion() + ")")
-		}
-		if hasVendoredLibs {
-			msg := "\n"
-			msg += i18n.Tr("WARNING: The sketch is compiled using one or more custom libraries.") + "\n"
-			msg += i18n.Tr("Currently, Build Profiles only support libraries available through Arduino Library Manager.")
-			feedback.Warning(msg)
-		}
-
-		newProfileName := "my_profile_name"
-		if split := strings.Split(compileRequest.GetFqbn(), ":"); len(split) > 2 {
-			newProfileName = split[2]
-		}
-		profileOut = fmt.Sprintln("profiles:")
-		profileOut += fmt.Sprintln("  " + newProfileName + ":")
-		profileOut += fmt.Sprintln("    fqbn: " + compileRequest.GetFqbn())
-		profileOut += fmt.Sprintln("    platforms:")
-		boardPlatform := builderRes.GetBoardPlatform()
-		profileOut += fmt.Sprintln("      - platform: " + boardPlatform.GetId() + " (" + boardPlatform.GetVersion() + ")")
-		if url := boardPlatform.GetPackageUrl(); url != "" {
-			profileOut += fmt.Sprintln("        platform_index_url: " + url)
-		}
 
-		if buildPlatform := builderRes.GetBuildPlatform(); buildPlatform != nil &&
-			buildPlatform.GetId() != boardPlatform.GetId() &&
-			buildPlatform.GetVersion() != boardPlatform.GetVersion() {
-			profileOut += fmt.Sprintln("      - platform: " + buildPlatform.GetId() + " (" + buildPlatform.GetVersion() + ")")
-			if url := buildPlatform.GetPackageUrl(); url != "" {
+			newProfileName := "my_profile_name"
+			if split := strings.Split(compileRequest.GetFqbn(), ":"); len(split) > 2 {
+				newProfileName = split[2]
+			}
+			profileOut = fmt.Sprintln("profiles:")
+			profileOut += fmt.Sprintln("  " + newProfileName + ":")
+			profileOut += fmt.Sprintln("    fqbn: " + compileRequest.GetFqbn())
+			profileOut += fmt.Sprintln("    platforms:")
+			boardPlatform := builderRes.GetBoardPlatform()
+			profileOut += fmt.Sprintln("      - platform: " + boardPlatform.GetId() + " (" + boardPlatform.GetVersion() + ")")
+			if url := boardPlatform.GetPackageUrl(); url != "" {
 				profileOut += fmt.Sprintln("        platform_index_url: " + url)
 			}
+
+			if buildPlatform := builderRes.GetBuildPlatform(); buildPlatform != nil &&
+				buildPlatform.GetId() != boardPlatform.GetId() &&
+				buildPlatform.GetVersion() != boardPlatform.GetVersion() {
+				profileOut += fmt.Sprintln("      - platform: " + buildPlatform.GetId() + " (" + buildPlatform.GetVersion() + ")")
+				if url := buildPlatform.GetPackageUrl(); url != "" {
+					profileOut += fmt.Sprintln("        platform_index_url: " + url)
+				}
+			}
+			if len(libs) > 0 {
+				profileOut += fmt.Sprintln("    libraries:")
+				profileOut += fmt.Sprint(libs)
+			}
+			profileOut += fmt.Sprintln()
+		} else {
+			// An error occurred, output the buffered build errors instead of the profile
+			if stdOut, stdErr, err := feedback.DirectStreams(); err == nil {
+				stdOut.Write([]byte(stdIO.Stdout))
+				stdErr.Write([]byte(stdIO.Stderr))
+			}
 		}
-		if len(libs) > 0 {
-			profileOut += fmt.Sprintln("    libraries:")
-			profileOut += fmt.Sprint(libs)
-		}
-		profileOut += fmt.Sprintln()
 	}
 
-	stdIO := stdIORes()
-	successful := (compileError == nil)
 	res := &compileResult{
 		CompilerOut:   stdIO.Stdout,
 		CompilerErr:   stdIO.Stderr,
@@ -437,6 +448,10 @@ func (r *compileResult) String() string {
 		return strings.Join(r.BuilderResult.BuildProperties, fmt.Sprintln())
 	}
 
+	if r.Success && r.ProfileOut != "" {
+		return r.ProfileOut
+	}
+
 	if r.hideStats {
 		return ""
 	}
@@ -485,9 +500,6 @@ func (r *compileResult) String() string {
 		}
 		res += fmt.Sprintln(platforms.Render())
 	}
-	if r.ProfileOut != "" {
-		res += fmt.Sprintln(r.ProfileOut)
-	}
 	return strings.TrimRight(res, fmt.Sprintln())
 }
 

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

@@ -84,8 +84,8 @@ func reset() {
 	stdErr = os.Stderr
 	feedbackOut = os.Stdout
 	feedbackErr = os.Stderr
-	bufferOut = &bytes.Buffer{}
-	bufferErr = &bytes.Buffer{}
+	bufferOut = bytes.NewBuffer(nil)
+	bufferErr = bytes.NewBuffer(nil)
 	bufferWarnings = nil
 	format = Text
 	formatSelected = false

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

@@ -68,7 +68,7 @@ func OutputStreams() (io.Writer, io.Writer, func() *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{}
+	out, err := bytes.NewBuffer(nil), bytes.NewBuffer(nil)
 	return out, err, func() *OutputStreamsResult {
 		return &OutputStreamsResult{
 			Stdout: out.String(),

Diff for: internal/integrationtest/compile_4/dump_profile_test.go

@@ -0,0 +1,71 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2023 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 (
+	"strings"
+	"testing"
+
+	"github.com/arduino/arduino-cli/internal/integrationtest"
+	"github.com/arduino/go-paths-helper"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDumpProfileClean(t *testing.T) {
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	t.Cleanup(env.CleanUp)
+
+	// Install Arduino AVR Boards
+	_, _, err := cli.Run("core", "install", "arduino:[email protected]")
+	require.NoError(t, err)
+
+	validSketchPath, err := paths.New("testdata", "ValidSketch").Abs()
+	require.NoError(t, err)
+	invalidSketchPath, err := paths.New("testdata", "InvalidSketch").Abs()
+	require.NoError(t, err)
+
+	validProfile := `profiles:
+  uno:
+    fqbn: arduino:avr:uno
+    platforms:
+      - platform: arduino:avr (1.8.6)`
+	t.Run("NoVerbose", func(t *testing.T) {
+		stdout, stderr, err := cli.Run("compile", "-b", "arduino:avr:uno", validSketchPath.String(), "--dump-profile")
+		require.NoError(t, err)
+		require.Empty(t, stderr)
+		profile := strings.TrimSpace(string(stdout))
+		require.Equal(t, validProfile, profile)
+	})
+	t.Run("Verbose", func(t *testing.T) {
+		stdout, stderr, err := cli.Run("compile", "-b", "arduino:avr:uno", validSketchPath.String(), "--dump-profile", "--verbose")
+		require.NoError(t, err)
+		require.Empty(t, stderr)
+		profile := strings.TrimSpace(string(stdout))
+		require.Equal(t, validProfile, profile)
+	})
+	t.Run("ErrorNoVerbose", func(t *testing.T) {
+		stdout, stderr, err := cli.Run("compile", "-b", "arduino:avr:uno", invalidSketchPath.String(), "--dump-profile")
+		require.Error(t, err)
+		require.NotEmpty(t, stderr)
+		require.NotContains(t, string(stdout), validProfile)
+	})
+	t.Run("ErrorVerbose", func(t *testing.T) {
+		stdout, stderr, err := cli.Run("compile", "-b", "arduino:avr:uno", invalidSketchPath.String(), "--dump-profile", "--verbose")
+		require.Error(t, err)
+		require.NotEmpty(t, stderr)
+		require.NotContains(t, string(stdout), validProfile)
+	})
+}

Diff for: internal/integrationtest/compile_4/testdata/InvalidSketch/InvalidSketch.ino

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

Diff for: internal/integrationtest/compile_4/testdata/ValidSketch/ValidSketch.ino

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