From d005bb118e832fa20d1cc75716c0ecced33d965b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 8 Jun 2022 19:30:53 +0200 Subject: [PATCH 1/8] Factored a method in library.List --- arduino/libraries/librarieslist.go | 6 +++--- legacy/builder/resolve_library.go | 13 ++----------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/arduino/libraries/librarieslist.go b/arduino/libraries/librarieslist.go index 9008bf2229b..84062203b76 100644 --- a/arduino/libraries/librarieslist.go +++ b/arduino/libraries/librarieslist.go @@ -41,10 +41,10 @@ func (list *List) Add(libs ...*Library) { } } -// Remove removes the library from the list -func (list *List) Remove(library *Library) { +// Remove removes the given library from the list +func (list *List) Remove(libraryToRemove *Library) { for i, lib := range *list { - if lib == library { + if lib == libraryToRemove { *list = append((*list)[:i], (*list)[i+1:]...) return } diff --git a/legacy/builder/resolve_library.go b/legacy/builder/resolve_library.go index 3cb3c07aee3..0f3de384fe8 100644 --- a/legacy/builder/resolve_library.go +++ b/legacy/builder/resolve_library.go @@ -58,20 +58,11 @@ func ResolveLibrary(ctx *types.Context, header string) *libraries.Library { } } + candidates.Remove(selected) ctx.LibrariesResolutionResults[header] = types.LibraryResolutionResult{ Library: selected, - NotUsedLibraries: filterOutLibraryFrom(candidates, selected), + NotUsedLibraries: candidates, } return selected } - -func filterOutLibraryFrom(libs libraries.List, libraryToRemove *libraries.Library) libraries.List { - filteredOutLibraries := []*libraries.Library{} - for _, lib := range libs { - if lib != libraryToRemove { - filteredOutLibraries = append(filteredOutLibraries, lib) - } - } - return filteredOutLibraries -} From 5ad7d48e76617b9334aa9157f29126d5c1e389aa Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 8 Jun 2022 19:31:37 +0200 Subject: [PATCH 2/8] Fixed lint check --- legacy/builder/resolve_library.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/legacy/builder/resolve_library.go b/legacy/builder/resolve_library.go index 0f3de384fe8..9e63b058623 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 49405ac296b26435cefc6c2a08cbd4b2ce0a0a4b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 9 Jun 2022 13:05:38 +0200 Subject: [PATCH 3/8] Moved ResolveLibrary function in the correct source file --- legacy/builder/container_find_includes.go | 45 +++++++++++++++ legacy/builder/resolve_library.go | 68 ----------------------- 2 files changed, 45 insertions(+), 68 deletions(-) delete mode 100644 legacy/builder/resolve_library.go diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index e7b8cbe0994..0c035936b8f 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -455,3 +455,48 @@ func queueSourceFilesFromFolder(ctx *types.Context, sourceFileQueue *types.Uniqu return nil } + +func ResolveLibrary(ctx *types.Context, header string) *libraries.Library { + resolver := ctx.LibrariesResolver + importedLibraries := ctx.ImportedLibraries + + candidates := resolver.AlternativesFor(header) + + if ctx.Verbose { + ctx.Info(tr("Alternatives for %[1]s: %[2]s", header, candidates)) + ctx.Info(fmt.Sprintf("ResolveLibrary(%s)", header)) + ctx.Info(fmt.Sprintf(" -> %s: %s", tr("candidates"), candidates)) + } + + if len(candidates) == 0 { + return nil + } + + for _, candidate := range candidates { + if importedLibraries.Contains(candidate) { + return nil + } + } + + selected := resolver.ResolveFor(header, ctx.TargetPlatform.Platform.Architecture) + 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 + // different header name but are stored in a parent folder with identical name, like + // ./libraries1/Lib/lib1.h and ./libraries2/Lib/lib2.h + // Without this check the library resolution would be stuck in a loop. + // This behaviour has been reported in this issue: + // https://github.com/arduino/arduino-cli/issues/973 + if selected == alreadyImported { + selected = alreadyImported + } + } + + candidates.Remove(selected) + ctx.LibrariesResolutionResults[header] = types.LibraryResolutionResult{ + Library: selected, + NotUsedLibraries: candidates, + } + + return selected +} diff --git a/legacy/builder/resolve_library.go b/legacy/builder/resolve_library.go deleted file mode 100644 index 9e63b058623..00000000000 --- a/legacy/builder/resolve_library.go +++ /dev/null @@ -1,68 +0,0 @@ -// This file is part of arduino-cli. -// -// Copyright 2020 ARDUINO SA (http://www.arduino.cc/) -// -// This software is released under the GNU General Public License version 3, -// which covers the main part of arduino-cli. -// The terms of this license can be found at: -// https://www.gnu.org/licenses/gpl-3.0.en.html -// -// You can be released from the requirements of the above licenses by purchasing -// a commercial license. Buying such a license is mandatory if you want to -// modify or otherwise use the software for commercial activities involving the -// Arduino software without disclosing the source code of your own applications. -// To purchase a commercial license, send an email to license@arduino.cc. - -package builder - -import ( - "fmt" - - "github.com/arduino/arduino-cli/arduino/libraries" - "github.com/arduino/arduino-cli/legacy/builder/types" -) - -func ResolveLibrary(ctx *types.Context, header string) *libraries.Library { - resolver := ctx.LibrariesResolver - importedLibraries := ctx.ImportedLibraries - - candidates := resolver.AlternativesFor(header) - - if ctx.Verbose { - ctx.Info(tr("Alternatives for %[1]s: %[2]s", header, candidates)) - ctx.Info(fmt.Sprintf("ResolveLibrary(%s)", header)) - ctx.Info(fmt.Sprintf(" -> %s: %s", tr("candidates"), candidates)) - } - - if len(candidates) == 0 { - return nil - } - - for _, candidate := range candidates { - if importedLibraries.Contains(candidate) { - return nil - } - } - - selected := resolver.ResolveFor(header, ctx.TargetPlatform.Platform.Architecture) - 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 - // different header name but are stored in a parent folder with identical name, like - // ./libraries1/Lib/lib1.h and ./libraries2/Lib/lib2.h - // Without this check the library resolution would be stuck in a loop. - // This behaviour has been reported in this issue: - // https://github.com/arduino/arduino-cli/issues/973 - if selected == alreadyImported { - selected = alreadyImported - } - } - - candidates.Remove(selected) - ctx.LibrariesResolutionResults[header] = types.LibraryResolutionResult{ - Library: selected, - NotUsedLibraries: candidates, - } - - return selected -} From 6a109bac0b0aff82d3e182dcb4422ead6fee3876 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 15 Jun 2022 13:28:31 +0200 Subject: [PATCH 4/8] Rename variable 'includes' -> 'includeFolders' --- legacy/builder/container_find_includes.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 0c035936b8f..a85922ca715 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -345,9 +345,9 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu var include string cache.ExpectFile(sourcePath) - includes := ctx.IncludeFolders + includeFolders := ctx.IncludeFolders if library, ok := sourceFile.Origin.(*libraries.Library); ok && library.UtilityDir != nil { - includes = append(includes, library.UtilityDir) + includeFolders = append(includeFolders, library.UtilityDir) } if library, ok := sourceFile.Origin.(*libraries.Library); ok { @@ -371,7 +371,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu } } else { var preproc_stdout []byte - preproc_stdout, preproc_stderr, preproc_err = preprocessor.GCC(sourcePath, targetFilePath, includes, ctx.BuildProperties) + preproc_stdout, preproc_stderr, preproc_err = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, ctx.BuildProperties) if ctx.Verbose { ctx.WriteStdout(preproc_stdout) ctx.WriteStdout(preproc_stderr) @@ -406,7 +406,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu if preproc_err == nil || preproc_stderr == nil { // Filename came from cache, so run preprocessor to obtain error to show var preproc_stdout []byte - preproc_stdout, preproc_stderr, preproc_err = preprocessor.GCC(sourcePath, targetFilePath, includes, ctx.BuildProperties) + preproc_stdout, preproc_stderr, preproc_err = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, ctx.BuildProperties) if ctx.Verbose { ctx.WriteStdout(preproc_stdout) } From 35798bfcdb159479ddb4311fae6cb9802a254373 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 15 Jun 2022 13:30:11 +0200 Subject: [PATCH 5/8] Rename variables to use snakeCase (golang idiomatic) --- legacy/builder/container_find_includes.go | 36 +++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index a85922ca715..5f2e6900659 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -361,8 +361,8 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu } } - var preproc_err error - var preproc_stderr []byte + var preprocErr error + var preprocStderr []byte if unchanged && cache.valid { include = cache.Next().Include @@ -370,24 +370,24 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu ctx.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath)) } } else { - var preproc_stdout []byte - preproc_stdout, preproc_stderr, preproc_err = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, ctx.BuildProperties) + var preprocStdout []byte + preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, ctx.BuildProperties) if ctx.Verbose { - ctx.WriteStdout(preproc_stdout) - ctx.WriteStdout(preproc_stderr) + ctx.WriteStdout(preprocStdout) + ctx.WriteStdout(preprocStderr) } // Unwrap error and see if it is an ExitError. - _, is_exit_error := errors.Cause(preproc_err).(*exec.ExitError) - if preproc_err == nil { + _, isExitErr := errors.Cause(preprocErr).(*exec.ExitError) + if preprocErr == nil { // Preprocessor successful, done include = "" - } else if !is_exit_error || preproc_stderr == nil { + } else if !isExitErr || preprocStderr == nil { // Ignore ExitErrors (e.g. gcc returning // non-zero status), but bail out on // other errors - return errors.WithStack(preproc_err) + return errors.WithStack(preprocErr) } else { - include = IncludesFinderWithRegExp(string(preproc_stderr)) + include = IncludesFinderWithRegExp(string(preprocStderr)) if include == "" && ctx.Verbose { ctx.Info(tr("Error while detecting libraries included by %[1]s", sourcePath)) } @@ -403,14 +403,14 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu library := ResolveLibrary(ctx, include) if library == nil { // Library could not be resolved, show error - if preproc_err == nil || preproc_stderr == nil { + if preprocErr == nil || preprocStderr == nil { // Filename came from cache, so run preprocessor to obtain error to show - var preproc_stdout []byte - preproc_stdout, preproc_stderr, preproc_err = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, ctx.BuildProperties) + var preprocStdout []byte + preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, ctx.BuildProperties) if ctx.Verbose { - ctx.WriteStdout(preproc_stdout) + ctx.WriteStdout(preprocStdout) } - if preproc_err == nil { + if preprocErr == nil { // If there is a missing #include in the cache, but running // gcc does not reproduce that, there is something wrong. // Returning an error here will cause the cache to be @@ -418,8 +418,8 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu return errors.New(tr("Internal error in cache")) } } - ctx.WriteStderr(preproc_stderr) - return errors.WithStack(preproc_err) + ctx.WriteStderr(preprocStderr) + return errors.WithStack(preprocErr) } // Add this library to the list of libraries, the From 55509719c9ae746439e221a08bc27186f8fd2c39 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 15 Jun 2022 13:31:29 +0200 Subject: [PATCH 6/8] Rename variable 'include' -> 'missingIncludeH' --- legacy/builder/container_find_includes.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 5f2e6900659..74cc562dde7 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -342,7 +342,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu first := true for { - var include string + var missingIncludeH string cache.ExpectFile(sourcePath) includeFolders := ctx.IncludeFolders @@ -365,7 +365,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu var preprocStderr []byte if unchanged && cache.valid { - include = cache.Next().Include + missingIncludeH = cache.Next().Include if first && ctx.Verbose { ctx.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath)) } @@ -380,27 +380,27 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu _, isExitErr := errors.Cause(preprocErr).(*exec.ExitError) if preprocErr == nil { // Preprocessor successful, done - include = "" + missingIncludeH = "" } else if !isExitErr || preprocStderr == nil { // Ignore ExitErrors (e.g. gcc returning // non-zero status), but bail out on // other errors return errors.WithStack(preprocErr) } else { - include = IncludesFinderWithRegExp(string(preprocStderr)) - if include == "" && ctx.Verbose { + missingIncludeH = IncludesFinderWithRegExp(string(preprocStderr)) + if missingIncludeH == "" && ctx.Verbose { ctx.Info(tr("Error while detecting libraries included by %[1]s", sourcePath)) } } } - if include == "" { + if missingIncludeH == "" { // No missing includes found, we're done cache.ExpectEntry(sourcePath, "", nil) return nil } - library := ResolveLibrary(ctx, include) + library := ResolveLibrary(ctx, missingIncludeH) if library == nil { // Library could not be resolved, show error if preprocErr == nil || preprocStderr == nil { @@ -426,7 +426,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu // include path and queue its source files for further // include scanning ctx.ImportedLibraries = append(ctx.ImportedLibraries, library) - appendIncludeFolder(ctx, cache, sourcePath, include, library.SourceDir) + appendIncludeFolder(ctx, cache, sourcePath, missingIncludeH, library.SourceDir) sourceDirs := library.SourceDirs() for _, sourceDir := range sourceDirs { queueSourceFilesFromFolder(ctx, sourceFileQueue, library, sourceDir.Dir, sourceDir.Recurse) From 52b32980b5838d0b1ffb40fe1fca442d6a193337 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 10 Jun 2022 16:28:19 +0200 Subject: [PATCH 7/8] Use pointers for SourceFile queues --- legacy/builder/types/accessories.go | 8 ++++---- legacy/builder/types/types.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/legacy/builder/types/accessories.go b/legacy/builder/types/accessories.go index 8decf206162..6624fdb84cc 100644 --- a/legacy/builder/types/accessories.go +++ b/legacy/builder/types/accessories.go @@ -17,14 +17,14 @@ package types import "golang.org/x/exp/slices" -type UniqueSourceFileQueue []SourceFile +type UniqueSourceFileQueue []*SourceFile func (queue UniqueSourceFileQueue) Len() int { return len(queue) } func (queue UniqueSourceFileQueue) Less(i, j int) bool { return false } func (queue UniqueSourceFileQueue) Swap(i, j int) { panic("Who called me?!?") } -func (queue *UniqueSourceFileQueue) Push(value SourceFile) { - equals := func(elem SourceFile) bool { +func (queue *UniqueSourceFileQueue) Push(value *SourceFile) { + equals := func(elem *SourceFile) bool { return elem.Origin == value.Origin && elem.RelativePath.EqualsTo(value.RelativePath) } if !slices.ContainsFunc(*queue, equals) { @@ -32,7 +32,7 @@ func (queue *UniqueSourceFileQueue) Push(value SourceFile) { } } -func (queue *UniqueSourceFileQueue) Pop() SourceFile { +func (queue *UniqueSourceFileQueue) Pop() *SourceFile { old := *queue x := old[0] *queue = old[1:] diff --git a/legacy/builder/types/types.go b/legacy/builder/types/types.go index 8d34056fff5..b02b6fd13ea 100644 --- a/legacy/builder/types/types.go +++ b/legacy/builder/types/types.go @@ -33,15 +33,15 @@ type SourceFile struct { // Create a SourceFile containing the given source file path within the // given origin. The given path can be absolute, or relative within the // origin's root source folder -func MakeSourceFile(ctx *Context, origin interface{}, path *paths.Path) (SourceFile, error) { +func MakeSourceFile(ctx *Context, origin interface{}, path *paths.Path) (*SourceFile, error) { if path.IsAbs() { var err error path, err = sourceRoot(ctx, origin).RelTo(path) if err != nil { - return SourceFile{}, err + return nil, err } } - return SourceFile{Origin: origin, RelativePath: path}, nil + return &SourceFile{Origin: origin, RelativePath: path}, nil } // Return the build root for the given origin, where build products will From e6b20fec0feb719da7520d2f7ace55d1b555931d Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 7 Jun 2023 10:10:41 +0200 Subject: [PATCH 8/8] Clean up implementation of UniqueSourceFileQueue --- legacy/builder/types/accessories.go | 17 +++++++---------- legacy/builder/types/types.go | 5 +++++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/legacy/builder/types/accessories.go b/legacy/builder/types/accessories.go index 6624fdb84cc..b4de7a1a18a 100644 --- a/legacy/builder/types/accessories.go +++ b/legacy/builder/types/accessories.go @@ -19,19 +19,16 @@ import "golang.org/x/exp/slices" type UniqueSourceFileQueue []*SourceFile -func (queue UniqueSourceFileQueue) Len() int { return len(queue) } -func (queue UniqueSourceFileQueue) Less(i, j int) bool { return false } -func (queue UniqueSourceFileQueue) Swap(i, j int) { panic("Who called me?!?") } - func (queue *UniqueSourceFileQueue) Push(value *SourceFile) { - equals := func(elem *SourceFile) bool { - return elem.Origin == value.Origin && elem.RelativePath.EqualsTo(value.RelativePath) - } - if !slices.ContainsFunc(*queue, equals) { + if !queue.Contains(value) { *queue = append(*queue, value) } } +func (queue UniqueSourceFileQueue) Contains(target *SourceFile) bool { + return slices.ContainsFunc(queue, target.Equals) +} + func (queue *UniqueSourceFileQueue) Pop() *SourceFile { old := *queue x := old[0] @@ -39,6 +36,6 @@ func (queue *UniqueSourceFileQueue) Pop() *SourceFile { return x } -func (queue *UniqueSourceFileQueue) Empty() bool { - return queue.Len() == 0 +func (queue UniqueSourceFileQueue) Empty() bool { + return len(queue) == 0 } diff --git a/legacy/builder/types/types.go b/legacy/builder/types/types.go index b02b6fd13ea..938b9658476 100644 --- a/legacy/builder/types/types.go +++ b/legacy/builder/types/types.go @@ -30,6 +30,11 @@ type SourceFile struct { RelativePath *paths.Path } +func (f *SourceFile) Equals(g *SourceFile) bool { + return f.Origin == g.Origin && + f.RelativePath.EqualsTo(g.RelativePath) +} + // Create a SourceFile containing the given source file path within the // given origin. The given path can be absolute, or relative within the // origin's root source folder