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 <matthijs@stdin.nl>

Author: matthijskooijman

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)
 	}

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
 }
 

src/arduino.cc/builder/includes_to_include_folders.go

@@ -36,22 +36,26 @@ 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
 	actualPlatform := ctx.ActualPlatform
 	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?!?

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 <SPI.h>\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)
 }

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