Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: excessive CPU/Disk usage when multiple build-paths points in the sketch directory #2342

Merged
merged 4 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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