Skip to content

Commit

Permalink
bugfix: excessive CPU/Disk usage when multiple build-paths points in …
Browse files Browse the repository at this point in the history
…the sketch directory (#2342)

* Use paths.ReadDirRecursive filtered where possible

* Do not add sketch files from known build paths

* Improved builder globals

* Added integration test
  • Loading branch information
cmaglie committed Sep 26, 2023
1 parent 422aa87 commit c004b42
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 39 deletions.
44 changes: 21 additions & 23 deletions arduino/globals/globals.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,33 +16,31 @@
package globals

var (
empty struct{}

// MainFileValidExtension is the extension that must be used for files in new sketches
MainFileValidExtension string = ".ino"

// MainFileValidExtensions lists valid extensions for a sketch file
MainFileValidExtensions = map[string]struct{}{
MainFileValidExtension: empty,
MainFileValidExtensions = map[string]bool{
MainFileValidExtension: true,
// .pde extension is deprecated and must not be used for new sketches
".pde": empty,
".pde": true,
}

// AdditionalFileValidExtensions lists any file extension the builder considers as valid
AdditionalFileValidExtensions = map[string]struct{}{
".h": empty,
".c": empty,
".hpp": empty,
".hh": empty,
".cpp": empty,
".cxx": empty,
".cc": empty,
".S": empty,
".adoc": empty,
".md": empty,
".json": empty,
".tpp": empty,
".ipp": empty,
AdditionalFileValidExtensions = map[string]bool{
".h": true,
".c": true,
".hpp": true,
".hh": true,
".cpp": true,
".cxx": true,
".cc": true,
".S": true,
".adoc": true,
".md": true,
".json": true,
".tpp": true,
".ipp": true,
}

// SourceFilesValidExtensions lists valid extensions for source files (no headers).
Expand All @@ -57,10 +55,10 @@ var (
}

// HeaderFilesValidExtensions lists valid extensions for header files
HeaderFilesValidExtensions = map[string]struct{}{
".h": empty,
".hpp": empty,
".hh": empty,
HeaderFilesValidExtensions = map[string]bool{
".h": true,
".hpp": true,
".hh": true,
}

// DefaultIndexURL is the default index url
Expand Down
34 changes: 19 additions & 15 deletions arduino/sketch/sketch.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func New(path *paths.Path) (*Sketch, error) {
} else if !exist {
return nil, fmt.Errorf("%s: %s", tr("no such file or directory"), path)
}
if _, validIno := globals.MainFileValidExtensions[path.Ext()]; validIno && !path.IsDir() {
if globals.MainFileValidExtensions[path.Ext()] && !path.IsDir() {
path = path.Parent()
}

Expand Down Expand Up @@ -121,7 +121,7 @@ func New(path *paths.Path) (*Sketch, error) {
f.Close()

ext := p.Ext()
if _, found := globals.MainFileValidExtensions[ext]; found {
if globals.MainFileValidExtensions[ext] {
if p.EqualsTo(mainFile) {
// The main file must not be included in the lists of other files
continue
Expand All @@ -132,7 +132,7 @@ func New(path *paths.Path) (*Sketch, error) {
sketch.OtherSketchFiles.Add(p)
sketch.RootFolderFiles.Add(p)
}
} else if _, found := globals.AdditionalFileValidExtensions[ext]; found {
} else if globals.AdditionalFileValidExtensions[ext] {
// If the user exported the compiles binaries to the Sketch "build" folder
// they would be picked up but we don't want them, so we skip them like so
if p.IsInsideDir(sketch.FullPath.Join("build")) {
Expand All @@ -158,22 +158,26 @@ func New(path *paths.Path) (*Sketch, error) {
// supportedFiles reads all files recursively contained in Sketch and
// filter out unneded or unsupported ones and returns them
func (s *Sketch) supportedFiles() (*paths.PathList, error) {
files, err := s.FullPath.ReadDirRecursive()
if err != nil {
return nil, err
filterValidExtensions := func(p *paths.Path) bool {
return globals.MainFileValidExtensions[p.Ext()] || globals.AdditionalFileValidExtensions[p.Ext()]
}
files.FilterOutDirs()
files.FilterOutHiddenFiles()
validExtensions := []string{}
for ext := range globals.MainFileValidExtensions {
validExtensions = append(validExtensions, ext)

filterOutBuildPaths := func(p *paths.Path) bool {
return !p.Join("build.options.json").Exist()
}
for ext := range globals.AdditionalFileValidExtensions {
validExtensions = append(validExtensions, ext)

files, err := s.FullPath.ReadDirRecursiveFiltered(
filterOutBuildPaths,
paths.AndFilter(
paths.FilterOutPrefixes("."),
filterValidExtensions,
paths.FilterOutDirectories(),
),
)
if err != nil {
return nil, err
}
files.FilterSuffix(validExtensions...)
return &files, nil

}

// GetProfile returns the requested profile or nil if the profile
Expand Down
2 changes: 1 addition & 1 deletion commands/upload/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {

// Sometimes we may have particular files like:
// Blink.ino.with_bootloader.bin
if _, ok := globals.MainFileValidExtensions[filepath.Ext(name)]; !ok {
if !globals.MainFileValidExtensions[filepath.Ext(name)] {
// just ignore those files
continue
}
Expand Down
23 changes: 23 additions & 0 deletions internal/integrationtest/compile_1/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"crypto/md5"
"encoding/hex"
"encoding/json"
"fmt"
"os"
"sort"
"strings"
Expand Down Expand Up @@ -1220,6 +1221,28 @@ func buildWithCustomBuildPath(t *testing.T, env *integrationtest.Environment, cl
// Run build twice, to verify the build still works when the build directory is present at the start
_, _, err = cli.Run("compile", "-b", "arduino:avr:uno", "--build-path", buildPath.String(), sketchPath.String())
require.NoError(t, err)

// Run again a couple of times with a different build path, to verify that old build
// path is not copied back in the sketch build recursively.
// https://github.com/arduino/arduino-cli/issues/2266
secondBuildPath := sketchPath.Join("build2")
_, _, err = cli.Run("compile", "-b", "arduino:avr:uno", "--build-path", secondBuildPath.String(), sketchPath.String())
require.NoError(t, err)
_, _, err = cli.Run("compile", "-b", "arduino:avr:uno", "--build-path", buildPath.String(), sketchPath.String())
require.NoError(t, err)
_, _, err = cli.Run("compile", "-b", "arduino:avr:uno", "--build-path", secondBuildPath.String(), sketchPath.String())
require.NoError(t, err)
_, _, err = cli.Run("compile", "-b", "arduino:avr:uno", "--build-path", buildPath.String(), sketchPath.String())
require.NoError(t, err)

// Print build path content for debugging purposes
bp, _ := buildPath.ReadDirRecursive()
fmt.Println("Build path content:")
for _, file := range bp {
fmt.Println("> ", file.String())
}

require.False(t, buildPath.Join("sketch", "build2", "sketch").Exist())
})

t.Run("SameAsSektch", func(t *testing.T) {
Expand Down

0 comments on commit c004b42

Please sign in to comment.