Skip to content

legacy: Make a lot of Builder internals private #2325

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

Merged
merged 12 commits into from
Sep 21, 2023
77 changes: 33 additions & 44 deletions arduino/builder/build_options_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,17 @@ import (
"path/filepath"
"strings"

"github.com/arduino/arduino-cli/arduino/builder/logger"
"github.com/arduino/arduino-cli/arduino/builder/utils"
"github.com/arduino/arduino-cli/arduino/builder/internal/utils"
"github.com/arduino/arduino-cli/arduino/cores"
"github.com/arduino/arduino-cli/arduino/sketch"
"github.com/arduino/go-paths-helper"
properties "github.com/arduino/go-properties-orderedmap"
"github.com/pkg/errors"
)

// BuildOptionsManager fixdoc
type BuildOptionsManager struct {
currentOptions *properties.Map
currentBuildOptionsJSON []byte
// buildOptions fixdoc
type buildOptions struct {
currentOptions *properties.Map

hardwareDirs paths.PathList
builtInToolsDirs paths.PathList
Expand All @@ -45,11 +43,10 @@ type BuildOptionsManager struct {
customBuildProperties []string
compilerOptimizationFlags string
clean bool
builderLogger *logger.BuilderLogger
}

// NewBuildOptionsManager fixdoc
func NewBuildOptionsManager(
// newBuildOptions fixdoc
func newBuildOptions(
hardwareDirs, builtInToolsDirs, otherLibrariesDirs paths.PathList,
builtInLibrariesDirs, buildPath *paths.Path,
sketch *sketch.Sketch,
Expand All @@ -58,8 +55,7 @@ func NewBuildOptionsManager(
clean bool,
compilerOptimizationFlags string,
runtimePlatformPath, buildCorePath *paths.Path,
buildLogger *logger.BuilderLogger,
) *BuildOptionsManager {
) *buildOptions {
opts := properties.NewMap()

opts.Set("hardwareFolders", strings.Join(hardwareDirs.AsStrings(), ","))
Expand All @@ -85,7 +81,7 @@ func NewBuildOptionsManager(
}
opts.Set("additionalFiles", strings.Join(additionalFilesRelative, ","))

return &BuildOptionsManager{
return &buildOptions{
currentOptions: opts,
hardwareDirs: hardwareDirs,
builtInToolsDirs: builtInToolsDirs,
Expand All @@ -98,46 +94,39 @@ func NewBuildOptionsManager(
customBuildProperties: customBuildProperties,
compilerOptimizationFlags: compilerOptimizationFlags,
clean: clean,
builderLogger: buildLogger,
}
}

// WipeBuildPath fixdoc
func (m *BuildOptionsManager) WipeBuildPath() error {
buildOptionsJSON, err := json.MarshalIndent(m.currentOptions, "", " ")
func (b *Builder) createBuildOptionsJSON() error {
buildOptionsJSON, err := json.MarshalIndent(b.buildOptions.currentOptions, "", " ")
if err != nil {
return errors.WithStack(err)
}
m.currentBuildOptionsJSON = buildOptionsJSON

if err := m.wipeBuildPath(); err != nil {
return errors.WithStack(err)
}
return m.buildPath.Join("build.options.json").WriteFile(buildOptionsJSON)
return b.buildOptions.buildPath.Join("build.options.json").WriteFile(buildOptionsJSON)
}

func (m *BuildOptionsManager) wipeBuildPath() error {
wipe := func() error {
// FIXME: this should go outside legacy and behind a `logrus` call so users can
// control when this should be printed.
// logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_BUILD_OPTIONS_CHANGED + constants.MSG_REBUILD_ALL)
if err := m.buildPath.RemoveAll(); err != nil {
return errors.WithMessage(err, tr("cleaning build path"))
}
if err := m.buildPath.MkdirAll(); err != nil {
return errors.WithMessage(err, tr("cleaning build path"))
}
return nil
func (b *Builder) wipeBuildPath() error {
// FIXME: this should go outside legacy and behind a `logrus` call so users can
// control when this should be printed.
// logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_BUILD_OPTIONS_CHANGED + constants.MSG_REBUILD_ALL)
if err := b.buildOptions.buildPath.RemoveAll(); err != nil {
return errors.WithMessage(err, tr("cleaning build path"))
}
if err := b.buildOptions.buildPath.MkdirAll(); err != nil {
return errors.WithMessage(err, tr("cleaning build path"))
}
return nil
}

if m.clean {
return wipe()
func (b *Builder) wipeBuildPathIfBuildOptionsChanged() error {
if b.buildOptions.clean {
return b.wipeBuildPath()
}

// Load previous build options map
var buildOptionsJSONPrevious []byte
var _err error
if buildOptionsFile := m.buildPath.Join("build.options.json"); buildOptionsFile.Exist() {
if buildOptionsFile := b.buildOptions.buildPath.Join("build.options.json"); buildOptionsFile.Exist() {
buildOptionsJSONPrevious, _err = buildOptionsFile.ReadFile()
if _err != nil {
return errors.WithStack(_err)
Expand All @@ -150,12 +139,12 @@ func (m *BuildOptionsManager) wipeBuildPath() error {

var prevOpts *properties.Map
if err := json.Unmarshal(buildOptionsJSONPrevious, &prevOpts); err != nil || prevOpts == nil {
m.builderLogger.Info(tr("%[1]s invalid, rebuilding all", "build.options.json"))
return wipe()
b.logger.Info(tr("%[1]s invalid, rebuilding all", "build.options.json"))
return b.wipeBuildPath()
}

// Since we might apply a side effect we clone it
currentOptions := m.currentOptions.Clone()
currentOptions := b.buildOptions.currentOptions.Clone()
// If SketchLocation path is different but filename is the same, consider it equal
if filepath.Base(currentOptions.Get("sketchLocation")) == filepath.Base(prevOpts.Get("sketchLocation")) {
currentOptions.Remove("sketchLocation")
Expand All @@ -167,10 +156,10 @@ func (m *BuildOptionsManager) wipeBuildPath() error {
// check if any of the files contained in the core folders has changed
// since the json was generated - like platform.txt or similar
// if so, trigger a "safety" wipe
targetCoreFolder := m.runtimePlatformPath
coreFolder := m.buildCorePath
targetCoreFolder := b.buildOptions.runtimePlatformPath
coreFolder := b.buildOptions.buildCorePath
realCoreFolder := coreFolder.Parent().Parent()
jsonPath := m.buildPath.Join("build.options.json")
jsonPath := b.buildOptions.buildPath.Join("build.options.json")
coreUnchanged, _ := utils.DirContentIsOlderThan(realCoreFolder, jsonPath, ".txt")
if coreUnchanged && targetCoreFolder != nil && !realCoreFolder.EqualsTo(targetCoreFolder) {
coreUnchanged, _ = utils.DirContentIsOlderThan(targetCoreFolder, jsonPath, ".txt")
Expand All @@ -180,5 +169,5 @@ func (m *BuildOptionsManager) wipeBuildPath() error {
}
}

return wipe()
return b.wipeBuildPath()
}
68 changes: 38 additions & 30 deletions arduino/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@ package builder
import (
"errors"
"fmt"
"io"

"github.com/arduino/arduino-cli/arduino/builder/compilation"
"github.com/arduino/arduino-cli/arduino/builder/detector"
"github.com/arduino/arduino-cli/arduino/builder/logger"
"github.com/arduino/arduino-cli/arduino/builder/progress"
"github.com/arduino/arduino-cli/arduino/builder/internal/compilation"
"github.com/arduino/arduino-cli/arduino/builder/internal/detector"
"github.com/arduino/arduino-cli/arduino/builder/internal/logger"
"github.com/arduino/arduino-cli/arduino/builder/internal/progress"
"github.com/arduino/arduino-cli/arduino/cores"
"github.com/arduino/arduino-cli/arduino/libraries"
"github.com/arduino/arduino-cli/arduino/libraries/librariesmanager"
"github.com/arduino/arduino-cli/arduino/sketch"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-paths-helper"
"github.com/arduino/go-properties-orderedmap"
)
Expand Down Expand Up @@ -77,14 +80,15 @@ type Builder struct {
targetPlatform *cores.PlatformRelease
actualPlatform *cores.PlatformRelease

buildArtifacts *BuildArtifacts
buildArtifacts *buildArtifacts

*detector.SketchLibrariesDetector
*BuildOptionsManager
buildOptions *buildOptions

libsDetector *detector.SketchLibrariesDetector
}

// BuildArtifacts contains the result of various build
type BuildArtifacts struct {
// buildArtifacts contains the result of various build
type buildArtifacts struct {
// populated by BuildCore
coreArchiveFilePath *paths.Path
coreObjectsFiles paths.PathList
Expand Down Expand Up @@ -115,8 +119,8 @@ func NewBuilder(
useCachedLibrariesResolution bool,
librariesManager *librariesmanager.LibrariesManager,
libraryDirs paths.PathList,
logger *logger.BuilderLogger,
progressStats *progress.Struct,
stdout, stderr io.Writer, verbose bool, warningsLevel string,
progresCB rpc.TaskProgressCB,
) (*Builder, error) {
buildProperties := properties.NewMap()
if boardBuildProperties != nil {
Expand Down Expand Up @@ -165,10 +169,7 @@ func NewBuilder(
return nil, ErrSketchCannotBeLocatedInBuildPath
}

if progressStats == nil {
progressStats = progress.New(nil)
}

logger := logger.New(stdout, stderr, verbose, warningsLevel)
libsManager, libsResolver, verboseOut, err := detector.LibrariesLoader(
useCachedLibrariesResolution, librariesManager,
builtInLibrariesDirs, libraryDirs, otherLibrariesDirs,
Expand Down Expand Up @@ -196,18 +197,18 @@ func NewBuilder(
sourceOverrides: sourceOverrides,
onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase,
compilationDatabase: compilation.NewDatabase(buildPath.Join("compile_commands.json")),
Progress: progressStats,
Progress: progress.New(progresCB),
executableSectionsSize: []ExecutableSectionSize{},
buildArtifacts: &BuildArtifacts{},
buildArtifacts: &buildArtifacts{},
targetPlatform: targetPlatform,
actualPlatform: actualPlatform,
SketchLibrariesDetector: detector.NewSketchLibrariesDetector(
libsDetector: detector.NewSketchLibrariesDetector(
libsManager, libsResolver,
useCachedLibrariesResolution,
onlyUpdateCompilationDatabase,
logger,
),
BuildOptionsManager: NewBuildOptionsManager(
buildOptions: newBuildOptions(
hardwareDirs, builtInToolsDirs, otherLibrariesDirs,
builtInLibrariesDirs, buildPath,
sk,
Expand All @@ -217,7 +218,6 @@ func NewBuilder(
buildProperties.Get("compiler.optimization_flags"),
buildProperties.GetPath("runtime.platform.path"),
buildProperties.GetPath("build.core.path"), // TODO can we buildCorePath ?
logger,
),
}, nil
}
Expand All @@ -237,6 +237,11 @@ func (b *Builder) ExecutableSectionsSize() ExecutablesFileSections {
return b.executableSectionsSize
}

// ImportedLibraries fixdoc
func (b *Builder) ImportedLibraries() libraries.List {
return b.libsDetector.ImportedLibraries()
}

// Preprocess fixdoc
func (b *Builder) Preprocess() error {
b.Progress.AddSubSteps(6)
Expand All @@ -249,7 +254,10 @@ func (b *Builder) preprocess() error {
return err
}

if err := b.BuildOptionsManager.WipeBuildPath(); err != nil {
if err := b.wipeBuildPathIfBuildOptionsChanged(); err != nil {
return err
}
if err := b.createBuildOptionsJSON(); err != nil {
return err
}
b.Progress.CompleteStep()
Expand All @@ -268,7 +276,7 @@ func (b *Builder) preprocess() error {
b.Progress.PushProgress()

b.logIfVerbose(false, tr("Detecting libraries used..."))
err := b.SketchLibrariesDetector.FindIncludes(
err := b.libsDetector.FindIncludes(
b.buildPath,
b.buildProperties.GetPath("build.core.path"),
b.buildProperties.GetPath("build.variant.path"),
Expand All @@ -284,12 +292,12 @@ func (b *Builder) preprocess() error {
b.Progress.CompleteStep()
b.Progress.PushProgress()

b.warnAboutArchIncompatibleLibraries(b.SketchLibrariesDetector.ImportedLibraries())
b.warnAboutArchIncompatibleLibraries(b.libsDetector.ImportedLibraries())
b.Progress.CompleteStep()
b.Progress.PushProgress()

b.logIfVerbose(false, tr("Generating function prototypes..."))
if err := b.preprocessSketch(b.SketchLibrariesDetector.IncludeFolders()); err != nil {
if err := b.preprocessSketch(b.libsDetector.IncludeFolders()); err != nil {
return err
}
b.Progress.CompleteStep()
Expand Down Expand Up @@ -327,18 +335,18 @@ func (b *Builder) Build() error {

buildErr := b.build()

b.SketchLibrariesDetector.PrintUsedAndNotUsedLibraries(buildErr != nil)
b.libsDetector.PrintUsedAndNotUsedLibraries(buildErr != nil)
b.Progress.CompleteStep()
b.Progress.PushProgress()

b.printUsedLibraries(b.SketchLibrariesDetector.ImportedLibraries())
b.printUsedLibraries(b.libsDetector.ImportedLibraries())
b.Progress.CompleteStep()
b.Progress.PushProgress()

if buildErr != nil {
return buildErr
}
if err := b.exportProjectCMake(b.SketchLibrariesDetector.ImportedLibraries(), b.SketchLibrariesDetector.IncludeFolders()); err != nil {
if err := b.exportProjectCMake(b.libsDetector.ImportedLibraries(), b.libsDetector.IncludeFolders()); err != nil {
return err
}
b.Progress.CompleteStep()
Expand All @@ -362,7 +370,7 @@ func (b *Builder) build() error {
b.Progress.CompleteStep()
b.Progress.PushProgress()

if err := b.BuildSketch(b.SketchLibrariesDetector.IncludeFolders()); err != nil {
if err := b.BuildSketch(b.libsDetector.IncludeFolders()); err != nil {
return err
}
b.Progress.CompleteStep()
Expand All @@ -381,13 +389,13 @@ func (b *Builder) build() error {
b.Progress.CompleteStep()
b.Progress.PushProgress()

if err := b.removeUnusedCompiledLibraries(b.SketchLibrariesDetector.ImportedLibraries()); err != nil {
if err := b.removeUnusedCompiledLibraries(b.libsDetector.ImportedLibraries()); err != nil {
return err
}
b.Progress.CompleteStep()
b.Progress.PushProgress()

if err := b.buildLibraries(b.SketchLibrariesDetector.IncludeFolders(), b.SketchLibrariesDetector.ImportedLibraries()); err != nil {
if err := b.buildLibraries(b.libsDetector.IncludeFolders(), b.libsDetector.ImportedLibraries()); err != nil {
return err
}
b.Progress.CompleteStep()
Expand Down
2 changes: 1 addition & 1 deletion arduino/builder/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"strings"

"github.com/arduino/arduino-cli/arduino/builder/cpp"
"github.com/arduino/arduino-cli/arduino/builder/utils"
"github.com/arduino/arduino-cli/arduino/builder/internal/utils"
"github.com/arduino/arduino-cli/buildcache"
f "github.com/arduino/arduino-cli/internal/algorithms"
"github.com/arduino/go-paths-helper"
Expand Down
7 changes: 3 additions & 4 deletions arduino/builder/export_cmake.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,11 @@ import (
"slices"
"strings"

"github.com/arduino/go-paths-helper"
properties "github.com/arduino/go-properties-orderedmap"

"github.com/arduino/arduino-cli/arduino/builder/utils"
"github.com/arduino/arduino-cli/arduino/builder/internal/utils"
"github.com/arduino/arduino-cli/arduino/globals"
"github.com/arduino/arduino-cli/arduino/libraries"
"github.com/arduino/go-paths-helper"
properties "github.com/arduino/go-properties-orderedmap"
)

var lineMatcher = regexp.MustCompile(`^#line\s\d+\s"`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import (
"strings"
"time"

"github.com/arduino/arduino-cli/arduino/builder/logger"
"github.com/arduino/arduino-cli/arduino/builder/preprocessor"
"github.com/arduino/arduino-cli/arduino/builder/utils"
"github.com/arduino/arduino-cli/arduino/builder/internal/logger"
"github.com/arduino/arduino-cli/arduino/builder/internal/preprocessor"
"github.com/arduino/arduino-cli/arduino/builder/internal/utils"
"github.com/arduino/arduino-cli/arduino/cores"
"github.com/arduino/arduino-cli/arduino/globals"
"github.com/arduino/arduino-cli/arduino/libraries"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package detector_test
import (
"testing"

"github.com/arduino/arduino-cli/arduino/builder/detector"
"github.com/arduino/arduino-cli/arduino/builder/internal/detector"
"github.com/stretchr/testify/require"
)

Expand Down
Loading