[breaking] Fix regression in core caching (arduino#2145)

* Added integration test

* Removing useless constants

* Fixed regression in core caching

* Updated tests

Author: cmaglie

Diff for: arduino/cores/board.go

@@ -116,16 +116,16 @@ func (b *Board) GetConfigOptionValues(option string) *properties.Map {
 
 // GetBuildProperties returns the build properties and the build
 // platform for the Board with the configuration passed as parameter.
-func (b *Board) GetBuildProperties(userConfigs *properties.Map) (*properties.Map, error) {
+func (b *Board) GetBuildProperties(fqbn *FQBN) (*properties.Map, error) {
 	b.buildConfigOptionsStructures()
 
 	// Override default configs with user configs
 	config := b.defaultConfig.Clone()
-	config.Merge(userConfigs)
+	config.Merge(fqbn.Configs)
 
 	// Start with board's base properties
 	buildProperties := b.Properties.Clone()
-	buildProperties.Set("build.fqbn", b.FQBN())
+	buildProperties.Set("build.fqbn", fqbn.String())
 	buildProperties.Set("build.arch", strings.ToUpper(b.PlatformRelease.Platform.Architecture))
 
 	// Add all sub-configurations one by one (a config is: option=value)
@@ -157,7 +157,7 @@ func (b *Board) GeneratePropertiesForConfiguration(config string) (*properties.M
 	if err != nil {
 		return nil, fmt.Errorf(tr("parsing fqbn: %s"), err)
 	}
-	return b.GetBuildProperties(fqbn.Configs)
+	return b.GetBuildProperties(fqbn)
 }
 
 // GetIdentificationProperties calculates and returns a list of properties sets

Diff for: arduino/cores/board_test.go

@@ -220,7 +220,7 @@ func TestBoardOptions(t *testing.T) {
 	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.fqbn", "arduino:avr:mega:cpu=atmega2560")
 	expConf2560.Set("build.mcu", "atmega2560")
 	expConf2560.Set("build.variant", "mega")
 	expConf2560.Set("menu.cpu.atmega1280", "ATmega1280")
@@ -281,7 +281,7 @@ func TestBoardOptions(t *testing.T) {
 	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.fqbn", "arduino:avr:mega:cpu=atmega1280")
 	expConf1280.Set("build.mcu", "atmega1280")
 	expConf1280.Set("build.variant", "mega")
 	expConf1280.Set("menu.cpu.atmega1280", "ATmega1280")
@@ -342,7 +342,7 @@ func TestBoardOptions(t *testing.T) {
 	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.fqbn", "watterott:avr:attiny841:core=spencekonde,info=info")
 	expWatterott.Set("build.mcu", "attiny841")
 	expWatterott.Set("build.variant", "tiny14")
 	expWatterott.Set("menu.core.arduino", "Standard Arduino")

Diff for: arduino/cores/packagemanager/package_manager.go

@@ -303,7 +303,7 @@ func (pme *Explorer) ResolveFQBN(fqbn *cores.FQBN) (
 			fmt.Errorf(tr("board %s not found"), fqbn.StringWithoutConfig())
 	}
 
-	boardBuildProperties, err := board.GetBuildProperties(fqbn.Configs)
+	boardBuildProperties, err := board.GetBuildProperties(fqbn)
 	if err != nil {
 		return targetPackage, boardPlatformRelease, board, nil, nil,
 			fmt.Errorf(tr("getting build properties for board %[1]s: %[2]s"), board, err)

Diff for: docs/UPGRADING.md

@@ -2,6 +2,34 @@
 
 Here you can find a list of migration guides to handle breaking changes between releases of the CLI.
 
+## 0.32.2
+
+### golang API: method `github.com/arduino/arduino-cli/arduino/cores/Board.GetBuildProperties` changed signature
+
+The method:
+
+```go
+func (b *Board) GetBuildProperties(userConfigs *properties.Map) (*properties.Map, error) { ... }
+```
+
+now requires a full `FQBN` object;
+
+```go
+func (b *Board) GetBuildProperties(fqbn *FQBN) (*properties.Map, error) { ... }
+```
+
+Existing code may be updated from:
+
+```go
+b.GetBuildProperties(fqbn.Configs)
+```
+
+to
+
+```
+b.GetBuildProperties(fqbn)
+```
+
 ## 0.32.0
 
 ### `arduino-cli` doesn't lookup anymore in the current directory for configuration file.

Diff for: go.mod

@@ -51,6 +51,7 @@ require (
 	github.com/rogpeppe/go-internal v1.3.0
 	github.com/xeipuuv/gojsonschema v1.2.0
 	go.bug.st/testifyjson v1.1.1
+	golang.org/x/exp v0.0.0-20230321023759-10a507213a29
 	golang.org/x/term v0.6.0
 	gopkg.in/yaml.v3 v3.0.1
 )

Diff for: go.sum

@@ -155,8 +155,8 @@ github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
 github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
@@ -397,6 +397,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
 golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
+golang.org/x/exp v0.0.0-20230321023759-10a507213a29 h1:ooxPy7fPvB4kwsA2h+iBNHkAbp/4JxTSwCmvdjEYmug=
+golang.org/x/exp v0.0.0-20230321023759-10a507213a29/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
 golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
@@ -602,7 +604,6 @@ golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE=
 google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M=

Diff for: internal/integrationtest/compile_3/core_caching_test.go

@@ -0,0 +1,68 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2022 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to [email protected].
+
+package compile_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/arduino/arduino-cli/internal/integrationtest"
+	"github.com/arduino/go-paths-helper"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/slices"
+)
+
+func TestCompileCoreCacheGeneration(t *testing.T) {
+	// See:
+	// https://forum.arduino.cc/t/teensy-compile-sketch-fails-clear-out-temp-build-completes/1110104/6
+	// https://forum.pjrc.com/threads/72572-Teensyduino-1-59-Beta-2?p=324071&viewfull=1#post324071
+	// https://github.com/arduino/arduino-ide/issues/1990
+
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	// Run update-index with our test index
+	_, _, err := cli.Run("core", "install", "arduino:[email protected]")
+	require.NoError(t, err)
+
+	// Prepare sketch
+	sketch, err := paths.New("testdata", "bare_minimum").Abs()
+	require.NoError(t, err)
+
+	// Perform two compile that should result in different cached cores
+	_, _, err = cli.Run("compile", "-b", "arduino:avr:mega", sketch.String())
+	require.NoError(t, err)
+	_, _, err = cli.Run("compile", "-b", "arduino:avr:mega:cpu=atmega1280", sketch.String())
+	require.NoError(t, err)
+
+	// Perform the same compile again and track the cached cores
+	extractCachedCoreFromStdout := func(stdout []byte) string {
+		prefix := "Using precompiled core: "
+		lines := strings.Split(string(stdout), "\n")
+		i := slices.IndexFunc(lines, func(l string) bool {
+			return strings.Contains(l, prefix)
+		})
+		require.NotEqual(t, -1, i, "Could not find precompiled core in output")
+		return strings.TrimPrefix(lines[i], prefix)
+	}
+	stdout, _, err := cli.Run("compile", "-b", "arduino:avr:mega", sketch.String(), "-v")
+	require.NoError(t, err)
+	core1 := extractCachedCoreFromStdout(stdout)
+	stdout, _, err = cli.Run("compile", "-b", "arduino:avr:mega:cpu=atmega1280", sketch.String(), "-v")
+	require.NoError(t, err)
+	core2 := extractCachedCoreFromStdout(stdout)
+	require.NotEqual(t, core1, core2, "Precompile core must be different!")
+}

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

@@ -28,7 +28,6 @@ const BUILD_PROPERTIES_COMPILER_C_ELF_FLAGS = "compiler.c.elf.flags"
 const BUILD_PROPERTIES_COMPILER_LDFLAGS = "compiler.ldflags"
 const BUILD_PROPERTIES_COMPILER_CPP_FLAGS = "compiler.cpp.flags"
 const BUILD_PROPERTIES_COMPILER_WARNING_FLAGS = "compiler.warning_flags"
-const BUILD_PROPERTIES_FQBN = "build.fqbn"
 const BUILD_PROPERTIES_INCLUDES = "includes"
 const BUILD_PROPERTIES_OBJECT_FILE = "object_file"
 const BUILD_PROPERTIES_RUNTIME_PLATFORM_PATH = "runtime.platform.path"

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

@@ -95,8 +95,10 @@ func compileCore(ctx *types.Context, buildPath *paths.Path, buildCachePath *path
 	var targetArchivedCore *paths.Path
 	var buildCacheErr error
 	if buildCachePath != nil {
-		archivedCoreName := GetCachedCoreArchiveDirName(buildProperties.Get(constants.BUILD_PROPERTIES_FQBN),
-			buildProperties.Get("compiler.optimization_flags"), realCoreFolder)
+		archivedCoreName := GetCachedCoreArchiveDirName(
+			buildProperties.Get("build.fqbn"),
+			buildProperties.Get("compiler.optimization_flags"),
+			realCoreFolder)
 		targetArchivedCore = buildCachePath.Join(archivedCoreName, "core.a")
 		_, buildCacheErr = buildcache.New(buildCachePath).GetOrCreate(archivedCoreName)