Implement generic variant handling - #35
Conversation
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>
There was a problem hiding this comment.
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
Onlyto 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
win32kfeature 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.
| 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 |
There was a problem hiding this comment.
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")
There was a problem hiding this comment.
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
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
v4vsv3the same aspower10vspower8andrva23u64vsrva20u64.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"