diff --git a/commands/service_compile.go b/commands/service_compile.go index 1586ef6f953..9da7ab82c71 100644 --- a/commands/service_compile.go +++ b/commands/service_compile.go @@ -169,7 +169,7 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu if buildPathArg := req.GetBuildPath(); buildPathArg != "" { buildPath = paths.New(req.GetBuildPath()).Canonical() if in, _ := buildPath.IsInsideDir(sk.FullPath); in && buildPath.IsDir() { - if sk.AdditionalFiles, err = removeBuildFromSketchFiles(sk.AdditionalFiles, buildPath); err != nil { + if sk.AdditionalFiles, err = removeBuildPathFromSketchFiles(sk.AdditionalFiles, buildPath); err != nil { return err } } @@ -220,10 +220,6 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu return err } - actualPlatform := buildPlatform - otherLibrariesDirs := paths.NewPathList(req.GetLibraries()...) - otherLibrariesDirs.Add(s.settings.LibrariesDir()) - var libsManager *librariesmanager.LibrariesManager if profile != nil { libsManager = lm @@ -253,6 +249,11 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu if req.GetVerbose() { verbosity = logger.VerbosityVerbose } + + librariesDirs := paths.NewPathList(req.GetLibraries()...) // Array of collection of libraries directories + librariesDirs.Add(s.settings.LibrariesDir()) + libraryDirs := paths.NewPathList(req.GetLibrary()...) // Array of single-library directories + sketchBuilder, err := builder.NewBuilder( ctx, sk, @@ -264,16 +265,16 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu int(req.GetJobs()), req.GetBuildProperties(), s.settings.HardwareDirectories(), - otherLibrariesDirs, + librariesDirs, s.settings.IDEBuiltinLibrariesDir(), fqbn, req.GetClean(), req.GetSourceOverride(), req.GetCreateCompilationDatabaseOnly(), - targetPlatform, actualPlatform, + targetPlatform, buildPlatform, req.GetSkipLibrariesDiscovery(), libsManager, - paths.NewPathList(req.GetLibrary()...), + libraryDirs, outStream, errStream, verbosity, req.GetWarnings(), progressCB, pme.GetEnvVarsForSpawnedProcess(), @@ -462,15 +463,15 @@ func maybePurgeBuildCache(compilationsBeforePurge uint, cacheTTL time.Duration) buildcache.New(paths.TempDir().Join("arduino", "sketches")).Purge(cacheTTL) } -// removeBuildFromSketchFiles removes the files contained in the build directory from +// removeBuildPathFromSketchFiles removes the files contained in the build directory from // the list of the sketch files -func removeBuildFromSketchFiles(files paths.PathList, build *paths.Path) (paths.PathList, error) { +func removeBuildPathFromSketchFiles(files paths.PathList, build *paths.Path) (paths.PathList, error) { var res paths.PathList ignored := false for _, file := range files { if isInside, _ := file.IsInsideDir(build); !isInside { - res = append(res, file) - } else if !ignored { + res.Add(file) + } else { ignored = true } } diff --git a/commands/service_monitor.go b/commands/service_monitor.go index 012d4ddf8bc..8297b1cf763 100644 --- a/commands/service_monitor.go +++ b/commands/service_monitor.go @@ -27,7 +27,6 @@ import ( "github.com/arduino/arduino-cli/internal/arduino/cores" "github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager" pluggableMonitor "github.com/arduino/arduino-cli/internal/arduino/monitor" - "github.com/arduino/arduino-cli/internal/i18n" "github.com/arduino/arduino-cli/pkg/fqbn" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/arduino/go-properties-orderedmap" @@ -265,10 +264,7 @@ func findMonitorAndSettingsForProtocolAndBoard(pme *packagemanager.Explorer, pro } else if recipe, ok := boardPlatform.MonitorsDevRecipes[protocol]; ok { // If we have a recipe we must resolve it cmdLine := boardProperties.ExpandPropsInString(recipe) - cmdArgs, err := properties.SplitQuotedString(cmdLine, `"'`, false) - if err != nil { - return nil, nil, &cmderrors.InvalidArgumentError{Message: i18n.Tr("Invalid recipe in platform.txt"), Cause: err} - } + cmdArgs, _ := properties.SplitQuotedString(cmdLine, `"'`, false) id := fmt.Sprintf("%s-%s", boardPlatform, protocol) return pluggableMonitor.New(id, cmdArgs...), boardSettings, nil } diff --git a/commands/service_upload.go b/commands/service_upload.go index 10f759998d7..f18a5f247ee 100644 --- a/commands/service_upload.go +++ b/commands/service_upload.go @@ -720,10 +720,7 @@ func runTool(ctx context.Context, recipeID string, props *properties.Map, outStr return errors.New(i18n.Tr("no upload port provided")) } cmdLine := props.ExpandPropsInString(recipe) - cmdArgs, err := properties.SplitQuotedString(cmdLine, `"'`, false) - if err != nil { - return errors.New(i18n.Tr("invalid recipe '%[1]s': %[2]s", recipe, err)) - } + cmdArgs, _ := properties.SplitQuotedString(cmdLine, `"'`, false) // Run Tool logrus.WithField("phase", "upload").Tracef("Executing upload tool: %s", cmdLine) diff --git a/internal/arduino/builder/builder.go b/internal/arduino/builder/builder.go index 0c711273a5d..c96415217c6 100644 --- a/internal/arduino/builder/builder.go +++ b/internal/arduino/builder/builder.go @@ -59,9 +59,6 @@ type Builder struct { // Parallel processes jobs int - // Custom build properties defined by user (line by line as "key=value" pairs) - customBuildProperties []string - // core related coreBuildCachePath *paths.Path extraCoreBuildCachePaths paths.PathList @@ -89,7 +86,7 @@ type Builder struct { lineOffset int targetPlatform *cores.PlatformRelease - actualPlatform *cores.PlatformRelease + buildPlatform *cores.PlatformRelease buildArtifacts *buildArtifacts @@ -125,19 +122,20 @@ func NewBuilder( coreBuildCachePath *paths.Path, extraCoreBuildCachePaths paths.PathList, jobs int, - requestBuildProperties []string, - hardwareDirs, otherLibrariesDirs paths.PathList, + customBuildProperties []string, + hardwareDirs paths.PathList, + librariesDirs paths.PathList, builtInLibrariesDirs *paths.Path, fqbn *fqbn.FQBN, clean bool, sourceOverrides map[string]string, onlyUpdateCompilationDatabase bool, - targetPlatform, actualPlatform *cores.PlatformRelease, + targetPlatform, buildPlatform *cores.PlatformRelease, useCachedLibrariesResolution bool, librariesManager *librariesmanager.LibrariesManager, - libraryDirs paths.PathList, + customLibraryDirs paths.PathList, stdout, stderr io.Writer, verbosity logger.Verbosity, warningsLevel string, - progresCB rpc.TaskProgressCB, + progressCB rpc.TaskProgressCB, toolEnv []string, ) (*Builder, error) { buildProperties := properties.NewMap() @@ -146,14 +144,12 @@ func NewBuilder( } if sk != nil { buildProperties.SetPath("sketch_path", sk.FullPath) + buildProperties.Set("build.project_name", sk.MainFile.Base()) + buildProperties.SetPath("build.source.path", sk.FullPath) } if buildPath != nil { buildProperties.SetPath("build.path", buildPath) } - if sk != nil { - buildProperties.Set("build.project_name", sk.MainFile.Base()) - buildProperties.SetPath("build.source.path", sk.FullPath) - } if optimizeForDebug { if debugFlags, ok := buildProperties.GetOk("compiler.optimization_flags.debug"); ok { buildProperties.Set("compiler.optimization_flags", debugFlags) @@ -165,12 +161,11 @@ func NewBuilder( } // Add user provided custom build properties - customBuildProperties, err := properties.LoadFromSlice(requestBuildProperties) - if err != nil { + if p, err := properties.LoadFromSlice(customBuildProperties); err == nil { + buildProperties.Merge(p) + } else { return nil, fmt.Errorf("invalid build properties: %w", err) } - buildProperties.Merge(customBuildProperties) - customBuildPropertiesArgs := append(requestBuildProperties, "build.warn_data_percentage=75") sketchBuildPath, err := buildPath.Join("sketch").Abs() if err != nil { @@ -190,16 +185,20 @@ func NewBuilder( } log := logger.New(stdout, stderr, verbosity, warningsLevel) - libsManager, libsResolver, verboseOut, err := detector.LibrariesLoader( - useCachedLibrariesResolution, librariesManager, - builtInLibrariesDirs, libraryDirs, otherLibrariesDirs, - actualPlatform, targetPlatform, + libsManager, libsResolver, libsLoadingWarnings, err := detector.LibrariesLoader( + useCachedLibrariesResolution, + librariesManager, + builtInLibrariesDirs, + customLibraryDirs, + librariesDirs, + buildPlatform, + targetPlatform, ) if err != nil { return nil, err } if log.VerbosityLevel() == logger.VerbosityVerbose { - log.Warn(string(verboseOut)) + log.Warn(string(libsLoadingWarnings)) } diagnosticStore := diagnostics.NewStore() @@ -212,7 +211,6 @@ func NewBuilder( coreBuildPath: coreBuildPath, librariesBuildPath: librariesBuildPath, jobs: jobs, - customBuildProperties: customBuildPropertiesArgs, coreBuildCachePath: coreBuildCachePath, extraCoreBuildCachePaths: extraCoreBuildCachePaths, logger: log, @@ -220,17 +218,19 @@ func NewBuilder( sourceOverrides: sourceOverrides, onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase, compilationDatabase: compilation.NewDatabase(buildPath.Join("compile_commands.json")), - Progress: progress.New(progresCB), + Progress: progress.New(progressCB), executableSectionsSize: []ExecutableSectionSize{}, buildArtifacts: &buildArtifacts{}, targetPlatform: targetPlatform, - actualPlatform: actualPlatform, + buildPlatform: buildPlatform, toolEnv: toolEnv, buildOptions: newBuildOptions( - hardwareDirs, otherLibrariesDirs, - builtInLibrariesDirs, buildPath, + hardwareDirs, + librariesDirs, + builtInLibrariesDirs, + buildPath, sk, - customBuildPropertiesArgs, + customBuildProperties, fqbn, clean, buildProperties.Get("compiler.optimization_flags"), @@ -322,10 +322,19 @@ func (b *Builder) preprocess() error { b.librariesBuildPath, b.buildProperties, b.targetPlatform.Platform.Architecture, + b.jobs, ) if err != nil { return err } + if b.libsDetector.IncludeFoldersChanged() && b.librariesBuildPath.Exist() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { + b.logger.Info(i18n.Tr("The list of included libraries has been changed... rebuilding all libraries.")) + } + if err := b.librariesBuildPath.RemoveAll(); err != nil { + return err + } + } b.Progress.CompleteStep() b.warnAboutArchIncompatibleLibraries(b.libsDetector.ImportedLibraries()) @@ -492,10 +501,7 @@ func (b *Builder) prepareCommandForRecipe(buildProperties *properties.Map, recip commandLine = properties.DeleteUnexpandedPropsFromString(commandLine) } - parts, err := properties.SplitQuotedString(commandLine, `"'`, false) - if err != nil { - return nil, err - } + args, _ := properties.SplitQuotedString(commandLine, `"'`, false) // if the overall commandline is too long for the platform // try reducing the length by making the filenames relative @@ -503,18 +509,18 @@ func (b *Builder) prepareCommandForRecipe(buildProperties *properties.Map, recip var relativePath string if len(commandLine) > 30000 { relativePath = buildProperties.Get("build.path") - for i, arg := range parts { + for i, arg := range args { if _, err := os.Stat(arg); os.IsNotExist(err) { continue } rel, err := filepath.Rel(relativePath, arg) if err == nil && !strings.Contains(rel, "..") && len(rel) < len(arg) { - parts[i] = rel + args[i] = rel } } } - command, err := paths.NewProcess(b.toolEnv, parts...) + command, err := paths.NewProcess(b.toolEnv, args...) if err != nil { return nil, err } diff --git a/internal/arduino/builder/compilation.go b/internal/arduino/builder/compilation.go index c739a2e37de..fdd8573c02f 100644 --- a/internal/arduino/builder/compilation.go +++ b/internal/arduino/builder/compilation.go @@ -111,7 +111,6 @@ func (b *Builder) compileFiles( return objectFiles, nil } -// CompileFilesRecursive fixdoc func (b *Builder) compileFileWithRecipe( sourcePath *paths.Path, source *paths.Path, @@ -119,28 +118,21 @@ func (b *Builder) compileFileWithRecipe( includes []string, recipe string, ) (*paths.Path, error) { - properties := b.buildProperties.Clone() - properties.Set("compiler.warning_flags", properties.Get("compiler.warning_flags."+b.logger.WarningsLevel())) - properties.Set("includes", strings.Join(includes, " ")) - properties.SetPath("source_file", source) relativeSource, err := sourcePath.RelTo(source) if err != nil { return nil, err } depsFile := buildPath.Join(relativeSource.String() + ".d") objectFile := buildPath.Join(relativeSource.String() + ".o") - - properties.SetPath("object_file", objectFile) - err = objectFile.Parent().MkdirAll() - if err != nil { - return nil, err - } - - objIsUpToDate, err := utils.ObjFileIsUpToDate(source, objectFile, depsFile) - if err != nil { + if err := objectFile.Parent().MkdirAll(); err != nil { return nil, err } + properties := b.buildProperties.Clone() + properties.Set("compiler.warning_flags", properties.Get("compiler.warning_flags."+b.logger.WarningsLevel())) + properties.Set("includes", strings.Join(includes, " ")) + properties.SetPath("source_file", source) + properties.SetPath("object_file", objectFile) command, err := b.prepareCommandForRecipe(properties, recipe, false) if err != nil { return nil, err @@ -148,41 +140,50 @@ func (b *Builder) compileFileWithRecipe( if b.compilationDatabase != nil { b.compilationDatabase.Add(source, command) } - if !objIsUpToDate && !b.onlyUpdateCompilationDatabase { - commandStdout, commandStderr := &bytes.Buffer{}, &bytes.Buffer{} - command.RedirectStdoutTo(commandStdout) - command.RedirectStderrTo(commandStderr) + objIsUpToDate, err := utils.ObjFileIsUpToDate(source, objectFile, depsFile) + if err != nil { + return nil, err + } + if objIsUpToDate { if b.logger.VerbosityLevel() == logger.VerbosityVerbose { - b.logger.Info(utils.PrintableCommand(command.GetArgs())) - } - // Since this compile could be multithreaded, we first capture the command output - if err := command.Start(); err != nil { - return nil, err + b.logger.Info(i18n.Tr("Using previously compiled file: %[1]s", objectFile)) } - err := command.Wait() - // and transfer all at once at the end... + return objectFile, nil + } + if b.onlyUpdateCompilationDatabase { if b.logger.VerbosityLevel() == logger.VerbosityVerbose { - b.logger.WriteStdout(commandStdout.Bytes()) + b.logger.Info(i18n.Tr("Skipping compile of: %[1]s", objectFile)) } - b.logger.WriteStderr(commandStderr.Bytes()) + return objectFile, nil + } - // Parse the output of the compiler to gather errors and warnings... - if b.diagnosticStore != nil { - b.diagnosticStore.Parse(command.GetArgs(), commandStdout.Bytes()) - b.diagnosticStore.Parse(command.GetArgs(), commandStderr.Bytes()) - } + commandStdout, commandStderr := &bytes.Buffer{}, &bytes.Buffer{} + command.RedirectStdoutTo(commandStdout) + command.RedirectStderrTo(commandStderr) + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { + b.logger.Info(utils.PrintableCommand(command.GetArgs())) + } + // Since this compile could be multithreaded, we first capture the command output + if err := command.Start(); err != nil { + return nil, err + } + err = command.Wait() + // and transfer all at once at the end... + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { + b.logger.WriteStdout(commandStdout.Bytes()) + } + b.logger.WriteStderr(commandStderr.Bytes()) - // ...and then return the error - if err != nil { - return nil, err - } - } else if b.logger.VerbosityLevel() == logger.VerbosityVerbose { - if objIsUpToDate { - b.logger.Info(i18n.Tr("Using previously compiled file: %[1]s", objectFile)) - } else { - b.logger.Info(i18n.Tr("Skipping compile of: %[1]s", objectFile)) - } + // Parse the output of the compiler to gather errors and warnings... + if b.diagnosticStore != nil { + b.diagnosticStore.Parse(command.GetArgs(), commandStdout.Bytes()) + b.diagnosticStore.Parse(command.GetArgs(), commandStderr.Bytes()) + } + + // ...and then return the error + if err != nil { + return nil, err } return objectFile, nil diff --git a/internal/arduino/builder/core.go b/internal/arduino/builder/core.go index c6d87ec7d66..593b7b575d5 100644 --- a/internal/arduino/builder/core.go +++ b/internal/arduino/builder/core.go @@ -164,7 +164,7 @@ func (b *Builder) compileCore() (*paths.Path, paths.PathList, error) { b.logger.Info(i18n.Tr("Archiving built core (caching) in: %[1]s", targetArchivedCore)) } else if os.IsNotExist(err) { b.logger.Info(i18n.Tr("Unable to cache built core, please tell %[1]s maintainers to follow %[2]s", - b.actualPlatform, + b.buildPlatform, "https://arduino.github.io/arduino-cli/latest/platform-specification/#recipes-to-build-the-corea-archive-file")) } else { b.logger.Info(i18n.Tr("Error archiving built core (caching) in %[1]s: %[2]s", targetArchivedCore, err)) diff --git a/internal/arduino/builder/internal/detector/cache.go b/internal/arduino/builder/internal/detector/cache.go index 247a1e8de30..62b3d355678 100644 --- a/internal/arduino/builder/internal/detector/cache.go +++ b/internal/arduino/builder/internal/detector/cache.go @@ -19,6 +19,7 @@ import ( "encoding/json" "fmt" + "github.com/arduino/arduino-cli/internal/arduino/builder/internal/runner" "github.com/arduino/go-paths-helper" ) @@ -28,17 +29,18 @@ type detectorCache struct { } type detectorCacheEntry struct { - AddedIncludePath *paths.Path `json:"added_include_path,omitempty"` - CompilingSourcePath *paths.Path `json:"compiling_source_path,omitempty"` - MissingIncludeH *string `json:"missing_include_h,omitempty"` + AddedIncludePath *paths.Path `json:"added_include_path,omitempty"` + Compile *sourceFile `json:"compile,omitempty"` + CompileTask *runner.Task `json:"compile_task,omitempty"` + MissingIncludeH *string `json:"missing_include_h,omitempty"` } func (e *detectorCacheEntry) String() string { if e.AddedIncludePath != nil { return "Added include path: " + e.AddedIncludePath.String() } - if e.CompilingSourcePath != nil { - return "Compiling source path: " + e.CompilingSourcePath.String() + if e.Compile != nil && e.CompileTask != nil { + return "Compiling: " + e.Compile.String() + " / " + e.CompileTask.String() } if e.MissingIncludeH != nil { if *e.MissingIncludeH == "" { @@ -109,6 +111,14 @@ func (c *detectorCache) Peek() *detectorCacheEntry { return nil } +// EntriesAhead returns the entries that are ahead of the current cache position. +func (c *detectorCache) EntriesAhead() []*detectorCacheEntry { + if c.curr < len(c.entries) { + return c.entries[c.curr:] + } + return nil +} + // Save writes the current cache to the given file. func (c *detectorCache) Save(cacheFile *paths.Path) error { // Cut off the cache if it is not fully consumed diff --git a/internal/arduino/builder/internal/detector/detector.go b/internal/arduino/builder/internal/detector/detector.go index b6fd97a2a80..0a724cfe948 100644 --- a/internal/arduino/builder/internal/detector/detector.go +++ b/internal/arduino/builder/internal/detector/detector.go @@ -23,6 +23,7 @@ import ( "fmt" "os/exec" "regexp" + "slices" "strings" "time" @@ -59,6 +60,8 @@ type SketchLibrariesDetector struct { includeFolders paths.PathList logger *logger.BuilderLogger diagnosticStore *diagnostics.Store + preRunner *runner.Runner + detectedChangeInLibraries bool } // NewSketchLibrariesDetector todo @@ -191,6 +194,12 @@ func (l *SketchLibrariesDetector) IncludeFolders() paths.PathList { return l.includeFolders } +// IncludeFoldersChanged returns true if the include folders list changed +// from the previous compile. +func (l *SketchLibrariesDetector) IncludeFoldersChanged() bool { + return l.detectedChangeInLibraries +} + // addIncludeFolder add the given folder to the include path. func (l *SketchLibrariesDetector) addIncludeFolder(folder *paths.Path) { l.includeFolders = append(l.includeFolders, folder) @@ -208,8 +217,9 @@ func (l *SketchLibrariesDetector) FindIncludes( librariesBuildPath *paths.Path, buildProperties *properties.Map, platformArch string, + jobs int, ) error { - err := l.findIncludes(ctx, buildPath, buildCorePath, buildVariantPath, sketchBuildPath, sketch, librariesBuildPath, buildProperties, platformArch) + err := l.findIncludes(ctx, buildPath, buildCorePath, buildVariantPath, sketchBuildPath, sketch, librariesBuildPath, buildProperties, platformArch, jobs) if err != nil && l.onlyUpdateCompilationDatabase { l.logger.Info( fmt.Sprintf( @@ -233,18 +243,23 @@ func (l *SketchLibrariesDetector) findIncludes( librariesBuildPath *paths.Path, buildProperties *properties.Map, platformArch string, + jobs int, ) error { - librariesResolutionCache := buildPath.Join("libraries.cache") - if l.useCachedLibrariesResolution && librariesResolutionCache.Exist() { - d, err := librariesResolutionCache.ReadFile() + librariesResolutionCachePath := buildPath.Join("libraries.cache") + var cachedIncludeFolders paths.PathList + if librariesResolutionCachePath.Exist() { + d, err := librariesResolutionCachePath.ReadFile() if err != nil { return err } - if err := json.Unmarshal(d, &l.includeFolders); err != nil { + if err := json.Unmarshal(d, &cachedIncludeFolders); err != nil { return err } + } + if l.useCachedLibrariesResolution && librariesResolutionCachePath.Exist() { + l.includeFolders = cachedIncludeFolders if l.logger.VerbosityLevel() == logger.VerbosityVerbose { - l.logger.Info("Using cached library discovery: " + librariesResolutionCache.String()) + l.logger.Info("Using cached library discovery: " + librariesResolutionCachePath.String()) } return nil } @@ -254,6 +269,18 @@ func (l *SketchLibrariesDetector) findIncludes( l.logger.Warn(i18n.Tr("Failed to load library discovery cache: %[1]s", err)) } + // Pre-run cache entries + l.preRunner = runner.New(ctx, jobs) + for _, entry := range l.cache.EntriesAhead() { + if entry.Compile != nil && entry.CompileTask != nil { + upToDate, _ := entry.Compile.ObjFileIsUpToDate() + if !upToDate { + l.preRunner.Enqueue(entry.CompileTask) + } + } + } + defer l.preRunner.Cancel() + l.addIncludeFolder(buildCorePath) if buildVariantPath != nil { l.addIncludeFolder(buildVariantPath) @@ -291,6 +318,15 @@ func (l *SketchLibrariesDetector) findIncludes( cachePath.Remove() return err } + + // Create a new pre-runner if the previous one was cancelled + if l.preRunner == nil { + l.preRunner = runner.New(ctx, jobs) + // Push in the remainder of the queue + for _, sourceFile := range *sourceFileQueue { + l.preRunner.Enqueue(l.gccPreprocessTask(sourceFile, buildProperties)) + } + } } // Finalize the cache @@ -305,13 +341,28 @@ func (l *SketchLibrariesDetector) findIncludes( if d, err := json.Marshal(l.includeFolders); err != nil { return err - } else if err := librariesResolutionCache.WriteFile(d); err != nil { + } else if err := librariesResolutionCachePath.WriteFile(d); err != nil { return err } - + l.detectedChangeInLibraries = !slices.Equal( + cachedIncludeFolders.AsStrings(), + l.includeFolders.AsStrings()) return nil } +func (l *SketchLibrariesDetector) gccPreprocessTask(sourceFile *sourceFile, buildProperties *properties.Map) *runner.Task { + // Libraries may require the "utility" directory to be added to the include + // search path, but only for the source code of the library, so we temporary + // copy the current search path list and add the library' utility directory + // if needed. + includeFolders := l.includeFolders + if extraInclude := sourceFile.ExtraIncludePath; extraInclude != nil { + includeFolders = append(includeFolders, extraInclude) + } + + return preprocessor.GCC(sourceFile.SourcePath, paths.NullPath(), includeFolders, buildProperties) +} + func (l *SketchLibrariesDetector) findMissingIncludesInCompilationUnit( ctx context.Context, sourceFileQueue *uniqueSourceFileQueue, @@ -320,10 +371,7 @@ func (l *SketchLibrariesDetector) findMissingIncludesInCompilationUnit( platformArch string, ) error { sourceFile := sourceFileQueue.Pop() - sourcePath := sourceFile.SourcePath() - targetFilePath := paths.NullPath() - depPath := sourceFile.DepfilePath() - objPath := sourceFile.ObjectPath() + sourcePath := sourceFile.SourcePath // TODO: This should perhaps also compare against the // include.cache file timestamp. Now, it only checks if the file @@ -337,26 +385,18 @@ func (l *SketchLibrariesDetector) findMissingIncludesInCompilationUnit( // 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 := utils.ObjFileIsUpToDate(sourcePath, objPath, depPath) + unchanged, err := sourceFile.ObjFileIsUpToDate() if err != nil { return err } first := true for { - l.cache.Expect(&detectorCacheEntry{CompilingSourcePath: sourcePath}) - - // Libraries may require the "utility" directory to be added to the include - // search path, but only for the source code of the library, so we temporary - // copy the current search path list and add the library' utility directory - // if needed. - includeFolders := l.includeFolders - if extraInclude := sourceFile.ExtraIncludePath(); extraInclude != nil { - includeFolders = append(includeFolders, extraInclude) - } + preprocTask := l.gccPreprocessTask(sourceFile, buildProperties) + l.cache.Expect(&detectorCacheEntry{Compile: sourceFile, CompileTask: preprocTask}) var preprocErr error - var preprocFirstResult *runner.Result + var preprocResult *runner.Result var missingIncludeH string if entry := l.cache.Peek(); unchanged && entry != nil && entry.MissingIncludeH != nil { @@ -366,20 +406,40 @@ func (l *SketchLibrariesDetector) findMissingIncludesInCompilationUnit( } first = false } else { - preprocFirstResult, preprocErr = preprocessor.GCC(ctx, sourcePath, targetFilePath, includeFolders, buildProperties) + if l.preRunner != nil { + if r := l.preRunner.Results(preprocTask); r != nil { + preprocResult = r + preprocErr = preprocResult.Error + } + } + if preprocResult == nil { + // The pre-runner missed this task, maybe the cache is outdated + // or maybe the source code changed. + + // Stop the pre-runner + if l.preRunner != nil { + preRunner := l.preRunner + l.preRunner = nil + go preRunner.Cancel() + } + + // Run the actual preprocessor + preprocResult = preprocTask.Run(ctx) + preprocErr = preprocResult.Error + } if l.logger.VerbosityLevel() == logger.VerbosityVerbose { - l.logger.WriteStdout(preprocFirstResult.Stdout) + l.logger.WriteStdout(preprocResult.Stdout) } // Unwrap error and see if it is an ExitError. var exitErr *exec.ExitError if preprocErr == nil { // Preprocessor successful, done missingIncludeH = "" - } else if isExitErr := errors.As(preprocErr, &exitErr); !isExitErr || len(preprocFirstResult.Stderr) == 0 { + } else if isExitErr := errors.As(preprocErr, &exitErr); !isExitErr || len(preprocResult.Stderr) == 0 { // Ignore ExitErrors (e.g. gcc returning non-zero status), but bail out on other errors return preprocErr } else { - missingIncludeH = IncludesFinderWithRegExp(string(preprocFirstResult.Stderr)) + missingIncludeH = IncludesFinderWithRegExp(string(preprocResult.Stderr)) if missingIncludeH == "" && l.logger.VerbosityLevel() == logger.VerbosityVerbose { l.logger.Info(i18n.Tr("Error while detecting libraries included by %[1]s", sourcePath)) } @@ -396,25 +456,24 @@ func (l *SketchLibrariesDetector) findMissingIncludesInCompilationUnit( library := l.resolveLibrary(missingIncludeH, platformArch) if library == nil { // Library could not be resolved, show error - if preprocErr == nil || len(preprocFirstResult.Stderr) == 0 { - // Filename came from cache, so run preprocessor to obtain error to show - result, err := preprocessor.GCC(ctx, sourcePath, targetFilePath, includeFolders, buildProperties) + + // If preprocess result came from cache, run the preprocessor to obtain the actual error to show + if preprocErr == nil || len(preprocResult.Stderr) == 0 { + preprocResult = preprocTask.Run(ctx) + preprocErr = preprocResult.Error if l.logger.VerbosityLevel() == logger.VerbosityVerbose { - l.logger.WriteStdout(result.Stdout) + l.logger.WriteStdout(preprocResult.Stdout) } - if err == nil { + if preprocErr == nil { // If there is a missing #include in the cache, but running // gcc does not reproduce that, there is something wrong. // Returning an error here will cause the cache to be // deleted, so hopefully the next compilation will succeed. return errors.New(i18n.Tr("Internal error in cache")) } - l.diagnosticStore.Parse(result.Args, result.Stderr) - l.logger.WriteStderr(result.Stderr) - return err } - l.diagnosticStore.Parse(preprocFirstResult.Args, preprocFirstResult.Stderr) - l.logger.WriteStderr(preprocFirstResult.Stderr) + l.diagnosticStore.Parse(preprocResult.Args, preprocResult.Stderr) + l.logger.WriteStderr(preprocResult.Stderr) return preprocErr } @@ -508,8 +567,11 @@ func findIncludeForOldCompilers(source string) string { func LibrariesLoader( useCachedLibrariesResolution bool, librariesManager *librariesmanager.LibrariesManager, - builtInLibrariesDirs *paths.Path, libraryDirs, otherLibrariesDirs paths.PathList, - actualPlatform, targetPlatform *cores.PlatformRelease, + builtInLibrariesDir *paths.Path, + customLibraryDirs paths.PathList, // libraryDirs paths.PathList, + librariesDirs paths.PathList, // otherLibrariesDirs paths.PathList, + buildPlatform *cores.PlatformRelease, + targetPlatform *cores.PlatformRelease, ) (*librariesmanager.LibrariesManager, *librariesresolver.Cpp, []byte, error) { verboseOut := &bytes.Buffer{} lm := librariesManager @@ -521,21 +583,20 @@ func LibrariesLoader( if librariesManager == nil { lmb := librariesmanager.NewBuilder() - builtInLibrariesFolders := builtInLibrariesDirs - if builtInLibrariesFolders != nil { - if err := builtInLibrariesFolders.ToAbs(); err != nil { + if builtInLibrariesDir != nil { + if err := builtInLibrariesDir.ToAbs(); err != nil { return nil, nil, nil, err } lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ - Path: builtInLibrariesFolders, + Path: builtInLibrariesDir, Location: libraries.IDEBuiltIn, }) } - if actualPlatform != targetPlatform { + if buildPlatform != targetPlatform { lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ - PlatformRelease: actualPlatform, - Path: actualPlatform.GetLibrariesDir(), + PlatformRelease: buildPlatform, + Path: buildPlatform.GetLibrariesDir(), Location: libraries.ReferencedPlatformBuiltIn, }) } @@ -545,7 +606,7 @@ func LibrariesLoader( Location: libraries.PlatformBuiltIn, }) - librariesFolders := otherLibrariesDirs + librariesFolders := librariesDirs if err := librariesFolders.ToAbs(); err != nil { return nil, nil, nil, err } @@ -556,7 +617,7 @@ func LibrariesLoader( }) } - for _, dir := range libraryDirs { + for _, dir := range customLibraryDirs { lmb.AddLibrariesDir(librariesmanager.LibrariesDir{ Path: dir, Location: libraries.Unmanaged, @@ -566,18 +627,12 @@ func LibrariesLoader( newLm, libsLoadingWarnings := lmb.Build() for _, status := range libsLoadingWarnings { - // With the refactoring of the initialization step of the CLI we changed how - // errors are returned when loading platforms and libraries, that meant returning a list of - // errors instead of a single one to enhance the experience for the user. - // I have no intention right now to start a refactoring of the legacy package too, so - // here's this shitty solution for now. - // When we're gonna refactor the legacy package this will be gone. verboseOut.Write([]byte(status.Message())) } lm = newLm } allLibs := lm.FindAllInstalled() - resolver := librariesresolver.NewCppResolver(allLibs, targetPlatform, actualPlatform) + resolver := librariesresolver.NewCppResolver(allLibs, targetPlatform, buildPlatform) return lm, resolver, verboseOut.Bytes(), nil } diff --git a/internal/arduino/builder/internal/detector/source_file.go b/internal/arduino/builder/internal/detector/source_file.go index 3ebb52d0387..d99cb1d862a 100644 --- a/internal/arduino/builder/internal/detector/source_file.go +++ b/internal/arduino/builder/internal/detector/source_file.go @@ -16,83 +16,74 @@ package detector import ( + "fmt" "slices" + "github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils" "github.com/arduino/go-paths-helper" ) type sourceFile struct { - // Path to the source file within the sketch/library root folder - relativePath *paths.Path + // SourcePath is the path to the source file + SourcePath *paths.Path `json:"source_path"` + + // ObjectPath is the path to the object file that will be generated + ObjectPath *paths.Path `json:"object_path"` + + // DepfilePath is the path to the dependency file that will be generated + DepfilePath *paths.Path `json:"depfile_path"` // ExtraIncludePath contains an extra include path that must be // used to compile this source file. // This is mainly used for source files that comes from old-style libraries // (Arduino IDE <1.5) requiring an extra include path to the "utility" folder. - extraIncludePath *paths.Path - - // 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. - sourceRoot *paths.Path + ExtraIncludePath *paths.Path `json:"extra_include_path,omitempty"` +} - // The build root for the given origin, where build products will - // be placed. Any directories inside SourceFile.RelativePath will be - // appended here. - buildRoot *paths.Path +func (f *sourceFile) String() string { + return fmt.Sprintf("SourcePath:%s SourceRoot:%s BuildRoot:%s ExtraInclude:%s", + f.SourcePath, f.ObjectPath, f.DepfilePath, f.ExtraIncludePath) } -// Equals fixdoc +// Equals checks if a sourceFile is equal to another. func (f *sourceFile) Equals(g *sourceFile) bool { - return f.relativePath.EqualsTo(g.relativePath) && - f.buildRoot.EqualsTo(g.buildRoot) && - f.sourceRoot.EqualsTo(g.sourceRoot) + return f.SourcePath.EqualsTo(g.SourcePath) && + f.ObjectPath.EqualsTo(g.ObjectPath) && + f.DepfilePath.EqualsTo(g.DepfilePath) && + ((f.ExtraIncludePath == nil && g.ExtraIncludePath == nil) || + (f.ExtraIncludePath != nil && g.ExtraIncludePath != nil && f.ExtraIncludePath.EqualsTo(g.ExtraIncludePath))) } // makeSourceFile create a sourceFile object for the given source file path. // The given sourceFilePath can be absolute, or relative within the sourceRoot root folder. -func makeSourceFile(sourceRoot, buildRoot, sourceFilePath *paths.Path, extraIncludePath ...*paths.Path) (*sourceFile, error) { - res := &sourceFile{ - buildRoot: buildRoot, - sourceRoot: sourceRoot, - } - - if len(extraIncludePath) > 1 { +func makeSourceFile(sourceRoot, buildRoot, sourceFilePath *paths.Path, extraIncludePaths ...*paths.Path) (*sourceFile, error) { + if len(extraIncludePaths) > 1 { panic("only one extra include path allowed") } - if len(extraIncludePath) > 0 { - res.extraIncludePath = extraIncludePath[0] + var extraIncludePath *paths.Path + if len(extraIncludePaths) > 0 { + extraIncludePath = extraIncludePaths[0] } if sourceFilePath.IsAbs() { var err error - sourceFilePath, err = res.sourceRoot.RelTo(sourceFilePath) + sourceFilePath, err = sourceRoot.RelTo(sourceFilePath) if err != nil { return nil, err } } - res.relativePath = sourceFilePath + res := &sourceFile{ + SourcePath: sourceRoot.JoinPath(sourceFilePath), + ObjectPath: buildRoot.Join(sourceFilePath.String() + ".o"), + DepfilePath: buildRoot.Join(sourceFilePath.String() + ".d"), + ExtraIncludePath: extraIncludePath, + } return res, nil } -// ExtraIncludePath returns the extra include path required to build the source file. -func (f *sourceFile) ExtraIncludePath() *paths.Path { - return f.extraIncludePath -} - -// SourcePath return the full path to the source file. -func (f *sourceFile) SourcePath() *paths.Path { - return f.sourceRoot.JoinPath(f.relativePath) -} - -// ObjectPath return the full path to the object file. -func (f *sourceFile) ObjectPath() *paths.Path { - return f.buildRoot.Join(f.relativePath.String() + ".o") -} - -// DepfilePath return the full path to the dependency file. -func (f *sourceFile) DepfilePath() *paths.Path { - return f.buildRoot.Join(f.relativePath.String() + ".d") +// ObjFileIsUpToDate checks if the compile object file is up to date. +func (f *sourceFile) ObjFileIsUpToDate() (unchanged bool, err error) { + return utils.ObjFileIsUpToDate(f.SourcePath, f.ObjectPath, f.DepfilePath) } // uniqueSourceFileQueue is a queue of source files that does not allow duplicates. diff --git a/internal/arduino/builder/internal/preprocessor/arduino_preprocessor.go b/internal/arduino/builder/internal/preprocessor/arduino_preprocessor.go index e69adeae154..dd847c26eda 100644 --- a/internal/arduino/builder/internal/preprocessor/arduino_preprocessor.go +++ b/internal/arduino/builder/internal/preprocessor/arduino_preprocessor.go @@ -45,11 +45,11 @@ func PreprocessSketchWithArduinoPreprocessor( sourceFile := buildPath.Join("sketch", sk.MainFile.Base()+".cpp") targetFile := buildPath.Join("preproc", "sketch_merged.cpp") - gccResult, err := GCC(ctx, sourceFile, targetFile, includeFolders, buildProperties) + gccResult := GCC(sourceFile, targetFile, includeFolders, buildProperties).Run(ctx) verboseOut.Write(gccResult.Stdout) verboseOut.Write(gccResult.Stderr) - if err != nil { - return nil, err + if gccResult.Error != nil { + return nil, gccResult.Error } arduinoPreprocessorProperties := properties.NewMap() @@ -66,18 +66,14 @@ func PreprocessSketchWithArduinoPreprocessor( } commandLine := arduinoPreprocessorProperties.ExpandPropsInString(pattern) - parts, err := properties.SplitQuotedString(commandLine, `"'`, false) - if err != nil { - return nil, err - } - - command, err := paths.NewProcess(nil, parts...) + args, _ := properties.SplitQuotedString(commandLine, `"'`, false) + command, err := paths.NewProcess(nil, args...) if err != nil { return nil, err } if runtime.GOOS == "windows" { // chdir in the uppermost directory to avoid UTF-8 bug in clang (https://github.com/arduino/arduino-preprocessor/issues/2) - command.SetDir(filepath.VolumeName(parts[0]) + "/") + command.SetDir(filepath.VolumeName(args[0]) + "/") } verboseOut.WriteString(commandLine) diff --git a/internal/arduino/builder/internal/preprocessor/ctags.go b/internal/arduino/builder/internal/preprocessor/ctags.go index c09e24aeecd..f66fbabf5cf 100644 --- a/internal/arduino/builder/internal/preprocessor/ctags.go +++ b/internal/arduino/builder/internal/preprocessor/ctags.go @@ -57,10 +57,10 @@ func PreprocessSketchWithCtags( // Run GCC preprocessor sourceFile := buildPath.Join("sketch", sketch.MainFile.Base()+".cpp") - result, err := GCC(ctx, sourceFile, ctagsTarget, includes, buildProperties) + result := GCC(sourceFile, ctagsTarget, includes, buildProperties).Run(ctx) stdout.Write(result.Stdout) stderr.Write(result.Stderr) - if err != nil { + if err := result.Error; err != nil { if !onlyUpdateCompilationDatabase { return &runner.Result{Args: result.Args, Stdout: stdout.Bytes(), Stderr: stderr.Bytes()}, err } diff --git a/internal/arduino/builder/internal/preprocessor/gcc.go b/internal/arduino/builder/internal/preprocessor/gcc.go index 24364482a95..31dc2922ab7 100644 --- a/internal/arduino/builder/internal/preprocessor/gcc.go +++ b/internal/arduino/builder/internal/preprocessor/gcc.go @@ -16,15 +16,10 @@ package preprocessor import ( - "bytes" - "context" - "errors" - "fmt" "strings" "github.com/arduino/arduino-cli/internal/arduino/builder/cpp" "github.com/arduino/arduino-cli/internal/arduino/builder/internal/runner" - "github.com/arduino/arduino-cli/internal/i18n" "github.com/arduino/go-paths-helper" "github.com/arduino/go-properties-orderedmap" "go.bug.st/f" @@ -33,10 +28,9 @@ import ( // GCC performs a run of the gcc preprocess (macro/includes expansion). The function outputs the result // to targetFilePath. Returns the stdout/stderr of gcc if any. func GCC( - ctx context.Context, sourceFilePath, targetFilePath *paths.Path, includes paths.PathList, buildProperties *properties.Map, -) (*runner.Result, error) { +) *runner.Task { gccBuildProperties := properties.NewMap() gccBuildProperties.Set("preproc.macros.flags", "-w -x c++ -E -CC") gccBuildProperties.Merge(buildProperties) @@ -60,61 +54,15 @@ func GCC( } pattern := gccBuildProperties.Get(gccPreprocRecipeProperty) - if pattern == "" { - return nil, errors.New(i18n.Tr("%s pattern is missing", gccPreprocRecipeProperty)) - } - commandLine := gccBuildProperties.ExpandPropsInString(pattern) commandLine = properties.DeleteUnexpandedPropsFromString(commandLine) - args, err := properties.SplitQuotedString(commandLine, `"'`, false) - if err != nil { - return nil, err - } + args, _ := properties.SplitQuotedString(commandLine, `"'`, false) // Remove -MMD argument if present. Leaving it will make gcc try // to create a /dev/null.d dependency file, which won't work. args = f.Filter(args, f.NotEquals("-MMD")) - proc, err := paths.NewProcess(nil, args...) - if err != nil { - return nil, err - } - - stdout := bytes.NewBuffer(nil) - stderr := bytes.NewBuffer(nil) - - ctx, cancel := context.WithCancel(ctx) - defer cancel() - count := 0 - stderrLimited := writerFunc(func(p []byte) (int, error) { - // Limit the size of the stderr buffer to 100KB - n, err := stderr.Write(p) - count += n - if count > 100*1024 { - fmt.Fprintln(stderr, i18n.Tr("Compiler error output has been truncated.")) - cancel() - } - return n, err - }) - - proc.RedirectStdoutTo(stdout) - proc.RedirectStderrTo(stderrLimited) - - // Append gcc arguments to stdout before running the command - fmt.Fprintln(stdout, strings.Join(args, " ")) - - if err := proc.Start(); err != nil { - return &runner.Result{}, err - } - - // Wait for the process to finish - err = proc.WaitWithinContext(ctx) - - return &runner.Result{Args: proc.GetArgs(), Stdout: stdout.Bytes(), Stderr: stderr.Bytes()}, err -} - -type writerFunc func(p []byte) (n int, err error) - -func (f writerFunc) Write(p []byte) (n int, err error) { - return f(p) + // Limit the stderr output to 100 KiB + // https://github.com/arduino/arduino-cli/pull/2883 + return runner.NewTaskWithLimitedStderr(100*1024, args...) } diff --git a/internal/arduino/builder/internal/runner/runner.go b/internal/arduino/builder/internal/runner/runner.go new file mode 100644 index 00000000000..89565bdbc4c --- /dev/null +++ b/internal/arduino/builder/internal/runner/runner.go @@ -0,0 +1,117 @@ +// This file is part of arduino-cli. +// +// Copyright 2024 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package runner + +import ( + "context" + "runtime" + "sync" +) + +// Runner is a helper to run commands in a queue, the commands are immediately exectuded +// in a goroutine as they are enqueued. The runner can be stopped by calling Cancel. +type Runner struct { + lock sync.Mutex + queue chan<- *enqueuedCommand + results map[string]<-chan *Result + ctx context.Context + ctxCancel func() + wg sync.WaitGroup +} + +type enqueuedCommand struct { + task *Task + accept func(*Result) +} + +func (cmd *enqueuedCommand) String() string { + return cmd.task.String() +} + +// New creates a new Runner with the given number of workers. +// If workers is 0, the number of workers will be the number of available CPUs. +func New(inCtx context.Context, workers int) *Runner { + ctx, cancel := context.WithCancel(inCtx) + queue := make(chan *enqueuedCommand, 1000) + r := &Runner{ + ctx: ctx, + ctxCancel: cancel, + queue: queue, + results: map[string]<-chan *Result{}, + } + + // Spawn workers + if workers == 0 { + workers = runtime.NumCPU() + } + for i := 0; i < workers; i++ { + r.wg.Add(1) + go func() { + worker(ctx, queue) + r.wg.Done() + }() + } + + return r +} + +func worker(ctx context.Context, queue <-chan *enqueuedCommand) { + done := ctx.Done() + for { + select { + case <-done: + return + default: + } + + select { + case <-done: + return + case cmd := <-queue: + result := cmd.task.Run(ctx) + cmd.accept(result) + } + } +} + +func (r *Runner) Enqueue(task *Task) { + r.lock.Lock() + defer r.lock.Unlock() + + result := make(chan *Result, 1) + r.results[task.String()] = result + r.queue <- &enqueuedCommand{ + task: task, + accept: func(res *Result) { + result <- res + }, + } +} + +func (r *Runner) Results(task *Task) *Result { + r.lock.Lock() + result, ok := r.results[task.String()] + r.lock.Unlock() + if !ok { + return nil + } + return <-result +} + +func (r *Runner) Cancel() { + r.ctxCancel() + r.wg.Wait() +} diff --git a/internal/arduino/builder/internal/runner/runner_test.go b/internal/arduino/builder/internal/runner/runner_test.go new file mode 100644 index 00000000000..ed499299912 --- /dev/null +++ b/internal/arduino/builder/internal/runner/runner_test.go @@ -0,0 +1,53 @@ +// This file is part of arduino-cli. +// +// Copyright 2024 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package runner_test + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/arduino/arduino-cli/internal/arduino/builder/internal/runner" + "github.com/stretchr/testify/require" +) + +func TestRunMultipleTask(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + defer cancel() + r := runner.New(ctx, 0) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 1 ; echo -n 0")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 2 ; echo -n 1")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 3 ; echo -n 2")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 4 ; echo -n 3")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 5 ; echo -n 4")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 6 ; echo -n 5")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 7 ; echo -n 6")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 8 ; echo -n 7")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 9 ; echo -n 8")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 10 ; echo -n 9")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 11 ; echo -n 10")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 12 ; echo -n 11")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 13 ; echo -n 12")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 14 ; echo -n 13")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 15 ; echo -n 14")) + r.Enqueue(runner.NewTask("bash", "-c", "sleep 16 ; echo -n 15")) + require.Nil(t, r.Results(runner.NewTask("bash", "-c", "echo -n 5"))) + fmt.Println(string(r.Results(runner.NewTask("bash", "-c", "sleep 3 ; echo -n 2")).Stdout)) + fmt.Println("Cancelling") + r.Cancel() + fmt.Println("Runner completed") +} diff --git a/internal/arduino/builder/internal/runner/task.go b/internal/arduino/builder/internal/runner/task.go index 05d5f63105a..9fc0afa5ffd 100644 --- a/internal/arduino/builder/internal/runner/task.go +++ b/internal/arduino/builder/internal/runner/task.go @@ -15,9 +15,89 @@ package runner +import ( + "bytes" + "context" + "fmt" + "strings" + + "github.com/arduino/arduino-cli/internal/i18n" + "github.com/arduino/go-paths-helper" +) + +// Task is a command to be executed +type Task struct { + Args []string `json:"args"` + LimitStderr int `json:"-"` +} + +// NewTask creates a new Task +func NewTask(args ...string) *Task { + return &Task{Args: args} +} + +// NewTaskWithLimitedStderr creates a new Task with a hard-limit on the stderr output +func NewTaskWithLimitedStderr(limit int, args ...string) *Task { + return &Task{Args: args, LimitStderr: limit} +} + +func (t *Task) String() string { + return strings.Join(t.Args, " ") +} + // Result contains the output of a command execution type Result struct { Args []string Stdout []byte Stderr []byte + Error error +} + +// Run executes the command and returns the result +func (t *Task) Run(ctx context.Context) *Result { + proc, err := paths.NewProcess(nil, t.Args...) + if err != nil { + return &Result{Args: t.Args, Error: err} + } + + stdout := bytes.NewBuffer(nil) + stderr := bytes.NewBuffer(nil) + proc.RedirectStdoutTo(stdout) + + if t.LimitStderr > 0 { + innerCtx, cancel := context.WithCancel(ctx) + defer cancel() + + count := 0 + stderrLimited := writerFunc(func(p []byte) (int, error) { + n, err := stderr.Write(p) + count += n + if count > t.LimitStderr { + fmt.Fprintln(stderr, i18n.Tr("Compiler error output has been truncated.")) + cancel() + } + return n, err + }) + + ctx = innerCtx + proc.RedirectStderrTo(stderrLimited) + } else { + proc.RedirectStderrTo(stderr) + } + + // Append arguments to stdout + fmt.Fprintln(stdout, t.String()) + + // Execute command and wait for the process to finish + if err := proc.Start(); err != nil { + return &Result{Error: err} + } + err = proc.WaitWithinContext(ctx) + return &Result{Args: proc.GetArgs(), Stdout: stdout.Bytes(), Stderr: stderr.Bytes(), Error: err} +} + +type writerFunc func(p []byte) (n int, err error) + +func (f writerFunc) Write(p []byte) (n int, err error) { + return f(p) } diff --git a/internal/arduino/builder/sizer.go b/internal/arduino/builder/sizer.go index ae188b48f2b..d084b4760bc 100644 --- a/internal/arduino/builder/sizer.go +++ b/internal/arduino/builder/sizer.go @@ -199,15 +199,17 @@ func (b *Builder) checkSize() (ExecutablesFileSections, error) { return executableSectionsSize, errors.New(i18n.Tr("data section exceeds available space in board")) } + warnDataPercentage := 75 if w := properties.Get("build.warn_data_percentage"); w != "" { - warnDataPercentage, err := strconv.Atoi(w) - if err != nil { - return executableSectionsSize, err - } - if maxDataSize > 0 && dataSize > maxDataSize*warnDataPercentage/100 { - b.logger.Warn(i18n.Tr("Low memory available, stability problems may occur.")) + if p, err := strconv.Atoi(w); err == nil { + warnDataPercentage = p + } else { + b.logger.Warn(i18n.Tr("Invalid value for build.warn_data_percentage: %s", w)) } } + if maxDataSize > 0 && dataSize > maxDataSize*warnDataPercentage/100 { + b.logger.Warn(i18n.Tr("Low memory available, stability problems may occur.")) + } return executableSectionsSize, nil } diff --git a/internal/arduino/cores/packagemanager/loader.go b/internal/arduino/cores/packagemanager/loader.go index 2effccb825d..a9eb8c7984e 100644 --- a/internal/arduino/cores/packagemanager/loader.go +++ b/internal/arduino/cores/packagemanager/loader.go @@ -718,11 +718,8 @@ func (pme *Explorer) loadDiscoveries(release *cores.PlatformRelease) []error { } cmd := configuration.ExpandPropsInString(pattern) - if cmdArgs, err := properties.SplitQuotedString(cmd, `"'`, true); err != nil { - merr = append(merr, err) - } else { - pme.discoveryManager.Add(discoveryID, cmdArgs...) - } + cmdArgs, _ := properties.SplitQuotedString(cmd, `"'`, true) + pme.discoveryManager.Add(discoveryID, cmdArgs...) } return merr diff --git a/internal/integrationtest/compile_4/compile_test.go b/internal/integrationtest/compile_4/compile_test.go index f0916138246..b5a1c882d98 100644 --- a/internal/integrationtest/compile_4/compile_test.go +++ b/internal/integrationtest/compile_4/compile_test.go @@ -1045,15 +1045,14 @@ func TestBuildOptionsFile(t *testing.T) { "sketchLocation" ]`) requirejson.Query(t, buildOptionsBytes, ".fqbn", `"arduino:avr:uno"`) - requirejson.Query(t, buildOptionsBytes, ".customBuildProperties", `"build.warn_data_percentage=75"`) // Recompiling a second time should provide the same result _, _, err = cli.Run("compile", "-b", "arduino:avr:uno", "--build-path", buildPath.String(), sketchPath.String()) require.NoError(t, err) - buildOptionsBytes, err = buildPath.Join("build.options.json").ReadFile() + buildOptionsBytes2, err := buildPath.Join("build.options.json").ReadFile() require.NoError(t, err) - requirejson.Query(t, buildOptionsBytes, ".customBuildProperties", `"build.warn_data_percentage=75"`) + require.Equal(t, buildOptionsBytes, buildOptionsBytes2) // Recompiling with a new build option must produce a new `build.options.json` _, _, err = cli.Run("compile", "-b", "arduino:avr:uno", "--build-path", buildPath.String(), @@ -1062,7 +1061,7 @@ func TestBuildOptionsFile(t *testing.T) { ) require.NoError(t, err) - buildOptionsBytes, err = buildPath.Join("build.options.json").ReadFile() + buildOptionsBytes3, err := buildPath.Join("build.options.json").ReadFile() require.NoError(t, err) - requirejson.Query(t, buildOptionsBytes, ".customBuildProperties", `"custom=prop,build.warn_data_percentage=75"`) + require.NotEqual(t, buildOptionsBytes, buildOptionsBytes3) } diff --git a/internal/integrationtest/compile_4/lib_discovery_caching_test.go b/internal/integrationtest/compile_4/lib_discovery_caching_test.go new file mode 100644 index 00000000000..9d8bd4b5277 --- /dev/null +++ b/internal/integrationtest/compile_4/lib_discovery_caching_test.go @@ -0,0 +1,110 @@ +// This file is part of arduino-cli. +// +// Copyright 2023 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package compile_test + +import ( + "testing" + + "github.com/arduino/arduino-cli/internal/integrationtest" + "github.com/arduino/go-paths-helper" + "github.com/stretchr/testify/require" + "go.bug.st/testifyjson/requirejson" +) + +func TestLibDiscoveryCache(t *testing.T) { + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + t.Cleanup(env.CleanUp) + + // Install Arduino AVR Boards + _, _, err := cli.Run("core", "install", "arduino:avr@1.8.6") + require.NoError(t, err) + + // Copy the testdata sketchbook + testdata, err := paths.New("testdata", "libraries_discovery_caching").Abs() + require.NoError(t, err) + sketchbook := cli.SketchbookDir() + require.NoError(t, sketchbook.RemoveAll()) + require.NoError(t, testdata.CopyDirTo(cli.SketchbookDir())) + + buildpath, err := paths.MkTempDir("", "tmpbuildpath") + require.NoError(t, err) + t.Cleanup(func() { buildpath.RemoveAll() }) + + { + sketchA := sketchbook.Join("SketchA") + { + outjson, _, err := cli.Run("compile", "-v", "-b", "arduino:avr:uno", "--build-path", buildpath.String(), "--json", sketchA.String()) + require.NoError(t, err) + j := requirejson.Parse(t, outjson) + j.MustContain(`{"builder_result":{ + "used_libraries": [ + { "name": "LibA" }, + { "name": "LibB" } + ], + }}`) + } + + // Update SketchA + require.NoError(t, sketchA.Join("SketchA.ino").WriteFile([]byte(` +#include +#include +void setup() {} +void loop() {libAFunction();} +`))) + + { + // This compile should FAIL! + outjson, _, err := cli.Run("compile", "-v", "-b", "arduino:avr:uno", "--build-path", buildpath.String(), "--json", sketchA.String()) + require.Error(t, err) + j := requirejson.Parse(t, outjson) + j.MustContain(`{ +"builder_result":{ + "used_libraries": [ + { "name": "LibC" }, + { "name": "LibA" } + ], + "diagnostics": [ + { + "severity": "ERROR", + "message": "'libAFunction' was not declared in this scope\n void loop() {libAFunction();}\n ^~~~~~~~~~~~" + } + ] +}}`) + j.Query(".compiler_out").MustContain(`"The list of included libraries has been changed... rebuilding all libraries."`) + } + + { + // This compile should FAIL! + outjson, _, err := cli.Run("compile", "-v", "-b", "arduino:avr:uno", "--build-path", buildpath.String(), "--json", sketchA.String()) + require.Error(t, err) + j := requirejson.Parse(t, outjson) + j.MustContain(`{ +"builder_result":{ + "used_libraries": [ + { "name": "LibC" }, + { "name": "LibA" } + ], + "diagnostics": [ + { + "severity": "ERROR", + "message": "'libAFunction' was not declared in this scope\n void loop() {libAFunction();}\n ^~~~~~~~~~~~" + } + ] +}}`) + j.Query(".compiler_out").MustNotContain(`"The list of included libraries has changed... rebuilding all libraries."`) + } + } +} diff --git a/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/SketchA/SketchA.ino b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/SketchA/SketchA.ino new file mode 100644 index 00000000000..b95755d1f82 --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/SketchA/SketchA.ino @@ -0,0 +1,3 @@ +#include +void setup() {} +void loop() {libAFunction();} diff --git a/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibA/LibA.h b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibA/LibA.h new file mode 100644 index 00000000000..5cc5459e363 --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibA/LibA.h @@ -0,0 +1,6 @@ + +#include + +#ifndef CHECK +void libAFunction(); +#endif diff --git a/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibA/file1.cpp b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibA/file1.cpp new file mode 100644 index 00000000000..08474b5de80 --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibA/file1.cpp @@ -0,0 +1,5 @@ +#include + +#ifndef CHECK +void libAFunction() {} +#endif diff --git a/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibB/LibB.h b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibB/LibB.h new file mode 100644 index 00000000000..e69de29bb2d diff --git a/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibC/LibB.h b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibC/LibB.h new file mode 100644 index 00000000000..4dbb6c43193 --- /dev/null +++ b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibC/LibB.h @@ -0,0 +1,2 @@ + +#define CHECK diff --git a/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibC/LibC.h b/internal/integrationtest/compile_4/testdata/libraries_discovery_caching/libraries/LibC/LibC.h new file mode 100644 index 00000000000..e69de29bb2d