Add compare and matching for OS features

Signed-off-by: Derek McGowan <[email protected]>

Author: dmcgowan

compare.go

@@ -218,9 +218,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
 }
 
@@ -247,9 +258,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
 }

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)
+					}
+				})
+			}
+		})
+	}
+}

platforms.go

@@ -114,6 +114,7 @@ import (
 	"path"
 	"regexp"
 	"runtime"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -141,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 {
 	return newDefaultMatcher(platform)
 }
@@ -151,9 +156,40 @@ 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.Variant == normalized.Variant {
+		if len(normalized.OSFeatures) == 0 {
+			return true
+		}
+		if len(m.OSFeatures) >= len(normalized.OSFeatures) {
+			// Ensure that normalized.OSFeatures is a subet of
+			// m.OSFeatures
+			j := 0
+			for _, feature := range normalized.OSFeatures {
+				for ; j < len(m.OSFeatures); j++ {
+					if feature == m.OSFeatures[j] {
+						// Don't increment j since the list is sorted
+						// but may contain duplicates
+						// TODO: Deduplicate list during normalize so
+						// that j can be incremented here
+						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 we hit the end, then feature was not found
+				if j == len(m.OSFeatures) {
+					return false
+				}
+			}
+			return true
+		}
+	}
+	return false
 }
 
 func (m *matcher) String() string {
@@ -311,6 +347,9 @@ 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 {
+		sort.Strings(platform.OSFeatures)
+	}
 
 	return platform
 }