Unverified Commit f7b22f76 authored by Umberto Baldi's avatar Umberto Baldi Committed by GitHub

fix `core list --all` sometimes crashing (#1519)

* fix packages being added to the package manager even if they were not respecting the specification

* add test to check if if the bug it's fixed

* fix platform release being nil and causing panic with indexes not compliant with specs

* add test to check if the bug is fixed

* add comments and optimize the code a little bit

* Apply suggestions from code review
Co-authored-by: default avatarper1234 <accounts@perglass.com>
Co-authored-by: default avatarper1234 <accounts@perglass.com>
parent 8e6f93f7
...@@ -145,6 +145,10 @@ func (pm *PackageManager) LoadHardwareFromDirectory(path *paths.Path) []*status. ...@@ -145,6 +145,10 @@ func (pm *PackageManager) LoadHardwareFromDirectory(path *paths.Path) []*status.
statuses = append(statuses, errs...) statuses = append(statuses, errs...)
} }
} }
// If the Package does not contain Platforms or Tools we remove it since does not contain anything valuable
if len(targetPackage.Platforms) == 0 && len(targetPackage.Tools) == 0 {
delete(pm.Packages, packager)
}
} }
return statuses return statuses
...@@ -262,7 +266,6 @@ func (pm *PackageManager) loadPlatform(targetPackage *cores.Package, platformPat ...@@ -262,7 +266,6 @@ func (pm *PackageManager) loadPlatform(targetPackage *cores.Package, platformPat
// case: ARCHITECTURE/VERSION/boards.txt // case: ARCHITECTURE/VERSION/boards.txt
// let's dive into VERSION directories // let's dive into VERSION directories
platform := targetPackage.GetOrCreatePlatform(architecture)
versionDirs, err := platformPath.ReadDir() versionDirs, err := platformPath.ReadDir()
if err != nil { if err != nil {
return status.Newf(codes.FailedPrecondition, tr("reading dir %[1]s: %[2]s"), platformPath, err) return status.Newf(codes.FailedPrecondition, tr("reading dir %[1]s: %[2]s"), platformPath, err)
...@@ -280,6 +283,7 @@ func (pm *PackageManager) loadPlatform(targetPackage *cores.Package, platformPat ...@@ -280,6 +283,7 @@ func (pm *PackageManager) loadPlatform(targetPackage *cores.Package, platformPat
if err != nil { if err != nil {
return status.Newf(codes.FailedPrecondition, tr("invalid version dir %[1]s: %[2]s"), versionDir, err) return status.Newf(codes.FailedPrecondition, tr("invalid version dir %[1]s: %[2]s"), versionDir, err)
} }
platform := targetPackage.GetOrCreatePlatform(architecture)
release := platform.GetOrCreateRelease(version) release := platform.GetOrCreateRelease(version)
if err := pm.loadPlatformRelease(release, versionDir); err != nil { if err := pm.loadPlatformRelease(release, versionDir); err != nil {
return status.Newf(codes.FailedPrecondition, tr("loading platform release %[1]s: %[2]s"), release, err) return status.Newf(codes.FailedPrecondition, tr("loading platform release %[1]s: %[2]s"), release, err)
......
...@@ -38,19 +38,23 @@ func GetPlatforms(req *rpc.PlatformListRequest) ([]*rpc.Platform, error) { ...@@ -38,19 +38,23 @@ func GetPlatforms(req *rpc.PlatformListRequest) ([]*rpc.Platform, error) {
for _, platform := range targetPackage.Platforms { for _, platform := range targetPackage.Platforms {
platformRelease := packageManager.GetInstalledPlatformRelease(platform) platformRelease := packageManager.GetInstalledPlatformRelease(platform)
// The All flags adds to the list of installed platforms the installable platforms (from the indexes)
// If both All and UpdatableOnly are set All takes precedence // If both All and UpdatableOnly are set All takes precedence
if req.All { if req.All {
installedVersion := "" installedVersion := ""
if platformRelease == nil { if platformRelease == nil { // if the platform is not installed
platformRelease = platform.GetLatestRelease() platformRelease = platform.GetLatestRelease()
} else { } else {
installedVersion = platformRelease.Version.String() installedVersion = platformRelease.Version.String()
} }
rpcPlatform := commands.PlatformReleaseToRPC(platform.GetLatestRelease()) // it could happen, especially with indexes not perfectly compliant with specs that a platform do not contain a valid release
if platformRelease != nil {
rpcPlatform := commands.PlatformReleaseToRPC(platformRelease)
rpcPlatform.Installed = installedVersion rpcPlatform.Installed = installedVersion
res = append(res, rpcPlatform) res = append(res, rpcPlatform)
continue continue
} }
}
if platformRelease != nil { if platformRelease != nil {
latest := platform.GetLatestRelease() latest := platform.GetLatestRelease()
...@@ -58,11 +62,10 @@ func GetPlatforms(req *rpc.PlatformListRequest) ([]*rpc.Platform, error) { ...@@ -58,11 +62,10 @@ func GetPlatforms(req *rpc.PlatformListRequest) ([]*rpc.Platform, error) {
return nil, &arduino.PlatformNotFoundError{Platform: platform.String(), Cause: fmt.Errorf(tr("the platform has no releases"))} return nil, &arduino.PlatformNotFoundError{Platform: platform.String(), Cause: fmt.Errorf(tr("the platform has no releases"))}
} }
if req.UpdatableOnly { // show only the updatable platforms
if latest == platformRelease { if req.UpdatableOnly && latest == platformRelease {
continue continue
} }
}
rpcPlatform := commands.PlatformReleaseToRPC(platformRelease) rpcPlatform := commands.PlatformReleaseToRPC(platformRelease)
rpcPlatform.Installed = platformRelease.Version.String() rpcPlatform.Installed = platformRelease.Version.String()
......
This diff is collapsed.
...@@ -777,3 +777,21 @@ def test_core_list_outdated_core(run_command): ...@@ -777,3 +777,21 @@ def test_core_list_outdated_core(run_command):
latest_version = semver.parse_version_info(samd_core["latest"]) latest_version = semver.parse_version_info(samd_core["latest"])
# Installed version must be older than latest # Installed version must be older than latest
assert installed_version.compare(latest_version) == -1 assert installed_version.compare(latest_version) == -1
def test_core_loading_package_manager(run_command, data_dir):
# Create empty architecture folder (this condition is normally produced by `core uninstall`)
(Path(data_dir) / "packages" / "foovendor" / "hardware" / "fooarch").mkdir(parents=True)
result = run_command(["core", "list", "--all", "--format", "json"])
assert result.ok # this should not make the cli crash
def test_core_index_without_checksum(run_command):
assert run_command(["config", "init", "--dest-dir", "."])
url = "https://raw.githubusercontent.com/keyboardio/ArduinoCore-GD32-Keyboardio/ae5938af2f485910729e7d27aa233032a1cb4734/package_gd32_index.json" # noqa: E501
assert run_command(["config", "add", "board_manager.additional_urls", url])
assert run_command(["core", "update-index"])
result = run_command(["core", "list", "--all"])
assert result.ok # this should not make the cli crash
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment