Add support for compile with debug optimizations (#593)

* add debug symbols option in CLI and gRPC insterface

* Calculate precompiled core path usign also build optimization flags

* Align failing lecacy tests with new hashing behaviour

Author: Roberto Sora

Diff for: cli/compile/compile.go

@@ -48,6 +48,7 @@ var (
 	verify             bool     // Upload, verify uploaded binary after the upload.
 	exportFile         string   // The compiled binary is written to this file
 	libraries          []string // List of custom libraries paths separated by commas. Or can be used multiple times for multiple libraries paths.
+	optimizeForDebug   bool     // Optimize compile output for debug, not for release
 )
 
 // NewCommand created a new `compile` command
@@ -80,6 +81,7 @@ func NewCommand() *cobra.Command {
 	command.Flags().StringVar(&vidPid, "vid-pid", "", "When specified, VID/PID specific build properties are used, if boards supports them.")
 	command.Flags().StringSliceVar(&libraries, "libraries", []string{},
 		"List of custom libraries paths separated by commas. Or can be used multiple times for multiple libraries paths.")
+	command.Flags().BoolVar(&optimizeForDebug, "optimize-for-debug", false, "Optional, optimize compile output for debug, not for release.")
 
 	return command
 }
@@ -99,20 +101,21 @@ func run(cmd *cobra.Command, args []string) {
 	sketchPath := initSketchPath(path)
 
 	_, err = compile.Compile(context.Background(), &rpc.CompileReq{
-		Instance:        inst,
-		Fqbn:            fqbn,
-		SketchPath:      sketchPath.String(),
-		ShowProperties:  showProperties,
-		Preprocess:      preprocess,
-		BuildCachePath:  buildCachePath,
-		BuildPath:       buildPath,
-		BuildProperties: buildProperties,
-		Warnings:        warnings,
-		Verbose:         verbose,
-		Quiet:           quiet,
-		VidPid:          vidPid,
-		ExportFile:      exportFile,
-		Libraries:       libraries,
+		Instance:         inst,
+		Fqbn:             fqbn,
+		SketchPath:       sketchPath.String(),
+		ShowProperties:   showProperties,
+		Preprocess:       preprocess,
+		BuildCachePath:   buildCachePath,
+		BuildPath:        buildPath,
+		BuildProperties:  buildProperties,
+		Warnings:         warnings,
+		Verbose:          verbose,
+		Quiet:            quiet,
+		VidPid:           vidPid,
+		ExportFile:       exportFile,
+		Libraries:        libraries,
+		OptimizeForDebug: optimizeForDebug,
 	}, os.Stdout, os.Stderr, viper.GetString("logging.level") == "debug")
 
 	if err != nil {

Diff for: commands/compile/compile.go

@@ -103,6 +103,9 @@ func Compile(ctx context.Context, req *rpc.CompileReq, outStream, errStream io.W
 
 	builderCtx.Verbose = req.GetVerbose()
 
+	// Optimize for debug
+	builderCtx.OptimizeForDebug = req.GetOptimizeForDebug()
+
 	builderCtx.CoreBuildCachePath = paths.TempDir().Join("arduino-core-cache")
 
 	builderCtx.Jobs = int(req.GetJobs())

Diff for: legacy/builder/builder_utils/utils.go

@@ -519,20 +519,3 @@ func PrepareCommandForRecipe(ctx *types.Context, buildProperties *properties.Map
 
 	return command, nil
 }
-
-// GetCachedCoreArchiveFileName returns the filename to be used to store
-// the global cached core.a.
-func GetCachedCoreArchiveFileName(fqbn string, coreFolder *paths.Path) string {
-	fqbnToUnderscore := strings.Replace(fqbn, ":", "_", -1)
-	fqbnToUnderscore = strings.Replace(fqbnToUnderscore, "=", "_", -1)
-	if absCoreFolder, err := coreFolder.Abs(); err == nil {
-		coreFolder = absCoreFolder
-	} // silently continue if absolute path can't be detected
-	hash := utils.MD5Sum([]byte(coreFolder.String()))
-	realName := "core_" + fqbnToUnderscore + "_" + hash + ".a"
-	if len(realName) > 100 {
-		// avoid really long names, simply hash the final part
-		realName = "core_" + utils.MD5Sum([]byte(fqbnToUnderscore+"_"+hash)) + ".a"
-	}
-	return realName
-}

Diff for: legacy/builder/create_build_options_map.go

@@ -17,7 +17,6 @@ package builder
 
 import (
 	"encoding/json"
-
 	"github.com/arduino/arduino-cli/legacy/builder/i18n"
 	"github.com/arduino/arduino-cli/legacy/builder/types"
 )
@@ -30,7 +29,6 @@ func (s *CreateBuildOptionsMap) Run(ctx *types.Context) error {
 	if err != nil {
 		return i18n.WrapError(err)
 	}
-
 	ctx.BuildOptionsJson = string(bytes)
 
 	return nil

Diff for: legacy/builder/phases/core_builder.go

@@ -17,6 +17,7 @@ package phases
 
 import (
 	"os"
+	"strings"
 
 	"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
 	"github.com/arduino/arduino-cli/legacy/builder/constants"
@@ -90,7 +91,8 @@ func compileCore(ctx *types.Context, buildPath *paths.Path, buildCachePath *path
 
 	var targetArchivedCore *paths.Path
 	if buildCachePath != nil {
-		archivedCoreName := builder_utils.GetCachedCoreArchiveFileName(buildProperties.Get(constants.BUILD_PROPERTIES_FQBN), realCoreFolder)
+		archivedCoreName := GetCachedCoreArchiveFileName(buildProperties.Get(constants.BUILD_PROPERTIES_FQBN),
+			buildProperties.Get("compiler.optimization_flags"), realCoreFolder)
 		targetArchivedCore = buildCachePath.Join(archivedCoreName)
 		canUseArchivedCore := !builder_utils.CoreOrReferencedCoreHasChanged(realCoreFolder, targetCoreFolder, targetArchivedCore)
 
@@ -129,3 +131,20 @@ func compileCore(ctx *types.Context, buildPath *paths.Path, buildCachePath *path
 
 	return archiveFile, variantObjectFiles, nil
 }
+
+// GetCachedCoreArchiveFileName returns the filename to be used to store
+// the global cached core.a.
+func GetCachedCoreArchiveFileName(fqbn string, optimizationFlags string, coreFolder *paths.Path) string {
+	fqbnToUnderscore := strings.Replace(fqbn, ":", "_", -1)
+	fqbnToUnderscore = strings.Replace(fqbnToUnderscore, "=", "_", -1)
+	if absCoreFolder, err := coreFolder.Abs(); err == nil {
+		coreFolder = absCoreFolder
+	} // silently continue if absolute path can't be detected
+	hash := utils.MD5Sum([]byte(coreFolder.String() + optimizationFlags))
+	realName := "core_" + fqbnToUnderscore + "_" + hash + ".a"
+	if len(realName) > 100 {
+		// avoid really long names, simply hash the final part
+		realName = "core_" + utils.MD5Sum([]byte(fqbnToUnderscore+"_"+hash)) + ".a"
+	}
+	return realName
+}

Diff for: legacy/builder/setup_build_properties.go

@@ -69,6 +69,17 @@ func (s *SetupBuildProperties) Run(ctx *types.Context) error {
 	buildProperties.Set("ide_version", ctx.ArduinoAPIVersion)
 	buildProperties.Set("runtime.os", utils.PrettyOSName())
 
+	if ctx.OptimizeForDebug {
+		if buildProperties.ContainsKey("compiler.optimization_flags.debug") {
+			buildProperties.Set("compiler.optimization_flags", buildProperties.Get("compiler.optimization_flags.debug"))
+		}
+	} else {
+		if buildProperties.ContainsKey("compiler.optimization_flags.release") {
+			buildProperties.Set("compiler.optimization_flags", buildProperties.Get("compiler.optimization_flags.release"))
+		}
+	}
+	ctx.OptimizationFlags = buildProperties.Get("compiler.optimization_flags")
+
 	variant := buildProperties.Get("build.variant")
 	if variant == "" {
 		buildProperties.Set("build.variant.path", "")

Diff for: legacy/builder/test/builder_test.go

@@ -24,8 +24,8 @@ import (
 	"github.com/arduino/go-paths-helper"
 
 	"github.com/arduino/arduino-cli/legacy/builder"
-	"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
 	"github.com/arduino/arduino-cli/legacy/builder/constants"
+	"github.com/arduino/arduino-cli/legacy/builder/phases"
 	"github.com/arduino/arduino-cli/legacy/builder/types"
 	"github.com/stretchr/testify/require"
 )
@@ -379,7 +379,7 @@ func TestBuilderCacheCoreAFile(t *testing.T) {
 
 	// Pick timestamp of cached core
 	coreFolder := paths.New("downloaded_hardware", "arduino", "avr")
-	coreFileName := builder_utils.GetCachedCoreArchiveFileName(ctx.FQBN.String(), coreFolder)
+	coreFileName := phases.GetCachedCoreArchiveFileName(ctx.FQBN.String(), ctx.OptimizationFlags, coreFolder)
 	cachedCoreFile := ctx.CoreBuildCachePath.Join(coreFileName)
 	coreStatBefore, err := cachedCoreFile.Stat()
 	require.NoError(t, err)

Diff for: legacy/builder/test/create_build_options_map_test.go

@@ -35,6 +35,7 @@ func TestCreateBuildOptionsMap(t *testing.T) {
 		Verbose:            true,
 		BuildPath:          paths.New("buildPath"),
 		DebugLevel:         5,
+		OptimizationFlags:  "-Os",
 	}
 
 	create := builder.CreateBuildOptionsMap{}
@@ -45,6 +46,7 @@ func TestCreateBuildOptionsMap(t *testing.T) {
   "additionalFiles": "",
   "builtInLibrariesFolders": "",
   "builtInToolsFolders": "tools",
+  "compiler.optimization_flags": "-Os",
   "customBuildProperties": "",
   "fqbn": "my:nice:fqbn",
   "hardwareFolders": "hardware,hardware2",

Diff for: legacy/builder/test/store_build_options_map_test.go

@@ -37,6 +37,7 @@ func TestStoreBuildOptionsMap(t *testing.T) {
 		CustomBuildProperties: []string{"custom=prop"},
 		Verbose:               true,
 		DebugLevel:            5,
+		OptimizationFlags:     "-Os",
 	}
 
 	buildPath := SetupBuildPath(t, ctx)
@@ -63,6 +64,7 @@ func TestStoreBuildOptionsMap(t *testing.T) {
   "additionalFiles": "",
   "builtInLibrariesFolders": "built-in libraries",
   "builtInToolsFolders": "tools",
+  "compiler.optimization_flags": "-Os",
   "customBuildProperties": "custom=prop",
   "fqbn": "my:nice:fqbn",
   "hardwareFolders": "hardware",

Diff for: legacy/builder/types/context.go

@@ -94,6 +94,10 @@ type Context struct {
 	Verbose           bool
 	DebugPreprocessor bool
 
+	// Compile optimization settings
+	OptimizeForDebug  bool
+	OptimizationFlags string
+
 	// Dry run, only create progress map
 	Progress ProgressStruct
 
@@ -140,6 +144,7 @@ func (ctx *Context) ExtractBuildOptions() *properties.Map {
 	opts.Set("runtime.ide.version", ctx.ArduinoAPIVersion)
 	opts.Set("customBuildProperties", strings.Join(ctx.CustomBuildProperties, ","))
 	opts.Set("additionalFiles", strings.Join(additionalFilesRelative, ","))
+	opts.Set("compiler.optimization_flags", ctx.OptimizationFlags)
 	return opts
 }
 
@@ -156,6 +161,7 @@ func (ctx *Context) InjectBuildOptions(opts *properties.Map) {
 	ctx.FQBN = fqbn
 	ctx.ArduinoAPIVersion = opts.Get("runtime.ide.version")
 	ctx.CustomBuildProperties = strings.Split(opts.Get("customBuildProperties"), ",")
+	ctx.OptimizationFlags = opts.Get("compiler.optimization_flags")
 }
 
 func (ctx *Context) GetLogger() i18n.Logger {

Diff for: rpc/commands/compile.pb.go

@@ -37,6 +37,7 @@ message CompileReq {
   string exportFile = 13;   // The compiled binary is written to this file
   int32 jobs = 14;   // The max number of concurrent compiler instances to run (as make -jx)
   repeated string libraries = 15; // List of custom libraries paths separated by commas. Or can be used multiple times for multiple libraries paths.
+  bool optimizeForDebug = 16; // Optimize compile output for debug, not for release
 }
 
 message CompileResp {