Skip to content

Commit fb1b9a0

Browse files
cmagliecmaglie
authored
[skip-changelog] Return error instead of nil in package/library manager getters (#2476)
It makes explicit that the function may fails to get the package manager or the library manager.
1 parent 6732ae0 commit fb1b9a0

26 files changed

+127
-116
lines changed

‎commands/board/details.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ import (
2828
// Details returns all details for a board including tools and HW identifiers.
2929
// This command basically gather al the information and translates it into the required grpc struct properties
3030
func Details(ctx context.Context, req *rpc.BoardDetailsRequest) (*rpc.BoardDetailsResponse, error) {
31-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
32-
if pme == nil {
33-
return nil, &cmderrors.InvalidInstanceError{}
31+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
32+
if err != nil {
33+
return nil, err
3434
}
3535
defer release()
3636

‎commands/board/list.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,9 @@ func identify(pme *packagemanager.Explorer, port *discovery.Port) ([]*rpc.BoardL
205205
// In case of errors partial results from discoveries that didn't fail
206206
// are returned.
207207
func List(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, discoveryStartErrors []error, e error) {
208-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
209-
if pme == nil {
210-
return nil, nil, &cmderrors.InvalidInstanceError{}
208+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
209+
if err != nil {
210+
return nil, nil, err
211211
}
212212
defer release()
213213

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

261261
// Watch returns a channel that receives boards connection and disconnection events.
262262
func Watch(ctx context.Context, req *rpc.BoardListWatchRequest) (<-chan *rpc.BoardListWatchResponse, error) {
263-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
264-
if pme == nil {
265-
return nil, &cmderrors.InvalidInstanceError{}
263+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
264+
if err != nil {
265+
return nil, err
266266
}
267267
defer release()
268268
dm := pme.DiscoveryManager()

‎commands/board/listall.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"strings"
2222

2323
"github.com/arduino/arduino-cli/commands"
24-
"github.com/arduino/arduino-cli/commands/cmderrors"
2524
"github.com/arduino/arduino-cli/commands/internal/instances"
2625
"github.com/arduino/arduino-cli/internal/arduino/cores"
2726
"github.com/arduino/arduino-cli/internal/arduino/utils"
@@ -30,9 +29,9 @@ import (
3029

3130
// ListAll FIXMEDOC
3231
func ListAll(ctx context.Context, req *rpc.BoardListAllRequest) (*rpc.BoardListAllResponse, error) {
33-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
34-
if pme == nil {
35-
return nil, &cmderrors.InvalidInstanceError{}
32+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
33+
if err != nil {
34+
return nil, err
3635
}
3736
defer release()
3837

‎commands/board/search.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"strings"
2222

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

‎commands/compile/compile.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,15 @@ func Compile(ctx context.Context, req *rpc.CompileRequest, outStream, errStream
5757
exportBinaries = reqExportBinaries.GetValue()
5858
}
5959

60-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
61-
if pme == nil {
62-
return nil, &cmderrors.InvalidInstanceError{}
60+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
61+
if err != nil {
62+
return nil, err
6363
}
6464
defer release()
6565

66-
lm := instances.GetLibraryManager(req.GetInstance())
67-
if lm == nil {
68-
return nil, &cmderrors.InvalidInstanceError{}
66+
lm, err := instances.GetLibraryManager(req.GetInstance())
67+
if err != nil {
68+
return nil, err
6969
}
7070

7171
logrus.Tracef("Compile %s for %s started", req.GetSketchPath(), req.GetFqbn())

‎commands/core/download.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ var tr = i18n.Tr
3030

3131
// PlatformDownload FIXMEDOC
3232
func PlatformDownload(ctx context.Context, req *rpc.PlatformDownloadRequest, downloadCB rpc.DownloadProgressCB) (*rpc.PlatformDownloadResponse, error) {
33-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
34-
if pme == nil {
35-
return nil, &cmderrors.InvalidInstanceError{}
33+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
34+
if err != nil {
35+
return nil, err
3636
}
3737
defer release()
3838

‎commands/core/install.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ import (
2929
// PlatformInstall FIXMEDOC
3030
func PlatformInstall(ctx context.Context, req *rpc.PlatformInstallRequest, downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) (*rpc.PlatformInstallResponse, error) {
3131
install := func() error {
32-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
33-
if pme == nil {
34-
return &cmderrors.InvalidInstanceError{}
32+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
33+
if err != nil {
34+
return err
3535
}
3636
defer release()
3737

‎commands/core/search.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"strings"
2222

2323
"github.com/arduino/arduino-cli/commands"
24-
"github.com/arduino/arduino-cli/commands/cmderrors"
2524
"github.com/arduino/arduino-cli/commands/internal/instances"
2625
"github.com/arduino/arduino-cli/internal/arduino/cores"
2726
"github.com/arduino/arduino-cli/internal/arduino/utils"
@@ -30,9 +29,9 @@ import (
3029

3130
// PlatformSearch FIXMEDOC
3231
func PlatformSearch(req *rpc.PlatformSearchRequest) (*rpc.PlatformSearchResponse, error) {
33-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
34-
if pme == nil {
35-
return nil, &cmderrors.InvalidInstanceError{}
32+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
33+
if err != nil {
34+
return nil, err
3635
}
3736
defer release()
3837

‎commands/core/uninstall.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ func PlatformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, t
3838

3939
// platformUninstall is the implementation of platform unistaller
4040
func platformUninstall(ctx context.Context, req *rpc.PlatformUninstallRequest, taskCB rpc.TaskProgressCB) error {
41-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
42-
if pme == nil {
41+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
42+
if err != nil {
4343
return &cmderrors.InvalidInstanceError{}
4444
}
4545
defer release()

‎commands/core/upgrade.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"context"
2020

2121
"github.com/arduino/arduino-cli/commands"
22-
"github.com/arduino/arduino-cli/commands/cmderrors"
2322
"github.com/arduino/arduino-cli/commands/internal/instances"
2423
"github.com/arduino/arduino-cli/internal/arduino/cores"
2524
"github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager"
@@ -29,9 +28,9 @@ import (
2928
// PlatformUpgrade FIXMEDOC
3029
func PlatformUpgrade(ctx context.Context, req *rpc.PlatformUpgradeRequest, downloadCB rpc.DownloadProgressCB, taskCB rpc.TaskProgressCB) (*rpc.PlatformUpgradeResponse, error) {
3130
upgrade := func() (*cores.PlatformRelease, error) {
32-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
33-
if pme == nil {
34-
return nil, &cmderrors.InvalidInstanceError{}
31+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
32+
if err != nil {
33+
return nil, err
3534
}
3635
defer release()
3736

‎commands/debug/debug.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ var tr = i18n.Tr
4444
func Debug(ctx context.Context, req *rpc.GetDebugConfigRequest, inStream io.Reader, out io.Writer, interrupt <-chan os.Signal) (*rpc.DebugResponse, error) {
4545

4646
// Get debugging command line to run debugger
47-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
48-
if pme == nil {
49-
return nil, &cmderrors.InvalidInstanceError{}
47+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
48+
if err != nil {
49+
return nil, err
5050
}
5151
defer release()
5252

‎commands/debug/debug_info.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,19 @@ import (
3838

3939
// GetDebugConfig returns metadata to start debugging with the specified board
4040
func GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.GetDebugConfigResponse, error) {
41-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
42-
if pme == nil {
43-
return nil, &cmderrors.InvalidInstanceError{}
41+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
42+
if err != nil {
43+
return nil, err
4444
}
4545
defer release()
4646
return getDebugProperties(req, pme, false)
4747
}
4848

4949
// IsDebugSupported checks if the given board/programmer configuration supports debugging.
5050
func IsDebugSupported(ctx context.Context, req *rpc.IsDebugSupportedRequest) (*rpc.IsDebugSupportedResponse, error) {
51-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
52-
if pme == nil {
53-
return nil, &cmderrors.InvalidInstanceError{}
51+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
52+
if err != nil {
53+
return nil, err
5454
}
5555
defer release()
5656
configRequest := &rpc.GetDebugConfigRequest{

‎commands/instances.go

+12-5
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,11 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
190190
// after reinitializing an instance after installing or uninstalling a core.
191191
// If this is not done the information of the uninstall core is kept in memory,
192192
// even if it should not.
193-
pmb, commitPackageManager := instances.GetPackageManager(instance).NewBuilder()
193+
pm, err := instances.GetPackageManager(instance)
194+
if err != nil {
195+
return err
196+
}
197+
pmb, commitPackageManager := pm.NewBuilder()
194198

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

288-
pme, release := instances.GetPackageManagerExplorer(instance)
292+
pme, release, err := instances.GetPackageManagerExplorer(instance)
293+
if err != nil {
294+
return err
295+
}
289296
defer release()
290297

291298
for _, err := range pme.LoadDiscoveries() {
@@ -389,9 +396,9 @@ func Destroy(ctx context.Context, req *rpc.DestroyRequest) (*rpc.DestroyResponse
389396
// UpdateLibrariesIndex updates the library_index.json
390397
func UpdateLibrariesIndex(ctx context.Context, req *rpc.UpdateLibrariesIndexRequest, downloadCB rpc.DownloadProgressCB) error {
391398
logrus.Info("Updating libraries index")
392-
lm := instances.GetLibraryManager(req.GetInstance())
393-
if lm == nil {
394-
return &cmderrors.InvalidInstanceError{}
399+
lm, err := instances.GetLibraryManager(req.GetInstance())
400+
if err != nil {
401+
return err
395402
}
396403

397404
if err := lm.IndexFile.Parent().MkdirAll(); err != nil {

‎commands/internal/instances/instances.go

+13-11
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package instances
33
import (
44
"sync"
55

6+
"github.com/arduino/arduino-cli/commands/cmderrors"
67
"github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager"
78
"github.com/arduino/arduino-cli/internal/arduino/libraries/librariesmanager"
89
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
@@ -26,36 +27,37 @@ var instancesMux sync.Mutex
2627
// GetPackageManager returns a PackageManager. If the package manager is not found
2728
// (because the instance is invalid or has been destroyed), nil is returned.
2829
// Deprecated: use GetPackageManagerExplorer instead.
29-
func GetPackageManager(inst *rpc.Instance) *packagemanager.PackageManager {
30+
func GetPackageManager(inst *rpc.Instance) (*packagemanager.PackageManager, error) {
3031
instancesMux.Lock()
3132
i := instances[inst.GetId()]
3233
instancesMux.Unlock()
3334
if i == nil {
34-
return nil
35+
return nil, &cmderrors.InvalidInstanceError{}
3536
}
36-
return i.pm
37+
return i.pm, nil
3738
}
3839

3940
// GetPackageManagerExplorer returns a new package manager Explorer. The
4041
// explorer holds a read lock on the underlying PackageManager and it should
4142
// be released by calling the returned "release" function.
42-
func GetPackageManagerExplorer(req *rpc.Instance) (explorer *packagemanager.Explorer, release func()) {
43-
pm := GetPackageManager(req)
44-
if pm == nil {
45-
return nil, nil
43+
func GetPackageManagerExplorer(req *rpc.Instance) (explorer *packagemanager.Explorer, release func(), _err error) {
44+
pm, err := GetPackageManager(req)
45+
if err != nil {
46+
return nil, nil, err
4647
}
47-
return pm.NewExplorer()
48+
pme, release := pm.NewExplorer()
49+
return pme, release, nil
4850
}
4951

5052
// GetLibraryManager returns the library manager for the given instance.
51-
func GetLibraryManager(inst *rpc.Instance) *librariesmanager.LibrariesManager {
53+
func GetLibraryManager(inst *rpc.Instance) (*librariesmanager.LibrariesManager, error) {
5254
instancesMux.Lock()
5355
i := instances[inst.GetId()]
5456
instancesMux.Unlock()
5557
if i == nil {
56-
return nil
58+
return nil, &cmderrors.InvalidInstanceError{}
5759
}
58-
return i.lm
60+
return i.lm, nil
5961
}
6062

6163
// SetLibraryManager sets the library manager for the given instance.

‎commands/lib/download.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ var tr = i18n.Tr
3535
func LibraryDownload(ctx context.Context, req *rpc.LibraryDownloadRequest, downloadCB rpc.DownloadProgressCB) (*rpc.LibraryDownloadResponse, error) {
3636
logrus.Info("Executing `arduino-cli lib download`")
3737

38-
lm := instances.GetLibraryManager(req.GetInstance())
39-
if lm == nil {
40-
return nil, &cmderrors.InvalidInstanceError{}
38+
lm, err := instances.GetLibraryManager(req.GetInstance())
39+
if err != nil {
40+
return nil, err
4141
}
4242

4343
logrus.Info("Preparing download")

‎commands/lib/install.go

+11-5
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ import (
3333

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

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

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

156159
// GitLibraryInstall FIXMEDOC
157160
func GitLibraryInstall(ctx context.Context, req *rpc.GitLibraryInstallRequest, taskCB rpc.TaskProgressCB) error {
158-
lm := instances.GetLibraryManager(req.GetInstance())
161+
lm, err := instances.GetLibraryManager(req.GetInstance())
162+
if err != nil {
163+
return err
164+
}
159165
if err := lm.InstallGitLib(req.GetUrl(), req.GetOverwrite()); err != nil {
160166
return &cmderrors.FailedLibraryInstallError{Cause: err}
161167
}

‎commands/lib/list.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ type installedLib struct {
3636

3737
// LibraryList FIXMEDOC
3838
func LibraryList(ctx context.Context, req *rpc.LibraryListRequest) (*rpc.LibraryListResponse, error) {
39-
pme, release := instances.GetPackageManagerExplorer(req.GetInstance())
40-
if pme == nil {
41-
return nil, &cmderrors.InvalidInstanceError{}
39+
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
40+
if err != nil {
41+
return nil, err
4242
}
4343
defer release()
4444

45-
lm := instances.GetLibraryManager(req.GetInstance())
46-
if lm == nil {
47-
return nil, &cmderrors.InvalidInstanceError{}
45+
lm, err := instances.GetLibraryManager(req.GetInstance())
46+
if err != nil {
47+
return nil, err
4848
}
4949

5050
nameFilter := strings.ToLower(req.GetName())

0 commit comments

Comments
 (0)