Add prioritization to tool selection in absence of explicit dependencies

Author: cmaglie

arduino/cores/packagemanager/package_manager.go

@@ -618,25 +618,50 @@ func (pme *Explorer) GetTool(toolID string) *cores.Tool {
 func (pme *Explorer) FindToolsRequiredForBoard(board *cores.Board) ([]*cores.ToolRelease, error) {
 	pme.log.Infof("Searching tools required for board %s", board)
 
-	// core := board.Properties["build.core"]
 	platform := board.PlatformRelease
 
-	// maps "PACKAGER:TOOL" => ToolRelease
-	foundTools := map[string]*cores.ToolRelease{}
-
-	// a Platform may not specify required tools (because it's a platform that comes from a
-	// user/hardware dir without a package_index.json) then add all available tools
-	for _, targetPackage := range pme.packages {
-		for _, tool := range targetPackage.Tools {
-			rel := tool.GetLatestInstalled()
-			if rel != nil {
-				foundTools[rel.Tool.Name] = rel
+	// maps tool name => all available ToolRelease
+	allToolsAlternatives := map[string][]*cores.ToolRelease{}
+	for _, tool := range pme.GetAllInstalledToolsReleases() {
+		alternatives := allToolsAlternatives[tool.Tool.Name]
+		alternatives = append(alternatives, tool)
+		allToolsAlternatives[tool.Tool.Name] = alternatives
+	}
+
+	// selectBest select the tool with best matching, applying the following rules
+	// in order of priority:
+	// - the tool comes from the requested packager
+	// - the tool has the greatest version
+	// - the tool packager comes first in alphabetic order
+	selectBest := func(tools []*cores.ToolRelease, packager string) *cores.ToolRelease {
+		selected := tools[0]
+		for _, tool := range tools[1:] {
+			if tool.Tool.Package.Name != selected.Tool.Package.Name {
+				if tool.Tool.Package.Name == packager {
+					selected = tool
+				}
+				continue
+			}
+			if !tool.Version.Equal(selected.Version) {
+				if tool.Version.GreaterThan(selected.Version) {
+					selected = tool
+				}
+				continue
+			}
+			if tool.Tool.Package.Name < selected.Tool.Package.Name {
+				selected = tool
 			}
 		}
+		return selected
 	}
 
-	// replace the default tools above with the specific required by the current platform
+	// First select the specific tools required by the current platform
 	requiredTools := []*cores.ToolRelease{}
+	// The Sorting of the tool dependencies is required because some platforms may depends
+	// on more than one version of the same tool. For example adafruit:samd has both
+	// [email protected] and [email protected]. To allow the runtime property
+	// {runtime.tools.bossac.path} to be correctly set to the 1.8.0 version we must ensure
+	// taht the returned array is sorted by version.
 	platform.ToolDependencies.Sort()
 	for _, toolDep := range platform.ToolDependencies {
 		pme.log.WithField("tool", toolDep).Infof("Required tool")
@@ -645,11 +670,15 @@ func (pme *Explorer) FindToolsRequiredForBoard(board *cores.Board) ([]*cores.Too
 			return nil, fmt.Errorf(tr("tool release not found: %s"), toolDep)
 		}
 		requiredTools = append(requiredTools, tool)
-		delete(foundTools, tool.Tool.Name)
+		delete(allToolsAlternatives, tool.Tool.Name)
 	}
 
-	for _, toolRel := range foundTools {
-		requiredTools = append(requiredTools, toolRel)
+	// Since a Platform may not specify the required tools (because it's a platform that comes
+	// from a user/hardware dir without a package_index.json) then add all available tools giving
+	// priority to tools coming from the same packager
+	for _, tools := range allToolsAlternatives {
+		tool := selectBest(tools, platform.Platform.Package.Name)
+		requiredTools = append(requiredTools, tool)
 	}
 	return requiredTools, nil
 }

internal/integrationtest/compile/compile_test.go

@@ -0,0 +1,65 @@
+// 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 (
+	"testing"
+
+	"github.com/arduino/arduino-cli/internal/integrationtest"
+	"github.com/arduino/go-paths-helper"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRuntimeToolPropertiesGeneration(t *testing.T) {
+	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)
+
+	// Install test data into datadir
+	testdata := paths.New("testdata", "platforms_with_conflicting_tools")
+	hardwareDir := cli.DataDir().Join("packages")
+	err = testdata.Join("alice").CopyDirTo(hardwareDir.Join("alice"))
+	require.NoError(t, err)
+	err = testdata.Join("bob").CopyDirTo(hardwareDir.Join("bob"))
+	require.NoError(t, err)
+
+	sketch, err := paths.New("testdata", "bare_minimum").Abs()
+	require.NoError(t, err)
+
+	// Multiple runs must always produce the same result
+	for i := 0; i < 3; i++ {
+		stdout, _, err := cli.Run("compile", "-b", "alice:avr:alice", "--show-properties", sketch.String())
+		require.NoError(t, err)
+		// the tools coming from the same packager are selected
+		require.Contains(t, string(stdout), "runtime.tools.avr-gcc.path="+hardwareDir.String()+"/alice/tools/avr-gcc/50.0.0")
+		require.Contains(t, string(stdout), "runtime.tools.avrdude.path="+hardwareDir.String()+"/alice/tools/avrdude/1.0.0")
+
+		stdout, _, err = cli.Run("compile", "-b", "bob:avr:bob", "--show-properties", sketch.String())
+		require.NoError(t, err)
+		// the latest version available are selected
+		require.Contains(t, string(stdout), "runtime.tools.avr-gcc.path="+hardwareDir.String()+"/alice/tools/avr-gcc/50.0.0")
+		require.Contains(t, string(stdout), "runtime.tools.avrdude.path="+hardwareDir.String()+"/arduino/tools/avrdude/6.3.0-arduino17")
+
+		stdout, _, err = cli.Run("compile", "-b", "arduino:avr:uno", "--show-properties", sketch.String())
+		require.NoError(t, err)
+		// the selected tools are listed as platform dependencies from the index.json
+		require.Contains(t, string(stdout), "runtime.tools.avr-gcc.path="+hardwareDir.String()+"/arduino/tools/avr-gcc/7.3.0-atmel3.6.1-arduino7")
+		require.Contains(t, string(stdout), "runtime.tools.avrdude.path="+hardwareDir.String()+"/arduino/tools/avrdude/6.3.0-arduino17")
+	}
+}

internal/integrationtest/compile/testdata/bare_minimum/bare_minimum.ino

@@ -0,0 +1,2 @@
+void setup() {}
+void loop() {}

internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt

@@ -0,0 +1 @@
+alice.name=MyBoard

internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avr-gcc/50.0.0/.keep

@@ -0,0 +1 @@
+bob.name=Alice's Board