From 33a565a39550427d01a67b5a37a663a5ac0445b9 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 6 May 2016 11:37:57 +0200 Subject: [PATCH 01/12] Remove error return value from resolveLibraries This function never fails, so no need to return and check for an error value. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/includes_to_include_folders.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/arduino.cc/builder/includes_to_include_folders.go b/src/arduino.cc/builder/includes_to_include_folders.go index 3a3671ed..f167780c 100644 --- a/src/arduino.cc/builder/includes_to_include_folders.go +++ b/src/arduino.cc/builder/includes_to_include_folders.go @@ -34,7 +34,6 @@ import ( "strings" "arduino.cc/builder/constants" - "arduino.cc/builder/i18n" "arduino.cc/builder/types" "arduino.cc/builder/utils" "arduino.cc/properties" @@ -51,10 +50,7 @@ func (s *IncludesToIncludeFolders) Run(ctx *types.Context) error { libraryResolutionResults := ctx.LibrariesResolutionResults importedLibraries := ctx.ImportedLibraries - newlyImportedLibraries, err := resolveLibraries(includes, headerToLibraries, importedLibraries, []*types.Platform{actualPlatform, platform}, libraryResolutionResults) - if err != nil { - return i18n.WrapError(err) - } + newlyImportedLibraries := resolveLibraries(includes, headerToLibraries, importedLibraries, []*types.Platform{actualPlatform, platform}, libraryResolutionResults) foldersWithSources := ctx.FoldersWithSourceFiles @@ -89,7 +85,7 @@ func resolveIncludeFolders(importedLibraries []*types.Library, buildProperties p } //FIXME it's also resolving previously resolved libraries -func resolveLibraries(includes []string, headerToLibraries map[string][]*types.Library, importedLibraries []*types.Library, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) ([]*types.Library, error) { +func resolveLibraries(includes []string, headerToLibraries map[string][]*types.Library, importedLibraries []*types.Library, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) []*types.Library { markImportedLibrary := make(map[*types.Library]bool) for _, library := range importedLibraries { markImportedLibrary[library] = true @@ -103,7 +99,7 @@ func resolveLibraries(includes []string, headerToLibraries map[string][]*types.L newlyImportedLibraries = append(newlyImportedLibraries, library) } - return newlyImportedLibraries, nil + return newlyImportedLibraries } func resolveLibrary(header string, headerToLibraries map[string][]*types.Library, markImportedLibrary map[*types.Library]bool, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) { From 3ff8ebbb179087cc605620cf55a634645d0641cb Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 21 Jul 2016 10:10:16 +0200 Subject: [PATCH 02/12] Let IncludesFinderWithRegExp return at most 1 include Originally, this code was written to find all include statements in a source file, for further processing. However, since a while the include finding was changed to run the preprocessor to see if any includes are missing, and this class is used to find the missing includes in the error message. In practice, the preprocessor will bail out at the first missing include, since there is no meaningful way for it to continue. This means there will be at most one include in the error output. If there would be multiple includes (either because the preprocessor decided to continue, or there is some other output that accidentially matches the regex), it would be harmful to actually process all of them, since the absence of the first include might influence the presence of the second (e.g. in the presence of #ifdefs). This commit enforces that only one include is returned at a time, slightly simplifying the code in the process. The filename and class should be renamed to singular as well, but this will be left for a future commit to reduce noise. Signed-off-by: Matthijs Kooijman --- .../builder/container_find_includes.go | 3 +-- .../builder/includes_finder_with_regexp.go | 21 ++++++++----------- src/arduino.cc/builder/types/context.go | 2 +- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 7d2e05c5..5641d440 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -120,14 +120,13 @@ func findIncludesUntilDone(ctx *types.Context, sourceFilePath string) error { return i18n.WrapError(err) } } - if len(ctx.IncludesJustFound) == 0 { + if ctx.IncludeJustFound == "" { done = true } else if len(ctx.ImportedLibraries) == len(importedLibraries) { err := runCommand(ctx, &GCCPreprocRunner{TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}) return i18n.WrapError(err) } importedLibraries = ctx.ImportedLibraries - ctx.IncludesJustFound = []string{} } return nil } diff --git a/src/arduino.cc/builder/includes_finder_with_regexp.go b/src/arduino.cc/builder/includes_finder_with_regexp.go index 6bc81882..83aceff0 100644 --- a/src/arduino.cc/builder/includes_finder_with_regexp.go +++ b/src/arduino.cc/builder/includes_finder_with_regexp.go @@ -45,24 +45,21 @@ type IncludesFinderWithRegExp struct { func (s *IncludesFinderWithRegExp) Run(ctx *types.Context) error { source := *s.Source - matches := INCLUDE_REGEXP.FindAllStringSubmatch(source, -1) - includes := []string{} - for _, match := range matches { - includes = append(includes, strings.TrimSpace(match[1])) + match := INCLUDE_REGEXP.FindStringSubmatch(source) + if match != nil { + ctx.IncludeJustFound = strings.TrimSpace(match[1]) + } else { + ctx.IncludeJustFound = findIncludeForOldCompilers(source) } - if len(includes) == 0 { - include := findIncludesForOldCompilers(source) - if include != "" { - includes = append(includes, include) - } + + if ctx.IncludeJustFound != "" { + ctx.Includes = utils.AppendIfNotPresent(ctx.Includes, ctx.IncludeJustFound) } - ctx.IncludesJustFound = includes - ctx.Includes = utils.AppendIfNotPresent(ctx.Includes, includes...) return nil } -func findIncludesForOldCompilers(source string) string { +func findIncludeForOldCompilers(source string) string { lines := strings.Split(source, "\n") for _, line := range lines { splittedLine := strings.Split(line, ":") diff --git a/src/arduino.cc/builder/types/context.go b/src/arduino.cc/builder/types/context.go index 75a052b2..ec322ea8 100644 --- a/src/arduino.cc/builder/types/context.go +++ b/src/arduino.cc/builder/types/context.go @@ -61,7 +61,7 @@ type Context struct { HeaderToLibraries map[string][]*Library ImportedLibraries []*Library LibrariesResolutionResults map[string]LibraryResolutionResult - IncludesJustFound []string + IncludeJustFound string IncludeFolders []string OutputGccMinusM string From e6bb6fd410c4174ac71655c29efae2e42451edc5 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 21 Jul 2016 11:13:30 +0200 Subject: [PATCH 03/12] Let the IncludesFinder tests also check ctx.IncludeJustFound Signed-off-by: Matthijs Kooijman --- .../builder/test/includes_finder_with_regexp_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/arduino.cc/builder/test/includes_finder_with_regexp_test.go b/src/arduino.cc/builder/test/includes_finder_with_regexp_test.go index 161416be..25ce576e 100644 --- a/src/arduino.cc/builder/test/includes_finder_with_regexp_test.go +++ b/src/arduino.cc/builder/test/includes_finder_with_regexp_test.go @@ -53,6 +53,7 @@ func TestIncludesFinderWithRegExp(t *testing.T) { includes := ctx.Includes require.Equal(t, 1, len(includes)) require.Equal(t, "SPI.h", includes[0]) + require.Equal(t, "SPI.h", ctx.IncludeJustFound) } func TestIncludesFinderWithRegExpEmptyOutput(t *testing.T) { @@ -68,6 +69,7 @@ func TestIncludesFinderWithRegExpEmptyOutput(t *testing.T) { includes := ctx.Includes require.Equal(t, 0, len(includes)) + require.Equal(t, "", ctx.IncludeJustFound) } func TestIncludesFinderWithRegExpPreviousIncludes(t *testing.T) { @@ -91,6 +93,7 @@ func TestIncludesFinderWithRegExpPreviousIncludes(t *testing.T) { sort.Strings(includes) require.Equal(t, "SPI.h", includes[0]) require.Equal(t, "test.h", includes[1]) + require.Equal(t, "SPI.h", ctx.IncludeJustFound) } func TestIncludesFinderWithRegExpPaddedIncludes(t *testing.T) { @@ -110,6 +113,7 @@ func TestIncludesFinderWithRegExpPaddedIncludes(t *testing.T) { require.Equal(t, 1, len(includes)) sort.Strings(includes) require.Equal(t, "Wire.h", includes[0]) + require.Equal(t, "Wire.h", ctx.IncludeJustFound) } func TestIncludesFinderWithRegExpPaddedIncludes2(t *testing.T) { @@ -129,6 +133,7 @@ func TestIncludesFinderWithRegExpPaddedIncludes2(t *testing.T) { require.Equal(t, 1, len(includes)) sort.Strings(includes) require.Equal(t, "Wire.h", includes[0]) + require.Equal(t, "Wire.h", ctx.IncludeJustFound) } func TestIncludesFinderWithRegExpPaddedIncludes3(t *testing.T) { @@ -147,6 +152,7 @@ func TestIncludesFinderWithRegExpPaddedIncludes3(t *testing.T) { require.Equal(t, 1, len(includes)) sort.Strings(includes) require.Equal(t, "SPI.h", includes[0]) + require.Equal(t, "SPI.h", ctx.IncludeJustFound) } func TestIncludesFinderWithRegExpPaddedIncludes4(t *testing.T) { @@ -165,4 +171,5 @@ func TestIncludesFinderWithRegExpPaddedIncludes4(t *testing.T) { require.Equal(t, 1, len(includes)) sort.Strings(includes) require.Equal(t, "register.h", includes[0]) + require.Equal(t, "register.h", ctx.IncludeJustFound) } From 6b2988903b356a4cffb0eecfd0089d73063756a9 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 6 May 2016 12:03:51 +0200 Subject: [PATCH 04/12] Do not rebuild the include folder list repeatedely Previously, during include detection, every pass through IncludesToIncludeFolders would iterate over all included filenames found so far, and rebuild the list of include folders from the list of included libraries again. This commit consistes of these changes: - IncludesToIncludeFolders now looks at ctx.IncludeJustFound to find new libraries (instead of ctx.Includes which contains all included filenames found so far). To minimize changes in this commit, it still creates a slice with just the single include found and processes that. A future commit will simplify this. - With that change, ctx.Includes is no longer used so it is removed. - The resolveIncludeFolders function is removed. It used to build the list of include folders, based on the list of imported libaries. This list was built from scratch again every time, including the core and variant folders. These two folders are now added once, by ContainerFindIncludes, and adding the library's source folders to the include folder list is done by IncludesToIncludeFolders. - ContainerFindIncludes would start by calling IncludesToIncludeFolders (even before any includes were detected) to ensure that the include path contained the core and variant folders on the first preprocessor run already. Now that these paths are added by ContainerFindIncludes, there is no need for this extra call to IncludesToIncludeFolders, so it is removed. - resolveLibraries is modified to actually return only newly imported libraries, not all imported libraries. - resolveLibrary now returns the library it resolved to, instead of just adding it to the markImportedLibrary map. Signed-off-by: Matthijs Kooijman --- .../builder/container_find_includes.go | 8 ++-- .../builder/includes_finder_with_regexp.go | 5 -- .../builder/includes_to_include_folders.go | 48 ++++++++----------- .../test/includes_finder_with_regexp_test.go | 46 ------------------ src/arduino.cc/builder/types/context.go | 1 - 5 files changed, 23 insertions(+), 85 deletions(-) diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 5641d440..2824a0d7 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -42,14 +42,14 @@ import ( type ContainerFindIncludes struct{} func (s *ContainerFindIncludes) Run(ctx *types.Context) error { - err := runCommand(ctx, &IncludesToIncludeFolders{}) - if err != nil { - return i18n.WrapError(err) + ctx.IncludeFolders = append(ctx.IncludeFolders, ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_CORE_PATH]) + if ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH] != constants.EMPTY_STRING { + ctx.IncludeFolders = append(ctx.IncludeFolders, ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH]) } sketchBuildPath := ctx.SketchBuildPath sketch := ctx.Sketch - err = findIncludesUntilDone(ctx, filepath.Join(sketchBuildPath, filepath.Base(sketch.MainFile.Name)+".cpp")) + err := findIncludesUntilDone(ctx, filepath.Join(sketchBuildPath, filepath.Base(sketch.MainFile.Name)+".cpp")) if err != nil { return i18n.WrapError(err) } diff --git a/src/arduino.cc/builder/includes_finder_with_regexp.go b/src/arduino.cc/builder/includes_finder_with_regexp.go index 83aceff0..f093759d 100644 --- a/src/arduino.cc/builder/includes_finder_with_regexp.go +++ b/src/arduino.cc/builder/includes_finder_with_regexp.go @@ -31,7 +31,6 @@ package builder import ( "arduino.cc/builder/types" - "arduino.cc/builder/utils" "regexp" "strings" ) @@ -52,10 +51,6 @@ func (s *IncludesFinderWithRegExp) Run(ctx *types.Context) error { ctx.IncludeJustFound = findIncludeForOldCompilers(source) } - if ctx.IncludeJustFound != "" { - ctx.Includes = utils.AppendIfNotPresent(ctx.Includes, ctx.IncludeJustFound) - } - return nil } diff --git a/src/arduino.cc/builder/includes_to_include_folders.go b/src/arduino.cc/builder/includes_to_include_folders.go index f167780c..eafd294f 100644 --- a/src/arduino.cc/builder/includes_to_include_folders.go +++ b/src/arduino.cc/builder/includes_to_include_folders.go @@ -36,13 +36,14 @@ import ( "arduino.cc/builder/constants" "arduino.cc/builder/types" "arduino.cc/builder/utils" - "arduino.cc/properties" ) type IncludesToIncludeFolders struct{} func (s *IncludesToIncludeFolders) Run(ctx *types.Context) error { - includes := ctx.Includes + include := ctx.IncludeJustFound + includes := []string{include} + includeFolders := ctx.IncludeFolders headerToLibraries := ctx.HeaderToLibraries platform := ctx.TargetPlatform @@ -50,8 +51,11 @@ func (s *IncludesToIncludeFolders) Run(ctx *types.Context) error { libraryResolutionResults := ctx.LibrariesResolutionResults importedLibraries := ctx.ImportedLibraries - newlyImportedLibraries := resolveLibraries(includes, headerToLibraries, importedLibraries, []*types.Platform{actualPlatform, platform}, libraryResolutionResults) + if include == "" { + return nil; + } + newlyImportedLibraries := resolveLibraries(includes, headerToLibraries, importedLibraries, []*types.Platform{actualPlatform, platform}, libraryResolutionResults) foldersWithSources := ctx.FoldersWithSourceFiles for _, newlyImportedLibrary := range newlyImportedLibraries { @@ -61,61 +65,46 @@ func (s *IncludesToIncludeFolders) Run(ctx *types.Context) error { for _, sourceFolder := range sourceFolders { foldersWithSources.Push(sourceFolder) } + includeFolders = append(includeFolders, newlyImportedLibrary.SrcFolder) } } ctx.ImportedLibraries = importedLibraries - ctx.IncludeFolders = resolveIncludeFolders(newlyImportedLibraries, ctx.BuildProperties, ctx.Verbose) + ctx.IncludeFolders = includeFolders return nil } -func resolveIncludeFolders(importedLibraries []*types.Library, buildProperties properties.Map, verbose bool) []string { - var includeFolders []string - includeFolders = append(includeFolders, buildProperties[constants.BUILD_PROPERTIES_BUILD_CORE_PATH]) - if buildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH] != constants.EMPTY_STRING { - includeFolders = append(includeFolders, buildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH]) - } - - for _, library := range importedLibraries { - includeFolders = append(includeFolders, library.SrcFolder) - } - - return includeFolders -} - -//FIXME it's also resolving previously resolved libraries func resolveLibraries(includes []string, headerToLibraries map[string][]*types.Library, importedLibraries []*types.Library, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) []*types.Library { markImportedLibrary := make(map[*types.Library]bool) for _, library := range importedLibraries { markImportedLibrary[library] = true } - for _, header := range includes { - resolveLibrary(header, headerToLibraries, markImportedLibrary, platforms, libraryResolutionResults) - } - var newlyImportedLibraries []*types.Library - for library, _ := range markImportedLibrary { - newlyImportedLibraries = append(newlyImportedLibraries, library) + for _, header := range includes { + library := resolveLibrary(header, headerToLibraries, markImportedLibrary, platforms, libraryResolutionResults) + if library != nil { + newlyImportedLibraries = append(newlyImportedLibraries, library) + } } return newlyImportedLibraries } -func resolveLibrary(header string, headerToLibraries map[string][]*types.Library, markImportedLibrary map[*types.Library]bool, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) { +func resolveLibrary(header string, headerToLibraries map[string][]*types.Library, markImportedLibrary map[*types.Library]bool, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) *types.Library { libraries := append([]*types.Library{}, headerToLibraries[header]...) if libraries == nil || len(libraries) == 0 { - return + return nil } if len(libraries) == 1 { markImportedLibrary[libraries[0]] = true - return + return libraries[0] } if markImportedLibraryContainsOneOfCandidates(markImportedLibrary, libraries) { - return + return nil } reverse(libraries) @@ -147,6 +136,7 @@ func resolveLibrary(header string, headerToLibraries map[string][]*types.Library libraryResolutionResults[header] = types.LibraryResolutionResult{Library: library, NotUsedLibraries: filterOutLibraryFrom(libraries, library)} markImportedLibrary[library] = true + return library } //facepalm. sort.Reverse needs an Interface that implements Len/Less/Swap. It's a slice! What else for reversing it?!? diff --git a/src/arduino.cc/builder/test/includes_finder_with_regexp_test.go b/src/arduino.cc/builder/test/includes_finder_with_regexp_test.go index 25ce576e..d6bc1553 100644 --- a/src/arduino.cc/builder/test/includes_finder_with_regexp_test.go +++ b/src/arduino.cc/builder/test/includes_finder_with_regexp_test.go @@ -33,7 +33,6 @@ import ( "arduino.cc/builder" "arduino.cc/builder/types" "github.com/stretchr/testify/require" - "sort" "testing" ) @@ -50,9 +49,6 @@ func TestIncludesFinderWithRegExp(t *testing.T) { err := parser.Run(ctx) NoError(t, err) - includes := ctx.Includes - require.Equal(t, 1, len(includes)) - require.Equal(t, "SPI.h", includes[0]) require.Equal(t, "SPI.h", ctx.IncludeJustFound) } @@ -67,35 +63,9 @@ func TestIncludesFinderWithRegExpEmptyOutput(t *testing.T) { err := parser.Run(ctx) NoError(t, err) - includes := ctx.Includes - require.Equal(t, 0, len(includes)) require.Equal(t, "", ctx.IncludeJustFound) } -func TestIncludesFinderWithRegExpPreviousIncludes(t *testing.T) { - ctx := &types.Context{ - Includes: []string{"test.h"}, - } - - output := "/some/path/sketch.ino:1:17: fatal error: SPI.h: No such file or directory\n" + - "#include \n" + - "^\n" + - "compilation terminated." - - ctx.Source = output - - parser := builder.IncludesFinderWithRegExp{Source: &ctx.Source} - err := parser.Run(ctx) - NoError(t, err) - - includes := ctx.Includes - require.Equal(t, 2, len(includes)) - sort.Strings(includes) - require.Equal(t, "SPI.h", includes[0]) - require.Equal(t, "test.h", includes[1]) - require.Equal(t, "SPI.h", ctx.IncludeJustFound) -} - func TestIncludesFinderWithRegExpPaddedIncludes(t *testing.T) { ctx := &types.Context{} @@ -109,10 +79,6 @@ func TestIncludesFinderWithRegExpPaddedIncludes(t *testing.T) { err := parser.Run(ctx) NoError(t, err) - includes := ctx.Includes - require.Equal(t, 1, len(includes)) - sort.Strings(includes) - require.Equal(t, "Wire.h", includes[0]) require.Equal(t, "Wire.h", ctx.IncludeJustFound) } @@ -129,10 +95,6 @@ func TestIncludesFinderWithRegExpPaddedIncludes2(t *testing.T) { err := parser.Run(ctx) NoError(t, err) - includes := ctx.Includes - require.Equal(t, 1, len(includes)) - sort.Strings(includes) - require.Equal(t, "Wire.h", includes[0]) require.Equal(t, "Wire.h", ctx.IncludeJustFound) } @@ -148,10 +110,6 @@ func TestIncludesFinderWithRegExpPaddedIncludes3(t *testing.T) { err := parser.Run(ctx) NoError(t, err) - includes := ctx.Includes - require.Equal(t, 1, len(includes)) - sort.Strings(includes) - require.Equal(t, "SPI.h", includes[0]) require.Equal(t, "SPI.h", ctx.IncludeJustFound) } @@ -167,9 +125,5 @@ func TestIncludesFinderWithRegExpPaddedIncludes4(t *testing.T) { err := parser.Run(ctx) NoError(t, err) - includes := ctx.Includes - require.Equal(t, 1, len(includes)) - sort.Strings(includes) - require.Equal(t, "register.h", includes[0]) require.Equal(t, "register.h", ctx.IncludeJustFound) } diff --git a/src/arduino.cc/builder/types/context.go b/src/arduino.cc/builder/types/context.go index ec322ea8..379e64f0 100644 --- a/src/arduino.cc/builder/types/context.go +++ b/src/arduino.cc/builder/types/context.go @@ -56,7 +56,6 @@ type Context struct { WarningsLevel string // Libraries handling - Includes []string Libraries []*Library HeaderToLibraries map[string][]*Library ImportedLibraries []*Library From c7884488be8619e542678387dc9fc4ee465ba281 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 21 Jul 2016 11:36:24 +0200 Subject: [PATCH 05/12] Simplify IncludesToIncludeFolders This code was written to handle processing of multiple includes after each other, but it only needs to handle one. This allows simplifying the code a bit. Behaviour should not be changed. Signed-off-by: Matthijs Kooijman --- .../builder/includes_to_include_folders.go | 34 ++++++------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/src/arduino.cc/builder/includes_to_include_folders.go b/src/arduino.cc/builder/includes_to_include_folders.go index eafd294f..8539842d 100644 --- a/src/arduino.cc/builder/includes_to_include_folders.go +++ b/src/arduino.cc/builder/includes_to_include_folders.go @@ -42,7 +42,6 @@ type IncludesToIncludeFolders struct{} func (s *IncludesToIncludeFolders) Run(ctx *types.Context) error { include := ctx.IncludeJustFound - includes := []string{include} includeFolders := ctx.IncludeFolders headerToLibraries := ctx.HeaderToLibraries @@ -55,19 +54,19 @@ func (s *IncludesToIncludeFolders) Run(ctx *types.Context) error { return nil; } - newlyImportedLibraries := resolveLibraries(includes, headerToLibraries, importedLibraries, []*types.Platform{actualPlatform, platform}, libraryResolutionResults) + newlyImportedLibrary := resolveLibrary(include, headerToLibraries, importedLibraries, []*types.Platform{actualPlatform, platform}, libraryResolutionResults) + if newlyImportedLibrary == nil { + return nil; + } + foldersWithSources := ctx.FoldersWithSourceFiles - for _, newlyImportedLibrary := range newlyImportedLibraries { - if !sliceContainsLibrary(importedLibraries, newlyImportedLibrary) { - importedLibraries = append(importedLibraries, newlyImportedLibrary) - sourceFolders := types.LibraryToSourceFolder(newlyImportedLibrary) - for _, sourceFolder := range sourceFolders { - foldersWithSources.Push(sourceFolder) - } - includeFolders = append(includeFolders, newlyImportedLibrary.SrcFolder) - } + importedLibraries = append(importedLibraries, newlyImportedLibrary) + sourceFolders := types.LibraryToSourceFolder(newlyImportedLibrary) + for _, sourceFolder := range sourceFolders { + foldersWithSources.Push(sourceFolder) } + includeFolders = append(includeFolders, newlyImportedLibrary.SrcFolder) ctx.ImportedLibraries = importedLibraries ctx.IncludeFolders = includeFolders @@ -75,23 +74,12 @@ func (s *IncludesToIncludeFolders) Run(ctx *types.Context) error { return nil } -func resolveLibraries(includes []string, headerToLibraries map[string][]*types.Library, importedLibraries []*types.Library, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) []*types.Library { +func resolveLibrary(header string, headerToLibraries map[string][]*types.Library, importedLibraries []*types.Library, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) *types.Library { markImportedLibrary := make(map[*types.Library]bool) for _, library := range importedLibraries { markImportedLibrary[library] = true } - var newlyImportedLibraries []*types.Library - for _, header := range includes { - library := resolveLibrary(header, headerToLibraries, markImportedLibrary, platforms, libraryResolutionResults) - if library != nil { - newlyImportedLibraries = append(newlyImportedLibraries, library) - } - } - - return newlyImportedLibraries -} -func resolveLibrary(header string, headerToLibraries map[string][]*types.Library, markImportedLibrary map[*types.Library]bool, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) *types.Library { libraries := append([]*types.Library{}, headerToLibraries[header]...) if libraries == nil || len(libraries) == 0 { From 85de69a62ce968ab1fee3836f35756f09ff9f456 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Mon, 1 Aug 2016 20:31:39 +0200 Subject: [PATCH 06/12] Remove unneeded code from ContainerFindIncludes The removed code iterates over the detected libraries and adds their source directories to the queue for running further detection on them. However, IncludesToIncludeFolders (called by findIncludesUntilDone(), which is called a bit above the removed code) already does this, so there is no need to do it again. Since Ctx.FoldersWithSources is a UniqueStringQueue, this unneeded code was not affecting behaviour in any way. It seems likely that this code had a purpose previously, but it was lost in some refactor. However, a quick scan of the commit that introduced it (d6e378: Implemented library to library dependency full discovery) suggests that this code was useless from the start. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/container_find_includes.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 2824a0d7..5ed046f2 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -60,14 +60,6 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { if info, err := os.Stat(srcSubfolderPath); err == nil && info.IsDir() { foldersWithSources.Push(types.SourceFolder{Folder: srcSubfolderPath, Recurse: true}) } - if len(ctx.ImportedLibraries) > 0 { - for _, library := range ctx.ImportedLibraries { - sourceFolders := types.LibraryToSourceFolder(library) - for _, sourceFolder := range sourceFolders { - foldersWithSources.Push(sourceFolder) - } - } - } err = runCommand(ctx, &CollectAllSourceFilesFromFoldersWithSources{}) if err != nil { From 3a08c410b4ad04809b519c156d02e7467e63e39a Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Mon, 1 Aug 2016 20:53:39 +0200 Subject: [PATCH 07/12] Call findIncludesUntilDone from only one place Previously, it was called once for the main cpp file, and then in a loop for any extra source files from the sketch or dependent libraries. However, this can be simplified by just adding the main .cpp file to the queue, and then letting the loop handle it. This produces a tiny change in the order in which source files are processed for includes, but any setups affected by this are already prone to errors. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/container_find_includes.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 5ed046f2..16807bab 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -49,10 +49,7 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { sketchBuildPath := ctx.SketchBuildPath sketch := ctx.Sketch - err := findIncludesUntilDone(ctx, filepath.Join(sketchBuildPath, filepath.Base(sketch.MainFile.Name)+".cpp")) - if err != nil { - return i18n.WrapError(err) - } + ctx.CollectedSourceFiles.Push(filepath.Join(sketchBuildPath, filepath.Base(sketch.MainFile.Name)+".cpp")) foldersWithSources := ctx.FoldersWithSourceFiles foldersWithSources.Push(types.SourceFolder{Folder: ctx.SketchBuildPath, Recurse: false}) @@ -61,7 +58,7 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { foldersWithSources.Push(types.SourceFolder{Folder: srcSubfolderPath, Recurse: true}) } - err = runCommand(ctx, &CollectAllSourceFilesFromFoldersWithSources{}) + err := runCommand(ctx, &CollectAllSourceFilesFromFoldersWithSources{}) if err != nil { return i18n.WrapError(err) } From 02989392841fd16733752dc75322f45067be4041 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Mon, 1 Aug 2016 21:27:37 +0200 Subject: [PATCH 08/12] Remove Context.FoldersWithSourceFiles This field was used during include detection, to collect the source folders whose source files should be processed for includes. This field was handled by the CollectAllSourceFilesFromFoldersWithSources pass, which finds all source files in these folders and adds them to the Context.CollectedSourceFiles queue instead. This commit changes all code that used to add directories to the FoldersWithSourceFiles queue to instead add all contained source files to the CollectedSourceFiles queue. For this, the CollectAllSourceFilesFromFoldersWithSources pass is transformed into regular function and renamed to QueueSourceFilesInFolder, so it can be directly called without needing any particular context entries. Note that the filename that contains the latter function is no longer entirely accurate, but this will be fixed in a later commit (left untouched in this commit to simplify review). Signed-off-by: Matthijs Kooijman --- .../add_additional_entries_to_context.go | 1 - ..._source_files_from_folders_with_sources.go | 17 +-- .../builder/container_find_includes.go | 21 +-- .../builder/includes_to_include_folders.go | 4 +- .../add_additional_entries_to_context_test.go | 2 - ...ce_files_from_folders_with_sources_test.go | 127 ------------------ src/arduino.cc/builder/types/accessories.go | 23 ---- src/arduino.cc/builder/types/context.go | 1 - src/arduino.cc/builder/types/utils.go | 9 -- 9 files changed, 11 insertions(+), 194 deletions(-) delete mode 100644 src/arduino.cc/builder/test/collect_all_source_files_from_folders_with_sources_test.go diff --git a/src/arduino.cc/builder/add_additional_entries_to_context.go b/src/arduino.cc/builder/add_additional_entries_to_context.go index a4250c88..4a6e7c46 100644 --- a/src/arduino.cc/builder/add_additional_entries_to_context.go +++ b/src/arduino.cc/builder/add_additional_entries_to_context.go @@ -69,7 +69,6 @@ func (s *AddAdditionalEntriesToContext) Run(ctx *types.Context) error { } ctx.CollectedSourceFiles = &types.UniqueStringQueue{} - ctx.FoldersWithSourceFiles = &types.UniqueSourceFolderQueue{} ctx.LibrariesResolutionResults = make(map[string]types.LibraryResolutionResult) ctx.HardwareRewriteResults = make(map[*types.Platform][]types.PlatforKeyRewrite) diff --git a/src/arduino.cc/builder/collect_all_source_files_from_folders_with_sources.go b/src/arduino.cc/builder/collect_all_source_files_from_folders_with_sources.go index b9bc589f..64482920 100644 --- a/src/arduino.cc/builder/collect_all_source_files_from_folders_with_sources.go +++ b/src/arduino.cc/builder/collect_all_source_files_from_folders_with_sources.go @@ -35,24 +35,17 @@ import ( "arduino.cc/builder/utils" ) -type CollectAllSourceFilesFromFoldersWithSources struct{} - -func (s *CollectAllSourceFilesFromFoldersWithSources) Run(ctx *types.Context) error { - foldersWithSources := ctx.FoldersWithSourceFiles - sourceFiles := ctx.CollectedSourceFiles +func QueueSourceFilesFromFolder(queue *types.UniqueStringQueue, folder string, recurse bool) error { extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } filePaths := []string{} - for !foldersWithSources.Empty() { - sourceFolder := foldersWithSources.Pop().(types.SourceFolder) - err := utils.FindFilesInFolder(&filePaths, sourceFolder.Folder, extensions, sourceFolder.Recurse) - if err != nil { - return i18n.WrapError(err) - } + err := utils.FindFilesInFolder(&filePaths, folder, extensions, recurse) + if err != nil { + return i18n.WrapError(err) } for _, filePath := range filePaths { - sourceFiles.Push(filePath) + queue.Push(filePath) } return nil diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 16807bab..5303734a 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -51,32 +51,21 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { sketch := ctx.Sketch ctx.CollectedSourceFiles.Push(filepath.Join(sketchBuildPath, filepath.Base(sketch.MainFile.Name)+".cpp")) - foldersWithSources := ctx.FoldersWithSourceFiles - foldersWithSources.Push(types.SourceFolder{Folder: ctx.SketchBuildPath, Recurse: false}) + sourceFilePaths := ctx.CollectedSourceFiles + QueueSourceFilesFromFolder(sourceFilePaths, ctx.SketchBuildPath, /* recurse */ false) srcSubfolderPath := filepath.Join(ctx.SketchBuildPath, constants.SKETCH_FOLDER_SRC) if info, err := os.Stat(srcSubfolderPath); err == nil && info.IsDir() { - foldersWithSources.Push(types.SourceFolder{Folder: srcSubfolderPath, Recurse: true}) - } - - err := runCommand(ctx, &CollectAllSourceFilesFromFoldersWithSources{}) - if err != nil { - return i18n.WrapError(err) + QueueSourceFilesFromFolder(sourceFilePaths, srcSubfolderPath, /* recurse */ true) } - sourceFilePaths := ctx.CollectedSourceFiles - for !sourceFilePaths.Empty() { - err = findIncludesUntilDone(ctx, sourceFilePaths.Pop().(string)) - if err != nil { - return i18n.WrapError(err) - } - err := runCommand(ctx, &CollectAllSourceFilesFromFoldersWithSources{}) + err := findIncludesUntilDone(ctx, sourceFilePaths.Pop().(string)) if err != nil { return i18n.WrapError(err) } } - err = runCommand(ctx, &FailIfImportedLibraryIsWrong{}) + err := runCommand(ctx, &FailIfImportedLibraryIsWrong{}) if err != nil { return i18n.WrapError(err) } diff --git a/src/arduino.cc/builder/includes_to_include_folders.go b/src/arduino.cc/builder/includes_to_include_folders.go index 8539842d..4dfd7d09 100644 --- a/src/arduino.cc/builder/includes_to_include_folders.go +++ b/src/arduino.cc/builder/includes_to_include_folders.go @@ -59,12 +59,10 @@ func (s *IncludesToIncludeFolders) Run(ctx *types.Context) error { return nil; } - foldersWithSources := ctx.FoldersWithSourceFiles - importedLibraries = append(importedLibraries, newlyImportedLibrary) sourceFolders := types.LibraryToSourceFolder(newlyImportedLibrary) for _, sourceFolder := range sourceFolders { - foldersWithSources.Push(sourceFolder) + QueueSourceFilesFromFolder(ctx.CollectedSourceFiles, sourceFolder.Folder, sourceFolder.Recurse) } includeFolders = append(includeFolders, newlyImportedLibrary.SrcFolder) diff --git a/src/arduino.cc/builder/test/add_additional_entries_to_context_test.go b/src/arduino.cc/builder/test/add_additional_entries_to_context_test.go index a2bfbfc6..3bb057d4 100644 --- a/src/arduino.cc/builder/test/add_additional_entries_to_context_test.go +++ b/src/arduino.cc/builder/test/add_additional_entries_to_context_test.go @@ -52,7 +52,6 @@ func TestAddAdditionalEntriesToContextNoBuildPath(t *testing.T) { require.NotNil(t, ctx.WarningsLevel) require.True(t, ctx.CollectedSourceFiles.Empty()) - require.True(t, ctx.FoldersWithSourceFiles.Empty()) require.Equal(t, 0, len(ctx.LibrariesResolutionResults)) } @@ -72,7 +71,6 @@ func TestAddAdditionalEntriesToContextWithBuildPath(t *testing.T) { require.NotNil(t, ctx.WarningsLevel) require.True(t, ctx.CollectedSourceFiles.Empty()) - require.True(t, ctx.FoldersWithSourceFiles.Empty()) require.Equal(t, 0, len(ctx.LibrariesResolutionResults)) } diff --git a/src/arduino.cc/builder/test/collect_all_source_files_from_folders_with_sources_test.go b/src/arduino.cc/builder/test/collect_all_source_files_from_folders_with_sources_test.go deleted file mode 100644 index ce9ab58f..00000000 --- a/src/arduino.cc/builder/test/collect_all_source_files_from_folders_with_sources_test.go +++ /dev/null @@ -1,127 +0,0 @@ -/* - * This file is part of Arduino Builder. - * - * Arduino Builder is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA - * - * As a special exception, you may use this file as part of a free software - * library without restriction. Specifically, if other files instantiate - * templates or use macros or inline functions from this file, or you compile - * this file and link it with other files to produce an executable, this - * file does not by itself cause the resulting executable to be covered by - * the GNU General Public License. This exception does not however - * invalidate any other reasons why the executable file might be covered by - * the GNU General Public License. - * - * Copyright 2015 Arduino LLC (http://www.arduino.cc/) - */ - -package test - -import ( - "arduino.cc/builder" - "arduino.cc/builder/types" - "github.com/stretchr/testify/require" - "path/filepath" - "sort" - "testing" -) - -func TestCollectAllSourceFilesFromFoldersWithSources(t *testing.T) { - ctx := &types.Context{} - - sourceFiles := &types.UniqueStringQueue{} - ctx.CollectedSourceFiles = sourceFiles - foldersWithSources := &types.UniqueSourceFolderQueue{} - foldersWithSources.Push(types.SourceFolder{Folder: Abs(t, "sketch_with_config"), Recurse: true}) - ctx.FoldersWithSourceFiles = foldersWithSources - - commands := []types.Command{ - &builder.CollectAllSourceFilesFromFoldersWithSources{}, - } - - for _, command := range commands { - err := command.Run(ctx) - NoError(t, err) - } - - require.Equal(t, 1, len(*sourceFiles)) - require.Equal(t, 0, len(*foldersWithSources)) - sort.Strings(*sourceFiles) - - require.Equal(t, Abs(t, filepath.Join("sketch_with_config", "src", "includes", "de bug.cpp")), sourceFiles.Pop()) - require.Equal(t, 0, len(*sourceFiles)) -} - -func TestCollectAllSourceFilesFromFoldersWithSourcesOfLibrary(t *testing.T) { - ctx := &types.Context{} - - sourceFiles := &types.UniqueStringQueue{} - ctx.CollectedSourceFiles = sourceFiles - foldersWithSources := &types.UniqueSourceFolderQueue{} - foldersWithSources.Push(types.SourceFolder{Folder: Abs(t, filepath.Join("downloaded_libraries", "Bridge")), Recurse: true}) - ctx.FoldersWithSourceFiles = foldersWithSources - - commands := []types.Command{ - &builder.CollectAllSourceFilesFromFoldersWithSources{}, - } - - for _, command := range commands { - err := command.Run(ctx) - NoError(t, err) - } - - require.Equal(t, 9, len(*sourceFiles)) - require.Equal(t, 0, len(*foldersWithSources)) - sort.Strings(*sourceFiles) - - require.Equal(t, Abs(t, filepath.Join("downloaded_libraries", "Bridge", "src", "Bridge.cpp")), sourceFiles.Pop()) - require.Equal(t, Abs(t, filepath.Join("downloaded_libraries", "Bridge", "src", "BridgeClient.cpp")), sourceFiles.Pop()) - require.Equal(t, Abs(t, filepath.Join("downloaded_libraries", "Bridge", "src", "BridgeServer.cpp")), sourceFiles.Pop()) - require.Equal(t, Abs(t, filepath.Join("downloaded_libraries", "Bridge", "src", "BridgeUdp.cpp")), sourceFiles.Pop()) - require.Equal(t, Abs(t, filepath.Join("downloaded_libraries", "Bridge", "src", "Console.cpp")), sourceFiles.Pop()) - require.Equal(t, Abs(t, filepath.Join("downloaded_libraries", "Bridge", "src", "FileIO.cpp")), sourceFiles.Pop()) - require.Equal(t, Abs(t, filepath.Join("downloaded_libraries", "Bridge", "src", "HttpClient.cpp")), sourceFiles.Pop()) - require.Equal(t, Abs(t, filepath.Join("downloaded_libraries", "Bridge", "src", "Mailbox.cpp")), sourceFiles.Pop()) - require.Equal(t, Abs(t, filepath.Join("downloaded_libraries", "Bridge", "src", "Process.cpp")), sourceFiles.Pop()) - require.Equal(t, 0, len(*sourceFiles)) -} - -func TestCollectAllSourceFilesFromFoldersWithSourcesOfOldLibrary(t *testing.T) { - ctx := &types.Context{} - - sourceFiles := &types.UniqueStringQueue{} - ctx.CollectedSourceFiles = sourceFiles - foldersWithSources := &types.UniqueSourceFolderQueue{} - foldersWithSources.Push(types.SourceFolder{Folder: Abs(t, filepath.Join("libraries", "ShouldNotRecurseWithOldLibs")), Recurse: false}) - foldersWithSources.Push(types.SourceFolder{Folder: Abs(t, filepath.Join("libraries", "ShouldNotRecurseWithOldLibs", "utility")), Recurse: false}) - ctx.FoldersWithSourceFiles = foldersWithSources - - commands := []types.Command{ - &builder.CollectAllSourceFilesFromFoldersWithSources{}, - } - - for _, command := range commands { - err := command.Run(ctx) - NoError(t, err) - } - - require.Equal(t, 2, len(*sourceFiles)) - require.Equal(t, 0, len(*foldersWithSources)) - sort.Strings(*sourceFiles) - - require.Equal(t, Abs(t, filepath.Join("libraries", "ShouldNotRecurseWithOldLibs", "ShouldNotRecurseWithOldLibs.cpp")), sourceFiles.Pop()) - require.Equal(t, Abs(t, filepath.Join("libraries", "ShouldNotRecurseWithOldLibs", "utility", "utils.cpp")), sourceFiles.Pop()) - require.Equal(t, 0, len(*sourceFiles)) -} diff --git a/src/arduino.cc/builder/types/accessories.go b/src/arduino.cc/builder/types/accessories.go index 1d7b7fc2..3270ce26 100644 --- a/src/arduino.cc/builder/types/accessories.go +++ b/src/arduino.cc/builder/types/accessories.go @@ -51,26 +51,3 @@ func (queue *UniqueStringQueue) Pop() interface{} { func (queue *UniqueStringQueue) Empty() bool { return queue.Len() == 0 } - -type UniqueSourceFolderQueue []SourceFolder - -func (queue UniqueSourceFolderQueue) Len() int { return len(queue) } -func (queue UniqueSourceFolderQueue) Less(i, j int) bool { return false } -func (queue UniqueSourceFolderQueue) Swap(i, j int) { panic("Who called me?!?") } - -func (queue *UniqueSourceFolderQueue) Push(value SourceFolder) { - if !sliceContainsSourceFolder(*queue, value) { - *queue = append(*queue, value) - } -} - -func (queue *UniqueSourceFolderQueue) Pop() interface{} { - old := *queue - x := old[0] - *queue = old[1:] - return x -} - -func (queue *UniqueSourceFolderQueue) Empty() bool { - return queue.Len() == 0 -} diff --git a/src/arduino.cc/builder/types/context.go b/src/arduino.cc/builder/types/context.go index 379e64f0..d1421deb 100644 --- a/src/arduino.cc/builder/types/context.go +++ b/src/arduino.cc/builder/types/context.go @@ -47,7 +47,6 @@ type Context struct { SketchObjectFiles []string CollectedSourceFiles *UniqueStringQueue - FoldersWithSourceFiles *UniqueSourceFolderQueue Sketch *Sketch Source string diff --git a/src/arduino.cc/builder/types/utils.go b/src/arduino.cc/builder/types/utils.go index 1d26c93d..d7c6d40a 100644 --- a/src/arduino.cc/builder/types/utils.go +++ b/src/arduino.cc/builder/types/utils.go @@ -38,12 +38,3 @@ func sliceContains(slice []string, target string) bool { } return false } - -func sliceContainsSourceFolder(slice []SourceFolder, target SourceFolder) bool { - for _, elem := range slice { - if elem.Folder == target.Folder { - return true - } - } - return false -} From adc8cf91f78a3c30a4eb44e0126f427c62588391 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Mon, 1 Aug 2016 22:01:18 +0200 Subject: [PATCH 09/12] Remove done variable from findIncludesUntilDone This prepares for a change in the next commit. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/container_find_includes.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 5303734a..2ccdbd2b 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -85,8 +85,7 @@ func runCommand(ctx *types.Context, command types.Command) error { func findIncludesUntilDone(ctx *types.Context, sourceFilePath string) error { targetFilePath := utils.NULLFile() importedLibraries := ctx.ImportedLibraries - done := false - for !done { + for { commands := []types.Command{ &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourceFilePath, TargetFilePath: targetFilePath}, &IncludesFinderWithRegExp{Source: &ctx.SourceGccMinusE}, @@ -99,12 +98,14 @@ func findIncludesUntilDone(ctx *types.Context, sourceFilePath string) error { } } if ctx.IncludeJustFound == "" { - done = true - } else if len(ctx.ImportedLibraries) == len(importedLibraries) { + // No missing includes found, we're done + return nil + } + + if len(ctx.ImportedLibraries) == len(importedLibraries) { err := runCommand(ctx, &GCCPreprocRunner{TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}) return i18n.WrapError(err) } importedLibraries = ctx.ImportedLibraries } - return nil } From de46825f4096164bcb70d0185e70744759ee28de Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Mon, 1 Aug 2016 22:15:05 +0200 Subject: [PATCH 10/12] Remove IncludesToIncludeFolders.Run In previous commits, this method has been signficantly simplified, reducing it to little more than a wrapper around the resolveLibrary helper function. This commit renames resolveLibrary to ResolveLibrary to make it public and lets findIncludesUntilDone call it directly (instead of going through IncludesToIncludeFolders). findIncludesUntilDone now also takes care of processing the ResolveLibrary result, which was previously done by IncludesToIncludeFolders. The signature of ResolveLibrary is also changed to accept a Context, so it can now extract the needed variables itself, instead of needing IncludesToIncludeFolders to extract them. Note that the filename that contains ResolveLibrary is no longer entirely accurate, but this will be fixed in a later commit (to simplify review of this commit). Signed-off-by: Matthijs Kooijman --- .../builder/container_find_includes.go | 17 +++++++--- .../builder/includes_to_include_folders.go | 33 ++----------------- 2 files changed, 15 insertions(+), 35 deletions(-) diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 2ccdbd2b..fa920c16 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -84,12 +84,10 @@ func runCommand(ctx *types.Context, command types.Command) error { func findIncludesUntilDone(ctx *types.Context, sourceFilePath string) error { targetFilePath := utils.NULLFile() - importedLibraries := ctx.ImportedLibraries for { commands := []types.Command{ &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourceFilePath, TargetFilePath: targetFilePath}, &IncludesFinderWithRegExp{Source: &ctx.SourceGccMinusE}, - &IncludesToIncludeFolders{}, } for _, command := range commands { err := runCommand(ctx, command) @@ -102,10 +100,21 @@ func findIncludesUntilDone(ctx *types.Context, sourceFilePath string) error { return nil } - if len(ctx.ImportedLibraries) == len(importedLibraries) { + library := ResolveLibrary(ctx, ctx.IncludeJustFound) + if library == nil { + // Library could not be resolved, show error err := runCommand(ctx, &GCCPreprocRunner{TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}) return i18n.WrapError(err) } - importedLibraries = ctx.ImportedLibraries + + // 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) + ctx.IncludeFolders = append(ctx.IncludeFolders, library.SrcFolder) + sourceFolders := types.LibraryToSourceFolder(library) + for _, sourceFolder := range sourceFolders { + QueueSourceFilesFromFolder(ctx.CollectedSourceFiles, sourceFolder.Folder, sourceFolder.Recurse) + } } } diff --git a/src/arduino.cc/builder/includes_to_include_folders.go b/src/arduino.cc/builder/includes_to_include_folders.go index 4dfd7d09..4ba46f82 100644 --- a/src/arduino.cc/builder/includes_to_include_folders.go +++ b/src/arduino.cc/builder/includes_to_include_folders.go @@ -38,41 +38,12 @@ import ( "arduino.cc/builder/utils" ) -type IncludesToIncludeFolders struct{} - -func (s *IncludesToIncludeFolders) Run(ctx *types.Context) error { - include := ctx.IncludeJustFound - includeFolders := ctx.IncludeFolders +func ResolveLibrary(ctx *types.Context, header string) *types.Library { headerToLibraries := ctx.HeaderToLibraries - - platform := ctx.TargetPlatform - actualPlatform := ctx.ActualPlatform + platforms := []*types.Platform{ctx.ActualPlatform, ctx.TargetPlatform} libraryResolutionResults := ctx.LibrariesResolutionResults importedLibraries := ctx.ImportedLibraries - if include == "" { - return nil; - } - - newlyImportedLibrary := resolveLibrary(include, headerToLibraries, importedLibraries, []*types.Platform{actualPlatform, platform}, libraryResolutionResults) - if newlyImportedLibrary == nil { - return nil; - } - - importedLibraries = append(importedLibraries, newlyImportedLibrary) - sourceFolders := types.LibraryToSourceFolder(newlyImportedLibrary) - for _, sourceFolder := range sourceFolders { - QueueSourceFilesFromFolder(ctx.CollectedSourceFiles, sourceFolder.Folder, sourceFolder.Recurse) - } - includeFolders = append(includeFolders, newlyImportedLibrary.SrcFolder) - - ctx.ImportedLibraries = importedLibraries - ctx.IncludeFolders = includeFolders - - return nil -} - -func resolveLibrary(header string, headerToLibraries map[string][]*types.Library, importedLibraries []*types.Library, platforms []*types.Platform, libraryResolutionResults map[string]types.LibraryResolutionResult) *types.Library { markImportedLibrary := make(map[*types.Library]bool) for _, library := range importedLibraries { markImportedLibrary[library] = true From f1b08347daf33c5031a09e4cc437569731870bcc Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 2 Aug 2016 17:57:29 +0200 Subject: [PATCH 11/12] Move QueueSourceFilesFromFolder into container_find_includes.go This function used to be a pass, but it was reduced to a regular function in 1001916 (Remove Context.FoldersWithSourceFiles). By now, the function is only called from container_find_includes.go, so it makes sense to just move it into that file. This does not change any code, it just moves the function and renames it to be private. Signed-off-by: Matthijs Kooijman --- ..._source_files_from_folders_with_sources.go | 52 ------------------- .../builder/container_find_includes.go | 22 ++++++-- 2 files changed, 19 insertions(+), 55 deletions(-) delete mode 100644 src/arduino.cc/builder/collect_all_source_files_from_folders_with_sources.go diff --git a/src/arduino.cc/builder/collect_all_source_files_from_folders_with_sources.go b/src/arduino.cc/builder/collect_all_source_files_from_folders_with_sources.go deleted file mode 100644 index 64482920..00000000 --- a/src/arduino.cc/builder/collect_all_source_files_from_folders_with_sources.go +++ /dev/null @@ -1,52 +0,0 @@ -/* - * This file is part of Arduino Builder. - * - * Arduino Builder is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA - * - * As a special exception, you may use this file as part of a free software - * library without restriction. Specifically, if other files instantiate - * templates or use macros or inline functions from this file, or you compile - * this file and link it with other files to produce an executable, this - * file does not by itself cause the resulting executable to be covered by - * the GNU General Public License. This exception does not however - * invalidate any other reasons why the executable file might be covered by - * the GNU General Public License. - * - * Copyright 2015 Arduino LLC (http://www.arduino.cc/) - */ - -package builder - -import ( - "arduino.cc/builder/i18n" - "arduino.cc/builder/types" - "arduino.cc/builder/utils" -) - -func QueueSourceFilesFromFolder(queue *types.UniqueStringQueue, folder string, recurse bool) error { - extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } - - filePaths := []string{} - err := utils.FindFilesInFolder(&filePaths, folder, extensions, recurse) - if err != nil { - return i18n.WrapError(err) - } - - for _, filePath := range filePaths { - queue.Push(filePath) - } - - return nil -} diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index fa920c16..06aea4b3 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -52,10 +52,10 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { ctx.CollectedSourceFiles.Push(filepath.Join(sketchBuildPath, filepath.Base(sketch.MainFile.Name)+".cpp")) sourceFilePaths := ctx.CollectedSourceFiles - QueueSourceFilesFromFolder(sourceFilePaths, ctx.SketchBuildPath, /* recurse */ false) + queueSourceFilesFromFolder(sourceFilePaths, ctx.SketchBuildPath, /* recurse */ false) srcSubfolderPath := filepath.Join(ctx.SketchBuildPath, constants.SKETCH_FOLDER_SRC) if info, err := os.Stat(srcSubfolderPath); err == nil && info.IsDir() { - QueueSourceFilesFromFolder(sourceFilePaths, srcSubfolderPath, /* recurse */ true) + queueSourceFilesFromFolder(sourceFilePaths, srcSubfolderPath, /* recurse */ true) } for !sourceFilePaths.Empty() { @@ -114,7 +114,23 @@ func findIncludesUntilDone(ctx *types.Context, sourceFilePath string) error { ctx.IncludeFolders = append(ctx.IncludeFolders, library.SrcFolder) sourceFolders := types.LibraryToSourceFolder(library) for _, sourceFolder := range sourceFolders { - QueueSourceFilesFromFolder(ctx.CollectedSourceFiles, sourceFolder.Folder, sourceFolder.Recurse) + queueSourceFilesFromFolder(ctx.CollectedSourceFiles, sourceFolder.Folder, sourceFolder.Recurse) } } } + +func queueSourceFilesFromFolder(queue *types.UniqueStringQueue, folder string, recurse bool) error { + extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } + + filePaths := []string{} + err := utils.FindFilesInFolder(&filePaths, folder, extensions, recurse) + if err != nil { + return i18n.WrapError(err) + } + + for _, filePath := range filePaths { + queue.Push(filePath) + } + + return nil +} From c4cd9598c5bb3e29b2caedee815458353ce9a0d3 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 2 Aug 2016 18:00:47 +0200 Subject: [PATCH 12/12] Rename includes_to_include_folders.go to resolve_library.go Since 631b09f (Remove IncludesToIncludeFolders.Run), the primary function in this file changed to become ResolveLibrary. This updates the filename to match. No code is changed, this just renames a file. Signed-off-by: Matthijs Kooijman --- .../{includes_to_include_folders.go => resolve_library.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/arduino.cc/builder/{includes_to_include_folders.go => resolve_library.go} (100%) diff --git a/src/arduino.cc/builder/includes_to_include_folders.go b/src/arduino.cc/builder/resolve_library.go similarity index 100% rename from src/arduino.cc/builder/includes_to_include_folders.go rename to src/arduino.cc/builder/resolve_library.go