Skip to content

gRPC Compile will now fail if a platform core has been modified. #2551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 more 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)
}
125 changes: 125 additions & 0 deletions internal/integrationtest/daemon/detect_core_changes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// 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 [email protected].

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()

// Run a compile
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)
}
}
}
require.NoError(t, tryCompile())

// Change a file in the AVR core and check if the change is detected
avrCorePath := cli.DataDir().Join("packages", "arduino", "hardware", "avr", "1.8.6")
{
time.Sleep(time.Second) // await at least one second so the timestamp of the file is different

f, err := avrCorePath.Join("boards.txt").Append()
require.NoError(t, err)
_, err = f.WriteString("\n")
require.NoError(t, err)
require.NoError(t, f.Close())
}
err = tryCompile()
require.Error(t, err)
require.Contains(t, err.Error(), "The instance is no more 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())

// 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 more 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 more 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())
}
Loading