Skip to content

Commit

Permalink
[skip-changelog] refactoring: Fixing legacy pathnames (#2038)
Browse files Browse the repository at this point in the history
* Removed builderCtx.BuildCachePath

Because it creates a lot of confusion

* Removed useless assignment

* Do not store sketch build-path but generate it on demand

* Removed redundant SketchLocation field and related subroutines

* Replaced deprecated calls (from go linter)
  • Loading branch information
cmaglie authored Jan 27, 2023
1 parent a63aff4 commit 58c6bc3
Show file tree
Hide file tree
Showing 29 changed files with 132 additions and 255 deletions.
23 changes: 11 additions & 12 deletions arduino/sketch/sketch.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ import (
type Sketch struct {
Name string
MainFile *paths.Path
FullPath *paths.Path // FullPath is the path to the Sketch folder
BuildPath *paths.Path
FullPath *paths.Path // FullPath is the path to the Sketch folder
OtherSketchFiles paths.PathList // Sketch files that end in .ino other than main file
AdditionalFiles paths.PathList
RootFolderFiles paths.PathList // All files that are in the Sketch root
Expand Down Expand Up @@ -81,7 +80,6 @@ func New(path *paths.Path) (*Sketch, error) {
Name: path.Base(),
MainFile: mainFile,
FullPath: path,
BuildPath: GenBuildPath(path),
OtherSketchFiles: paths.PathList{},
AdditionalFiles: paths.PathList{},
RootFolderFiles: paths.PathList{},
Expand Down Expand Up @@ -293,14 +291,15 @@ func CheckForPdeFiles(sketch *paths.Path) []*paths.Path {
return files
}

// GenBuildPath generates a suitable name for the build folder.
// The sketchPath, if not nil, is also used to furhter differentiate build paths.
func GenBuildPath(sketchPath *paths.Path) *paths.Path {
path := ""
if sketchPath != nil {
path = sketchPath.String()
}
// 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", "sketch-"+s.Hash())
}

// Hash generate a unique hash for the given sketch.
func (s *Sketch) Hash() string {
path := s.FullPath.String()
md5SumBytes := md5.Sum([]byte(path))
md5Sum := strings.ToUpper(hex.EncodeToString(md5SumBytes[:]))
return paths.TempDir().Join("arduino", "sketch-"+md5Sum)
return strings.ToUpper(hex.EncodeToString(md5SumBytes[:]))
}
6 changes: 2 additions & 4 deletions arduino/sketch/sketch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,8 @@ func TestNewSketchFolderSymlink(t *testing.T) {

func TestGenBuildPath(t *testing.T) {
want := paths.TempDir().Join("arduino", "sketch-ACBD18DB4CC2F85CEDEF654FCCC4A4D8")
assert.True(t, GenBuildPath(paths.New("foo")).EquivalentTo(want))

want = paths.TempDir().Join("arduino", "sketch-D41D8CD98F00B204E9800998ECF8427E")
assert.True(t, GenBuildPath(nil).EquivalentTo(want))
assert.True(t, (&Sketch{FullPath: paths.New("foo")}).DefaultBuildPath().EquivalentTo(want))
assert.Equal(t, "ACBD18DB4CC2F85CEDEF654FCCC4A4D8", (&Sketch{FullPath: paths.New("foo")}).Hash())
}

func TestCheckForPdeFiles(t *testing.T) {
Expand Down
17 changes: 10 additions & 7 deletions commands/compile/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
}
builderCtx.UseCachedLibrariesResolution = req.GetSkipLibrariesDiscovery()
builderCtx.FQBN = fqbn
builderCtx.SketchLocation = sk.FullPath
builderCtx.Sketch = sk
builderCtx.ProgressCB = progressCB

// FIXME: This will be redundant when arduino-builder will be part of the cli
Expand All @@ -128,7 +128,7 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
builderCtx.OtherLibrariesDirs.Add(configuration.LibrariesDir(configuration.Settings))
builderCtx.LibraryDirs = paths.NewPathList(req.Library...)
if req.GetBuildPath() == "" {
builderCtx.BuildPath = sk.BuildPath
builderCtx.BuildPath = sk.DefaultBuildPath()
} else {
builderCtx.BuildPath = paths.New(req.GetBuildPath()).Canonical()
}
Expand All @@ -144,8 +144,6 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
// Optimize for debug
builderCtx.OptimizeForDebug = req.GetOptimizeForDebug()

builderCtx.CoreBuildCachePath = paths.TempDir().Join("arduino", "core-cache")

builderCtx.Jobs = int(req.GetJobs())

builderCtx.USBVidPid = req.GetVidPid()
Expand All @@ -154,12 +152,17 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
builderCtx.CustomBuildProperties = append(req.GetBuildProperties(), "build.warn_data_percentage=75")
builderCtx.CustomBuildProperties = append(req.GetBuildProperties(), securityKeysOverride...)

if req.GetBuildCachePath() != "" {
builderCtx.BuildCachePath = paths.New(req.GetBuildCachePath())
err = builderCtx.BuildCachePath.MkdirAll()
if req.GetBuildCachePath() == "" {
builderCtx.CoreBuildCachePath = paths.TempDir().Join("arduino", "core-cache")
} else {
buildCachePath, err := paths.New(req.GetBuildCachePath()).Abs()
if err != nil {
return nil, &arduino.PermissionDeniedError{Message: tr("Cannot create build cache directory"), Cause: err}
}
if err := buildCachePath.MkdirAll(); err != nil {
return nil, &arduino.PermissionDeniedError{Message: tr("Cannot create build cache directory"), Cause: err}
}
builderCtx.CoreBuildCachePath = buildCachePath.Join("core")
}

builderCtx.BuiltInLibrariesDirs = configuration.IDEBuiltinLibrariesDir(configuration.Settings)
Expand Down
4 changes: 3 additions & 1 deletion commands/debug/debug_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,11 @@ func getDebugProperties(req *debug.DebugConfigRequest, pme *packagemanager.Explo
}
}

importPath := sk.BuildPath
var importPath *paths.Path
if importDir := req.GetImportDir(); importDir != "" {
importPath = paths.New(importDir)
} else {
importPath = sk.DefaultBuildPath()
}
if !importPath.Exist() {
return nil, &arduino.NotFoundError{Message: tr("Compiled sketch not found in %s", importPath)}
Expand Down
2 changes: 1 addition & 1 deletion commands/upload/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,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.BuildPath, sk.Name + sk.MainFile.Ext(), nil
return sk.DefaultBuildPath(), sk.Name + sk.MainFile.Ext(), nil
}

func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {
Expand Down
4 changes: 2 additions & 2 deletions commands/upload/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) {
// 03: error: used both importPath and importFile
{"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", nil, nil, "<nil>", ""},
// 04: only sketch without FQBN
{"", "", blonk, nil, sketch.GenBuildPath(blonk.FullPath).String(), "Blonk.ino"},
{"", "", blonk, nil, blonk.DefaultBuildPath().String(), "Blonk.ino"},
// 05: use importFile to detect build.path and project_name, sketch is ignored.
{"testdata/build_path_2/Blink.ino.hex", "", blonk, nil, "testdata/build_path_2", "Blink.ino"},
// 06: use importPath as build.path and Blink as project name, ignore the sketch Blonk
Expand All @@ -95,7 +95,7 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) {
// 11: error: used both importPath and importFile
{"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", nil, fqbn, "<nil>", ""},
// 12: use sketch to determine project name and sketch+fqbn to determine build path
{"", "", blonk, fqbn, sketch.GenBuildPath(blonk.FullPath).String(), "Blonk.ino"},
{"", "", blonk, fqbn, blonk.DefaultBuildPath().String(), "Blonk.ino"},
// 13: use importFile to detect build.path and project_name, sketch+fqbn is ignored.
{"testdata/build_path_2/Blink.ino.hex", "", blonk, fqbn, "testdata/build_path_2", "Blink.ino"},
// 14: use importPath as build.path and Blink as project name, ignore the sketch Blonk, ignore fqbn
Expand Down
9 changes: 0 additions & 9 deletions legacy/builder/add_additional_entries_to_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,6 @@ func (*AddAdditionalEntriesToContext) Run(ctx *types.Context) error {
ctx.CoreBuildPath = coreBuildPath
}

if ctx.BuildCachePath != nil {
coreBuildCachePath, err := ctx.BuildCachePath.Join(constants.FOLDER_CORE).Abs()
if err != nil {
return errors.WithStack(err)
}

ctx.CoreBuildCachePath = coreBuildCachePath
}

if ctx.WarningsLevel == "" {
ctx.WarningsLevel = DEFAULT_WARNINGS_LEVEL
}
Expand Down
8 changes: 0 additions & 8 deletions legacy/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"reflect"
"time"

"github.com/arduino/arduino-cli/arduino/sketch"
"github.com/arduino/arduino-cli/i18n"
"github.com/arduino/arduino-cli/legacy/builder/phases"
"github.com/arduino/arduino-cli/legacy/builder/types"
Expand Down Expand Up @@ -129,10 +128,6 @@ func (s *PreprocessSketch) Run(ctx *types.Context) error {
type Preprocess struct{}

func (s *Preprocess) Run(ctx *types.Context) error {
if ctx.BuildPath == nil {
ctx.BuildPath = sketch.GenBuildPath(ctx.SketchLocation)
}

if err := ctx.BuildPath.MkdirAll(); err != nil {
return err
}
Expand Down Expand Up @@ -188,9 +183,6 @@ func RunBuilder(ctx *types.Context) error {
}

func RunParseHardware(ctx *types.Context) error {
if ctx.BuildPath == nil {
ctx.BuildPath = sketch.GenBuildPath(ctx.SketchLocation)
}
commands := []types.Command{
&ContainerSetupHardwareToolsLibsSketchAndProps{},
}
Expand Down
17 changes: 0 additions & 17 deletions legacy/builder/container_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package builder

import (
sk "github.com/arduino/arduino-cli/arduino/sketch"
"github.com/arduino/arduino-cli/legacy/builder/types"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -48,22 +47,6 @@ func (s *ContainerSetupHardwareToolsLibsSketchAndProps) Run(ctx *types.Context)
ctx.PushProgress()
}

if ctx.SketchLocation != nil {
// get abs path to sketch
sketchLocation, err := ctx.SketchLocation.Abs()
if err != nil {
return errors.WithStack(err)
}

// load sketch
sketch, err := sk.New(sketchLocation)
if err != nil {
return errors.WithStack(err)
}
sketch.BuildPath = ctx.BuildPath
ctx.SketchLocation = sketch.MainFile
ctx.Sketch = sketch
}
ctx.Progress.CompleteStep()
ctx.PushProgress()

Expand Down
17 changes: 2 additions & 15 deletions legacy/builder/fail_if_buildpath_equals_sketchpath.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,8 @@ import (
type FailIfBuildPathEqualsSketchPath struct{}

func (s *FailIfBuildPathEqualsSketchPath) Run(ctx *types.Context) error {
if ctx.BuildPath == nil || ctx.SketchLocation == nil {
return nil
}

buildPath, err := ctx.BuildPath.Abs()
if err != nil {
return errors.WithStack(err)
}

sketchPath, err := ctx.SketchLocation.Abs()
if err != nil {
return errors.WithStack(err)
}
sketchPath = sketchPath.Parent()

buildPath := ctx.BuildPath.Canonical()
sketchPath := ctx.Sketch.FullPath.Canonical()
if buildPath.EqualsTo(sketchPath) {
return errors.New(tr("Sketch cannot be located in build path. Please specify a different build path"))
}
Expand Down
9 changes: 1 addition & 8 deletions legacy/builder/setup_build_properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,7 @@ func (s *SetupBuildProperties) Run(ctx *types.Context) error {
buildProperties.Set("software", DEFAULT_SOFTWARE)
}

if ctx.SketchLocation != nil {
sourcePath, err := ctx.SketchLocation.Abs()
if err != nil {
return err
}
sourcePath = sourcePath.Parent()
buildProperties.SetPath("build.source.path", sourcePath)
}
buildProperties.SetPath("build.source.path", ctx.Sketch.FullPath)

now := time.Now()
buildProperties.Set("extra.time.utc", strconv.FormatInt(now.Unix(), 10))
Expand Down
55 changes: 0 additions & 55 deletions legacy/builder/sketch_loader.go

This file was deleted.

14 changes: 8 additions & 6 deletions legacy/builder/test/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,21 @@ import (
"testing"
"time"

"github.com/arduino/go-paths-helper"
"github.com/sirupsen/logrus"

"github.com/arduino/arduino-cli/arduino/sketch"
"github.com/arduino/arduino-cli/legacy/builder"
"github.com/arduino/arduino-cli/legacy/builder/constants"
"github.com/arduino/arduino-cli/legacy/builder/phases"
"github.com/arduino/arduino-cli/legacy/builder/types"
"github.com/arduino/go-paths-helper"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
)

func prepareBuilderTestContext(t *testing.T, sketchPath *paths.Path, fqbn string) *types.Context {
sk, err := sketch.New(sketchPath)
require.NoError(t, err)
return &types.Context{
SketchLocation: sketchPath,
Sketch: sk,
FQBN: parseFQBN(t, fqbn),
HardwareDirs: paths.NewPathList(filepath.Join("..", "hardware"), "downloaded_hardware"),
BuiltInToolsDirs: paths.NewPathList("downloaded_tools"),
Expand Down Expand Up @@ -261,7 +263,7 @@ func TestBuilderBridgeRedBearLab(t *testing.T) {
func TestBuilderSketchNoFunctions(t *testing.T) {
DownloadCoresAndToolsAndLibraries(t)

ctx := prepareBuilderTestContext(t, paths.New("sketch_no_functions", "main.ino"), "RedBearLab:avr:blend")
ctx := prepareBuilderTestContext(t, paths.New("sketch_no_functions", "sketch_no_functions.ino"), "RedBearLab:avr:blend")
ctx.HardwareDirs = append(ctx.HardwareDirs, paths.New("downloaded_board_manager_stuff"))
ctx.BuiltInToolsDirs = append(ctx.BuiltInToolsDirs, paths.New("downloaded_board_manager_stuff"))

Expand Down Expand Up @@ -371,7 +373,7 @@ func TestBuilderCacheCoreAFile(t *testing.T) {
SetupBuildPath(t, ctx)
defer ctx.BuildPath.RemoveAll()
SetupBuildCachePath(t, ctx)
defer ctx.BuildCachePath.RemoveAll()
defer ctx.CoreBuildCachePath.RemoveAll()

// Run build
bldr := builder.Builder{}
Expand Down
4 changes: 2 additions & 2 deletions legacy/builder/test/builder_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package test

import (
"io/ioutil"
"os"
"testing"
"time"

Expand All @@ -32,7 +32,7 @@ func sleep(t *testing.T) {
}

func tempFile(t *testing.T, prefix string) *paths.Path {
file, err := ioutil.TempFile("", prefix)
file, err := os.CreateTemp("", prefix)
file.Close()
NoError(t, err)
return paths.New(file.Name())
Expand Down
3 changes: 2 additions & 1 deletion legacy/builder/test/create_build_options_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package test
import (
"testing"

"github.com/arduino/arduino-cli/arduino/sketch"
"github.com/arduino/arduino-cli/legacy/builder"
"github.com/arduino/arduino-cli/legacy/builder/types"
"github.com/arduino/go-paths-helper"
Expand All @@ -29,7 +30,7 @@ func TestCreateBuildOptionsMap(t *testing.T) {
HardwareDirs: paths.NewPathList("hardware", "hardware2"),
BuiltInToolsDirs: paths.NewPathList("tools"),
OtherLibrariesDirs: paths.NewPathList("libraries"),
SketchLocation: paths.New("sketchLocation"),
Sketch: &sketch.Sketch{FullPath: paths.New("sketchLocation")},
FQBN: parseFQBN(t, "my:nice:fqbn"),
ArduinoAPIVersion: "ideVersion",
Verbose: true,
Expand Down
Loading

0 comments on commit 58c6bc3

Please sign in to comment.