Skip to content

Commit dde3064

Browse files
cmagliecmaglie
authored
legacy: Make a lot of Builder internals private (#2325)
* Removed logger as a field of BuildOptionManager * Made BuildOptionsManager a field of Builder * Renamed BuildOptionsManager -> BuildOptions * Made WipeBuildPath a method of Builder * Removed unused currentBuildOptionsJSON field * Separated wipeBuildPath and createBuildOptionsJSON actions * Made wipe() a proper method -> Buidler.wipeBuildPath() * Made BuildOptions private * Made SketchLibrariesDetector a private field of Buidler * Made BuildArtifacts private * Made all builder subpackages internal * Moved initialization of private structs inside NewBuilder
1 parent 0e6133f commit dde3064

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+95
-101
lines changed

‎arduino/builder/build_options_manager.go‎

Lines changed: 33 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,17 @@ import (
2020
"path/filepath"
2121
"strings"
2222

23-
"github.com/arduino/arduino-cli/arduino/builder/logger"
24-
"github.com/arduino/arduino-cli/arduino/builder/utils"
23+
"github.com/arduino/arduino-cli/arduino/builder/internal/utils"
2524
"github.com/arduino/arduino-cli/arduino/cores"
2625
"github.com/arduino/arduino-cli/arduino/sketch"
2726
"github.com/arduino/go-paths-helper"
2827
properties "github.com/arduino/go-properties-orderedmap"
2928
"github.com/pkg/errors"
3029
)
3130

32-
// BuildOptionsManager fixdoc
33-
type BuildOptionsManager struct {
34-
currentOptions *properties.Map
35-
currentBuildOptionsJSON []byte
31+
// buildOptions fixdoc
32+
type buildOptions struct {
33+
currentOptions *properties.Map
3634

3735
hardwareDirs paths.PathList
3836
builtInToolsDirs paths.PathList
@@ -45,11 +43,10 @@ type BuildOptionsManager struct {
4543
customBuildProperties []string
4644
compilerOptimizationFlags string
4745
clean bool
48-
builderLogger *logger.BuilderLogger
4946
}
5047

51-
// NewBuildOptionsManager fixdoc
52-
func NewBuildOptionsManager(
48+
// newBuildOptions fixdoc
49+
func newBuildOptions(
5350
hardwareDirs, builtInToolsDirs, otherLibrariesDirs paths.PathList,
5451
builtInLibrariesDirs, buildPath *paths.Path,
5552
sketch *sketch.Sketch,
@@ -58,8 +55,7 @@ func NewBuildOptionsManager(
5855
clean bool,
5956
compilerOptimizationFlags string,
6057
runtimePlatformPath, buildCorePath *paths.Path,
61-
buildLogger *logger.BuilderLogger,
62-
) *BuildOptionsManager {
58+
) *buildOptions {
6359
opts := properties.NewMap()
6460

6561
opts.Set("hardwareFolders", strings.Join(hardwareDirs.AsStrings(), ","))
@@ -85,7 +81,7 @@ func NewBuildOptionsManager(
8581
}
8682
opts.Set("additionalFiles", strings.Join(additionalFilesRelative, ","))
8783

88-
return &BuildOptionsManager{
84+
return &buildOptions{
8985
currentOptions: opts,
9086
hardwareDirs: hardwareDirs,
9187
builtInToolsDirs: builtInToolsDirs,
@@ -98,46 +94,39 @@ func NewBuildOptionsManager(
9894
customBuildProperties: customBuildProperties,
9995
compilerOptimizationFlags: compilerOptimizationFlags,
10096
clean: clean,
101-
builderLogger: buildLogger,
10297
}
10398
}
10499

105-
// WipeBuildPath fixdoc
106-
func (m *BuildOptionsManager) WipeBuildPath() error {
107-
buildOptionsJSON, err := json.MarshalIndent(m.currentOptions, "", " ")
100+
func (b *Builder) createBuildOptionsJSON() error {
101+
buildOptionsJSON, err := json.MarshalIndent(b.buildOptions.currentOptions, "", " ")
108102
if err != nil {
109103
return errors.WithStack(err)
110104
}
111-
m.currentBuildOptionsJSON = buildOptionsJSON
112-
113-
if err := m.wipeBuildPath(); err != nil {
114-
return errors.WithStack(err)
115-
}
116-
return m.buildPath.Join("build.options.json").WriteFile(buildOptionsJSON)
105+
return b.buildOptions.buildPath.Join("build.options.json").WriteFile(buildOptionsJSON)
117106
}
118107

119-
func (m *BuildOptionsManager) wipeBuildPath() error {
120-
wipe := func() error {
121-
// FIXME: this should go outside legacy and behind a `logrus` call so users can
122-
// control when this should be printed.
123-
// logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_BUILD_OPTIONS_CHANGED + constants.MSG_REBUILD_ALL)
124-
if err := m.buildPath.RemoveAll(); err != nil {
125-
return errors.WithMessage(err, tr("cleaning build path"))
126-
}
127-
if err := m.buildPath.MkdirAll(); err != nil {
128-
return errors.WithMessage(err, tr("cleaning build path"))
129-
}
130-
return nil
108+
func (b *Builder) wipeBuildPath() error {
109+
// FIXME: this should go outside legacy and behind a `logrus` call so users can
110+
// control when this should be printed.
111+
// logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_BUILD_OPTIONS_CHANGED + constants.MSG_REBUILD_ALL)
112+
if err := b.buildOptions.buildPath.RemoveAll(); err != nil {
113+
return errors.WithMessage(err, tr("cleaning build path"))
131114
}
115+
if err := b.buildOptions.buildPath.MkdirAll(); err != nil {
116+
return errors.WithMessage(err, tr("cleaning build path"))
117+
}
118+
return nil
119+
}
132120

133-
if m.clean {
134-
return wipe()
121+
func (b *Builder) wipeBuildPathIfBuildOptionsChanged() error {
122+
if b.buildOptions.clean {
123+
return b.wipeBuildPath()
135124
}
136125

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

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

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

183-
return wipe()
172+
return b.wipeBuildPath()
184173
}

‎arduino/builder/builder.go‎

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,17 @@ package builder
1818
import (
1919
"errors"
2020
"fmt"
21+
"io"
2122

22-
"github.com/arduino/arduino-cli/arduino/builder/compilation"
23-
"github.com/arduino/arduino-cli/arduino/builder/detector"
24-
"github.com/arduino/arduino-cli/arduino/builder/logger"
25-
"github.com/arduino/arduino-cli/arduino/builder/progress"
23+
"github.com/arduino/arduino-cli/arduino/builder/internal/compilation"
24+
"github.com/arduino/arduino-cli/arduino/builder/internal/detector"
25+
"github.com/arduino/arduino-cli/arduino/builder/internal/logger"
26+
"github.com/arduino/arduino-cli/arduino/builder/internal/progress"
2627
"github.com/arduino/arduino-cli/arduino/cores"
28+
"github.com/arduino/arduino-cli/arduino/libraries"
2729
"github.com/arduino/arduino-cli/arduino/libraries/librariesmanager"
2830
"github.com/arduino/arduino-cli/arduino/sketch"
31+
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
2932
"github.com/arduino/go-paths-helper"
3033
"github.com/arduino/go-properties-orderedmap"
3134
)
@@ -77,14 +80,15 @@ type Builder struct {
7780
targetPlatform *cores.PlatformRelease
7881
actualPlatform *cores.PlatformRelease
7982

80-
buildArtifacts *BuildArtifacts
83+
buildArtifacts *buildArtifacts
8184

82-
*detector.SketchLibrariesDetector
83-
*BuildOptionsManager
85+
buildOptions *buildOptions
86+
87+
libsDetector *detector.SketchLibrariesDetector
8488
}
8589

86-
// BuildArtifacts contains the result of various build
87-
type BuildArtifacts struct {
90+
// buildArtifacts contains the result of various build
91+
type buildArtifacts struct {
8892
// populated by BuildCore
8993
coreArchiveFilePath *paths.Path
9094
coreObjectsFiles paths.PathList
@@ -115,8 +119,8 @@ func NewBuilder(
115119
useCachedLibrariesResolution bool,
116120
librariesManager *librariesmanager.LibrariesManager,
117121
libraryDirs paths.PathList,
118-
logger *logger.BuilderLogger,
119-
progressStats *progress.Struct,
122+
stdout, stderr io.Writer, verbose bool, warningsLevel string,
123+
progresCB rpc.TaskProgressCB,
120124
) (*Builder, error) {
121125
buildProperties := properties.NewMap()
122126
if boardBuildProperties != nil {
@@ -165,10 +169,7 @@ func NewBuilder(
165169
return nil, ErrSketchCannotBeLocatedInBuildPath
166170
}
167171

168-
if progressStats == nil {
169-
progressStats = progress.New(nil)
170-
}
171-
172+
logger := logger.New(stdout, stderr, verbose, warningsLevel)
172173
libsManager, libsResolver, verboseOut, err := detector.LibrariesLoader(
173174
useCachedLibrariesResolution, librariesManager,
174175
builtInLibrariesDirs, libraryDirs, otherLibrariesDirs,
@@ -196,18 +197,18 @@ func NewBuilder(
196197
sourceOverrides: sourceOverrides,
197198
onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase,
198199
compilationDatabase: compilation.NewDatabase(buildPath.Join("compile_commands.json")),
199-
Progress: progressStats,
200+
Progress: progress.New(progresCB),
200201
executableSectionsSize: []ExecutableSectionSize{},
201-
buildArtifacts: &BuildArtifacts{},
202+
buildArtifacts: &buildArtifacts{},
202203
targetPlatform: targetPlatform,
203204
actualPlatform: actualPlatform,
204-
SketchLibrariesDetector: detector.NewSketchLibrariesDetector(
205+
libsDetector: detector.NewSketchLibrariesDetector(
205206
libsManager, libsResolver,
206207
useCachedLibrariesResolution,
207208
onlyUpdateCompilationDatabase,
208209
logger,
209210
),
210-
BuildOptionsManager: NewBuildOptionsManager(
211+
buildOptions: newBuildOptions(
211212
hardwareDirs, builtInToolsDirs, otherLibrariesDirs,
212213
builtInLibrariesDirs, buildPath,
213214
sk,
@@ -217,7 +218,6 @@ func NewBuilder(
217218
buildProperties.Get("compiler.optimization_flags"),
218219
buildProperties.GetPath("runtime.platform.path"),
219220
buildProperties.GetPath("build.core.path"), // TODO can we buildCorePath ?
220-
logger,
221221
),
222222
}, nil
223223
}
@@ -237,6 +237,11 @@ func (b *Builder) ExecutableSectionsSize() ExecutablesFileSections {
237237
return b.executableSectionsSize
238238
}
239239

240+
// ImportedLibraries fixdoc
241+
func (b *Builder) ImportedLibraries() libraries.List {
242+
return b.libsDetector.ImportedLibraries()
243+
}
244+
240245
// Preprocess fixdoc
241246
func (b *Builder) Preprocess() error {
242247
b.Progress.AddSubSteps(6)
@@ -249,7 +254,10 @@ func (b *Builder) preprocess() error {
249254
return err
250255
}
251256

252-
if err := b.BuildOptionsManager.WipeBuildPath(); err != nil {
257+
if err := b.wipeBuildPathIfBuildOptionsChanged(); err != nil {
258+
return err
259+
}
260+
if err := b.createBuildOptionsJSON(); err != nil {
253261
return err
254262
}
255263
b.Progress.CompleteStep()
@@ -268,7 +276,7 @@ func (b *Builder) preprocess() error {
268276
b.Progress.PushProgress()
269277

270278
b.logIfVerbose(false, tr("Detecting libraries used..."))
271-
err := b.SketchLibrariesDetector.FindIncludes(
279+
err := b.libsDetector.FindIncludes(
272280
b.buildPath,
273281
b.buildProperties.GetPath("build.core.path"),
274282
b.buildProperties.GetPath("build.variant.path"),
@@ -284,12 +292,12 @@ func (b *Builder) preprocess() error {
284292
b.Progress.CompleteStep()
285293
b.Progress.PushProgress()
286294

287-
b.warnAboutArchIncompatibleLibraries(b.SketchLibrariesDetector.ImportedLibraries())
295+
b.warnAboutArchIncompatibleLibraries(b.libsDetector.ImportedLibraries())
288296
b.Progress.CompleteStep()
289297
b.Progress.PushProgress()
290298

291299
b.logIfVerbose(false, tr("Generating function prototypes..."))
292-
if err := b.preprocessSketch(b.SketchLibrariesDetector.IncludeFolders()); err != nil {
300+
if err := b.preprocessSketch(b.libsDetector.IncludeFolders()); err != nil {
293301
return err
294302
}
295303
b.Progress.CompleteStep()
@@ -327,18 +335,18 @@ func (b *Builder) Build() error {
327335

328336
buildErr := b.build()
329337

330-
b.SketchLibrariesDetector.PrintUsedAndNotUsedLibraries(buildErr != nil)
338+
b.libsDetector.PrintUsedAndNotUsedLibraries(buildErr != nil)
331339
b.Progress.CompleteStep()
332340
b.Progress.PushProgress()
333341

334-
b.printUsedLibraries(b.SketchLibrariesDetector.ImportedLibraries())
342+
b.printUsedLibraries(b.libsDetector.ImportedLibraries())
335343
b.Progress.CompleteStep()
336344
b.Progress.PushProgress()
337345

338346
if buildErr != nil {
339347
return buildErr
340348
}
341-
if err := b.exportProjectCMake(b.SketchLibrariesDetector.ImportedLibraries(), b.SketchLibrariesDetector.IncludeFolders()); err != nil {
349+
if err := b.exportProjectCMake(b.libsDetector.ImportedLibraries(), b.libsDetector.IncludeFolders()); err != nil {
342350
return err
343351
}
344352
b.Progress.CompleteStep()
@@ -362,7 +370,7 @@ func (b *Builder) build() error {
362370
b.Progress.CompleteStep()
363371
b.Progress.PushProgress()
364372

365-
if err := b.BuildSketch(b.SketchLibrariesDetector.IncludeFolders()); err != nil {
373+
if err := b.BuildSketch(b.libsDetector.IncludeFolders()); err != nil {
366374
return err
367375
}
368376
b.Progress.CompleteStep()
@@ -381,13 +389,13 @@ func (b *Builder) build() error {
381389
b.Progress.CompleteStep()
382390
b.Progress.PushProgress()
383391

384-
if err := b.removeUnusedCompiledLibraries(b.SketchLibrariesDetector.ImportedLibraries()); err != nil {
392+
if err := b.removeUnusedCompiledLibraries(b.libsDetector.ImportedLibraries()); err != nil {
385393
return err
386394
}
387395
b.Progress.CompleteStep()
388396
b.Progress.PushProgress()
389397

390-
if err := b.buildLibraries(b.SketchLibrariesDetector.IncludeFolders(), b.SketchLibrariesDetector.ImportedLibraries()); err != nil {
398+
if err := b.buildLibraries(b.libsDetector.IncludeFolders(), b.libsDetector.ImportedLibraries()); err != nil {
391399
return err
392400
}
393401
b.Progress.CompleteStep()

‎arduino/builder/core.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"strings"
2424

2525
"github.com/arduino/arduino-cli/arduino/builder/cpp"
26-
"github.com/arduino/arduino-cli/arduino/builder/utils"
26+
"github.com/arduino/arduino-cli/arduino/builder/internal/utils"
2727
"github.com/arduino/arduino-cli/buildcache"
2828
f "github.com/arduino/arduino-cli/internal/algorithms"
2929
"github.com/arduino/go-paths-helper"

‎arduino/builder/export_cmake.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,11 @@ import (
2424
"slices"
2525
"strings"
2626

27-
"github.com/arduino/go-paths-helper"
28-
properties "github.com/arduino/go-properties-orderedmap"
29-
30-
"github.com/arduino/arduino-cli/arduino/builder/utils"
27+
"github.com/arduino/arduino-cli/arduino/builder/internal/utils"
3128
"github.com/arduino/arduino-cli/arduino/globals"
3229
"github.com/arduino/arduino-cli/arduino/libraries"
30+
"github.com/arduino/go-paths-helper"
31+
properties "github.com/arduino/go-properties-orderedmap"
3332
)
3433

3534
var lineMatcher = regexp.MustCompile(`^#line\s\d+\s"`)

‎arduino/builder/compilation/database.go‎ renamed to ‎arduino/builder/internal/compilation/database.go‎

File renamed without changes.

‎arduino/builder/compilation/database_test.go‎ renamed to ‎arduino/builder/internal/compilation/database_test.go‎

File renamed without changes.

‎arduino/builder/detector/detector.go‎ renamed to ‎arduino/builder/internal/detector/detector.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ import (
2525
"strings"
2626
"time"
2727

28-
"github.com/arduino/arduino-cli/arduino/builder/logger"
29-
"github.com/arduino/arduino-cli/arduino/builder/preprocessor"
30-
"github.com/arduino/arduino-cli/arduino/builder/utils"
28+
"github.com/arduino/arduino-cli/arduino/builder/internal/logger"
29+
"github.com/arduino/arduino-cli/arduino/builder/internal/preprocessor"
30+
"github.com/arduino/arduino-cli/arduino/builder/internal/utils"
3131
"github.com/arduino/arduino-cli/arduino/cores"
3232
"github.com/arduino/arduino-cli/arduino/globals"
3333
"github.com/arduino/arduino-cli/arduino/libraries"

‎arduino/builder/detector/detector_test.go‎ renamed to ‎arduino/builder/internal/detector/detector_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package detector_test
1818
import (
1919
"testing"
2020

21-
"github.com/arduino/arduino-cli/arduino/builder/detector"
21+
"github.com/arduino/arduino-cli/arduino/builder/internal/detector"
2222
"github.com/stretchr/testify/require"
2323
)
2424

‎arduino/builder/logger/logger.go‎ renamed to ‎arduino/builder/internal/logger/logger.go‎

File renamed without changes.

0 commit comments

Comments
 (0)