Skip to content

Conversation

@mirkoCrobu
Copy link
Contributor

Motivation

closes #738
The logic that updates the platform should make sure we do not upgrade above the 1.x.x release.
In other words there should be a variable specifying the maximum major version that will be reported as an update.

This is to avoid auto-updating to a breaking change.>

Change description

Additional Notes

Reviewer checklist

  • PR addresses a single concern.
  • PR title and description are properly filled.
  • Changes will be merged in main.
  • Changes are covered by tests.
  • Logging is meaningful in case of troubleshooting.

@mirkoCrobu mirkoCrobu self-assigned this Dec 10, 2025
@mirkoCrobu mirkoCrobu added the enhancement New feature or request label Dec 10, 2025
@mirkoCrobu mirkoCrobu requested a review from a team December 10, 2025 16:18
Name: "arduino:zephyr",
FromVersion: platformSummary.GetInstalledVersion(),
ToVersion: platformSummary.GetLatestVersion(),
ToVersion: bestVersion,
Copy link
Contributor

Choose a reason for hiding this comment

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

it doesn't seem that this ToVersion is used in the upgrade function, so I think we always upgrade to the latest version

if err != nil {
return Configuration{}, fmt.Errorf("invalid LIBRARIES_API_URL: %w", err)
}
maxVersionStr := os.Getenv("ARDUINO_APP_CLI__MAX_UPDATE_MAJOR_VERSION")
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make this a version constraint string, parsable with relaxed-semver https://pkg.go.dev/go.bug.st/relaxed-semver#readme-version-constraints. For instance, the default value should be <1.0.0, which will match anything less that isn't 1.0.0

Comment on lines +148 to +183
func findBestCandidate(installedStr string, availableVersions []string, maxMajorConfig int) (string, error) {
installedV, err := semver.NewVersion(installedStr)
if err != nil {
return "", err
}

maxMajor := uint64(maxMajorConfig)
if maxMajorConfig <= 0 {
maxMajor = installedV.Major()
}

var bestUpdateV *semver.Version

for _, vStr := range availableVersions {
candidateV, err := semver.NewVersion(vStr)
if err != nil {
continue
}

if candidateV.Major() > maxMajor {
continue
}

if !candidateV.GreaterThan(installedV) {
continue
}
if bestUpdateV == nil || candidateV.GreaterThan(bestUpdateV) {
bestUpdateV = candidateV
}
}

if bestUpdateV == nil {
return "", nil
}
return bestUpdateV.Original(), nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use the constraint in a way like this

Suggested change
func findBestCandidate(installedStr string, availableVersions []string, maxMajorConfig int) (string, error) {
installedV, err := semver.NewVersion(installedStr)
if err != nil {
return "", err
}
maxMajor := uint64(maxMajorConfig)
if maxMajorConfig <= 0 {
maxMajor = installedV.Major()
}
var bestUpdateV *semver.Version
for _, vStr := range availableVersions {
candidateV, err := semver.NewVersion(vStr)
if err != nil {
continue
}
if candidateV.Major() > maxMajor {
continue
}
if !candidateV.GreaterThan(installedV) {
continue
}
if bestUpdateV == nil || candidateV.GreaterThan(bestUpdateV) {
bestUpdateV = candidateV
}
}
if bestUpdateV == nil {
return "", nil
}
return bestUpdateV.Original(), nil
}
func findBestCandidate(availableVersions []string, constraint semver.Constraint) (string, error) {
releases := make([]*semver.Version, 0, len(availableVersions))
for _, verStr := range availableVersions{
if ver, err := semver.NewVersion(installedStr); err == nil {
if constraint.Match(ver){
releases = append(releases, ver)
}
}
}
if len(releases) == 0 {
return "", nil
}
slices.SortStableFunc(releases, func(a, b *semver.Version) int {
return a.Compare(b)
})
return releases[len(releases)-1].String(), nil
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants