From 148c2e904a16bceab07dff9f6d07cb1c802599fe Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 9 Jun 2022 17:20:34 +0200 Subject: [PATCH 1/8] Transformed FailIfImportedLibraryIsWrong into a functon There was no need to have it encapsulated in a Command --- legacy/builder/constants/constants.go | 1 - legacy/builder/container_find_includes.go | 37 +++++++++---- .../fail_if_imported_library_is_wrong.go | 52 ------------------- 3 files changed, 27 insertions(+), 63 deletions(-) delete mode 100644 legacy/builder/fail_if_imported_library_is_wrong.go diff --git a/legacy/builder/constants/constants.go b/legacy/builder/constants/constants.go index a91f454ffcf..19a828584e3 100644 --- a/legacy/builder/constants/constants.go +++ b/legacy/builder/constants/constants.go @@ -39,7 +39,6 @@ const FOLDER_TOOLS = "tools" const FOLDER_LIBRARIES = "libraries" const LIBRARY_ALL_ARCHS = "*" const LIBRARY_EMAIL = "email" -const LIBRARY_FOLDER_ARCH = "arch" const LIBRARY_FOLDER_SRC = "src" const LOG_LEVEL_DEBUG = "debug" const LOG_LEVEL_ERROR = "error" diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 74cc562dde7..1c86e3a8cc3 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -175,7 +175,7 @@ func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { } } - if err := runCommand(ctx, &FailIfImportedLibraryIsWrong{}); err != nil { + if err := failIfImportedLibraryIsWrong(ctx); err != nil { return errors.WithStack(err) } @@ -198,15 +198,6 @@ func appendIncludeFolder(ctx *types.Context, cache *includeCache, sourceFilePath cache.ExpectEntry(sourceFilePath, include, folder) } -func runCommand(ctx *types.Context, command types.Command) error { - PrintRingNameIfDebug(ctx, command) - err := command.Run(ctx) - if err != nil { - return errors.WithStack(err) - } - return nil -} - type includeCacheEntry struct { Sourcefile *paths.Path Include string @@ -500,3 +491,29 @@ func ResolveLibrary(ctx *types.Context, header string) *libraries.Library { return selected } + +func failIfImportedLibraryIsWrong(ctx *types.Context) error { + if len(ctx.ImportedLibraries) == 0 { + return nil + } + + for _, library := range ctx.ImportedLibraries { + if !library.IsLegacy { + if library.InstallDir.Join("arch").IsDir() { + return errors.New(tr("%[1]s folder is no longer supported! See %[2]s for more information", "'arch'", "http://goo.gl/gfFJzU")) + } + for _, propName := range libraries.MandatoryProperties { + if !library.Properties.ContainsKey(propName) { + return errors.New(tr("Missing '%[1]s' from library in %[2]s", propName, library.InstallDir)) + } + } + if library.Layout == libraries.RecursiveLayout { + if library.UtilityDir != nil { + return errors.New(tr("Library can't use both '%[1]s' and '%[2]s' folders. Double check in '%[3]s'.", "src", "utility", library.InstallDir)) + } + } + } + } + + return nil +} diff --git a/legacy/builder/fail_if_imported_library_is_wrong.go b/legacy/builder/fail_if_imported_library_is_wrong.go deleted file mode 100644 index 34a69d8d9c2..00000000000 --- a/legacy/builder/fail_if_imported_library_is_wrong.go +++ /dev/null @@ -1,52 +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 ( - "errors" - - "github.com/arduino/arduino-cli/arduino/libraries" - "github.com/arduino/arduino-cli/legacy/builder/constants" - "github.com/arduino/arduino-cli/legacy/builder/types" -) - -type FailIfImportedLibraryIsWrong struct{} - -func (s *FailIfImportedLibraryIsWrong) Run(ctx *types.Context) error { - if len(ctx.ImportedLibraries) == 0 { - return nil - } - - for _, library := range ctx.ImportedLibraries { - if !library.IsLegacy { - if library.InstallDir.Join(constants.LIBRARY_FOLDER_ARCH).IsDir() { - return errors.New(tr("%[1]s folder is no longer supported! See %[2]s for more information", "'arch'", "http://goo.gl/gfFJzU")) - } - for _, propName := range libraries.MandatoryProperties { - if !library.Properties.ContainsKey(propName) { - return errors.New(tr("Missing '%[1]s' from library in %[2]s", propName, library.InstallDir)) - } - } - if library.Layout == libraries.RecursiveLayout { - if library.UtilityDir != nil { - return errors.New(tr("Library can't use both '%[1]s' and '%[2]s' folders. Double check in '%[3]s'.", "src", "utility", library.InstallDir)) - } - } - } - } - - return nil -} From 0746619a0f2f0991afbfc9f44b177941e28063a3 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 10 Jun 2022 16:39:01 +0200 Subject: [PATCH 2/8] Move variable near the place it belongs --- legacy/builder/container_find_includes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 1c86e3a8cc3..b6983351891 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -333,7 +333,6 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu first := true for { - var missingIncludeH string cache.ExpectFile(sourcePath) includeFolders := ctx.IncludeFolders @@ -355,6 +354,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu var preprocErr error var preprocStderr []byte + var missingIncludeH string if unchanged && cache.valid { missingIncludeH = cache.Next().Include if first && ctx.Verbose { From 7de56418ad9468610bcc65e5e150e185441c3b5b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 10 Jun 2022 17:20:53 +0200 Subject: [PATCH 3/8] Dramatically simplified SourceFile object Removed dependency from types.Context --- legacy/builder/container_find_includes.go | 10 +-- legacy/builder/types/types.go | 75 ++++++++++++----------- 2 files changed, 43 insertions(+), 42 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index b6983351891..4b6dba0c1b3 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -309,10 +309,10 @@ func writeCache(cache *includeCache, path *paths.Path) error { func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQueue *types.UniqueSourceFileQueue) error { sourceFile := sourceFileQueue.Pop() - sourcePath := sourceFile.SourcePath(ctx) + sourcePath := sourceFile.SourcePath() targetFilePath := paths.NullPath() - depPath := sourceFile.DepfilePath(ctx) - objPath := sourceFile.ObjectPath(ctx) + depPath := sourceFile.DepfilePath() + objPath := sourceFile.ObjectPath() // TODO: This should perhaps also compare against the // include.cache file timestamp. Now, it only checks if the file @@ -336,11 +336,11 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu cache.ExpectFile(sourcePath) includeFolders := ctx.IncludeFolders - if library, ok := sourceFile.Origin.(*libraries.Library); ok && library.UtilityDir != nil { + if library := sourceFile.Library; library != nil && library.UtilityDir != nil { includeFolders = append(includeFolders, library.UtilityDir) } - if library, ok := sourceFile.Origin.(*libraries.Library); ok { + if library := sourceFile.Library; library != nil { if library.Precompiled && library.PrecompiledWithSources { // Fully precompiled libraries should have no dependencies // to avoid ABI breakage diff --git a/legacy/builder/types/types.go b/legacy/builder/types/types.go index 938b9658476..aae96e1c6f7 100644 --- a/legacy/builder/types/types.go +++ b/legacy/builder/types/types.go @@ -24,69 +24,70 @@ import ( ) type SourceFile struct { - // Sketch or Library pointer that this source file lives in - Origin interface{} // Path to the source file within the sketch/library root folder RelativePath *paths.Path + + // Set to the Library object of origin if this source file comes + // from a library + Library *libraries.Library + + // The source root for the given origin, where its source files + // can be found. Prepending this to SourceFile.RelativePath will give + // the full path to that source file. + sourceRoot *paths.Path + + // The build root for the given origin, where build products will + // be placed. Any directories inside SourceFile.RelativePath will be + // appended here. + buildRoot *paths.Path } func (f *SourceFile) Equals(g *SourceFile) bool { - return f.Origin == g.Origin && - f.RelativePath.EqualsTo(g.RelativePath) + return f.Library == g.Library && + f.RelativePath.EqualsTo(g.RelativePath) && + f.buildRoot.EqualsTo(g.buildRoot) && + f.sourceRoot.EqualsTo(g.sourceRoot) } // 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) { - if path.IsAbs() { - var err error - path, err = sourceRoot(ctx, origin).RelTo(path) - if err != nil { - return nil, err - } - } - return &SourceFile{Origin: origin, RelativePath: path}, nil -} + res := &SourceFile{} -// Return the build root for the given origin, where build products will -// be placed. Any directories inside SourceFile.RelativePath will be -// appended here. -func buildRoot(ctx *Context, origin interface{}) *paths.Path { switch o := origin.(type) { case *sketch.Sketch: - return ctx.SketchBuildPath + res.buildRoot = ctx.SketchBuildPath + res.sourceRoot = ctx.SketchBuildPath case *libraries.Library: - return ctx.LibrariesBuildPath.Join(o.DirName) + res.buildRoot = ctx.LibrariesBuildPath.Join(o.DirName) + res.sourceRoot = o.SourceDir + res.Library = o default: panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin)) } -} -// Return the source root for the given origin, where its source files -// can be found. Prepending this to SourceFile.RelativePath will give -// the full path to that source file. -func sourceRoot(ctx *Context, origin interface{}) *paths.Path { - switch o := origin.(type) { - case *sketch.Sketch: - return ctx.SketchBuildPath - case *libraries.Library: - return o.SourceDir - default: - panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin)) + if path.IsAbs() { + var err error + path, err = res.sourceRoot.RelTo(path) + if err != nil { + return nil, err + } } + res.RelativePath = path + return res, nil } -func (f *SourceFile) SourcePath(ctx *Context) *paths.Path { - return sourceRoot(ctx, f.Origin).JoinPath(f.RelativePath) +func (f *SourceFile) SourcePath() *paths.Path { + return f.sourceRoot.JoinPath(f.RelativePath) } -func (f *SourceFile) ObjectPath(ctx *Context) *paths.Path { - return buildRoot(ctx, f.Origin).Join(f.RelativePath.String() + ".o") +func (f *SourceFile) ObjectPath() *paths.Path { + return f.buildRoot.Join(f.RelativePath.String() + ".o") } -func (f *SourceFile) DepfilePath(ctx *Context) *paths.Path { - return buildRoot(ctx, f.Origin).Join(f.RelativePath.String() + ".d") +func (f *SourceFile) DepfilePath() *paths.Path { + return f.buildRoot.Join(f.RelativePath.String() + ".d") } type LibraryResolutionResult struct { From a6682032cff656c19d17346c7d1645053dd84cf9 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 10 Jun 2022 17:04:38 +0200 Subject: [PATCH 4/8] Added comments about utility folder role in library discovery --- legacy/builder/container_find_includes.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 4b6dba0c1b3..d80f687c167 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -335,6 +335,10 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu for { cache.ExpectFile(sourcePath) + // Libraries may require the "utility" directory to be added to the include + // search path, but only for the source code of the library, so we temporary + // copy the current search path list and add the library' utility directory + // if needed. includeFolders := ctx.IncludeFolders if library := sourceFile.Library; library != nil && library.UtilityDir != nil { includeFolders = append(includeFolders, library.UtilityDir) From b7fe792ab234901dc1ab30b27fa57d9bcb55ed94 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 10 Jun 2022 20:07:41 +0200 Subject: [PATCH 5/8] Simplified if construct --- legacy/builder/container_find_includes.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index d80f687c167..3182d5134f2 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -372,14 +372,11 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu ctx.WriteStdout(preprocStderr) } // Unwrap error and see if it is an ExitError. - _, isExitErr := errors.Cause(preprocErr).(*exec.ExitError) if preprocErr == nil { // Preprocessor successful, done missingIncludeH = "" - } else if !isExitErr || preprocStderr == nil { - // Ignore ExitErrors (e.g. gcc returning - // non-zero status), but bail out on - // other errors + } else if _, isExitErr := errors.Cause(preprocErr).(*exec.ExitError); !isExitErr || preprocStderr == nil { + // Ignore ExitErrors (e.g. gcc returning non-zero status), but bail out on other errors return errors.WithStack(preprocErr) } else { missingIncludeH = IncludesFinderWithRegExp(string(preprocStderr)) From feae077575f51e6b0ab49b56982ba9f2d95e85f2 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 12 Jun 2023 10:55:59 +0200 Subject: [PATCH 6/8] Made RelativePath private --- legacy/builder/types/types.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/legacy/builder/types/types.go b/legacy/builder/types/types.go index aae96e1c6f7..9472f6f93d2 100644 --- a/legacy/builder/types/types.go +++ b/legacy/builder/types/types.go @@ -25,7 +25,7 @@ import ( type SourceFile struct { // Path to the source file within the sketch/library root folder - RelativePath *paths.Path + relativePath *paths.Path // Set to the Library object of origin if this source file comes // from a library @@ -43,8 +43,7 @@ type SourceFile struct { } func (f *SourceFile) Equals(g *SourceFile) bool { - return f.Library == g.Library && - f.RelativePath.EqualsTo(g.RelativePath) && + return f.relativePath.EqualsTo(g.relativePath) && f.buildRoot.EqualsTo(g.buildRoot) && f.sourceRoot.EqualsTo(g.sourceRoot) } @@ -74,20 +73,20 @@ func MakeSourceFile(ctx *Context, origin interface{}, path *paths.Path) (*Source return nil, err } } - res.RelativePath = path + res.relativePath = path return res, nil } func (f *SourceFile) SourcePath() *paths.Path { - return f.sourceRoot.JoinPath(f.RelativePath) + return f.sourceRoot.JoinPath(f.relativePath) } func (f *SourceFile) ObjectPath() *paths.Path { - return f.buildRoot.Join(f.RelativePath.String() + ".o") + return f.buildRoot.Join(f.relativePath.String() + ".o") } func (f *SourceFile) DepfilePath() *paths.Path { - return f.buildRoot.Join(f.RelativePath.String() + ".d") + return f.buildRoot.Join(f.relativePath.String() + ".d") } type LibraryResolutionResult struct { From 48bc5225ba6ef30fb3a35b4ba9b3ea939e1058b0 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 12 Jun 2023 10:59:38 +0200 Subject: [PATCH 7/8] Skip includes detection of precompiled libraries early Instead of skipping include detection later, avoid to add the sources in the queue right from the beginning. --- legacy/builder/container_find_includes.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 3182d5134f2..910d7708c4b 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -343,16 +343,6 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu if library := sourceFile.Library; library != nil && library.UtilityDir != nil { includeFolders = append(includeFolders, library.UtilityDir) } - - if library := sourceFile.Library; library != nil { - if library.Precompiled && library.PrecompiledWithSources { - // Fully precompiled libraries should have no dependencies - // to avoid ABI breakage - if ctx.Verbose { - ctx.Info(tr("Skipping dependencies detection for precompiled library %[1]s", library.Name)) - } - return nil - } } var preprocErr error @@ -419,9 +409,16 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu // include scanning ctx.ImportedLibraries = append(ctx.ImportedLibraries, library) appendIncludeFolder(ctx, cache, sourcePath, missingIncludeH, library.SourceDir) - sourceDirs := library.SourceDirs() - for _, sourceDir := range sourceDirs { - queueSourceFilesFromFolder(ctx, sourceFileQueue, library, sourceDir.Dir, sourceDir.Recurse) + + if library.Precompiled && library.PrecompiledWithSources { + // Fully precompiled libraries should have no dependencies to avoid ABI breakage + if ctx.Verbose { + ctx.Info(tr("Skipping dependencies detection for precompiled library %[1]s", library.Name)) + } + } else { + for _, sourceDir := range library.SourceDirs() { + queueSourceFilesFromFolder(ctx, sourceFileQueue, library, sourceDir.Dir, sourceDir.Recurse) + } } first = false } From 22fd8e23dd491617aba64fa77c7bd92c63c2ada3 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 12 Jun 2023 16:46:10 +0200 Subject: [PATCH 8/8] Keep extra-include dirs due to "utility" folder in SourceFile object Also remove the reference to the original Library object because it's no more needed. --- legacy/builder/container_find_includes.go | 5 ++--- legacy/builder/types/types.go | 14 ++++++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 910d7708c4b..90457b8e7f6 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -340,9 +340,8 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu // copy the current search path list and add the library' utility directory // if needed. includeFolders := ctx.IncludeFolders - if library := sourceFile.Library; library != nil && library.UtilityDir != nil { - includeFolders = append(includeFolders, library.UtilityDir) - } + if extraInclude := sourceFile.ExtraIncludePath(); extraInclude != nil { + includeFolders = append(includeFolders, extraInclude) } var preprocErr error diff --git a/legacy/builder/types/types.go b/legacy/builder/types/types.go index 9472f6f93d2..2ebf92bae5a 100644 --- a/legacy/builder/types/types.go +++ b/legacy/builder/types/types.go @@ -27,9 +27,11 @@ type SourceFile struct { // Path to the source file within the sketch/library root folder relativePath *paths.Path - // Set to the Library object of origin if this source file comes - // from a library - Library *libraries.Library + // ExtraIncludePath contains an extra include path that must be + // used to compile this source file. + // This is mainly used for source files that comes from old-style libraries + // (Arduino IDE <1.5) requiring an extra include path to the "utility" folder. + extraIncludePath *paths.Path // The source root for the given origin, where its source files // can be found. Prepending this to SourceFile.RelativePath will give @@ -61,7 +63,7 @@ func MakeSourceFile(ctx *Context, origin interface{}, path *paths.Path) (*Source case *libraries.Library: res.buildRoot = ctx.LibrariesBuildPath.Join(o.DirName) res.sourceRoot = o.SourceDir - res.Library = o + res.extraIncludePath = o.UtilityDir default: panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin)) } @@ -77,6 +79,10 @@ func MakeSourceFile(ctx *Context, origin interface{}, path *paths.Path) (*Source return res, nil } +func (f *SourceFile) ExtraIncludePath() *paths.Path { + return f.extraIncludePath +} + func (f *SourceFile) SourcePath() *paths.Path { return f.sourceRoot.JoinPath(f.relativePath) }