From 9c44baf82045193ef7fb7bd210c2205905c7807e Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 28 Oct 2024 11:08:16 +0100 Subject: [PATCH] distrosort: add new `distrosort.Names` helper 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/ --- pkg/distro/id.go | 18 +++++------- pkg/distrosort/sort.go | 53 +++++++++++++++++++++++++++++++++ pkg/distrosort/sort_test.go | 58 +++++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 10 deletions(-) create mode 100644 pkg/distrosort/sort.go create mode 100644 pkg/distrosort/sort_test.go diff --git a/pkg/distro/id.go b/pkg/distro/id.go index 1cced4b9e2..914b859d56 100644 --- a/pkg/distro/id.go +++ b/pkg/distro/id.go @@ -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 { diff --git a/pkg/distrosort/sort.go b/pkg/distrosort/sort.go new file mode 100644 index 0000000000..c1a20d3c56 --- /dev/null +++ b/pkg/distrosort/sort.go @@ -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...) +} diff --git a/pkg/distrosort/sort_test.go b/pkg/distrosort/sort_test.go new file mode 100644 index 0000000000..5cade9dd62 --- /dev/null +++ b/pkg/distrosort/sort_test.go @@ -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) + } +}