Skip to content

Added parsing of pluggable monitors in package_index.json #1449

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 2 commits into from
Sep 15, 2021
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
33 changes: 33 additions & 0 deletions arduino/cores/cores.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type PlatformRelease struct {
BoardsManifest []*BoardManifest
ToolDependencies ToolDependencies
DiscoveryDependencies DiscoveryDependencies
MonitorDependencies MonitorDependencies
Help PlatformReleaseHelp `json:"-"`
Platform *Platform `json:"-"`
Properties *properties.Map `json:"-"`
Expand Down Expand Up @@ -142,6 +143,30 @@ func (d *DiscoveryDependency) String() string {
return fmt.Sprintf("%s:%s", d.Packager, d.Name)
}

// MonitorDependencies is a list of MonitorDependency
type MonitorDependencies []*MonitorDependency

// Sort the DiscoveryDependencies by name.
func (d MonitorDependencies) Sort() {
sort.Slice(d, func(i, j int) bool {
if d[i].Packager != d[j].Packager {
return d[i].Packager < d[j].Packager
}
return d[i].Name < d[j].Name
})
}

// MonitorDependency identifies a specific monitor, version is omitted
// since the latest version will always be used
type MonitorDependency struct {
Name string
Packager string
}

func (d *MonitorDependency) String() string {
return fmt.Sprintf("%s:%s", d.Packager, d.Name)
}

// GetOrCreateRelease returns the specified release corresponding the provided version,
// or creates a new one if not found.
func (platform *Platform) GetOrCreateRelease(version *semver.Version) *PlatformRelease {
Expand Down Expand Up @@ -268,6 +293,14 @@ func (release *PlatformRelease) RequiresToolRelease(toolRelease *ToolRelease) bo
return true
}
}
for _, monitor := range release.MonitorDependencies {
if monitor.Name == toolRelease.Tool.Name &&
monitor.Packager == toolRelease.Tool.Package.Name &&
// We always want the latest monitor version available
toolRelease.Version.Equal(toolRelease.Tool.LatestRelease().Version) {
return true
}
}
return false
}

Expand Down
28 changes: 28 additions & 0 deletions arduino/cores/packageindex/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type indexPlatformRelease struct {
Help indexHelp `json:"help,omitempty"`
ToolDependencies []indexToolDependency `json:"toolsDependencies"`
DiscoveryDependencies []indexDiscoveryDependency `json:"discoveryDependencies"`
MonitorDependencies []indexMonitorDependency `json:"monitorDependencies"`
}

// indexToolDependency represents a single dependency of a core from a tool.
Expand All @@ -76,6 +77,12 @@ type indexDiscoveryDependency struct {
Name string `json:"name"`
}

// indexMonitorDependency represents a single dependency of a core from a pluggable monitor tool.
type indexMonitorDependency struct {
Packager string `json:"packager"`
Name string `json:"name"`
}

// indexToolRelease represents a single Tool from package_index.json file.
type indexToolRelease struct {
Name string `json:"name,required"`
Expand Down Expand Up @@ -152,6 +159,14 @@ func IndexFromPlatformRelease(pr *cores.PlatformRelease) Index {
})
}

monitors := []indexMonitorDependency{}
for _, m := range pr.MonitorDependencies {
monitors = append(monitors, indexMonitorDependency{
Packager: m.Packager,
Name: m.Name,
})
}

packageTools := []*indexToolRelease{}
for name, tool := range pr.Platform.Package.Tools {
for _, toolRelease := range tool.Releases {
Expand Down Expand Up @@ -196,6 +211,7 @@ func IndexFromPlatformRelease(pr *cores.PlatformRelease) Index {
Help: indexHelp{Online: pr.Help.Online},
ToolDependencies: tools,
DiscoveryDependencies: discoveries,
MonitorDependencies: monitors,
}},
Tools: packageTools,
Help: indexHelp{Online: pr.Platform.Package.Help.Online},
Expand Down Expand Up @@ -251,6 +267,7 @@ func (inPlatformRelease indexPlatformRelease) extractPlatformIn(outPackage *core
outPlatformRelease.BoardsManifest = inPlatformRelease.extractBoardsManifest()
outPlatformRelease.ToolDependencies = inPlatformRelease.extractToolDependencies()
outPlatformRelease.DiscoveryDependencies = inPlatformRelease.extractDiscoveryDependencies()
outPlatformRelease.MonitorDependencies = inPlatformRelease.extractMonitorDependencies()
return nil
}

Expand All @@ -277,6 +294,17 @@ func (inPlatformRelease indexPlatformRelease) extractDiscoveryDependencies() cor
return res
}

func (inPlatformRelease indexPlatformRelease) extractMonitorDependencies() cores.MonitorDependencies {
res := make(cores.MonitorDependencies, len(inPlatformRelease.MonitorDependencies))
for i, discovery := range inPlatformRelease.MonitorDependencies {
res[i] = &cores.MonitorDependency{
Name: discovery.Name,
Packager: discovery.Packager,
}
}
return res
}

func (inPlatformRelease indexPlatformRelease) extractBoardsManifest() []*cores.BoardManifest {
boards := make([]*cores.BoardManifest, len(inPlatformRelease.Boards))
for i, board := range inPlatformRelease.Boards {
Expand Down
21 changes: 21 additions & 0 deletions arduino/cores/packageindex/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,16 @@ func TestIndexFromPlatformRelease(t *testing.T) {
Name: "serial-discovery",
},
},
MonitorDependencies: cores.MonitorDependencies{
{
Packager: "arduino",
Name: "ble-monitor",
},
{
Packager: "arduino",
Name: "serial-monitor",
},
},
Platform: &cores.Platform{
Name: "Arduino AVR Boards",
Architecture: "avr",
Expand Down Expand Up @@ -355,6 +365,16 @@ func TestIndexFromPlatformRelease(t *testing.T) {
Name: "serial-discovery",
},
},
MonitorDependencies: []indexMonitorDependency{
{
Packager: "arduino",
Name: "ble-monitor",
},
{
Packager: "arduino",
Name: "serial-monitor",
},
},
}},
Tools: []*indexToolRelease{
{
Expand Down Expand Up @@ -552,6 +572,7 @@ func TestIndexFromPlatformRelease(t *testing.T) {
require.ElementsMatch(t, expectedPlatform.Boards, indexPlatform.Boards)
require.ElementsMatch(t, expectedPlatform.ToolDependencies, indexPlatform.ToolDependencies)
require.ElementsMatch(t, expectedPlatform.DiscoveryDependencies, indexPlatform.DiscoveryDependencies)
require.ElementsMatch(t, expectedPlatform.MonitorDependencies, indexPlatform.MonitorDependencies)
}
}
}
23 changes: 23 additions & 0 deletions arduino/cores/packagemanager/package_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,17 @@ func (pm *PackageManager) FindToolsRequiredFromPlatformRelease(platform *cores.P
delete(foundTools, tool.Tool.Name)
}

platform.MonitorDependencies.Sort()
for _, monitorDep := range platform.MonitorDependencies {
pm.Log.WithField("monitor", monitorDep).Infof("Required monitor")
tool := pm.FindMonitorDependency(monitorDep)
if tool == nil {
return nil, fmt.Errorf(tr("monitor release not found: %s"), monitorDep)
}
requiredTools = append(requiredTools, tool)
delete(foundTools, tool.Tool.Name)
}

for _, toolRel := range foundTools {
requiredTools = append(requiredTools, toolRel)
}
Expand Down Expand Up @@ -563,3 +574,15 @@ func (pm *PackageManager) FindDiscoveryDependency(discovery *cores.DiscoveryDepe
return toolRelease.GetLatestInstalled()
}
}

// FindMonitorDependency returns the ToolRelease referenced by the MonitorDepenency or nil if
// the referenced monitor doesn't exists.
func (pm *PackageManager) FindMonitorDependency(discovery *cores.MonitorDependency) *cores.ToolRelease {
if pack := pm.Packages[discovery.Packager]; pack == nil {
return nil
} else if toolRelease := pack.Tools[discovery.Name]; toolRelease == nil {
return nil
} else {
return toolRelease.GetLatestInstalled()
}
}
28 changes: 27 additions & 1 deletion arduino/cores/packagemanager/package_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,24 @@ func TestFindToolsRequiredFromPlatformRelease(t *testing.T) {
toolRelease.InstallDir = fakePath
}

{
// ble-monitor tool
tool := pack.GetOrCreateTool("ble-monitor")
toolRelease := tool.GetOrCreateRelease(semver.ParseRelaxed("1.0.0"))
// We set this to fake the tool is installed
toolRelease.InstallDir = fakePath
tool.GetOrCreateRelease(semver.ParseRelaxed("0.1.0"))
}

{
// serial-monitor tool
tool := pack.GetOrCreateTool("serial-monitor")
tool.GetOrCreateRelease(semver.ParseRelaxed("1.0.0"))
toolRelease := tool.GetOrCreateRelease(semver.ParseRelaxed("0.1.0"))
// We set this to fake the tool is installed
toolRelease.InstallDir = fakePath
}

platform := pack.GetOrCreatePlatform("avr")
release := platform.GetOrCreateRelease(semver.MustParse("1.0.0"))
release.ToolDependencies = append(release.ToolDependencies, &cores.ToolDependency{
Expand All @@ -409,12 +427,20 @@ func TestFindToolsRequiredFromPlatformRelease(t *testing.T) {
Name: "serial-discovery",
Packager: "arduino",
})
release.MonitorDependencies = append(release.MonitorDependencies, &cores.MonitorDependency{
Name: "ble-monitor",
Packager: "arduino",
})
release.MonitorDependencies = append(release.MonitorDependencies, &cores.MonitorDependency{
Name: "serial-monitor",
Packager: "arduino",
})
// We set this to fake the platform is installed
release.InstallDir = fakePath

tools, err := pm.FindToolsRequiredFromPlatformRelease(release)
require.NoError(t, err)
require.Len(t, tools, 4)
require.Len(t, tools, 6)
}

func TestLegacyPackageConversionToPluggableDiscovery(t *testing.T) {
Expand Down
27 changes: 27 additions & 0 deletions arduino/cores/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,33 @@ func (packages Packages) GetPlatformReleaseDiscoveryDependencies(release *Platfo
return res, nil
}

// GetPlatformReleaseMonitorDependencies returns the monitor releases needed by the specified PlatformRelease
func (packages Packages) GetPlatformReleaseMonitorDependencies(release *PlatformRelease) ([]*ToolRelease, error) {
if release == nil {
return nil, fmt.Errorf(tr("release cannot be nil"))
}

res := []*ToolRelease{}
for _, monitor := range release.MonitorDependencies {
pkg, exists := packages[monitor.Packager]
if !exists {
return nil, fmt.Errorf(tr("package %s not found"), monitor.Packager)
}
tool, exists := pkg.Tools[monitor.Name]
if !exists {
return nil, fmt.Errorf(tr("tool %s not found"), monitor.Name)
}

// We always want to use the latest available release for monitors
latestRelease := tool.LatestRelease()
if latestRelease == nil {
return nil, fmt.Errorf(tr("can't find latest release of %s"), monitor.Name)
}
res = append(res, latestRelease)
}
return res, nil
}
Comment on lines +131 to +155
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is identical to GetPlatformReleaseDiscoveryDependencies apart from line 137, we could make generic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, also MonitorDependencies is identical to DiscoveryDependencies.

The problem is that release.MonitorDependencies is an array of MonitorDependecy... we can make it generic if we change the type of release.DiscoveryDependencies and release.MonitorDependencies to be the same type (for example reusingToolDependencies for both) but this change starts a chain reaction... I've another branch ready for that but I prefer to push it in another PR for review and not delay this one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right right, the type is different. Let's leave as it is now for the time being.


// GetOrCreatePlatform returns the Platform object with the specified architecture
// or creates a new one if not found
func (targetPackage *Package) GetOrCreatePlatform(architecture string) *Platform {
Expand Down
18 changes: 13 additions & 5 deletions i18n/data/en.po
Original file line number Diff line number Diff line change
Expand Up @@ -2398,6 +2398,7 @@ msgid "calling %[1]s: %[2]w"
msgstr "calling %[1]s: %[2]w"

#: arduino/cores/status.go:123
#: arduino/cores/status.go:150
msgid "can't find latest release of %s"
msgstr "can't find latest release of %s"

Expand Down Expand Up @@ -2771,7 +2772,7 @@ msgstr "invalid path creating config dir: %[1]s error: %[2]w"
msgid "invalid path writing inventory file: %[1]s error: %[2]w"
msgstr "invalid path writing inventory file: %[1]s error: %[2]w"

#: arduino/cores/packageindex/index.go:239
#: arduino/cores/packageindex/index.go:255
msgid "invalid platform archive size: %s"
msgstr "invalid platform archive size: %s"

Expand Down Expand Up @@ -2889,6 +2890,10 @@ msgstr "missing platform %[1]s:%[2]s referenced by board %[3]s"
msgid "missing platform release %[1]s:%[2]s referenced by board %[3]s"
msgstr "missing platform release %[1]s:%[2]s referenced by board %[3]s"

#: arduino/cores/packagemanager/package_manager.go:489
msgid "monitor release not found: %s"
msgstr "monitor release not found: %s"

#: arduino/libraries/librariesmanager/install.go:179
#: arduino/resources/install.go:94
msgid "moving extracted archive to destination dir: %s"
Expand Down Expand Up @@ -2958,14 +2963,15 @@ msgstr "opening target file: %s"
#: arduino/cores/packagemanager/download.go:73
#: arduino/cores/status.go:88
#: arduino/cores/status.go:113
#: arduino/cores/status.go:140
msgid "package %s not found"
msgstr "package %s not found"

#: arduino/cores/packagemanager/package_manager.go:259
msgid "package '%s' not found"
msgstr "package '%s' not found"

#: arduino/cores/status.go:167
#: arduino/cores/status.go:194
msgid "package not found"
msgstr "package not found"

Expand Down Expand Up @@ -3093,10 +3099,11 @@ msgstr "release %[1]s not found for tool %[2]s"

#: arduino/cores/status.go:82
#: arduino/cores/status.go:106
#: arduino/cores/status.go:133
msgid "release cannot be nil"
msgstr "release cannot be nil"

#: arduino/cores/status.go:183
#: arduino/cores/status.go:210
msgid "release not found"
msgstr "release not found"

Expand Down Expand Up @@ -3212,6 +3219,7 @@ msgstr "tool %s is not managed by package manager"

#: arduino/cores/status.go:92
#: arduino/cores/status.go:117
#: arduino/cores/status.go:144
msgid "tool %s not found"
msgstr "tool %s not found"

Expand All @@ -3223,7 +3231,7 @@ msgstr "tool '%[1]s' not found in package '%[2]s'"
msgid "tool not available for your OS"
msgstr "tool not available for your OS"

#: arduino/cores/status.go:171
#: arduino/cores/status.go:198
msgid "tool not found"
msgstr "tool not found"

Expand All @@ -3232,7 +3240,7 @@ msgid "tool not installed"
msgstr "tool not installed"

#: arduino/cores/packagemanager/package_manager.go:467
#: arduino/cores/packagemanager/package_manager.go:533
#: arduino/cores/packagemanager/package_manager.go:544
msgid "tool release not found: %s"
msgstr "tool release not found: %s"

Expand Down
14 changes: 7 additions & 7 deletions i18n/rice-box.go

Large diffs are not rendered by default.