Skip to content

Commit

Permalink
[skip-changelog] Return error instead of nil in package/library manag…
Browse files Browse the repository at this point in the history
…er getters (#2476)

It makes explicit that the function may fails to get the package manager
or the library manager.
  • Loading branch information
cmaglie authored Dec 22, 2023
1 parent 6732ae0 commit fb1b9a0
Show file tree
Hide file tree
Showing 26 changed files with 127 additions and 116 deletions.
6 changes: 3 additions & 3 deletions commands/board/details.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import (
// Details returns all details for a board including tools and HW identifiers.
// This command basically gather al the information and translates it into the required grpc struct properties
func Details(ctx context.Context, req *rpc.BoardDetailsRequest) (*rpc.BoardDetailsResponse, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()

Expand Down
12 changes: 6 additions & 6 deletions commands/board/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ func identify(pme *packagemanager.Explorer, port *discovery.Port) ([]*rpc.BoardL
// In case of errors partial results from discoveries that didn't fail
// are returned.
func List(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, discoveryStartErrors []error, e error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, nil, err
}
defer release()

Expand Down Expand Up @@ -260,9 +260,9 @@ func hasMatchingBoard(b *rpc.DetectedPort, fqbnFilter *cores.FQBN) bool {

// Watch returns a channel that receives boards connection and disconnection events.
func Watch(ctx context.Context, req *rpc.BoardListWatchRequest) (<-chan *rpc.BoardListWatchResponse, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()
dm := pme.DiscoveryManager()
Expand Down
7 changes: 3 additions & 4 deletions commands/board/listall.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"strings"

"github.com/arduino/arduino-cli/commands"
"github.com/arduino/arduino-cli/commands/cmderrors"
"github.com/arduino/arduino-cli/commands/internal/instances"
"github.com/arduino/arduino-cli/internal/arduino/cores"
"github.com/arduino/arduino-cli/internal/arduino/utils"
Expand All @@ -30,9 +29,9 @@ import (

// ListAll FIXMEDOC
func ListAll(ctx context.Context, req *rpc.BoardListAllRequest) (*rpc.BoardListAllResponse, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()

Expand Down
7 changes: 3 additions & 4 deletions commands/board/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"strings"

"github.com/arduino/arduino-cli/commands"
"github.com/arduino/arduino-cli/commands/cmderrors"
"github.com/arduino/arduino-cli/commands/internal/instances"
"github.com/arduino/arduino-cli/internal/arduino/utils"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
Expand All @@ -32,9 +31,9 @@ import (
// installed. Note that platforms that are not installed don't include boards' FQBNs.
// If no search argument is used all boards are returned.
func Search(ctx context.Context, req *rpc.BoardSearchRequest) (*rpc.BoardSearchResponse, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()

Expand Down
12 changes: 6 additions & 6 deletions commands/compile/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
exportBinaries = reqExportBinaries.GetValue()
}

pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()

lm := instances.GetLibraryManager(req.GetInstance())
if lm == nil {
return nil, &cmderrors.InvalidInstanceError{}
lm, err := instances.GetLibraryManager(req.GetInstance())
if err != nil {
return nil, err
}

logrus.Tracef("Compile %s for %s started", req.GetSketchPath(), req.GetFqbn())
Expand Down
6 changes: 3 additions & 3 deletions commands/core/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ var tr = i18n.Tr

// PlatformDownload FIXMEDOC
func PlatformDownload(ctx context.Context, req *rpc.PlatformDownloadRequest, downloadCB rpc.DownloadProgressCB) (*rpc.PlatformDownloadResponse, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()

Expand Down
6 changes: 3 additions & 3 deletions commands/core/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import (
// PlatformInstall FIXMEDOC
func PlatformInstall(ctx context.Context, req *rpc.PlatformInstallRequest, downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) (*rpc.PlatformInstallResponse, error) {
install := func() error {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return err
}
defer release()

Expand Down
7 changes: 3 additions & 4 deletions commands/core/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"strings"

"github.com/arduino/arduino-cli/commands"
"github.com/arduino/arduino-cli/commands/cmderrors"
"github.com/arduino/arduino-cli/commands/internal/instances"
"github.com/arduino/arduino-cli/internal/arduino/cores"
"github.com/arduino/arduino-cli/internal/arduino/utils"
Expand All @@ -30,9 +29,9 @@ import (

// PlatformSearch FIXMEDOC
func PlatformSearch(req *rpc.PlatformSearchRequest) (*rpc.PlatformSearchResponse, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()

Expand Down
4 changes: 2 additions & 2 deletions commands/core/uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func PlatformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, t

// platformUninstall is the implementation of platform unistaller
func platformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, taskCB rpc.TaskProgressCB) error {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return &cmderrors.InvalidInstanceError{}
}
defer release()
Expand Down
7 changes: 3 additions & 4 deletions commands/core/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"context"

"github.com/arduino/arduino-cli/commands"
"github.com/arduino/arduino-cli/commands/cmderrors"
"github.com/arduino/arduino-cli/commands/internal/instances"
"github.com/arduino/arduino-cli/internal/arduino/cores"
"github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager"
Expand All @@ -29,9 +28,9 @@ import (
// PlatformUpgrade FIXMEDOC
func PlatformUpgrade(ctx context.Context, req *rpc.PlatformUpgradeRequest, downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) (*rpc.PlatformUpgradeResponse, error) {
upgrade := func() (*cores.PlatformRelease, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()

Expand Down
6 changes: 3 additions & 3 deletions commands/debug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ var tr = i18n.Tr
func Debug(ctx context.Context, req *rpc.GetDebugConfigRequest, inStream io.Reader, out io.Writer, interrupt <-chan os.Signal) (*rpc.DebugResponse, error) {

// Get debugging command line to run debugger
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()

Expand Down
12 changes: 6 additions & 6 deletions commands/debug/debug_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,19 @@ import (

// GetDebugConfig returns metadata to start debugging with the specified board
func GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.GetDebugConfigResponse, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()
return getDebugProperties(req, pme, false)
}

// IsDebugSupported checks if the given board/programmer configuration supports debugging.
func IsDebugSupported(ctx context.Context, req *rpc.IsDebugSupportedRequest) (*rpc.IsDebugSupportedResponse, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()
configRequest := &rpc.GetDebugConfigRequest{
Expand Down
17 changes: 12 additions & 5 deletions commands/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,11 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
// after reinitializing an instance after installing or uninstalling a core.
// If this is not done the information of the uninstall core is kept in memory,
// even if it should not.
pmb, commitPackageManager := instances.GetPackageManager(instance).NewBuilder()
pm, err := instances.GetPackageManager(instance)
if err != nil {
return err
}
pmb, commitPackageManager := pm.NewBuilder()

// Load packages index
for _, URL := range allPackageIndexUrls {
Expand Down Expand Up @@ -285,7 +289,10 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
commitPackageManager()
}

pme, release := instances.GetPackageManagerExplorer(instance)
pme, release, err := instances.GetPackageManagerExplorer(instance)
if err != nil {
return err
}
defer release()

for _, err := range pme.LoadDiscoveries() {
Expand Down Expand Up @@ -389,9 +396,9 @@ func Destroy(ctx context.Context, req *rpc.DestroyRequest) (*rpc.DestroyResponse
// UpdateLibrariesIndex updates the library_index.json
func UpdateLibrariesIndex(ctx context.Context, req *rpc.UpdateLibrariesIndexRequest, downloadCB rpc.DownloadProgressCB) error {
logrus.Info("Updating libraries index")
lm := instances.GetLibraryManager(req.GetInstance())
if lm == nil {
return &cmderrors.InvalidInstanceError{}
lm, err := instances.GetLibraryManager(req.GetInstance())
if err != nil {
return err
}

if err := lm.IndexFile.Parent().MkdirAll(); err != nil {
Expand Down
24 changes: 13 additions & 11 deletions commands/internal/instances/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package instances
import (
"sync"

"github.com/arduino/arduino-cli/commands/cmderrors"
"github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager"
"github.com/arduino/arduino-cli/internal/arduino/libraries/librariesmanager"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
Expand All @@ -26,36 +27,37 @@ var instancesMux sync.Mutex
// GetPackageManager returns a PackageManager. If the package manager is not found
// (because the instance is invalid or has been destroyed), nil is returned.
// Deprecated: use GetPackageManagerExplorer instead.
func GetPackageManager(inst *rpc.Instance) *packagemanager.PackageManager {
func GetPackageManager(inst *rpc.Instance) (*packagemanager.PackageManager, error) {
instancesMux.Lock()
i := instances[inst.GetId()]
instancesMux.Unlock()
if i == nil {
return nil
return nil, &cmderrors.InvalidInstanceError{}
}
return i.pm
return i.pm, nil
}

// GetPackageManagerExplorer returns a new package manager Explorer. The
// explorer holds a read lock on the underlying PackageManager and it should
// be released by calling the returned "release" function.
func GetPackageManagerExplorer(req *rpc.Instance) (explorer *packagemanager.Explorer, release func()) {
pm := GetPackageManager(req)
if pm == nil {
return nil, nil
func GetPackageManagerExplorer(req *rpc.Instance) (explorer *packagemanager.Explorer, release func(), _err error) {
pm, err := GetPackageManager(req)
if err != nil {
return nil, nil, err
}
return pm.NewExplorer()
pme, release := pm.NewExplorer()
return pme, release, nil
}

// GetLibraryManager returns the library manager for the given instance.
func GetLibraryManager(inst *rpc.Instance) *librariesmanager.LibrariesManager {
func GetLibraryManager(inst *rpc.Instance) (*librariesmanager.LibrariesManager, error) {
instancesMux.Lock()
i := instances[inst.GetId()]
instancesMux.Unlock()
if i == nil {
return nil
return nil, &cmderrors.InvalidInstanceError{}
}
return i.lm
return i.lm, nil
}

// SetLibraryManager sets the library manager for the given instance.
Expand Down
6 changes: 3 additions & 3 deletions commands/lib/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ var tr = i18n.Tr
func LibraryDownload(ctx context.Context, req *rpc.LibraryDownloadRequest, downloadCB rpc.DownloadProgressCB) (*rpc.LibraryDownloadResponse, error) {
logrus.Info("Executing `arduino-cli lib download`")

lm := instances.GetLibraryManager(req.GetInstance())
if lm == nil {
return nil, &cmderrors.InvalidInstanceError{}
lm, err := instances.GetLibraryManager(req.GetInstance())
if err != nil {
return nil, err
}

logrus.Info("Preparing download")
Expand Down
16 changes: 11 additions & 5 deletions commands/lib/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ import (

// LibraryInstall resolves the library dependencies, then downloads and installs the libraries into the install location.
func LibraryInstall(ctx context.Context, req *rpc.LibraryInstallRequest, downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) error {
lm := instances.GetLibraryManager(req.GetInstance())
if lm == nil {
return &cmderrors.InvalidInstanceError{}
lm, err := instances.GetLibraryManager(req.GetInstance())
if err != nil {
return err
}

toInstall := map[string]*rpc.LibraryDependencyStatus{}
Expand Down Expand Up @@ -145,7 +145,10 @@ func installLibrary(lm *librariesmanager.LibrariesManager, libRelease *libraries

// ZipLibraryInstall FIXMEDOC
func ZipLibraryInstall(ctx context.Context, req *rpc.ZipLibraryInstallRequest, taskCB rpc.TaskProgressCB) error {
lm := instances.GetLibraryManager(req.GetInstance())
lm, err := instances.GetLibraryManager(req.GetInstance())
if err != nil {
return err
}
if err := lm.InstallZipLib(ctx, paths.New(req.GetPath()), req.GetOverwrite()); err != nil {
return &cmderrors.FailedLibraryInstallError{Cause: err}
}
Expand All @@ -155,7 +158,10 @@ func ZipLibraryInstall(ctx context.Context, req *rpc.ZipLibraryInstallRequest, t

// GitLibraryInstall FIXMEDOC
func GitLibraryInstall(ctx context.Context, req *rpc.GitLibraryInstallRequest, taskCB rpc.TaskProgressCB) error {
lm := instances.GetLibraryManager(req.GetInstance())
lm, err := instances.GetLibraryManager(req.GetInstance())
if err != nil {
return err
}
if err := lm.InstallGitLib(req.GetUrl(), req.GetOverwrite()); err != nil {
return &cmderrors.FailedLibraryInstallError{Cause: err}
}
Expand Down
12 changes: 6 additions & 6 deletions commands/lib/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ type installedLib struct {

// LibraryList FIXMEDOC
func LibraryList(ctx context.Context, req *rpc.LibraryListRequest) (*rpc.LibraryListResponse, error) {
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
if pme == nil {
return nil, &cmderrors.InvalidInstanceError{}
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
if err != nil {
return nil, err
}
defer release()

lm := instances.GetLibraryManager(req.GetInstance())
if lm == nil {
return nil, &cmderrors.InvalidInstanceError{}
lm, err := instances.GetLibraryManager(req.GetInstance())
if err != nil {
return nil, err
}

nameFilter := strings.ToLower(req.GetName())
Expand Down
Loading

0 comments on commit fb1b9a0

Please sign in to comment.