Skip to content

Return error instead of nil in package/library manager getters #2476

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 1 commit into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
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