From f369bbd96d0dfd1c08dee97b89465daa61343e33 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 20 Apr 2023 17:47:16 +0200 Subject: [PATCH 1/4] Added FQBNMatcher class --- arduino/cores/fqbn.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/arduino/cores/fqbn.go b/arduino/cores/fqbn.go index 35f48c2207e..a6159c600c6 100644 --- a/arduino/cores/fqbn.go +++ b/arduino/cores/fqbn.go @@ -110,3 +110,40 @@ func (fqbn *FQBN) Match(target *FQBN) bool { func (fqbn *FQBN) StringWithoutConfig() string { return fqbn.Package + ":" + fqbn.PlatformArch + ":" + fqbn.BoardID } + +// FQBNMatcher contains a pattern to match an FQBN +type FQBNMatcher struct { + Package string + PlatformArch string + BoardID string +} + +// ParseFQBNMatcher parse a formula for an FQBN pattern and returns the corresponding +// FQBNMatcher. In the formula is allowed the glob char `*`. The formula must contains +// the triple `PACKAGE:ARCHITECTURE:BOARDID`, some exaples are: +// - `arduino:avr:uno` +// - `*:avr:*` +// - `arduino:avr:mega*` +func ParseFQBNMatcher(formula string) (*FQBNMatcher, error) { + parts := strings.Split(strings.TrimSpace(formula), ":") + if len(parts) < 3 || len(parts) > 4 { + return nil, fmt.Errorf("invalid formula: %s", formula) + } + return &FQBNMatcher{ + Package: parts[0], + PlatformArch: parts[1], + BoardID: parts[2], + }, nil +} + +// Match checks if this FQBNMatcher matches the given fqbn +func (m *FQBNMatcher) Match(fqbn *FQBN) bool { + // TODO: allow in-fix syntax like `*name` + return (m.Package == fqbn.Package || m.Package == "*") && + (m.PlatformArch == fqbn.PlatformArch || m.PlatformArch == "*") && + (m.BoardID == fqbn.BoardID || m.BoardID == "*") +} + +func (m *FQBNMatcher) String() string { + return m.Package + "." + m.PlatformArch + ":" + m.BoardID +} From 74977ee955afb8d5db592fa3bd787990aa0b2384 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 20 Apr 2023 17:48:23 +0200 Subject: [PATCH 2/4] Added SupportedFQBN field in Library and updated lib loader --- arduino/libraries/libraries.go | 1 + arduino/libraries/loader.go | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/arduino/libraries/libraries.go b/arduino/libraries/libraries.go index 186142360c6..81b2d86fe49 100644 --- a/arduino/libraries/libraries.go +++ b/arduino/libraries/libraries.go @@ -59,6 +59,7 @@ type Library struct { Website string Category string Architectures []string + SupportedFQBN []*cores.FQBNMatcher Types []string `json:"types,omitempty"` diff --git a/arduino/libraries/loader.go b/arduino/libraries/loader.go index ec3d1dc164a..49fb0e8c5c2 100644 --- a/arduino/libraries/loader.go +++ b/arduino/libraries/loader.go @@ -19,6 +19,7 @@ import ( "fmt" "strings" + "github.com/arduino/arduino-cli/arduino/cores" "github.com/arduino/arduino-cli/arduino/globals" "github.com/arduino/arduino-cli/arduino/sketch" "github.com/arduino/go-paths-helper" @@ -82,7 +83,15 @@ func makeNewLibrary(libraryDir *paths.Path, location LibraryLocation) (*Library, libProperties.Set("architectures", "*") } library.Architectures = commaSeparatedToList(libProperties.Get("architectures")) - + if supported := libProperties.Get("supported"); supported != "" { + for _, formula := range strings.Split(supported, ",") { + constraint, err := cores.ParseFQBNMatcher(formula) + if err != nil { + return nil, errors.New(tr("invalid value '%[1]s': %[2]s", formula, err)) + } + library.SupportedFQBN = append(library.SupportedFQBN, constraint) + } + } libProperties.Set("category", strings.TrimSpace(libProperties.Get("category"))) if !ValidCategories[libProperties.Get("category")] { libProperties.Set("category", "Uncategorized") From 908588d170a0a893400ef05bc2e266caf6ea629b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 20 Apr 2023 17:48:57 +0200 Subject: [PATCH 3/4] cosmetic changes --- internal/cli/lib/list.go | 7 +------ legacy/builder/resolve_library.go | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/internal/cli/lib/list.go b/internal/cli/lib/list.go index ea644367db5..279ca971010 100644 --- a/internal/cli/lib/list.go +++ b/internal/cli/lib/list.go @@ -67,12 +67,7 @@ func List(instance *rpc.Instance, args []string, all bool, updatable bool) { } // GetList returns a list of installed libraries. -func GetList( - instance *rpc.Instance, - args []string, - all bool, - updatable bool, -) []*rpc.InstalledLibrary { +func GetList(instance *rpc.Instance, args []string, all bool, updatable bool) []*rpc.InstalledLibrary { name := "" if len(args) > 0 { name = args[0] diff --git a/legacy/builder/resolve_library.go b/legacy/builder/resolve_library.go index 3cb3c07aee3..fdacc1486dd 100644 --- a/legacy/builder/resolve_library.go +++ b/legacy/builder/resolve_library.go @@ -34,7 +34,7 @@ func ResolveLibrary(ctx *types.Context, header string) *libraries.Library { ctx.Info(fmt.Sprintf(" -> %s: %s", tr("candidates"), candidates)) } - if candidates == nil || len(candidates) == 0 { + if len(candidates) == 0 { return nil } From 505a0abb3003c2ccc6fab571522a20722619b04b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 20 Apr 2023 17:51:06 +0200 Subject: [PATCH 4/4] Use 'support' field (if available) to compute priority and compatibility --- arduino/libraries/libraries.go | 19 ++++++++--- arduino/libraries/librariesresolver/cpp.go | 19 +++++++---- .../libraries/librariesresolver/cpp_test.go | 32 +++++++++++-------- commands/lib/list.go | 6 ++-- legacy/builder/resolve_library.go | 2 +- 5 files changed, 51 insertions(+), 27 deletions(-) diff --git a/arduino/libraries/libraries.go b/arduino/libraries/libraries.go index 81b2d86fe49..4f308abcaa1 100644 --- a/arduino/libraries/libraries.go +++ b/arduino/libraries/libraries.go @@ -183,10 +183,21 @@ func (library *Library) IsArchitectureIndependent() bool { } // 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) +// the given FQBN. If this function returns false, the library may still +// be compatible with the given FQBN, but it's not explicitly declared. +func (library *Library) IsCompatibleWith(fqbn *cores.FQBN) bool { + // If the library does not specify compatibility use "architecture" field + if len(library.SupportedFQBN) == 0 { + return library.IsArchitectureIndependent() || library.IsOptimizedForArchitecture(fqbn.PlatformArch) + } + + // otherwise check if the given FQBN is supported + for _, supported := range library.SupportedFQBN { + if supported.Match(fqbn) { + return true + } + } + return false } // SourceDir represents a source dir of a library diff --git a/arduino/libraries/librariesresolver/cpp.go b/arduino/libraries/librariesresolver/cpp.go index 63da69f2772..f51b35acfbf 100644 --- a/arduino/libraries/librariesresolver/cpp.go +++ b/arduino/libraries/librariesresolver/cpp.go @@ -118,12 +118,12 @@ func (resolver *Cpp) AlternativesFor(header string) libraries.List { // ResolveFor finds the most suitable library for the specified combination of // header and architecture. If no libraries provides the requested header, nil is returned -func (resolver *Cpp) ResolveFor(header, architecture string) *libraries.Library { - logrus.Infof("Resolving include %s for arch %s", header, architecture) +func (resolver *Cpp) ResolveFor(header string, fqbn *cores.FQBN) *libraries.Library { + logrus.Infof("Resolving include %s for %s", header, fqbn) var found libraries.List var foundPriority int for _, lib := range resolver.headers[header] { - libPriority := ComputePriority(lib, header, architecture) + libPriority := ComputePriority(lib, header, fqbn) msg := " discarded" if found == nil || foundPriority < libPriority { found = libraries.List{} @@ -167,7 +167,7 @@ func simplify(name string) string { // 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 { +func ComputePriority(lib *libraries.Library, header string, fqbn *cores.FQBN) int { header = strings.TrimSuffix(header, filepath.Ext(header)) header = simplify(header) name := simplify(lib.Name) @@ -175,8 +175,15 @@ func ComputePriority(lib *libraries.Library, header, arch string) int { priority := 0 - // Bonus for core-optimized libraries - if lib.IsOptimizedForArchitecture(arch) { + if lib.IsCompatibleWith(fqbn) { + // Bonus for board-optimized libraries + + // give a bonus for libraries that declares specific compatibliity with a board. + // (it is more important than Location but less important than Name) + priority += 1020 + } else if lib.IsOptimizedForArchitecture(fqbn.PlatformArch) { + // Bonus for core-optimized libraries + // give a slightly better bonus for libraries that have specific optimization // (it is more important than Location but less important than Name) priority += 1010 diff --git a/arduino/libraries/librariesresolver/cpp_test.go b/arduino/libraries/librariesresolver/cpp_test.go index e89ac8305a7..331b1e7b486 100644 --- a/arduino/libraries/librariesresolver/cpp_test.go +++ b/arduino/libraries/librariesresolver/cpp_test.go @@ -18,6 +18,7 @@ package librariesresolver import ( "testing" + "github.com/arduino/arduino-cli/arduino/cores" "github.com/arduino/arduino-cli/arduino/libraries" "github.com/stretchr/testify/require" ) @@ -31,12 +32,17 @@ var l6 = &libraries.Library{Name: "Calculus Unified Lib", Location: libraries.Us var l7 = &libraries.Library{Name: "AnotherLib", Location: libraries.User} var bundleServo = &libraries.Library{Name: "Servo", Location: libraries.IDEBuiltIn, Architectures: []string{"avr", "sam", "samd"}} +func mustParseFQBN(fqbn string) *cores.FQBN { + res, _ := cores.ParseFQBN(fqbn) + return res +} + 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) + return resolver.ResolveFor(include, mustParseFQBN("x:"+arch+":y")) } func TestArchitecturePriority(t *testing.T) { @@ -96,19 +102,19 @@ func TestClosestMatchWithTotallyDifferentNames(t *testing.T) { libraryList.Add(l7) resolver := NewCppResolver() resolver.headers["XYZ.h"] = libraryList - res := resolver.ResolveFor("XYZ.h", "xyz") + res := resolver.ResolveFor("XYZ.h", mustParseFQBN("arduino:xyz:uno")) require.NotNil(t, res) require.Equal(t, l7, res, "selected library") } 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", mustParseFQBN("arduino:avr:uno")) + r2 := ComputePriority(l2, "calculus_lib.h", mustParseFQBN("arduino:avr:uno")) + r3 := ComputePriority(l3, "calculus_lib.h", mustParseFQBN("arduino:avr:uno")) + r4 := ComputePriority(l4, "calculus_lib.h", mustParseFQBN("arduino:avr:uno")) + r5 := ComputePriority(l5, "calculus_lib.h", mustParseFQBN("arduino:avr:uno")) + r6 := ComputePriority(l6, "calculus_lib.h", mustParseFQBN("arduino:avr:uno")) + r7 := ComputePriority(l7, "calculus_lib.h", mustParseFQBN("arduino:avr:uno")) require.True(t, r1 > r2) require.True(t, r2 > r3) require.True(t, r3 > r4) @@ -122,7 +128,7 @@ func TestCppHeaderResolverWithNilResult(t *testing.T) { libraryList := libraries.List{} libraryList.Add(l1) resolver.headers["aaa.h"] = libraryList - require.Nil(t, resolver.ResolveFor("bbb.h", "avr")) + require.Nil(t, resolver.ResolveFor("bbb.h", mustParseFQBN("arduino:avr:uno"))) } func TestCppHeaderResolver(t *testing.T) { @@ -133,7 +139,7 @@ func TestCppHeaderResolver(t *testing.T) { librarylist.Add(lib) } resolver.headers[header] = librarylist - return resolver.ResolveFor(header, "avr").Name + return resolver.ResolveFor(header, mustParseFQBN("arduino:avr:uno")).Name } require.Equal(t, "Calculus Lib", resolve("calculus_lib.h", l1, l2, l3, l4, l5, l6, l7)) require.Equal(t, "Calculus Lib-master", resolve("calculus_lib.h", l2, l3, l4, l5, l6, l7)) @@ -150,11 +156,11 @@ func TestCppHeaderResolverWithLibrariesInStrangeDirectoryNames(t *testing.T) { librarylist.Add(&libraries.Library{DirName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"*"}}) librarylist.Add(&libraries.Library{DirName: "onewireng_2_3_4", Name: "OneWireNg", Architectures: []string{"avr"}}) resolver.headers["OneWire.h"] = librarylist - require.Equal(t, "onewire_2_3_4", resolver.ResolveFor("OneWire.h", "avr").DirName) + require.Equal(t, "onewire_2_3_4", resolver.ResolveFor("OneWire.h", mustParseFQBN("arduino:avr:uno")).DirName) librarylist2 := libraries.List{} librarylist2.Add(&libraries.Library{DirName: "OneWire", Name: "OneWire", Architectures: []string{"*"}}) librarylist2.Add(&libraries.Library{DirName: "onewire_2_3_4", Name: "OneWire", Architectures: []string{"avr"}}) resolver.headers["OneWire.h"] = librarylist2 - require.Equal(t, "OneWire", resolver.ResolveFor("OneWire.h", "avr").DirName) + require.Equal(t, "OneWire", resolver.ResolveFor("OneWire.h", mustParseFQBN("arduino:avr:uno")).DirName) } diff --git a/commands/lib/list.go b/commands/lib/list.go index eb226c145c7..25852f01250 100644 --- a/commands/lib/list.go +++ b/commands/lib/list.go @@ -70,8 +70,8 @@ func LibraryList(ctx context.Context, req *rpc.LibraryListRequest) (*rpc.Library } } if latest, has := filteredRes[lib.Library.Name]; has { - latestPriority := librariesresolver.ComputePriority(latest.Library, "", fqbn.PlatformArch) - libPriority := librariesresolver.ComputePriority(lib.Library, "", fqbn.PlatformArch) + latestPriority := librariesresolver.ComputePriority(latest.Library, "", fqbn) + libPriority := librariesresolver.ComputePriority(lib.Library, "", fqbn) if latestPriority >= libPriority { // Pick library with the best priority continue @@ -80,7 +80,7 @@ func LibraryList(ctx context.Context, req *rpc.LibraryListRequest) (*rpc.Library // Check if library is compatible with board specified by FBQN lib.Library.CompatibleWith = map[string]bool{ - fqbnString: lib.Library.IsCompatibleWith(fqbn.PlatformArch), + fqbnString: lib.Library.IsCompatibleWith(fqbn), } filteredRes[lib.Library.Name] = lib diff --git a/legacy/builder/resolve_library.go b/legacy/builder/resolve_library.go index fdacc1486dd..195f02b6948 100644 --- a/legacy/builder/resolve_library.go +++ b/legacy/builder/resolve_library.go @@ -44,7 +44,7 @@ func ResolveLibrary(ctx *types.Context, header string) *libraries.Library { } } - selected := resolver.ResolveFor(header, ctx.TargetPlatform.Platform.Architecture) + selected := resolver.ResolveFor(header, ctx.FQBN) if alreadyImported := importedLibraries.FindByName(selected.Name); alreadyImported != nil { // Certain libraries might have the same name but be different. // This usually happens when the user includes two or more custom libraries that have