[breaking] gRPC updates to CompilerResponse, UploadUsingProgrammerResponse, and BurnBootloaderResponse (#2472)

* Refactored gRPC CompilerResponse

* Refactored gRPC UploadUsingProgrammerResponse and BurnBootloaderResponse

Author: cmaglie

commands/compile/compile.go

@@ -42,7 +42,7 @@ import (
 var tr = i18n.Tr
 
 // Compile FIXMEDOC
-func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream io.Writer, progressCB rpc.TaskProgressCB) (r *rpc.CompileResponse, e error) {
+func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream io.Writer, progressCB rpc.TaskProgressCB) (r *rpc.BuilderResult, e error) {
 
 	// There is a binding between the export binaries setting and the CLI flag to explicitly set it,
 	// since we want this binding to work also for the gRPC interface we must read it here in this
@@ -105,7 +105,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 		return nil, &cmderrors.InvalidFQBNError{Cause: err}
 	}
 
-	r = &rpc.CompileResponse{}
+	r = &rpc.BuilderResult{}
 	r.BoardPlatform = targetPlatform.ToRPCPlatformReference()
 	r.BuildPlatform = buildPlatform.ToRPCPlatformReference()
 

commands/daemon/daemon.go

@@ -181,16 +181,31 @@ func (s *ArduinoCoreServerImpl) SetSketchDefaults(ctx context.Context, req *rpc.
 // Compile FIXMEDOC
 func (s *ArduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.ArduinoCoreService_CompileServer) error {
 	syncSend := NewSynchronizedSend(stream.Send)
-	outStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.CompileResponse{OutStream: data}) })
-	errStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.CompileResponse{ErrStream: data}) })
-	compileResp, compileErr := compile.Compile(
-		stream.Context(), req, outStream, errStream,
-		func(p *rpc.TaskProgress) { syncSend.Send(&rpc.CompileResponse{Progress: p}) })
+	outStream := feedStreamTo(func(data []byte) {
+		syncSend.Send(&rpc.CompileResponse{
+			Message: &rpc.CompileResponse_OutStream{OutStream: data},
+		})
+	})
+	errStream := feedStreamTo(func(data []byte) {
+		syncSend.Send(&rpc.CompileResponse{
+			Message: &rpc.CompileResponse_ErrStream{ErrStream: data},
+		})
+	})
+	progressStream := func(p *rpc.TaskProgress) {
+		syncSend.Send(&rpc.CompileResponse{
+			Message: &rpc.CompileResponse_Progress{Progress: p},
+		})
+	}
+	compileRes, compileErr := compile.Compile(stream.Context(), req, outStream, errStream, progressStream)
 	outStream.Close()
 	errStream.Close()
 	var compileRespSendErr error
-	if compileResp != nil {
-		compileRespSendErr = syncSend.Send(compileResp)
+	if compileRes != nil {
+		compileRespSendErr = syncSend.Send(&rpc.CompileResponse{
+			Message: &rpc.CompileResponse_Result{
+				Result: compileRes,
+			},
+		})
 	}
 	if compileErr != nil {
 		return convertErrorToRPCStatus(compileErr)
@@ -287,8 +302,20 @@ func (s *ArduinoCoreServerImpl) Upload(req *rpc.UploadRequest, stream rpc.Arduin
 // UploadUsingProgrammer FIXMEDOC
 func (s *ArduinoCoreServerImpl) UploadUsingProgrammer(req *rpc.UploadUsingProgrammerRequest, stream rpc.ArduinoCoreService_UploadUsingProgrammerServer) error {
 	syncSend := NewSynchronizedSend(stream.Send)
-	outStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.UploadUsingProgrammerResponse{OutStream: data}) })
-	errStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.UploadUsingProgrammerResponse{ErrStream: data}) })
+	outStream := feedStreamTo(func(data []byte) {
+		syncSend.Send(&rpc.UploadUsingProgrammerResponse{
+			Message: &rpc.UploadUsingProgrammerResponse_OutStream{
+				OutStream: data,
+			},
+		})
+	})
+	errStream := feedStreamTo(func(data []byte) {
+		syncSend.Send(&rpc.UploadUsingProgrammerResponse{
+			Message: &rpc.UploadUsingProgrammerResponse_ErrStream{
+				ErrStream: data,
+			},
+		})
+	})
 	err := upload.UsingProgrammer(stream.Context(), req, outStream, errStream)
 	outStream.Close()
 	errStream.Close()
@@ -307,8 +334,20 @@ func (s *ArduinoCoreServerImpl) SupportedUserFields(ctx context.Context, req *rp
 // BurnBootloader FIXMEDOC
 func (s *ArduinoCoreServerImpl) BurnBootloader(req *rpc.BurnBootloaderRequest, stream rpc.ArduinoCoreService_BurnBootloaderServer) error {
 	syncSend := NewSynchronizedSend(stream.Send)
-	outStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.BurnBootloaderResponse{OutStream: data}) })
-	errStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.BurnBootloaderResponse{ErrStream: data}) })
+	outStream := feedStreamTo(func(data []byte) {
+		syncSend.Send(&rpc.BurnBootloaderResponse{
+			Message: &rpc.BurnBootloaderResponse_OutStream{
+				OutStream: data,
+			},
+		})
+	})
+	errStream := feedStreamTo(func(data []byte) {
+		syncSend.Send(&rpc.BurnBootloaderResponse{
+			Message: &rpc.BurnBootloaderResponse_ErrStream{
+				ErrStream: data,
+			},
+		})
+	})
 	resp, err := upload.BurnBootloader(stream.Context(), req, outStream, errStream)
 	outStream.Close()
 	errStream.Close()

docs/UPGRADING.md

@@ -72,6 +72,116 @@ breaking changes as needed.
   { "sketch_path": "/tmp/my_sketch" }
   ```
 
+### The gRPC response `cc.arduino.cli.commands.v1.CompileResponse` has been changed.
+
+The `CompilerResponse` message has been refactored to made explicit which fields are intended for streaming the build
+process and which fields are part of the build result.
+
+The old `CompilerResposne`:
+
+```protoc
+message CompileResponse {
+  // The output of the compilation process (stream)
+  bytes out_stream = 1;
+  // The error output of the compilation process (stream)
+  bytes err_stream = 2;
+  // The compiler build path
+  string build_path = 3;
+  // The libraries used in the build
+  repeated Library used_libraries = 4;
+  // The size of the executable split by sections
+  repeated ExecutableSectionSize executable_sections_size = 5;
+  // The platform where the board is defined
+  InstalledPlatformReference board_platform = 6;
+  // The platform used for the build (if referenced from the board platform)
+  InstalledPlatformReference build_platform = 7;
+  // Completions reports of the compilation process (stream)
+  TaskProgress progress = 8;
+  // Build properties used for compiling
+  repeated string build_properties = 9;
+  // Compiler errors and warnings
+  repeated CompileDiagnostic diagnostics = 10;
+}
+```
+
+has been split into a `CompilerResponse` and a `BuilderResult`:
+
+```protoc
+message CompileResponse {
+  oneof message {
+    // The output of the compilation process (stream)
+    bytes out_stream = 1;
+    // The error output of the compilation process (stream)
+    bytes err_stream = 2;
+    // Completions reports of the compilation process (stream)
+    TaskProgress progress = 3;
+    // The compilation result
+    BuilderResult result = 4;
+  }
+}
+
+message BuilderResult {
+  // The compiler build path
+  string build_path = 1;
+  // The libraries used in the build
+  repeated Library used_libraries = 2;
+  // The size of the executable split by sections
+  repeated ExecutableSectionSize executable_sections_size = 3;
+  // The platform where the board is defined
+  InstalledPlatformReference board_platform = 4;
+  // The platform used for the build (if referenced from the board platform)
+  InstalledPlatformReference build_platform = 5;
+  // Build properties used for compiling
+  repeated string build_properties = 7;
+  // Compiler errors and warnings
+  repeated CompileDiagnostic diagnostics = 8;
+}
+```
+
+with a clear distinction on which fields are streamed.
+
+### The gRPC response `cc.arduino.cli.commands.v1.UploadUsingProgrammerResponse` and `cc.arduino.cli.commands.v1.BurnBootloaderResponse` has been changed.
+
+The old messages:
+
+```protoc
+message UploadUsingProgrammerResponse {
+  // The output of the upload process.
+  bytes out_stream = 1;
+  // The error output of the upload process.
+  bytes err_stream = 2;
+}
+
+message BurnBootloaderResponse {
+  // The output of the burn bootloader process.
+  bytes out_stream = 1;
+  // The error output of the burn bootloader process.
+  bytes err_stream = 2;
+}
+```
+
+now have the `oneof` clause that makes explicit the streaming nature of the response:
+
+```protoc
+message UploadUsingProgrammerResponse {
+  oneof message {
+    // The output of the upload process.
+    bytes out_stream = 1;
+    // The error output of the upload process.
+    bytes err_stream = 2;
+  }
+}
+
+message BurnBootloaderResponse {
+  oneof message {
+    // The output of the burn bootloader process.
+    bytes out_stream = 1;
+    // The error output of the burn bootloader process.
+    bytes err_stream = 2;
+  }
+}
+```
+
 ### The gRPC `cc.arduino.cli.commands.v1.PlatformRelease` has been changed.
 
 We've added a new field called `compatible`. This field indicates if the current platform release is installable or not.

internal/cli/compile/compile.go

@@ -245,7 +245,7 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 		SkipLibrariesDiscovery:        skipLibrariesDiscovery,
 		DoNotExpandBuildProperties:    showProperties == arguments.ShowPropertiesUnexpanded,
 	}
-	compileRes, compileError := compile.Compile(context.Background(), compileRequest, stdOut, stdErr, nil)
+	builderRes, compileError := compile.Compile(context.Background(), compileRequest, stdOut, stdErr, nil)
 
 	var uploadRes *rpc.UploadResult
 	if compileError == nil && uploadAfterCompile {
@@ -300,7 +300,7 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 
 		libs := ""
 		hasVendoredLibs := false
-		for _, lib := range compileRes.GetUsedLibraries() {
+		for _, lib := range builderRes.GetUsedLibraries() {
 			if lib.GetLocation() != rpc.LibraryLocation_LIBRARY_LOCATION_USER && lib.GetLocation() != rpc.LibraryLocation_LIBRARY_LOCATION_UNMANAGED {
 				continue
 			}
@@ -325,13 +325,13 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 		profileOut += fmt.Sprintln("  " + newProfileName + ":")
 		profileOut += fmt.Sprintln("    fqbn: " + compileRequest.GetFqbn())
 		profileOut += fmt.Sprintln("    platforms:")
-		boardPlatform := compileRes.GetBoardPlatform()
+		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 := compileRes.GetBuildPlatform(); buildPlatform != nil &&
+		if buildPlatform := builderRes.GetBuildPlatform(); buildPlatform != nil &&
 			buildPlatform.GetId() != boardPlatform.GetId() &&
 			buildPlatform.GetVersion() != boardPlatform.GetVersion() {
 			profileOut += fmt.Sprintln("      - platform: " + buildPlatform.GetId() + " (" + buildPlatform.GetVersion() + ")")
@@ -350,12 +350,12 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 	res := &compileResult{
 		CompilerOut:   stdIO.Stdout,
 		CompilerErr:   stdIO.Stderr,
-		BuilderResult: result.NewCompileResponse(compileRes),
+		BuilderResult: result.NewBuilderResult(builderRes),
 		UploadResult: updatedUploadPortResult{
 			UpdatedUploadPort: result.NewPort(uploadRes.GetUpdatedUploadPort()),
 		},
 		ProfileOut:         profileOut,
-		Diagnostics:        result.NewCompileDiagnostics(compileRes.GetDiagnostics()),
+		Diagnostics:        result.NewCompileDiagnostics(builderRes.GetDiagnostics()),
 		Success:            compileError == nil,
 		showPropertiesMode: showProperties,
 		hideStats:          preprocess,
@@ -401,7 +401,7 @@ type updatedUploadPortResult struct {
 type compileResult struct {
 	CompilerOut        string                      `json:"compiler_out"`
 	CompilerErr        string                      `json:"compiler_err"`
-	BuilderResult      *result.CompileResponse     `json:"builder_result"`
+	BuilderResult      *result.BuilderResult       `json:"builder_result"`
 	UploadResult       updatedUploadPortResult     `json:"upload_result"`
 	Success            bool                        `json:"success"`
 	ProfileOut         string                      `json:"profile_out,omitempty"`

internal/cli/feedback/result/rpc.go

@@ -909,9 +909,7 @@ func NewMonitorPortSettingDescriptor(m *rpc.MonitorPortSettingDescriptor) *Monit
 	}
 }
 
-type CompileResponse struct {
-	OutStream              []byte                      `json:"out_stream,omitempty"`
-	ErrStream              []byte                      `json:"err_stream,omitempty"`
+type BuilderResult struct {
 	BuildPath              string                      `json:"build_path,omitempty"`
 	UsedLibraries          []*Library                  `json:"used_libraries,omitempty"`
 	ExecutableSectionsSize []*ExecutableSectionSize    `json:"executable_sections_size,omitempty"`
@@ -921,7 +919,7 @@ type CompileResponse struct {
 	Diagnostics            []*CompileDiagnostic        `json:"diagnostics,omitempty"`
 }
 
-func NewCompileResponse(c *rpc.CompileResponse) *CompileResponse {
+func NewBuilderResult(c *rpc.BuilderResult) *BuilderResult {
 	if c == nil {
 		return nil
 	}
@@ -934,9 +932,7 @@ func NewCompileResponse(c *rpc.CompileResponse) *CompileResponse {
 		executableSectionsSizes[i] = NewExecutableSectionSize(v)
 	}
 
-	return &CompileResponse{
-		OutStream:              c.GetOutStream(),
-		ErrStream:              c.GetErrStream(),
+	return &BuilderResult{
 		BuildPath:              c.GetBuildPath(),
 		UsedLibraries:          usedLibs,
 		ExecutableSectionsSize: executableSectionsSizes,

internal/cli/feedback/result/rpc_test.go

@@ -190,9 +190,9 @@ func TestAllFieldAreMapped(t *testing.T) {
 	monitorPortSettingDescriptorResult := result.NewMonitorPortSettingDescriptor(monitorPortSettingDescriptorRpc)
 	mustContainsAllPropertyOfRpcStruct(t, monitorPortSettingDescriptorRpc, monitorPortSettingDescriptorResult)
 
-	compileResponseRpc := &rpc.CompileResponse{}
-	compileResponseResult := result.NewCompileResponse(compileResponseRpc)
-	mustContainsAllPropertyOfRpcStruct(t, compileResponseRpc, compileResponseResult, "progress")
+	builderResultRpc := &rpc.BuilderResult{}
+	builderResultResult := result.NewBuilderResult(builderResultRpc)
+	mustContainsAllPropertyOfRpcStruct(t, builderResultRpc, builderResultResult)
 
 	executableSectionSizeRpc := &rpc.ExecutableSectionSize{}
 	executableSectionSizeResult := result.NewExecutableSectionSize(executableSectionSizeRpc)

internal/integrationtest/compile_3/compile_show_properties_test.go

@@ -20,13 +20,13 @@ import (
 	"testing"
 
 	"github.com/arduino/arduino-cli/internal/integrationtest"
-	"github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-properties-orderedmap"
 	"github.com/stretchr/testify/require"
 )
 
 type cliCompileResponse struct {
-	BuilderResult *commands.CompileResponse `json:"builder_result"`
+	BuilderResult *rpc.BuilderResult `json:"builder_result"`
 }
 
 func TestCompileShowProperties(t *testing.T) {