Fix handling of referenced cores (#467)

* Add tests for ResolveFQBN

This adds tests for ResolveFQBN (which returns more details than the
FindBoardWithFQBN that was already tested), but also adds an extra
boards.txt with boards that are derived from another platform/core (i.e.
have `build.core=package:core`.

These new tests are currently failing, because of a bug and insufficient
error handling, which will be fixed in subsequent commits.

* Fix resolution of referenced cores

A core is referenced by specifying `build.core=package:core` in board
properties. However, the code used the second part (core) both for
looking up the the package, as well as for the core to use from that
package. The most commonly used core reference is `arduino:arduino`
which works because both parts are identical, which is probably why this
bug has not shown up before.

This commit fixes the bug by simply using the right part to look up the
package.

* Improve error message when referencing missing package

This used to say e.g.

	missing platform adafruit:referenced:avr required for build

where two packagenames (adafruit and referenced) were joined together
which makes no sense. Now, it just mentions the missing package, and the
fqbn that references it.

* Fix segfault when referenced platform is missing

Before, when a platform was referenced through a `build.core` and the
package was present by the platform/architecure was missing, a
nullpointer was passed to GetInstalledPlatformRelease, which would
segfault.

Now, a proper error message is returned.

Author: matthijskooijman

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

@@ -177,13 +177,17 @@ func (pm *PackageManager) ResolveFQBN(fqbn *cores.FQBN) (
 	buildPlatformRelease := platformRelease
 	coreParts := strings.Split(buildProperties.Get("build.core"), ":")
 	if len(coreParts) > 1 {
-		referredPackage := coreParts[1]
+		referredPackage := coreParts[0]
 		buildPackage := pm.Packages[referredPackage]
 		if buildPackage == nil {
 			return targetPackage, platformRelease, board, buildProperties, nil,
-				fmt.Errorf("missing package %s:%s required for build", referredPackage, platform)
+				fmt.Errorf("missing package %s referenced by board %s", referredPackage, fqbn)
 		}
 		buildPlatform := buildPackage.Platforms[fqbn.PlatformArch]
+		if buildPlatform == nil {
+			return targetPackage, platformRelease, board, buildProperties, nil,
+				fmt.Errorf("missing platform %s:%s referenced by board %s", referredPackage, fqbn.PlatformArch, fqbn)
+		}
 		buildPlatformRelease = pm.GetInstalledPlatformRelease(buildPlatform)
 	}
 

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

@@ -33,6 +33,9 @@ import (
 var customHardware = paths.New("testdata", "custom_hardware")
 var dataDir1 = paths.New("testdata", "data_dir_1")
 
+// Intended to be used alongside dataDir1
+var extraHardware = paths.New("testdata", "extra_hardware")
+
 func TestFindBoardWithFQBN(t *testing.T) {
 	pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware)
 	pm.LoadHardwareFromDirectory(customHardware)
@@ -48,6 +51,128 @@ func TestFindBoardWithFQBN(t *testing.T) {
 	require.Equal(t, board.Name(), "Arduino/Genuino Mega or Mega 2560")
 }
 
+func TestResolveFQBN(t *testing.T) {
+	// Pass nil, since these paths are only used for installing
+	pm := packagemanager.NewPackageManager(nil, nil, nil, nil)
+	// Hardware from main packages directory
+	pm.LoadHardwareFromDirectory(dataDir1.Join("packages"))
+	// This contains the arduino:avr core
+	pm.LoadHardwareFromDirectory(customHardware)
+	// This contains the referenced:avr core
+	pm.LoadHardwareFromDirectory(extraHardware)
+
+	fqbn, err := cores.ParseFQBN("arduino:avr:uno")
+	require.Nil(t, err)
+	require.NotNil(t, fqbn)
+	pkg, platformRelease, board, props, buildPlatformRelease, err := pm.ResolveFQBN(fqbn)
+	require.Nil(t, err)
+	require.Equal(t, pkg, platformRelease.Platform.Package)
+	require.NotNil(t, platformRelease)
+	require.NotNil(t, platformRelease.Platform)
+	require.Equal(t, platformRelease.Platform.String(), "arduino:avr")
+	require.NotNil(t, board)
+	require.Equal(t, board.Name(), "Arduino/Genuino Uno")
+	require.NotNil(t, props)
+	require.Equal(t, platformRelease, buildPlatformRelease)
+
+	fqbn, err = cores.ParseFQBN("arduino:avr:mega")
+	require.Nil(t, err)
+	require.NotNil(t, fqbn)
+	pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn)
+	require.Nil(t, err)
+	require.Equal(t, pkg, platformRelease.Platform.Package)
+	require.NotNil(t, platformRelease)
+	require.NotNil(t, platformRelease.Platform)
+	require.Equal(t, platformRelease.Platform.String(), "arduino:avr")
+	require.NotNil(t, board)
+	require.Equal(t, board.Name(), "Arduino/Genuino Mega or Mega 2560")
+	require.NotNil(t, props)
+	require.Equal(t, platformRelease, buildPlatformRelease)
+
+	// Test a board referenced from the main AVR arduino platform
+	fqbn, err = cores.ParseFQBN("referenced:avr:uno")
+	require.Nil(t, err)
+	require.NotNil(t, fqbn)
+	pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn)
+	require.Nil(t, err)
+	require.Equal(t, pkg, platformRelease.Platform.Package)
+	require.NotNil(t, platformRelease)
+	require.NotNil(t, platformRelease.Platform)
+	require.Equal(t, platformRelease.Platform.String(), "referenced:avr")
+	require.NotNil(t, board)
+	require.Equal(t, board.Name(), "Referenced Uno")
+	require.NotNil(t, props)
+	require.NotNil(t, buildPlatformRelease)
+	require.NotNil(t, buildPlatformRelease.Platform)
+	require.Equal(t, buildPlatformRelease.Platform.String(), "arduino:avr")
+
+	// Test a board referenced from the Adafruit SAMD core (this tests
+	// deriving where the package and core name are different)
+	fqbn, err = cores.ParseFQBN("referenced:samd:feather_m0")
+	require.Nil(t, err)
+	require.NotNil(t, fqbn)
+	pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn)
+	require.Nil(t, err)
+	require.Equal(t, pkg, platformRelease.Platform.Package)
+	require.NotNil(t, platformRelease)
+	require.NotNil(t, platformRelease.Platform)
+	require.Equal(t, platformRelease.Platform.String(), "referenced:samd")
+	require.NotNil(t, board)
+	require.Equal(t, board.Name(), "Referenced Feather M0")
+	require.NotNil(t, props)
+	require.NotNil(t, buildPlatformRelease)
+	require.NotNil(t, buildPlatformRelease.Platform)
+	require.Equal(t, buildPlatformRelease.Platform.String(), "adafruit:samd")
+
+	// Test a board referenced from a non-existent package
+	fqbn, err = cores.ParseFQBN("referenced:avr:dummy_invalid_package")
+	require.Nil(t, err)
+	require.NotNil(t, fqbn)
+	pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn)
+	require.NotNil(t, err)
+	require.Equal(t, pkg, platformRelease.Platform.Package)
+	require.NotNil(t, platformRelease)
+	require.NotNil(t, platformRelease.Platform)
+	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, buildPlatformRelease)
+
+	// Test a board referenced from a non-existent platform/architecture
+	fqbn, err = cores.ParseFQBN("referenced:avr:dummy_invalid_platform")
+	require.Nil(t, err)
+	require.NotNil(t, fqbn)
+	pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn)
+	require.NotNil(t, err)
+	require.Equal(t, pkg, platformRelease.Platform.Package)
+	require.NotNil(t, platformRelease)
+	require.NotNil(t, platformRelease.Platform)
+	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, buildPlatformRelease)
+
+	// Test a board referenced from a non-existent core
+	// Note that ResolveFQBN does not actually check this currently
+	fqbn, err = cores.ParseFQBN("referenced:avr:dummy_invalid_core")
+	require.Nil(t, err)
+	require.NotNil(t, fqbn)
+	pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn)
+	require.Nil(t, err)
+	require.Equal(t, pkg, platformRelease.Platform.Package)
+	require.NotNil(t, platformRelease)
+	require.NotNil(t, platformRelease.Platform)
+	require.Equal(t, platformRelease.Platform.String(), "referenced:avr")
+	require.NotNil(t, board)
+	require.Equal(t, board.Name(), "Referenced dummy with invalid core")
+	require.NotNil(t, props)
+	require.NotNil(t, buildPlatformRelease)
+	require.NotNil(t, buildPlatformRelease.Platform)
+	require.Equal(t, buildPlatformRelease.Platform.String(), "arduino:avr")
+}
+
 func TestBoardOptionsFunctions(t *testing.T) {
 	pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware)
 	pm.LoadHardwareFromDirectory(customHardware)

Diff for: arduino/cores/packagemanager/testdata/extra_hardware/referenced/avr/boards.txt

@@ -0,0 +1,38 @@
+# Dummy board that is pretty much identical to the Uno, but defined in a
+# different package (to test using a core from a different package where
+# the package name and core name are the same).
+uno.name=Referenced Uno
+
+uno.upload.tool=avrdude
+uno.upload.protocol=arduino
+uno.upload.maximum_size=32256
+uno.upload.maximum_data_size=2048
+uno.upload.speed=115200
+
+uno.bootloader.tool=avrdude
+uno.bootloader.low_fuses=0xFF
+uno.bootloader.high_fuses=0xDE
+uno.bootloader.extended_fuses=0xFD
+uno.bootloader.unlock_bits=0x3F
+uno.bootloader.lock_bits=0x0F
+uno.bootloader.file=optiboot/optiboot_atmega328.hex
+
+uno.build.mcu=atmega328p
+uno.build.f_cpu=16000000L
+uno.build.board=AVR_UNO
+uno.build.core=arduino:arduino
+uno.build.variant=standard
+
+# Dummy board derived from a non-existent package
+dummy_invalid_package.name=Referenced dummy with invalid package
+dummy_invalid_package.build.core=nonexistent:arduino
+
+# Dummy board derived from a non-existent core
+dummy_invalid_core.name=Referenced dummy with invalid core
+dummy_invalid_core.build.core=arduino:nonexistent
+
+# Dummy board derived from a non-existent platform/architecture. The
+# platform is avr, which is implied by the directory this file is in. The
+# adafruit package in this test data only supplies a samd platform.
+dummy_invalid_platform.name=Referenced dummy with invalid platform
+dummy_invalid_platform.build.core=adafruit:arduino

Diff for: arduino/cores/packagemanager/testdata/extra_hardware/referenced/samd/boards.txt

@@ -0,0 +1,27 @@
+# Dummy board that is pretty much identical to the feather m0, but
+# defined in a different package (to test using a core from a different
+# package where the package name and core name are different).
+feather_m0.name=Referenced Feather M0
+feather_m0.upload.tool=bossac
+feather_m0.upload.protocol=sam-ba
+feather_m0.upload.maximum_size=262144
+feather_m0.upload.offset=0x2000
+feather_m0.upload.use_1200bps_touch=true
+feather_m0.upload.wait_for_upload_port=true
+feather_m0.upload.native_usb=true
+feather_m0.build.mcu=cortex-m0plus
+feather_m0.build.f_cpu=48000000L
+feather_m0.build.usb_product="Feather M0"
+feather_m0.build.usb_manufacturer="Adafruit"
+feather_m0.build.board=SAMD_ZERO
+feather_m0.build.core=adafruit:arduino
+feather_m0.build.extra_flags=-DARDUINO_SAMD_ZERO -DARM_MATH_CM0PLUS -DADAFRUIT_FEATHER_M0 -D__SAMD21G18A__ {build.usb_flags}
+feather_m0.build.ldscript=linker_scripts/gcc/flash_with_bootloader.ld
+feather_m0.build.openocdscript=openocd_scripts/feather_m0.cfg
+feather_m0.build.variant=feather_m0
+feather_m0.build.variant_system_lib=
+feather_m0.build.vid=0x239A
+feather_m0.build.pid=0x800B
+feather_m0.bootloader.tool=openocd
+feather_m0.bootloader.file=featherM0/bootloader-feather_m0-v2.0.0-adafruit.5.bin
+