Implement generic variant handling by tianon · Pull Request #35 · containerd/platforms

tianon · 2026-08-25T20:35:35Z

My old variant handling was creating explicit vector arrays, which doesn't scale well (algorithmically or maintainably).

This adjusts the logic to do more direct comparisons and handle "satisfiability" separate from "preferred order" which cleans up a lot of the comparisons.

This also makes it trivial to write a generic means of comparing variants that works for v4 vs v3 the same as power10 vs power8 and rva23u64 vs rva20u64.

As part of this work, it also folds in all the Windows-specific logic into a single matcher (because that's just standard platform matching logic that's naturally keyed off os, not a completely separate matcher).

It also genericizes the arm64 variant matching to the same vX.Y -> vX-1.Y+5 logic that Go uses.

See also https://github.com/opencontainers/image-spec/blob/v1.1.1/image-index.md#platform-variants

Assisted-By: "claude my eyes right out"

My old variant handling was creating explicit vector arrays, which doesn't scale well (algorithmically or maintainably). This adjusts the logic to do more direct comparisons and handle "satisfiability" separate from "preferred order" which cleans up a lot of the comparisons. This also makes it trivial to write a generic means of comparing variants that works for `v4` vs `v3` the same as `power10` vs `power8` and `rva23u64` vs `rva20u64`. As part of this work, it also folds in all the Windows-specific logic into a single matcher (because that's just standard platform matching logic that's naturally keyed off `os`, not a completely separate matcher). It also genericizes the arm64 variant matching to the same vX.Y -> vX-1.Y+5 logic that Go uses. Assisted-By: "claude my eyes right out" Signed-off-by: Tianon Gravi <tianon.gravi@docker.com>

Copilot

Pull request overview

This PR refactors platform variant compatibility and ordering to use generic parsing/comparison logic (instead of generating explicit variant vectors), adds generic “natural” ordering for variants/OS versions, and consolidates Windows OS version handling into the main matching path.

Changes:

Added a generic variant/version parser and comparison utilities, plus architecture-specific matching helpers (amd64/arm floors, arm64 v8↔v9 offset, generic “prefix+number+suffix” schemes).
Reworked Only to compute compatibility directly (Match) and sort independently (Less), avoiding unbounded vector generation for some variant schemes.
Folded Windows OS version matching into shared helpers and simplified OS feature subset logic (including win32k feature stripping behavior).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
variant.go	Introduces generic parsing/comparison utilities and matching helpers for versioned variants across architectures.
variant_test.go	Adds unit tests for variant parsing, matching helpers, and natural ordering.
platforms.go	Updates matcher behavior: factors OS version + OS feature subset logic into shared helpers and adjusts Windows matcher wrapping.
platform_windows_compat.go	Refactors Windows OS version parsing/matching to reuse the new generic parser.
defaults_test.go	Updates Windows MatchComparer test to explicitly wrap `NewMatcher` now that it returns only `Matcher`.
compare.go	Reimplements `Only` matching/sorting with direct compatibility checks and a natural sort-based ranking model.
compare_test.go	Expands test coverage for new arm64 arithmetic matching, generic variants (ppc64le/riscv64), and `Only`-specific ordering differences.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

tianon · 2026-08-25T21:42:37Z

 	if platform.OS == "windows" {
-		m.osvM = &windowsVersionMatcher{
-			windowsOSVersion: getWindowsOSVersion(platform.OSVersion),
-		}
-
 		// In prior versions, the win32k os feature was not considered for matching,
 		// strip out the win32k feature for comparison
-		var stripped Matcher = windowsStripFeaturesMatcher{m}
-
-		// In prior versions, on windows, the returned matcher implements a
-		// MatchComprarer interface.
-		// This preserves that behavior for backwards compatibility.
-		//
-		// TODO: This isn't actually used in this package, except for a test case,
-		// which may have been an unintended side of some refactor.
-		// It was likely intended to be used in `Ordered` but it is not since
-		// `Less` that is implemented here ends up getting masked due to wrapping.
-		if runtime.GOOS == "windows" {
-			return &windowsMatchComparer{stripped}
-		}
-		return stripped
+		return windowsStripFeaturesMatcher{m}
 	}
 	return m


https://github.com/search?q=%2FNewMatcher.*MatchComparer%2F&type=code is a somewhat naïve search, but it not having a single hit outside of this module itself (and obvious forks/copies of it) doesn't inspire confidence that this is an intentional interface that anyone's actually using/used -- how much does this concern really matter to maintainers? 😅

(I'm happy to spend more time on it, to be clear, but it seems silly to do so "just because")

A slightly better search that still returns the exact same set of files/repos: https://github.com/search?q=%2F%5B.%5D%5B%28%5DMatchComparer%5B%29%5D%2F&type=code

harryzcy

This is the great change overall

harryzcy · 2026-08-30T06:13:48Z

+	switch c.platform.Architecture {
+	case "amd64":
+		return true // 386 has no variant to check; it's a match-or-nothing fallback.
+	default: // arm64


Suggested change

default: // arm64

case "arm64":

Since it's arm64, it's better to match it explicitly

harryzcy · 2026-08-30T06:15:25Z

+	if c.platform.OS == "windows" {
+		// win32k is missing on Nano Server; ignore it for matching purposes.
+		if i := slices.Index(features, "win32k"); i >= 0 {
+			return slices.Delete(slices.Clone(features), i, i+1)


These code are duplicated with platform_windows_compat.go:169:

func (m windowsStripFeaturesMatcher) Match(p specs.Platform) bool { if i := slices.Index(p.OSFeatures, "win32k"); i >= 0 { p.OSFeatures = slices.Delete(slices.Clone(p.OSFeatures), i, i+1) } return m.Matcher.Match(p) }

harryzcy · 2026-08-30T20:17:50Z

+// Match and Less answer two different questions and are implemented
+// independently: Match asks "can this run here at all", and holds all the
+// compatibility logic — feature requirements, the arm64 cross-generation
+// offset, architecture fallbacks, and (on Windows) the stable-ABI version
+// window between host and container OS versions (see
+// checkWindowsHostAndContainerCompat) — computed directly (see variant.go)
+// rather than by generating and searching the set of every platform that
+// could be compatible, which would be unbounded for some architectures'
+// variant schemes. Less asks "how would these sort, biased towards this
+// host's own OS and architecture" and is a plain, match-independent sort
+// key.


I find this comment block quite verbose. Can we have a better wording for this?

harryzcy · 2026-08-30T20:21:04Z

+// For arm64/v9.x, will also match arm64/v9.{0..x-1} and arm64/v8.{0..x+5}
+// For arm64/v8.x, will also match arm64/v8.{0..x-1}
+// For arm/v8, will also match arm/v7, arm/v6 and arm/v5
+// For arm/v7, will also match arm/v6 and arm/v5
+// For arm/v6, will also match arm/v5


This is over emphasizing arm specific logic. This function used to only serve for arm so it was making sense. But now it's a lot more generic.

Should these comments go to arm64VariantMatch instead of here? and only leave one more two sentence summary in general term.

Or putting them in archMatch also makes sense.

harryzcy · 2026-08-30T20:26:49Z

+// sameShapeNumbers parses hostVariant and imageVariant and, if they share
+// the same prefix and suffix (differing only in their numbers), returns
+// both their numeric components for comparison.
+func sameShapeNumbers(hostVariant, imageVariant string) (hostNumbers, imageNumbers []int, ok bool) {


What's the behavior of variant with and without v prefix? are 8.3 and v8.3 treated differently? or should they get treated the same? I can see possibilities of user input error here.

It would also make sense to mandate certain pattern, like v is needed for arm64/* type. But the pattern may be different for different architecture types.

tianon-sso force-pushed the variants branch from e699a2d to 7997df6 Compare August 25, 2026 20:45

thaJeztah requested review from dmcgowan and a lite review from Copilot August 25, 2026 21:23

Copilot started reviewing on behalf of thaJeztah August 25, 2026 21:32 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

harryzcy reviewed Aug 30, 2026

View reviewed changes

Navigation Menu

Uh oh!

Implement generic variant handling - #35

Implement generic variant handling#35
tianon wants to merge 1 commit into
containerd:mainfrom
tianon-sso:variants

tianon commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

tianon Aug 25, 2026

Uh oh!

tianon Aug 25, 2026

Uh oh!

harryzcy left a comment

Uh oh!

harryzcy Aug 30, 2026

Uh oh!

harryzcy Aug 30, 2026

Uh oh!

harryzcy Aug 30, 2026

Uh oh!

harryzcy Aug 30, 2026

Uh oh!

harryzcy Aug 30, 2026

Uh oh!

harryzcy Aug 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

tianon commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harryzcy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants