Fix library priority selection on some convoluted cases (#565)

* Added test for new use cases

* Moved PriorityForArchitecture in librariesresolver

The method is used only in library resolver, there is not point in
having it as method of Library.

Removed also the unused SortByArchitecturePriority method.

* Fixed library priority selection

* Added another fix and test for lib priority

Author: cmaglie

arduino/libraries/libraries.go

@@ -16,8 +16,6 @@
 package libraries
 
 import (
-	"fmt"
-
 	"github.com/arduino/arduino-cli/arduino/cores"
 	paths "github.com/arduino/go-paths-helper"
 	properties "github.com/arduino/go-properties-orderedmap"
@@ -118,33 +116,6 @@ func (library *Library) IsArchitectureIndependent() bool {
 	return library.IsOptimizedForArchitecture("*")
 }
 
-// PriorityForArchitecture returns an integer that represents the
-// priority this lib has for the specified architecture based on
-// his location and the architectures directly supported (as exposed
-// on the `architecture` field of the `library.properties`)
-// This function returns an integer between 0 and 255, higher means
-// higher priority.
-func (library *Library) PriorityForArchitecture(arch string) uint8 {
-	bonus := uint8(0)
-
-	// Bonus for core-optimized libraries
-	if library.IsOptimizedForArchitecture(arch) {
-		bonus = 0x10
-	}
-
-	switch library.Location {
-	case IDEBuiltIn:
-		return bonus + 0x00
-	case ReferencedPlatformBuiltIn:
-		return bonus + 0x01
-	case PlatformBuiltIn:
-		return bonus + 0x02
-	case User:
-		return bonus + 0x03
-	}
-	panic(fmt.Sprintf("Invalid library location: %d", library.Location))
-}
-
 // SourceDir represents a source dir of a library
 type SourceDir struct {
 	Dir     *paths.Path

arduino/libraries/librarieslist.go

@@ -50,15 +50,6 @@ func (list *List) FindByName(name string) *Library {
 	return nil
 }
 
-// SortByArchitecturePriority sorts the libraries in descending order using
-// the Arduino lib priority ordering (the first has the higher priority)
-func (list *List) SortByArchitecturePriority(arch string) {
-	sort.Slice(*list, func(i, j int) bool {
-		a, b := (*list)[i], (*list)[j]
-		return a.PriorityForArchitecture(arch) > b.PriorityForArchitecture(arch)
-	})
-}
-
 // SortByName sorts the libraries by name
 func (list *List) SortByName() {
 	sort.Slice(*list, func(i, j int) bool {

arduino/libraries/librariesresolver/cpp.go

@@ -124,17 +124,36 @@ func computePriority(lib *libraries.Library, header, arch string) int {
 	header = simplify(header)
 	name := simplify(lib.Name)
 
-	priority := int(lib.PriorityForArchitecture(arch)) // between 0..255
+	priority := 0
+
+	// Bonus for core-optimized libraries
+	if lib.IsOptimizedForArchitecture(arch) || lib.IsArchitectureIndependent() {
+		priority += 0x0100
+	}
+
+	switch lib.Location {
+	case libraries.IDEBuiltIn:
+		priority += 0x0000
+	case libraries.ReferencedPlatformBuiltIn:
+		priority += 0x0001
+	case libraries.PlatformBuiltIn:
+		priority += 0x0002
+	case libraries.User:
+		priority += 0x0003
+	default:
+		panic(fmt.Sprintf("Invalid library location: %d", lib.Location))
+	}
+
 	if name == header {
-		priority += 0x500
+		priority += 0x0050
 	} else if name == header+"-master" {
-		priority += 0x400
+		priority += 0x0040
 	} else if strings.HasPrefix(name, header) {
-		priority += 0x300
+		priority += 0x0030
 	} else if strings.HasSuffix(name, header) {
-		priority += 0x200
+		priority += 0x0020
 	} else if strings.Contains(name, header) {
-		priority += 0x100
+		priority += 0x0010
 	}
 	return priority
 }

arduino/libraries/librariesresolver/cpp_test.go

@@ -29,6 +29,41 @@ var l4 = &libraries.Library{Name: "Another Calculus Lib", Location: libraries.Us
 var l5 = &libraries.Library{Name: "Yet Another Calculus Lib Improved", Location: libraries.User}
 var l6 = &libraries.Library{Name: "Calculus Unified Lib", Location: libraries.User}
 var l7 = &libraries.Library{Name: "AnotherLib", Location: libraries.User}
+var bundleServo = &libraries.Library{Name: "Servo", Location: libraries.IDEBuiltIn, Architectures: []string{"avr", "sam", "samd"}}
+var userServo = &libraries.Library{Name: "Servo", Location: libraries.User, Architectures: []string{"avr", "sam", "samd"}}
+var userServoAllArch = &libraries.Library{Name: "Servo", Location: libraries.User, Architectures: []string{"*"}}
+var userServoNonavr = &libraries.Library{Name: "Servo", Location: libraries.User, Architectures: []string{"sam", "samd"}}
+var userAnotherServo = &libraries.Library{Name: "AnotherServo", Location: libraries.User, Architectures: []string{"avr", "sam", "samd", "esp32"}}
+
+func runResolver(include string, arch string, libs ...*libraries.Library) *libraries.Library {
+	libraryList := libraries.List{}
+	libraryList.Add(libs...)
+	resolver := NewCppResolver()
+	resolver.headers[include] = libraryList
+	return resolver.ResolveFor(include, arch)
+}
+
+func TestArchitecturePriority(t *testing.T) {
+	res := runResolver("Servo.h", "avr", bundleServo, userServo)
+	require.NotNil(t, res)
+	require.Equal(t, userServo, res, "selected library")
+
+	res = runResolver("Servo.h", "avr", bundleServo, userServoNonavr)
+	require.NotNil(t, res)
+	require.Equal(t, bundleServo, res, "selected library")
+
+	res = runResolver("Servo.h", "avr", bundleServo, userAnotherServo)
+	require.NotNil(t, res)
+	require.Equal(t, bundleServo, res, "selected library")
+
+	res = runResolver("Servo.h", "esp32", bundleServo, userAnotherServo)
+	require.NotNil(t, res)
+	require.Equal(t, userAnotherServo, res, "selected library")
+
+	res = runResolver("Servo.h", "esp32", userServoAllArch, userAnotherServo)
+	require.NotNil(t, res)
+	require.Equal(t, userServoAllArch, res, "selected library")
+}
 
 func TestClosestMatchWithTotallyDifferentNames(t *testing.T) {
 	libraryList := libraries.List{}