[breaking] Allow setting extra paths for build-cache (#2612)

* Small refactoring of compileCore function This change helps to better understand next commits. * Added possibility to set extra build-cache dirs for cores * [breaking] --build-cache-path now saves under 'cores' subdir instead of 'core' * Added integration tests * Updated docs and fixed semantics of GetBuildCacheExtraPaths method * Updated json-schema for configuration * Update rpc/cc/arduino/cli/commands/v1/compile.proto Co-authored-by: Alessio Perugini <[email protected]> * Start integration test from a clean state * Do not force build-cache path creation if not needed * Update internal/cli/configuration/build_cache.go Co-authored-by: Umberto Baldi <[email protected]> --------- Co-authored-by: Alessio Perugini <[email protected]> Co-authored-by: Umberto Baldi <[email protected]>
arduino · May 29, 2024 · 81d517b · 81d517b
1 parent dc13ef6
commit 81d517b
Show file tree

Hide file tree

Showing 12 changed files with 355 additions and 131 deletions.
diff --git a/commands/service_compile.go b/commands/service_compile.go
@@ -178,18 +178,31 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 		s.settings.GetCompilationsBeforeBuildCachePurge(),
 		s.settings.GetBuildCacheTTL().Abs())
 
-	var coreBuildCachePath *paths.Path
-	if req.GetBuildCachePath() == "" {
-		coreBuildCachePath = paths.TempDir().Join("arduino", "cores")
-	} else {
-		buildCachePath, err := paths.New(req.GetBuildCachePath()).Abs()
+	var buildCachePath *paths.Path
+	if req.GetBuildCachePath() != "" {
+		p, err := paths.New(req.GetBuildCachePath()).Abs()
 		if err != nil {
 			return &cmderrors.PermissionDeniedError{Message: i18n.Tr("Cannot create build cache directory"), Cause: err}
 		}
-		if err := buildCachePath.MkdirAll(); err != nil {
-			return &cmderrors.PermissionDeniedError{Message: i18n.Tr("Cannot create build cache directory"), Cause: err}
-		}
-		coreBuildCachePath = buildCachePath.Join("core")
+		buildCachePath = p
+	} else if p, ok := s.settings.GetBuildCachePath(); ok {
+		buildCachePath = p
+	} else {
+		buildCachePath = paths.TempDir().Join("arduino")
+	}
+	if err := buildCachePath.MkdirAll(); err != nil {
+		return &cmderrors.PermissionDeniedError{Message: i18n.Tr("Cannot create build cache directory"), Cause: err}
+	}
+	coreBuildCachePath := buildCachePath.Join("cores")
+
+	var extraCoreBuildCachePaths paths.PathList
+	if len(req.GetBuildCacheExtraPaths()) == 0 {
+		extraCoreBuildCachePaths = s.settings.GetBuildCacheExtraPaths()
+	} else {
+		extraCoreBuildCachePaths = paths.NewPathList(req.GetBuildCacheExtraPaths()...)
+	}
+	for i, p := range extraCoreBuildCachePaths {
+		extraCoreBuildCachePaths[i] = p.Join("cores")
 	}
 
 	if _, err := pme.FindToolsRequiredForBuild(targetPlatform, buildPlatform); err != nil {
@@ -229,6 +242,7 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 		buildPath,
 		req.GetOptimizeForDebug(),
 		coreBuildCachePath,
+		extraCoreBuildCachePaths,
 		int(req.GetJobs()),
 		req.GetBuildProperties(),
 		s.settings.HardwareDirectories(),

diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md
@@ -4,6 +4,12 @@ Here you can find a list of migration guides to handle breaking changes between
 
 ## 1.0.0
 
+### `compile --build-cache-path` slightly changed directory format
+
+Now compiled cores are cached under the `cores` subdir of the path specified in `--build-cache-path`, previously it was
+saved under the `core` subdir. The new behaviour is coherent with the default cache directory `/tmp/arduino/cores/...`
+when the cache directory is not set by the user.
+
 ### Configuration file now supports only YAML format.
 
 The Arduino CLI configuration file now supports only the YAML format.

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -35,6 +35,8 @@
 - `updater` - configuration options related to Arduino CLI updates
   - `enable_notification` - set to `false` to disable notifications of new Arduino CLI releases, defaults to `true`
 - `build_cache` configuration options related to the compilation cache
+  - `path` - the path to the build cache, default is `$TMP/arduino`.
+  - `extra_paths` - a list of paths to look for precompiled artifacts if not found on `build_cache.path` setting.
   - `compilations_before_purge` - interval, in number of compilations, at which the cache is purged, defaults to `10`.
     When `0` the cache is never purged.
   - `ttl` - cache expiration time of build folders. If the cache is hit by a compilation the corresponding build files

diff --git a/internal/arduino/builder/builder.go b/internal/arduino/builder/builder.go
@@ -62,7 +62,8 @@ type Builder struct {
 	customBuildProperties []string
 
 	// core related
-	coreBuildCachePath *paths.Path
+	coreBuildCachePath       *paths.Path
+	extraCoreBuildCachePaths paths.PathList
 
 	logger *logger.BuilderLogger
 	clean  bool
@@ -121,6 +122,7 @@ func NewBuilder(
 	buildPath *paths.Path,
 	optimizeForDebug bool,
 	coreBuildCachePath *paths.Path,
+	extraCoreBuildCachePaths paths.PathList,
 	jobs int,
 	requestBuildProperties []string,
 	hardwareDirs, otherLibrariesDirs paths.PathList,
@@ -211,6 +213,7 @@ func NewBuilder(
 		jobs:                          jobs,
 		customBuildProperties:         customBuildPropertiesArgs,
 		coreBuildCachePath:            coreBuildCachePath,
+		extraCoreBuildCachePaths:      extraCoreBuildCachePaths,
 		logger:                        logger,
 		clean:                         clean,
 		sourceOverrides:               sourceOverrides,

diff --git a/internal/arduino/builder/core.go b/internal/arduino/builder/core.go
@@ -89,34 +89,56 @@ func (b *Builder) compileCore() (*paths.Path, paths.PathList, error) {
 			b.buildProperties.Get("compiler.optimization_flags"),
 			realCoreFolder,
 		)
-		targetArchivedCore = b.coreBuildCachePath.Join(archivedCoreName, "core.a")
-
-		if _, err := buildcache.New(b.coreBuildCachePath).GetOrCreate(archivedCoreName); errors.Is(err, buildcache.CreateDirErr) {
-			return nil, nil, errors.New(i18n.Tr("creating core cache folder: %s", err))
-		}
 
-		var canUseArchivedCore bool
-		if b.onlyUpdateCompilationDatabase || b.clean {
-			canUseArchivedCore = false
-		} else if isOlder, err := utils.DirContentIsOlderThan(realCoreFolder, targetArchivedCore); err != nil || !isOlder {
-			// Recreate the archive if ANY of the core files (including platform.txt) has changed
-			canUseArchivedCore = false
-		} else if targetCoreFolder == nil || realCoreFolder.EquivalentTo(targetCoreFolder) {
-			canUseArchivedCore = true
-		} else if isOlder, err := utils.DirContentIsOlderThan(targetCoreFolder, targetArchivedCore); err != nil || !isOlder {
-			// Recreate the archive if ANY of the build core files (including platform.txt) has changed
-			canUseArchivedCore = false
-		} else {
-			canUseArchivedCore = true
+		canUseArchivedCore := func(archivedCore *paths.Path) bool {
+			if b.onlyUpdateCompilationDatabase || b.clean {
+				return false
+			}
+			if isOlder, err := utils.DirContentIsOlderThan(realCoreFolder, archivedCore); err != nil || !isOlder {
+				// Recreate the archive if ANY of the core files (including platform.txt) has changed
+				return false
+			}
+			if targetCoreFolder == nil || realCoreFolder.EquivalentTo(targetCoreFolder) {
+				return true
+			}
+			if isOlder, err := utils.DirContentIsOlderThan(targetCoreFolder, archivedCore); err != nil || !isOlder {
+				// Recreate the archive if ANY of the build core files (including platform.txt) has changed
+				return false
+			}
+			return true
 		}
 
-		if canUseArchivedCore {
+		// If there is an archived core in the current build cache, use it
+		targetArchivedCore = b.coreBuildCachePath.Join(archivedCoreName, "core.a")
+		if canUseArchivedCore(targetArchivedCore) {
+			// Extend the build cache expiration time
+			if _, err := buildcache.New(b.coreBuildCachePath).GetOrCreate(archivedCoreName); errors.Is(err, buildcache.CreateDirErr) {
+				return nil, nil, errors.New(i18n.Tr("creating core cache folder: %s", err))
+			}
 			// use archived core
 			if b.logger.Verbose() {
 				b.logger.Info(i18n.Tr("Using precompiled core: %[1]s", targetArchivedCore))
 			}
 			return targetArchivedCore, variantObjectFiles, nil
 		}
+
+		// Otherwise try the extra build cache paths to see if there is a precompiled core
+		// that can be used
+		for _, extraCoreBuildCachePath := range b.extraCoreBuildCachePaths {
+			extraTargetArchivedCore := extraCoreBuildCachePath.Join(archivedCoreName, "core.a")
+			if canUseArchivedCore(extraTargetArchivedCore) {
+				// use archived core
+				if b.logger.Verbose() {
+					b.logger.Info(i18n.Tr("Using precompiled core: %[1]s", extraTargetArchivedCore))
+				}
+				return extraTargetArchivedCore, variantObjectFiles, nil
+			}
+		}
+
+		// Create the build cache folder for the core
+		if _, err := buildcache.New(b.coreBuildCachePath).GetOrCreate(archivedCoreName); errors.Is(err, buildcache.CreateDirErr) {
+			return nil, nil, errors.New(i18n.Tr("creating core cache folder: %s", err))
+		}
 	}
 
 	coreObjectFiles, err := b.compileFiles(

diff --git a/internal/cli/configuration/build_cache.go b/internal/cli/configuration/build_cache.go
@@ -15,7 +15,11 @@
 
 package configuration
 
-import "time"
+import (
+	"time"
+
+	"github.com/arduino/go-paths-helper"
+)
 
 // GetCompilationsBeforeBuildCachePurge returns the number of compilations before the build cache is purged.
 func (s *Settings) GetCompilationsBeforeBuildCachePurge() uint {
@@ -32,3 +36,21 @@ func (s *Settings) GetBuildCacheTTL() time.Duration {
 	}
 	return s.Defaults.GetDuration("build_cache.ttl")
 }
+
+// GetBuildCachePath returns the path to the build cache.
+func (s *Settings) GetBuildCachePath() (*paths.Path, bool) {
+	p, ok, _ := s.GetStringOk("build_cache.path")
+	if !ok {
+		return nil, false
+	}
+	return paths.New(p), true
+}
+
+// GetBuildCacheExtraPaths returns the extra paths to the build cache.
+// Those paths are visited to look for precompiled items if not found elsewhere.
+func (s *Settings) GetBuildCacheExtraPaths() paths.PathList {
+	if p, ok, _ := s.GetStringSliceOk("build_cache.extra_paths"); ok {
+		return paths.NewPathList(p...)
+	}
+	return nil
+}
diff --git a/internal/cli/configuration/configuration.schema.json b/internal/cli/configuration/configuration.schema.json
@@ -20,6 +20,17 @@
     "build_cache": {
       "description": "configuration options related to the compilation cache",
       "properties": {
+        "path": {
+          "description": "the path to the build cache, default is `$TMP/arduino`.",
+          "type": "string"
+        },
+        "extra_paths": {
+          "description": "a list of paths to look for precompiled artifacts if not found on `build_cache.path` setting.",
+          "type": "array",
+          "items": {
+            "type": "string"
+          }
+        },
         "compilations_before_purge": {
           "description": "interval, in number of compilations, at which the cache is purged, defaults to `10`. When `0` the cache is never purged.",
           "type": "integer",

diff --git a/internal/cli/configuration/defaults.go b/internal/cli/configuration/defaults.go
@@ -52,6 +52,8 @@ func SetDefaults(settings *Settings) {
 	setDefaultValueAndKeyTypeSchema("sketch.always_export_binaries", false)
 	setDefaultValueAndKeyTypeSchema("build_cache.ttl", (time.Hour * 24 * 30).String())
 	setDefaultValueAndKeyTypeSchema("build_cache.compilations_before_purge", uint(10))
+	setKeyTypeSchema("build_cache.path", "")
+	setKeyTypeSchema("build_cache.extra_paths", []string{})
 
 	// daemon settings
 	setDefaultValueAndKeyTypeSchema("daemon.port", "50051")

diff --git a/internal/integrationtest/compile_4/core_caching_test.go b/internal/integrationtest/compile_4/core_caching_test.go
@@ -0,0 +1,124 @@
+// 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 [email protected].
+
+package compile_test
+
+import (
+	"testing"
+
+	"github.com/arduino/arduino-cli/internal/integrationtest"
+	"github.com/arduino/go-paths-helper"
+	"github.com/stretchr/testify/require"
+)
+
+func TestBuildCacheCoreWithExtraDirs(t *testing.T) {
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	t.Cleanup(env.CleanUp)
+
+	// Install Arduino AVR Boards
+	_, _, err := cli.Run("core", "install", "arduino:[email protected]")
+	require.NoError(t, err)
+
+	// Main core cache
+	defaultCache := paths.TempDir().Join("arduino")
+	cache1, err := paths.MkTempDir("", "core_cache")
+	require.NoError(t, err)
+	t.Cleanup(func() { cache1.RemoveAll() })
+	cache2, err := paths.MkTempDir("", "extra_core_cache")
+	require.NoError(t, err)
+	t.Cleanup(func() { cache2.RemoveAll() })
+
+	sketch, err := paths.New("testdata", "BareMinimum").Abs()
+	require.NoError(t, err)
+
+	{
+		// Clean cache
+		require.NoError(t, defaultCache.RemoveAll())
+
+		// Compile sketch with empty cache
+		out, _, err := cli.Run("compile", "-v", "-b", "arduino:avr:uno", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Archiving built core (caching) in: "+defaultCache.String())
+
+		// Check that the core cache is re-used
+		out, _, err = cli.Run("compile", "-v", "-b", "arduino:avr:uno", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Using precompiled core: "+defaultCache.String())
+	}
+
+	{
+		env := cli.GetDefaultEnv()
+		env["ARDUINO_BUILD_CACHE_PATH"] = cache1.String()
+
+		// Compile sketch with empty cache user-defined core cache
+		out, _, err := cli.RunWithCustomEnv(env, "compile", "-v", "-b", "arduino:avr:uno", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Archiving built core (caching) in: "+cache1.String())
+
+		// Check that the core cache is re-used with user-defined core cache
+		out, _, err = cli.RunWithCustomEnv(env, "compile", "-v", "-b", "arduino:avr:uno", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Using precompiled core: "+cache1.String())
+
+		// Clean run should rebuild and save in user-defined core cache
+		out, _, err = cli.RunWithCustomEnv(env, "compile", "-v", "-b", "arduino:avr:uno", "--clean", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Archiving built core (caching) in: "+cache1.String())
+	}
+
+	{
+		env := cli.GetDefaultEnv()
+		env["ARDUINO_BUILD_CACHE_EXTRA_PATHS"] = cache1.String()
+
+		// Both extra and default cache are full, should use the default one
+		out, _, err := cli.RunWithCustomEnv(env, "compile", "-v", "-b", "arduino:avr:uno", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Using precompiled core: "+defaultCache.String())
+
+		// Clean run, should rebuild and save in default cache
+		out, _, err = cli.RunWithCustomEnv(env, "compile", "-v", "-b", "arduino:avr:uno", "--clean", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Archiving built core (caching) in: "+defaultCache.String())
+
+		// Clean default cache
+		require.NoError(t, defaultCache.RemoveAll())
+
+		// Now, extra is full and default is empty, should use extra
+		out, _, err = cli.RunWithCustomEnv(env, "compile", "-v", "-b", "arduino:avr:uno", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Using precompiled core: "+cache1.String())
+	}
+
+	{
+		env := cli.GetDefaultEnv()
+		env["ARDUINO_BUILD_CACHE_EXTRA_PATHS"] = cache1.String() // Populated
+		env["ARDUINO_BUILD_CACHE_PATH"] = cache2.String()        // Empty
+
+		// Extra cache is full, should use the cache1 (extra)
+		out, _, err := cli.RunWithCustomEnv(env, "compile", "-v", "-b", "arduino:avr:uno", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Using precompiled core: "+cache1.String())
+
+		// Clean run, should rebuild and save in cache2 (user defined default cache)
+		out, _, err = cli.RunWithCustomEnv(env, "compile", "-v", "-b", "arduino:avr:uno", "--clean", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Archiving built core (caching) in: "+cache2.String())
+
+		// Both caches are full, should use the cache2 (user defined default)
+		out, _, err = cli.RunWithCustomEnv(env, "compile", "-v", "-b", "arduino:avr:uno", sketch.String())
+		require.NoError(t, err)
+		require.Contains(t, string(out), "Using precompiled core: "+cache2.String())
+	}
+}
diff --git a/internal/integrationtest/compile_4/testdata/BareMinimum/BareMinimum.ino b/internal/integrationtest/compile_4/testdata/BareMinimum/BareMinimum.ino
@@ -0,0 +1,2 @@
+void setup() {}
+void loop() {}