Refactored build-properties creation (and removed some legacy code) (arduino#2082)

* Moved 'build.fqbn' and 'build.arch' properties generation

* Moved some runtime build path properties to proper function

* Factored tools runtime properties generation

* Factored subroutine for time-related properties generation

* Made the referenced-core determination a bit more readable

Equivalent change, should not change behaviour

* Factored 'build.variant' and related properties calculation

* Removed useless state variable 'BuildCore'

* Refactoring of some legacy properties generation subroutines

* Refactored generation of 'software' build property

* Refactored build properties overlaying logic

* Refactored custom global properties handling

* Moved corePlatform and variantPlatform determination inside a specific method

Author: cmaglie

arduino/cores/board.go

@@ -125,6 +125,8 @@ func (b *Board) GetBuildProperties(userConfigs *properties.Map) (*properties.Map
 
 	// Start with board's base properties
 	buildProperties := b.Properties.Clone()
+	buildProperties.Set("build.fqbn", b.FQBN())
+	buildProperties.Set("build.arch", strings.ToUpper(b.PlatformRelease.Platform.Architecture))
 
 	// Add all sub-configurations one by one (a config is: option=value)
 	// Check for residual invalid options...

arduino/cores/board_test.go

@@ -216,9 +216,11 @@ func TestBoardOptions(t *testing.T) {
 	expConf2560.Set("bootloader.low_fuses", "0xFF")
 	expConf2560.Set("bootloader.tool", "avrdude")
 	expConf2560.Set("bootloader.unlock_bits", "0x3F")
+	expConf2560.Set("build.arch", "AVR")
 	expConf2560.Set("build.board", "AVR_MEGA2560")
 	expConf2560.Set("build.core", "arduino")
 	expConf2560.Set("build.f_cpu", "16000000L")
+	expConf2560.Set("build.fqbn", "arduino:avr:mega")
 	expConf2560.Set("build.mcu", "atmega2560")
 	expConf2560.Set("build.variant", "mega")
 	expConf2560.Set("menu.cpu.atmega1280", "ATmega1280")
@@ -275,9 +277,11 @@ func TestBoardOptions(t *testing.T) {
 	expConf1280.Set("bootloader.low_fuses", "0xFF")
 	expConf1280.Set("bootloader.tool", "avrdude")
 	expConf1280.Set("bootloader.unlock_bits", "0x3F")
+	expConf1280.Set("build.arch", "AVR")
 	expConf1280.Set("build.board", "AVR_MEGA")
 	expConf1280.Set("build.core", "arduino")
 	expConf1280.Set("build.f_cpu", "16000000L")
+	expConf1280.Set("build.fqbn", "arduino:avr:mega")
 	expConf1280.Set("build.mcu", "atmega1280")
 	expConf1280.Set("build.variant", "mega")
 	expConf1280.Set("menu.cpu.atmega1280", "ATmega1280")
@@ -334,9 +338,11 @@ func TestBoardOptions(t *testing.T) {
 	expWatterott.Set("bootloader.low_fuses", "0xE2")
 	expWatterott.Set("bootloader.tool", "avrdude")
 	expWatterott.Set("bootloader.unlock_bits", "0xFF")
+	expWatterott.Set("build.arch", "AVR")
 	expWatterott.Set("build.board", "AVR_ATTINY841")
 	expWatterott.Set("build.core", "tiny841")
 	expWatterott.Set("build.f_cpu", "8000000L")
+	expWatterott.Set("build.fqbn", "watterott:avr:attiny841")
 	expWatterott.Set("build.mcu", "attiny841")
 	expWatterott.Set("build.variant", "tiny14")
 	expWatterott.Set("menu.core.arduino", "Standard Arduino")

arduino/cores/cores.go

@@ -336,8 +336,8 @@ func (release *PlatformRelease) RequiresToolRelease(toolRelease *ToolRelease) bo
 func (release *PlatformRelease) RuntimeProperties() *properties.Map {
 	res := properties.NewMap()
 	if release.InstallDir != nil {
-		res.Set("runtime.platform.path", release.InstallDir.String())
-		res.Set("runtime.hardware.path", release.InstallDir.Join("..").String())
+		res.SetPath("runtime.platform.path", release.InstallDir)
+		res.SetPath("runtime.hardware.path", release.InstallDir.Join(".."))
 	}
 
 	return res

arduino/cores/packagemanager/package_manager.go

@@ -18,9 +18,13 @@ package packagemanager
 import (
 	"fmt"
 	"net/url"
+	"os"
 	"path"
+	"path/filepath"
+	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/arduino/arduino-cli/arduino/cores"
 	"github.com/arduino/arduino-cli/arduino/cores/packageindex"
@@ -29,6 +33,7 @@ import (
 	"github.com/arduino/arduino-cli/i18n"
 	paths "github.com/arduino/go-paths-helper"
 	properties "github.com/arduino/go-properties-orderedmap"
+	"github.com/arduino/go-timeutils"
 	"github.com/sirupsen/logrus"
 	semver "go.bug.st/relaxed-semver"
 )
@@ -275,50 +280,141 @@ func (pme *Explorer) ResolveFQBN(fqbn *cores.FQBN) (
 		return targetPackage, nil, nil, nil, nil,
 			fmt.Errorf(tr("unknown platform %s:%s"), targetPackage, fqbn.PlatformArch)
 	}
-	platformRelease := pme.GetInstalledPlatformRelease(platform)
-	if platformRelease == nil {
+	boardPlatformRelease := pme.GetInstalledPlatformRelease(platform)
+	if boardPlatformRelease == nil {
 		return targetPackage, nil, nil, nil, nil,
 			fmt.Errorf(tr("platform %s is not installed"), platform)
 	}
 
 	// Find board
-	board := platformRelease.Boards[fqbn.BoardID]
+	board := boardPlatformRelease.Boards[fqbn.BoardID]
 	if board == nil {
-		return targetPackage, platformRelease, nil, nil, nil,
+		return targetPackage, boardPlatformRelease, nil, nil, nil,
 			fmt.Errorf(tr("board %s not found"), fqbn.StringWithoutConfig())
 	}
 
-	buildProperties, err := board.GetBuildProperties(fqbn.Configs)
+	boardBuildProperties, err := board.GetBuildProperties(fqbn.Configs)
 	if err != nil {
-		return targetPackage, platformRelease, board, nil, nil,
+		return targetPackage, boardPlatformRelease, board, nil, nil,
 			fmt.Errorf(tr("getting build properties for board %[1]s: %[2]s"), board, err)
 	}
 
-	// Determine the platform used for the build (in case the board refers
+	// Determine the platform used for the build and the variant (in case the board refers
 	// to a core contained in another platform)
-	buildPlatformRelease := platformRelease
-	coreParts := strings.Split(buildProperties.Get("build.core"), ":")
-	if len(coreParts) > 1 {
-		referredPackage := coreParts[0]
-		buildPackage := pme.packages[referredPackage]
-		if buildPackage == nil {
-			return targetPackage, platformRelease, board, buildProperties, nil,
-				fmt.Errorf(tr("missing package %[1]s referenced by board %[2]s"), referredPackage, fqbn)
+	core, corePlatformRelease, variant, variantPlatformRelease, err := pme.determineReferencedPlatformRelease(boardBuildProperties, boardPlatformRelease, fqbn)
+	if err != nil {
+		return targetPackage, boardPlatformRelease, board, nil, nil, err
+	}
+
+	// Create the build properties map by overlaying the properties of the
+	// referenced platform propeties, the board platform properties and the
+	// board specific properties.
+	buildProperties := properties.NewMap()
+	buildProperties.Merge(variantPlatformRelease.Properties)
+	buildProperties.Merge(corePlatformRelease.Properties)
+	buildProperties.Merge(boardPlatformRelease.Properties)
+	buildProperties.Merge(boardBuildProperties)
+
+	// Add runtime build properties
+	buildProperties.Merge(boardPlatformRelease.RuntimeProperties())
+	buildProperties.SetPath("build.core.path", corePlatformRelease.InstallDir.Join("cores", core))
+	buildProperties.SetPath("build.system.path", corePlatformRelease.InstallDir.Join("system"))
+	buildProperties.Set("build.variant.path", "")
+	if variant != "" {
+		buildProperties.SetPath("build.variant.path", variantPlatformRelease.InstallDir.Join("variants", variant))
+	}
+
+	for _, tool := range pme.GetAllInstalledToolsReleases() {
+		buildProperties.Merge(tool.RuntimeProperties())
+	}
+	requiredTools, err := pme.FindToolsRequiredForBuild(boardPlatformRelease, corePlatformRelease)
+	if err != nil {
+		return targetPackage, boardPlatformRelease, board, buildProperties, corePlatformRelease, err
+	}
+	for _, tool := range requiredTools {
+		buildProperties.Merge(tool.RuntimeProperties())
+	}
+	now := time.Now()
+	buildProperties.Set("extra.time.utc", strconv.FormatInt(now.Unix(), 10))
+	buildProperties.Set("extra.time.local", strconv.FormatInt(timeutils.LocalUnix(now), 10))
+	buildProperties.Set("extra.time.zone", strconv.Itoa(timeutils.TimezoneOffsetNoDST(now)))
+	buildProperties.Set("extra.time.dst", strconv.Itoa(timeutils.DaylightSavingsOffset(now)))
+
+	if !buildProperties.ContainsKey("runtime.ide.path") {
+		if ex, err := os.Executable(); err == nil {
+			buildProperties.Set("runtime.ide.path", filepath.Dir(ex))
+		} else {
+			buildProperties.Set("runtime.ide.path", "")
 		}
-		buildPlatform := buildPackage.Platforms[fqbn.PlatformArch]
-		if buildPlatform == nil {
-			return targetPackage, platformRelease, board, buildProperties, nil,
-				fmt.Errorf(tr("missing platform %[1]s:%[2]s referenced by board %[3]s"), referredPackage, fqbn.PlatformArch, fqbn)
+	}
+	buildProperties.Set("runtime.os", properties.GetOSSuffix())
+	buildProperties.Set("build.library_discovery_phase", "0")
+	// Deprecated properties
+	buildProperties.Set("ide_version", "10607")
+	buildProperties.Set("runtime.ide.version", "10607")
+	if !buildProperties.ContainsKey("software") {
+		buildProperties.Set("software", "ARDUINO")
+	}
+
+	buildProperties.Merge(pme.GetCustomGlobalProperties())
+
+	return targetPackage, boardPlatformRelease, board, buildProperties, corePlatformRelease, nil
+}
+
+func (pme *Explorer) determineReferencedPlatformRelease(boardBuildProperties *properties.Map, boardPlatformRelease *cores.PlatformRelease, fqbn *cores.FQBN) (string, *cores.PlatformRelease, string, *cores.PlatformRelease, error) {
+	core := boardBuildProperties.Get("build.core")
+	referredCore := ""
+	if split := strings.Split(core, ":"); len(split) > 1 {
+		referredCore, core = split[0], split[1]
+	}
+
+	variant := boardBuildProperties.Get("build.variant")
+	referredVariant := ""
+	if split := strings.Split(variant, ":"); len(split) > 1 {
+		referredVariant, variant = split[0], split[1]
+	}
+
+	// core and variant cannot refer to two different platforms
+	if referredCore != "" && referredVariant != "" && referredCore != referredVariant {
+		return "", nil, "", nil,
+			fmt.Errorf(tr("'build.core' and 'build.variant' refer to different platforms: %[1]s and %[2]s"), referredCore+":"+core, referredVariant+":"+variant)
+	}
+
+	// extract the referred platform
+	var referredPlatformRelease *cores.PlatformRelease
+	referredPackageName := referredCore
+	if referredPackageName == "" {
+		referredPackageName = referredVariant
+	}
+	if referredPackageName != "" {
+		referredPackage := pme.packages[referredPackageName]
+		if referredPackage == nil {
+			return "", nil, "", nil,
+				fmt.Errorf(tr("missing package %[1]s referenced by board %[2]s"), referredPackageName, fqbn)
+		}
+		referredPlatform := referredPackage.Platforms[fqbn.PlatformArch]
+		if referredPlatform == nil {
+			return "", nil, "", nil,
+				fmt.Errorf(tr("missing platform %[1]s:%[2]s referenced by board %[3]s"), referredPackageName, fqbn.PlatformArch, fqbn)
 		}
-		buildPlatformRelease = pme.GetInstalledPlatformRelease(buildPlatform)
-		if buildPlatformRelease == nil {
-			return targetPackage, platformRelease, board, buildProperties, nil,
-				fmt.Errorf(tr("missing platform release %[1]s:%[2]s referenced by board %[3]s"), referredPackage, fqbn.PlatformArch, fqbn)
+		referredPlatformRelease = pme.GetInstalledPlatformRelease(referredPlatform)
+		if referredPlatformRelease == nil {
+			return "", nil, "", nil,
+				fmt.Errorf(tr("missing platform release %[1]s:%[2]s referenced by board %[3]s"), referredPackageName, fqbn.PlatformArch, fqbn)
 		}
 	}
 
-	// No errors... phew!
-	return targetPackage, platformRelease, board, buildProperties, buildPlatformRelease, nil
+	corePlatformRelease := boardPlatformRelease
+	if referredCore != "" {
+		corePlatformRelease = referredPlatformRelease
+	}
+
+	variantPlatformRelease := boardPlatformRelease
+	if referredVariant != "" {
+		variantPlatformRelease = referredPlatformRelease
+	}
+
+	return core, corePlatformRelease, variant, variantPlatformRelease, nil
 }
 
 // LoadPackageIndex loads a package index by looking up the local cached file from the specified URL

arduino/cores/packagemanager/package_manager_test.go

@@ -141,7 +141,7 @@ func TestResolveFQBN(t *testing.T) {
 	require.Equal(t, platformRelease.Platform.String(), "referenced:avr")
 	require.NotNil(t, board)
 	require.Equal(t, board.Name(), "Referenced dummy with invalid package")
-	require.NotNil(t, props)
+	require.Nil(t, props)
 	require.Nil(t, buildPlatformRelease)
 
 	// Test a board referenced from a non-existent platform/architecture
@@ -156,7 +156,7 @@ func TestResolveFQBN(t *testing.T) {
 	require.Equal(t, platformRelease.Platform.String(), "referenced:avr")
 	require.NotNil(t, board)
 	require.Equal(t, board.Name(), "Referenced dummy with invalid platform")
-	require.NotNil(t, props)
+	require.Nil(t, props)
 	require.Nil(t, buildPlatformRelease)
 
 	// Test a board referenced from a non-existent core

commands/compile/compile.go

@@ -181,9 +181,6 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 
 	builderCtx.BuiltInLibrariesDirs = configuration.IDEBuiltinLibrariesDir(configuration.Settings)
 
-	// Will be deprecated.
-	builderCtx.ArduinoAPIVersion = "10607"
-
 	builderCtx.Stdout = outStream
 	builderCtx.Stderr = errStream
 	builderCtx.Clean = req.GetClean()

commands/upload/upload.go

@@ -292,20 +292,6 @@ func runProgramAction(pme *packagemanager.Explorer,
 		uploadProperties.Merge(programmer.Properties)
 	}
 
-	for _, tool := range pme.GetAllInstalledToolsReleases() {
-		uploadProperties.Merge(tool.RuntimeProperties())
-	}
-	if requiredTools, err := pme.FindToolsRequiredForBuild(boardPlatform, buildPlatform); err == nil {
-		for _, requiredTool := range requiredTools {
-			logrus.WithField("tool", requiredTool).Info("Tool required for upload")
-			if requiredTool.IsInstalled() {
-				uploadProperties.Merge(requiredTool.RuntimeProperties())
-			} else {
-				errStream.Write([]byte(tr("Warning: tool '%s' is not installed. It might not be available for your OS.", requiredTool)))
-			}
-		}
-	}
-
 	// Certain tools require the user to provide custom fields at run time,
 	// if they've been provided set them
 	// For more info:

legacy/builder/builder.go

@@ -31,7 +31,6 @@ var tr = i18n.Tr
 
 const DEFAULT_DEBUG_LEVEL = 5
 const DEFAULT_WARNINGS_LEVEL = "none"
-const DEFAULT_SOFTWARE = "ARDUINO"
 
 type Builder struct{}