From 61969728b2463b441cc898d3a2acaf898300bbf8 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 21 Jan 2022 23:31:19 +0100 Subject: [PATCH 01/24] Perform earlier exclusion of includes search of precompiled libraries --- legacy/builder/container_find_includes.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index c8c5325eac9..3a1d2453620 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -327,17 +327,6 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t includes = append(includes, library.UtilityDir) } - if library, ok := sourceFile.Origin.(*libraries.Library); ok { - 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 preproc_err error var preproc_stderr []byte @@ -399,7 +388,15 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t appendIncludeFolder(ctx, cache, sourcePath, include, library.SourceDir) sourceDirs := library.SourceDirs() for _, sourceDir := range sourceDirs { - queueSourceFilesFromFolder(ctx, ctx.CollectedSourceFiles, 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 { + queueSourceFilesFromFolder(ctx, ctx.CollectedSourceFiles, library, sourceDir.Dir, sourceDir.Recurse) + } } first = false } From 32a5c3c453e45373d3f36d53ef47fc63834f03c7 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 17 Oct 2019 13:12:34 +0200 Subject: [PATCH 02/24] includeCache: put cacheFilePath as class member Also added methods to Load and Save the cache from disk without the need to pass cacheFilePath back and forth. --- legacy/builder/container_find_includes.go | 53 ++++++++++++----------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 3a1d2453620..9789fedfc90 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -121,8 +121,7 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { } func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { - cachePath := ctx.BuildPath.Join("includes.cache") - cache := readCache(cachePath) + cache := loadCacheFrom(ctx.BuildPath.Join("includes.cache")) appendIncludeFolder(ctx, cache, nil, "", ctx.BuildProperties.GetPath("build.core.path")) if ctx.BuildProperties.Get("build.variant.path") != "" { @@ -144,17 +143,15 @@ func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { } for !sourceFilePaths.Empty() { - err := findIncludesUntilDone(ctx, cache, sourceFilePaths.Pop()) - if err != nil { - cachePath.Remove() + if err := findIncludesUntilDone(ctx, cache, sourceFilePaths.Pop()); err != nil { + cache.Remove() return errors.WithStack(err) } } // Finalize the cache cache.ExpectEnd() - err = writeCache(cache, cachePath) - if err != nil { + if err := cache.WriteToFile(); err != nil { return errors.WithStack(err) } @@ -203,6 +200,10 @@ func (entry *includeCacheEntry) Equals(other *includeCacheEntry) bool { type includeCache struct { // Are the cache contents valid so far? valid bool + + // The file where to save the cache + cacheFilePath *paths.Path + // Index into entries of the next entry to be processed. Unused // when the cache is invalid. next int @@ -255,39 +256,39 @@ func (cache *includeCache) ExpectEnd() { } } -// Read the cache from the given file -func readCache(path *paths.Path) *includeCache { - bytes, err := path.ReadFile() - if err != nil { - // Return an empty, invalid cache - return &includeCache{} +// Remove removes the cache file from disk. +func (cache *includeCache) Remove() error { + return cache.cacheFilePath.Remove() +} + +// loadCacheFrom read the cache from the given file +func loadCacheFrom(path *paths.Path) *includeCache { + result := &includeCache{ + cacheFilePath: path, + valid: false, } - result := &includeCache{} - err = json.Unmarshal(bytes, &result.entries) - if err != nil { - // Return an empty, invalid cache - return &includeCache{} + if bytes, err := path.ReadFile(); err != nil { + return result + } else if err = json.Unmarshal(bytes, &result.entries); err != nil { + return result } result.valid = true return result } -// Write the given cache to the given file if it is invalidated. If the +// WriteToFile the cache file if it is invalidated. If the // cache is still valid, just update the timestamps of the file. -func writeCache(cache *includeCache, path *paths.Path) error { +func (cache *includeCache) WriteToFile() error { // If the cache was still valid all the way, just touch its file // (in case any source file changed without influencing the // includes). If it was invalidated, overwrite the cache with // the new contents. if cache.valid { - path.Chtimes(time.Now(), time.Now()) + cache.cacheFilePath.Chtimes(time.Now(), time.Now()) } else { - bytes, err := json.MarshalIndent(cache.entries, "", " ") - if err != nil { + if bytes, err := json.MarshalIndent(cache.entries, "", " "); err != nil { return errors.WithStack(err) - } - err = path.WriteFile(bytes) - if err != nil { + } else if err := cache.cacheFilePath.WriteFile(bytes); err != nil { return errors.WithStack(err) } } From 2a64bc7f25cf5319d81656021581829685430021 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 17 Oct 2019 16:45:27 +0200 Subject: [PATCH 03/24] Moved SourceFile, UniqueSourceFileQueue and related methods in container_find_includes.go There is no point in keeping those structures inside module types because: - they are used only in 'container_find_includes.go' - the entry in Context is used only in for library discovery This commit prepares for the extraction of the library discovery logic from legacy package. --- .../add_additional_entries_to_context.go | 2 - legacy/builder/container_find_includes.go | 116 ++++++++++++++++-- .../add_additional_entries_to_context_test.go | 4 - legacy/builder/types/accessories.go | 23 ---- legacy/builder/types/context.go | 2 - legacy/builder/types/types.go | 63 ---------- legacy/builder/types/utils.go | 9 -- 7 files changed, 105 insertions(+), 114 deletions(-) diff --git a/legacy/builder/add_additional_entries_to_context.go b/legacy/builder/add_additional_entries_to_context.go index 775b6cdc63e..23d4a296146 100644 --- a/legacy/builder/add_additional_entries_to_context.go +++ b/legacy/builder/add_additional_entries_to_context.go @@ -63,8 +63,6 @@ func (*AddAdditionalEntriesToContext) Run(ctx *types.Context) error { ctx.WarningsLevel = DEFAULT_WARNINGS_LEVEL } - ctx.CollectedSourceFiles = &types.UniqueSourceFileQueue{} - ctx.LibrariesResolutionResults = map[string]types.LibraryResolutionResult{} ctx.HardwareRewriteResults = map[*cores.PlatformRelease][]types.PlatforKeyRewrite{} diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 9789fedfc90..f6f9ecc29c6 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -99,6 +99,7 @@ import ( "time" "github.com/arduino/arduino-cli/arduino/libraries" + "github.com/arduino/arduino-cli/arduino/sketch" "github.com/arduino/arduino-cli/legacy/builder/builder_utils" "github.com/arduino/arduino-cli/legacy/builder/types" "github.com/arduino/arduino-cli/legacy/builder/utils" @@ -129,21 +130,21 @@ func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { } sketch := ctx.Sketch - mergedfile, err := types.MakeSourceFile(ctx, sketch, paths.New(sketch.MainFile.Base()+".cpp")) + mergedfile, err := MakeSourceFilee(ctx, sketch, paths.New(sketch.MainFile.Base()+".cpp")) if err != nil { return errors.WithStack(err) } - ctx.CollectedSourceFiles.Push(mergedfile) + queue := &UniqueSourceFileQueue{} + queue.Push(mergedfile) - sourceFilePaths := ctx.CollectedSourceFiles - queueSourceFilesFromFolder(ctx, sourceFilePaths, sketch, ctx.SketchBuildPath, false /* recurse */) + queueSourceFilesFromFolder(ctx, queue, sketch, ctx.SketchBuildPath, false /* recurse */) srcSubfolderPath := ctx.SketchBuildPath.Join("src") if srcSubfolderPath.IsDir() { - queueSourceFilesFromFolder(ctx, sourceFilePaths, sketch, srcSubfolderPath, true /* recurse */) + queueSourceFilesFromFolder(ctx, queue, sketch, srcSubfolderPath, true /* recurse */) } - for !sourceFilePaths.Empty() { - if err := findIncludesUntilDone(ctx, cache, sourceFilePaths.Pop()); err != nil { + for !queue.Empty() { + if err := findIncludesUntilDone(ctx, cache, queue, queue.Pop()); err != nil { cache.Remove() return errors.WithStack(err) } @@ -295,7 +296,7 @@ func (cache *includeCache) WriteToFile() error { return nil } -func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile types.SourceFile) error { +func findIncludesUntilDone(ctx *types.Context, cache *includeCache, queue *UniqueSourceFileQueue, sourceFile SourceFile) error { sourcePath := sourceFile.SourcePath(ctx) targetFilePath := paths.NullPath() depPath := sourceFile.DepfilePath(ctx) @@ -396,14 +397,14 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t ctx.Info(tr("Skipping dependencies detection for precompiled library %[1]s", library.Name)) } } else { - queueSourceFilesFromFolder(ctx, ctx.CollectedSourceFiles, library, sourceDir.Dir, sourceDir.Recurse) + queueSourceFilesFromFolder(ctx, queue, library, sourceDir.Dir, sourceDir.Recurse) } } first = false } } -func queueSourceFilesFromFolder(ctx *types.Context, queue *types.UniqueSourceFileQueue, origin interface{}, folder *paths.Path, recurse bool) error { +func queueSourceFilesFromFolder(ctx *types.Context, queue *UniqueSourceFileQueue, origin interface{}, folder *paths.Path, recurse bool) error { extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } filePaths := []string{} @@ -413,7 +414,7 @@ func queueSourceFilesFromFolder(ctx *types.Context, queue *types.UniqueSourceFil } for _, filePath := range filePaths { - sourceFile, err := types.MakeSourceFile(ctx, origin, paths.New(filePath)) + sourceFile, err := MakeSourceFilee(ctx, origin, paths.New(filePath)) if err != nil { return errors.WithStack(err) } @@ -422,3 +423,96 @@ func queueSourceFilesFromFolder(ctx *types.Context, queue *types.UniqueSourceFil return nil } + +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 +} + +// 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 MakeSourceFilee(ctx *types.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 SourceFile{Origin: origin, RelativePath: path}, nil +} + +// 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 *types.Context, origin interface{}) *paths.Path { + switch o := origin.(type) { + case *sketch.Sketch: + return ctx.SketchBuildPath + case *libraries.Library: + return ctx.LibrariesBuildPath.Join(o.Name) + 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 *types.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)) + } +} + +func (f *SourceFile) SourcePath(ctx *types.Context) *paths.Path { + return sourceRoot(ctx, f.Origin).JoinPath(f.RelativePath) +} + +func (f *SourceFile) ObjectPath(ctx *types.Context) *paths.Path { + return buildRoot(ctx, f.Origin).Join(f.RelativePath.String() + ".o") +} + +func (f *SourceFile) DepfilePath(ctx *types.Context) *paths.Path { + return buildRoot(ctx, f.Origin).Join(f.RelativePath.String() + ".d") +} + +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) { + if !sliceContainsSourceFile(*queue, value) { + *queue = append(*queue, value) + } +} + +func (queue *UniqueSourceFileQueue) Pop() SourceFile { + old := *queue + x := old[0] + *queue = old[1:] + return x +} + +func (queue *UniqueSourceFileQueue) Empty() bool { + return queue.Len() == 0 +} + +func sliceContainsSourceFile(slice []SourceFile, target SourceFile) bool { + for _, elem := range slice { + if elem.Origin == target.Origin && elem.RelativePath.EqualsTo(target.RelativePath) { + return true + } + } + return false +} diff --git a/legacy/builder/test/add_additional_entries_to_context_test.go b/legacy/builder/test/add_additional_entries_to_context_test.go index ab64c26ec4c..51e5452daf3 100644 --- a/legacy/builder/test/add_additional_entries_to_context_test.go +++ b/legacy/builder/test/add_additional_entries_to_context_test.go @@ -38,8 +38,6 @@ func TestAddAdditionalEntriesToContextNoBuildPath(t *testing.T) { require.NotNil(t, ctx.WarningsLevel) - require.True(t, ctx.CollectedSourceFiles.Empty()) - require.Equal(t, 0, len(ctx.LibrariesResolutionResults)) } @@ -57,7 +55,5 @@ func TestAddAdditionalEntriesToContextWithBuildPath(t *testing.T) { require.NotNil(t, ctx.WarningsLevel) - require.True(t, ctx.CollectedSourceFiles.Empty()) - require.Equal(t, 0, len(ctx.LibrariesResolutionResults)) } diff --git a/legacy/builder/types/accessories.go b/legacy/builder/types/accessories.go index b0c85d37e20..c178c15c61e 100644 --- a/legacy/builder/types/accessories.go +++ b/legacy/builder/types/accessories.go @@ -43,29 +43,6 @@ func (queue *UniqueStringQueue) Empty() bool { return queue.Len() == 0 } -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) { - if !sliceContainsSourceFile(*queue, value) { - *queue = append(*queue, value) - } -} - -func (queue *UniqueSourceFileQueue) Pop() SourceFile { - old := *queue - x := old[0] - *queue = old[1:] - return x -} - -func (queue *UniqueSourceFileQueue) Empty() bool { - return queue.Len() == 0 -} - type BufferedUntilNewLineWriter struct { PrintFunc PrintFunc Buffer bytes.Buffer diff --git a/legacy/builder/types/context.go b/legacy/builder/types/context.go index 77610afbb8e..503ee48594d 100644 --- a/legacy/builder/types/context.go +++ b/legacy/builder/types/context.go @@ -111,8 +111,6 @@ type Context struct { SketchObjectFiles paths.PathList IgnoreSketchFolderNameErrors bool - CollectedSourceFiles *UniqueSourceFileQueue - Sketch *sketch.Sketch Source string SourceGccMinusE string diff --git a/legacy/builder/types/types.go b/legacy/builder/types/types.go index 70ee1c34608..fbb7dc2d58e 100644 --- a/legacy/builder/types/types.go +++ b/legacy/builder/types/types.go @@ -16,75 +16,12 @@ package types import ( - "fmt" "strconv" "github.com/arduino/arduino-cli/arduino/libraries" - "github.com/arduino/arduino-cli/arduino/sketch" paths "github.com/arduino/go-paths-helper" ) -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 -} - -// 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 SourceFile{}, err - } - } - return SourceFile{Origin: origin, RelativePath: path}, nil -} - -// 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 - case *libraries.Library: - return ctx.LibrariesBuildPath.Join(o.Name) - 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)) - } -} - -func (f *SourceFile) SourcePath(ctx *Context) *paths.Path { - return sourceRoot(ctx, f.Origin).JoinPath(f.RelativePath) -} - -func (f *SourceFile) ObjectPath(ctx *Context) *paths.Path { - return buildRoot(ctx, f.Origin).Join(f.RelativePath.String() + ".o") -} - -func (f *SourceFile) DepfilePath(ctx *Context) *paths.Path { - return buildRoot(ctx, f.Origin).Join(f.RelativePath.String() + ".d") -} - type SketchFile struct { Name *paths.Path } diff --git a/legacy/builder/types/utils.go b/legacy/builder/types/utils.go index 7102cd555ca..3b514831518 100644 --- a/legacy/builder/types/utils.go +++ b/legacy/builder/types/utils.go @@ -24,12 +24,3 @@ func sliceContains(slice []string, target string) bool { } return false } - -func sliceContainsSourceFile(slice []SourceFile, target SourceFile) bool { - for _, elem := range slice { - if elem.Origin == target.Origin && elem.RelativePath.EqualsTo(target.RelativePath) { - return true - } - } - return false -} From 63fe343615028057d6c7ac1f0125cfbf8abd6079 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 17 Oct 2019 16:53:30 +0200 Subject: [PATCH 04/24] Slightly refactored methods of UniqueSourceFileQueue - Added 'Contains(...)' method (instead of sliceContainsSourceFile(...)) - Removed useless comparators --- legacy/builder/container_find_includes.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index f6f9ecc29c6..600c5bb4a3c 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -487,12 +487,12 @@ func (f *SourceFile) DepfilePath(ctx *types.Context) *paths.Path { 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) Len() int { + return len(queue) +} func (queue *UniqueSourceFileQueue) Push(value SourceFile) { - if !sliceContainsSourceFile(*queue, value) { + if !queue.Contains(value) { *queue = append(*queue, value) } } @@ -508,8 +508,8 @@ func (queue *UniqueSourceFileQueue) Empty() bool { return queue.Len() == 0 } -func sliceContainsSourceFile(slice []SourceFile, target SourceFile) bool { - for _, elem := range slice { +func (queue *UniqueSourceFileQueue) Contains(target SourceFile) bool { + for _, elem := range *queue { if elem.Origin == target.Origin && elem.RelativePath.EqualsTo(target.RelativePath) { return true } From 082591c04699670ed0add5abe5cba6fefb556333 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 17 Oct 2019 17:17:17 +0200 Subject: [PATCH 05/24] UniqueSourceFileQueue do not drop already processed files anymore --- legacy/builder/container_find_includes.go | 32 ++++++++++++----------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 600c5bb4a3c..02af9a51e85 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -485,31 +485,33 @@ func (f *SourceFile) DepfilePath(ctx *types.Context) *paths.Path { return buildRoot(ctx, f.Origin).Join(f.RelativePath.String() + ".d") } -type UniqueSourceFileQueue []SourceFile +type UniqueSourceFileQueue struct { + queue []SourceFile + curr int +} -func (queue UniqueSourceFileQueue) Len() int { - return len(queue) +func (q *UniqueSourceFileQueue) Len() int { + return len(q.queue) - q.curr } -func (queue *UniqueSourceFileQueue) Push(value SourceFile) { - if !queue.Contains(value) { - *queue = append(*queue, value) +func (q *UniqueSourceFileQueue) Push(value SourceFile) { + if !q.Contains(value) { + q.queue = append(q.queue, value) } } -func (queue *UniqueSourceFileQueue) Pop() SourceFile { - old := *queue - x := old[0] - *queue = old[1:] - return x +func (q *UniqueSourceFileQueue) Pop() SourceFile { + res := q.queue[q.curr] + q.curr++ + return res } -func (queue *UniqueSourceFileQueue) Empty() bool { - return queue.Len() == 0 +func (q *UniqueSourceFileQueue) Empty() bool { + return q.Len() == 0 } -func (queue *UniqueSourceFileQueue) Contains(target SourceFile) bool { - for _, elem := range *queue { +func (q *UniqueSourceFileQueue) Contains(target SourceFile) bool { + for _, elem := range q.queue { if elem.Origin == target.Origin && elem.RelativePath.EqualsTo(target.RelativePath) { return true } From d315ce68d4f84b2a333554a17ff5d1b273567a75 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 17 Oct 2019 19:15:04 +0200 Subject: [PATCH 06/24] Create CppIncludesFinder object to handle library discovery. All the fields needed to perform library discovery have been moved into the CppIncludeFinder so there is no more need to pass them around as parameters making it much more lightweight. --- legacy/builder/container_find_includes.go | 123 ++++++++++++---------- 1 file changed, 70 insertions(+), 53 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 02af9a51e85..3d950467e91 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -122,45 +122,63 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { } func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { - cache := loadCacheFrom(ctx.BuildPath.Join("includes.cache")) + finder := &CppIncludesFinder{ + ctx: ctx, + } + if err := finder.DetectLibraries(); err != nil { + return err + } + if err := runCommand(ctx, &FailIfImportedLibraryIsWrong{}); err != nil { + return errors.WithStack(err) + } + + return nil +} + +// CppIncludesFinder implements an algorithm to automatically detect +// libraries used in a sketch and a way to cache this result for +// increasing detection speed on already processed sketches. +type CppIncludesFinder struct { + ctx *types.Context + cache *includeCache + sketch *sketch.Sketch + queue *UniqueSourceFileQueue +} + +func (f *CppIncludesFinder) DetectLibraries() error { + f.cache = loadCacheFrom(f.ctx.BuildPath.Join("includes.cache")) + f.sketch = f.ctx.Sketch + f.queue = &UniqueSourceFileQueue{} - appendIncludeFolder(ctx, cache, nil, "", ctx.BuildProperties.GetPath("build.core.path")) - if ctx.BuildProperties.Get("build.variant.path") != "" { - appendIncludeFolder(ctx, cache, nil, "", ctx.BuildProperties.GetPath("build.variant.path")) + f.appendIncludeFolder(nil, "", f.ctx.BuildProperties.GetPath("build.core.path")) + if f.ctx.BuildProperties.Get("build.variant.path") != "" { + f.appendIncludeFolder(nil, "", f.ctx.BuildProperties.GetPath("build.variant.path")) } - sketch := ctx.Sketch - mergedfile, err := MakeSourceFilee(ctx, sketch, paths.New(sketch.MainFile.Base()+".cpp")) + mergedfile, err := MakeSourceFile(f.ctx, f.sketch, paths.New(f.sketch.MainFile.Base()+".cpp")) if err != nil { return errors.WithStack(err) } - queue := &UniqueSourceFileQueue{} - queue.Push(mergedfile) + f.queue.Push(mergedfile) - queueSourceFilesFromFolder(ctx, queue, sketch, ctx.SketchBuildPath, false /* recurse */) - srcSubfolderPath := ctx.SketchBuildPath.Join("src") + f.queueSourceFilesFromFolder(f.sketch, f.ctx.SketchBuildPath, false /* recurse */) + srcSubfolderPath := f.ctx.SketchBuildPath.Join("src") if srcSubfolderPath.IsDir() { - queueSourceFilesFromFolder(ctx, queue, sketch, srcSubfolderPath, true /* recurse */) + f.queueSourceFilesFromFolder(f.sketch, srcSubfolderPath, true /* recurse */) } - for !queue.Empty() { - if err := findIncludesUntilDone(ctx, cache, queue, queue.Pop()); err != nil { - cache.Remove() + for !f.queue.Empty() { + if err := f.findIncludesUntilDone(f.queue.Pop()); err != nil { + f.cache.Remove() return errors.WithStack(err) } } // Finalize the cache - cache.ExpectEnd() - if err := cache.WriteToFile(); err != nil { - return errors.WithStack(err) - } - - err = runCommand(ctx, &FailIfImportedLibraryIsWrong{}) - if err != nil { + f.cache.ExpectEnd() + if err := f.cache.WriteToFile(); err != nil { return errors.WithStack(err) } - return nil } @@ -169,9 +187,9 @@ func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { // include (e.g. what #include line in what file it was resolved from) // and should be the empty string for the default include folders, like // the core or variant. -func appendIncludeFolder(ctx *types.Context, cache *includeCache, sourceFilePath *paths.Path, include string, folder *paths.Path) { - ctx.IncludeFolders = append(ctx.IncludeFolders, folder) - cache.ExpectEntry(sourceFilePath, include, folder) +func (f *CppIncludesFinder) appendIncludeFolder(sourceFilePath *paths.Path, include string, folder *paths.Path) { + f.ctx.IncludeFolders = append(f.ctx.IncludeFolders, folder) + f.cache.ExpectEntry(sourceFilePath, include, folder) } func runCommand(ctx *types.Context, command types.Command) error { @@ -296,11 +314,11 @@ func (cache *includeCache) WriteToFile() error { return nil } -func findIncludesUntilDone(ctx *types.Context, cache *includeCache, queue *UniqueSourceFileQueue, sourceFile SourceFile) error { - sourcePath := sourceFile.SourcePath(ctx) +func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { + sourcePath := sourceFile.SourcePath(f.ctx) targetFilePath := paths.NullPath() - depPath := sourceFile.DepfilePath(ctx) - objPath := sourceFile.ObjectPath(ctx) + depPath := sourceFile.DepfilePath(f.ctx) + objPath := sourceFile.ObjectPath(f.ctx) // TODO: This should perhaps also compare against the // include.cache file timestamp. Now, it only checks if the file @@ -314,7 +332,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, queue *Uniqu // TODO: This reads the dependency file, but the actual building // does it again. Should the result be somehow cached? Perhaps // remove the object file if it is found to be stale? - unchanged, err := builder_utils.ObjFileIsUpToDate(ctx, sourcePath, objPath, depPath) + unchanged, err := builder_utils.ObjFileIsUpToDate(f.ctx, sourcePath, objPath, depPath) if err != nil { return errors.WithStack(err) } @@ -322,23 +340,22 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, queue *Uniqu first := true for { var include string - cache.ExpectFile(sourcePath) + f.cache.ExpectFile(sourcePath) - includes := ctx.IncludeFolders + includes := f.ctx.IncludeFolders if library, ok := sourceFile.Origin.(*libraries.Library); ok && library.UtilityDir != nil { includes = append(includes, library.UtilityDir) } var preproc_err error var preproc_stderr []byte - - if unchanged && cache.valid { - include = cache.Next().Include - if first && ctx.Verbose { - ctx.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath)) + if unchanged && f.cache.valid { + include = f.cache.Next().Include + if first && f.ctx.Verbose { + f.ctx.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath)) } } else { - preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes) + preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, includes) // Unwrap error and see if it is an ExitError. _, is_exit_error := errors.Cause(preproc_err).(*exec.ExitError) if preproc_err == nil { @@ -351,26 +368,26 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, queue *Uniqu return errors.WithStack(preproc_err) } else { include = IncludesFinderWithRegExp(string(preproc_stderr)) - if include == "" && ctx.Verbose { - ctx.Info(tr("Error while detecting libraries included by %[1]s", sourcePath)) + if include == "" && f.ctx.Verbose { + f.ctx.Info(tr("Error while detecting libraries included by %[1]s", sourcePath)) } } } if include == "" { // No missing includes found, we're done - cache.ExpectEntry(sourcePath, "", nil) + f.cache.ExpectEntry(sourcePath, "", nil) return nil } - library := ResolveLibrary(ctx, include) + library := ResolveLibrary(f.ctx, include) if library == nil { // Library could not be resolved, show error // err := runCommand(ctx, &GCCPreprocRunner{SourceFilePath: sourcePath, TargetFileName: paths.New(constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E), Includes: includes}) // return errors.WithStack(err) if preproc_err == nil || preproc_stderr == nil { // Filename came from cache, so run preprocessor to obtain error to show - preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes) + preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, includes) if preproc_err == nil { // If there is a missing #include in the cache, but running // gcc does not reproduce that, there is something wrong. @@ -379,32 +396,32 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, queue *Uniqu return errors.New(tr("Internal error in cache")) } } - ctx.Stderr.Write(preproc_stderr) + f.ctx.Stderr.Write(preproc_stderr) return errors.WithStack(preproc_err) } // Add this library to the list of libraries, the // include path and queue its source files for further // include scanning - ctx.ImportedLibraries = append(ctx.ImportedLibraries, library) - appendIncludeFolder(ctx, cache, sourcePath, include, library.SourceDir) + f.ctx.ImportedLibraries = append(f.ctx.ImportedLibraries, library) + f.appendIncludeFolder(sourcePath, include, library.SourceDir) sourceDirs := library.SourceDirs() for _, sourceDir := range sourceDirs { 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)) + if f.ctx.Verbose { + f.ctx.Info(tr("Skipping dependencies detection for precompiled library %[1]s", library.Name)) } } else { - queueSourceFilesFromFolder(ctx, queue, library, sourceDir.Dir, sourceDir.Recurse) + f.queueSourceFilesFromFolder(library, sourceDir.Dir, sourceDir.Recurse) } } first = false } } -func queueSourceFilesFromFolder(ctx *types.Context, queue *UniqueSourceFileQueue, origin interface{}, folder *paths.Path, recurse bool) error { +func (f *CppIncludesFinder) queueSourceFilesFromFolder(origin interface{}, folder *paths.Path, recurse bool) error { extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } filePaths := []string{} @@ -414,11 +431,11 @@ func queueSourceFilesFromFolder(ctx *types.Context, queue *UniqueSourceFileQueue } for _, filePath := range filePaths { - sourceFile, err := MakeSourceFilee(ctx, origin, paths.New(filePath)) + sourceFile, err := MakeSourceFile(f.ctx, origin, paths.New(filePath)) if err != nil { return errors.WithStack(err) } - queue.Push(sourceFile) + f.queue.Push(sourceFile) } return nil @@ -434,7 +451,7 @@ 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 MakeSourceFilee(ctx *types.Context, origin interface{}, path *paths.Path) (SourceFile, error) { +func MakeSourceFile(ctx *types.Context, origin interface{}, path *paths.Path) (SourceFile, error) { if path.IsAbs() { var err error path, err = sourceRoot(ctx, origin).RelTo(path) From 319f47a85d2964e5aceaed02172afac2d0a7b7ab Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 17 Oct 2019 19:45:08 +0200 Subject: [PATCH 07/24] Replaced SourceFile.Origin field with SourceFile.Library Previously the Origin interface{} field was used to store the "origin" of the source file (being sketch or library). Actually Origin is really needed only to reconstruct BuildPath and RootPath when the origin is a Library, in all the other cases we can reconstruct BuildPath and RootPath looking inside Context. With the above in mind, this commit removes the non-idiomatic interface{} by replacing it with a Library, so now the meaning of the SourceFile.Library field is "the library where the source file is contained or nil if not part of a library". --- legacy/builder/container_find_includes.go | 53 ++++++++++------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 3d950467e91..44fc69c6804 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -155,16 +155,16 @@ func (f *CppIncludesFinder) DetectLibraries() error { f.appendIncludeFolder(nil, "", f.ctx.BuildProperties.GetPath("build.variant.path")) } - mergedfile, err := MakeSourceFile(f.ctx, f.sketch, paths.New(f.sketch.MainFile.Base()+".cpp")) + mergedfile, err := MakeSourceFile(f.ctx, nil, paths.New(f.sketch.MainFile.Base()+".cpp")) if err != nil { return errors.WithStack(err) } f.queue.Push(mergedfile) - f.queueSourceFilesFromFolder(f.sketch, f.ctx.SketchBuildPath, false /* recurse */) + f.queueSourceFilesFromFolder(nil, f.ctx.SketchBuildPath, false /* recurse */) srcSubfolderPath := f.ctx.SketchBuildPath.Join("src") if srcSubfolderPath.IsDir() { - f.queueSourceFilesFromFolder(f.sketch, srcSubfolderPath, true /* recurse */) + f.queueSourceFilesFromFolder(nil, srcSubfolderPath, true /* recurse */) } for !f.queue.Empty() { @@ -343,8 +343,8 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { f.cache.ExpectFile(sourcePath) includes := f.ctx.IncludeFolders - if library, ok := sourceFile.Origin.(*libraries.Library); ok && library.UtilityDir != nil { - includes = append(includes, library.UtilityDir) + if sourceFile.Library != nil && sourceFile.Library.UtilityDir != nil { + includes = append(includes, sourceFile.Library.UtilityDir) } var preproc_err error @@ -421,7 +421,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { } } -func (f *CppIncludesFinder) queueSourceFilesFromFolder(origin interface{}, folder *paths.Path, recurse bool) error { +func (f *CppIncludesFinder) queueSourceFilesFromFolder(lib *libraries.Library, folder *paths.Path, recurse bool) error { extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } filePaths := []string{} @@ -431,7 +431,7 @@ func (f *CppIncludesFinder) queueSourceFilesFromFolder(origin interface{}, folde } for _, filePath := range filePaths { - sourceFile, err := MakeSourceFile(f.ctx, origin, paths.New(filePath)) + sourceFile, err := MakeSourceFile(f.ctx, lib, paths.New(filePath)) if err != nil { return errors.WithStack(err) } @@ -442,8 +442,9 @@ func (f *CppIncludesFinder) queueSourceFilesFromFolder(origin interface{}, folde } type SourceFile struct { - // Sketch or Library pointer that this source file lives in - Origin interface{} + // Library pointer that this source file lives in or nil if not part of a library + Library *libraries.Library + // Path to the source file within the sketch/library root folder RelativePath *paths.Path } @@ -451,55 +452,47 @@ 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 *types.Context, origin interface{}, path *paths.Path) (SourceFile, error) { +func MakeSourceFile(ctx *types.Context, lib *libraries.Library, path *paths.Path) (SourceFile, error) { if path.IsAbs() { var err error - path, err = sourceRoot(ctx, origin).RelTo(path) + path, err = sourceRoot(ctx, lib).RelTo(path) if err != nil { return SourceFile{}, err } } - return SourceFile{Origin: origin, RelativePath: path}, nil + return SourceFile{Library: lib, RelativePath: path}, nil } // 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 *types.Context, origin interface{}) *paths.Path { - switch o := origin.(type) { - case *sketch.Sketch: +func buildRoot(ctx *types.Context, lib *libraries.Library) *paths.Path { + if lib == nil { return ctx.SketchBuildPath - case *libraries.Library: - return ctx.LibrariesBuildPath.Join(o.Name) - default: - panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin)) } + return ctx.LibrariesBuildPath.Join(lib.Name) } // 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 *types.Context, origin interface{}) *paths.Path { - switch o := origin.(type) { - case *sketch.Sketch: +func sourceRoot(ctx *types.Context, lib *libraries.Library) *paths.Path { + if lib == nil { return ctx.SketchBuildPath - case *libraries.Library: - return o.SourceDir - default: - panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin)) } + return lib.SourceDir } func (f *SourceFile) SourcePath(ctx *types.Context) *paths.Path { - return sourceRoot(ctx, f.Origin).JoinPath(f.RelativePath) + return sourceRoot(ctx, f.Library).JoinPath(f.RelativePath) } func (f *SourceFile) ObjectPath(ctx *types.Context) *paths.Path { - return buildRoot(ctx, f.Origin).Join(f.RelativePath.String() + ".o") + return buildRoot(ctx, f.Library).Join(f.RelativePath.String() + ".o") } func (f *SourceFile) DepfilePath(ctx *types.Context) *paths.Path { - return buildRoot(ctx, f.Origin).Join(f.RelativePath.String() + ".d") + return buildRoot(ctx, f.Library).Join(f.RelativePath.String() + ".d") } type UniqueSourceFileQueue struct { @@ -529,7 +522,7 @@ func (q *UniqueSourceFileQueue) Empty() bool { func (q *UniqueSourceFileQueue) Contains(target SourceFile) bool { for _, elem := range q.queue { - if elem.Origin == target.Origin && elem.RelativePath.EqualsTo(target.RelativePath) { + if elem.Library == target.Library && elem.RelativePath.EqualsTo(target.RelativePath) { return true } } From d0bfe13bbaf9bf23fba6fc624ac0a5ec92a82991 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Oct 2019 15:12:07 +0200 Subject: [PATCH 08/24] Add library "Utility" folder in include paths together with library I don't know the reason why it was not placed here already, the git log doesn't give much hints. Anyway this change prepares for the next commit where the Library field is being removed from SourceFile struct. --- legacy/builder/container_find_includes.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 44fc69c6804..6722833e791 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -343,9 +343,6 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { f.cache.ExpectFile(sourcePath) includes := f.ctx.IncludeFolders - if sourceFile.Library != nil && sourceFile.Library.UtilityDir != nil { - includes = append(includes, sourceFile.Library.UtilityDir) - } var preproc_err error var preproc_stderr []byte @@ -405,6 +402,10 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { // include scanning f.ctx.ImportedLibraries = append(f.ctx.ImportedLibraries, library) f.appendIncludeFolder(sourcePath, include, library.SourceDir) + if library.UtilityDir != nil { + // TODO: Use library.SourceDirs() instead? + includes = append(includes, library.UtilityDir) + } sourceDirs := library.SourceDirs() for _, sourceDir := range sourceDirs { if library.Precompiled && library.PrecompiledWithSources { From 648a6b92fd25926854a575e940be21d6ce89383c Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Oct 2019 15:20:19 +0200 Subject: [PATCH 09/24] Some variable rename to make linter happy --- legacy/builder/container_find_includes.go | 26 +++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 6722833e791..415b186192c 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -344,27 +344,27 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { includes := f.ctx.IncludeFolders - var preproc_err error - var preproc_stderr []byte + var preprocErr error + var preprocStderr []byte if unchanged && f.cache.valid { include = f.cache.Next().Include if first && f.ctx.Verbose { f.ctx.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath)) } } else { - preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, includes) + preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, includes) // Unwrap error and see if it is an ExitError. - _, is_exit_error := errors.Cause(preproc_err).(*exec.ExitError) - if preproc_err == nil { + _, isExitError := errors.Cause(preprocErr).(*exec.ExitError) + if preprocErr == nil { // Preprocessor successful, done include = "" - } else if !is_exit_error || preproc_stderr == nil { + } else if !isExitError || 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 == "" && f.ctx.Verbose { f.ctx.Info(tr("Error while detecting libraries included by %[1]s", sourcePath)) } @@ -382,10 +382,10 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { // Library could not be resolved, show error // err := runCommand(ctx, &GCCPreprocRunner{SourceFilePath: sourcePath, TargetFileName: paths.New(constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E), Includes: includes}) // return errors.WithStack(err) - if preproc_err == nil || preproc_stderr == nil { + if preprocErr == nil || preprocStderr == nil { // Filename came from cache, so run preprocessor to obtain error to show - preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, includes) - if preproc_err == nil { + preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, includes) + 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 @@ -393,8 +393,8 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { return errors.New(tr("Internal error in cache")) } } - f.ctx.Stderr.Write(preproc_stderr) - return errors.WithStack(preproc_err) + f.ctx.Stderr.Write(preprocStderr) + return errors.WithStack(preprocErr) } // Add this library to the list of libraries, the From 6525495f37ee331cc9015041ad3c13c7f1e4d8f9 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Oct 2019 17:23:24 +0200 Subject: [PATCH 10/24] Removed local variable --- legacy/builder/container_find_includes.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 415b186192c..95e5d0ef00d 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -342,8 +342,6 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { var include string f.cache.ExpectFile(sourcePath) - includes := f.ctx.IncludeFolders - var preprocErr error var preprocStderr []byte if unchanged && f.cache.valid { @@ -352,7 +350,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { f.ctx.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath)) } } else { - preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, includes) + preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, f.ctx.IncludeFolders) // Unwrap error and see if it is an ExitError. _, isExitError := errors.Cause(preprocErr).(*exec.ExitError) if preprocErr == nil { @@ -384,7 +382,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { // return errors.WithStack(err) if preprocErr == nil || preprocStderr == nil { // Filename came from cache, so run preprocessor to obtain error to show - preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, includes) + preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, f.ctx.IncludeFolders) if preprocErr == nil { // If there is a missing #include in the cache, but running // gcc does not reproduce that, there is something wrong. @@ -404,7 +402,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { f.appendIncludeFolder(sourcePath, include, library.SourceDir) if library.UtilityDir != nil { // TODO: Use library.SourceDirs() instead? - includes = append(includes, library.UtilityDir) + f.ctx.IncludeFolders = append(f.ctx.IncludeFolders, library.UtilityDir) } sourceDirs := library.SourceDirs() for _, sourceDir := range sourceDirs { From 5acd9426a5807a538bb964b6ebff5e7b4ccde6a8 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Oct 2019 17:56:25 +0200 Subject: [PATCH 11/24] Added debugging prints for SourceFile structs --- legacy/builder/container_find_includes.go | 17 +++++++++++++++-- .../test/includes_to_include_folders_test.go | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 95e5d0ef00d..26e579b18e9 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -105,6 +105,7 @@ import ( "github.com/arduino/arduino-cli/legacy/builder/utils" "github.com/arduino/go-paths-helper" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) type ContainerFindIncludes struct{} @@ -143,12 +144,14 @@ type CppIncludesFinder struct { cache *includeCache sketch *sketch.Sketch queue *UniqueSourceFileQueue + log *logrus.Entry } func (f *CppIncludesFinder) DetectLibraries() error { f.cache = loadCacheFrom(f.ctx.BuildPath.Join("includes.cache")) f.sketch = f.ctx.Sketch f.queue = &UniqueSourceFileQueue{} + f.log = logrus.WithField("task", "DetectingLibraries") f.appendIncludeFolder(nil, "", f.ctx.BuildProperties.GetPath("build.core.path")) if f.ctx.BuildProperties.Get("build.variant.path") != "" { @@ -159,6 +162,7 @@ func (f *CppIncludesFinder) DetectLibraries() error { if err != nil { return errors.WithStack(err) } + f.log.Debugf("Queueing merged sketch: %s", mergedfile) f.queue.Push(mergedfile) f.queueSourceFilesFromFolder(nil, f.ctx.SketchBuildPath, false /* recurse */) @@ -188,6 +192,7 @@ func (f *CppIncludesFinder) DetectLibraries() error { // and should be the empty string for the default include folders, like // the core or variant. func (f *CppIncludesFinder) appendIncludeFolder(sourceFilePath *paths.Path, include string, folder *paths.Path) { + f.log.Debugf("Using include folder: %s", folder) f.ctx.IncludeFolders = append(f.ctx.IncludeFolders, folder) f.cache.ExpectEntry(sourceFilePath, include, folder) } @@ -422,7 +427,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { func (f *CppIncludesFinder) queueSourceFilesFromFolder(lib *libraries.Library, folder *paths.Path, recurse bool) error { extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } - + f.log.Debugf(" Queueing source files from %s (recurse %v)", folder, recurse) filePaths := []string{} err := utils.FindFilesInFolder(&filePaths, folder.String(), extensions, recurse) if err != nil { @@ -434,6 +439,7 @@ func (f *CppIncludesFinder) queueSourceFilesFromFolder(lib *libraries.Library, f if err != nil { return errors.WithStack(err) } + f.log.Debugf(" Queuing %s", sourceFile) f.queue.Push(sourceFile) } @@ -446,6 +452,13 @@ type SourceFile struct { // Path to the source file within the sketch/library root folder RelativePath *paths.Path + + ctx *types.Context +} + +func (f SourceFile) String() string { + return fmt.Sprintf("Root: %s - Path: %s - BuildPath: %s", + sourceRoot(f.ctx, f.Library), f.RelativePath, buildRoot(f.ctx, f.Library)) } // Create a SourceFile containing the given source file path within the @@ -459,7 +472,7 @@ func MakeSourceFile(ctx *types.Context, lib *libraries.Library, path *paths.Path return SourceFile{}, err } } - return SourceFile{Library: lib, RelativePath: path}, nil + return SourceFile{Library: lib, RelativePath: path, ctx: ctx}, nil } // Return the build root for the given origin, where build products will diff --git a/legacy/builder/test/includes_to_include_folders_test.go b/legacy/builder/test/includes_to_include_folders_test.go index 86836bd5c49..8fc5878e82f 100644 --- a/legacy/builder/test/includes_to_include_folders_test.go +++ b/legacy/builder/test/includes_to_include_folders_test.go @@ -23,6 +23,7 @@ import ( "github.com/arduino/arduino-cli/legacy/builder" "github.com/arduino/arduino-cli/legacy/builder/types" paths "github.com/arduino/go-paths-helper" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" ) @@ -297,6 +298,7 @@ func TestIncludesToIncludeFoldersSubfolders(t *testing.T) { Verbose: true, } + logrus.SetLevel(logrus.DebugLevel) buildPath := SetupBuildPath(t, ctx) defer buildPath.RemoveAll() From 1da2a07c59ec8bd9579b225035e600525626372f Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Oct 2019 21:32:11 +0200 Subject: [PATCH 12/24] Removed reference to Library in SourceFile struct --- legacy/builder/container_find_includes.go | 88 +++++++++-------------- 1 file changed, 35 insertions(+), 53 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 26e579b18e9..2210c3cbcd0 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -98,7 +98,6 @@ import ( "os/exec" "time" - "github.com/arduino/arduino-cli/arduino/libraries" "github.com/arduino/arduino-cli/arduino/sketch" "github.com/arduino/arduino-cli/legacy/builder/builder_utils" "github.com/arduino/arduino-cli/legacy/builder/types" @@ -158,17 +157,17 @@ func (f *CppIncludesFinder) DetectLibraries() error { f.appendIncludeFolder(nil, "", f.ctx.BuildProperties.GetPath("build.variant.path")) } - mergedfile, err := MakeSourceFile(f.ctx, nil, paths.New(f.sketch.MainFile.Base()+".cpp")) + mergedfile, err := MakeSourceFile(f.ctx.SketchBuildPath, f.ctx.SketchBuildPath, paths.New(f.sketch.MainFile.Base()+".cpp")) if err != nil { return errors.WithStack(err) } f.log.Debugf("Queueing merged sketch: %s", mergedfile) f.queue.Push(mergedfile) - f.queueSourceFilesFromFolder(nil, f.ctx.SketchBuildPath, false /* recurse */) + f.queueSourceFilesFromFolder(f.ctx.SketchBuildPath, f.ctx.SketchBuildPath, false /* recurse */) srcSubfolderPath := f.ctx.SketchBuildPath.Join("src") if srcSubfolderPath.IsDir() { - f.queueSourceFilesFromFolder(nil, srcSubfolderPath, true /* recurse */) + f.queueSourceFilesFromFolder(srcSubfolderPath, f.ctx.SketchBuildPath, true /* recurse */) } for !f.queue.Empty() { @@ -320,10 +319,10 @@ func (cache *includeCache) WriteToFile() error { } func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { - sourcePath := sourceFile.SourcePath(f.ctx) + sourcePath := sourceFile.SourcePath() targetFilePath := paths.NullPath() - depPath := sourceFile.DepfilePath(f.ctx) - objPath := sourceFile.ObjectPath(f.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 @@ -344,11 +343,11 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { first := true for { - var include string f.cache.ExpectFile(sourcePath) var preprocErr error var preprocStderr []byte + var include string if unchanged && f.cache.valid { include = f.cache.Next().Include if first && f.ctx.Verbose { @@ -410,6 +409,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { f.ctx.IncludeFolders = append(f.ctx.IncludeFolders, library.UtilityDir) } sourceDirs := library.SourceDirs() + buildDir := f.ctx.LibrariesBuildPath.Join(library.Name) for _, sourceDir := range sourceDirs { if library.Precompiled && library.PrecompiledWithSources { // Fully precompiled libraries should have no dependencies @@ -418,24 +418,24 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { f.ctx.Info(tr("Skipping dependencies detection for precompiled library %[1]s", library.Name)) } } else { - f.queueSourceFilesFromFolder(library, sourceDir.Dir, sourceDir.Recurse) + f.queueSourceFilesFromFolder(buildDir, sourceDir.Dir, sourceDir.Recurse) } } first = false } } -func (f *CppIncludesFinder) queueSourceFilesFromFolder(lib *libraries.Library, folder *paths.Path, recurse bool) error { +func (f *CppIncludesFinder) queueSourceFilesFromFolder(srcDir, buildDir *paths.Path, recurse bool) error { extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } - f.log.Debugf(" Queueing source files from %s (recurse %v)", folder, recurse) + f.log.Debugf(" Queueing source files from %s (recurse %v)", srcDir, recurse) filePaths := []string{} - err := utils.FindFilesInFolder(&filePaths, folder.String(), extensions, recurse) + err := utils.FindFilesInFolder(&filePaths, srcDir.String(), extensions, recurse) if err != nil { return errors.WithStack(err) } for _, filePath := range filePaths { - sourceFile, err := MakeSourceFile(f.ctx, lib, paths.New(filePath)) + sourceFile, err := MakeSourceFile(srcDir, buildDir, paths.New(filePath)) if err != nil { return errors.WithStack(err) } @@ -447,64 +447,44 @@ func (f *CppIncludesFinder) queueSourceFilesFromFolder(lib *libraries.Library, f } type SourceFile struct { - // Library pointer that this source file lives in or nil if not part of a library - Library *libraries.Library + // SourceRoot is the path to the source code root directory + SourceRoot *paths.Path + + // BuildRoot is the path to the build root directory + BuildRoot *paths.Path // Path to the source file within the sketch/library root folder RelativePath *paths.Path - - ctx *types.Context } func (f SourceFile) String() string { return fmt.Sprintf("Root: %s - Path: %s - BuildPath: %s", - sourceRoot(f.ctx, f.Library), f.RelativePath, buildRoot(f.ctx, f.Library)) + f.SourceRoot, f.RelativePath, f.BuildRoot) } -// 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 *types.Context, lib *libraries.Library, path *paths.Path) (SourceFile, error) { +// MakeSourceFile creates a SourceFile containing the given source file path +// within the given sourceRoot. +func MakeSourceFile(sourceRoot, buildRoot, path *paths.Path) (SourceFile, error) { if path.IsAbs() { - var err error - path, err = sourceRoot(ctx, lib).RelTo(path) - if err != nil { + if relPath, err := sourceRoot.RelTo(path); err == nil { + path = relPath + } else { return SourceFile{}, err } } - return SourceFile{Library: lib, RelativePath: path, ctx: ctx}, nil -} - -// 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 *types.Context, lib *libraries.Library) *paths.Path { - if lib == nil { - return ctx.SketchBuildPath - } - return ctx.LibrariesBuildPath.Join(lib.Name) -} - -// 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 *types.Context, lib *libraries.Library) *paths.Path { - if lib == nil { - return ctx.SketchBuildPath - } - return lib.SourceDir + return SourceFile{SourceRoot: sourceRoot, BuildRoot: buildRoot, RelativePath: path}, nil } -func (f *SourceFile) SourcePath(ctx *types.Context) *paths.Path { - return sourceRoot(ctx, f.Library).JoinPath(f.RelativePath) +func (f *SourceFile) SourcePath() *paths.Path { + return f.SourceRoot.JoinPath(f.RelativePath) } -func (f *SourceFile) ObjectPath(ctx *types.Context) *paths.Path { - return buildRoot(ctx, f.Library).Join(f.RelativePath.String() + ".o") +func (f *SourceFile) ObjectPath() *paths.Path { + return f.BuildRoot.Join(f.RelativePath.String() + ".o") } -func (f *SourceFile) DepfilePath(ctx *types.Context) *paths.Path { - return buildRoot(ctx, f.Library).Join(f.RelativePath.String() + ".d") +func (f *SourceFile) DepfilePath() *paths.Path { + return f.BuildRoot.Join(f.RelativePath.String() + ".d") } type UniqueSourceFileQueue struct { @@ -534,7 +514,9 @@ func (q *UniqueSourceFileQueue) Empty() bool { func (q *UniqueSourceFileQueue) Contains(target SourceFile) bool { for _, elem := range q.queue { - if elem.Library == target.Library && elem.RelativePath.EqualsTo(target.RelativePath) { + if elem.BuildRoot.EqualsTo(target.BuildRoot) && + elem.SourceRoot.EqualsTo(target.SourceRoot) && + elem.RelativePath.EqualsTo(target.RelativePath) { return true } } From 06c66e7d5a87ab1025008437a6f9d7c19ad75e13 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Oct 2019 21:45:01 +0200 Subject: [PATCH 13/24] Added some documentation to SourceFile struct and methods --- legacy/builder/container_find_includes.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 2210c3cbcd0..06ee8a99571 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -446,6 +446,8 @@ func (f *CppIncludesFinder) queueSourceFilesFromFolder(srcDir, buildDir *paths.P return nil } +// SourceFile represent a source file, it keeps a reference to the root source +// directory and the corresponding build root directory. type SourceFile struct { // SourceRoot is the path to the source code root directory SourceRoot *paths.Path @@ -463,26 +465,30 @@ func (f SourceFile) String() string { } // MakeSourceFile creates a SourceFile containing the given source file path -// within the given sourceRoot. -func MakeSourceFile(sourceRoot, buildRoot, path *paths.Path) (SourceFile, error) { - if path.IsAbs() { - if relPath, err := sourceRoot.RelTo(path); err == nil { - path = relPath +// within the given sourceRoot. If srcPath is absolute it is transformed to +// relative to sourceRoot. +func MakeSourceFile(sourceRoot, buildRoot, srcPath *paths.Path) (SourceFile, error) { + if srcPath.IsAbs() { + if relPath, err := sourceRoot.RelTo(srcPath); err == nil { + srcPath = relPath } else { return SourceFile{}, err } } - return SourceFile{SourceRoot: sourceRoot, BuildRoot: buildRoot, RelativePath: path}, nil + return SourceFile{SourceRoot: sourceRoot, BuildRoot: buildRoot, RelativePath: srcPath}, nil } +// SourcePath returns the path to the source file func (f *SourceFile) SourcePath() *paths.Path { return f.SourceRoot.JoinPath(f.RelativePath) } +// ObjectPath returns the path to the object file (.o) func (f *SourceFile) ObjectPath() *paths.Path { return f.BuildRoot.Join(f.RelativePath.String() + ".o") } +// DepfilePath returns the path to the dependencies file (.d) func (f *SourceFile) DepfilePath() *paths.Path { return f.BuildRoot.Join(f.RelativePath.String() + ".d") } From c388af52d977344858a53268be0997b1c91c6d81 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Oct 2019 21:48:52 +0200 Subject: [PATCH 14/24] Added SourceFile.Equals() and converted all SourceFile object to pointers --- legacy/builder/container_find_includes.go | 30 ++++++++++++++--------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 06ee8a99571..468c8ed52b1 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -318,7 +318,7 @@ func (cache *includeCache) WriteToFile() error { return nil } -func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile SourceFile) error { +func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error { sourcePath := sourceFile.SourcePath() targetFilePath := paths.NullPath() depPath := sourceFile.DepfilePath() @@ -459,7 +459,7 @@ type SourceFile struct { RelativePath *paths.Path } -func (f SourceFile) String() string { +func (f *SourceFile) String() string { return fmt.Sprintf("Root: %s - Path: %s - BuildPath: %s", f.SourceRoot, f.RelativePath, f.BuildRoot) } @@ -467,15 +467,15 @@ func (f SourceFile) String() string { // MakeSourceFile creates a SourceFile containing the given source file path // within the given sourceRoot. If srcPath is absolute it is transformed to // relative to sourceRoot. -func MakeSourceFile(sourceRoot, buildRoot, srcPath *paths.Path) (SourceFile, error) { +func MakeSourceFile(sourceRoot, buildRoot, srcPath *paths.Path) (*SourceFile, error) { if srcPath.IsAbs() { if relPath, err := sourceRoot.RelTo(srcPath); err == nil { srcPath = relPath } else { - return SourceFile{}, err + return nil, err } } - return SourceFile{SourceRoot: sourceRoot, BuildRoot: buildRoot, RelativePath: srcPath}, nil + return &SourceFile{SourceRoot: sourceRoot, BuildRoot: buildRoot, RelativePath: srcPath}, nil } // SourcePath returns the path to the source file @@ -493,8 +493,16 @@ func (f *SourceFile) DepfilePath() *paths.Path { return f.BuildRoot.Join(f.RelativePath.String() + ".d") } +// Equals return true if the SourceFile equals to the SourceFile +// passed as parameter +func (f *SourceFile) Equals(other *SourceFile) bool { + return f.BuildRoot.EqualsTo(other.BuildRoot) && + f.SourceRoot.EqualsTo(other.SourceRoot) && + f.RelativePath.EqualsTo(other.RelativePath) +} + type UniqueSourceFileQueue struct { - queue []SourceFile + queue []*SourceFile curr int } @@ -502,13 +510,13 @@ func (q *UniqueSourceFileQueue) Len() int { return len(q.queue) - q.curr } -func (q *UniqueSourceFileQueue) Push(value SourceFile) { +func (q *UniqueSourceFileQueue) Push(value *SourceFile) { if !q.Contains(value) { q.queue = append(q.queue, value) } } -func (q *UniqueSourceFileQueue) Pop() SourceFile { +func (q *UniqueSourceFileQueue) Pop() *SourceFile { res := q.queue[q.curr] q.curr++ return res @@ -518,11 +526,9 @@ func (q *UniqueSourceFileQueue) Empty() bool { return q.Len() == 0 } -func (q *UniqueSourceFileQueue) Contains(target SourceFile) bool { +func (q *UniqueSourceFileQueue) Contains(target *SourceFile) bool { for _, elem := range q.queue { - if elem.BuildRoot.EqualsTo(target.BuildRoot) && - elem.SourceRoot.EqualsTo(target.SourceRoot) && - elem.RelativePath.EqualsTo(target.RelativePath) { + if elem.Equals(target) { return true } } From b3b262b85119fb16a55f78a869a1d8999dc68573 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Oct 2019 22:18:03 +0200 Subject: [PATCH 15/24] Added documentation to UniqueSourceFileQueue and fixed Pop method --- legacy/builder/container_find_includes.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 468c8ed52b1..df8c0381759 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -501,31 +501,42 @@ func (f *SourceFile) Equals(other *SourceFile) bool { f.RelativePath.EqualsTo(other.RelativePath) } +// UniqueSourceFileQueue is a queue of SourceFile. A SourceFile +// can be pushed in the queue only once. type UniqueSourceFileQueue struct { queue []*SourceFile curr int } +// Len returns the number of element waiting in the queue func (q *UniqueSourceFileQueue) Len() int { return len(q.queue) - q.curr } +// Push insert a new element in the queue func (q *UniqueSourceFileQueue) Push(value *SourceFile) { if !q.Contains(value) { q.queue = append(q.queue, value) } } +// Pop return the first element in the queue or nil if the queue is empty func (q *UniqueSourceFileQueue) Pop() *SourceFile { + if q.Empty() { + return nil + } res := q.queue[q.curr] q.curr++ return res } +// Empty returns true if the queue is empty func (q *UniqueSourceFileQueue) Empty() bool { return q.Len() == 0 } +// Contains return true if the target elemen has been already added +// in the queue (even if the element has been alread popped out) func (q *UniqueSourceFileQueue) Contains(target *SourceFile) bool { for _, elem := range q.queue { if elem.Equals(target) { From 9699a1b756ab7a9f786309f514b474e6dd7a4940 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 21 Oct 2019 23:56:30 +0200 Subject: [PATCH 16/24] FindFilesInFolder now returns the result instead of changine the reference --- legacy/builder/container_find_includes.go | 3 +-- legacy/builder/create_cmake_rule.go | 12 ++++-------- legacy/builder/utils/utils.go | 8 +++++--- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index df8c0381759..5f6b8f688d5 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -428,8 +428,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error func (f *CppIncludesFinder) queueSourceFilesFromFolder(srcDir, buildDir *paths.Path, recurse bool) error { extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } f.log.Debugf(" Queueing source files from %s (recurse %v)", srcDir, recurse) - filePaths := []string{} - err := utils.FindFilesInFolder(&filePaths, srcDir.String(), extensions, recurse) + filePaths, err := utils.FindFilesInFolder(srcDir.String(), extensions, recurse) if err != nil { return errors.WithStack(err) } diff --git a/legacy/builder/create_cmake_rule.go b/legacy/builder/create_cmake_rule.go index 9e8f499605c..bd1e588839c 100644 --- a/legacy/builder/create_cmake_rule.go +++ b/legacy/builder/create_cmake_rule.go @@ -89,8 +89,7 @@ func (s *ExportProjectCMake) Run(ctx *types.Context) error { } // Remove stray folders contining incompatible or not needed libraries archives - var files []string - utils.FindFilesInFolder(&files, libDir.Join("src").String(), staticLibsExtensions, true) + files, _ := utils.FindFilesInFolder(libDir.Join("src").String(), staticLibsExtensions, true) for _, file := range files { staticLibDir := filepath.Dir(file) if !isStaticLib || !strings.Contains(staticLibDir, mcu) { @@ -126,8 +125,7 @@ func (s *ExportProjectCMake) Run(ctx *types.Context) error { } // remove "#line 1 ..." from exported c_make folder sketch - var sketchFiles []string - utils.FindFilesInFolder(&sketchFiles, cmakeFolder.Join("sketch").String(), extensions, false) + sketchFiles, _ := utils.FindFilesInFolder(cmakeFolder.Join("sketch").String(), extensions, false) for _, file := range sketchFiles { input, err := ioutil.ReadFile(file) @@ -161,14 +159,12 @@ func (s *ExportProjectCMake) Run(ctx *types.Context) error { extractCompileFlags(ctx, constants.RECIPE_CPP_PATTERN, &defines, &dynamicLibsFromGccMinusL, &linkerflags, &linkDirectories) // Extract folders with .h in them for adding in include list - var headerFiles []string isHeader := func(ext string) bool { return DOTHEXTENSION[ext] } - utils.FindFilesInFolder(&headerFiles, cmakeFolder.String(), isHeader, true) + headerFiles, _ := utils.FindFilesInFolder(cmakeFolder.String(), isHeader, true) foldersContainingDotH := findUniqueFoldersRelative(headerFiles, cmakeFolder.String()) // Extract folders with .a in them for adding in static libs paths list - var staticLibs []string - utils.FindFilesInFolder(&staticLibs, cmakeFolder.String(), staticLibsExtensions, true) + staticLibs, _ := utils.FindFilesInFolder(cmakeFolder.String(), staticLibsExtensions, true) // Generate the CMakeLists global file diff --git a/legacy/builder/utils/utils.go b/legacy/builder/utils/utils.go index 2f60ff75719..da9653985f5 100644 --- a/legacy/builder/utils/utils.go +++ b/legacy/builder/utils/utils.go @@ -259,7 +259,8 @@ func FindAllSubdirectories(folder string, output *[]string) error { return gohasissues.Walk(folder, walkFunc) } -func FindFilesInFolder(files *[]string, folder string, extensions CheckExtensionFunc, recurse bool) error { +func FindFilesInFolder(folder string, extensions CheckExtensionFunc, recurse bool) ([]string, error) { + files := []string{} walkFunc := func(path string, info os.FileInfo, err error) error { if err != nil { return err @@ -295,10 +296,11 @@ func FindFilesInFolder(files *[]string, folder string, extensions CheckExtension } currentFile.Close() - *files = append(*files, path) + files = append(files, path) return nil } - return gohasissues.Walk(folder, walkFunc) + err := gohasissues.Walk(folder, walkFunc) + return files, err } func AppendIfNotPresent(target []string, elements ...string) []string { From 111d6ea87124f2c75aeecc66ed5e0d94ee1397f8 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 22 Oct 2019 10:37:44 +0200 Subject: [PATCH 17/24] Added includeCache.String() and includeCacheItem.String() --- legacy/builder/container_find_includes.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 5f6b8f688d5..23203833777 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -212,8 +212,13 @@ type includeCacheEntry struct { } func (entry *includeCacheEntry) String() string { - return fmt.Sprintf("SourceFile: %s; Include: %s; IncludePath: %s", - entry.Sourcefile, entry.Include, entry.Includepath) + if entry.Sourcefile == nil { + return fmt.Sprintf("Include path added: %s", entry.Includepath) + } + if entry.Include == "" { + return fmt.Sprintf("%s successfully compiled", entry.Sourcefile) + } + return fmt.Sprintf("%s requires %s: include path added %s", entry.Sourcefile, entry.Include, entry.Includepath) } func (entry *includeCacheEntry) Equals(other *includeCacheEntry) bool { @@ -318,6 +323,14 @@ func (cache *includeCache) WriteToFile() error { return nil } +func (cache *includeCache) String() string { + res := "[\n" + for _, entry := range cache.entries { + res = res + " " + entry.String() + "\n" + } + return res + "]" +} + func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error { sourcePath := sourceFile.SourcePath() targetFilePath := paths.NullPath() From 3323453c582fd6df704d6572d448bec885808eba Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 25 Oct 2019 11:21:54 +0200 Subject: [PATCH 18/24] fixed docs for includeCache; renamed some methods; added Invalidate() method --- legacy/builder/container_find_includes.go | 66 ++++++++++++----------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 23203833777..635f983831b 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -193,7 +193,7 @@ func (f *CppIncludesFinder) DetectLibraries() error { func (f *CppIncludesFinder) appendIncludeFolder(sourceFilePath *paths.Path, include string, folder *paths.Path) { f.log.Debugf("Using include folder: %s", folder) f.ctx.IncludeFolders = append(f.ctx.IncludeFolders, folder) - f.cache.ExpectEntry(sourceFilePath, include, folder) + f.cache.AddAndCheckEntry(sourceFilePath, include, folder) } func runCommand(ctx *types.Context, command types.Command) error { @@ -238,49 +238,53 @@ type includeCache struct { entries []*includeCacheEntry } -// Return the next cache entry. Should only be called when the cache is -// valid and a next entry is available (the latter can be checked with -// ExpectFile). Does not advance the cache. -func (cache *includeCache) Next() *includeCacheEntry { +// Peek returns the next cache entry if the cache is valid and the next +// entry exists, otherwise it returns nil. Does not advance the cache. +func (cache *includeCache) Peek() *includeCacheEntry { + if !cache.valid || cache.next >= len(cache.entries) { + return nil + } return cache.entries[cache.next] } -// Check that the next cache entry is about the given file. If it is -// not, or no entry is available, the cache is invalidated. Does not -// advance the cache. +// Invalidate invalidates the cache. +func (cache *includeCache) Invalidate() { + cache.valid = false + cache.entries = cache.entries[:cache.next] +} + +// ExpectFile check that the next cache entry is about the given file. +// If it is not, or no entry is available, the cache is invalidated. +// Does not advance the cache. func (cache *includeCache) ExpectFile(sourcefile *paths.Path) { - if cache.valid && (cache.next >= len(cache.entries) || !cache.Next().Sourcefile.EqualsTo(sourcefile)) { - cache.valid = false - cache.entries = cache.entries[:cache.next] + if next := cache.Peek(); next == nil || !next.Sourcefile.EqualsTo(sourcefile) { + cache.Invalidate() } } -// Check that the next entry matches the given values. If so, advance -// the cache. If not, the cache is invalidated. If the cache is -// invalidated, or was already invalid, an entry with the given values -// is appended. -func (cache *includeCache) ExpectEntry(sourcefile *paths.Path, include string, librarypath *paths.Path) { - entry := &includeCacheEntry{Sourcefile: sourcefile, Include: include, Includepath: librarypath} - if cache.valid { - if cache.next < len(cache.entries) && cache.Next().Equals(entry) { - cache.next++ - } else { - cache.valid = false - cache.entries = cache.entries[:cache.next] - } +// AddAndCheckEntry check that the next entry matches the given values. +// If so, advance the cache. If not, the cache is invalidated. If the +// cache is invalidated, or was already invalid, an entry with the given +// values is appended. +func (cache *includeCache) AddAndCheckEntry(sourcefile *paths.Path, include string, librarypath *paths.Path) { + expected := &includeCacheEntry{Sourcefile: sourcefile, Include: include, Includepath: librarypath} + if next := cache.Peek(); next == nil || !next.Equals(expected) { + cache.Invalidate() + } else { + cache.next++ } if !cache.valid { - cache.entries = append(cache.entries, entry) + cache.entries = append(cache.entries, expected) + cache.next++ } } -// Check that the cache is completely consumed. If not, the cache is -// invalidated. +// ExpectEnd check that the cache is completely consumed. If not, the +// cache is invalidated. func (cache *includeCache) ExpectEnd() { if cache.valid && cache.next < len(cache.entries) { - cache.valid = false - cache.entries = cache.entries[:cache.next] + cache.Invalidate() } } @@ -362,7 +366,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error var preprocStderr []byte var include string if unchanged && f.cache.valid { - include = f.cache.Next().Include + include = f.cache.Peek().Include if first && f.ctx.Verbose { f.ctx.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath)) } @@ -388,7 +392,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error if include == "" { // No missing includes found, we're done - f.cache.ExpectEntry(sourcePath, "", nil) + f.cache.AddAndCheckEntry(sourcePath, "", nil) return nil } From e1a1581203fb4807d15ca4d1ad8aaaa0b4c35131 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 25 Oct 2019 12:23:56 +0200 Subject: [PATCH 19/24] CppIncludesFinder: Removing dependency on ctx (part 1) This is a series of commit to make library detection context-agnostic. In this commit the IncludeFolders usage has been removed from the internal logic, a new field CppIncludesFinder.IncludeDirsFound has been made for this purpose. The new field is setup before by the caller before doing the actual discovery. --- legacy/builder/container_find_includes.go | 65 +++++++++++++---------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 635f983831b..cd0166df3f9 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -122,12 +122,15 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { } func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { - finder := &CppIncludesFinder{ - ctx: ctx, + finder := NewCppIncludesFinder(ctx) + finder.UseIncludeDir(ctx.BuildProperties.GetPath("build.core.path")) + if variantPath := ctx.BuildProperties.GetPath("build.variant.path"); variantPath != nil { + finder.UseIncludeDir(variantPath) } if err := finder.DetectLibraries(); err != nil { return err } + ctx.IncludeFolders.AddAllMissing(finder.IncludeDirsFound) if err := runCommand(ctx, &FailIfImportedLibraryIsWrong{}); err != nil { return errors.WithStack(err) } @@ -139,22 +142,30 @@ func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { // libraries used in a sketch and a way to cache this result for // increasing detection speed on already processed sketches. type CppIncludesFinder struct { - ctx *types.Context - cache *includeCache - sketch *sketch.Sketch - queue *UniqueSourceFileQueue - log *logrus.Entry + IncludeDirsFound paths.PathList + ctx *types.Context + cache *includeCache + sketch *sketch.Sketch + queue *UniqueSourceFileQueue + log *logrus.Entry +} + +// NewCppIncludesFinder create a new include +func NewCppIncludesFinder(ctx *types.Context) *CppIncludesFinder { + return &CppIncludesFinder{ + ctx: ctx, + cache: loadCacheFrom(ctx.BuildPath.Join("includes.cache")), + sketch: ctx.Sketch, + queue: &UniqueSourceFileQueue{}, + log: logrus.WithField("task", "DetectingLibraries"), + } } +// DetectLibraries runs a library detection algorithm func (f *CppIncludesFinder) DetectLibraries() error { - f.cache = loadCacheFrom(f.ctx.BuildPath.Join("includes.cache")) - f.sketch = f.ctx.Sketch - f.queue = &UniqueSourceFileQueue{} - f.log = logrus.WithField("task", "DetectingLibraries") - - f.appendIncludeFolder(nil, "", f.ctx.BuildProperties.GetPath("build.core.path")) - if f.ctx.BuildProperties.Get("build.variant.path") != "" { - f.appendIncludeFolder(nil, "", f.ctx.BuildProperties.GetPath("build.variant.path")) + for _, includeDir := range f.IncludeDirsFound { + f.log.Debugf("Using include directory: %s", includeDir) + f.cache.AddAndCheckEntry(nil, "", includeDir) } mergedfile, err := MakeSourceFile(f.ctx.SketchBuildPath, f.ctx.SketchBuildPath, paths.New(f.sketch.MainFile.Base()+".cpp")) @@ -185,15 +196,9 @@ func (f *CppIncludesFinder) DetectLibraries() error { return nil } -// Append the given folder to the include path and match or append it to -// the cache. sourceFilePath and include indicate the source of this -// include (e.g. what #include line in what file it was resolved from) -// and should be the empty string for the default include folders, like -// the core or variant. -func (f *CppIncludesFinder) appendIncludeFolder(sourceFilePath *paths.Path, include string, folder *paths.Path) { - f.log.Debugf("Using include folder: %s", folder) - f.ctx.IncludeFolders = append(f.ctx.IncludeFolders, folder) - f.cache.AddAndCheckEntry(sourceFilePath, include, folder) +// UseIncludeDir adds an include directory to the current library discovery +func (f *CppIncludesFinder) UseIncludeDir(includeDir *paths.Path) { + f.IncludeDirsFound.Add(includeDir) } func runCommand(ctx *types.Context, command types.Command) error { @@ -371,7 +376,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error f.ctx.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath)) } } else { - preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, f.ctx.IncludeFolders) + preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, f.IncludeDirsFound) // Unwrap error and see if it is an ExitError. _, isExitError := errors.Cause(preprocErr).(*exec.ExitError) if preprocErr == nil { @@ -403,7 +408,7 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error // return errors.WithStack(err) if preprocErr == nil || preprocStderr == nil { // Filename came from cache, so run preprocessor to obtain error to show - preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, f.ctx.IncludeFolders) + preprocStderr, preprocErr = GCCPreprocRunnerForDiscoveringIncludes(f.ctx, sourcePath, targetFilePath, f.IncludeDirsFound) if preprocErr == nil { // If there is a missing #include in the cache, but running // gcc does not reproduce that, there is something wrong. @@ -420,10 +425,14 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error // include path and queue its source files for further // include scanning f.ctx.ImportedLibraries = append(f.ctx.ImportedLibraries, library) - f.appendIncludeFolder(sourcePath, include, library.SourceDir) + + f.log.Debugf("Using library include folder: %s", library.SourceDir) + f.IncludeDirsFound.Add(library.SourceDir) + f.cache.AddAndCheckEntry(sourcePath, include, library.SourceDir) + if library.UtilityDir != nil { // TODO: Use library.SourceDirs() instead? - f.ctx.IncludeFolders = append(f.ctx.IncludeFolders, library.UtilityDir) + f.IncludeDirsFound.Add(library.UtilityDir) } sourceDirs := library.SourceDirs() buildDir := f.ctx.LibrariesBuildPath.Join(library.Name) From 4b0b58817ed56989fd1b13ddab3103ebba6e9442 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 25 Oct 2019 14:11:57 +0200 Subject: [PATCH 20/24] CppIncludesFinder: Removing dependency on ctx (part 2) --- legacy/builder/container_find_includes.go | 85 ++++++++++++----------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index cd0166df3f9..ded7cd6b691 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -98,7 +98,6 @@ import ( "os/exec" "time" - "github.com/arduino/arduino-cli/arduino/sketch" "github.com/arduino/arduino-cli/legacy/builder/builder_utils" "github.com/arduino/arduino-cli/legacy/builder/types" "github.com/arduino/arduino-cli/legacy/builder/utils" @@ -127,9 +126,16 @@ func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { if variantPath := ctx.BuildProperties.GetPath("build.variant.path"); variantPath != nil { finder.UseIncludeDir(variantPath) } + finder.AddSourceFile(ctx.SketchBuildPath, ctx.SketchBuildPath, paths.New(ctx.Sketch.MainFile.Base()+".cpp")) + finder.AddSourceDir(ctx.SketchBuildPath, ctx.SketchBuildPath, false /* recurse */) + if srcSubfolderPath := ctx.SketchBuildPath.Join("src"); srcSubfolderPath.IsDir() { + finder.AddSourceDir(srcSubfolderPath, ctx.SketchBuildPath, true /* recurse */) + } + if err := finder.DetectLibraries(); err != nil { return err } + ctx.IncludeFolders.AddAllMissing(finder.IncludeDirsFound) if err := runCommand(ctx, &FailIfImportedLibraryIsWrong{}); err != nil { return errors.WithStack(err) @@ -145,7 +151,6 @@ type CppIncludesFinder struct { IncludeDirsFound paths.PathList ctx *types.Context cache *includeCache - sketch *sketch.Sketch queue *UniqueSourceFileQueue log *logrus.Entry } @@ -153,11 +158,10 @@ type CppIncludesFinder struct { // NewCppIncludesFinder create a new include func NewCppIncludesFinder(ctx *types.Context) *CppIncludesFinder { return &CppIncludesFinder{ - ctx: ctx, - cache: loadCacheFrom(ctx.BuildPath.Join("includes.cache")), - sketch: ctx.Sketch, - queue: &UniqueSourceFileQueue{}, - log: logrus.WithField("task", "DetectingLibraries"), + ctx: ctx, + cache: loadCacheFrom(ctx.BuildPath.Join("includes.cache")), + queue: &UniqueSourceFileQueue{}, + log: logrus.WithField("task", "DetectingLibraries"), } } @@ -168,19 +172,6 @@ func (f *CppIncludesFinder) DetectLibraries() error { f.cache.AddAndCheckEntry(nil, "", includeDir) } - mergedfile, err := MakeSourceFile(f.ctx.SketchBuildPath, f.ctx.SketchBuildPath, paths.New(f.sketch.MainFile.Base()+".cpp")) - if err != nil { - return errors.WithStack(err) - } - f.log.Debugf("Queueing merged sketch: %s", mergedfile) - f.queue.Push(mergedfile) - - f.queueSourceFilesFromFolder(f.ctx.SketchBuildPath, f.ctx.SketchBuildPath, false /* recurse */) - srcSubfolderPath := f.ctx.SketchBuildPath.Join("src") - if srcSubfolderPath.IsDir() { - f.queueSourceFilesFromFolder(srcSubfolderPath, f.ctx.SketchBuildPath, true /* recurse */) - } - for !f.queue.Empty() { if err := f.findIncludesUntilDone(f.queue.Pop()); err != nil { f.cache.Remove() @@ -201,6 +192,38 @@ func (f *CppIncludesFinder) UseIncludeDir(includeDir *paths.Path) { f.IncludeDirsFound.Add(includeDir) } +// AddSourceFile adds a source file to be examined to look for library imports +func (f *CppIncludesFinder) AddSourceFile(sourceRoot, buildRoot, srcPath *paths.Path) error { + if file, err := MakeSourceFile(sourceRoot, buildRoot, srcPath); err == nil { + f.log.Debugf("Queueing source file: %s", file) + f.queue.Push(file) + } else { + return errors.WithStack(err) + } + return nil +} + +// AddSourceDir adds a directory of source file to be examined to look for library imports +func (f *CppIncludesFinder) AddSourceDir(srcDir, buildDir *paths.Path, recurse bool) error { + extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } + f.log.Debugf(" Queueing source files from %s (recurse %v)", srcDir, recurse) + filePaths, err := utils.FindFilesInFolder(srcDir.String(), extensions, recurse) + if err != nil { + return errors.WithStack(err) + } + + for _, filePath := range filePaths { + sourceFile, err := MakeSourceFile(srcDir, buildDir, paths.New(filePath)) + if err != nil { + return errors.WithStack(err) + } + f.log.Debugf(" Queuing %s", sourceFile) + f.queue.Push(sourceFile) + } + + return nil +} + func runCommand(ctx *types.Context, command types.Command) error { PrintRingNameIfDebug(ctx, command) err := command.Run(ctx) @@ -444,33 +467,13 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error f.ctx.Info(tr("Skipping dependencies detection for precompiled library %[1]s", library.Name)) } } else { - f.queueSourceFilesFromFolder(buildDir, sourceDir.Dir, sourceDir.Recurse) + f.AddSourceDir(buildDir, sourceDir.Dir, sourceDir.Recurse) } } first = false } } -func (f *CppIncludesFinder) queueSourceFilesFromFolder(srcDir, buildDir *paths.Path, recurse bool) error { - extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } - f.log.Debugf(" Queueing source files from %s (recurse %v)", srcDir, recurse) - filePaths, err := utils.FindFilesInFolder(srcDir.String(), extensions, recurse) - if err != nil { - return errors.WithStack(err) - } - - for _, filePath := range filePaths { - sourceFile, err := MakeSourceFile(srcDir, buildDir, paths.New(filePath)) - if err != nil { - return errors.WithStack(err) - } - f.log.Debugf(" Queuing %s", sourceFile) - f.queue.Push(sourceFile) - } - - return nil -} - // SourceFile represent a source file, it keeps a reference to the root source // directory and the corresponding build root directory. type SourceFile struct { From bbaf472ea4229fad10f7eea92717e9ece3e8e2d8 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 25 Oct 2019 14:24:59 +0200 Subject: [PATCH 21/24] Using Library.SourceDirs() to obtain lib source dirs --- legacy/builder/container_find_includes.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index ded7cd6b691..c6e2716a252 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -448,18 +448,13 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error // include path and queue its source files for further // include scanning f.ctx.ImportedLibraries = append(f.ctx.ImportedLibraries, library) - - f.log.Debugf("Using library include folder: %s", library.SourceDir) - f.IncludeDirsFound.Add(library.SourceDir) f.cache.AddAndCheckEntry(sourcePath, include, library.SourceDir) - if library.UtilityDir != nil { - // TODO: Use library.SourceDirs() instead? - f.IncludeDirsFound.Add(library.UtilityDir) - } sourceDirs := library.SourceDirs() buildDir := f.ctx.LibrariesBuildPath.Join(library.Name) for _, sourceDir := range sourceDirs { + f.log.Debugf("Using library include folder: %s", sourceDir.Dir) + f.IncludeDirsFound.Add(sourceDir.Dir) if library.Precompiled && library.PrecompiledWithSources { // Fully precompiled libraries should have no dependencies // to avoid ABI breakage From 1ce2fceaebec359805b2dc471a5f37541c712b48 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 28 Oct 2019 08:39:05 +0100 Subject: [PATCH 22/24] Added test for build caching --- .../try_build_of_problematic_sketch_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/legacy/builder/test/try_build_of_problematic_sketch_test.go b/legacy/builder/test/try_build_of_problematic_sketch_test.go index 92c5afb1f3f..8fa368fc527 100644 --- a/legacy/builder/test/try_build_of_problematic_sketch_test.go +++ b/legacy/builder/test/try_build_of_problematic_sketch_test.go @@ -17,12 +17,15 @@ package test import ( + "fmt" "path/filepath" "testing" + "time" "github.com/arduino/arduino-cli/legacy/builder" "github.com/arduino/arduino-cli/legacy/builder/types" paths "github.com/arduino/go-paths-helper" + "github.com/stretchr/testify/require" ) func TestTryBuild001(t *testing.T) { @@ -225,7 +228,21 @@ func makeDefaultContext(t *testing.T) *types.Context { func tryBuild(t *testing.T, sketchPath ...string) { ctx := makeDefaultContext(t) + ctx.DebugLevel = 999 + s1 := time.Now() tryBuildWithContext(t, ctx, sketchPath...) + t1 := time.Now().Sub(s1) + + ctx2 := makeDefaultContext(t) + ctx2.DebugLevel = 999 + ctx2.BuildPath = ctx.BuildPath + s2 := time.Now() + tryBuildWithContext(t, ctx2, sketchPath...) + t2 := time.Now().Sub(s2) + + fmt.Println("Build time non-cached:", t1) + fmt.Println("Build time cached:", t2) + require.True(t, t1.Microseconds() > t2.Microseconds()*9/10, "cached build time should be much lower") } func tryBuildWithContext(t *testing.T, ctx *types.Context, sketchPath ...string) { From 8e1d2808790b0b7b48c8386fd8d9c545c73a2c2b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 28 Oct 2019 14:52:11 +0100 Subject: [PATCH 23/24] Moving ctx.BuildPath usage out of CppIncludeFinder --- legacy/builder/container_find_includes.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index c6e2716a252..18a47620e8d 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -121,7 +121,7 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { } func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { - finder := NewCppIncludesFinder(ctx) + finder := NewCppIncludesFinder(ctx, ctx.BuildPath.Join("includes.cache")) finder.UseIncludeDir(ctx.BuildProperties.GetPath("build.core.path")) if variantPath := ctx.BuildProperties.GetPath("build.variant.path"); variantPath != nil { finder.UseIncludeDir(variantPath) @@ -156,10 +156,10 @@ type CppIncludesFinder struct { } // NewCppIncludesFinder create a new include -func NewCppIncludesFinder(ctx *types.Context) *CppIncludesFinder { +func NewCppIncludesFinder(ctx *types.Context, cachePath *paths.Path) *CppIncludesFinder { return &CppIncludesFinder{ ctx: ctx, - cache: loadCacheFrom(ctx.BuildPath.Join("includes.cache")), + cache: loadCacheFrom(cachePath), queue: &UniqueSourceFileQueue{}, log: logrus.WithField("task", "DetectingLibraries"), } From 23d781f779773a810f702fcc634bdfc6af7cb50e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 28 Oct 2019 15:11:16 +0100 Subject: [PATCH 24/24] Moving ctx.ImportedLibraries usage out of CppIncludeFinder --- legacy/builder/container_find_includes.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 18a47620e8d..11eb83a7a78 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -98,6 +98,7 @@ import ( "os/exec" "time" + "github.com/arduino/arduino-cli/arduino/libraries" "github.com/arduino/arduino-cli/legacy/builder/builder_utils" "github.com/arduino/arduino-cli/legacy/builder/types" "github.com/arduino/arduino-cli/legacy/builder/utils" @@ -136,6 +137,7 @@ func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { return err } + ctx.ImportedLibraries.Add(finder.LibrariesFound...) ctx.IncludeFolders.AddAllMissing(finder.IncludeDirsFound) if err := runCommand(ctx, &FailIfImportedLibraryIsWrong{}); err != nil { return errors.WithStack(err) @@ -148,6 +150,7 @@ func (s *ContainerFindIncludes) findIncludes(ctx *types.Context) error { // libraries used in a sketch and a way to cache this result for // increasing detection speed on already processed sketches. type CppIncludesFinder struct { + LibrariesFound libraries.List IncludeDirsFound paths.PathList ctx *types.Context cache *includeCache @@ -444,10 +447,9 @@ func (f *CppIncludesFinder) findIncludesUntilDone(sourceFile *SourceFile) error return errors.WithStack(preprocErr) } - // Add this library to the list of libraries, the - // include path and queue its source files for further - // include scanning - f.ctx.ImportedLibraries = append(f.ctx.ImportedLibraries, library) + // Add this library to the list of libraries found, add the include path + // and queue its source files for further include scanning + f.LibrariesFound.Add(library) f.cache.AddAndCheckEntry(sourcePath, include, library.SourceDir) sourceDirs := library.SourceDirs()