introduce BuilderArtifacts to better isolate write operations

alessio-perugini · alessio-perugini · commit 4cb3c903fcc7 · 2023-09-12T20:50:17.000+02:00
diff --git a/arduino/builder/builder.go b/arduino/builder/builder.go
@@ -72,9 +72,24 @@ type Builder struct {
 	// C++ Parsing
 	lineOffset int
 
+	buildArtifacts *BuildArtifacts
+
 	*BuildOptionsManager
 }
 
+// BuildArtifacts contains the result of various build
+type BuildArtifacts struct {
+	// populated by BuildCore
+	coreArchiveFilePath *paths.Path
+	coreObjectsFiles    paths.PathList
+
+	// populated by BuildLibraries
+	librariesObjectFiles paths.PathList
+
+	// populated by BuildSketch
+	sketchObjectFiles paths.PathList
+}
+
 // NewBuilder creates a sketch Builder.
 func NewBuilder(
 	sk *sketch.Sketch,
@@ -160,6 +175,8 @@ func NewBuilder(
 		onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase,
 		compilationDatabase:           compilation.NewDatabase(buildPath.Join("compile_commands.json")),
 		Progress:                      progressStats,
+		executableSectionsSize:        []ExecutableSectionSize{},
+		buildArtifacts:                &BuildArtifacts{},
 		BuildOptionsManager: NewBuildOptionsManager(
 			hardwareDirs, builtInToolsDirs, otherLibrariesDirs,
 			builtInLibrariesDirs, buildPath,
diff --git a/arduino/builder/core.go b/arduino/builder/core.go
@@ -32,9 +32,9 @@ import (
 )
 
 // BuildCore fixdoc
-func (b *Builder) BuildCore(actualPlatform *cores.PlatformRelease) (paths.PathList, *paths.Path, error) {
+func (b *Builder) BuildCore(actualPlatform *cores.PlatformRelease) error {
 	if err := b.coreBuildPath.MkdirAll(); err != nil {
-		return nil, nil, errors.WithStack(err)
+		return errors.WithStack(err)
 	}
 
 	if b.coreBuildCachePath != nil {
@@ -45,16 +45,17 @@ func (b *Builder) BuildCore(actualPlatform *cores.PlatformRelease) (paths.PathLi
 			// compileCore function).
 			b.coreBuildCachePath = nil
 		} else if err := b.coreBuildCachePath.MkdirAll(); err != nil {
-			return nil, nil, errors.WithStack(err)
+			return errors.WithStack(err)
 		}
 	}
 
 	archiveFile, objectFiles, err := b.compileCore(actualPlatform)
 	if err != nil {
-		return nil, nil, errors.WithStack(err)
+		return errors.WithStack(err)
 	}
-
-	return objectFiles, archiveFile, nil
+	b.buildArtifacts.coreObjectsFiles = objectFiles
+	b.buildArtifacts.coreArchiveFilePath = archiveFile
+	return nil
 }
 
 func (b *Builder) compileCore(actualPlatform *cores.PlatformRelease) (*paths.Path, paths.PathList, error) {
diff --git a/arduino/builder/libraries.go b/arduino/builder/libraries.go
@@ -37,20 +37,20 @@ var (
 )
 
 // BuildLibraries fixdoc
-func (b *Builder) BuildLibraries(includesFolders paths.PathList, importedLibraries libraries.List) (paths.PathList, error) {
+func (b *Builder) BuildLibraries(includesFolders paths.PathList, importedLibraries libraries.List) error {
 	includes := f.Map(includesFolders.AsStrings(), cpp.WrapWithHyphenI)
 	libs := importedLibraries
 
 	if err := b.librariesBuildPath.MkdirAll(); err != nil {
-		return nil, errors.WithStack(err)
+		return errors.WithStack(err)
 	}
 
 	librariesObjectFiles, err := b.compileLibraries(libs, includes)
 	if err != nil {
-		return nil, errors.WithStack(err)
+		return errors.WithStack(err)
 	}
-
-	return librariesObjectFiles, nil
+	b.buildArtifacts.librariesObjectFiles = librariesObjectFiles
+	return nil
 }
 
 func directoryContainsFile(folder *paths.Path) bool {
diff --git a/arduino/builder/linker.go b/arduino/builder/linker.go
@@ -25,7 +25,7 @@ import (
 )
 
 // Link fixdoc
-func (b *Builder) Link(sketchObjectFiles, librariesObjectFiles, coreObjectsFiles paths.PathList, coreArchiveFilePath *paths.Path) error {
+func (b *Builder) Link() error {
 	if b.onlyUpdateCompilationDatabase {
 		if b.logger.Verbose() {
 			b.logger.Info(tr("Skip linking of final executable."))
@@ -34,21 +34,21 @@ func (b *Builder) Link(sketchObjectFiles, librariesObjectFiles, coreObjectsFiles
 	}
 
 	// TODO can we remove this multiple assignations?
-	objectFilesSketch := sketchObjectFiles
-	objectFilesLibraries := librariesObjectFiles
-	objectFilesCore := coreObjectsFiles
+	objectFilesSketch := b.buildArtifacts.sketchObjectFiles
+	objectFilesLibraries := b.buildArtifacts.librariesObjectFiles
+	objectFilesCore := b.buildArtifacts.coreObjectsFiles
 
 	objectFiles := paths.NewPathList()
 	objectFiles.AddAll(objectFilesSketch)
 	objectFiles.AddAll(objectFilesLibraries)
 	objectFiles.AddAll(objectFilesCore)
 
-	coreDotARelPath, err := b.buildPath.RelTo(coreArchiveFilePath)
+	coreDotARelPath, err := b.buildPath.RelTo(b.buildArtifacts.coreArchiveFilePath)
 	if err != nil {
 		return errors.WithStack(err)
 	}
 
-	if err := b.link(objectFiles, coreDotARelPath, coreArchiveFilePath); err != nil {
+	if err := b.link(objectFiles, coreDotARelPath, b.buildArtifacts.coreArchiveFilePath); err != nil {
 		return errors.WithStack(err)
 	}
 
diff --git a/arduino/builder/sketch.go b/arduino/builder/sketch.go
@@ -181,11 +181,11 @@ func writeIfDifferent(source []byte, destPath *paths.Path) error {
 }
 
 // BuildSketch fixdoc
-func (b *Builder) BuildSketch(includesFolders paths.PathList) (paths.PathList, error) {
+func (b *Builder) BuildSketch(includesFolders paths.PathList) error {
 	includes := f.Map(includesFolders.AsStrings(), cpp.WrapWithHyphenI)
 
 	if err := b.sketchBuildPath.MkdirAll(); err != nil {
-		return nil, errors.WithStack(err)
+		return errors.WithStack(err)
 	}
 
 	sketchObjectFiles, err := utils.CompileFiles(
@@ -197,7 +197,7 @@ func (b *Builder) BuildSketch(includesFolders paths.PathList) (paths.PathList, e
 		b.Progress,
 	)
 	if err != nil {
-		return nil, errors.WithStack(err)
+		return errors.WithStack(err)
 	}
 
 	// The "src/" subdirectory of a sketch is compiled recursively
@@ -212,12 +212,13 @@ func (b *Builder) BuildSketch(includesFolders paths.PathList) (paths.PathList, e
 			b.Progress,
 		)
 		if err != nil {
-			return nil, errors.WithStack(err)
+			return errors.WithStack(err)
 		}
 		sketchObjectFiles.AddAll(srcObjectFiles)
 	}
 
-	return sketchObjectFiles, nil
+	b.buildArtifacts.sketchObjectFiles = sketchObjectFiles
+	return nil
 }
 
 // MergeSketchWithBootloader fixdoc
diff --git a/legacy/builder/builder.go b/legacy/builder/builder.go
@@ -63,12 +63,7 @@ func (s *Builder) Run(ctx *types.Context) error {
 		}),
 
 		types.BareCommand(func(ctx *types.Context) error {
-			sketchObjectFiles, err := ctx.Builder.BuildSketch(ctx.SketchLibrariesDetector.IncludeFolders())
-			if err != nil {
-				return err
-			}
-			ctx.SketchObjectFiles = sketchObjectFiles
-			return nil
+			return ctx.Builder.BuildSketch(ctx.SketchLibrariesDetector.IncludeFolders())
 		}),
 
 		types.BareCommand(func(ctx *types.Context) error {
@@ -87,16 +82,7 @@ func (s *Builder) Run(ctx *types.Context) error {
 		}),
 
 		types.BareCommand(func(ctx *types.Context) error {
-			librariesObjectFiles, err := ctx.Builder.BuildLibraries(
-				ctx.SketchLibrariesDetector.IncludeFolders(),
-				ctx.SketchLibrariesDetector.ImportedLibraries(),
-			)
-			if err != nil {
-				return err
-			}
-			ctx.LibrariesObjectFiles = librariesObjectFiles
-
-			return nil
+			return ctx.Builder.BuildLibraries(ctx.SketchLibrariesDetector.IncludeFolders(), ctx.SketchLibrariesDetector.ImportedLibraries())
 		}),
 		types.BareCommand(func(ctx *types.Context) error {
 			return recipeByPrefixSuffixRunner(ctx, "recipe.hooks.libraries.postbuild", ".pattern", true)
@@ -108,12 +94,7 @@ func (s *Builder) Run(ctx *types.Context) error {
 		}),
 
 		types.BareCommand(func(ctx *types.Context) error {
-			objectFiles, archiveFile, err := ctx.Builder.BuildCore(ctx.ActualPlatform)
-
-			ctx.CoreObjectsFiles = objectFiles
-			ctx.CoreArchiveFilePath = archiveFile
-
-			return err
+			return ctx.Builder.BuildCore(ctx.ActualPlatform)
 		}),
 
 		types.BareCommand(func(ctx *types.Context) error {
@@ -126,12 +107,7 @@ func (s *Builder) Run(ctx *types.Context) error {
 		}),
 
 		types.BareCommand(func(ctx *types.Context) error {
-			return ctx.Builder.Link(
-				ctx.SketchObjectFiles,
-				ctx.LibrariesObjectFiles,
-				ctx.CoreObjectsFiles,
-				ctx.CoreArchiveFilePath,
-			)
+			return ctx.Builder.Link()
 		}),
 
 		types.BareCommand(func(ctx *types.Context) error {
diff --git a/legacy/builder/types/context.go b/legacy/builder/types/context.go
@@ -39,9 +39,4 @@ type Context struct {
 	PackageManager *packagemanager.Explorer
 	TargetPlatform *cores.PlatformRelease
 	ActualPlatform *cores.PlatformRelease
-
-	CoreArchiveFilePath  *paths.Path
-	CoreObjectsFiles     paths.PathList
-	LibrariesObjectFiles paths.PathList
-	SketchObjectFiles    paths.PathList
 }

Original file line number	Diff line number	Diff line change
`@@ -32,9 +32,9 @@ import (`
`32`	`32`	`)`
`33`	`33`
`34`	`34`	`// BuildCore fixdoc`
`35`		`-func (b Builder) BuildCore(actualPlatform cores.PlatformRelease) (paths.PathList, *paths.Path, error) {`
	`35`	`+func (b Builder) BuildCore(actualPlatform cores.PlatformRelease) error {`
`36`	`36`	`if err := b.coreBuildPath.MkdirAll(); err != nil {`
`37`		`- return nil, nil, errors.WithStack(err)`
	`37`	`+ return errors.WithStack(err)`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`if b.coreBuildCachePath != nil {`
`@@ -45,16 +45,17 @@ func (b Builder) BuildCore(actualPlatform cores.PlatformRelease) (paths.PathLi`
`45`	`45`	`// compileCore function).`
`46`	`46`	`b.coreBuildCachePath = nil`
`47`	`47`	`} else if err := b.coreBuildCachePath.MkdirAll(); err != nil {`
`48`		`- return nil, nil, errors.WithStack(err)`
	`48`	`+ return errors.WithStack(err)`
`49`	`49`	`}`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`archiveFile, objectFiles, err := b.compileCore(actualPlatform)`
`53`	`53`	`if err != nil {`
`54`		`- return nil, nil, errors.WithStack(err)`
	`54`	`+ return errors.WithStack(err)`
`55`	`55`	`}`
`56`		`-`
`57`		`- return objectFiles, archiveFile, nil`
	`56`	`+ b.buildArtifacts.coreObjectsFiles = objectFiles`
	`57`	`+ b.buildArtifacts.coreArchiveFilePath = archiveFile`
	`58`	`+ return nil`
`58`	`59`	`}`
`59`	`60`
`60`	`61`	`func (b Builder) compileCore(actualPlatform cores.PlatformRelease) (*paths.Path, paths.PathList, error) {`
Original file line number	Diff line number	Diff line change
`@@ -37,20 +37,20 @@ var (`
`37`	`37`	`)`
`38`	`38`
`39`	`39`	`// BuildLibraries fixdoc`
`40`		`-func (b *Builder) BuildLibraries(includesFolders paths.PathList, importedLibraries libraries.List) (paths.PathList, error) {`
	`40`	`+func (b *Builder) BuildLibraries(includesFolders paths.PathList, importedLibraries libraries.List) error {`
`41`	`41`	`includes := f.Map(includesFolders.AsStrings(), cpp.WrapWithHyphenI)`
`42`	`42`	`libs := importedLibraries`
`43`	`43`
`44`	`44`	`if err := b.librariesBuildPath.MkdirAll(); err != nil {`
`45`		`- return nil, errors.WithStack(err)`
	`45`	`+ return errors.WithStack(err)`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`librariesObjectFiles, err := b.compileLibraries(libs, includes)`
`49`	`49`	`if err != nil {`
`50`		`- return nil, errors.WithStack(err)`
	`50`	`+ return errors.WithStack(err)`
`51`	`51`	`}`
`52`		`-`
`53`		`- return librariesObjectFiles, nil`
	`52`	`+ b.buildArtifacts.librariesObjectFiles = librariesObjectFiles`
	`53`	`+ return nil`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`func directoryContainsFile(folder *paths.Path) bool {`
Original file line number	Diff line number	Diff line change
`@@ -181,11 +181,11 @@ func writeIfDifferent(source []byte, destPath *paths.Path) error {`
`181`	`181`	`}`
`182`	`182`
`183`	`183`	`// BuildSketch fixdoc`
`184`		`-func (b *Builder) BuildSketch(includesFolders paths.PathList) (paths.PathList, error) {`
	`184`	`+func (b *Builder) BuildSketch(includesFolders paths.PathList) error {`
`185`	`185`	`includes := f.Map(includesFolders.AsStrings(), cpp.WrapWithHyphenI)`
`186`	`186`
`187`	`187`	`if err := b.sketchBuildPath.MkdirAll(); err != nil {`
`188`		`- return nil, errors.WithStack(err)`
	`188`	`+ return errors.WithStack(err)`
`189`	`189`	`}`
`190`	`190`
`191`	`191`	`sketchObjectFiles, err := utils.CompileFiles(`
`@@ -197,7 +197,7 @@ func (b *Builder) BuildSketch(includesFolders paths.PathList) (paths.PathList, e`
`197`	`197`	`b.Progress,`
`198`	`198`	`)`
`199`	`199`	`if err != nil {`
`200`		`- return nil, errors.WithStack(err)`
	`200`	`+ return errors.WithStack(err)`
`201`	`201`	`}`
`202`	`202`
`203`	`203`	`// The "src/" subdirectory of a sketch is compiled recursively`
`@@ -212,12 +212,13 @@ func (b *Builder) BuildSketch(includesFolders paths.PathList) (paths.PathList, e`
`212`	`212`	`b.Progress,`
`213`	`213`	`)`
`214`	`214`	`if err != nil {`
`215`		`- return nil, errors.WithStack(err)`
	`215`	`+ return errors.WithStack(err)`
`216`	`216`	`}`
`217`	`217`	`sketchObjectFiles.AddAll(srcObjectFiles)`
`218`	`218`	`}`
`219`	`219`
`220`		`- return sketchObjectFiles, nil`
	`220`	`+ b.buildArtifacts.sketchObjectFiles = sketchObjectFiles`
	`221`	`+ return nil`
`221`	`222`	`}`
`222`	`223`
`223`	`224`	`// MergeSketchWithBootloader fixdoc`