Skip to content

[breaking] Fix regression in core caching #2145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions arduino/cores/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions arduino/cores/board_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion arduino/cores/packagemanager/package_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 28 additions & 0 deletions docs/UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
5 changes: 3 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
68 changes: 68 additions & 0 deletions internal/integrationtest/compile_3/core_caching_test.go
Original file line number Diff line number Diff line change
@@ -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!")
}
1 change: 0 additions & 1 deletion legacy/builder/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 4 additions & 2 deletions legacy/builder/phases/core_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down