[skip-changelog] legacy: refacored makeSourceFile / other minor code moved (#2303)

* Slightly de-entangled makeSourceFile

* Moved some initializations directly into NewBuilder

* Pruned constants

* Pruned more constants

* Fixed lint warnings

Author: cmaglie

Diff for: arduino/builder/builder.go

@@ -18,31 +18,56 @@ package builder
 import (
 	"github.com/arduino/arduino-cli/arduino/sketch"
 	"github.com/arduino/go-paths-helper"
-)
-
-// nolint
-const (
-	BuildPropertiesArchiveFile          = "archive_file"
-	BuildPropertiesArchiveFilePath      = "archive_file_path"
-	BuildPropertiesObjectFile           = "object_file"
-	RecipeARPattern                     = "recipe.ar.pattern"
-	BuildPropertiesIncludes             = "includes"
-	BuildPropertiesCompilerWarningFlags = "compiler.warning_flags"
-	Space                               = " "
+	"github.com/arduino/go-properties-orderedmap"
 )
 
 // Builder is a Sketch builder.
 type Builder struct {
-	sketch *sketch.Sketch
+	sketch          *sketch.Sketch
+	buildProperties *properties.Map
 
 	// core related
 	coreBuildCachePath *paths.Path
 }
 
 // NewBuilder creates a sketch Builder.
-func NewBuilder(sk *sketch.Sketch, coreBuildCachePath *paths.Path) *Builder {
+func NewBuilder(
+	sk *sketch.Sketch,
+	boardBuildProperties *properties.Map,
+	buildPath *paths.Path,
+	optimizeForDebug bool,
+	coreBuildCachePath *paths.Path,
+) *Builder {
+	buildProperties := properties.NewMap()
+	if boardBuildProperties != nil {
+		buildProperties.Merge(boardBuildProperties)
+	}
+
+	if buildPath != nil {
+		buildProperties.SetPath("build.path", buildPath)
+	}
+	if sk != nil {
+		buildProperties.Set("build.project_name", sk.MainFile.Base())
+		buildProperties.SetPath("build.source.path", sk.FullPath)
+	}
+	if optimizeForDebug {
+		if debugFlags, ok := buildProperties.GetOk("compiler.optimization_flags.debug"); ok {
+			buildProperties.Set("compiler.optimization_flags", debugFlags)
+		}
+	} else {
+		if releaseFlags, ok := buildProperties.GetOk("compiler.optimization_flags.release"); ok {
+			buildProperties.Set("compiler.optimization_flags", releaseFlags)
+		}
+	}
+
 	return &Builder{
 		sketch:             sk,
+		buildProperties:    buildProperties,
 		coreBuildCachePath: coreBuildCachePath,
 	}
 }
+
+// GetBuildProperties returns the build properties for running this build
+func (b *Builder) GetBuildProperties() *properties.Map {
+	return b.buildProperties
+}

Diff for: arduino/builder/detector/detector.go

@@ -264,16 +264,16 @@ func (l *SketchLibrariesDetector) findIncludes(
 
 	if !l.useCachedLibrariesResolution {
 		sketch := sketch
-		mergedfile, err := makeSourceFile(sketchBuildPath, librariesBuildPath, sketch, paths.New(sketch.MainFile.Base()+".cpp"))
+		mergedfile, err := makeSourceFile(sketchBuildPath, sketchBuildPath, paths.New(sketch.MainFile.Base()+".cpp"))
 		if err != nil {
 			return errors.WithStack(err)
 		}
 		sourceFileQueue.push(mergedfile)
 
-		l.queueSourceFilesFromFolder(sketchBuildPath, librariesBuildPath, sourceFileQueue, sketch, sketchBuildPath, false /* recurse */)
+		l.queueSourceFilesFromFolder(sourceFileQueue, sketchBuildPath, false /* recurse */, sketchBuildPath, sketchBuildPath)
 		srcSubfolderPath := sketchBuildPath.Join("src")
 		if srcSubfolderPath.IsDir() {
-			l.queueSourceFilesFromFolder(sketchBuildPath, librariesBuildPath, sourceFileQueue, sketch, srcSubfolderPath, true /* recurse */)
+			l.queueSourceFilesFromFolder(sourceFileQueue, srcSubfolderPath, true /* recurse */, sketchBuildPath, sketchBuildPath)
 		}
 
 		for !sourceFileQueue.empty() {
@@ -419,20 +419,21 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
 			}
 		} else {
 			for _, sourceDir := range library.SourceDirs() {
-				l.queueSourceFilesFromFolder(sketchBuildPath, librariesBuildPath, sourceFileQueue, library, sourceDir.Dir, sourceDir.Recurse)
+				l.queueSourceFilesFromFolder(sourceFileQueue, sourceDir.Dir, sourceDir.Recurse,
+					library.SourceDir, librariesBuildPath.Join(library.DirName), library.UtilityDir)
 			}
 		}
 		first = false
 	}
 }
 
 func (l *SketchLibrariesDetector) queueSourceFilesFromFolder(
-	sketchBuildPath *paths.Path,
-	librariesBuildPath *paths.Path,
 	sourceFileQueue *uniqueSourceFileQueue,
-	origin interface{},
 	folder *paths.Path,
 	recurse bool,
+	sourceDir *paths.Path,
+	buildDir *paths.Path,
+	extraIncludePath ...*paths.Path,
 ) error {
 	sourceFileExtensions := []string{}
 	for k := range globals.SourceFilesValidExtensions {
@@ -444,7 +445,7 @@ func (l *SketchLibrariesDetector) queueSourceFilesFromFolder(
 	}
 
 	for _, filePath := range filePaths {
-		sourceFile, err := makeSourceFile(sketchBuildPath, librariesBuildPath, origin, filePath)
+		sourceFile, err := makeSourceFile(sourceDir, buildDir, filePath, extraIncludePath...)
 		if err != nil {
 			return errors.WithStack(err)
 		}
@@ -537,33 +538,42 @@ func (f *sourceFile) Equals(g *sourceFile) bool {
 // given origin. The given path can be absolute, or relative within the
 // origin's root source folder
 func makeSourceFile(
-	sketchBuildPath *paths.Path,
-	librariesBuildPath *paths.Path,
-	origin interface{},
-	path *paths.Path,
+	sourceDir *paths.Path,
+	buildDir *paths.Path,
+	sourceFilePath *paths.Path,
+	extraIncludePath ...*paths.Path,
 ) (*sourceFile, error) {
-	res := &sourceFile{}
-
-	switch o := origin.(type) {
-	case *sketch.Sketch:
-		res.buildRoot = sketchBuildPath
-		res.sourceRoot = sketchBuildPath
-	case *libraries.Library:
-		res.buildRoot = librariesBuildPath.Join(o.DirName)
-		res.sourceRoot = o.SourceDir
-		res.extraIncludePath = o.UtilityDir
-	default:
-		panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin))
-	}
-
-	if path.IsAbs() {
+	res := &sourceFile{
+		buildRoot:  buildDir,
+		sourceRoot: sourceDir,
+	}
+
+	if len(extraIncludePath) > 1 {
+		panic("only one extra include path allowed")
+	}
+	if len(extraIncludePath) > 0 {
+		res.extraIncludePath = extraIncludePath[0]
+	}
+	// switch o := origin.(type) {
+	// case *sketch.Sketch:
+	// 	res.buildRoot = sketchBuildPath
+	// 	res.sourceRoot = sketchBuildPath
+	// case *libraries.Library:
+	// 	res.buildRoot = librariesBuildPath.Join(o.DirName)
+	// 	res.sourceRoot = o.SourceDir
+	// 	res.extraIncludePath = o.UtilityDir
+	// default:
+	// 	panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin))
+	// }
+
+	if sourceFilePath.IsAbs() {
 		var err error
-		path, err = res.sourceRoot.RelTo(path)
+		sourceFilePath, err = res.sourceRoot.RelTo(sourceFilePath)
 		if err != nil {
 			return nil, err
 		}
 	}
-	res.relativePath = path
+	res.relativePath = sourceFilePath
 	return res, nil
 }
 

Diff for: arduino/builder/sketch.go

@@ -23,7 +23,6 @@ import (
 	"github.com/arduino/arduino-cli/arduino/builder/cpp"
 	"github.com/arduino/arduino-cli/i18n"
 	"github.com/arduino/go-paths-helper"
-	"github.com/arduino/go-properties-orderedmap"
 
 	"github.com/pkg/errors"
 )
@@ -166,29 +165,3 @@ func writeIfDifferent(source []byte, destPath *paths.Path) error {
 	// Source and destination are the same, don't write anything
 	return nil
 }
-
-// SetupBuildProperties adds the build properties related to the sketch to the
-// default board build properties map.
-func (b *Builder) SetupBuildProperties(boardBuildProperties *properties.Map, buildPath *paths.Path, optimizeForDebug bool) *properties.Map {
-	buildProperties := properties.NewMap()
-	buildProperties.Merge(boardBuildProperties)
-
-	if buildPath != nil {
-		buildProperties.SetPath("build.path", buildPath)
-	}
-	if b.sketch != nil {
-		buildProperties.Set("build.project_name", b.sketch.MainFile.Base())
-		buildProperties.SetPath("build.source.path", b.sketch.FullPath)
-	}
-	if optimizeForDebug {
-		if debugFlags, ok := buildProperties.GetOk("compiler.optimization_flags.debug"); ok {
-			buildProperties.Set("compiler.optimization_flags", debugFlags)
-		}
-	} else {
-		if releaseFlags, ok := buildProperties.GetOk("compiler.optimization_flags.release"); ok {
-			buildProperties.Set("compiler.optimization_flags", releaseFlags)
-		}
-	}
-
-	return buildProperties
-}

Diff for: arduino/builder/sketch_test.go

@@ -48,7 +48,7 @@ func TestMergeSketchSources(t *testing.T) {
 	}
 	mergedSources := strings.ReplaceAll(string(mergedBytes), "%s", pathToGoldenSource)
 
-	b := NewBuilder(sk, nil)
+	b := NewBuilder(sk, nil, nil, false, nil)
 	offset, source, err := b.sketchMergeSources(nil)
 	require.Nil(t, err)
 	require.Equal(t, 2, offset)
@@ -61,7 +61,7 @@ func TestMergeSketchSourcesArduinoIncluded(t *testing.T) {
 	require.NotNil(t, sk)
 
 	// ensure not to include Arduino.h when it's already there
-	b := NewBuilder(sk, nil)
+	b := NewBuilder(sk, nil, nil, false, nil)
 	_, source, err := b.sketchMergeSources(nil)
 	require.Nil(t, err)
 	require.Equal(t, 1, strings.Count(source, "<Arduino.h>"))
@@ -76,7 +76,7 @@ func TestCopyAdditionalFiles(t *testing.T) {
 	sk1, err := sketch.New(paths.New("testdata", t.Name()))
 	require.Nil(t, err)
 	require.Equal(t, sk1.AdditionalFiles.Len(), 1)
-	b1 := NewBuilder(sk1, nil)
+	b1 := NewBuilder(sk1, nil, nil, false, nil)
 
 	// copy the sketch over, create a fake main file we don't care about it
 	// but we need it for `SketchLoad` to succeed later

Diff for: arduino/builder/utils/utils.go

@@ -520,8 +520,8 @@ func compileFileWithRecipe(
 	verboseStdout, verboseInfo, errOut := &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}
 
 	properties := buildProperties.Clone()
-	properties.Set(builder.BuildPropertiesCompilerWarningFlags, properties.Get(builder.BuildPropertiesCompilerWarningFlags+"."+warningsLevel))
-	properties.Set(builder.BuildPropertiesIncludes, strings.Join(includes, builder.Space))
+	properties.Set("compiler.warning_flags", properties.Get("compiler.warning_flags."+warningsLevel))
+	properties.Set("includes", strings.Join(includes, " "))
 	properties.SetPath("source_file", source)
 	relativeSource, err := sourcePath.RelTo(source)
 	if err != nil {
@@ -530,7 +530,7 @@ func compileFileWithRecipe(
 	depsFile := buildPath.Join(relativeSource.String() + ".d")
 	objectFile := buildPath.Join(relativeSource.String() + ".o")
 
-	properties.SetPath(builder.BuildPropertiesObjectFile, objectFile)
+	properties.SetPath("object_file", objectFile)
 	err = objectFile.Parent().MkdirAll()
 	if err != nil {
 		return nil, nil, nil, nil, errors.WithStack(err)
@@ -615,11 +615,11 @@ func ArchiveCompiledFiles(
 
 	for _, objectFile := range objectFilesToArchive {
 		properties := buildProperties.Clone()
-		properties.Set(builder.BuildPropertiesArchiveFile, archiveFilePath.Base())
-		properties.SetPath(builder.BuildPropertiesArchiveFilePath, archiveFilePath)
-		properties.SetPath(builder.BuildPropertiesObjectFile, objectFile)
+		properties.Set("archive_file", archiveFilePath.Base())
+		properties.SetPath("archive_file_path", archiveFilePath)
+		properties.SetPath("object_file", objectFile)
 
-		command, err := PrepareCommandForRecipe(properties, builder.RecipeARPattern, false)
+		command, err := PrepareCommandForRecipe(properties, "recipe.ar.pattern", false)
 		if err != nil {
 			return nil, verboseInfobuf.Bytes(), errors.WithStack(err)
 		}

Diff for: commands/compile/compile.go

@@ -98,7 +98,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 	if err != nil {
 		return nil, &arduino.InvalidFQBNError{Cause: err}
 	}
-	targetPackage, targetPlatform, targetBoard, buildProperties, buildPlatform, err := pme.ResolveFQBN(fqbn)
+	targetPackage, targetPlatform, targetBoard, boardBuildProperties, buildPlatform, err := pme.ResolveFQBN(fqbn)
 	if err != nil {
 		if targetPlatform == nil {
 			return nil, &arduino.PlatformNotFoundError{
@@ -115,21 +115,21 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 
 	// Setup sign keys if requested
 	if req.KeysKeychain != "" {
-		buildProperties.Set("build.keys.keychain", req.GetKeysKeychain())
+		boardBuildProperties.Set("build.keys.keychain", req.GetKeysKeychain())
 	}
 	if req.SignKey != "" {
-		buildProperties.Set("build.keys.sign_key", req.GetSignKey())
+		boardBuildProperties.Set("build.keys.sign_key", req.GetSignKey())
 	}
 	if req.EncryptKey != "" {
-		buildProperties.Set("build.keys.encrypt_key", req.GetEncryptKey())
+		boardBuildProperties.Set("build.keys.encrypt_key", req.GetEncryptKey())
 	}
 	// At the current time we do not have a way of knowing if a board supports the secure boot or not,
 	// so, if the flags to override the default keys are used, we try override the corresponding platform property nonetheless.
 	// It's not possible to use the default name for the keys since there could be more tools to sign and encrypt.
 	// So it's mandatory to use all three flags to sign and encrypt the binary
-	keychainProp := buildProperties.ContainsKey("build.keys.keychain")
-	signProp := buildProperties.ContainsKey("build.keys.sign_key")
-	encryptProp := buildProperties.ContainsKey("build.keys.encrypt_key")
+	keychainProp := boardBuildProperties.ContainsKey("build.keys.keychain")
+	signProp := boardBuildProperties.ContainsKey("build.keys.sign_key")
+	encryptProp := boardBuildProperties.ContainsKey("build.keys.encrypt_key")
 	// we verify that all the properties for the secure boot keys are defined or none of them is defined.
 	if !(keychainProp == signProp && signProp == encryptProp) {
 		return nil, fmt.Errorf(tr("Firmware encryption/signing requires all the following properties to be defined: %s", "build.keys.keychain, build.keys.sign_key, build.keys.encrypt_key"))
@@ -169,10 +169,9 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 		coreBuildCachePath = buildCachePath.Join("core")
 	}
 
-	sketchBuilder := bldr.NewBuilder(sk, coreBuildCachePath)
+	sketchBuilder := bldr.NewBuilder(sk, boardBuildProperties, buildPath, req.GetOptimizeForDebug(), coreBuildCachePath)
 
-	// Add build properites related to sketch data
-	buildProperties = sketchBuilder.SetupBuildProperties(buildProperties, buildPath, req.GetOptimizeForDebug())
+	buildProperties := sketchBuilder.GetBuildProperties()
 
 	// Add user provided custom build properties
 	customBuildPropertiesArgs := append(req.GetBuildProperties(), "build.warn_data_percentage=75")

Diff for: legacy/builder/constants/constants.go

@@ -21,11 +21,8 @@ const BUILD_PROPERTIES_ARCH_OVERRIDE_CHECK = "architecture.override_check"
 const BUILD_PROPERTIES_BOOTLOADER_FILE = "bootloader.file"
 const BUILD_PROPERTIES_BOOTLOADER_NOBLINK = "bootloader.noblink"
 const BUILD_PROPERTIES_BUILD_BOARD = "build.board"
-const BUILD_PROPERTIES_BUILD_MCU = "build.mcu"
 const BUILD_PROPERTIES_COMPILER_LDFLAGS = "compiler.ldflags"
 const BUILD_PROPERTIES_COMPILER_CPP_FLAGS = "compiler.cpp.flags"
-const BUILD_PROPERTIES_RUNTIME_PLATFORM_PATH = "runtime.platform.path"
-const EMPTY_STRING = ""
 const FOLDER_BOOTLOADERS = "bootloaders"
 const FOLDER_CORE = "core"
 const FOLDER_SKETCH = "sketch"

Diff for: legacy/builder/create_cmake_rule.go

@@ -200,7 +200,7 @@ func (s *ExportProjectCMake) Run(ctx *types.Context) error {
 	for _, library := range ctx.SketchLibrariesDetector.ImportedLibraries() {
 		// Copy used libraries in the correct folder
 		libDir := libBaseFolder.Join(library.DirName)
-		mcu := ctx.BuildProperties.Get(constants.BUILD_PROPERTIES_BUILD_MCU)
+		mcu := ctx.BuildProperties.Get("build.mcu")
 		copyDir(library.InstallDir.String(), libDir.String(), validExportExtensions)
 
 		// Read cmake options if available

Diff for: legacy/builder/merge_sketch_with_bootloader.go

@@ -55,15 +55,15 @@ func (s *MergeSketchWithBootloader) Run(ctx *types.Context) error {
 		return nil
 	}
 
-	bootloader := constants.EMPTY_STRING
+	bootloader := ""
 	if bootloaderNoBlink, ok := buildProperties.GetOk(constants.BUILD_PROPERTIES_BOOTLOADER_NOBLINK); ok {
 		bootloader = bootloaderNoBlink
 	} else {
 		bootloader = buildProperties.Get(constants.BUILD_PROPERTIES_BOOTLOADER_FILE)
 	}
 	bootloader = buildProperties.ExpandPropsInString(bootloader)
 
-	bootloaderPath := buildProperties.GetPath(constants.BUILD_PROPERTIES_RUNTIME_PLATFORM_PATH).Join(constants.FOLDER_BOOTLOADERS, bootloader)
+	bootloaderPath := buildProperties.GetPath("runtime.platform.path").Join(constants.FOLDER_BOOTLOADERS, bootloader)
 	if bootloaderPath.NotExist() {
 		if ctx.Verbose {
 			ctx.Warn(tr("Bootloader file specified but missing: %[1]s", bootloaderPath))

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

@@ -31,7 +31,6 @@ import (
 	"github.com/arduino/arduino-cli/buildcache"
 	"github.com/arduino/arduino-cli/i18n"
 	f "github.com/arduino/arduino-cli/internal/algorithms"
-	"github.com/arduino/arduino-cli/legacy/builder/constants"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
@@ -102,7 +101,7 @@ func compileCore(
 ) (*paths.Path, paths.PathList, error) {
 	coreFolder := buildProperties.GetPath("build.core.path")
 	variantFolder := buildProperties.GetPath("build.variant.path")
-	targetCoreFolder := buildProperties.GetPath(constants.BUILD_PROPERTIES_RUNTIME_PLATFORM_PATH)
+	targetCoreFolder := buildProperties.GetPath("runtime.platform.path")
 
 	includes := []string{coreFolder.String()}
 	if variantFolder != nil && variantFolder.IsDir() {