Refactoring 'compile' commands

arduino · cmaglie · Aug 30, 2021 · Feb 10, 2021 · Jun 25, 2021 · Jun 30, 2021
commit ce3f802820ee87870f8022e87403d80bc925b2b5
diff --git a/arduino/libraries/libraries.go b/arduino/libraries/libraries.go
@@ -111,7 +111,7 @@ func (library *Library) ToRPCLibrary() (*rpc.Library, error) {
 		var err error
 		headers, err = library.SourceHeaders()
 		if err != nil {
-			return nil, fmt.Errorf(tr("gathering library headers: %w"), err)
+			return nil, fmt.Errorf(tr("reading library headers: %w"), err)
 		}
 	}
 

diff --git a/cli/compile/compile.go b/cli/compile/compile.go
@@ -29,7 +29,6 @@ import (
 	"github.com/arduino/arduino-cli/cli/output"
 	"github.com/arduino/arduino-cli/configuration"
 	"github.com/arduino/arduino-cli/i18n"
-	"google.golang.org/grpc/status"
 
 	"github.com/arduino/arduino-cli/cli/errorcodes"
 	"github.com/arduino/arduino-cli/cli/instance"
@@ -179,18 +178,18 @@ func run(cmd *cobra.Command, args []string) {
 		SourceOverride:                overrides,
 		Library:                       library,
 	}
-	compileOut := new(bytes.Buffer)
-	compileErr := new(bytes.Buffer)
+	compileStdOut := new(bytes.Buffer)
+	compileStdErr := new(bytes.Buffer)
 	verboseCompile := configuration.Settings.GetString("logging.level") == "debug"
 	var compileRes *rpc.CompileResponse
-	var st *status.Status
+	var compileError error
 	if output.OutputFormat == "json" {
-		compileRes, st = compile.Compile(context.Background(), compileRequest, compileOut, compileErr, verboseCompile)
+		compileRes, compileError = compile.Compile(context.Background(), compileRequest, compileStdOut, compileStdErr, verboseCompile)
 	} else {
-		compileRes, st = compile.Compile(context.Background(), compileRequest, os.Stdout, os.Stderr, verboseCompile)
+		compileRes, compileError = compile.Compile(context.Background(), compileRequest, os.Stdout, os.Stderr, verboseCompile)
 	}
 
-	if st == nil && uploadAfterCompile {
+	if compileError == nil && uploadAfterCompile {
 		var sk *sketch.Sketch
 		sk, err := sketch.New(sketchPath)
 		if err != nil {
@@ -248,13 +247,13 @@ func run(cmd *cobra.Command, args []string) {
 	}
 
 	feedback.PrintResult(&compileResult{
-		CompileOut:    compileOut.String(),
-		CompileErr:    compileErr.String(),
+		CompileOut:    compileStdOut.String(),
+		CompileErr:    compileStdErr.String(),
 		BuilderResult: compileRes,
-		Success:       st == nil,
+		Success:       compileError == nil,
 	})
-	if st != nil && output.OutputFormat != "json" {
-		feedback.Errorf(tr("Error during build: %v"), st.Message())
+	if compileError != nil && output.OutputFormat != "json" {
+		feedback.Errorf(tr("Error during build: %v"), compileError)
 		os.Exit(errorcodes.ErrGeneric)
 	}
 }

diff --git a/commands/compile/compile.go b/commands/compile/compile.go
@@ -17,6 +17,7 @@ package compile
 
 import (
 	"context"
+	"errors"
 	"io"
 	"path/filepath"
 	"sort"
@@ -37,17 +38,14 @@ import (
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	paths "github.com/arduino/go-paths-helper"
 	properties "github.com/arduino/go-properties-orderedmap"
-	"github.com/pkg/errors"
 	"github.com/segmentio/stats/v4"
 	"github.com/sirupsen/logrus"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 var tr = i18n.Tr
 
 // Compile FIXMEDOC
-func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream io.Writer, debug bool) (r *rpc.CompileResponse, e *status.Status) {
+func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream io.Writer, debug bool) (r *rpc.CompileResponse, 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
@@ -91,29 +89,29 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 
 	pm := commands.GetPackageManager(req.GetInstance().GetId())
 	if pm == nil {
-		return nil, status.New(codes.InvalidArgument, tr("Invalid instance"))
+		return nil, &commands.InvalidInstanceError{}
 	}
 
 	logrus.Tracef("Compile %s for %s started", req.GetSketchPath(), req.GetFqbn())
 	if req.GetSketchPath() == "" {
-		return nil, status.New(codes.InvalidArgument, tr("Missing sketch path"))
+		return nil, &commands.MissingSketchPathError{}
 	}
 	sketchPath := paths.New(req.GetSketchPath())
 	sk, err := sketch.New(sketchPath)
 	if err != nil {
-		return nil, status.Newf(codes.NotFound, tr("Error opening sketch: %s"), err)
+		return nil, &commands.SketchNotFoundError{Cause: err}
 	}
 
 	fqbnIn := req.GetFqbn()
 	if fqbnIn == "" && sk != nil && sk.Metadata != nil {
 		fqbnIn = sk.Metadata.CPU.Fqbn
 	}
 	if fqbnIn == "" {
-		return nil, status.New(codes.InvalidArgument, tr("No FQBN (Fully Qualified Board Name) provided"))
+		return nil, &commands.MissingFQBNError{}
 	}
 	fqbn, err := cores.ParseFQBN(fqbnIn)
 	if err != nil {
-		return nil, status.Newf(codes.InvalidArgument, tr("Invalid FQBN: %s"), err)
+		return nil, &commands.InvalidFQBNError{Cause: err}
 	}
 
 	targetPlatform := pm.FindPlatform(&packagemanager.PlatformReference{
@@ -126,7 +124,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 		// 	"\"%[1]s:%[2]s\" platform is not installed, please install it by running \""+
 		// 		version.GetAppName()+" core install %[1]s:%[2]s\".", fqbn.Package, fqbn.PlatformArch)
 		// feedback.Error(errorMessage)
-		return nil, status.New(codes.NotFound, tr("Platform not installed"))
+		return nil, &commands.PlatformNotFound{Platform: targetPlatform.String(), Cause: errors.New(tr("platform not installed"))}
 	}
 
 	builderCtx := &types.Context{}
@@ -149,7 +147,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 		builderCtx.BuildPath = paths.New(req.GetBuildPath())
 	}
 	if err = builderCtx.BuildPath.MkdirAll(); err != nil {
-		return nil, status.Newf(codes.PermissionDenied, tr("Cannot create build directory: %s"), err)
+		return nil, &commands.PermissionDeniedError{Message: tr("Cannot create build directory"), Cause: err}
 	}
 	builderCtx.CompilationDatabase = bldr.NewCompilationDatabase(
 		builderCtx.BuildPath.Join("compile_commands.json"),
@@ -179,7 +177,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 		builderCtx.BuildCachePath = paths.New(req.GetBuildCachePath())
 		err = builderCtx.BuildCachePath.MkdirAll()
 		if err != nil {
-			return nil, status.Newf(codes.PermissionDenied, tr("Cannot create build cache directory: %s"), err)
+			return nil, &commands.PermissionDeniedError{Message: tr("Cannot create build cache directory"), Cause: err}
 		}
 	}
 
@@ -222,14 +220,22 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 
 	// if --preprocess or --show-properties were passed, we can stop here
 	if req.GetShowProperties() {
-		return r, status.Convert(builder.RunParseHardwareAndDumpBuildProperties(builderCtx))
+		compileErr := builder.RunParseHardwareAndDumpBuildProperties(builderCtx)
+		if compileErr != nil {
+			compileErr = &commands.CompileFailedError{Message: err.Error()}
+		}
+		return r, compileErr
 	} else if req.GetPreprocess() {
-		return r, status.Convert(builder.RunPreprocess(builderCtx))
+		compileErr := builder.RunPreprocess(builderCtx)
+		if compileErr != nil {
+			compileErr = &commands.CompileFailedError{Message: err.Error()}
+		}
+		return r, compileErr
 	}
 
 	// if it's a regular build, go on...
 	if err := builder.RunBuilder(builderCtx); err != nil {
-		return r, status.Convert(err)
+		return r, &commands.CompileFailedError{Message: err.Error()}
 	}
 
 	// If the export directory is set we assume you want to export the binaries
@@ -251,17 +257,17 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 		}
 		logrus.WithField("path", exportPath).Trace("Saving sketch to export path.")
 		if err := exportPath.MkdirAll(); err != nil {
-			return r, status.New(codes.PermissionDenied, errors.Wrap(err, tr("Error creating output dir")).Error())
+			return r, &commands.PermissionDeniedError{Message: tr("Error creating output dir"), Cause: err}
 		}
 
 		// Copy all "sketch.ino.*" artifacts to the export directory
 		baseName, ok := builderCtx.BuildProperties.GetOk("build.project_name") // == "sketch.ino"
 		if !ok {
-			return r, status.New(codes.Internal, tr("Missing 'build.project_name' build property"))
+			return r, &commands.MissingPlatformPropertyError{Property: "build.project_name"}
 		}
 		buildFiles, err := builderCtx.BuildPath.ReadDir()
 		if err != nil {
-			return r, status.Newf(codes.PermissionDenied, tr("Error reading build directory: %s"), err)
+			return r, &commands.PermissionDeniedError{Message: tr("Error reading build directory"), Cause: err}
 		}
 		buildFiles.FilterPrefix(baseName)
 		for _, buildFile := range buildFiles {
@@ -271,7 +277,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 				WithField("dest", exportedFile).
 				Trace("Copying artifact.")
 			if err = buildFile.CopyTo(exportedFile); err != nil {
-				return r, status.New(codes.PermissionDenied, tr("Error copying output file %[1]s: %[2]s", buildFile, err))
+				return r, &commands.PermissionDeniedError{Message: tr("Error copying output file %s", buildFile), Cause: err}
 			}
 		}
 	}
@@ -280,7 +286,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 	for _, lib := range builderCtx.ImportedLibraries {
 		rpcLib, err := lib.ToRPCLibrary()
 		if err != nil {
-			return r, status.Newf(codes.PermissionDenied, tr("Error converting library %[1]s to rpc struct: %[2]s", lib.Name, err))
+			return r, &commands.PermissionDeniedError{Message: tr("Error getting information for library %s", lib.Name), Cause: err}
 		}
 		importedLibs = append(importedLibs, rpcLib)
 	}

diff --git a/commands/errors.go b/commands/errors.go
@@ -171,6 +171,30 @@ func (e *MissingPlatformPropertyError) ToRPCStatus() *status.Status {
 	return status.New(codes.FailedPrecondition, e.Error())
 }
 
+// PlatformNotFound is returned when a platform is not found
+type PlatformNotFound struct {
+	Platform string
+}
+
+func (e *PlatformNotFound) Error() string {
+	return tr("Platform '%s' is not installed", e.Platform)
+}
+
+func (e *PlatformNotFound) ToRPCStatus() *status.Status {
+	return status.New(codes.FailedPrecondition, e.Error())
+}
+
+// MissingSketchPathError is returned when the sketch path is mandatory and not specified
+type MissingSketchPathError struct{}
+
+func (e *MissingSketchPathError) Error() string {
+	return tr("Missing sketch path")
+}
+
+func (e *MissingSketchPathError) ToRPCStatus() *status.Status {
+	return status.New(codes.InvalidArgument, e.Error())
+}
+
 // SketchNotFoundError is returned when the sketch is not found
 type SketchNotFoundError struct {
 	Cause error
@@ -206,6 +230,24 @@ func (e *FailedUploadError) ToRPCStatus() *status.Status {
 	return status.New(codes.Internal, e.Error())
 }
 
+// CompileFailedError is returned when the compile fails
+type CompileFailedError struct {
+	Message string
+	Cause   error
+}
+
+func (e *CompileFailedError) Error() string {
+	return composeErrorMsg(e.Message, e.Cause)
+}
+
+func (e *CompileFailedError) Unwrap() error {
+	return e.Cause
+}
+
+func (e *CompileFailedError) ToRPCStatus() *status.Status {
+	return status.New(codes.Internal, e.Error())
+}
+
 // InvalidArgumentError is returned when an invalid argument is passed to the command
 type InvalidArgumentError struct {
 	Message string