Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}
Expand Down
93 changes: 93 additions & 0 deletions compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package platforms

import (
"reflect"
"sort"
"testing"
)

Expand Down Expand Up @@ -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)
}
})
}
})
}
}
42 changes: 40 additions & 2 deletions platforms.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,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),
Expand Down Expand Up @@ -177,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 {
Copy link
Member

Choose a reason for hiding this comment

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

iiuc this is backwards. normalized is the manifest platform, but manifest platforms should be subset of daemon/matcher platform, not the other way around.

Copy link
Member

Choose a reason for hiding this comment

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

Anyway, what I can think out is that I think we could differentiate which kinds of os.features we need to Match (For example, I can think out adding some prefix or a single-char annotation for example); Other os.features (including win32k or random previous garbage os.features generated by vendors) will not be considered as a part of Match.

But all os.features will be participate in sorting and formating.

Copy link
Member

@hsiangkao hsiangkao Mar 26, 2026

Choose a reason for hiding this comment

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

Otherwise, if no Match functionality for some erofs os.features for example, it will make such os.features just another random flag, and almost useless except another kind of a comment for human beings somewhere.

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 {
Expand Down Expand Up @@ -350,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
}
Loading