[skip-changelog] Legacy removing (take 2) (#1664)

* Fixed all lint warnings * Replaced findFilesInFolder with go-paths equivalent * Removed some constants * Removed unused parameter * Removed redundant builder_utils.CompileFilesRecursive function * Simplified structure of CompileFiles * Inlined call to compileFilesWithRecipe * Removed unused function FindAllSubdirectories * Rewrite of FindFilesInFolder using go-paths-helper * Updated dependency go.sum and licensed cache * Removed duplication of extensions table * Applied suggestion from codereview * Update legacy/builder/builder_utils/utils.go Co-authored-by: Silvano Cerza <[email protected]> * Applied code review suggestions * Applied code review suggestions Co-authored-by: Silvano Cerza <[email protected]>
arduino · Feb 17, 2022 · 56a2ccd · 56a2ccd
1 parent 10107d2
commit 56a2ccd
Show file tree

Hide file tree

Showing 26 changed files with 212 additions and 326 deletions.
diff --git a/.licenses/go/github.com/arduino/go-paths-helper.dep.yml b/.licenses/go/github.com/arduino/go-paths-helper.dep.yml
@@ -1,8 +1,8 @@
 ---
 name: github.com/arduino/go-paths-helper
-version: v1.6.1
+version: v1.7.0
 type: go
-summary:
+summary: 
 homepage: https://pkg.go.dev/github.com/arduino/go-paths-helper
 license: gpl-2.0-or-later
 licenses:

diff --git a/arduino/discovery/discovery_client/go.sum b/arduino/discovery/discovery_client/go.sum
@@ -45,8 +45,8 @@ github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kd
 github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c/go.mod h1:HK7SpkEax/3P+0w78iRQx1sz1vCDYYw9RXwHjQTB5i8=
 github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
 github.com/arduino/go-paths-helper v1.2.0/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
-github.com/arduino/go-paths-helper v1.6.1 h1:lha+/BuuBsx0qTZ3gy6IO1kU23lObWdQ/UItkzVWQ+0=
-github.com/arduino/go-paths-helper v1.6.1/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
+github.com/arduino/go-paths-helper v1.7.0 h1:S9l5BP2aogz1CgyqqnncXt0PLpK4yvwOW/wu/LaR3tc=
+github.com/arduino/go-paths-helper v1.7.0/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
 github.com/arduino/go-properties-orderedmap v1.6.0 h1:gp2JoWRETtqwsZ+UHu/PBuYWYH2x2+d+uipDxS4WmvM=
 github.com/arduino/go-properties-orderedmap v1.6.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk=
 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b/go.mod h1:uwGy5PpN4lqW97FiLnbcx+xx8jly5YuPMJWfVwwjJiQ=

diff --git a/client_example/go.sum b/client_example/go.sum
@@ -45,7 +45,7 @@ github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kd
 github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c/go.mod h1:HK7SpkEax/3P+0w78iRQx1sz1vCDYYw9RXwHjQTB5i8=
 github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
 github.com/arduino/go-paths-helper v1.2.0/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
-github.com/arduino/go-paths-helper v1.6.1/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
+github.com/arduino/go-paths-helper v1.7.0/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
 github.com/arduino/go-properties-orderedmap v1.6.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk=
 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b/go.mod h1:uwGy5PpN4lqW97FiLnbcx+xx8jly5YuPMJWfVwwjJiQ=
 github.com/arduino/go-win32-utils v0.0.0-20180330194947-ed041402e83b/go.mod h1:iIPnclBMYm1g32Q5kXoqng4jLhMStReIP7ZxaoUC2y8=

diff --git a/docsgen/go.sum b/docsgen/go.sum
@@ -48,8 +48,8 @@ github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c h1:agh2JT9
 github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c/go.mod h1:HK7SpkEax/3P+0w78iRQx1sz1vCDYYw9RXwHjQTB5i8=
 github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
 github.com/arduino/go-paths-helper v1.2.0/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
-github.com/arduino/go-paths-helper v1.6.1 h1:lha+/BuuBsx0qTZ3gy6IO1kU23lObWdQ/UItkzVWQ+0=
-github.com/arduino/go-paths-helper v1.6.1/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
+github.com/arduino/go-paths-helper v1.7.0 h1:S9l5BP2aogz1CgyqqnncXt0PLpK4yvwOW/wu/LaR3tc=
+github.com/arduino/go-paths-helper v1.7.0/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
 github.com/arduino/go-properties-orderedmap v1.6.0 h1:gp2JoWRETtqwsZ+UHu/PBuYWYH2x2+d+uipDxS4WmvM=
 github.com/arduino/go-properties-orderedmap v1.6.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk=
 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b h1:9hDi4F2st6dbLC3y4i02zFT5quS4X6iioWifGlVwfy4=

diff --git a/go.mod b/go.mod
@@ -4,7 +4,7 @@ go 1.16
 
 require (
 	github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c
-	github.com/arduino/go-paths-helper v1.6.1
+	github.com/arduino/go-paths-helper v1.7.0
 	github.com/arduino/go-properties-orderedmap v1.6.0
 	github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b
 	github.com/arduino/go-win32-utils v0.0.0-20180330194947-ed041402e83b

diff --git a/go.sum b/go.sum
@@ -48,8 +48,8 @@ github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c h1:agh2JT9
 github.com/arduino/board-discovery v0.0.0-20180823133458-1ba29327fb0c/go.mod h1:HK7SpkEax/3P+0w78iRQx1sz1vCDYYw9RXwHjQTB5i8=
 github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
 github.com/arduino/go-paths-helper v1.2.0/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
-github.com/arduino/go-paths-helper v1.6.1 h1:lha+/BuuBsx0qTZ3gy6IO1kU23lObWdQ/UItkzVWQ+0=
-github.com/arduino/go-paths-helper v1.6.1/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
+github.com/arduino/go-paths-helper v1.7.0 h1:S9l5BP2aogz1CgyqqnncXt0PLpK4yvwOW/wu/LaR3tc=
+github.com/arduino/go-paths-helper v1.7.0/go.mod h1:V82BWgAAp4IbmlybxQdk9Bpkz8M4Qyx+RAFKaG9NuvU=
 github.com/arduino/go-properties-orderedmap v1.6.0 h1:gp2JoWRETtqwsZ+UHu/PBuYWYH2x2+d+uipDxS4WmvM=
 github.com/arduino/go-properties-orderedmap v1.6.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk=
 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b h1:9hDi4F2st6dbLC3y4i02zFT5quS4X6iioWifGlVwfy4=

diff --git a/legacy/builder/builder.go b/legacy/builder/builder.go
@@ -30,10 +30,6 @@ import (
 
 var tr = i18n.Tr
 
-var MAIN_FILE_VALID_EXTENSIONS = map[string]bool{".ino": true, ".pde": true}
-var ADDITIONAL_FILE_VALID_EXTENSIONS = map[string]bool{".h": true, ".c": true, ".hpp": true, ".hh": true, ".cpp": true, ".S": true}
-var ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS = map[string]bool{".c": true, ".cpp": true, ".S": true}
-
 const DEFAULT_DEBUG_LEVEL = 5
 const DEFAULT_WARNINGS_LEVEL = "none"
 const DEFAULT_SOFTWARE = "ARDUINO"

diff --git a/legacy/builder/builder_utils/utils.go b/legacy/builder/builder_utils/utils.go
@@ -16,13 +16,15 @@
 package builder_utils
 
 import (
+	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"sync"
 
+	"github.com/arduino/arduino-cli/arduino/globals"
 	"github.com/arduino/arduino-cli/i18n"
 	"github.com/arduino/arduino-cli/legacy/builder/constants"
 	"github.com/arduino/arduino-cli/legacy/builder/types"
@@ -35,93 +37,6 @@ import (
 
 var tr = i18n.Tr
 
-func CompileFilesRecursive(ctx *types.Context, sourcePath *paths.Path, buildPath *paths.Path, buildProperties *properties.Map, includes []string) (paths.PathList, error) {
-	objectFiles, err := CompileFiles(ctx, sourcePath, false, buildPath, buildProperties, includes)
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-
-	folders, err := utils.ReadDirFiltered(sourcePath.String(), utils.FilterDirs)
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-
-	for _, folder := range folders {
-		subFolderObjectFiles, err := CompileFilesRecursive(ctx, sourcePath.Join(folder.Name()), buildPath.Join(folder.Name()), buildProperties, includes)
-		if err != nil {
-			return nil, errors.WithStack(err)
-		}
-		objectFiles.AddAll(subFolderObjectFiles)
-	}
-
-	return objectFiles, nil
-}
-
-func CompileFiles(ctx *types.Context, sourcePath *paths.Path, recurse bool, buildPath *paths.Path, buildProperties *properties.Map, includes []string) (paths.PathList, error) {
-	sSources, err := findFilesInFolder(sourcePath, ".S", recurse)
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-	cSources, err := findFilesInFolder(sourcePath, ".c", recurse)
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-	cppSources, err := findFilesInFolder(sourcePath, ".cpp", recurse)
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-
-	ctx.Progress.AddSubSteps(len(sSources) + len(cSources) + len(cppSources))
-	defer ctx.Progress.RemoveSubSteps()
-
-	sObjectFiles, err := compileFilesWithRecipe(ctx, sourcePath, sSources, buildPath, buildProperties, includes, constants.RECIPE_S_PATTERN)
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-	cObjectFiles, err := compileFilesWithRecipe(ctx, sourcePath, cSources, buildPath, buildProperties, includes, constants.RECIPE_C_PATTERN)
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-	cppObjectFiles, err := compileFilesWithRecipe(ctx, sourcePath, cppSources, buildPath, buildProperties, includes, constants.RECIPE_CPP_PATTERN)
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-
-	objectFiles := paths.NewPathList()
-	objectFiles.AddAll(sObjectFiles)
-	objectFiles.AddAll(cObjectFiles)
-	objectFiles.AddAll(cppObjectFiles)
-	return objectFiles, nil
-}
-
-func findFilesInFolder(sourcePath *paths.Path, extension string, recurse bool) (paths.PathList, error) {
-	files, err := utils.ReadDirFiltered(sourcePath.String(), utils.FilterFilesWithExtensions(extension))
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-	var sources paths.PathList
-	for _, file := range files {
-		sources = append(sources, sourcePath.Join(file.Name()))
-	}
-
-	if recurse {
-		folders, err := utils.ReadDirFiltered(sourcePath.String(), utils.FilterDirs)
-		if err != nil {
-			return nil, errors.WithStack(err)
-		}
-
-		for _, folder := range folders {
-			otherSources, err := findFilesInFolder(sourcePath.Join(folder.Name()), extension, recurse)
-			if err != nil {
-				return nil, errors.WithStack(err)
-			}
-			sources = append(sources, otherSources...)
-		}
-	}
-
-	return sources, nil
-}
-
 func findAllFilesInFolder(sourcePath string, recurse bool) ([]string, error) {
 	files, err := utils.ReadDirFiltered(sourcePath, utils.FilterFiles())
 	if err != nil {
@@ -153,17 +68,46 @@ func findAllFilesInFolder(sourcePath string, recurse bool) ([]string, error) {
 	return sources, nil
 }
 
-func compileFilesWithRecipe(ctx *types.Context, sourcePath *paths.Path, sources paths.PathList, buildPath *paths.Path, buildProperties *properties.Map, includes []string, recipe string) (paths.PathList, error) {
+func CompileFiles(ctx *types.Context, sourcePath *paths.Path, buildPath *paths.Path, buildProperties *properties.Map, includes []string) (paths.PathList, error) {
+	return compileFiles(ctx, sourcePath, false, buildPath, buildProperties, includes)
+}
+
+func CompileFilesRecursive(ctx *types.Context, sourcePath *paths.Path, buildPath *paths.Path, buildProperties *properties.Map, includes []string) (paths.PathList, error) {
+	return compileFiles(ctx, sourcePath, true, buildPath, buildProperties, includes)
+}
+
+func compileFiles(ctx *types.Context, sourcePath *paths.Path, recurse bool, buildPath *paths.Path, buildProperties *properties.Map, includes []string) (paths.PathList, error) {
+	var sources paths.PathList
+	var err error
+	if recurse {
+		sources, err = sourcePath.ReadDirRecursive()
+	} else {
+		sources, err = sourcePath.ReadDir()
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	validExtensions := []string{}
+	for ext := range globals.SourceFilesValidExtensions {
+		validExtensions = append(validExtensions, ext)
+	}
+
+	sources.FilterSuffix(validExtensions...)
+	ctx.Progress.AddSubSteps(len(sources))
+	defer ctx.Progress.RemoveSubSteps()
+
 	objectFiles := paths.NewPathList()
+	var objectFilesMux sync.Mutex
 	if len(sources) == 0 {
 		return objectFiles, nil
 	}
-	var objectFilesMux sync.Mutex
 	var errorsList []error
 	var errorsMux sync.Mutex
 
 	queue := make(chan *paths.Path)
 	job := func(source *paths.Path) {
+		recipe := fmt.Sprintf("recipe%s.o.pattern", source.Ext())
 		objectFile, err := compileFileWithRecipe(ctx, sourcePath, source, buildPath, buildProperties, includes, recipe)
 		if err != nil {
 			errorsMux.Lock()
@@ -233,7 +177,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath *paths.Path, source *p
 		return nil, errors.WithStack(err)
 	}
 
-	objIsUpToDate, err := ObjFileIsUpToDate(ctx, source, objectFile, depsFile)
+	objIsUpToDate, err := ObjFileIsUpToDate(source, objectFile, depsFile)
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
@@ -260,7 +204,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath *paths.Path, source *p
 	return objectFile, nil
 }
 
-func ObjFileIsUpToDate(ctx *types.Context, sourceFile, objectFile, dependencyFile *paths.Path) (bool, error) {
+func ObjFileIsUpToDate(sourceFile, objectFile, dependencyFile *paths.Path) (bool, error) {
 	logrus.Debugf("Checking previous results for %v (result = %v, dep = %v)", sourceFile, objectFile, dependencyFile)
 	if objectFile == nil || dependencyFile == nil {
 		logrus.Debugf("Not found: nil")
@@ -371,10 +315,7 @@ func unescapeDep(s string) string {
 }
 
 func removeEndingBackSlash(s string) string {
-	if strings.HasSuffix(s, "\\") {
-		s = s[:len(s)-1]
-	}
-	return s
+	return strings.TrimSuffix(s, "\\")
 }
 
 func nonEmptyString(s string) bool {

diff --git a/legacy/builder/constants/constants.go b/legacy/builder/constants/constants.go
@@ -66,10 +66,7 @@ const PLATFORM_URL = "url"
 const PLATFORM_VERSION = "version"
 const RECIPE_AR_PATTERN = "recipe.ar.pattern"
 const RECIPE_C_COMBINE_PATTERN = "recipe.c.combine.pattern"
-const RECIPE_C_PATTERN = "recipe.c.o.pattern"
-const RECIPE_CPP_PATTERN = "recipe.cpp.o.pattern"
 const RECIPE_PREPROC_MACROS = "recipe.preproc.macros"
-const RECIPE_S_PATTERN = "recipe.S.o.pattern"
 const REWRITING_DISABLED = "disabled"
 const REWRITING = "rewriting"
 const SPACE = " "

diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go
@@ -98,6 +98,7 @@ import (
 	"os/exec"
 	"time"
 
+	"github.com/arduino/arduino-cli/arduino/globals"
 	"github.com/arduino/arduino-cli/arduino/libraries"
 	"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
 	"github.com/arduino/arduino-cli/legacy/builder/types"
@@ -312,7 +313,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
 	// TODO: This reads the dependency file, but the actual building
 	// does it again. Should the result be somehow cached? Perhaps
 	// remove the object file if it is found to be stale?
-	unchanged, err := builder_utils.ObjFileIsUpToDate(ctx, sourcePath, objPath, depPath)
+	unchanged, err := builder_utils.ObjFileIsUpToDate(sourcePath, objPath, depPath)
 	if err != nil {
 		return errors.WithStack(err)
 	}
@@ -406,16 +407,17 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
 }
 
 func queueSourceFilesFromFolder(ctx *types.Context, queue *types.UniqueSourceFileQueue, origin interface{}, folder *paths.Path, recurse bool) error {
-	extensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS_NO_HEADERS[ext] }
-
-	filePaths := []string{}
-	err := utils.FindFilesInFolder(&filePaths, folder.String(), extensions, recurse)
+	sourceFileExtensions := []string{}
+	for k := range globals.SourceFilesValidExtensions {
+		sourceFileExtensions = append(sourceFileExtensions, k)
+	}
+	filePaths, err := utils.FindFilesInFolder(folder, recurse, sourceFileExtensions)
 	if err != nil {
 		return errors.WithStack(err)
 	}
 
 	for _, filePath := range filePaths {
-		sourceFile, err := types.MakeSourceFile(ctx, origin, paths.New(filePath))
+		sourceFile, err := types.MakeSourceFile(ctx, origin, filePath)
 		if err != nil {
 			return errors.WithStack(err)
 		}