The build_cache.path setting now affect also the sketches cache

Author: cmaglie

commands/service_compile.go

@@ -160,9 +160,8 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 		return errors.New(i18n.Tr("Firmware encryption/signing requires all the following properties to be defined: %s", "build.keys.keychain, build.keys.sign_key, build.keys.encrypt_key"))
 	}
 
-	// Generate or retrieve build path
+	// Retrieve build path from user arguments
 	var buildPath *paths.Path
-	isDefaultBuildPath := false
 	if buildPathArg := req.GetBuildPath(); buildPathArg != "" {
 		buildPath = paths.New(req.GetBuildPath()).Canonical()
 		if in, _ := buildPath.IsInsideDir(sk.FullPath); in && buildPath.IsDir() {
@@ -171,34 +170,22 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 			}
 		}
 	}
-	if buildPath == nil {
-		buildPath = sk.DefaultBuildPath()
-		isDefaultBuildPath = true
-	}
-	if err = buildPath.MkdirAll(); err != nil {
-		return &cmderrors.PermissionDeniedError{Message: i18n.Tr("Cannot create build directory"), Cause: err}
-	}
 
+	// If no build path has been set by the user:
+	// - set up the build cache directory
+	// - set the sketch build path inside the build cache directory.
 	var coreBuildCachePath *paths.Path
 	var extraCoreBuildCachePaths paths.PathList
-	if isDefaultBuildPath {
-		buildcache.New(buildPath.Parent()).GetOrCreate(buildPath.Base())
-		// cache is purged after compilation to not remove entries that might be required
-
-		defer maybePurgeBuildCache(
-			s.settings.GetCompilationsBeforeBuildCachePurge(),
-			s.settings.GetBuildCacheTTL().Abs())
-
+	if buildPath == nil {
 		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}
-			}
-			buildCachePath = p
+		if p := req.GetBuildCachePath(); p != "" {
+			buildCachePath = paths.New(p)
 		} else {
 			buildCachePath = s.settings.GetBuildCachePath()
 		}
+		if err := buildCachePath.ToAbs(); 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}
 		}
@@ -212,6 +199,17 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 		for i, p := range extraCoreBuildCachePaths {
 			extraCoreBuildCachePaths[i] = p.Join("cores")
 		}
+
+		buildPath = s.getDefaultSketchBuildPath(sk, buildCachePath)
+		buildcache.New(buildPath.Parent()).GetOrCreate(buildPath.Base())
+
+		// cache is purged after compilation to not remove entries that might be required
+		defer maybePurgeBuildCache(
+			s.settings.GetCompilationsBeforeBuildCachePurge(),
+			s.settings.GetBuildCacheTTL().Abs())
+	}
+	if err = buildPath.MkdirAll(); err != nil {
+		return &cmderrors.PermissionDeniedError{Message: i18n.Tr("Cannot create build directory"), Cause: err}
 	}
 
 	if _, err := pme.FindToolsRequiredForBuild(targetPlatform, buildPlatform); err != nil {
@@ -420,6 +418,16 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 	return nil
 }
 
+// getDefaultSketchBuildPath generates the default build directory for a given sketch.
+// The sketch build path is inside the build cache path and is unique for each sketch.
+// If overriddenBuildCachePath is nil the build cache path is taken from the settings.
+func (s *arduinoCoreServerImpl) getDefaultSketchBuildPath(sk *sketch.Sketch, overriddenBuildCachePath *paths.Path) *paths.Path {
+	if overriddenBuildCachePath == nil {
+		overriddenBuildCachePath = s.settings.GetBuildCachePath()
+	}
+	return overriddenBuildCachePath.Join("sketches", sk.Hash())
+}
+
 // maybePurgeBuildCache runs the build files cache purge if the policy conditions are met.
 func maybePurgeBuildCache(compilationsBeforePurge uint, cacheTTL time.Duration) {
 	// 0 means never purge

commands/service_compile_test.go

@@ -0,0 +1,32 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2024 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 commands
+
+import (
+	"testing"
+
+	"github.com/arduino/arduino-cli/internal/arduino/sketch"
+	paths "github.com/arduino/go-paths-helper"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGenBuildPath(t *testing.T) {
+	want := paths.TempDir().Join("arduino", "sketches", "ACBD18DB4CC2F85CEDEF654FCCC4A4D8")
+	srv := NewArduinoCoreServer().(*arduinoCoreServerImpl)
+	act := srv.getDefaultSketchBuildPath(&sketch.Sketch{FullPath: paths.New("foo")}, nil)
+	assert.True(t, act.EquivalentTo(want))
+	assert.Equal(t, "ACBD18DB4CC2F85CEDEF654FCCC4A4D8", (&sketch.Sketch{FullPath: paths.New("foo")}).Hash())
+}

commands/service_debug.go

@@ -214,7 +214,7 @@ func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer)
 	defer release()
 
 	// Exec debugger
-	commandLine, err := getCommandLine(debugConfReq, pme)
+	commandLine, err := s.getDebugCommandLine(debugConfReq, pme)
 	if err != nil {
 		return err
 	}
@@ -254,9 +254,9 @@ func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer)
 	return sendResult(&rpc.DebugResponse_Result{})
 }
 
-// getCommandLine compose a debug command represented by a core recipe
-func getCommandLine(req *rpc.GetDebugConfigRequest, pme *packagemanager.Explorer) ([]string, error) {
-	debugInfo, err := getDebugProperties(req, pme, false)
+// getDebugCommandLine compose a debug command represented by a core recipe
+func (s *arduinoCoreServerImpl) getDebugCommandLine(req *rpc.GetDebugConfigRequest, pme *packagemanager.Explorer) ([]string, error) {
+	debugInfo, err := s.getDebugProperties(req, pme, false)
 	if err != nil {
 		return nil, err
 	}

commands/service_debug_config.go

@@ -44,7 +44,7 @@ func (s *arduinoCoreServerImpl) GetDebugConfig(ctx context.Context, req *rpc.Get
 		return nil, err
 	}
 	defer release()
-	return getDebugProperties(req, pme, false)
+	return s.getDebugProperties(req, pme, false)
 }
 
 // IsDebugSupported checks if the given board/programmer configuration supports debugging.
@@ -63,7 +63,7 @@ func (s *arduinoCoreServerImpl) IsDebugSupported(ctx context.Context, req *rpc.I
 		ImportDir:   "",
 		Programmer:  req.GetProgrammer(),
 	}
-	expectedOutput, err := getDebugProperties(configRequest, pme, true)
+	expectedOutput, err := s.getDebugProperties(configRequest, pme, true)
 	var x *cmderrors.FailedDebugError
 	if errors.As(err, &x) {
 		return &rpc.IsDebugSupportedResponse{DebuggingSupported: false}, nil
@@ -79,7 +79,7 @@ func (s *arduinoCoreServerImpl) IsDebugSupported(ctx context.Context, req *rpc.I
 		checkFQBN := minimumFQBN.Clone()
 		checkFQBN.Configs.Remove(config)
 		configRequest.Fqbn = checkFQBN.String()
-		checkOutput, err := getDebugProperties(configRequest, pme, true)
+		checkOutput, err := s.getDebugProperties(configRequest, pme, true)
 		if err == nil && reflect.DeepEqual(expectedOutput, checkOutput) {
 			minimumFQBN.Configs.Remove(config)
 		}
@@ -90,7 +90,7 @@ func (s *arduinoCoreServerImpl) IsDebugSupported(ctx context.Context, req *rpc.I
 	}, nil
 }
 
-func getDebugProperties(req *rpc.GetDebugConfigRequest, pme *packagemanager.Explorer, skipSketchChecks bool) (*rpc.GetDebugConfigResponse, error) {
+func (s *arduinoCoreServerImpl) getDebugProperties(req *rpc.GetDebugConfigRequest, pme *packagemanager.Explorer, skipSketchChecks bool) (*rpc.GetDebugConfigResponse, error) {
 	var (
 		sketchName             string
 		sketchDefaultFQBN      string
@@ -109,7 +109,7 @@ func getDebugProperties(req *rpc.GetDebugConfigRequest, pme *packagemanager.Expl
 		}
 		sketchName = sk.Name
 		sketchDefaultFQBN = sk.GetDefaultFQBN()
-		sketchDefaultBuildPath = sk.DefaultBuildPath()
+		sketchDefaultBuildPath = s.getDefaultSketchBuildPath(sk, nil)
 	} else {
 		// Use placeholder sketch data
 		sketchName = "Sketch"

commands/service_debug_test.go

@@ -66,15 +66,16 @@ func TestGetCommandLine(t *testing.T) {
 	pme, release := pm.NewExplorer()
 	defer release()
 
+	srv := NewArduinoCoreServer().(*arduinoCoreServerImpl)
 	{
 		// Check programmer not found
 		req.Programmer = "not-existent"
-		_, err := getCommandLine(req, pme)
+		_, err := srv.getDebugCommandLine(req, pme)
 		require.Error(t, err)
 	}
 
 	req.Programmer = "edbg"
-	command, err := getCommandLine(req, pme)
+	command, err := srv.getDebugCommandLine(req, pme)
 	require.Nil(t, err)
 	commandToTest := strings.Join(command, " ")
 	require.Equal(t, filepath.FromSlash(goldCommand), filepath.FromSlash(commandToTest))
@@ -97,7 +98,7 @@ func TestGetCommandLine(t *testing.T) {
 		fmt.Sprintf(" --file \"%s/arduino-test/samd/variants/mkr1000/openocd_scripts/arduino_zero.cfg\"", customHardware) +
 		fmt.Sprintf(" -c \"gdb_port pipe\" -c \"telnet_port 0\" %s/build/arduino-test.samd.mkr1000/hello.ino.elf", sketchPath)
 
-	command2, err := getCommandLine(req2, pme)
+	command2, err := srv.getDebugCommandLine(req2, pme)
 	assert.Nil(t, err)
 	commandToTest2 := strings.Join(command2, " ")
 	assert.Equal(t, filepath.FromSlash(goldCommand2), filepath.FromSlash(commandToTest2))

commands/service_upload.go

@@ -189,7 +189,7 @@ func (s *arduinoCoreServerImpl) Upload(req *rpc.UploadRequest, stream rpc.Arduin
 		})
 	})
 	defer errStream.Close()
-	updatedPort, err := runProgramAction(
+	updatedPort, err := s.runProgramAction(
 		stream.Context(),
 		pme,
 		sk,
@@ -259,7 +259,7 @@ func (s *arduinoCoreServerImpl) UploadUsingProgrammer(req *rpc.UploadUsingProgra
 	}, streamAdapter)
 }
 
-func runProgramAction(ctx context.Context, pme *packagemanager.Explorer,
+func (s *arduinoCoreServerImpl) runProgramAction(ctx context.Context, pme *packagemanager.Explorer,
 	sk *sketch.Sketch,
 	importFile, importDir, fqbnIn string, userPort *rpc.Port,
 	programmerID string,
@@ -432,7 +432,7 @@ func runProgramAction(ctx context.Context, pme *packagemanager.Explorer,
 	}
 
 	if !burnBootloader {
-		importPath, sketchName, err := determineBuildPathAndSketchName(importFile, importDir, sk)
+		importPath, sketchName, err := s.determineBuildPathAndSketchName(importFile, importDir, sk)
 		if err != nil {
 			return nil, &cmderrors.NotFoundError{Message: i18n.Tr("Error finding build artifacts"), Cause: err}
 		}
@@ -735,7 +735,7 @@ func runTool(recipeID string, props *properties.Map, outStream, errStream io.Wri
 	return nil
 }
 
-func determineBuildPathAndSketchName(importFile, importDir string, sk *sketch.Sketch) (*paths.Path, string, error) {
+func (s *arduinoCoreServerImpl) determineBuildPathAndSketchName(importFile, importDir string, sk *sketch.Sketch) (*paths.Path, string, error) {
 	// In general, compiling a sketch will produce a set of files that are
 	// named as the sketch but have different extensions, for example Sketch.ino
 	// may produce: Sketch.ino.bin; Sketch.ino.hex; Sketch.ino.zip; etc...
@@ -788,7 +788,7 @@ func determineBuildPathAndSketchName(importFile, importDir string, sk *sketch.Sk
 
 	// Case 4: only sketch specified. In this case we use the generated build path
 	// and the given sketch name.
-	return sk.DefaultBuildPath(), sk.Name + sk.MainFile.Ext(), nil
+	return s.getDefaultSketchBuildPath(sk, nil), sk.Name + sk.MainFile.Ext(), nil
 }
 
 func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {

commands/service_upload_burnbootloader.go

@@ -72,7 +72,7 @@ func (s *arduinoCoreServerImpl) BurnBootloader(req *rpc.BurnBootloaderRequest, s
 	}
 	defer release()
 
-	if _, err := runProgramAction(
+	if _, err := s.runProgramAction(
 		stream.Context(),
 		pme,
 		nil, // sketch

commands/service_upload_test.go

@@ -71,6 +71,8 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) {
 	fqbn, err := cores.ParseFQBN("arduino:samd:mkr1000")
 	require.NoError(t, err)
 
+	srv := NewArduinoCoreServer().(*arduinoCoreServerImpl)
+
 	tests := []test{
 		// 00: error: no data passed in
 		{"", "", nil, nil, "<nil>", ""},
@@ -81,7 +83,7 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) {
 		// 03: error: used both importPath and importFile
 		{"testdata/upload/build_path_2/Blink.ino.hex", "testdata/upload/build_path_2", nil, nil, "<nil>", ""},
 		// 04: only sketch without FQBN
-		{"", "", blonk, nil, blonk.DefaultBuildPath().String(), "Blonk.ino"},
+		{"", "", blonk, nil, srv.getDefaultSketchBuildPath(blonk, nil).String(), "Blonk.ino"},
 		// 05: use importFile to detect build.path and project_name, sketch is ignored.
 		{"testdata/upload/build_path_2/Blink.ino.hex", "", blonk, nil, "testdata/upload/build_path_2", "Blink.ino"},
 		// 06: use importPath as build.path and Blink as project name, ignore the sketch Blonk
@@ -97,7 +99,7 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) {
 		// 11: error: used both importPath and importFile
 		{"testdata/upload/build_path_2/Blink.ino.hex", "testdata/upload/build_path_2", nil, fqbn, "<nil>", ""},
 		// 12: use sketch to determine project name and sketch+fqbn to determine build path
-		{"", "", blonk, fqbn, blonk.DefaultBuildPath().String(), "Blonk.ino"},
+		{"", "", blonk, fqbn, srv.getDefaultSketchBuildPath(blonk, nil).String(), "Blonk.ino"},
 		// 13: use importFile to detect build.path and project_name, sketch+fqbn is ignored.
 		{"testdata/upload/build_path_2/Blink.ino.hex", "", blonk, fqbn, "testdata/upload/build_path_2", "Blink.ino"},
 		// 14: use importPath as build.path and Blink as project name, ignore the sketch Blonk, ignore fqbn
@@ -111,7 +113,7 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) {
 	}
 	for i, test := range tests {
 		t.Run(fmt.Sprintf("SubTest%02d", i), func(t *testing.T) {
-			buildPath, sketchName, err := determineBuildPathAndSketchName(test.importFile, test.importDir, test.sketch)
+			buildPath, sketchName, err := srv.determineBuildPathAndSketchName(test.importFile, test.importDir, test.sketch)
 			if test.resBuildPath == "<nil>" {
 				require.Error(t, err)
 				require.Nil(t, buildPath)
@@ -183,10 +185,11 @@ func TestUploadPropertiesComposition(t *testing.T) {
 	pme, release := pm.NewExplorer()
 	defer release()
 
+	srv := NewArduinoCoreServer().(*arduinoCoreServerImpl)
 	testRunner := func(t *testing.T, test test, verboseVerify bool) {
 		outStream := &bytes.Buffer{}
 		errStream := &bytes.Buffer{}
-		_, err := runProgramAction(
+		_, err := srv.runProgramAction(
 			context.Background(),
 			pme,
 			nil,                     // sketch

internal/arduino/sketch/sketch.go

@@ -279,12 +279,6 @@ func (e *InvalidSketchFolderNameError) Error() string {
 	return i18n.Tr("no valid sketch found in %[1]s: missing %[2]s", e.SketchFolder, e.SketchFile)
 }
 
-// DefaultBuildPath generates the default build directory for a given sketch.
-// The build path is in a temporary directory and is unique for each sketch.
-func (s *Sketch) DefaultBuildPath() *paths.Path {
-	return paths.TempDir().Join("arduino", "sketches", s.Hash())
-}
-
 // Hash generate a unique hash for the given sketch.
 func (s *Sketch) Hash() string {
 	path := s.FullPath.String()

internal/arduino/sketch/sketch_test.go

@@ -285,12 +285,6 @@ func TestNewSketchFolderSymlink(t *testing.T) {
 	require.True(t, sketch.RootFolderFiles.ContainsEquivalentTo(sketchPathSymlink.Join("s_file.S")))
 }
 
-func TestGenBuildPath(t *testing.T) {
-	want := paths.TempDir().Join("arduino", "sketches", "ACBD18DB4CC2F85CEDEF654FCCC4A4D8")
-	assert.True(t, (&Sketch{FullPath: paths.New("foo")}).DefaultBuildPath().EquivalentTo(want))
-	assert.Equal(t, "ACBD18DB4CC2F85CEDEF654FCCC4A4D8", (&Sketch{FullPath: paths.New("foo")}).Hash())
-}
-
 func TestNewSketchWithSymlink(t *testing.T) {
 	sketchPath, _ := paths.New("testdata", "SketchWithSymlink").Abs()
 	mainFilePath := sketchPath.Join("SketchWithSymlink.ino")

internal/cli/compile/compile.go

@@ -95,7 +95,7 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer, settings *rpc.Configuration) *
 	compileCommand.Flags().BoolVar(&dumpProfile, "dump-profile", false, i18n.Tr("Create and print a profile configuration from the build."))
 	showPropertiesArg.AddToCommand(compileCommand)
 	compileCommand.Flags().BoolVar(&preprocess, "preprocess", false, i18n.Tr("Print preprocessed code to stdout instead of compiling."))
-	compileCommand.Flags().StringVar(&buildCachePath, "build-cache-path", "", i18n.Tr("Builds of 'core.a' are saved into this path to be cached and reused."))
+	compileCommand.Flags().StringVar(&buildCachePath, "build-cache-path", "", i18n.Tr("Builds of cores and sketches are saved into this path to be cached and reused."))
 	compileCommand.Flags().StringVar(&exportDir, "output-dir", "", i18n.Tr("Save build artifacts in this directory."))
 	compileCommand.Flags().StringVar(&buildPath, "build-path", "",
 		i18n.Tr("Path where to save compiled files. If omitted, a directory will be created in the default temporary path of your OS."))

rpc/cc/arduino/cli/commands/v1/compile.pb.go

@@ -36,7 +36,8 @@ message CompileRequest {
   bool show_properties = 4;
   // Print preprocessed code to stdout instead of compiling.
   bool preprocess = 5;
-  // Builds of 'core.a' are saved into this path to be cached and reused.
+  // Builds of core and sketches are saved into this path to be cached and
+  // reused.
   string build_cache_path = 6;
   // Path to use to store the files used for the compilation. If omitted,
   // a directory will be created in the operating system's default temporary