[breaking] Fix: lib list --fqbn and lib examples --fqbn do not show platform bundled lib when lib of same name is installed globally (#2113)

* Added test

* Factored function to determine library compatibility

* Made ComputePriority function public

* fix: use the libraries resolution algorithm to determine library priority

* Slightly refactored 'lib list' command call

* Updated UPGRADING.md

* Added test for a similar bug in `lib examples`

See #1656

Author: cmaglie

arduino/libraries/libraries.go

@@ -181,6 +181,13 @@ func (library *Library) IsArchitectureIndependent() bool {
 	return library.IsOptimizedForArchitecture("*") || library.Architectures == nil || len(library.Architectures) == 0
 }
 
+// IsCompatibleWith returns true if the library declares compatibility with
+// the given architecture. If this function returns false, the library may still
+// be compatible with the given architecture, but it's not explicitly declared.
+func (library *Library) IsCompatibleWith(arch string) bool {
+	return library.IsArchitectureIndependent() || library.IsOptimizedForArchitecture(arch)
+}
+
 // SourceDir represents a source dir of a library
 type SourceDir struct {
 	Dir     *paths.Path
@@ -205,21 +212,6 @@ func (library *Library) SourceDirs() []SourceDir {
 	return dirs
 }
 
-// LocationPriorityFor returns a number representing the location priority for the given library
-// using the given platform and referenced-platform. Higher value means higher priority.
-func (library *Library) LocationPriorityFor(platformRelease, refPlatformRelease *cores.PlatformRelease) int {
-	if library.Location == IDEBuiltIn {
-		return 1
-	} else if library.ContainerPlatform == refPlatformRelease {
-		return 2
-	} else if library.ContainerPlatform == platformRelease {
-		return 3
-	} else if library.Location == User {
-		return 4
-	}
-	return 0
-}
-
 // DeclaredHeaders returns the C++ headers that the library declares in library.properties
 func (library *Library) DeclaredHeaders() []string {
 	if library.declaredHeaders == nil {

arduino/libraries/librariesresolver/cpp.go

@@ -123,7 +123,7 @@ func (resolver *Cpp) ResolveFor(header, architecture string) *libraries.Library
 	var found libraries.List
 	var foundPriority int
 	for _, lib := range resolver.headers[header] {
-		libPriority := computePriority(lib, header, architecture)
+		libPriority := ComputePriority(lib, header, architecture)
 		msg := "  discarded"
 		if found == nil || foundPriority < libPriority {
 			found = libraries.List{}
@@ -164,7 +164,10 @@ func simplify(name string) string {
 	return name
 }
 
-func computePriority(lib *libraries.Library, header, arch string) int {
+// ComputePriority returns an integer value representing the priority of the library
+// for the specified header and architecture. The higher the value, the higher the
+// priority.
+func ComputePriority(lib *libraries.Library, header, arch string) int {
 	header = strings.TrimSuffix(header, filepath.Ext(header))
 	header = simplify(header)
 	name := simplify(lib.Name)

arduino/libraries/librariesresolver/cpp_test.go

@@ -102,13 +102,13 @@ func TestClosestMatchWithTotallyDifferentNames(t *testing.T) {
 }
 
 func TestCppHeaderPriority(t *testing.T) {
-	r1 := computePriority(l1, "calculus_lib.h", "avr")
-	r2 := computePriority(l2, "calculus_lib.h", "avr")
-	r3 := computePriority(l3, "calculus_lib.h", "avr")
-	r4 := computePriority(l4, "calculus_lib.h", "avr")
-	r5 := computePriority(l5, "calculus_lib.h", "avr")
-	r6 := computePriority(l6, "calculus_lib.h", "avr")
-	r7 := computePriority(l7, "calculus_lib.h", "avr")
+	r1 := ComputePriority(l1, "calculus_lib.h", "avr")
+	r2 := ComputePriority(l2, "calculus_lib.h", "avr")
+	r3 := ComputePriority(l3, "calculus_lib.h", "avr")
+	r4 := ComputePriority(l4, "calculus_lib.h", "avr")
+	r5 := ComputePriority(l5, "calculus_lib.h", "avr")
+	r6 := ComputePriority(l6, "calculus_lib.h", "avr")
+	r7 := ComputePriority(l7, "calculus_lib.h", "avr")
 	require.True(t, r1 > r2)
 	require.True(t, r2 > r3)
 	require.True(t, r3 > r4)

commands/lib/list.go

@@ -24,6 +24,7 @@ import (
 	"github.com/arduino/arduino-cli/arduino/libraries"
 	"github.com/arduino/arduino-cli/arduino/libraries/librariesindex"
 	"github.com/arduino/arduino-cli/arduino/libraries/librariesmanager"
+	"github.com/arduino/arduino-cli/arduino/libraries/librariesresolver"
 	"github.com/arduino/arduino-cli/commands"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 )
@@ -49,7 +50,7 @@ func LibraryList(ctx context.Context, req *rpc.LibraryListRequest) (*rpc.Library
 	nameFilter := strings.ToLower(req.GetName())
 
 	var allLibs []*installedLib
-	if f := req.GetFqbn(); f != "" {
+	if fqbnString := req.GetFqbn(); fqbnString != "" {
 		allLibs = listLibraries(lm, req.GetUpdatable(), true)
 		fqbn, err := cores.ParseFQBN(req.GetFqbn())
 		if err != nil {
@@ -69,22 +70,17 @@ func LibraryList(ctx context.Context, req *rpc.LibraryListRequest) (*rpc.Library
 				}
 			}
 			if latest, has := filteredRes[lib.Library.Name]; has {
-				if latest.Library.LocationPriorityFor(boardPlatform, refBoardPlatform) >= lib.Library.LocationPriorityFor(boardPlatform, refBoardPlatform) {
+				latestPriority := librariesresolver.ComputePriority(latest.Library, "", fqbn.PlatformArch)
+				libPriority := librariesresolver.ComputePriority(lib.Library, "", fqbn.PlatformArch)
+				if latestPriority >= libPriority {
 					// Pick library with the best priority
 					continue
 				}
 			}
 
 			// Check if library is compatible with board specified by FBQN
-			compatible := false
-			for _, arch := range lib.Library.Architectures {
-				compatible = (arch == fqbn.PlatformArch || arch == "*")
-				if compatible {
-					break
-				}
-			}
 			lib.Library.CompatibleWith = map[string]bool{
-				f: compatible,
+				fqbnString: lib.Library.IsCompatibleWith(fqbn.PlatformArch),
 			}
 
 			filteredRes[lib.Library.Name] = lib

docs/UPGRADING.md

@@ -4,6 +4,8 @@ Here you can find a list of migration guides to handle breaking changes between
 
 ## 0.32.0
 
+### `arduino-cli` doesn't lookup anymore in the current directory for configuration file.
+
 Configuration file lookup in current working directory and its parents is dropped. The command line flag `--config-file`
 must be specified to use an alternative configuration file from the one in the data directory.
 
@@ -49,6 +51,10 @@ $ arduino-cli outdated --format json
 }
 ```
 
+### golang API: method `github.com/arduino/arduino-cli/arduino/libraries/Library.LocationPriorityFor` removed
+
+That method was outdated and must not be used.
+
 ## 0.31.0
 
 ### Added `post_install` script support for tools

internal/cli/lib/list.go

@@ -45,7 +45,9 @@ not listed, they can be listed by adding the --all flag.`),
 		Example: "  " + os.Args[0] + " lib list",
 		Args:    cobra.MaximumNArgs(1),
 		Run: func(cmd *cobra.Command, args []string) {
-			runListCommand(args, all, updatable)
+			instance := instance.CreateAndInit()
+			logrus.Info("Executing `arduino-cli lib list`")
+			List(instance, args, all, updatable)
 		},
 	}
 	listCommand.Flags().BoolVar(&all, "all", false, tr("Include built-in libraries (from platforms and IDE) in listing."))
@@ -54,12 +56,6 @@ not listed, they can be listed by adding the --all flag.`),
 	return listCommand
 }
 
-func runListCommand(args []string, all bool, updatable bool) {
-	instance := instance.CreateAndInit()
-	logrus.Info("Executing `arduino-cli lib list`")
-	List(instance, args, all, updatable)
-}
-
 // List gets and prints a list of installed libraries.
 func List(instance *rpc.Instance, args []string, all bool, updatable bool) {
 	installedLibs := GetList(instance, args, all, updatable)

internal/integrationtest/lib/lib_test.go

@@ -1547,3 +1547,43 @@ func TestLibQueryParameters(t *testing.T) {
 	require.Contains(t, string(stdout),
 		"Starting download                             \x1b[36murl\x1b[0m=\"https://downloads.arduino.cc/libraries/github.com/firmata/Firmata-2.5.9.zip?query=upgrade-builtin\"\n")
 }
+
+func TestLibBundlesWhenLibWithTheSameNameIsInstalledGlobally(t *testing.T) {
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	// See: https://github.com/arduino/arduino-cli/issues/1566
+	_, _, err := cli.Run("core", "install", "arduino:[email protected]")
+	require.NoError(t, err)
+	{
+		stdout, _, err := cli.Run("lib", "list", "--all", "--fqbn", "arduino:samd:mkrzero", "USBHost", "--format", "json")
+		require.NoError(t, err)
+		j := requirejson.Parse(t, stdout)
+		j.Query(`.[0].library.name`).MustEqual(`"USBHost"`)
+		j.Query(`.[0].library.compatible_with."arduino:samd:mkrzero"`).MustEqual(`true`)
+	}
+	_, _, err = cli.Run("lib", "install", "[email protected]")
+	require.NoError(t, err)
+	{
+		// Check that the architecture-specific library is still listed
+		stdout, _, err := cli.Run("lib", "list", "--all", "--fqbn", "arduino:samd:mkrzero", "USBHost", "--format", "json")
+		require.NoError(t, err)
+		j := requirejson.Parse(t, stdout)
+		j.Query(`.[0].library.name`).MustEqual(`"USBHost"`)
+		j.Query(`.[0].library.compatible_with."arduino:samd:mkrzero"`).MustEqual(`true`)
+	}
+
+	// See: https://github.com/arduino/arduino-cli/issues/1656
+	{
+		_, _, err = cli.Run("core", "update-index", "--additional-urls", "https://arduino.esp8266.com/stable/package_esp8266com_index.json")
+		require.NoError(t, err)
+		_, _, err = cli.Run("core", "install", "--additional-urls", "https://arduino.esp8266.com/stable/package_esp8266com_index.json", "esp8266:[email protected]")
+		require.NoError(t, err)
+		_, _, err = cli.Run("lib", "install", "[email protected]")
+		require.NoError(t, err)
+		stdout, _, err := cli.Run("lib", "examples", "--fqbn", "esp8266:esp8266:generic", "ArduinoOTA", "--format", "json")
+		require.NoError(t, err)
+		requirejson.Parse(t, stdout).Query(`.[].library.examples[0]`).MustContain(`"BasicOTA"`)
+		requirejson.Parse(t, stdout).Query(`.[].library.examples[1]`).MustContain(`"OTALeds"`)
+	}
+}