Reuse (cache) object archives in large sketch projects (#2464)

facchinm · cmaglie · web-flow · commit 72dd2499c796 · 2023-12-20T11:09:32.000+01:00
* Reuse archiveCompiledFiles helper for long commandline shrink

Since archiveCompiledFiles already handles hot cache correctly, this avoids objs.a being rebuilt even if files don't change.

Would be ideal if PathList could expose a generic Filter API (to get rid of the "duplicated" filter)

* Upgrade go-paths / remove duplicate filter function

* Consider existing archives during the build

* Simplified archiveCompiledFiles function signature

It doesn't make sense anymore to keep path and filename separated.

* Added integration test

---------

Co-authored-by: Cristian Maglie &lt;c.maglie@arduino.cc&gt;
diff --git a/.licenses/go/github.com/arduino/go-paths-helper.dep.yml b/.licenses/go/github.com/arduino/go-paths-helper.dep.yml
@@ -1,6 +1,6 @@
 ---
 name: github.com/arduino/go-paths-helper
-version: v1.10.1
+version: v1.11.0
 type: go
 summary: 
 homepage: https://pkg.go.dev/github.com/arduino/go-paths-helper
diff --git a/go.mod b/go.mod
@@ -7,7 +7,7 @@ replace github.com/mailru/easyjson => github.com/cmaglie/easyjson v0.8.1
 
 require (
 	github.com/ProtonMail/go-crypto v0.0.0-20230828082145-3c4c8a2d2371
-	github.com/arduino/go-paths-helper v1.10.1
+	github.com/arduino/go-paths-helper v1.11.0
 	github.com/arduino/go-properties-orderedmap v1.8.0
 	github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b
 	github.com/arduino/go-win32-utils v1.0.0
diff --git a/go.sum b/go.sum
@@ -11,8 +11,8 @@ github.com/acomagu/bufpipe v1.0.3/go.mod h1:mxdxdup/WdsKVreO5GpW4+M/1CE2sMG4jeGJ
 github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA=
 github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c=
 github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck=
-github.com/arduino/go-paths-helper v1.10.1 h1:j8InnhLrSeoPiOvTnZL0XMFt7l407ciTBJJJs7W9bs4=
-github.com/arduino/go-paths-helper v1.10.1/go.mod h1:jcpW4wr0u69GlXhTYydsdsqAjLaYK5n7oWHfKqOG6LM=
+github.com/arduino/go-paths-helper v1.11.0 h1:hkpGb9AtCTByTj2FKutuHWb3klDf4kAKL10hW+fN+oE=
+github.com/arduino/go-paths-helper v1.11.0/go.mod h1:jcpW4wr0u69GlXhTYydsdsqAjLaYK5n7oWHfKqOG6LM=
 github.com/arduino/go-properties-orderedmap v1.8.0 h1:wEfa6hHdpezrVOh787OmClsf/Kd8qB+zE3P2Xbrn0CQ=
 github.com/arduino/go-properties-orderedmap v1.8.0/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk=
 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b h1:9hDi4F2st6dbLC3y4i02zFT5quS4X6iioWifGlVwfy4=
diff --git a/internal/arduino/builder/archive_compiled_files.go b/internal/arduino/builder/archive_compiled_files.go
@@ -20,9 +20,7 @@ import (
 )
 
 // ArchiveCompiledFiles fixdoc
-func (b *Builder) archiveCompiledFiles(buildPath *paths.Path, archiveFile *paths.Path, objectFilesToArchive paths.PathList) (*paths.Path, error) {
-	archiveFilePath := buildPath.JoinPath(archiveFile)
-
+func (b *Builder) archiveCompiledFiles(archiveFilePath *paths.Path, objectFilesToArchive paths.PathList) (*paths.Path, error) {
 	if b.onlyUpdateCompilationDatabase {
 		if b.logger.Verbose() {
 			b.logger.Info(tr("Skipping archive creation of: %[1]s", archiveFilePath))
diff --git a/internal/arduino/builder/core.go b/internal/arduino/builder/core.go
@@ -128,7 +128,7 @@ func (b *Builder) compileCore() (*paths.Path, paths.PathList, error) {
 		return nil, nil, err
 	}
 
-	archiveFile, err := b.archiveCompiledFiles(b.coreBuildPath, paths.New("core.a"), coreObjectFiles)
+	archiveFile, err := b.archiveCompiledFiles(b.coreBuildPath.Join("core.a"), coreObjectFiles)
 	if err != nil {
 		return nil, nil, err
 	}
diff --git a/internal/arduino/builder/libraries.go b/internal/arduino/builder/libraries.go
@@ -196,7 +196,7 @@ func (b *Builder) compileLibrary(library *libraries.Library, includes []string)
 			return nil, err
 		}
 		if library.DotALinkage {
-			archiveFile, err := b.archiveCompiledFiles(libraryBuildPath, paths.New(library.DirName+".a"), libObjectFiles)
+			archiveFile, err := b.archiveCompiledFiles(libraryBuildPath.Join(library.DirName+".a"), libObjectFiles)
 			if err != nil {
 				return nil, err
 			}
diff --git a/internal/arduino/builder/linker.go b/internal/arduino/builder/linker.go
@@ -53,34 +53,31 @@ func (b *Builder) link() error {
 		// it may happen that a subdir/spi.o inside the archive may be overwritten by a anotherdir/spi.o
 		// because thery are both named spi.o.
 
-		properties := b.buildProperties.Clone()
-		archives := paths.NewPathList()
+		// Put all the existing archives apart from the other object files
+		existingArchives := objectFiles.Clone()
+		existingArchives.FilterSuffix(".a")
+		objectFiles.FilterOutSuffix(".a")
+
+		// Generate an archive for each directory from the remaining object files
+		newArchives := paths.NewPathList()
 		for _, object := range objectFiles {
-			if object.HasSuffix(".a") {
-				archives.Add(object)
-				continue
-			}
 			archive := object.Parent().Join("objs.a")
-			if !archives.Contains(archive) {
-				archives.Add(archive)
-				// Cleanup old archives
-				_ = archive.Remove()
-			}
-			properties.Set("archive_file", archive.Base())
-			properties.SetPath("archive_file_path", archive)
-			properties.SetPath("object_file", object)
-
-			command, err := b.prepareCommandForRecipe(properties, "recipe.ar.pattern", false)
-			if err != nil {
-				return err
-			}
-
-			if err := b.execCommand(command); err != nil {
-				return err
-			}
+			newArchives.AddIfMissing(archive)
+		}
+		for _, archive := range newArchives {
+			archiveDir := archive.Parent()
+			relatedObjectFiles := objectFiles.Clone()
+			relatedObjectFiles.Filter(func(object *paths.Path) bool {
+				// extract all the object files that are in the same directory of the archive
+				return object.Parent().EquivalentTo(archiveDir)
+			})
+			b.archiveCompiledFiles(archive, relatedObjectFiles)
 		}
 
-		objectFileList = strings.Join(f.Map(archives.AsStrings(), wrapWithDoubleQuotes), " ")
+		// Put everything together
+		allArchives := existingArchives.Clone()
+		allArchives.AddAll(newArchives)
+		objectFileList = strings.Join(f.Map(allArchives.AsStrings(), wrapWithDoubleQuotes), " ")
 		objectFileList = "-Wl,--whole-archive " + objectFileList + " -Wl,--no-whole-archive"
 	}
 
diff --git a/internal/integrationtest/compile_1/compile_test.go b/internal/integrationtest/compile_1/compile_test.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"regexp"
 	"sort"
 	"strings"
 	"testing"
@@ -830,8 +831,17 @@ func TestCompileWithArchivesAndLongPaths(t *testing.T) {
 	sketchPath := paths.New(libOutput)
 	sketchPath = sketchPath.Join("examples", "ArduinoIoTCloud-Advanced")
 
-	_, _, err = cli.Run("compile", "-b", "esp8266:esp8266:huzzah", sketchPath.String(), "--config-file", "arduino-cli.yaml")
-	require.NoError(t, err)
+	t.Run("Compile", func(t *testing.T) {
+		_, _, err = cli.Run("compile", "-b", "esp8266:esp8266:huzzah", sketchPath.String(), "--config-file", "arduino-cli.yaml")
+		require.NoError(t, err)
+	})
+
+	t.Run("CheckCachingOfFolderArchives", func(t *testing.T) {
+		// Run compile again and check if the archive is re-used (cached)
+		out, _, err := cli.Run("compile", "-b", "esp8266:esp8266:huzzah", sketchPath.String(), "--config-file", "arduino-cli.yaml", "-v")
+		require.NoError(t, err)
+		require.True(t, regexp.MustCompile(`(?m)^Using previously compiled file:.*libraries.ArduinoIoTCloud.objs\.a$`).Match(out))
+	})
 }
 
 func TestCompileWithPrecompileLibrary(t *testing.T) {

Original file line number	Diff line number	Diff line change
`@@ -128,7 +128,7 @@ func (b Builder) compileCore() (paths.Path, paths.PathList, error) {`
`128`	`128`	`return nil, nil, err`
`129`	`129`	`}`
`130`	`130`
`131`		`- archiveFile, err := b.archiveCompiledFiles(b.coreBuildPath, paths.New("core.a"), coreObjectFiles)`
	`131`	`+ archiveFile, err := b.archiveCompiledFiles(b.coreBuildPath.Join("core.a"), coreObjectFiles)`
`132`	`132`	`if err != nil {`
`133`	`133`	`return nil, nil, err`
`134`	`134`	`}`
Original file line number	Diff line number	Diff line change
`@@ -196,7 +196,7 @@ func (b Builder) compileLibrary(library libraries.Library, includes []string)`
`196`	`196`	`return nil, err`
`197`	`197`	`}`
`198`	`198`	`if library.DotALinkage {`
`199`		`- archiveFile, err := b.archiveCompiledFiles(libraryBuildPath, paths.New(library.DirName+".a"), libObjectFiles)`
	`199`	`+ archiveFile, err := b.archiveCompiledFiles(libraryBuildPath.Join(library.DirName+".a"), libObjectFiles)`
`200`	`200`	`if err != nil {`
`201`	`201`	`return nil, err`
`202`	`202`	`}`