Skip to content

Commit

Permalink
gRPC: Compile will now fail if a platform core has been modified. (#…
Browse files Browse the repository at this point in the history
…2551)

* Slighlty moved variables near their utilization place

* Added facility to keep timestamps of used files

* Compile will fail if platforms are changed

* Added integration test

* Make linter happy

* Fixed grammar

* Test all monitored files
  • Loading branch information
cmaglie committed Feb 27, 2024
1 parent b987df1 commit af0b60e
Show file tree
Hide file tree
Showing 8 changed files with 408 additions and 132 deletions.
16 changes: 16 additions & 0 deletions commands/cmderrors/cmderrors.go
Original file line number Diff line number Diff line change
Expand Up @@ -869,3 +869,19 @@ func (e *MultipleLibraryInstallDetected) Error() string {
func (e *MultipleLibraryInstallDetected) ToRPCStatus() *status.Status {
return status.New(codes.InvalidArgument, e.Error())
}

// InstanceNeedsReinitialization
type InstanceNeedsReinitialization struct {
}

func (e *InstanceNeedsReinitialization) Error() string {
return tr("The instance is no longer valid and needs to be reinitialized")
}

// ToRPCStatus converts the error into a *status.Status
func (e *InstanceNeedsReinitialization) ToRPCStatus() *status.Status {
st, _ := status.
New(codes.InvalidArgument, e.Error()).
WithDetails(&rpc.InstanceNeedsReinitializationError{})
return st
}
4 changes: 4 additions & 0 deletions commands/compile/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
}
defer release()

if pme.Dirty() {
return nil, &cmderrors.InstanceNeedsReinitialization{}
}

lm, err := instances.GetLibraryManager(req.GetInstance())
if err != nil {
return nil, err
Expand Down
48 changes: 48 additions & 0 deletions internal/arduino/cores/cores.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"path/filepath"
"sort"
"strings"
"time"

"github.com/arduino/arduino-cli/internal/arduino/globals"
"github.com/arduino/arduino-cli/internal/arduino/resources"
Expand Down Expand Up @@ -70,13 +71,59 @@ type PlatformRelease struct {
Programmers map[string]*Programmer `json:"-"`
Menus *properties.Map `json:"-"`
InstallDir *paths.Path `json:"-"`
Timestamps *TimestampsStore // Contains the timestamps of the files used to build this PlatformRelease
IsTrusted bool `json:"-"`
PluggableDiscoveryAware bool `json:"-"` // true if the Platform supports pluggable discovery (no compatibility layer required)
Monitors map[string]*MonitorDependency `json:"-"`
MonitorsDevRecipes map[string]string `json:"-"`
Compatible bool `json:"-"` // true if at all ToolDependencies are available for the current OS/ARCH.
}

// TimestampsStore is a generic structure to store timestamps
type TimestampsStore struct {
timestamps map[*paths.Path]*time.Time
}

// NewTimestampsStore creates a new TimestampsStore
func NewTimestampsStore() *TimestampsStore {
return &TimestampsStore{
timestamps: map[*paths.Path]*time.Time{},
}
}

// AddFile adds a file to the TimestampsStore
func (t *TimestampsStore) AddFile(path *paths.Path) {
if info, err := path.Stat(); err != nil {
t.timestamps[path] = nil // Save a missing file with a nil timestamp
} else {
modtime := info.ModTime()
t.timestamps[path] = &modtime
}
}

// Dirty returns true if one of the files stored in the TimestampsStore has been
// changed after being added to the store.
func (t *TimestampsStore) Dirty() bool {
for path, timestamp := range t.timestamps {
if info, err := path.Stat(); err != nil {
if timestamp != nil {
return true
}
} else {
if timestamp == nil || info.ModTime() != *timestamp {
return true
}
}
}
return false
}

// Dirty returns true if one of the files of this PlatformRelease has been changed
// (it means that the PlatformRelease should be rebuilt to be used correctly).
func (release *PlatformRelease) Dirty() bool {
return release.Timestamps.Dirty()
}

// BoardManifest contains information about a board. These metadata are usually
// provided by the package_index.json
type BoardManifest struct {
Expand Down Expand Up @@ -207,6 +254,7 @@ func (platform *Platform) GetOrCreateRelease(version *semver.Version) *PlatformR
Properties: properties.NewMap(),
Programmers: map[string]*Programmer{},
Platform: platform,
Timestamps: NewTimestampsStore(),
}
platform.Releases[tag] = release
return release
Expand Down
27 changes: 17 additions & 10 deletions internal/arduino/cores/packagemanager/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,29 +239,32 @@ func (pm *Builder) loadPlatform(targetPackage *cores.Package, architecture strin
func (pm *Builder) loadPlatformRelease(platform *cores.PlatformRelease, path *paths.Path) error {
platform.InstallDir = path

// Some useful paths
installedJSONPath := path.Join("installed.json")
platformTxtPath := path.Join("platform.txt")
platformTxtLocalPath := path.Join("platform.local.txt")
programmersTxtPath := path.Join("programmers.txt")

// If the installed.json file is found load it, this is done to handle the
// case in which the platform's index and its url have been deleted locally,
// if we don't load it some information about the platform is lost
installedJSONPath := path.Join("installed.json")
platform.Timestamps.AddFile(installedJSONPath)
if installedJSONPath.Exist() {
if _, err := pm.LoadPackageIndexFromFile(installedJSONPath); err != nil {
return fmt.Errorf(tr("loading %[1]s: %[2]s"), installedJSONPath, err)
}
}

// TODO: why CLONE?
platform.Properties = platform.Properties.Clone()

// Create platform properties
platform.Properties = platform.Properties.Clone() // TODO: why CLONE?
if p, err := properties.SafeLoad(platformTxtPath.String()); err == nil {
platformTxtPath := path.Join("platform.txt")
platform.Timestamps.AddFile(platformTxtPath)
if p, err := properties.SafeLoadFromPath(platformTxtPath); err == nil {
platform.Properties.Merge(p)
} else {
return fmt.Errorf(tr("loading %[1]s: %[2]s"), platformTxtPath, err)
}
if p, err := properties.SafeLoad(platformTxtLocalPath.String()); err == nil {

platformTxtLocalPath := path.Join("platform.local.txt")
platform.Timestamps.AddFile(platformTxtLocalPath)
if p, err := properties.SafeLoadFromPath(platformTxtLocalPath); err == nil {
platform.Properties.Merge(p)
} else {
return fmt.Errorf(tr("loading %[1]s: %[2]s"), platformTxtLocalPath, err)
Expand All @@ -287,7 +290,9 @@ func (pm *Builder) loadPlatformRelease(platform *cores.PlatformRelease, path *pa
}

// Create programmers properties
if programmersProperties, err := properties.SafeLoad(programmersTxtPath.String()); err == nil {
programmersTxtPath := path.Join("programmers.txt")
platform.Timestamps.AddFile(programmersTxtPath)
if programmersProperties, err := properties.SafeLoadFromPath(programmersTxtPath); err == nil {
for programmerID, programmerProps := range programmersProperties.FirstLevelOf() {
if !platform.PluggableDiscoveryAware {
convertUploadToolsToPluggableDiscovery(programmerProps)
Expand Down Expand Up @@ -410,12 +415,14 @@ func (pm *Builder) loadBoards(platform *cores.PlatformRelease) error {
}

boardsTxtPath := platform.InstallDir.Join("boards.txt")
platform.Timestamps.AddFile(boardsTxtPath)
allBoardsProperties, err := properties.LoadFromPath(boardsTxtPath)
if err != nil {
return err
}

boardsLocalTxtPath := platform.InstallDir.Join("boards.local.txt")
platform.Timestamps.AddFile(boardsLocalTxtPath)
if boardsLocalProperties, err := properties.SafeLoadFromPath(boardsLocalTxtPath); err == nil {
allBoardsProperties.Merge(boardsLocalProperties)
} else {
Expand Down
9 changes: 9 additions & 0 deletions internal/arduino/cores/packagemanager/package_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"os"
"path"
"path/filepath"
"slices"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -898,3 +899,11 @@ func (pme *Explorer) NormalizeFQBN(fqbn *cores.FQBN) (*cores.FQBN, error) {
}
return normalizedFqbn, nil
}

// Dirty returns true if one of the loaded platforms needs to be re-initialized
// due to file changes in the platform releases.
func (pme *Explorer) Dirty() bool {
return slices.ContainsFunc(
pme.InstalledPlatformReleases(),
(*cores.PlatformRelease).Dirty)
}
136 changes: 136 additions & 0 deletions internal/integrationtest/daemon/detect_core_changes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// This file is part of arduino-cli.
//
// Copyright 2022 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 license@arduino.cc.

package daemon_test

import (
"context"
"errors"
"fmt"
"io"
"testing"
"time"

"github.com/arduino/arduino-cli/internal/integrationtest"
"github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-paths-helper"

"github.com/stretchr/testify/require"
)

func TestDetectionOfChangesInCoreBeforeCompile(t *testing.T) {
// See: https://github.com/arduino/arduino-cli/issues/2523

env, cli := integrationtest.CreateEnvForDaemon(t)
defer env.CleanUp()

// Create a new instance of the daemon
grpcInst := cli.Create()
require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
fmt.Printf("INIT> %v\n", ir.GetMessage())
}))

// Install avr core
installCl, err := grpcInst.PlatformInstall(context.Background(), "arduino", "avr", "", true)
require.NoError(t, err)
for {
installResp, err := installCl.Recv()
if errors.Is(err, io.EOF) {
break
}
require.NoError(t, err)
fmt.Printf("INSTALL> %v\n", installResp)
}
installCl.CloseSend()

// Utility functions: tryCompile
sketchPath, err := paths.New("testdata", "bare_minimum").Abs()
require.NoError(t, err)
tryCompile := func() error {
compileCl, err := grpcInst.Compile(context.Background(), "arduino:avr:uno", sketchPath.String(), "")
require.NoError(t, err)
defer compileCl.CloseSend()
for {
if compileResp, err := compileCl.Recv(); errors.Is(err, io.EOF) {
return nil
} else if err != nil {
return err
} else {
fmt.Printf("COMPILE> %v\n", compileResp)
}
}
}

// Utility functions: tryTouch will touch a file and see if the compile detects the change
tryTouch := func(fileToTouch *paths.Path) {
time.Sleep(time.Second) // await at least one second so the timestamp of the file is different

// touch the file
f, err := fileToTouch.Append()
require.NoError(t, err)
_, err = f.WriteString("\n")
require.NoError(t, err)
require.NoError(t, f.Close())

// try compile: should fail
err = tryCompile()
require.Error(t, err)
require.Contains(t, err.Error(), "The instance is no longer valid and needs to be reinitialized")

// re-init instance
require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
fmt.Printf("INIT> %v\n", ir.GetMessage())
}))

// try compile: should succeed
require.NoError(t, tryCompile())
}

avrCorePath := cli.DataDir().Join("packages", "arduino", "hardware", "avr", "1.8.6")
tryTouch(avrCorePath.Join("installed.json"))
tryTouch(avrCorePath.Join("platform.txt"))
tryTouch(avrCorePath.Join("platform.local.txt"))
tryTouch(avrCorePath.Join("programmers.txt"))
tryTouch(avrCorePath.Join("boards.txt"))
tryTouch(avrCorePath.Join("boards.local.txt"))

// Delete a file and check if the change is detected
require.NoError(t, avrCorePath.Join("programmers.txt").Remove())
err = tryCompile()
require.Error(t, err)
require.Contains(t, err.Error(), "The instance is no longer valid and needs to be reinitialized")

// Re-init instance and check again
require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
fmt.Printf("INIT> %v\n", ir.GetMessage())
}))
require.NoError(t, tryCompile())

// Create a file and check if the change is detected
{
f, err := avrCorePath.Join("programmers.txt").Create()
require.NoError(t, err)
require.NoError(t, f.Close())
}
err = tryCompile()
require.Error(t, err)
require.Contains(t, err.Error(), "The instance is no longer valid and needs to be reinitialized")

// Re-init instance and check again
require.NoError(t, grpcInst.Init("", "", func(ir *commands.InitResponse) {
fmt.Printf("INIT> %v\n", ir.GetMessage())
}))
require.NoError(t, tryCompile())
}

0 comments on commit af0b60e

Please sign in to comment.