platforms: use full platform identity consistently

build/resolver/driver_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/containerd/platforms"
 	"github.com/docker/buildx/builder"
+	"github.com/docker/buildx/util/platformutil"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/require"
 )
@@ -300,6 +301,62 @@ func TestSplitNodeMultiPlatformNoUnify(t *testing.T) {
 	require.Equal(t, "builder-amd64-riscv64", res[1].Node().Builder)
 }
 
+func TestSelectNodeWindowsOSVersion(t *testing.T) {
+	r := makeTestResolver(map[string][]ocispecs.Platform{
+		"builder-ltsc2019": {platforms.MustParse("windows(10.0.17763)/amd64")},
+		"builder-ltsc2022": {platforms.MustParse("windows(10.0.20348)/amd64")},
+	})
+
+	res, perfect, err := r.resolve(context.TODO(), []ocispecs.Platform{platforms.MustParse("windows(10.0.17763)/amd64")}, nil, platforms.Only, nil)
+	require.NoError(t, err)
+	require.True(t, perfect)
+	require.Len(t, res, 1)
+	require.Equal(t, "builder-ltsc2019", res[0].Node().Builder)
+
+	res, perfect, err = r.resolve(context.TODO(), []ocispecs.Platform{platforms.MustParse("windows(10.0.20348)/amd64")}, nil, platforms.Only, nil)
+	require.NoError(t, err)
+	require.True(t, perfect)
+	require.Len(t, res, 1)
+	require.Equal(t, "builder-ltsc2022", res[0].Node().Builder)
+
+	// no node runs a release new enough for this image
+	_, perfect, err = r.resolve(context.TODO(), []ocispecs.Platform{platforms.MustParse("windows(10.0.26100)/amd64")}, nil, platforms.Only, nil)
+	require.NoError(t, err)
+	require.False(t, perfect)
+}
+
+func TestSelectNodeAdditionalPlatformsOSVersion(t *testing.T) {
+	// mirrors what Resolve does with the platforms reported by the workers of a
+	// single node: entries that only differ by OS version have to survive the
+	// dedupe, otherwise only one windows release can ever be matched
+	additional := func(int, builder.Node) []ocispecs.Platform {
+		return platformutil.Dedupe([]ocispecs.Platform{
+			platforms.MustParse("windows(10.0.17763)/amd64"),
+			platforms.MustParse("windows(10.0.20348)/amd64"),
+		})
+	}
+
+	for _, tc := range []struct {
+		name     string
+		platform string
+	}{
+		{"ltsc2019", "windows(10.0.17763)/amd64"},
+		{"ltsc2022", "windows(10.0.20348)/amd64"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			r := makeTestResolver(map[string][]ocispecs.Platform{
+				"builder-win": {platforms.MustParse("linux/amd64")},
+			})
+
+			res, perfect, err := r.resolve(context.TODO(), []ocispecs.Platform{platforms.MustParse(tc.platform)}, nil, platforms.Only, additional)
+			require.NoError(t, err)
+			require.True(t, perfect)
+			require.Len(t, res, 1)
+			require.Equal(t, "builder-win", res[0].Node().Builder)
+		})
+	}
+}
+
 func makeTestResolver(nodes map[string][]ocispecs.Platform) *nodeResolver {
 	var ns []builder.Node
 	for name, platforms := range nodes {

builder/node.go

@@ -6,7 +6,6 @@ import (
 	"sort"
 	"strings"
 
-	"github.com/containerd/platforms"
 	"github.com/docker/buildx/driver"
 	"github.com/docker/buildx/store"
 	"github.com/docker/buildx/store/storeutil"
@@ -218,10 +217,7 @@ func (n *Node) MarshalJSON() ([]byte, error) {
 		status = "error"
 		nerr = strings.TrimSpace(n.Err.Error())
 	}
-	var pp []string
-	for _, p := range n.Platforms {
-		pp = append(pp, platforms.Format(p))
-	}
+	pp := platformutil.Format(n.Platforms)
 	return json.Marshal(struct {
 		Name           string
 		Endpoint       string

builder/node_test.go

@@ -0,0 +1,47 @@
+package builder
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/containerd/platforms"
+	"github.com/docker/buildx/store"
+	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNodeMarshalJSONPlatforms(t *testing.T) {
+	t.Parallel()
+
+	n := Node{
+		Node: store.Node{Name: "n0", Endpoint: "docker-container://buildx_buildkit_n0"},
+		Platforms: []ocispecs.Platform{
+			platforms.MustParse("windows(10.0.17763)/amd64"),
+			platforms.MustParse("windows(10.0.20348)/amd64"),
+			platforms.MustParse("windows(10.0.20348+win32k)/amd64"),
+			platforms.MustParse("linux/x86_64"),
+		},
+	}
+
+	dt, err := json.Marshal(&n)
+	require.NoError(t, err)
+
+	var out struct {
+		Platforms []string
+	}
+	require.NoError(t, json.Unmarshal(dt, &out))
+	require.Equal(t, []string{
+		"windows(10.0.17763)/amd64",
+		"windows(10.0.20348)/amd64",
+		"windows(10.0.20348+win32k)/amd64",
+		"linux/amd64",
+	}, out.Platforms)
+}
+
+func TestNodeMarshalJSONNoPlatforms(t *testing.T) {
+	t.Parallel()
+
+	dt, err := json.Marshal(&Node{Node: store.Node{Name: "n0"}})
+	require.NoError(t, err)
+	require.NotContains(t, string(dt), "Platforms")
+}

store/nodegroup.go

@@ -6,7 +6,6 @@ import (
 	"slices"
 	"time"
 
-	"github.com/containerd/platforms"
 	"github.com/docker/buildx/util/confutil"
 	"github.com/docker/buildx/util/platformutil"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
@@ -176,7 +175,7 @@ func (ng *NodeGroup) validateDuplicates(ep string, idx int) error {
 
 	m := map[string]struct{}{}
 	for _, p := range ng.Nodes[idx].Platforms {
-		m[platforms.FormatAll(platforms.Normalize(p))] = struct{}{}
+		m[platformutil.Key(p)] = struct{}{}
 	}
 
 	for i := range ng.Nodes {
@@ -213,7 +212,7 @@ func (ng *NodeGroup) nextNodeName() string {
 func filterPlatforms(in []ocispecs.Platform, m map[string]struct{}) []ocispecs.Platform {
 	out := make([]ocispecs.Platform, 0, len(in))
 	for _, p := range in {
-		if _, ok := m[platforms.FormatAll(platforms.Normalize(p))]; !ok {
+		if _, ok := m[platformutil.Key(p)]; !ok {
 			out = append(out, p)
 		}
 	}

store/nodegroup_test.go

@@ -3,9 +3,7 @@ package store
 import (
 	"testing"
 
-	"github.com/containerd/platforms"
 	"github.com/docker/buildx/util/platformutil"
-	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/require"
 )
 
@@ -55,14 +53,6 @@ func TestNodeGroupUpdateFiltersOnlyMatchingPlatforms(t *testing.T) {
 	err = ng.Update("n2", "ctx-b", []string{"windows(10.0.20348)/amd64", "linux/x86_64"}, true, true, nil, "", nil)
 	require.NoError(t, err)
 
-	require.Equal(t, []string{"windows(10.0.17763)/amd64"}, formatAll(ng.Nodes[0].Platforms))
-	require.Equal(t, []string{"windows(10.0.20348)/amd64", "linux/amd64"}, formatAll(ng.Nodes[1].Platforms))
-}
-
-func formatAll(pp []ocispecs.Platform) []string {
-	out := make([]string, 0, len(pp))
-	for _, p := range pp {
-		out = append(out, platforms.FormatAll(p))
-	}
-	return out
+	require.Equal(t, []string{"windows(10.0.17763)/amd64"}, platformutil.Format(ng.Nodes[0].Platforms))
+	require.Equal(t, []string{"windows(10.0.20348)/amd64", "linux/amd64"}, platformutil.Format(ng.Nodes[1].Platforms))
 }

util/platformutil/parse.go

@@ -38,12 +38,19 @@ func parse(in string) (ocispecs.Platform, error) {
 	return platforms.Parse(in)
 }
 
+// Key returns the canonical string identifying a platform. Unlike
+// [platforms.Format] it keeps the OS version and OS features so that Windows
+// platforms from different releases do not collapse into each other.
+func Key(p ocispecs.Platform) string {
+	return platforms.FormatAll(platforms.Normalize(p))
+}
+
 func Dedupe(in []ocispecs.Platform) []ocispecs.Platform {
 	m := map[string]struct{}{}
 	out := make([]ocispecs.Platform, 0, len(in))
 	for _, p := range in {
 		p := platforms.Normalize(p)
-		key := platforms.FormatAll(p)
+		key := Key(p)
 		if _, ok := m[key]; ok {
 			continue
 		}
@@ -58,7 +65,7 @@ func FormatInGroups(gg ...[]ocispecs.Platform) []string {
 	out := make([]string, 0, len(gg))
 	for i, g := range gg {
 		for _, p := range g {
-			v := platforms.FormatAll(platforms.Normalize(p))
+			v := Key(p)
 			if _, ok := m[v]; ok {
 				continue
 			}
@@ -78,7 +85,7 @@ func Format(in []ocispecs.Platform) []string {
 	}
 	out := make([]string, 0, len(in))
 	for _, p := range in {
-		out = append(out, platforms.Format(p))
+		out = append(out, Key(p))
 	}
 	return out
 }

util/platformutil/parse_test.go

@@ -25,7 +25,7 @@ func TestDedupePreservesOSVersionAndFeatures(t *testing.T) {
 		"windows(10.0.20348)/amd64",
 		"windows(10.0.20348+win32k)/amd64",
 		"linux/amd64",
-	}, formatAll(got))
+	}, Format(got))
 }
 
 func TestFormatInGroupsPreservesOSVersionAndFeatures(t *testing.T) {
@@ -52,10 +52,20 @@ func TestFormatInGroupsPreservesOSVersionAndFeatures(t *testing.T) {
 	}, got)
 }
 
-func formatAll(pp []ocispecs.Platform) []string {
-	out := make([]string, 0, len(pp))
-	for _, p := range pp {
-		out = append(out, platforms.FormatAll(p))
-	}
-	return out
+func TestFormatPreservesOSVersionAndFeatures(t *testing.T) {
+	t.Parallel()
+
+	require.Equal(t, []string{
+		"windows(10.0.17763)/amd64",
+		"windows(10.0.20348+win32k)/amd64",
+		"linux/amd64",
+		"linux/arm/v7",
+	}, Format([]ocispecs.Platform{
+		platforms.MustParse("windows(10.0.17763)/amd64"),
+		platforms.MustParse("windows(10.0.20348+win32k)/amd64"),
+		platforms.MustParse("linux/x86_64"),
+		platforms.MustParse("linux/arm/v7"),
+	}))
+
+	require.Nil(t, Format(nil))
 }