diff --git a/compare.go b/compare.go index 24403f3..802b860 100644 --- a/compare.go +++ b/compare.go @@ -213,9 +213,20 @@ func (c orderedPlatformComparer) Less(p1 specs.Platform, p2 specs.Platform) bool return true } if p1m || p2m { + if p1m && p2m { + // Prefer one with most matching features + if len(p1.OSFeatures) != len(p2.OSFeatures) { + return len(p1.OSFeatures) > len(p2.OSFeatures) + } + } return false } } + if len(p1.OSFeatures) > 0 || len(p2.OSFeatures) > 0 { + p1.OSFeatures = nil + p2.OSFeatures = nil + return c.Less(p1, p2) + } return false } @@ -242,9 +253,20 @@ func (c anyPlatformComparer) Less(p1, p2 specs.Platform) bool { p2m = true } if p1m && p2m { - return false + if len(p1.OSFeatures) != len(p2.OSFeatures) { + return len(p1.OSFeatures) > len(p2.OSFeatures) + } + break } } + + // If neither match and has features, strip features and compare + if !p1m && !p2m && (len(p1.OSFeatures) > 0 || len(p2.OSFeatures) > 0) { + p1.OSFeatures = nil + p2.OSFeatures = nil + return c.Less(p1, p2) + } + // If one matches, and the other does, sort match first return p1m && !p2m } diff --git a/compare_test.go b/compare_test.go index ef5c2a1..a5fddc1 100644 --- a/compare_test.go +++ b/compare_test.go @@ -17,6 +17,8 @@ package platforms import ( + "reflect" + "sort" "testing" ) @@ -592,3 +594,94 @@ func TestOnlyStrict(t *testing.T) { }) } } + +func TestCompareOSFeatures(t *testing.T) { + for _, tc := range []struct { + platform string + platforms []string + expected []string + }{ + { + "linux/amd64", + []string{"windows/amd64", "linux/amd64", "linux(+other)/amd64", "linux/arm64"}, + []string{"linux/amd64", "linux(+other)/amd64", "windows/amd64", "linux/arm64"}, + }, + { + "linux(+none)/amd64", + []string{"windows/amd64", "linux/amd64", "linux/arm64", "linux(+other)/amd64"}, + []string{"linux/amd64", "linux(+other)/amd64", "windows/amd64", "linux/arm64"}, + }, + { + "linux(+other)/amd64", + []string{"windows/amd64", "linux/amd64", "linux/arm64", "linux(+other)/amd64"}, + []string{"linux(+other)/amd64", "linux/amd64", "windows/amd64", "linux/arm64"}, + }, + { + "linux(+af+other+zf)/amd64", + []string{"windows/amd64", "linux/amd64", "linux/arm64", "linux(+other)/amd64"}, + []string{"linux(+other)/amd64", "linux/amd64", "windows/amd64", "linux/arm64"}, + }, + { + "linux(+f1+f2)/amd64", + []string{"linux/amd64", "linux(+f2)/amd64", "linux(+f1)/amd64", "linux(+f1+f2)/amd64"}, + []string{"linux(+f1+f2)/amd64", "linux(+f2)/amd64", "linux(+f1)/amd64", "linux/amd64"}, + }, + { + // This test should likely fail and be updated when os version is considered for linux + "linux(7.2+other)/amd64", + []string{"linux/amd64", "linux(+other)/amd64", "linux(7.1)/amd64", "linux(7.2+other)/amd64"}, + []string{"linux(+other)/amd64", "linux(7.2+other)/amd64", "linux/amd64", "linux(7.1)/amd64"}, + }, + } { + testcase := tc + t.Run(testcase.platform, func(t *testing.T) { + t.Parallel() + p, err := Parse(testcase.platform) + if err != nil { + t.Fatal(err) + } + + for _, stc := range []struct { + name string + mc MatchComparer + }{ + { + name: "only", + mc: Only(p), + }, + { + name: "only strict", + mc: OnlyStrict(p), + }, + { + name: "ordered", + mc: Ordered(p), + }, + { + name: "any", + mc: Any(p), + }, + } { + mc := stc.mc + testcase := testcase + t.Run(stc.name, func(t *testing.T) { + p, err := ParseAll(testcase.platforms) + if err != nil { + t.Fatal(err) + } + sort.Slice(p, func(i, j int) bool { + return mc.Less(p[i], p[j]) + }) + actual := make([]string, len(p)) + for i, ps := range p { + actual[i] = FormatAll(ps) + } + + if !reflect.DeepEqual(testcase.expected, actual) { + t.Errorf("Wrong platform order:\nExpected: %#v\nActual: %#v", testcase.expected, actual) + } + }) + } + }) + } +} diff --git a/go.mod b/go.mod index d110efb..50c854c 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/containerd/platforms -go 1.20 +go 1.21 require ( github.com/containerd/log v0.1.0 diff --git a/platforms.go b/platforms.go index 14d65ab..289b56c 100644 --- a/platforms.go +++ b/platforms.go @@ -114,6 +114,7 @@ import ( "path" "regexp" "runtime" + "slices" "strconv" "strings" @@ -121,12 +122,10 @@ import ( ) var ( - specifierRe = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`) - osAndVersionRe = regexp.MustCompile(`^([A-Za-z0-9_-]+)(?:\(([A-Za-z0-9_.-]*)\))?$`) + specifierRe = regexp.MustCompile(`^[A-Za-z0-9_.-]+$`) + osRe = regexp.MustCompile(`^([A-Za-z0-9_-]+)(?:\(([A-Za-z0-9_.-]*)((?:\+[A-Za-z0-9_.-]+)*)\))?$`) ) -const osAndVersionFormat = "%s(%s)" - // Platform is a type alias for convenience, so there is no need to import image-spec package everywhere. type Platform = specs.Platform @@ -143,6 +142,10 @@ type Matcher interface { // functionality. // // Applications should opt to use `Match` over directly parsing specifiers. +// +// For OSFeatures, this matcher will match if the platform to match has +// OSFeatures which are a subset of the OSFeatures of the platform +// provided to NewMatcher. func NewMatcher(platform specs.Platform) Matcher { m := &matcher{ Platform: Normalize(platform), @@ -178,10 +181,39 @@ type matcher struct { func (m *matcher) Match(platform specs.Platform) bool { normalized := Normalize(platform) - return m.OS == normalized.OS && + if m.OS == normalized.OS && m.Architecture == normalized.Architecture && m.Variant == normalized.Variant && - m.matchOSVersion(platform) + m.matchOSVersion(platform) { + if len(normalized.OSFeatures) == 0 { + return true + } + if len(m.OSFeatures) >= len(normalized.OSFeatures) { + // Ensure that normalized.OSFeatures is a subset of + // m.OSFeatures + j := 0 + for _, feature := range normalized.OSFeatures { + found := false + for ; j < len(m.OSFeatures); j++ { + if feature == m.OSFeatures[j] { + found = true + j++ + break + } + // Since both lists are ordered, if the feature is less + // than what is seen, it is not in the list + if feature < m.OSFeatures[j] { + return false + } + } + if !found { + return false + } + } + return true + } + } + return false } func (m *matcher) matchOSVersion(platform specs.Platform) bool { @@ -210,11 +242,14 @@ func ParseAll(specifiers []string) ([]specs.Platform, error) { // Parse parses the platform specifier syntax into a platform declaration. // -// Platform specifiers are in the format `[()]||[()]/[/]`. +// Platform specifiers are in the format `[()]||[()]/[/]`. // The minimum required information for a platform specifier is the operating -// system or architecture. The OSVersion can be part of the OS like `windows(10.0.17763)` -// When an OSVersion is specified, then specs.Platform.OSVersion is populated with that value, -// and an empty string otherwise. +// system or architecture. The "os options" may be OSVersion which can be part of the OS +// like `windows(10.0.17763)`. When an OSVersion is specified, then specs.Platform.OSVersion is +// populated with that value, and an empty string otherwise. The "os options" may also include an +// array of OSFeatures, each feature prefixed with '+', without any other separator, and provided +// after the OSVersion when the OSVersion is specified. An "os options" with version and features +// is like `windows(10.0.17763+win32k)`. // If there is only a single string (no slashes), the // value will be matched against the known set of operating systems, then fall // back to the known set of architectures. The missing component will be @@ -231,14 +266,17 @@ func Parse(specifier string) (specs.Platform, error) { var p specs.Platform for i, part := range parts { if i == 0 { - // First element is [()] - osVer := osAndVersionRe.FindStringSubmatch(part) - if osVer == nil { - return specs.Platform{}, fmt.Errorf("%q is an invalid OS component of %q: OSAndVersion specifier component must match %q: %w", part, specifier, osAndVersionRe.String(), errInvalidArgument) + // First element is [([+]*)] + osOptions := osRe.FindStringSubmatch(part) + if osOptions == nil { + return specs.Platform{}, fmt.Errorf("%q is an invalid OS component of %q: OSAndVersion specifier component must match %q: %w", part, specifier, osRe.String(), errInvalidArgument) } - p.OS = normalizeOS(osVer[1]) - p.OSVersion = osVer[2] + p.OS = normalizeOS(osOptions[1]) + p.OSVersion = osOptions[2] + if osOptions[3] != "" { + p.OSFeatures = strings.Split(osOptions[3][1:], "+") + } } else { if !specifierRe.MatchString(part) { return specs.Platform{}, fmt.Errorf("%q is an invalid component of %q: platform specifier component must match %q: %w", part, specifier, specifierRe.String(), errInvalidArgument) @@ -322,8 +360,17 @@ func FormatAll(platform specs.Platform) string { return "unknown" } - if platform.OSVersion != "" { - OSAndVersion := fmt.Sprintf(osAndVersionFormat, platform.OS, platform.OSVersion) + osOptions := platform.OSVersion + features := platform.OSFeatures + if !slices.IsSorted(features) { + features = slices.Clone(features) + slices.Sort(features) + } + if len(features) > 0 { + osOptions += "+" + strings.Join(features, "+") + } + if osOptions != "" { + OSAndVersion := fmt.Sprintf("%s(%s)", platform.OS, osOptions) return path.Join(OSAndVersion, platform.Architecture, platform.Variant) } return path.Join(platform.OS, platform.Architecture, platform.Variant) @@ -336,6 +383,11 @@ func FormatAll(platform specs.Platform) string { func Normalize(platform specs.Platform) specs.Platform { platform.OS = normalizeOS(platform.OS) platform.Architecture, platform.Variant = normalizeArch(platform.Architecture, platform.Variant) + if len(platform.OSFeatures) > 0 { + platform.OSFeatures = slices.Clone(platform.OSFeatures) + slices.Sort(platform.OSFeatures) + platform.OSFeatures = slices.Compact(platform.OSFeatures) + } return platform } diff --git a/platforms_test.go b/platforms_test.go index 8a26f5c..c9e8326 100644 --- a/platforms_test.go +++ b/platforms_test.go @@ -343,6 +343,54 @@ func TestParseSelector(t *testing.T) { formatted: path.Join("windows(10.0.17763)", defaultArch, defaultVariant), useV2Format: true, }, + { + input: "windows(10.0.17763+win32k)", + expected: specs.Platform{ + OS: "windows", + OSVersion: "10.0.17763", + OSFeatures: []string{"win32k"}, + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("windows(10.0.17763+win32k)", defaultArch, defaultVariant), + useV2Format: true, + }, + { + input: "linux(+gpu)", + expected: specs.Platform{ + OS: "linux", + OSVersion: "", + OSFeatures: []string{"gpu"}, + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("linux(+gpu)", defaultArch, defaultVariant), + useV2Format: true, + }, + { + input: "linux(+gpu+simd)", + expected: specs.Platform{ + OS: "linux", + OSVersion: "", + OSFeatures: []string{"gpu", "simd"}, + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("linux(+gpu+simd)", defaultArch, defaultVariant), + useV2Format: true, + }, + { + input: "linux(+unsorted+erofs)", + expected: specs.Platform{ + OS: "linux", + OSVersion: "", + OSFeatures: []string{"unsorted", "erofs"}, + Architecture: defaultArch, + Variant: defaultVariant, + }, + formatted: path.Join("linux(+erofs+unsorted)", defaultArch, defaultVariant), + useV2Format: true, + }, } { t.Run(testcase.input, func(t *testing.T) { if testcase.skip {