Bugfix: do not overwrite board properties during compile (#1820) (Fix #1614)

* Bugfix: do not overwrite board properies during compile

Fix #1614

* Removed AddBuildPropertyIfMissinc 'legacy' (inlined in other functions)

* Increase code coverage

legacy/builder/add_build_board_property_if_missing.go

-// This file is part of arduino-cli.
-//
-// Copyright 2020 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 license@arduino.cc.
-
-package builder
-
-import (
-	"strings"
-
-	"github.com/arduino/arduino-cli/legacy/builder/constants"
-	"github.com/arduino/arduino-cli/legacy/builder/types"
-)
-
-type AddBuildBoardPropertyIfMissing struct{}
-
-func (*AddBuildBoardPropertyIfMissing) Run(ctx *types.Context) error {
-	packages := ctx.Hardware
-
-	for _, aPackage := range packages {
-		for _, platform := range aPackage.Platforms {
-			for _, platformRelease := range platform.Releases {
-				for _, board := range platformRelease.Boards {
-					if board.Properties.Get("build.board") == "" {
-						board.Properties.Set("build.board", strings.ToUpper(platform.Architecture+"_"+board.BoardID))
-						ctx.Info(tr("Warning: Board %[1]s doesn't define a %[2]s preference. Auto-set to: %[3]s",
-							aPackage.Name+":"+platform.Architecture+":"+board.BoardID,
-							"'build.board'",
-							board.Properties.Get(constants.BUILD_PROPERTIES_BUILD_BOARD)))
-					}
-				}
-			}
-		}
-	}
-
-	return nil
-}

legacy/builder/container_setup.go

@@ -36,7 +36,6 @@ func (s *ContainerSetupHardwareToolsLibsSketchAndProps) Run(ctx *types.Context)
 		&RewriteHardwareKeys{},
 		&TargetBoardResolver{},
 		&ToolsLoader{},
-		&AddBuildBoardPropertyIfMissing{},
 		&LibrariesLoader{},
 	}
 

legacy/builder/setup_build_properties.go

@@ -36,12 +36,11 @@ func (s *SetupBuildProperties) Run(ctx *types.Context) error {
 
 	targetPlatform := ctx.TargetPlatform
 	actualPlatform := ctx.ActualPlatform
-	targetBoard := ctx.TargetBoard
 
 	buildProperties := properties.NewMap()
 	buildProperties.Merge(actualPlatform.Properties)
 	buildProperties.Merge(targetPlatform.Properties)
-	buildProperties.Merge(targetBoard.Properties)
+	buildProperties.Merge(ctx.TargetBoardBuildProperties)
 
 	if ctx.BuildPath != nil {
 		buildProperties.SetPath("build.path", ctx.BuildPath)

legacy/builder/target_board_resolver.go

@@ -30,9 +30,7 @@ func (s *TargetBoardResolver) Run(ctx *types.Context) error {
 		return fmt.Errorf("%s: %w", tr("Error resolving FQBN"), err)
 	}
 
-	targetBoard.Properties = buildProperties // FIXME....
-
-	core := targetBoard.Properties.Get("build.core")
+	core := buildProperties.Get("build.core")
 	if core == "" {
 		core = "arduino"
 	}
@@ -44,8 +42,17 @@ func (s *TargetBoardResolver) Run(ctx *types.Context) error {
 		ctx.Info(tr("Using core '%[1]s' from platform in folder: %[2]s", core, actualPlatform.InstallDir))
 	}
 
+	if buildProperties.Get("build.board") == "" {
+		architecture := targetBoard.PlatformRelease.Platform.Architecture
+		defaultBuildBoard := strings.ToUpper(architecture + "_" + targetBoard.BoardID)
+		buildProperties.Set("build.board", defaultBuildBoard)
+		ctx.Info(tr("Warning: Board %[1]s doesn't define a %[2]s preference. Auto-set to: %[3]s",
+			targetBoard.String(), "'build.board'", defaultBuildBoard))
+	}
+
 	ctx.BuildCore = core
 	ctx.TargetBoard = targetBoard
+	ctx.TargetBoardBuildProperties = buildProperties
 	ctx.TargetPlatform = targetPlatform
 	ctx.TargetPackage = targetPackage
 	ctx.ActualPlatform = actualPlatform

legacy/builder/test/add_build_board_property_if_missing_test.go

@@ -44,7 +44,6 @@ func TestAddBuildBoardPropertyIfMissing(t *testing.T) {
 	commands := []types.Command{
 		&builder.HardwareLoader{},
 		&builder.TargetBoardResolver{},
-		&builder.AddBuildBoardPropertyIfMissing{},
 	}
 
 	for _, command := range commands {
@@ -60,8 +59,9 @@ func TestAddBuildBoardPropertyIfMissing(t *testing.T) {
 	require.Equal(t, "avr", targetPlatform.Platform.Architecture)
 	targetBoard := ctx.TargetBoard
 	require.Equal(t, "mymega", targetBoard.BoardID)
-	require.Equal(t, "atmega2560", targetBoard.Properties.Get("build.mcu"))
-	require.Equal(t, "AVR_MYMEGA2560", targetBoard.Properties.Get("build.board"))
+	targetBoardBuildProperties := ctx.TargetBoardBuildProperties
+	require.Equal(t, "atmega2560", targetBoardBuildProperties.Get("build.mcu"))
+	require.Equal(t, "AVR_MYMEGA", targetBoardBuildProperties.Get("build.board"))
 }
 
 func TestAddBuildBoardPropertyIfMissingNotMissing(t *testing.T) {
@@ -76,7 +76,6 @@ func TestAddBuildBoardPropertyIfMissingNotMissing(t *testing.T) {
 	commands := []types.Command{
 		&builder.HardwareLoader{},
 		&builder.TargetBoardResolver{},
-		&builder.AddBuildBoardPropertyIfMissing{},
 	}
 
 	for _, command := range commands {
@@ -90,6 +89,7 @@ func TestAddBuildBoardPropertyIfMissingNotMissing(t *testing.T) {
 	require.Equal(t, "avr", targetPlatform.Platform.Architecture)
 	targetBoard := ctx.TargetBoard
 	require.Equal(t, "mymega", targetBoard.BoardID)
-	require.Equal(t, "atmega1280", targetBoard.Properties.Get("build.mcu"))
-	require.Equal(t, "AVR_MYMEGA", targetBoard.Properties.Get("build.board"))
+	targetBoardBuildProperties := ctx.TargetBoardBuildProperties
+	require.Equal(t, "atmega1280", targetBoardBuildProperties.Get("build.mcu"))
+	require.Equal(t, "MYMEGA1280", targetBoardBuildProperties.Get("build.board"))
 }

legacy/builder/test/target_board_resolver_test.go

@@ -47,7 +47,8 @@ func TestTargetBoardResolverUno(t *testing.T) {
 	require.Equal(t, "avr", targetPlatform.Platform.Architecture)
 	targetBoard := ctx.TargetBoard
 	require.Equal(t, "uno", targetBoard.BoardID)
-	require.Equal(t, "atmega328p", targetBoard.Properties.Get("build.mcu"))
+	targetBoardBuildProperties := ctx.TargetBoardBuildProperties
+	require.Equal(t, "atmega328p", targetBoardBuildProperties.Get("build.mcu"))
 }
 
 func TestTargetBoardResolverDue(t *testing.T) {
@@ -72,7 +73,8 @@ func TestTargetBoardResolverDue(t *testing.T) {
 	require.Equal(t, "sam", targetPlatform.Platform.Architecture)
 	targetBoard := ctx.TargetBoard
 	require.Equal(t, "arduino_due_x", targetBoard.BoardID)
-	require.Equal(t, "cortex-m3", targetBoard.Properties.Get("build.mcu"))
+	targetBoardBuildProperties := ctx.TargetBoardBuildProperties
+	require.Equal(t, "cortex-m3", targetBoardBuildProperties.Get("build.mcu"))
 }
 
 func TestTargetBoardResolverMega1280(t *testing.T) {
@@ -97,8 +99,9 @@ func TestTargetBoardResolverMega1280(t *testing.T) {
 	require.Equal(t, "avr", targetPlatform.Platform.Architecture)
 	targetBoard := ctx.TargetBoard
 	require.Equal(t, "mega", targetBoard.BoardID)
-	require.Equal(t, "atmega1280", targetBoard.Properties.Get("build.mcu"))
-	require.Equal(t, "AVR_MEGA", targetBoard.Properties.Get("build.board"))
+	targetBoardBuildProperties := ctx.TargetBoardBuildProperties
+	require.Equal(t, "atmega1280", targetBoardBuildProperties.Get("build.mcu"))
+	require.Equal(t, "AVR_MEGA", targetBoardBuildProperties.Get("build.board"))
 }
 
 func TestTargetBoardResolverMega2560(t *testing.T) {
@@ -123,8 +126,9 @@ func TestTargetBoardResolverMega2560(t *testing.T) {
 	require.Equal(t, "avr", targetPlatform.Platform.Architecture)
 	targetBoard := ctx.TargetBoard
 	require.Equal(t, "mega", targetBoard.BoardID)
-	require.Equal(t, "atmega2560", targetBoard.Properties.Get("build.mcu"))
-	require.Equal(t, "AVR_MEGA2560", targetBoard.Properties.Get("build.board"))
+	targetBoardBuildProperties := ctx.TargetBoardBuildProperties
+	require.Equal(t, "atmega2560", targetBoardBuildProperties.Get("build.mcu"))
+	require.Equal(t, "AVR_MEGA2560", targetBoardBuildProperties.Get("build.board"))
 }
 
 func TestTargetBoardResolverCustomYun(t *testing.T) {
@@ -149,8 +153,9 @@ func TestTargetBoardResolverCustomYun(t *testing.T) {
 	require.Equal(t, "avr", targetPlatform.Platform.Architecture)
 	targetBoard := ctx.TargetBoard
 	require.Equal(t, "custom_yun", targetBoard.BoardID)
-	require.Equal(t, "atmega32u4", targetBoard.Properties.Get("build.mcu"))
-	require.Equal(t, "AVR_YUN", targetBoard.Properties.Get("build.board"))
+	targetBoardBuildProperties := ctx.TargetBoardBuildProperties
+	require.Equal(t, "atmega32u4", targetBoardBuildProperties.Get("build.mcu"))
+	require.Equal(t, "AVR_YUN", targetBoardBuildProperties.Get("build.board"))
 }
 
 func TestTargetBoardResolverCustomCore(t *testing.T) {
@@ -176,5 +181,6 @@ func TestTargetBoardResolverCustomCore(t *testing.T) {
 	targetBoard := ctx.TargetBoard
 	require.Equal(t, "attiny841", targetBoard.BoardID)
 	require.Equal(t, "tiny841", ctx.BuildCore)
-	require.Equal(t, "tiny14", targetBoard.Properties.Get("build.variant"))
+	targetBoardBuildProperties := ctx.TargetBoardBuildProperties
+	require.Equal(t, "tiny14", targetBoardBuildProperties.Get("build.variant"))
 }

legacy/builder/test/user_hardware/my_avr_platform/avr/boards.txt

@@ -70,7 +70,8 @@ mymega.menu.cpu.atmega2560.bootloader.extended_fuses=0xFD
 mymega.menu.cpu.atmega2560.bootloader.file={bootloader._folder}/stk500boot_v2_mega2560.hex
 
 mymega.menu.cpu.atmega2560.build.mcu=atmega2560
-mymega.menu.cpu.atmega2560.build.board=AVR_MYMEGA2560
+# Do not define build.board to test autogeneration
+#mymega.menu.cpu.atmega2560.build.board=AVR_MYMEGA2560
 
 mymega.menu.cpu.atmega1280=ATmega1280
 
@@ -84,4 +85,5 @@ mymega.menu.cpu.atmega1280.bootloader.extended_fuses=0xF5
 mymega.menu.cpu.atmega1280.bootloader.file={bootloader._folder}/ATmegaBOOT_168_atmega1280.hex
 
 mymega.menu.cpu.atmega1280.build.mcu=atmega1280
-mymega.menu.cpu.atmega1280.build.board=AVR_MYMEGA
+# define custom build.board for testing
+mymega.menu.cpu.atmega1280.build.board=MYMEGA1280

legacy/builder/types/context.go

@@ -87,6 +87,7 @@ type Context struct {
 	AllTools                   []*cores.ToolRelease
 	RequiredTools              []*cores.ToolRelease
 	TargetBoard                *cores.Board
+	TargetBoardBuildProperties *properties.Map
 	TargetPackage              *cores.Package
 	TargetPlatform             *cores.PlatformRelease
 	ActualPlatform             *cores.PlatformRelease