From 33a565a39550427d01a67b5a37a663a5ac0445b9 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 6 May 2016 11:37:57 +0200 Subject: [PATCH 01/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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 From c062430eb9db237d1fbf450e1ce0557e2da66716 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 2 Aug 2016 15:15:08 +0200 Subject: [PATCH 13/21] Add SourceFile struct This struct contains the path of a source file, together with the library or sketch it is contained in. This allows the SourceFile methods to generate the full paths to the source file, object file and dependency file, making it easier to pass around source files. This commit only adds the struct. Next commits will start to use this struct in the include detection, where it fills an immediate need, later it will be used in more places as well. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/types/types.go | 62 +++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/arduino.cc/builder/types/types.go b/src/arduino.cc/builder/types/types.go index 96f97955..d9826bca 100644 --- a/src/arduino.cc/builder/types/types.go +++ b/src/arduino.cc/builder/types/types.go @@ -31,6 +31,7 @@ package types import ( "os" + "fmt" "path/filepath" "strconv" @@ -38,6 +39,67 @@ import ( "arduino.cc/properties" ) +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 string +} + +// 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 string) (SourceFile, error) { + if filepath.IsAbs(path) { + var err error + path, err = filepath.Rel(sourceRoot(ctx, origin), 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{}) string { + switch o := origin.(type) { + case *Sketch: + return ctx.SketchBuildPath + case *Library: + return filepath.Join(ctx.LibrariesBuildPath, 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{}) string { + switch o := origin.(type) { + case *Sketch: + return ctx.SketchBuildPath + case *Library: + return o.SrcFolder + default: + panic("Unexpected origin for SourceFile: " + fmt.Sprint(origin)) + } +} + +func (f *SourceFile) SourcePath(ctx *Context) string { + return filepath.Join(sourceRoot(ctx, f.Origin), f.RelativePath) +} + +func (f *SourceFile) ObjectPath(ctx *Context) string { + return filepath.Join(buildRoot(ctx, f.Origin), f.RelativePath + ".o") +} + +func (f *SourceFile) DepfilePath(ctx *Context) string { + return filepath.Join(buildRoot(ctx, f.Origin), f.RelativePath + ".d") +} + type SketchFile struct { Name string Source string From 87f3397cc217caaaa076479b0fca4f4ec29f28a7 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 2 Aug 2016 15:17:33 +0200 Subject: [PATCH 14/21] Add UniqueSourceFileQueue This is a queue of SourceFile objects, similar to UniqueStringQueue. It is not used yet, only added. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/types/accessories.go | 23 +++++++++++++++++++++ src/arduino.cc/builder/types/utils.go | 9 ++++++++ 2 files changed, 32 insertions(+) diff --git a/src/arduino.cc/builder/types/accessories.go b/src/arduino.cc/builder/types/accessories.go index 3270ce26..6601bbaa 100644 --- a/src/arduino.cc/builder/types/accessories.go +++ b/src/arduino.cc/builder/types/accessories.go @@ -51,3 +51,26 @@ func (queue *UniqueStringQueue) Pop() interface{} { 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 +} diff --git a/src/arduino.cc/builder/types/utils.go b/src/arduino.cc/builder/types/utils.go index d7c6d40a..a7200f0e 100644 --- a/src/arduino.cc/builder/types/utils.go +++ b/src/arduino.cc/builder/types/utils.go @@ -38,3 +38,12 @@ 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 == target.RelativePath { + return true + } + } + return false +} From 0c64d158c7ff9b20aeff0691fab281d19ad78a07 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 2 Aug 2016 15:21:33 +0200 Subject: [PATCH 15/21] Add appendIncludeFolder helper function This helper does not do much, but it will be expanded in a later commit. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/container_find_includes.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 06aea4b3..adf5f6ca 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -42,9 +42,9 @@ import ( type ContainerFindIncludes struct{} func (s *ContainerFindIncludes) Run(ctx *types.Context) error { - ctx.IncludeFolders = append(ctx.IncludeFolders, ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_CORE_PATH]) + appendIncludeFolder(ctx, 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]) + appendIncludeFolder(ctx, ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH]) } sketchBuildPath := ctx.SketchBuildPath @@ -73,6 +73,11 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { return nil } +// Append the given folder to the include path. +func appendIncludeFolder(ctx *types.Context, folder string) { + ctx.IncludeFolders = append(ctx.IncludeFolders, folder) +} + func runCommand(ctx *types.Context, command types.Command) error { PrintRingNameIfDebug(ctx, command) err := command.Run(ctx) @@ -111,7 +116,7 @@ func findIncludesUntilDone(ctx *types.Context, sourceFilePath string) error { // include path and queue its source files for further // include scanning ctx.ImportedLibraries = append(ctx.ImportedLibraries, library) - ctx.IncludeFolders = append(ctx.IncludeFolders, library.SrcFolder) + appendIncludeFolder(ctx, library.SrcFolder) sourceFolders := types.LibraryToSourceFolder(library) for _, sourceFolder := range sourceFolders { queueSourceFilesFromFolder(ctx.CollectedSourceFiles, sourceFolder.Folder, sourceFolder.Recurse) From c1c9b1b584c64ef504a067696810ff82226c6a89 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 2 Aug 2016 15:46:00 +0200 Subject: [PATCH 16/21] Store SourceFiles in Context.CollectedSourceFiles Previously, the include detection kept a queue of strings representing full paths to source files that still needed to be processed and iterated over it. This commit turns this into a queue containing the (new) SourceFile struct. This gives the include detection code a bit more context about a source file, allowing to also generate the object and dependency file paths. Previously this was not feasible, since it did not know the origin (library/sketch) of a path, so it could not decide where within the build path the object file should live. This does not actually use this new context yet, but it will be useful when introducing caching next. Signed-off-by: Matthijs Kooijman --- .../add_additional_entries_to_context.go | 2 +- .../builder/container_find_includes.go | 29 ++++++++++++------- src/arduino.cc/builder/types/context.go | 2 +- 3 files changed, 20 insertions(+), 13 deletions(-) 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 4a6e7c46..91bc2a0e 100644 --- a/src/arduino.cc/builder/add_additional_entries_to_context.go +++ b/src/arduino.cc/builder/add_additional_entries_to_context.go @@ -68,7 +68,7 @@ func (s *AddAdditionalEntriesToContext) Run(ctx *types.Context) error { ctx.WarningsLevel = DEFAULT_WARNINGS_LEVEL } - ctx.CollectedSourceFiles = &types.UniqueStringQueue{} + ctx.CollectedSourceFiles = &types.UniqueSourceFileQueue{} ctx.LibrariesResolutionResults = make(map[string]types.LibraryResolutionResult) ctx.HardwareRewriteResults = make(map[*types.Platform][]types.PlatforKeyRewrite) diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index adf5f6ca..8d5d7346 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -47,25 +47,28 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { appendIncludeFolder(ctx, ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH]) } - sketchBuildPath := ctx.SketchBuildPath sketch := ctx.Sketch - ctx.CollectedSourceFiles.Push(filepath.Join(sketchBuildPath, filepath.Base(sketch.MainFile.Name)+".cpp")) + mergedfile, err := types.MakeSourceFile(ctx, sketch, filepath.Base(sketch.MainFile.Name)+".cpp") + if err != nil { + return i18n.WrapError(err) + } + ctx.CollectedSourceFiles.Push(mergedfile) sourceFilePaths := ctx.CollectedSourceFiles - queueSourceFilesFromFolder(sourceFilePaths, ctx.SketchBuildPath, /* recurse */ false) + queueSourceFilesFromFolder(ctx, sourceFilePaths, sketch, 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(ctx, sourceFilePaths, sketch, srcSubfolderPath, /* recurse */ true) } for !sourceFilePaths.Empty() { - err := findIncludesUntilDone(ctx, sourceFilePaths.Pop().(string)) + err := findIncludesUntilDone(ctx, sourceFilePaths.Pop()) if err != nil { return i18n.WrapError(err) } } - err := runCommand(ctx, &FailIfImportedLibraryIsWrong{}) + err = runCommand(ctx, &FailIfImportedLibraryIsWrong{}) if err != nil { return i18n.WrapError(err) } @@ -87,11 +90,11 @@ func runCommand(ctx *types.Context, command types.Command) error { return nil } -func findIncludesUntilDone(ctx *types.Context, sourceFilePath string) error { +func findIncludesUntilDone(ctx *types.Context, sourceFile types.SourceFile) error { targetFilePath := utils.NULLFile() for { commands := []types.Command{ - &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourceFilePath, TargetFilePath: targetFilePath}, + &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourceFile.SourcePath(ctx), TargetFilePath: targetFilePath}, &IncludesFinderWithRegExp{Source: &ctx.SourceGccMinusE}, } for _, command := range commands { @@ -119,12 +122,12 @@ func findIncludesUntilDone(ctx *types.Context, sourceFilePath string) error { appendIncludeFolder(ctx, library.SrcFolder) sourceFolders := types.LibraryToSourceFolder(library) for _, sourceFolder := range sourceFolders { - queueSourceFilesFromFolder(ctx.CollectedSourceFiles, sourceFolder.Folder, sourceFolder.Recurse) + queueSourceFilesFromFolder(ctx, ctx.CollectedSourceFiles, library, sourceFolder.Folder, sourceFolder.Recurse) } } } -func queueSourceFilesFromFolder(queue *types.UniqueStringQueue, folder string, recurse bool) error { +func queueSourceFilesFromFolder(ctx *types.Context, queue *types.UniqueSourceFileQueue, origin interface{}, folder string, recurse bool) error { extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] } filePaths := []string{} @@ -134,7 +137,11 @@ func queueSourceFilesFromFolder(queue *types.UniqueStringQueue, folder string, r } for _, filePath := range filePaths { - queue.Push(filePath) + sourceFile, err := types.MakeSourceFile(ctx, origin, filePath) + if (err != nil) { + return i18n.WrapError(err) + } + queue.Push(sourceFile) } return nil diff --git a/src/arduino.cc/builder/types/context.go b/src/arduino.cc/builder/types/context.go index d1421deb..6fb9f27b 100644 --- a/src/arduino.cc/builder/types/context.go +++ b/src/arduino.cc/builder/types/context.go @@ -46,7 +46,7 @@ type Context struct { PreprocPath string SketchObjectFiles []string - CollectedSourceFiles *UniqueStringQueue + CollectedSourceFiles *UniqueSourceFileQueue Sketch *Sketch Source string From 8b70ca72ca32588af7bdbf8dc72a24bda7ed1cce Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 2 Aug 2016 16:23:47 +0200 Subject: [PATCH 17/21] Document the include detection process Signed-off-by: Matthijs Kooijman --- .../builder/container_find_includes.go | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 8d5d7346..13696a67 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -27,6 +27,40 @@ * Copyright 2015 Arduino LLC (http://www.arduino.cc/) */ +/* + +Include detection + +This code is responsible for figuring out what libraries the current +sketch needs an populates both Context.ImportedLibraries with a list of +Library objects, and Context.IncludeFolders with a list of folders to +put on the include path. + +Simply put, every #include in a source file pulls in the library that +provides that source file. This includes source files in the selected +libraries, so libraries can recursively include other libraries as well. + +To implement this, the gcc preprocessor is used. A queue is created +containing, at first, the source files in the sketch. Each of the files +in the queue is processed in turn by running the preprocessor on it. If +the preprocessor provides an error, the output is examined to see if the +error is a missing header file originating from a #include directive. + +The filename is extracted from that #include directive, and a library is +found that provides it. If multiple libraries provide the same file, one +is slected (how this selection works is not described here, see the +ResolveLibrary function for that). The library selected in this way is +added to the include path through Context.IncludeFolders and the list of +libraries to include in the link through Context.ImportedLibraries. + +Furthermore, all of the library source files are added to the queue, to +be processed as well. When the preprocessor completes without showing an +#include error, processing of the file is complete and it advances to +the next. When no library can be found for a included filename, an error +is shown and the process is aborted. + +*/ + package builder import ( From 874b3b68d9cbcce18af78136f8dba2c8e05f7ed9 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 2 Aug 2016 17:25:53 +0200 Subject: [PATCH 18/21] Cache the results of include detection This greatly reduces compilation time of big sketches (or sketches using big libraries) when only a small change has been mad. Instead of rerunning include detection for *all* source files, it is now only rerun for changed files (and usually more if the actual list of includes changed). Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/constants/constants.go | 2 + .../builder/container_find_includes.go | 230 ++++++++++++++++-- 2 files changed, 215 insertions(+), 17 deletions(-) diff --git a/src/arduino.cc/builder/constants/constants.go b/src/arduino.cc/builder/constants/constants.go index bf5cfc35..62147d62 100644 --- a/src/arduino.cc/builder/constants/constants.go +++ b/src/arduino.cc/builder/constants/constants.go @@ -84,6 +84,7 @@ const FILE_PLATFORM_KEYS_REWRITE_TXT = "platform.keys.rewrite.txt" const FILE_PLATFORM_LOCAL_TXT = "platform.local.txt" const FILE_PLATFORM_TXT = "platform.txt" const FILE_PROGRAMMERS_TXT = "programmers.txt" +const FILE_INCLUDES_CACHE = "includes.cache" const FOLDER_BOOTLOADERS = "bootloaders" const FOLDER_CORE = "core" const FOLDER_CORES = "cores" @@ -180,6 +181,7 @@ const MSG_USING_LIBRARY = "Using library {0} in folder: {1} {2}" const MSG_USING_BOARD = "Using board '{0}' from platform in folder: {1}" const MSG_USING_CORE = "Using core '{0}' from platform in folder: {1}" const MSG_USING_PREVIOUS_COMPILED_FILE = "Using previously compiled file: {0}" +const MSG_USING_CACHED_INCLUDES = "Using cached library dependencies for file: {0}" const MSG_WARNING_LIB_INVALID_CATEGORY = "WARNING: Category '{0}' in library {1} is not valid. Setting to '{2}'" const MSG_WARNING_PLATFORM_MISSING_VALUE = "Warning: platform.txt from core '{0}' misses property '{1}', using default value '{2}'. Consider upgrading this core." const MSG_WARNING_PLATFORM_OLD_VALUES = "Warning: platform.txt from core '{0}' contains deprecated {1}, automatically converted to {2}. Consider upgrading this core." diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 13696a67..6083155b 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -59,14 +59,61 @@ be processed as well. When the preprocessor completes without showing an the next. When no library can be found for a included filename, an error is shown and the process is aborted. +Caching + +Since this process is fairly slow (requiring at least one invocation of +the preprocessor per source file), its results are cached. + +Just caching the complete result (i.e. the resulting list of imported +libraries) seems obvious, but such a cache is hard to invalidate. Making +a list of all the source and header files used to create the list and +check if any of them changed is probably feasible, but this would also +require caching the full list of libraries to invalidate the cache when +the include to library resolution might have a different result. Another +downside of a complete cache is that any changes requires re-running +everything, even if no includes were actually changed. + +Instead, caching happens by keeping a sort of "journal" of the steps in +the include detection, essentially tracing each file processed and each +include path entry added. The cache is used by retracing these steps: +The include detection process is executed normally, except that instead +of running the preprocessor, the include filenames are (when possible) +read from the cache. Then, the include file to library resolution is +again executed normally. The results are checked against the cache and +as long as the results match, the cache is considered valid. + +When a source file (or any of the files it includes, as indicated by the +.d file) is changed, the preprocessor is executed as normal for the +file, ignoring any includes from the cache. This does not, however, +invalidate the cache: If the results from the preprocessor match the +entries in the cache, the cache remains valid and can again be used for +the next (unchanged) file. + +The cache file uses the JSON format and contains a list of entries. Each +entry represents a discovered library and contains: + - Sourcefile: The source file that the include was found in + - Include: The included filename found + - Includepath: The addition to the include path + +There are also some special entries: + - When adding the initial include path entries, such as for the core + and variant paths. These are not discovered, so the Sourcefile and + Include fields will be empty. + - When a file contains no (more) missing includes, an entry with an + empty Include and IncludePath is generated. + */ package builder import ( + "encoding/json" + "io/ioutil" "os" "path/filepath" + "time" + "arduino.cc/builder/builder_utils" "arduino.cc/builder/constants" "arduino.cc/builder/i18n" "arduino.cc/builder/types" @@ -76,9 +123,12 @@ import ( type ContainerFindIncludes struct{} func (s *ContainerFindIncludes) Run(ctx *types.Context) error { - appendIncludeFolder(ctx, ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_CORE_PATH]) + cachePath := filepath.Join(ctx.BuildPath, constants.FILE_INCLUDES_CACHE) + cache := readCache(cachePath) + + appendIncludeFolder(ctx, cache, "", "", ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_CORE_PATH]) if ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH] != constants.EMPTY_STRING { - appendIncludeFolder(ctx, ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH]) + appendIncludeFolder(ctx, cache, "", "", ctx.BuildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH]) } sketch := ctx.Sketch @@ -96,12 +146,20 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { } for !sourceFilePaths.Empty() { - err := findIncludesUntilDone(ctx, sourceFilePaths.Pop()) + err := findIncludesUntilDone(ctx, cache, sourceFilePaths.Pop()) if err != nil { + os.Remove(cachePath) return i18n.WrapError(err) } } + // Finalize the cache + cache.ExpectEnd() + err = writeCache(cache, cachePath) + if err != nil { + return i18n.WrapError(err) + } + err = runCommand(ctx, &FailIfImportedLibraryIsWrong{}) if err != nil { return i18n.WrapError(err) @@ -110,9 +168,14 @@ func (s *ContainerFindIncludes) Run(ctx *types.Context) error { return nil } -// Append the given folder to the include path. -func appendIncludeFolder(ctx *types.Context, folder string) { +// 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 appendIncludeFolder(ctx *types.Context, cache *includeCache, sourceFilePath string, include string, folder string) { ctx.IncludeFolders = append(ctx.IncludeFolders, folder) + cache.ExpectEntry(sourceFilePath, include, folder) } func runCommand(ctx *types.Context, command types.Command) error { @@ -124,25 +187,157 @@ func runCommand(ctx *types.Context, command types.Command) error { return nil } -func findIncludesUntilDone(ctx *types.Context, sourceFile types.SourceFile) error { +type includeCacheEntry struct { + Sourcefile string + Include string + Includepath string +} + +type includeCache struct { + // Are the cache contents valid so far? + valid bool + // Index into entries of the next entry to be processed. Unused + // when the cache is invalid. + next int + 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 { + 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. +func (cache *includeCache) ExpectFile(sourcefile string) { + if cache.valid && cache.next < len(cache.entries) && cache.Next().Sourcefile != sourcefile { + cache.valid = false + cache.entries = cache.entries[:cache.next] + } +} + +// 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 string, include string, librarypath string) { + entry := includeCacheEntry{Sourcefile: sourcefile, Include: include, Includepath: librarypath} + if cache.valid { + if cache.next < len(cache.entries) && cache.Next() == entry { + cache.next++ + } else { + cache.valid = false + cache.entries = cache.entries[:cache.next] + } + } + + if !cache.valid { + cache.entries = append(cache.entries, entry) + } +} + +// 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] + } +} + +// Read the cache from the given file +func readCache(path string) *includeCache { + bytes, err := ioutil.ReadFile(path) + if err != nil { + // Return an empty, invalid cache + return &includeCache{} + } + result := &includeCache{} + err = json.Unmarshal(bytes, &result.entries) + if err != nil { + // Return an empty, invalid cache + return &includeCache{} + } + result.valid = true + return result +} + +// Write the given cache to the given file if it is invalidated. If the +// cache is still valid, just update the timestamps of the file. +func writeCache(cache *includeCache, path string) 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 { + os.Chtimes(path, time.Now(), time.Now()) + } else { + bytes, err := json.MarshalIndent(cache.entries, "", " ") + if err != nil { + return i18n.WrapError(err) + } + err = utils.WriteFileBytes(path, bytes) + if err != nil { + return i18n.WrapError(err) + } + } + return nil +} + +func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile types.SourceFile) error { + sourcePath := sourceFile.SourcePath(ctx) targetFilePath := utils.NULLFile() + + // TODO: This should perhaps also compare against the + // include.cache file timestamp. Now, it only checks if the file + // changed after the object file was generated, but if it + // changed between generating the cache and the object file, + // this could show the file as unchanged when it really is + // changed. Changing files during a build isn't really + // supported, but any problems from it should at least be + // resolved when doing another build, which is not currently the + // case. + // 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(sourcePath, sourceFile.ObjectPath(ctx), sourceFile.DepfilePath(ctx)) + if err != nil { + return i18n.WrapError(err) + } + + first := true for { - commands := []types.Command{ - &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourceFile.SourcePath(ctx), TargetFilePath: targetFilePath}, - &IncludesFinderWithRegExp{Source: &ctx.SourceGccMinusE}, - } - for _, command := range commands { - err := runCommand(ctx, command) - if err != nil { - return i18n.WrapError(err) + var include string + cache.ExpectFile(sourcePath) + if unchanged && cache.valid { + include = cache.Next().Include + if first && ctx.Verbose { + ctx.GetLogger().Println(constants.LOG_LEVEL_INFO, constants.MSG_USING_CACHED_INCLUDES, sourcePath) } + } else { + commands := []types.Command{ + &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourcePath, TargetFilePath: targetFilePath}, + &IncludesFinderWithRegExp{Source: &ctx.SourceGccMinusE}, + } + for _, command := range commands { + err := runCommand(ctx, command) + if err != nil { + return i18n.WrapError(err) + } + } + include = ctx.IncludeJustFound } - if ctx.IncludeJustFound == "" { + + if include == "" { // No missing includes found, we're done + cache.ExpectEntry(sourcePath, "", "") return nil } - library := ResolveLibrary(ctx, ctx.IncludeJustFound) + library := ResolveLibrary(ctx, include) if library == nil { // Library could not be resolved, show error err := runCommand(ctx, &GCCPreprocRunner{TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}) @@ -153,11 +348,12 @@ func findIncludesUntilDone(ctx *types.Context, sourceFile types.SourceFile) erro // include path and queue its source files for further // include scanning ctx.ImportedLibraries = append(ctx.ImportedLibraries, library) - appendIncludeFolder(ctx, library.SrcFolder) + appendIncludeFolder(ctx, cache, sourcePath, include, library.SrcFolder) sourceFolders := types.LibraryToSourceFolder(library) for _, sourceFolder := range sourceFolders { queueSourceFilesFromFolder(ctx, ctx.CollectedSourceFiles, library, sourceFolder.Folder, sourceFolder.Recurse) } + first = false } } From b7d8f28a5a9bec4ed0ad99091a64ad5f687db079 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 3 Aug 2016 21:43:45 +0200 Subject: [PATCH 19/21] Show errors during include detection properly When an include could not be resolved, the preprocessor was ran again to show the include error as reported by the compiler to the user. However, the preprocessor was now always run against the main sketch, instead of the file currently being processed. Unless the problem was in fact in the main sketch, this caused no error to be shown and include detection continued with the next file instead of being aborted as intended. This fixes the problem by running the preprocessor on the right file. In the future, this error handling might be a bit more improved, but for now this should be sufficient. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/container_add_prototypes.go | 5 ++++- src/arduino.cc/builder/container_find_includes.go | 2 +- src/arduino.cc/builder/gcc_preproc_runner.go | 5 ++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/arduino.cc/builder/container_add_prototypes.go b/src/arduino.cc/builder/container_add_prototypes.go index 8a138b44..004c85f9 100644 --- a/src/arduino.cc/builder/container_add_prototypes.go +++ b/src/arduino.cc/builder/container_add_prototypes.go @@ -30,6 +30,8 @@ package builder import ( + "path/filepath" + "arduino.cc/builder/constants" "arduino.cc/builder/ctags" "arduino.cc/builder/i18n" @@ -39,8 +41,9 @@ import ( type ContainerAddPrototypes struct{} func (s *ContainerAddPrototypes) Run(ctx *types.Context) error { + sourceFile := filepath.Join(ctx.SketchBuildPath, filepath.Base(ctx.Sketch.MainFile.Name)+".cpp") commands := []types.Command{ - &GCCPreprocRunner{TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}, + &GCCPreprocRunner{SourceFilePath: sourceFile, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}, &ReadFileAndStoreInContext{Target: &ctx.SourceGccMinusE}, &FilterSketchSource{Source: &ctx.SourceGccMinusE}, &CTagsTargetFileSaver{Source: &ctx.SourceGccMinusE, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}, diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index 6083155b..ab480fa2 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -340,7 +340,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t library := ResolveLibrary(ctx, include) if library == nil { // Library could not be resolved, show error - err := runCommand(ctx, &GCCPreprocRunner{TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}) + err := runCommand(ctx, &GCCPreprocRunner{SourceFilePath: sourcePath, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}) return i18n.WrapError(err) } diff --git a/src/arduino.cc/builder/gcc_preproc_runner.go b/src/arduino.cc/builder/gcc_preproc_runner.go index 193aa59b..e12141e1 100644 --- a/src/arduino.cc/builder/gcc_preproc_runner.go +++ b/src/arduino.cc/builder/gcc_preproc_runner.go @@ -42,13 +42,12 @@ import ( ) type GCCPreprocRunner struct { + SourceFilePath string TargetFileName string } func (s *GCCPreprocRunner) Run(ctx *types.Context) error { - sketchBuildPath := ctx.SketchBuildPath - sketch := ctx.Sketch - properties, targetFilePath, err := prepareGCCPreprocRecipeProperties(ctx, filepath.Join(sketchBuildPath, filepath.Base(sketch.MainFile.Name)+".cpp"), s.TargetFileName) + properties, targetFilePath, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.TargetFileName) if err != nil { return i18n.WrapError(err) } From 7dd4e1c30409f0498e8a57be96dcadb9a36d1089 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 3 Aug 2016 22:43:14 +0200 Subject: [PATCH 20/21] Add Library.UtilityFolder This path is constructed and checked when loading the library instead of in various places among the code, slightly simplifying the code. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/libraries_loader.go | 12 +++++++++++- src/arduino.cc/builder/phases/libraries_builder.go | 12 ++++-------- src/arduino.cc/builder/types/types.go | 9 +++------ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/arduino.cc/builder/libraries_loader.go b/src/arduino.cc/builder/libraries_loader.go index 1b3d3d23..ba2a0c7a 100644 --- a/src/arduino.cc/builder/libraries_loader.go +++ b/src/arduino.cc/builder/libraries_loader.go @@ -113,6 +113,14 @@ func makeLibrary(libraryFolder string, debugLevel int, logger i18n.Logger) (*typ return makeNewLibrary(libraryFolder, debugLevel, logger) } +func addUtilityFolder(library *types.Library) { + utilitySourcePath := filepath.Join(library.Folder, constants.LIBRARY_FOLDER_UTILITY) + stat, err := os.Stat(utilitySourcePath) + if err == nil && stat.IsDir() { + library.UtilityFolder = utilitySourcePath + } +} + func makeNewLibrary(libraryFolder string, debugLevel int, logger i18n.Logger) (*types.Library, error) { libProperties, err := properties.Load(filepath.Join(libraryFolder, constants.LIBRARY_PROPERTIES), logger) if err != nil { @@ -130,12 +138,14 @@ func makeNewLibrary(libraryFolder string, debugLevel int, logger i18n.Logger) (* } library := &types.Library{} + library.Folder = libraryFolder if stat, err := os.Stat(filepath.Join(libraryFolder, constants.LIBRARY_FOLDER_SRC)); err == nil && stat.IsDir() { library.Layout = types.LIBRARY_RECURSIVE library.SrcFolder = filepath.Join(libraryFolder, constants.LIBRARY_FOLDER_SRC) } else { library.Layout = types.LIBRARY_FLAT library.SrcFolder = libraryFolder + addUtilityFolder(library) } subFolders, err := utils.ReadDirFiltered(libraryFolder, utils.FilterDirs) @@ -173,7 +183,6 @@ func makeNewLibrary(libraryFolder string, debugLevel int, logger i18n.Logger) (* } library.License = libProperties[constants.LIBRARY_LICENSE] - library.Folder = libraryFolder library.Name = filepath.Base(libraryFolder) library.Version = strings.TrimSpace(libProperties[constants.LIBRARY_VERSION]) library.Author = strings.TrimSpace(libProperties[constants.LIBRARY_AUTHOR]) @@ -197,6 +206,7 @@ func makeLegacyLibrary(libraryFolder string) (*types.Library, error) { Archs: []string{constants.LIBRARY_ALL_ARCHS}, IsLegacy: true, } + addUtilityFolder(library) return library, nil } diff --git a/src/arduino.cc/builder/phases/libraries_builder.go b/src/arduino.cc/builder/phases/libraries_builder.go index 18785cfe..816c512f 100644 --- a/src/arduino.cc/builder/phases/libraries_builder.go +++ b/src/arduino.cc/builder/phases/libraries_builder.go @@ -30,7 +30,6 @@ package phases import ( - "os" "path/filepath" "arduino.cc/builder/builder_utils" @@ -107,20 +106,17 @@ func compileLibrary(library *types.Library, buildPath string, buildProperties pr objectFiles = []string{archiveFile} } } else { - utilitySourcePath := filepath.Join(library.SrcFolder, constants.LIBRARY_FOLDER_UTILITY) - stat, err := os.Stat(utilitySourcePath) - haveUtilityDir := err == nil && stat.IsDir() - if haveUtilityDir { - includes = append(includes, utils.WrapWithHyphenI(utilitySourcePath)) + if library.UtilityFolder != "" { + includes = append(includes, utils.WrapWithHyphenI(library.UtilityFolder)) } objectFiles, err = builder_utils.CompileFiles(objectFiles, library.SrcFolder, false, libraryBuildPath, buildProperties, includes, verbose, warningsLevel, logger) if err != nil { return nil, i18n.WrapError(err) } - if haveUtilityDir { + if library.UtilityFolder != "" { utilityBuildPath := filepath.Join(libraryBuildPath, constants.LIBRARY_FOLDER_UTILITY) - objectFiles, err = builder_utils.CompileFiles(objectFiles, utilitySourcePath, false, utilityBuildPath, buildProperties, includes, verbose, warningsLevel, logger) + objectFiles, err = builder_utils.CompileFiles(objectFiles, library.UtilityFolder, false, utilityBuildPath, buildProperties, includes, verbose, warningsLevel, logger) if err != nil { return nil, i18n.WrapError(err) } diff --git a/src/arduino.cc/builder/types/types.go b/src/arduino.cc/builder/types/types.go index d9826bca..842fc817 100644 --- a/src/arduino.cc/builder/types/types.go +++ b/src/arduino.cc/builder/types/types.go @@ -30,7 +30,6 @@ package types import ( - "os" "fmt" "path/filepath" "strconv" @@ -166,6 +165,7 @@ const ( type Library struct { Folder string SrcFolder string + UtilityFolder string Layout LibraryLayout Name string Archs []string @@ -262,11 +262,8 @@ func LibraryToSourceFolder(library *Library) []SourceFolder { sourceFolders := []SourceFolder{} recurse := library.Layout == LIBRARY_RECURSIVE sourceFolders = append(sourceFolders, SourceFolder{Folder: library.SrcFolder, Recurse: recurse}) - if library.Layout == LIBRARY_FLAT { - utility := filepath.Join(library.SrcFolder, constants.LIBRARY_FOLDER_UTILITY) - if info, err := os.Stat(utility); err == nil && info.IsDir() { - sourceFolders = append(sourceFolders, SourceFolder{Folder: utility, Recurse: false}) - } + if library.UtilityFolder != "" { + sourceFolders = append(sourceFolders, SourceFolder{Folder: library.UtilityFolder, Recurse: false}) } return sourceFolders } From 6f5e2422d5e4d9b43bab31d3255bf5689ca7f3a9 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 3 Aug 2016 22:45:46 +0200 Subject: [PATCH 21/21] Put utility folders in the include path during include detection When compiling source files from a library, the utility folder of the library itself is in the include path. When doing include detection, this did not happen, which could lead to compilation errors. Because of the broken (but recently fixed) errorhandling in include detection, this error was not visible to the user. It would only have a visible effect if a library included another library *after* including a file from its utility folder through the include path, and that other library was not otherwise pulled in. In practice, this would almost never occur, but with the new caching the cache would be invalidated and this problem became visible. This is fixed by simply including the utility folder in the include path during include detection. This required explicitly passing the include path through the GCCPreprocRunnerForDiscoveringIncludes and GCCPreprocRunner. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/container_add_prototypes.go | 2 +- src/arduino.cc/builder/container_find_includes.go | 9 +++++++-- src/arduino.cc/builder/gcc_preproc_runner.go | 9 +++++---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/arduino.cc/builder/container_add_prototypes.go b/src/arduino.cc/builder/container_add_prototypes.go index 004c85f9..378a0689 100644 --- a/src/arduino.cc/builder/container_add_prototypes.go +++ b/src/arduino.cc/builder/container_add_prototypes.go @@ -43,7 +43,7 @@ type ContainerAddPrototypes struct{} func (s *ContainerAddPrototypes) Run(ctx *types.Context) error { sourceFile := filepath.Join(ctx.SketchBuildPath, filepath.Base(ctx.Sketch.MainFile.Name)+".cpp") commands := []types.Command{ - &GCCPreprocRunner{SourceFilePath: sourceFile, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}, + &GCCPreprocRunner{SourceFilePath: sourceFile, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, Includes: ctx.IncludeFolders}, &ReadFileAndStoreInContext{Target: &ctx.SourceGccMinusE}, &FilterSketchSource{Source: &ctx.SourceGccMinusE}, &CTagsTargetFileSaver{Source: &ctx.SourceGccMinusE, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}, diff --git a/src/arduino.cc/builder/container_find_includes.go b/src/arduino.cc/builder/container_find_includes.go index ab480fa2..096e4a60 100644 --- a/src/arduino.cc/builder/container_find_includes.go +++ b/src/arduino.cc/builder/container_find_includes.go @@ -312,6 +312,11 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t for { var include string cache.ExpectFile(sourcePath) + + includes := ctx.IncludeFolders + if library, ok := sourceFile.Origin.(*types.Library); ok && library.UtilityFolder != "" { + includes = append(includes, library.UtilityFolder) + } if unchanged && cache.valid { include = cache.Next().Include if first && ctx.Verbose { @@ -319,7 +324,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t } } else { commands := []types.Command{ - &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourcePath, TargetFilePath: targetFilePath}, + &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourcePath, TargetFilePath: targetFilePath, Includes: includes}, &IncludesFinderWithRegExp{Source: &ctx.SourceGccMinusE}, } for _, command := range commands { @@ -340,7 +345,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t library := ResolveLibrary(ctx, include) if library == nil { // Library could not be resolved, show error - err := runCommand(ctx, &GCCPreprocRunner{SourceFilePath: sourcePath, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}) + err := runCommand(ctx, &GCCPreprocRunner{SourceFilePath: sourcePath, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, Includes: includes}) return i18n.WrapError(err) } diff --git a/src/arduino.cc/builder/gcc_preproc_runner.go b/src/arduino.cc/builder/gcc_preproc_runner.go index e12141e1..962df725 100644 --- a/src/arduino.cc/builder/gcc_preproc_runner.go +++ b/src/arduino.cc/builder/gcc_preproc_runner.go @@ -44,10 +44,11 @@ import ( type GCCPreprocRunner struct { SourceFilePath string TargetFileName string + Includes []string } func (s *GCCPreprocRunner) Run(ctx *types.Context) error { - properties, targetFilePath, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.TargetFileName) + properties, targetFilePath, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.TargetFileName, s.Includes) if err != nil { return i18n.WrapError(err) } @@ -72,10 +73,11 @@ func (s *GCCPreprocRunner) Run(ctx *types.Context) error { type GCCPreprocRunnerForDiscoveringIncludes struct { SourceFilePath string TargetFilePath string + Includes []string } func (s *GCCPreprocRunnerForDiscoveringIncludes) Run(ctx *types.Context) error { - properties, _, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.TargetFilePath) + properties, _, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.TargetFilePath, s.Includes) if err != nil { return i18n.WrapError(err) } @@ -98,7 +100,7 @@ func (s *GCCPreprocRunnerForDiscoveringIncludes) Run(ctx *types.Context) error { return nil } -func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, targetFilePath string) (properties.Map, string, error) { +func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (properties.Map, string, error) { if targetFilePath != utils.NULLFile() { preprocPath := ctx.PreprocPath err := utils.EnsureFolderExists(preprocPath) @@ -112,7 +114,6 @@ func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string properties[constants.BUILD_PROPERTIES_SOURCE_FILE] = sourceFilePath properties[constants.BUILD_PROPERTIES_PREPROCESSED_FILE_PATH] = targetFilePath - includes := ctx.IncludeFolders includes = utils.Map(includes, utils.WrapWithHyphenI) properties[constants.BUILD_PROPERTIES_INCLUDES] = strings.Join(includes, constants.SPACE) builder_utils.RemoveHyphenMDDFlagFromGCCCommandLine(properties)