From 0e2c87c7e8955cd7cd7cb129b03cca4a52416391 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 31 Jan 2017 17:05:29 +0100 Subject: [PATCH 1/6] Added test for findBestLibraryWithHeader() function Signed-off-by: Cristian Maglie --- .../builder/resolve_library_test.go | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 src/arduino.cc/builder/resolve_library_test.go diff --git a/src/arduino.cc/builder/resolve_library_test.go b/src/arduino.cc/builder/resolve_library_test.go new file mode 100644 index 00000000..13a9f108 --- /dev/null +++ b/src/arduino.cc/builder/resolve_library_test.go @@ -0,0 +1,71 @@ +/* + * This file is part of Arduino Builder. + * + * Arduino Builder is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As a special exception, you may use this file as part of a free software + * library without restriction. Specifically, if other files instantiate + * templates or use macros or inline functions from this file, or you compile + * this file and link it with other files to produce an executable, this + * file does not by itself cause the resulting executable to be covered by + * the GNU General Public License. This exception does not however + * invalidate any other reasons why the executable file might be covered by + * the GNU General Public License. + * + * Copyright 2017 Arduino LLC (http://www.arduino.cc/) + */ + +package builder + +import ( + "testing" + + "arduino.cc/builder/types" + + "github.com/stretchr/testify/require" +) + +func TestFindBestLibraryWithHeader(t *testing.T) { + l1 := &types.Library{Name: "Calculus Lib"} + l2 := &types.Library{Name: "Calculus Lib-master"} + l3 := &types.Library{Name: "Calculus Lib Improved"} + l4 := &types.Library{Name: "Another Calculus Lib"} + l5 := &types.Library{Name: "Yet Another Calculus Lib Improved"} + l6 := &types.Library{Name: "AnotherLib"} + + // Test exact name matching + res := findBestLibraryWithHeader("calculus_lib.h", []*types.Library{l6, l5, l4, l3, l2, l1}) + require.Equal(t, l1.Name, res.Name) + + // Test exact name with "-master" postfix matching + res = findBestLibraryWithHeader("calculus_lib.h", []*types.Library{l6, l5, l4, l3, l2}) + require.Equal(t, l2.Name, res.Name) + + // Test prefix matching + res = findBestLibraryWithHeader("calculus_lib.h", []*types.Library{l6, l5, l4, l3}) + require.Equal(t, l3.Name, res.Name) + + // Test postfix matching + res = findBestLibraryWithHeader("calculus_lib.h", []*types.Library{l6, l5, l4}) + require.Equal(t, l4.Name, res.Name) + + // Test "contains"" matching + res = findBestLibraryWithHeader("calculus_lib.h", []*types.Library{l6, l5}) + require.Equal(t, l5.Name, res.Name) + + // Test none matching + res = findBestLibraryWithHeader("calculus_lib.h", []*types.Library{l6}) + require.Nil(t, res) +} From adf0ab3c155f3245c6f841816f8bc1b1461920f4 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 31 Jan 2017 13:35:48 +0100 Subject: [PATCH 2/6] Removed useless statements markImportedLibrary is lost after function returns so it's useless to update it just before returning. Signed-off-by: Cristian Maglie --- src/arduino.cc/builder/resolve_library.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/arduino.cc/builder/resolve_library.go b/src/arduino.cc/builder/resolve_library.go index 4ba46f82..64abf809 100644 --- a/src/arduino.cc/builder/resolve_library.go +++ b/src/arduino.cc/builder/resolve_library.go @@ -56,7 +56,6 @@ func ResolveLibrary(ctx *types.Context, header string) *types.Library { } if len(libraries) == 1 { - markImportedLibrary[libraries[0]] = true return libraries[0] } @@ -92,7 +91,6 @@ func ResolveLibrary(ctx *types.Context, header string) *types.Library { libraryResolutionResults[header] = types.LibraryResolutionResult{Library: library, NotUsedLibraries: filterOutLibraryFrom(libraries, library)} - markImportedLibrary[library] = true return library } From d675a98957890feef20194ace314c0a0593b67ba Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 31 Jan 2017 13:39:15 +0100 Subject: [PATCH 3/6] Slightly optimize library resolution Even when there is only one candidate library for a particular header we should always check if the library is already included. Signed-off-by: Cristian Maglie --- src/arduino.cc/builder/resolve_library.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/arduino.cc/builder/resolve_library.go b/src/arduino.cc/builder/resolve_library.go index 64abf809..0f52c4eb 100644 --- a/src/arduino.cc/builder/resolve_library.go +++ b/src/arduino.cc/builder/resolve_library.go @@ -55,14 +55,14 @@ func ResolveLibrary(ctx *types.Context, header string) *types.Library { return nil } - if len(libraries) == 1 { - return libraries[0] - } - if markImportedLibraryContainsOneOfCandidates(markImportedLibrary, libraries) { return nil } + if len(libraries) == 1 { + return libraries[0] + } + reverse(libraries) var library *types.Library From fadda87df8b05800e99e65502214f54c57d2ad0e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 31 Jan 2017 13:41:05 +0100 Subject: [PATCH 4/6] In ResolveLibrary the existence-map `markImportedLibrary` is useless It may have had its purposes in the past, but now it's basically useless since it's only used to cycle in `for... range` loops. Signed-off-by: Cristian Maglie --- src/arduino.cc/builder/resolve_library.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/arduino.cc/builder/resolve_library.go b/src/arduino.cc/builder/resolve_library.go index 0f52c4eb..d2eae777 100644 --- a/src/arduino.cc/builder/resolve_library.go +++ b/src/arduino.cc/builder/resolve_library.go @@ -44,18 +44,13 @@ func ResolveLibrary(ctx *types.Context, header string) *types.Library { libraryResolutionResults := ctx.LibrariesResolutionResults importedLibraries := ctx.ImportedLibraries - markImportedLibrary := make(map[*types.Library]bool) - for _, library := range importedLibraries { - markImportedLibrary[library] = true - } - libraries := append([]*types.Library{}, headerToLibraries[header]...) if libraries == nil || len(libraries) == 0 { return nil } - if markImportedLibraryContainsOneOfCandidates(markImportedLibrary, libraries) { + if importedLibraryContainsOneOfCandidates(importedLibraries, libraries) { return nil } @@ -87,7 +82,7 @@ func ResolveLibrary(ctx *types.Context, header string) *types.Library { library = libraries[0] } - library = useAlreadyImportedLibraryWithSameNameIfExists(library, markImportedLibrary) + library = useAlreadyImportedLibraryWithSameNameIfExists(library, importedLibraries) libraryResolutionResults[header] = types.LibraryResolutionResult{Library: library, NotUsedLibraries: filterOutLibraryFrom(libraries, library)} @@ -101,10 +96,10 @@ func reverse(data []*types.Library) { } } -func markImportedLibraryContainsOneOfCandidates(markImportedLibrary map[*types.Library]bool, libraries []*types.Library) bool { - for markedLibrary, _ := range markImportedLibrary { - for _, library := range libraries { - if markedLibrary == library { +func importedLibraryContainsOneOfCandidates(imported []*types.Library, candidates []*types.Library) bool { + for _, i := range imported { + for _, j := range candidates { + if i == j { return true } } @@ -112,8 +107,8 @@ func markImportedLibraryContainsOneOfCandidates(markImportedLibrary map[*types.L return false } -func useAlreadyImportedLibraryWithSameNameIfExists(library *types.Library, markImportedLibrary map[*types.Library]bool) *types.Library { - for lib, _ := range markImportedLibrary { +func useAlreadyImportedLibraryWithSameNameIfExists(library *types.Library, imported []*types.Library) *types.Library { + for _, lib := range imported { if lib.Name == library.Name { return lib } From b54aba3780744f4796a09c49f6efb5691dfd646e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 31 Jan 2017 13:42:53 +0100 Subject: [PATCH 5/6] Small indentation make-up Signed-off-by: Cristian Maglie --- src/arduino.cc/builder/resolve_library.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/arduino.cc/builder/resolve_library.go b/src/arduino.cc/builder/resolve_library.go index d2eae777..a07faeab 100644 --- a/src/arduino.cc/builder/resolve_library.go +++ b/src/arduino.cc/builder/resolve_library.go @@ -84,7 +84,10 @@ func ResolveLibrary(ctx *types.Context, header string) *types.Library { library = useAlreadyImportedLibraryWithSameNameIfExists(library, importedLibraries) - libraryResolutionResults[header] = types.LibraryResolutionResult{Library: library, NotUsedLibraries: filterOutLibraryFrom(libraries, library)} + libraryResolutionResults[header] = types.LibraryResolutionResult{ + Library: library, + NotUsedLibraries: filterOutLibraryFrom(libraries, library), + } return library } From aea468e033b9f3061b92f32422d29b1a8e939d90 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 31 Jan 2017 13:43:35 +0100 Subject: [PATCH 6/6] Relax library matching rules when detecting libraries This allows to set the correct library priority, for example, if the library is named "My Library" but the include file is "my_library.h". Signed-off-by: Cristian Maglie --- src/arduino.cc/builder/resolve_library.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/arduino.cc/builder/resolve_library.go b/src/arduino.cc/builder/resolve_library.go index a07faeab..35639628 100644 --- a/src/arduino.cc/builder/resolve_library.go +++ b/src/arduino.cc/builder/resolve_library.go @@ -218,7 +218,7 @@ func findBestLibraryWithHeader(header string, libraries []*types.Library) *types func findLibWithName(name string, libraries []*types.Library) *types.Library { for _, library := range libraries { - if library.Name == name { + if simplifyName(library.Name) == simplifyName(name) { return library } } @@ -227,7 +227,7 @@ func findLibWithName(name string, libraries []*types.Library) *types.Library { func findLibWithNameStartingWith(name string, libraries []*types.Library) *types.Library { for _, library := range libraries { - if strings.HasPrefix(library.Name, name) { + if strings.HasPrefix(simplifyName(library.Name), simplifyName(name)) { return library } } @@ -236,7 +236,7 @@ func findLibWithNameStartingWith(name string, libraries []*types.Library) *types func findLibWithNameEndingWith(name string, libraries []*types.Library) *types.Library { for _, library := range libraries { - if strings.HasSuffix(library.Name, name) { + if strings.HasSuffix(simplifyName(library.Name), simplifyName(name)) { return library } } @@ -245,13 +245,17 @@ func findLibWithNameEndingWith(name string, libraries []*types.Library) *types.L func findLibWithNameContaining(name string, libraries []*types.Library) *types.Library { for _, library := range libraries { - if strings.Contains(library.Name, name) { + if strings.Contains(simplifyName(library.Name), simplifyName(name)) { return library } } return nil } +func simplifyName(name string) string { + return strings.ToLower(strings.Replace(name, "_", " ", -1)) +} + // thank you golang: I can not use/recycle/adapt utils.SliceContains func sliceContainsLibrary(slice []*types.Library, target *types.Library) bool { for _, value := range slice {