Skip to content

Commit

Permalink
distrosort: add new distrosort.Names helper
Browse files Browse the repository at this point in the history
This commit adds a new `distrosort.Names()` helper that can be
used sort sort distro names. It will take version numbers of
the distro into account and sort by semantic version. This
is useful in e.g. user-interfaces where we want to present
the version in a nicely sorted way, e.g. a version like
`1.4` is sorted before `1.10` (which a `string.Sort()` gets
wrong).

Sorting follows the go-version library which supports semantic
versioning [0] so anything supported there will be sorted
correctly including `-beta` suffixes and similar (see the
spec for details). However this does not work today because
the underlying distroid parser does only support major/minor.
But once this is fixed full sort will work.

Note that this is an extra pkg because the `distro` and `distroid`
would be circular imports otherwise. The alternative might be to
create a distro.ID package.

[0] https://semver.org/
  • Loading branch information
mvo5 committed Nov 6, 2024
1 parent 5a54cb7 commit 9c44baf
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 10 deletions.
18 changes: 8 additions & 10 deletions pkg/distro/id.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,20 @@ type ID struct {
MinorVersion int
}

func (id ID) String() string {
func (id ID) versionString() string {
if id.MinorVersion == -1 {
return fmt.Sprintf("%s-%d", id.Name, id.MajorVersion)
return fmt.Sprintf("%d", id.MajorVersion)
} else {
return fmt.Sprintf("%d.%d", id.MajorVersion, id.MinorVersion)
}
}

return fmt.Sprintf("%s-%d.%d", id.Name, id.MajorVersion, id.MinorVersion)
func (id ID) String() string {
return fmt.Sprintf("%s-%s", id.Name, id.versionString())
}

func (id ID) Version() (*version.Version, error) {
var verStr string
if id.MinorVersion == -1 {
verStr = fmt.Sprintf("%d", id.MajorVersion)
} else {
verStr = fmt.Sprintf("%d.%d", id.MajorVersion, id.MinorVersion)
}
return version.NewVersion(verStr)
return version.NewVersion(id.versionString())
}

type ParseError struct {
Expand Down
53 changes: 53 additions & 0 deletions pkg/distrosort/sort.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package distrosort

import (
"errors"
"sort"

"github.com/osbuild/images/pkg/distroidparser"
)

// Names sorts the given list of distro names by name, version
// taking version semantics into account (i.e. sorting 8.1 lower then
// 8.10).
//
// Invalid version numbers will create errors but the sorting continue
// and invalid numbers are sorted lower than anything else (so the
// result is still usable in a {G,T}UI).
//
// Note that full semantic versioning (see semver.org) is not
// supported today but it would be once the underlying distroid parser
// supports better spliting.
func Names(distros []string) error {
var errs []error

parser := distroidparser.NewDefaultParser()
sort.Slice(distros, func(i, j int) bool {
id1, err := parser.Parse(distros[i])
if err != nil {
errs = append(errs, err)
return true
}
id2, err := parser.Parse(distros[j])
if err != nil {
errs = append(errs, err)
return true
}
if id1.Name != id2.Name {
return id1.Name < id2.Name
}
ver1, err := id1.Version()
if err != nil {
errs = append(errs, err)
return true
}
ver2, err := id2.Version()
if err != nil {
errs = append(errs, err)
return true
}

return ver1.LessThan(ver2)
})
return errors.Join(errs...)
}
58 changes: 58 additions & 0 deletions pkg/distrosort/sort_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package distrosort_test

import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/osbuild/images/pkg/distrosort"
)

func TestSortNames(t *testing.T) {
for _, tc := range []struct {
inp []string
sorted []string
}{
{
// distro names are sorted by first
[]string{"foo-2", "bar-1", "foo-1"},
[]string{"bar-1", "foo-1", "foo-2"},
}, {
// multiple "-" are okay
[]string{"foo-bar-2", "bar-foo-1", "foo-bar-1"},
[]string{"bar-foo-1", "foo-bar-1", "foo-bar-2"},
}, {
// 1.4 is smaller than 1.10, sort.Strings will get this
// wrong
[]string{"foo-1.10", "foo-1.4"},
[]string{"foo-1.4", "foo-1.10"},
},
} {
err := distrosort.Names(tc.inp)
assert.NoError(t, err, tc.inp)
assert.Equal(t, tc.sorted, tc.inp)
}
}

func TestSortNamesInvalidVersion(t *testing.T) {
for _, tc := range []struct {
inp []string
expectedErr string
}{
{
[]string{"foo-1.x", "foo-2"},
"error when parsing distro name (foo-1.x): parsing minor version failed, inner error:\nstrconv.Atoi: parsing \"x\": invalid syntax",
}, {
// missing "-" is not supported
[]string{"foo", "bar-1"},
`error when parsing distro name (foo): A dash is expected to separate distro name and version`,
}, {
// foo-1.4-beta is not supported
[]string{"foo-1.4", "foo-1.4-beta", "foo-1.0"},
"error when parsing distro name (foo-1.4-beta): parsing major version failed, inner error:\nstrconv.Atoi: parsing \"beta\": invalid syntax",
},
} {
err := distrosort.Names(tc.inp)
assert.ErrorContains(t, err, tc.expectedErr, tc.inp)
}
}

0 comments on commit 9c44baf

Please sign in to comment.