From 2c5db46af28b384aed8ac4b3627eb13d97db05ef Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 21 Sep 2022 11:23:24 +0200 Subject: [PATCH 1/6] Add prioritization to tool selection in absence of explicit dependencies --- .../cores/packagemanager/package_manager.go | 60 ++++++++++++----- .../integrationtest/compile/compile_test.go | 65 +++++++++++++++++++ .../testdata/bare_minimum/bare_minimum.ino | 2 + .../alice/hardware/avr/1.0.0/boards.txt | 1 + .../alice/tools/avr-gcc/50.0.0/.keep | 0 .../alice/tools/avrdude/1.0.0/.keep | 0 .../bob/hardware/avr/1.0.0/boards.txt | 1 + 7 files changed, 114 insertions(+), 15 deletions(-) create mode 100644 internal/integrationtest/compile/compile_test.go create mode 100644 internal/integrationtest/compile/testdata/bare_minimum/bare_minimum.ino create mode 100644 internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt create mode 100644 internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avr-gcc/50.0.0/.keep create mode 100644 internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avrdude/1.0.0/.keep create mode 100644 internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt diff --git a/arduino/cores/packagemanager/package_manager.go b/arduino/cores/packagemanager/package_manager.go index ad23629840d..2d96b0368ef 100644 --- a/arduino/cores/packagemanager/package_manager.go +++ b/arduino/cores/packagemanager/package_manager.go @@ -618,25 +618,51 @@ 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] + namePriority := map[string]int{packager: 1} + for _, tool := range tools[1:] { + if namePriority[tool.Tool.Package.Name] != namePriority[selected.Tool.Package.Name] { + if namePriority[tool.Tool.Package.Name] > namePriority[selected.Tool.Package.Name] { + 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 + // bossac@1.8.0-48-gb176eee and bossac@1.7.0. 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 +671,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 } diff --git a/internal/integrationtest/compile/compile_test.go b/internal/integrationtest/compile/compile_test.go new file mode 100644 index 00000000000..0ef8c739a02 --- /dev/null +++ b/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 license@arduino.cc. + +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:avr@1.8.5") + 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") + } +} diff --git a/internal/integrationtest/compile/testdata/bare_minimum/bare_minimum.ino b/internal/integrationtest/compile/testdata/bare_minimum/bare_minimum.ino new file mode 100644 index 00000000000..660bdbccfdb --- /dev/null +++ b/internal/integrationtest/compile/testdata/bare_minimum/bare_minimum.ino @@ -0,0 +1,2 @@ +void setup() {} +void loop() {} diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt new file mode 100644 index 00000000000..ec403ca11bc --- /dev/null +++ b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt @@ -0,0 +1 @@ +alice.name=MyBoard \ No newline at end of file diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avr-gcc/50.0.0/.keep b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avr-gcc/50.0.0/.keep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avrdude/1.0.0/.keep b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avrdude/1.0.0/.keep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt new file mode 100644 index 00000000000..8bf1736244c --- /dev/null +++ b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt @@ -0,0 +1 @@ +bob.name=Alice's Board \ No newline at end of file From 496b5ccba004519d29ac161bf4969164b5fa2d16 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 22 Sep 2022 12:09:11 +0200 Subject: [PATCH 2/6] Made tests working on windows too --- .../integrationtest/compile/compile_test.go | 19 +++++++++++++------ .../alice/hardware/avr/1.0.0/boards.txt | 3 ++- .../bob/hardware/avr/1.0.0/boards.txt | 3 ++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/internal/integrationtest/compile/compile_test.go b/internal/integrationtest/compile/compile_test.go index 0ef8c739a02..f020e7c4780 100644 --- a/internal/integrationtest/compile/compile_test.go +++ b/internal/integrationtest/compile/compile_test.go @@ -20,6 +20,7 @@ import ( "github.com/arduino/arduino-cli/internal/integrationtest" "github.com/arduino/go-paths-helper" + "github.com/arduino/go-properties-orderedmap" "github.com/stretchr/testify/require" ) @@ -46,20 +47,26 @@ func TestRuntimeToolPropertiesGeneration(t *testing.T) { for i := 0; i < 3; i++ { stdout, _, err := cli.Run("compile", "-b", "alice:avr:alice", "--show-properties", sketch.String()) require.NoError(t, err) + res, err := properties.LoadFromBytes(stdout) + 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") + require.True(t, res.GetPath("runtime.tools.avr-gcc.path").EquivalentTo(hardwareDir.Join("alice", "tools", "avr-gcc", "50.0.0"))) + require.True(t, res.GetPath("runtime.tools.avrdude.path").EquivalentTo(hardwareDir.Join("alice", "tools", "avrdude", "1.0.0"))) stdout, _, err = cli.Run("compile", "-b", "bob:avr:bob", "--show-properties", sketch.String()) require.NoError(t, err) + res, err = properties.LoadFromBytes(stdout) + 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") + require.True(t, res.GetPath("runtime.tools.avr-gcc.path").EquivalentTo(hardwareDir.Join("alice", "tools", "avr-gcc", "50.0.0"))) + require.True(t, res.GetPath("runtime.tools.avrdude.path").EquivalentTo(hardwareDir.Join("arduino", "tools", "avrdude", "6.3.0-arduino17"))) stdout, _, err = cli.Run("compile", "-b", "arduino:avr:uno", "--show-properties", sketch.String()) require.NoError(t, err) + res, err = properties.LoadFromBytes(stdout) + 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") + require.True(t, res.GetPath("runtime.tools.avr-gcc.path").EquivalentTo(hardwareDir.Join("arduino", "tools", "avr-gcc", "7.3.0-atmel3.6.1-arduino7"))) + require.True(t, res.GetPath("runtime.tools.avrdude.path").EquivalentTo(hardwareDir.Join("arduino", "tools", "avrdude", "6.3.0-arduino17"))) } } diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt index ec403ca11bc..3ae4a8a5acc 100644 --- a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt +++ b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt @@ -1 +1,2 @@ -alice.name=MyBoard \ No newline at end of file +alice.name=MyBoard +alice.build.board=ALICE \ No newline at end of file diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt index 8bf1736244c..28d23e9d7a6 100644 --- a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt +++ b/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt @@ -1 +1,2 @@ -bob.name=Alice's Board \ No newline at end of file +bob.name=Bob's Board +bob.build.board=BOB \ No newline at end of file From 14ded16b96dce15161ddd9174be555aede923a0b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 11 Oct 2022 15:07:17 +0200 Subject: [PATCH 3/6] Updated docs --- docs/package_index_json-specification.md | 37 +++++++++++++++++------- docs/platform-specification.md | 12 +++++--- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/docs/package_index_json-specification.md b/docs/package_index_json-specification.md index c07cfa59aa5..d37b4d6fd3e 100644 --- a/docs/package_index_json-specification.md +++ b/docs/package_index_json-specification.md @@ -176,17 +176,6 @@ The other fields are: macOS you can use the command `shasum -a 256 filename` to generate SHA-256 checksums. There are free options for Windows, including md5deep. There are also online utilities for generating checksums. -##### How a tool's path is determined in platform.txt - -When the IDE needs a tool, it downloads the corresponding archive file and unpacks the content into a private folder -that can be referenced from `platform.txt` using one of the following properties: - -- `{runtime.tools.TOOLNAME-VERSION.path}` -- `{runtime.tools.TOOLNAME.path}` - -For example, to obtain the avr-gcc 4.8.1 folder we can use `{runtime.tools.avr-gcc-4.8.1-arduino5.path}` or -`{runtime.tools.avr-gcc.path}`. - ### Platforms definitions Finally, let's see how `PLATFORMS` are made. @@ -270,6 +259,32 @@ rules Arduino IDE follows for parsing versions Note: if you miss a bracket in the JSON index, then add the URL to your Preferences, and open Boards Manager it can cause the Arduino IDE to no longer load until you have deleted the file from your arduino15 folder. +#### How a tool's path is determined in platform.txt + +When the IDE needs a tool, it downloads the corresponding archive file and unpacks the content into a private folder +that can be referenced from `platform.txt` using one of the following properties: + +- `{runtime.tools.TOOLNAME-VERSION.path}` +- `{runtime.tools.TOOLNAME.path}` + +For example, to obtain the avr-gcc 4.8.1 folder we can use `{runtime.tools.avr-gcc-4.8.1.path}` or +`{runtime.tools.avr-gcc.path}`. + +In general the same tool may be provided by different packagers (for example the Arduino packager may provide an +`arduino:avr-gcc` and another 3rd party packager may provide their own `3rdparty:avr-gcc`). The rules to disambiguate +are as follows: + +- The property `{runtime.tools.TOOLNAME.path}` points, in order of priority, to: + + 1. the tool, version and packager specified via `toolsDependencies` in the `package_index.json` + 1. the highest version of the tool provided by the packager of the current platform + 1. the highest version of the tool provided by any other packager (in case of tie, the first packager in alphabetical + order wins) + +- The property `{runtime.tools.TOOLNAME-VERSION.path}` points, in order of priority, to: + 1. the tool and version provided by the packager of the current platform + 1. the tool and version provided by any other packager (in case of tie, the first packager in alphabetical order wins) + ### Example JSON index file ```json diff --git a/docs/platform-specification.md b/docs/platform-specification.md index cf0d08f547b..5f26ac779b9 100644 --- a/docs/platform-specification.md +++ b/docs/platform-specification.md @@ -766,14 +766,18 @@ tools.avrdude.config.path={path}/etc/avrdude.conf tools.avrdude.upload.pattern="{cmd.path}" "-C{config.path}" -p{build.mcu} -c{upload.port.protocol} -P{upload.port.address} -b{upload.speed} -D "-Uflash:w:{build.path}/{build.project_name}.hex:i" ``` -A **{runtime.tools.TOOL_NAME.path}** and **{runtime.tools.TOOL_NAME-TOOL_VERSION.path}** property is generated for the -tools of Arduino AVR Boards and any other platform installed via Boards Manager. **{runtime.tools.TOOL_NAME.path}** -points to the latest version of the tool available. - The tool configuration properties are available globally without the prefix. For example, the **tools.avrdude.cmd.path** property can be used as **{cmd.path}** inside the recipe, and the same happens for all the other avrdude configuration variables. +### How to retrieve tools path via `{runtime.tools.*}` properties + +A **{runtime.tools.TOOLNAME.path}** and **{runtime.tools.TOOLNAME-TOOLVERSION.path}** property is generated for the +tools provided by the current platform and for any other platform installed via Boards Manager. + +See [`{runtime.tools.*.path}` rules](package_index_json-specification.md#how-a-tools-path-is-determined-in-platformtxt) +for details on how the runtime properties are determined. + ### Environment variables All the tools launched to compile or upload a sketch will have the following environment variable set: From 8a44930824b8cc8b85ddaf9e837c813d1f2f518e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 23 Nov 2022 14:20:54 +0100 Subject: [PATCH 4/6] Added tool selection from referenced platform --- .../cores/packagemanager/package_manager.go | 27 ++++++----- .../packagemanager/package_manager_test.go | 45 ++++++++++++++++++- .../data_dir_1/package_test_index.json | 45 +++++++++++++++++++ .../test/hardware/avr/1.1.0/boards.txt | 0 .../packages/test/tools/bossac/1.7.5/.keep | 0 commands/debug/debug_info.go | 4 +- commands/upload/upload.go | 2 +- docs/UPGRADING.md | 17 +++++++ docs/package_index_json-specification.md | 6 +++ legacy/builder/target_board_resolver.go | 8 ++-- 10 files changed, 133 insertions(+), 21 deletions(-) create mode 100644 arduino/cores/packagemanager/testdata/data_dir_1/package_test_index.json create mode 100644 arduino/cores/packagemanager/testdata/data_dir_1/packages/test/hardware/avr/1.1.0/boards.txt create mode 100644 arduino/cores/packagemanager/testdata/data_dir_1/packages/test/tools/bossac/1.7.5/.keep diff --git a/arduino/cores/packagemanager/package_manager.go b/arduino/cores/packagemanager/package_manager.go index 2d96b0368ef..8cd630e87d8 100644 --- a/arduino/cores/packagemanager/package_manager.go +++ b/arduino/cores/packagemanager/package_manager.go @@ -614,11 +614,9 @@ func (pme *Explorer) GetTool(toolID string) *cores.Tool { } } -// FindToolsRequiredForBoard FIXMEDOC -func (pme *Explorer) FindToolsRequiredForBoard(board *cores.Board) ([]*cores.ToolRelease, error) { - pme.log.Infof("Searching tools required for board %s", board) - - platform := board.PlatformRelease +// FindToolsRequiredForBuild returns the list of ToolReleases needed to build for the specified +// plaftorm. The buildPlatform may be different depending on the selected board. +func (pme *Explorer) FindToolsRequiredForBuild(platform, buildPlatform *cores.PlatformRelease) ([]*cores.ToolRelease, error) { // maps tool name => all available ToolRelease allToolsAlternatives := map[string][]*cores.ToolRelease{} @@ -631,14 +629,19 @@ func (pme *Explorer) FindToolsRequiredForBoard(board *cores.Board) ([]*cores.Too // selectBest select the tool with best matching, applying the following rules // in order of priority: // - the tool comes from the requested packager + // - the tool comes from the build platform packager // - the tool has the greatest version // - the tool packager comes first in alphabetic order - selectBest := func(tools []*cores.ToolRelease, packager string) *cores.ToolRelease { + packagerPriority := map[string]int{} + packagerPriority[platform.Platform.Package.Name] = 2 + if buildPlatform != nil { + packagerPriority[buildPlatform.Platform.Package.Name] = 1 + } + selectBest := func(tools []*cores.ToolRelease) *cores.ToolRelease { selected := tools[0] - namePriority := map[string]int{packager: 1} for _, tool := range tools[1:] { - if namePriority[tool.Tool.Package.Name] != namePriority[selected.Tool.Package.Name] { - if namePriority[tool.Tool.Package.Name] > namePriority[selected.Tool.Package.Name] { + if packagerPriority[tool.Tool.Package.Name] != packagerPriority[selected.Tool.Package.Name] { + if packagerPriority[tool.Tool.Package.Name] > packagerPriority[selected.Tool.Package.Name] { selected = tool } continue @@ -662,7 +665,7 @@ func (pme *Explorer) FindToolsRequiredForBoard(board *cores.Board) ([]*cores.Too // on more than one version of the same tool. For example adafruit:samd has both // bossac@1.8.0-48-gb176eee and bossac@1.7.0. 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. + // that the returned array is sorted by version. platform.ToolDependencies.Sort() for _, toolDep := range platform.ToolDependencies { pme.log.WithField("tool", toolDep).Infof("Required tool") @@ -676,9 +679,9 @@ func (pme *Explorer) FindToolsRequiredForBoard(board *cores.Board) ([]*cores.Too // 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 + // priority to tools coming from the same packager or referenced packager for _, tools := range allToolsAlternatives { - tool := selectBest(tools, platform.Platform.Package.Name) + tool := selectBest(tools) requiredTools = append(requiredTools, tool) } return requiredTools, nil diff --git a/arduino/cores/packagemanager/package_manager_test.go b/arduino/cores/packagemanager/package_manager_test.go index 580311d3225..6c35ace186f 100644 --- a/arduino/cores/packagemanager/package_manager_test.go +++ b/arduino/cores/packagemanager/package_manager_test.go @@ -287,6 +287,8 @@ func TestFindToolsRequiredForBoard(t *testing.T) { loadIndex("https://dl.espressif.com/dl/package_esp32_index.json") loadIndex("http://arduino.esp8266.com/stable/package_esp8266com_index.json") loadIndex("https://adafruit.github.io/arduino-board-index/package_adafruit_index.json") + loadIndex("https://test.com/package_test_index.json") + // We ignore the errors returned since they might not be necessarily blocking // but just warnings for the user, like in the case a board is not loaded // because of malformed menus @@ -310,8 +312,13 @@ func TestFindToolsRequiredForBoard(t *testing.T) { }) require.NotNil(t, esptool0413) + testPlatform := pme.FindPlatformRelease(&packagemanager.PlatformReference{ + Package: "test", + PlatformArchitecture: "avr", + PlatformVersion: semver.MustParse("1.1.0")}) + testConflictingToolsInDifferentPackages := func() { - tools, err := pme.FindToolsRequiredForBoard(esp32) + tools, err := pme.FindToolsRequiredForBuild(esp32.PlatformRelease, nil) require.NoError(t, err) require.Contains(t, tools, esptool231) require.NotContains(t, tools, esptool0413) @@ -331,10 +338,44 @@ func TestFindToolsRequiredForBoard(t *testing.T) { testConflictingToolsInDifferentPackages() testConflictingToolsInDifferentPackages() + { + // Test buildPlatform dependencies + arduinoBossac180 := pme.FindToolDependency(&cores.ToolDependency{ + ToolPackager: "arduino", + ToolName: "bossac", + ToolVersion: semver.ParseRelaxed("1.8.0-48-gb176eee"), + }) + require.NotNil(t, arduinoBossac180) + testBossac175 := pme.FindToolDependency(&cores.ToolDependency{ + ToolPackager: "test", + ToolName: "bossac", + ToolVersion: semver.ParseRelaxed("1.7.5"), + }) + require.NotNil(t, testBossac175) + + tools, err := pme.FindToolsRequiredForBuild(esp32.PlatformRelease, nil) + require.NoError(t, err) + require.Contains(t, tools, esptool231) + require.NotContains(t, tools, esptool0413) + // When building without testPlatform dependency, arduino:bossac should be selected + // since it has the higher version + require.NotContains(t, tools, testBossac175) + require.Contains(t, tools, arduinoBossac180) + + tools, err = pme.FindToolsRequiredForBuild(esp32.PlatformRelease, testPlatform) + require.NoError(t, err) + require.Contains(t, tools, esptool231) + require.NotContains(t, tools, esptool0413) + // When building with testPlatform dependency, test:bossac should be selected + // because it has dependency priority + require.Contains(t, tools, testBossac175) + require.NotContains(t, tools, arduinoBossac180) + } + feather, err := pme.FindBoardWithFQBN("adafruit:samd:adafruit_feather_m0_express") require.NoError(t, err) require.NotNil(t, feather) - featherTools, err := pme.FindToolsRequiredForBoard(feather) + featherTools, err := pme.FindToolsRequiredForBuild(feather.PlatformRelease, nil) require.NoError(t, err) require.NotNil(t, featherTools) diff --git a/arduino/cores/packagemanager/testdata/data_dir_1/package_test_index.json b/arduino/cores/packagemanager/testdata/data_dir_1/package_test_index.json new file mode 100644 index 00000000000..daba4bf1504 --- /dev/null +++ b/arduino/cores/packagemanager/testdata/data_dir_1/package_test_index.json @@ -0,0 +1,45 @@ +{ + "packages": [ + { + "name": "test", + "websiteURL": "https://test.com", + "email": "test@test.com", + "help": { + "online": "https://test.com" + }, + "maintainer": "Test", + "platforms": [ + { + "architecture": "avr", + "boards": [], + "name": "Test AVR Boards", + "category": "Test", + "version": "1.1.0", + "archiveFileName": "test-1.1.0.tar.bz2", + "checksum": "SHA-256:4e72d4267d9a8d86874edcd021dc661854a5136c0eed947a6fe10366bc51e373", + "help": { + "online": "https://test.com" + }, + "url": "https://test.com/test-1.1.0.tar.bz2", + "toolsDependencies": [], + "size": "12345" + } + ], + "tools": [ + { + "name": "bossac", + "version": "1.7.5", + "systems": [ + { + "host": "i386-apple-darwin11", + "checksum": "MD5:603bcce8e59683ac27054b3197a53254", + "size": "96372129", + "archiveFileName": "bossac.tar.bz2", + "url": "https://test.com/bossac.tar.bz2" + } + ] + } + ] + } + ] +} diff --git a/arduino/cores/packagemanager/testdata/data_dir_1/packages/test/hardware/avr/1.1.0/boards.txt b/arduino/cores/packagemanager/testdata/data_dir_1/packages/test/hardware/avr/1.1.0/boards.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/arduino/cores/packagemanager/testdata/data_dir_1/packages/test/tools/bossac/1.7.5/.keep b/arduino/cores/packagemanager/testdata/data_dir_1/packages/test/tools/bossac/1.7.5/.keep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/commands/debug/debug_info.go b/commands/debug/debug_info.go index 9b5a1ff920a..38e0caf4936 100644 --- a/commands/debug/debug_info.go +++ b/commands/debug/debug_info.go @@ -66,7 +66,7 @@ func getDebugProperties(req *debug.DebugConfigRequest, pme *packagemanager.Explo } // Find target board and board properties - _, platformRelease, board, boardProperties, referencedPlatformRelease, err := pme.ResolveFQBN(fqbn) + _, platformRelease, _, boardProperties, referencedPlatformRelease, err := pme.ResolveFQBN(fqbn) if err != nil { return nil, &arduino.UnknownFQBNError{Cause: err} } @@ -98,7 +98,7 @@ func getDebugProperties(req *debug.DebugConfigRequest, pme *packagemanager.Explo for _, tool := range pme.GetAllInstalledToolsReleases() { toolProperties.Merge(tool.RuntimeProperties()) } - if requiredTools, err := pme.FindToolsRequiredForBoard(board); err == nil { + if requiredTools, err := pme.FindToolsRequiredForBuild(platformRelease, referencedPlatformRelease); err == nil { for _, requiredTool := range requiredTools { logrus.WithField("tool", requiredTool).Info("Tool required for debug") toolProperties.Merge(requiredTool.RuntimeProperties()) diff --git a/commands/upload/upload.go b/commands/upload/upload.go index c3b9e86e0a0..3168281fa03 100644 --- a/commands/upload/upload.go +++ b/commands/upload/upload.go @@ -295,7 +295,7 @@ func runProgramAction(pme *packagemanager.Explorer, for _, tool := range pme.GetAllInstalledToolsReleases() { uploadProperties.Merge(tool.RuntimeProperties()) } - if requiredTools, err := pme.FindToolsRequiredForBoard(board); err == nil { + if requiredTools, err := pme.FindToolsRequiredForBuild(boardPlatform, buildPlatform); err == nil { for _, requiredTool := range requiredTools { logrus.WithField("tool", requiredTool).Info("Tool required for upload") if requiredTool.IsInstalled() { diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 9c9cb4b068c..0d867fe05db 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -87,6 +87,23 @@ plus `Name`, `Version`, and an `UpToDate` boolean flag. `InstallZipLib` method `archivePath` is now a `paths.Path` instead of a `string`. +### golang API change in `github.com/arduino/arduino-cli/rduino/cores/packagemanager.Explorer` + +The `packagemanager.Explorer` method `FindToolsRequiredForBoard`: + +```go +func (pme *Explorer) FindToolsRequiredForBoard(board *cores.Board) ([]*cores.ToolRelease, error) { ... } +``` + +has been renamed to `FindToolsRequiredForBuild: + +```go +func (pme *Explorer) FindToolsRequiredForBuild(platform, buildPlatform *cores.PlatformRelease) ([]*cores.ToolRelease, error) { ... } +``` + +moreover it now requires the `platform` and the `buildPlatform` (a.k.a. the referenced platform core used for the +compile) instead of the `board`. Usually these two value are obtained from the `Explorer.ResolveFQBN(...)` method. + ## 0.29.0 ### Removed gRPC API: `cc.arduino.cli.commands.v1.UpdateCoreLibrariesIndex`, `Outdated`, and `Upgrade` diff --git a/docs/package_index_json-specification.md b/docs/package_index_json-specification.md index d37b4d6fd3e..7a4268ef490 100644 --- a/docs/package_index_json-specification.md +++ b/docs/package_index_json-specification.md @@ -278,11 +278,17 @@ are as follows: 1. the tool, version and packager specified via `toolsDependencies` in the `package_index.json` 1. the highest version of the tool provided by the packager of the current platform + 1. the highest version of the tool provided by the packager of the referenced platform used for compile (see + ["Referencing another core, variant or tool"](platform-specification.md#referencing-another-core-variant-or-tool) + for more info) 1. the highest version of the tool provided by any other packager (in case of tie, the first packager in alphabetical order wins) - The property `{runtime.tools.TOOLNAME-VERSION.path}` points, in order of priority, to: 1. the tool and version provided by the packager of the current platform + 1. the tool and version provided by the packager of the referenced platform used for compile (see + ["Referencing another core, variant or tool"](platform-specification.md#referencing-another-core-variant-or-tool) + for more info) 1. the tool and version provided by any other packager (in case of tie, the first packager in alphabetical order wins) ### Example JSON index file diff --git a/legacy/builder/target_board_resolver.go b/legacy/builder/target_board_resolver.go index fa5e34b7db6..59d1361b557 100644 --- a/legacy/builder/target_board_resolver.go +++ b/legacy/builder/target_board_resolver.go @@ -25,7 +25,7 @@ import ( type TargetBoardResolver struct{} func (s *TargetBoardResolver) Run(ctx *types.Context) error { - targetPackage, targetPlatform, targetBoard, buildProperties, actualPlatform, err := ctx.PackageManager.ResolveFQBN(ctx.FQBN) + targetPackage, targetPlatform, targetBoard, buildProperties, buildPlatform, err := ctx.PackageManager.ResolveFQBN(ctx.FQBN) if err != nil { return fmt.Errorf("%s: %w", tr("Error resolving FQBN"), err) } @@ -39,7 +39,7 @@ func (s *TargetBoardResolver) Run(ctx *types.Context) error { if ctx.Verbose { ctx.Info(tr("Using board '%[1]s' from platform in folder: %[2]s", targetBoard.BoardID, targetPlatform.InstallDir)) - ctx.Info(tr("Using core '%[1]s' from platform in folder: %[2]s", core, actualPlatform.InstallDir)) + ctx.Info(tr("Using core '%[1]s' from platform in folder: %[2]s", core, buildPlatform.InstallDir)) } if buildProperties.Get("build.board") == "" { @@ -50,7 +50,7 @@ func (s *TargetBoardResolver) Run(ctx *types.Context) error { targetBoard.String(), "'build.board'", defaultBuildBoard)) } - requiredTools, err := ctx.PackageManager.FindToolsRequiredForBoard(targetBoard) + requiredTools, err := ctx.PackageManager.FindToolsRequiredForBuild(targetPlatform, buildPlatform) if err != nil { return err } @@ -60,7 +60,7 @@ func (s *TargetBoardResolver) Run(ctx *types.Context) error { ctx.TargetBoardBuildProperties = buildProperties ctx.TargetPlatform = targetPlatform ctx.TargetPackage = targetPackage - ctx.ActualPlatform = actualPlatform + ctx.ActualPlatform = buildPlatform ctx.RequiredTools = requiredTools return nil } From adb168fddc238c5aad0907dd14691503ab00637b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 30 Jan 2023 16:47:42 +0100 Subject: [PATCH 5/6] Added some explanatory comments --- arduino/cores/packagemanager/package_manager_test.go | 2 +- internal/integrationtest/compile/compile_test.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arduino/cores/packagemanager/package_manager_test.go b/arduino/cores/packagemanager/package_manager_test.go index 6c35ace186f..ef94087c6fc 100644 --- a/arduino/cores/packagemanager/package_manager_test.go +++ b/arduino/cores/packagemanager/package_manager_test.go @@ -287,7 +287,7 @@ func TestFindToolsRequiredForBoard(t *testing.T) { loadIndex("https://dl.espressif.com/dl/package_esp32_index.json") loadIndex("http://arduino.esp8266.com/stable/package_esp8266com_index.json") loadIndex("https://adafruit.github.io/arduino-board-index/package_adafruit_index.json") - loadIndex("https://test.com/package_test_index.json") + loadIndex("https://test.com/package_test_index.json") // this is not downloaded, it just picks the "local cached" file package_test_index.json // We ignore the errors returned since they might not be necessarily blocking // but just warnings for the user, like in the case a board is not loaded diff --git a/internal/integrationtest/compile/compile_test.go b/internal/integrationtest/compile/compile_test.go index f020e7c4780..4ed80280c9a 100644 --- a/internal/integrationtest/compile/compile_test.go +++ b/internal/integrationtest/compile/compile_test.go @@ -43,7 +43,9 @@ func TestRuntimeToolPropertiesGeneration(t *testing.T) { sketch, err := paths.New("testdata", "bare_minimum").Abs() require.NoError(t, err) - // Multiple runs must always produce the same result + // As seen in https://github.com/arduino/arduino-cli/issues/73 the map randomess + // may make the function fail half of the times. Repeating the test 3 times + // greatly increases the chances to trigger the bad case. for i := 0; i < 3; i++ { stdout, _, err := cli.Run("compile", "-b", "alice:avr:alice", "--show-properties", sketch.String()) require.NoError(t, err) From 27a8394ba5d904839a9c2c82208a149bca5c362a Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 30 Jan 2023 16:48:40 +0100 Subject: [PATCH 6/6] Moved integration tests into 'compile_3' group --- internal/integrationtest/{compile => compile_3}/compile_test.go | 0 .../{compile => compile_3}/testdata/bare_minimum/bare_minimum.ino | 0 .../alice/hardware/avr/1.0.0/boards.txt | 0 .../alice/tools/avr-gcc/50.0.0/.keep | 0 .../alice/tools/avrdude/1.0.0/.keep | 0 .../bob/hardware/avr/1.0.0/boards.txt | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename internal/integrationtest/{compile => compile_3}/compile_test.go (100%) rename internal/integrationtest/{compile => compile_3}/testdata/bare_minimum/bare_minimum.ino (100%) rename internal/integrationtest/{compile => compile_3}/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt (100%) rename internal/integrationtest/{compile => compile_3}/testdata/platforms_with_conflicting_tools/alice/tools/avr-gcc/50.0.0/.keep (100%) rename internal/integrationtest/{compile => compile_3}/testdata/platforms_with_conflicting_tools/alice/tools/avrdude/1.0.0/.keep (100%) rename internal/integrationtest/{compile => compile_3}/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt (100%) diff --git a/internal/integrationtest/compile/compile_test.go b/internal/integrationtest/compile_3/compile_test.go similarity index 100% rename from internal/integrationtest/compile/compile_test.go rename to internal/integrationtest/compile_3/compile_test.go diff --git a/internal/integrationtest/compile/testdata/bare_minimum/bare_minimum.ino b/internal/integrationtest/compile_3/testdata/bare_minimum/bare_minimum.ino similarity index 100% rename from internal/integrationtest/compile/testdata/bare_minimum/bare_minimum.ino rename to internal/integrationtest/compile_3/testdata/bare_minimum/bare_minimum.ino diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt b/internal/integrationtest/compile_3/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt similarity index 100% rename from internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt rename to internal/integrationtest/compile_3/testdata/platforms_with_conflicting_tools/alice/hardware/avr/1.0.0/boards.txt diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avr-gcc/50.0.0/.keep b/internal/integrationtest/compile_3/testdata/platforms_with_conflicting_tools/alice/tools/avr-gcc/50.0.0/.keep similarity index 100% rename from internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avr-gcc/50.0.0/.keep rename to internal/integrationtest/compile_3/testdata/platforms_with_conflicting_tools/alice/tools/avr-gcc/50.0.0/.keep diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avrdude/1.0.0/.keep b/internal/integrationtest/compile_3/testdata/platforms_with_conflicting_tools/alice/tools/avrdude/1.0.0/.keep similarity index 100% rename from internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/alice/tools/avrdude/1.0.0/.keep rename to internal/integrationtest/compile_3/testdata/platforms_with_conflicting_tools/alice/tools/avrdude/1.0.0/.keep diff --git a/internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt b/internal/integrationtest/compile_3/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt similarity index 100% rename from internal/integrationtest/compile/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt rename to internal/integrationtest/compile_3/testdata/platforms_with_conflicting_tools/bob/hardware/avr/1.0.0/boards.txt