Drop support of package_bundled_index.json and builtin_tools_versions.txt (#2424)

Author: alessio-perugini

Diff for: commands/compile/compile.go

@@ -187,7 +187,6 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
 		int(req.GetJobs()),
 		req.GetBuildProperties(),
 		configuration.HardwareDirectories(configuration.Settings),
-		configuration.BuiltinToolsDirectories(configuration.Settings),
 		otherLibrariesDirs,
 		configuration.IDEBuiltinLibrariesDir(configuration.Settings),
 		fqbn,

Diff for: docs/UPGRADING.md

@@ -4,6 +4,10 @@ Here you can find a list of migration guides to handle breaking changes between
 
 ## 0.36.0
 
+### Drop support for `builtin.tools`
+
+We're dropping the `builtin.tools` support. It was the equivalent of Arduino IDE 1.x bundled tools directory.
+
 ### Some golang modules from `github.com/arduino/arduino-cli/*` have been made private.
 
 The following golang modules are no longer available as public API:

Diff for: docs/configuration.md

@@ -12,8 +12,6 @@
   - `builtin.libraries` - the libraries in this directory will be available to all platforms without the need for the
     user to install them, but with the lowest priority over other installed libraries with the same name, it's the
     equivalent of the Arduino IDE's bundled libraries directory.
-  - `builtin.tools` - it's a list of directories of tools that will be available to all platforms without the need for
-    the user to install them, it's the equivalent of the Arduino IDE 1.x bundled tools directory.
 - `library` - configuration options relating to Arduino libraries.
   - `enable_unsafe_install` - set to `true` to enable the use of the `--git-url` and `--zip-file` flags with
     [`arduino-cli lib install`][arduino cli lib install]. These are considered "unsafe" installation methods because

Diff for: internal/arduino/builder/build_options_manager.go

@@ -33,7 +33,6 @@ type buildOptions struct {
 	currentOptions *properties.Map
 
 	hardwareDirs              paths.PathList
-	builtInToolsDirs          paths.PathList
 	otherLibrariesDirs        paths.PathList
 	builtInLibrariesDirs      *paths.Path
 	buildPath                 *paths.Path
@@ -47,7 +46,7 @@ type buildOptions struct {
 
 // newBuildOptions fixdoc
 func newBuildOptions(
-	hardwareDirs, builtInToolsDirs, otherLibrariesDirs paths.PathList,
+	hardwareDirs, otherLibrariesDirs paths.PathList,
 	builtInLibrariesDirs, buildPath *paths.Path,
 	sketch *sketch.Sketch,
 	customBuildProperties []string,
@@ -59,7 +58,6 @@ func newBuildOptions(
 	opts := properties.NewMap()
 
 	opts.Set("hardwareFolders", strings.Join(hardwareDirs.AsStrings(), ","))
-	opts.Set("builtInToolsFolders", strings.Join(builtInToolsDirs.AsStrings(), ","))
 	opts.Set("otherLibrariesFolders", strings.Join(otherLibrariesDirs.AsStrings(), ","))
 	opts.SetPath("sketchLocation", sketch.FullPath)
 	opts.Set("fqbn", fqbn.String())
@@ -84,7 +82,6 @@ func newBuildOptions(
 	return &buildOptions{
 		currentOptions:            opts,
 		hardwareDirs:              hardwareDirs,
-		builtInToolsDirs:          builtInToolsDirs,
 		otherLibrariesDirs:        otherLibrariesDirs,
 		builtInLibrariesDirs:      builtInLibrariesDirs,
 		buildPath:                 buildPath,

Diff for: internal/arduino/builder/builder.go

@@ -121,7 +121,7 @@ func NewBuilder(
 	coreBuildCachePath *paths.Path,
 	jobs int,
 	requestBuildProperties []string,
-	hardwareDirs, builtInToolsDirs, otherLibrariesDirs paths.PathList,
+	hardwareDirs, otherLibrariesDirs paths.PathList,
 	builtInLibrariesDirs *paths.Path,
 	fqbn *cores.FQBN,
 	clean bool,
@@ -223,7 +223,7 @@ func NewBuilder(
 			logger,
 		),
 		buildOptions: newBuildOptions(
-			hardwareDirs, builtInToolsDirs, otherLibrariesDirs,
+			hardwareDirs, otherLibrariesDirs,
 			builtInLibrariesDirs, buildPath,
 			sk,
 			customBuildPropertiesArgs,

Diff for: internal/arduino/cores/cores.go

@@ -70,7 +70,6 @@ type PlatformRelease struct {
 	Programmers             map[string]*Programmer        `json:"-"`
 	Menus                   *properties.Map               `json:"-"`
 	InstallDir              *paths.Path                   `json:"-"`
-	IsIDEBundled            bool                          `json:"-"`
 	IsTrusted               bool                          `json:"-"`
 	PluggableDiscoveryAware bool                          `json:"-"` // true if the Platform supports pluggable discovery (no compatibility layer required)
 	Monitors                map[string]*MonitorDependency `json:"-"`

Diff for: internal/arduino/cores/packagemanager/loader.go

@@ -18,8 +18,6 @@ package packagemanager
 import (
 	"errors"
 	"fmt"
-	"os"
-	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -36,12 +34,7 @@ import (
 // LoadHardware read all plaforms from the configured paths
 func (pm *Builder) LoadHardware() []error {
 	hardwareDirs := configuration.HardwareDirectories(configuration.Settings)
-	merr := pm.LoadHardwareFromDirectories(hardwareDirs)
-
-	bundleToolDirs := configuration.BuiltinToolsDirectories(configuration.Settings)
-	merr = append(merr, pm.LoadToolsFromBundleDirectories(bundleToolDirs)...)
-
-	return merr
+	return pm.LoadHardwareFromDirectories(hardwareDirs)
 }
 
 // LoadHardwareFromDirectories load plaforms from a set of directories
@@ -203,49 +196,9 @@ func (pm *Builder) loadPlatform(targetPackage *cores.Package, architecture strin
 			return &cmderrors.InvalidVersionError{Cause: fmt.Errorf("%s: %s", platformTxtPath, err)}
 		}
 
-		// Check if package_bundled_index.json exists.
-		// This is used indirectly by the Java IDE since it's necessary for the arduino-builder
-		// to find cores bundled with that version of the IDE.
-		// TODO: This piece of logic MUST be removed as soon as the Java IDE stops using the arduino-builder.
-		isIDEBundled := false
-		packageBundledIndexPath := platformPath.Parent().Parent().Join("package_index_bundled.json")
-		if packageBundledIndexPath.Exist() {
-			// particular case: ARCHITECTURE/boards.txt with package_bundled_index.json
-
-			// this is an unversioned Platform with a package_index_bundled.json that
-			// gives information about the version and tools needed
-
-			// Parse the bundled index and merge to the general index
-			index, err := pm.LoadPackageIndexFromFile(packageBundledIndexPath)
-			if err != nil {
-				return fmt.Errorf("%s: %w", tr("parsing IDE bundled index"), err)
-			}
-
-			// Now export the bundled index in a temporary core.Packages to retrieve the bundled package version
-			tmp := cores.NewPackages()
-			index.MergeIntoPackages(tmp)
-			if tmpPackage := tmp.GetOrCreatePackage(targetPackage.Name); tmpPackage == nil {
-				pm.log.Warnf("Can't determine bundle platform version for %s", targetPackage.Name)
-			} else if tmpPlatform := tmpPackage.GetOrCreatePlatform(architecture); tmpPlatform == nil {
-				pm.log.Warnf("Can't determine bundle platform version for %s:%s", targetPackage.Name, architecture)
-			} else if tmpPlatformRelease := tmpPlatform.GetLatestRelease(); tmpPlatformRelease == nil {
-				pm.log.Warnf("Can't determine bundle platform version for %s:%s, no valid release found", targetPackage.Name, architecture)
-			} else {
-				version = tmpPlatformRelease.Version
-			}
-
-			isIDEBundled = true
-		}
-
 		platform := targetPackage.GetOrCreatePlatform(architecture)
-		if !isIDEBundled {
-			platform.ManuallyInstalled = true
-		}
+		platform.ManuallyInstalled = true
 		release := platform.GetOrCreateRelease(version)
-		release.IsIDEBundled = isIDEBundled
-		if isIDEBundled {
-			pm.log.Infof("Package is built-in")
-		}
 		if err := pm.loadPlatformRelease(release, platformPath); err != nil {
 			return fmt.Errorf("%s: %w", tr("loading platform release %s", release), err)
 		}
@@ -658,83 +611,6 @@ func (pm *Builder) loadToolReleaseFromDirectory(tool *cores.Tool, version *semve
 	}
 }
 
-// LoadToolsFromBundleDirectories FIXMEDOC
-func (pm *Builder) LoadToolsFromBundleDirectories(dirs paths.PathList) []error {
-	var merr []error
-	for _, dir := range dirs {
-		if err := pm.LoadToolsFromBundleDirectory(dir); err != nil {
-			merr = append(merr, fmt.Errorf("%s: %w", tr("loading bundled tools from %s", dir), err))
-		}
-	}
-	return merr
-}
-
-// LoadToolsFromBundleDirectory FIXMEDOC
-func (pm *Builder) LoadToolsFromBundleDirectory(toolsPath *paths.Path) error {
-	pm.log.Infof("Loading tools from bundle dir: %s", toolsPath)
-
-	// We scan toolsPath content to find a "builtin_tools_versions.txt", if such file exists
-	// then the all the tools are available in the same directory, mixed together, and their
-	// name and version are written in the "builtin_tools_versions.txt" file.
-	// If no "builtin_tools_versions.txt" is found, then the directory structure is the classic
-	// TOOLSPATH/TOOL-NAME/TOOL-VERSION and it will be parsed as such and associated to an
-	// "unnamed" packager.
-
-	// TODO: get rid of "builtin_tools_versions.txt"
-
-	// Search for builtin_tools_versions.txt
-	builtinToolsVersionsTxtPath := ""
-	findBuiltInToolsVersionsTxt := func(currentPath string, info os.FileInfo, err error) error {
-		if err != nil {
-			// Ignore errors
-			return nil
-		}
-		if builtinToolsVersionsTxtPath != "" {
-			return filepath.SkipDir
-		}
-		if info.Name() == "builtin_tools_versions.txt" {
-			builtinToolsVersionsTxtPath = currentPath
-			return filepath.SkipDir
-		}
-		return nil
-	}
-	if err := filepath.Walk(toolsPath.String(), findBuiltInToolsVersionsTxt); err != nil {
-		return fmt.Errorf(tr("searching for builtin_tools_versions.txt in %[1]s: %[2]s"), toolsPath, err)
-	}
-
-	if builtinToolsVersionsTxtPath != "" {
-		// If builtin_tools_versions.txt is found create tools based on the info
-		// contained in that file
-		pm.log.Infof("Found builtin_tools_versions.txt")
-		toolPath, err := paths.New(builtinToolsVersionsTxtPath).Parent().Abs()
-		if err != nil {
-			return fmt.Errorf(tr("getting parent dir of %[1]s: %[2]s"), builtinToolsVersionsTxtPath, err)
-		}
-
-		all, err := properties.Load(builtinToolsVersionsTxtPath)
-		if err != nil {
-			return fmt.Errorf(tr("reading %[1]s: %[2]s"), builtinToolsVersionsTxtPath, err)
-		}
-
-		for packager, toolsData := range all.FirstLevelOf() {
-			targetPackage := pm.packages.GetOrCreatePackage(packager)
-
-			for toolName, toolVersion := range toolsData.AsMap() {
-				tool := targetPackage.GetOrCreateTool(toolName)
-				version := semver.ParseRelaxed(toolVersion)
-				release := tool.GetOrCreateRelease(version)
-				release.InstallDir = toolPath
-				pm.log.WithField("tool", release).Infof("Loaded tool")
-			}
-		}
-	} else {
-		// otherwise load the tools inside the unnamed package
-		unnamedPackage := pm.packages.GetOrCreatePackage("")
-		pm.LoadToolsFromPackageDir(unnamedPackage, toolsPath)
-	}
-	return nil
-}
-
 // LoadDiscoveries load all discoveries for all loaded platforms
 // Returns error if:
 // * A PluggableDiscovery instance can't be created

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

@@ -621,8 +621,7 @@ func (pme *Explorer) GetInstalledPlatformRelease(platform *cores.Platform) *core
 	}
 
 	debug := func(msg string, pl *cores.PlatformRelease) {
-		pme.log.WithField("bundle", pl.IsIDEBundled).
-			WithField("version", pl.Version).
+		pme.log.WithField("version", pl.Version).
 			WithField("managed", pme.IsManagedPlatformRelease(pl)).
 			Debugf("%s: %s", msg, pl)
 	}
@@ -634,20 +633,20 @@ func (pme *Explorer) GetInstalledPlatformRelease(platform *cores.Platform) *core
 	for _, candidate := range releases[1:] {
 		candidateIsManaged := pme.IsManagedPlatformRelease(candidate)
 		debug("candidate", candidate)
-		// TODO: Disentangle this algorithm and make it more straightforward
-		if bestIsManaged == candidateIsManaged {
-			if best.IsIDEBundled == candidate.IsIDEBundled {
-				if candidate.Version.GreaterThan(best.Version) {
-					best = candidate
-				}
-			}
-			if best.IsIDEBundled && !candidate.IsIDEBundled {
+		if !candidateIsManaged && !bestIsManaged {
+			if candidate.Version.GreaterThan(best.Version) {
 				best = candidate
 			}
+			continue
 		}
-		if !bestIsManaged && candidateIsManaged {
+		if !candidateIsManaged {
+			continue
+		}
+		if !bestIsManaged {
 			best = candidate
 			bestIsManaged = true
+		} else if candidate.Version.GreaterThan(best.Version) {
+			best = candidate
 		}
 		debug("current best", best)
 	}

Diff for: internal/cli/configuration/directories.go

@@ -42,11 +42,6 @@ func HardwareDirectories(settings *viper.Viper) paths.PathList {
 	return res
 }
 
-// BuiltinToolsDirectories returns all paths that may contains bundled-tools.
-func BuiltinToolsDirectories(settings *viper.Viper) paths.PathList {
-	return paths.NewPathList(settings.GetStringSlice("directories.builtin.Tools")...)
-}
-
 // IDEBuiltinLibrariesDir returns the IDE-bundled libraries path. Usually
 // this directory is present in the Arduino IDE.
 func IDEBuiltinLibrariesDir(settings *viper.Viper) *paths.Path {

Diff for: internal/integrationtest/compile_4/compile_test.go

@@ -1030,7 +1030,6 @@ func TestBuildOptionsFile(t *testing.T) {
 
 	requirejson.Query(t, buildOptionsBytes, "keys", `[
 		"additionalFiles",
-		"builtInToolsFolders",
 		"compiler.optimization_flags",
 		"customBuildProperties",
 		"fqbn",

Diff for: internal/integrationtest/core/core_test.go

@@ -20,6 +20,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"os"
+	"path/filepath"
 	"runtime"
 	"sort"
 	"strconv"
@@ -518,6 +519,60 @@ func TestCoreListAllManuallyInstalledCore(t *testing.T) {
 	]}`)
 }
 
+func TestCoreListShowsLatestVersionWhenMultipleReleasesOfAManuallyInstalledCoreArePresent(t *testing.T) {
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	_, _, err := cli.Run("core", "update-index")
+	require.NoError(t, err)
+
+	// Verifies only cores in board manager are shown
+	stdout, _, err := cli.Run("core", "list", "--all", "--format", "json")
+	require.NoError(t, err)
+	requirejson.Query(t, stdout, `.platforms | length > 0`, `true`)
+	length, err := strconv.Atoi(requirejson.Parse(t, stdout).Query(".platforms | length").String())
+	require.NoError(t, err)
+
+	// Manually installs a core in sketchbooks hardware folder
+	gitUrl := "https://github.com/arduino/ArduinoCore-avr.git"
+	repoDir := cli.SketchbookDir().Join("hardware", "arduino-beta-development", "avr")
+	_, err = git.PlainClone(filepath.Join(repoDir.String(), "1.8.3"), false, &git.CloneOptions{
+		URL:           gitUrl,
+		ReferenceName: plumbing.NewTagReferenceName("1.8.3"),
+	})
+	require.NoError(t, err)
+
+	tmp := paths.New(t.TempDir(), "1.8.4")
+	_, err = git.PlainClone(tmp.String(), false, &git.CloneOptions{
+		URL:           gitUrl,
+		ReferenceName: plumbing.NewTagReferenceName("1.8.4"),
+	})
+	require.NoError(t, err)
+
+	err = tmp.Rename(repoDir.Join("1.8.4"))
+	require.NoError(t, err)
+
+	// When manually installing 2 releases of the same core, the newest one takes precedence
+	stdout, _, err = cli.Run("core", "list", "--all", "--format", "json")
+	require.NoError(t, err)
+	requirejson.Query(t, stdout, `.platforms | length`, fmt.Sprint(length+1))
+	requirejson.Contains(t, stdout, `{"platforms":[
+		{
+			"id": "arduino-beta-development:avr",
+			"latest_version": "1.8.4",
+			"installed_version": "1.8.4",
+			"releases": {
+				"1.8.3": {
+					"name": "Arduino AVR Boards"
+				},
+				"1.8.3": {
+					"name": "Arduino AVR Boards"
+				}
+			}
+		}
+	]}`)
+}
+
 func TestCoreListUpdatableAllFlags(t *testing.T) {
 	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
 	defer env.CleanUp()