Skip to content

Commit 1e7c7d1

Browse files
hellocn9hellocn9
committed
refactor(copy): Optimize platform copying logic to support multi-platform filtering
1 parent 8b9cbc3 commit 1e7c7d1

File tree

3 files changed

+89
-94
lines changed

3 files changed

+89
-94
lines changed

‎cmd/oras/internal/option/platform.go‎

Lines changed: 5 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -38,33 +38,25 @@ func (opts *Platform) ApplyFlags(fs *pflag.FlagSet) {
3838
if opts.FlagDescription == "" {
3939
opts.FlagDescription = "request platform"
4040
}
41-
fs.StringSliceVarP(&opts.platform, "platform", "", []string{}, opts.FlagDescription+" in the form of `os[/arch][/variant][:os_version]` or comma-separated list for multiple platforms")
41+
fs.StringSliceVarP(&opts.platform, "platform", "", nil, opts.FlagDescription+" in the form of `os[/arch][/variant][:os_version]` or comma-separated list for multiple platforms (supported in oras cp only)")
4242
}
4343

4444
// Parse parses the input platform flag to an oci platform type.
4545
func (opts *Platform) Parse(*cobra.Command) error {
4646
if len(opts.platform) == 0 {
4747
return nil
4848
}
49-
50-
if len(opts.platform) == 1 {
51-
// Single platform case - existing behavior
52-
return opts.parseSinglePlatform(opts.platform[0])
53-
}
54-
55-
return opts.parseMultiplePlatform(opts.platform)
49+
return opts.parsePlatform(opts.platform)
5650
}
5751

58-
// parseMultiplePlatform parses multiple platforms
59-
func (opts *Platform) parseMultiplePlatform(platformStrings []string) error {
60-
// Multiple platforms case
52+
// parsePlatform parses multiple platforms
53+
func (opts *Platform) parsePlatform(platformStrings []string) error {
6154
opts.Platforms = make([]*ocispec.Platform, 0, len(platformStrings))
6255
for _, platformStr := range platformStrings {
6356
platformStr = strings.TrimSpace(platformStr)
6457
if platformStr == "" {
6558
continue
6659
}
67-
6860
var p ocispec.Platform
6961
platformPart, osVersion, _ := strings.Cut(platformStr, ":")
7062
parts := strings.Split(platformPart, "/")
@@ -94,37 +86,6 @@ func (opts *Platform) parseMultiplePlatform(platformStrings []string) error {
9486
if len(opts.Platforms) > 0 {
9587
opts.Platform = opts.Platforms[0]
9688
}
97-
98-
return nil
99-
}
100-
101-
// parseSinglePlatform maintains the original parsing behavior for a single platform
102-
func (opts *Platform) parseSinglePlatform(platformStr string) error {
103-
// OS[/Arch[/Variant]][:OSVersion]
104-
// If Arch is not provided, will use GOARCH instead
105-
var platformPart string
106-
var p ocispec.Platform
107-
platformPart, p.OSVersion, _ = strings.Cut(platformStr, ":")
108-
parts := strings.Split(platformPart, "/")
109-
switch len(parts) {
110-
case 3:
111-
p.Variant = parts[2]
112-
fallthrough
113-
case 2:
114-
p.Architecture = parts[1]
115-
case 1:
116-
p.Architecture = runtime.GOARCH
117-
default:
118-
return fmt.Errorf("failed to parse platform %q: expected format os[/arch[/variant]]", platformStr)
119-
}
120-
p.OS = parts[0]
121-
if p.OS == "" {
122-
return fmt.Errorf("invalid platform: OS cannot be empty")
123-
}
124-
if p.Architecture == "" {
125-
return fmt.Errorf("invalid platform: Architecture cannot be empty")
126-
}
127-
opts.Platform = &p
12889
return nil
12990
}
13091

@@ -136,5 +97,5 @@ type ArtifactPlatform struct {
13697
// ApplyFlags applies flags to a command flag set.
13798
func (opts *ArtifactPlatform) ApplyFlags(fs *pflag.FlagSet) {
13899
opts.FlagDescription = "set artifact platform"
139-
fs.StringSliceVarP(&opts.platform, "artifact-platform", "", []string{}, "[Experimental] "+opts.FlagDescription+" in the form of `os[/arch][/variant][:os_version]`")
100+
fs.StringSliceVarP(&opts.platform, "artifact-platform", "", nil, "[Experimental] "+opts.FlagDescription+" in the form of `os[/arch][/variant][:os_version]`")
140101
}

‎cmd/oras/internal/option/platform_test.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import (
2727
func TestPlatform_ApplyFlags(t *testing.T) {
2828
var test struct{ Platform }
2929
ApplyFlags(&test, pflag.NewFlagSet("oras-test", pflag.ExitOnError))
30-
if test.platform != "" {
30+
if len(test.platform) != 0 {
3131
t.Fatalf("expecting platform to be empty but got: %v", test.platform)
3232
}
3333
}
@@ -37,11 +37,11 @@ func TestPlatform_Parse_err(t *testing.T) {
3737
name string
3838
opts *Platform
3939
}{
40-
{name: "empty arch 1", opts: &Platform{"os/", nil, ""}},
41-
{name: "empty arch 2", opts: &Platform{"os//variant", nil, ""}},
42-
{name: "empty os", opts: &Platform{"/arch", nil, ""}},
43-
{name: "empty os with variant", opts: &Platform{"/arch/variant", nil, ""}},
44-
{name: "trailing slash", opts: &Platform{"os/arch/variant/llama", nil, ""}},
40+
{name: "empty arch 1", opts: &Platform{platform: []string{"os/"}}},
41+
{name: "empty arch 2", opts: &Platform{platform: []string{"os//variant"}}},
42+
{name: "empty os", opts: &Platform{platform: []string{"/arch"}}},
43+
{name: "empty os with variant", opts: &Platform{platform: []string{"/arch/variant"}}},
44+
{name: "trailing slash", opts: &Platform{platform: []string{"os/arch/variant/llama"}}},
4545
}
4646
for _, tt := range tests {
4747
t.Run(tt.name, func(t *testing.T) {
@@ -60,13 +60,13 @@ func TestPlatform_Parse(t *testing.T) {
6060
opts *Platform
6161
want *ocispec.Platform
6262
}{
63-
{name: "empty", opts: &Platform{platform: ""}, want: nil},
64-
{name: "default arch", opts: &Platform{platform: "os"}, want: &ocispec.Platform{OS: "os", Architecture: runtime.GOARCH}},
65-
{name: "os&arch", opts: &Platform{platform: "os/aRcH"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH"}},
66-
{name: "empty variant", opts: &Platform{platform: "os/aRcH/"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: ""}},
67-
{name: "os&arch&variant", opts: &Platform{platform: "os/aRcH/vAriAnt"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: "vAriAnt"}},
68-
{name: "os version", opts: &Platform{platform: "os/aRcH/vAriAnt:osversion"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: "vAriAnt", OSVersion: "osversion"}},
69-
{name: "long os version", opts: &Platform{platform: "os/aRcH"}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH"}},
63+
{name: "empty", opts: &Platform{platform: []string{}}, want: nil},
64+
{name: "default arch", opts: &Platform{platform: []string{"os"}}, want: &ocispec.Platform{OS: "os", Architecture: runtime.GOARCH}},
65+
{name: "os&arch", opts: &Platform{platform: []string{"os/aRcH"}}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH"}},
66+
{name: "empty variant", opts: &Platform{platform: []string{"os/aRcH/"}}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: ""}},
67+
{name: "os&arch&variant", opts: &Platform{platform: []string{"os/aRcH/vAriAnt"}}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: "vAriAnt"}},
68+
{name: "os version", opts: &Platform{platform: []string{"os/aRcH/vAriAnt:osversion"}}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH", Variant: "vAriAnt", OSVersion: "osversion"}},
69+
{name: "long os version", opts: &Platform{platform: []string{"os/aRcH"}}, want: &ocispec.Platform{OS: "os", Architecture: "aRcH"}},
7070
}
7171
for _, tt := range tests {
7272
t.Run(tt.name, func(t *testing.T) {

‎cmd/oras/root/cp.go‎

Lines changed: 71 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -196,47 +196,81 @@ func copyMultiplePlatforms(ctx context.Context, statusHandler status.CopyHandler
196196
return fmt.Errorf("source reference %s is not an index or manifest list", opts.From.Reference)
197197
}
198198

199+
index, filteredManifests, err := filterManifestByPlatform(ctx, src, root, opts)
200+
if err != nil {
201+
return err
202+
}
203+
204+
// If all platforms are specified, we can just copy the root descriptor
205+
if len(index.Manifests) == len(filteredManifests) {
206+
opts.Platform.Platform = nil
207+
return copySinglePlatformOrRecursive(ctx, statusHandler, metadataHandler, src, dst, opts)
208+
}
209+
210+
// Perform multiple copies
211+
return doMultipleCopy(ctx, statusHandler, metadataHandler, src, dst, opts, index, filteredManifests)
212+
}
213+
214+
func filterManifestByPlatform(ctx context.Context, src oras.ReadOnlyGraphTarget, root ocispec.Descriptor, opts *copyOptions) (ocispec.Index, []ocispec.Descriptor, error) {
199215
// For indexes/lists, fetch the index content
200216
indexContent, err := content.FetchAll(ctx, src, root)
201217
if err != nil {
202-
return fmt.Errorf("failed to fetch index: %w", err)
218+
return ocispec.Index{}, nil, fmt.Errorf("failed to fetch index: %w", err)
203219
}
204220

205221
var index ocispec.Index
206-
if err := json.Unmarshal(indexContent, &index); err != nil {
207-
return fmt.Errorf("failed to parse index: %w", err)
222+
if err = json.Unmarshal(indexContent, &index); err != nil {
223+
return ocispec.Index{}, nil, fmt.Errorf("failed to parse index: %w", err)
208224
}
209225

210-
var availablePlatforms []string
211226
// Filter manifests based on the specified platforms
227+
var availablePlatforms []string
212228
var filteredManifests []ocispec.Descriptor
213-
matchedPlatforms := make(map[string]bool)
214229
for _, manifest := range index.Manifests {
215230
if manifest.Platform == nil {
216231
continue
217232
}
218-
availablePlatforms = append(availablePlatforms, fmt.Sprintf("%s/%s", manifest.Platform.OS, manifest.Platform.Architecture))
233+
var platformStr string
234+
if manifest.Platform.Variant != "" {
235+
platformStr = fmt.Sprintf("%s/%s/%s", manifest.Platform.OS, manifest.Platform.Architecture, manifest.Platform.Variant)
236+
} else {
237+
platformStr = fmt.Sprintf("%s/%s", manifest.Platform.OS, manifest.Platform.Architecture)
238+
}
239+
availablePlatforms = append(availablePlatforms, platformStr)
219240
if matchesAnyPlatform(manifest.Platform, opts.Platform.Platforms) {
220241
filteredManifests = append(filteredManifests, manifest)
221-
matchedPlatforms[fmt.Sprintf("%s/%s", manifest.Platform.OS, manifest.Platform.Architecture)] = true
222242
}
223243
}
224244

225-
if len(filteredManifests) != len(opts.Platform.Platforms) {
226-
227-
var unmatchedPlatforms []string
228-
for _, platform := range opts.Platform.Platforms {
229-
platformStr := fmt.Sprintf("%s/%s", platform.OS, platform.Architecture)
230-
if !matchedPlatforms[platformStr] {
231-
unmatchedPlatforms = append(unmatchedPlatforms, platformStr)
232-
}
245+
// Check if any platforms were unmatched
246+
var unmatchedPlatforms []string
247+
for _, platform := range opts.Platform.Platforms {
248+
var platformStr string
249+
var contains bool
250+
if platform.Variant != "" {
251+
platformStr = fmt.Sprintf("%s/%s/%s", platform.OS, platform.Architecture, platform.Variant)
252+
contains = slices.ContainsFunc(filteredManifests, func(manifest ocispec.Descriptor) bool {
253+
return manifest.Platform.OS == platform.OS && manifest.Platform.Architecture == platform.Architecture && manifest.Platform.Variant == platform.Variant
254+
})
255+
} else {
256+
platformStr = fmt.Sprintf("%s/%s", platform.OS, platform.Architecture)
257+
contains = slices.ContainsFunc(filteredManifests, func(manifest ocispec.Descriptor) bool {
258+
return manifest.Platform.OS == platform.OS && manifest.Platform.Architecture == platform.Architecture
259+
})
233260
}
234-
235-
// Return error with details about unmatched platforms
236-
return fmt.Errorf("only %d of %d requested platforms were matched: unmatched platforms: [%s]; available platforms in index: [%s]",
237-
len(filteredManifests), len(opts.Platform.Platforms), strings.Join(unmatchedPlatforms, ", "), strings.Join(availablePlatforms, ", "))
261+
if !contains {
262+
unmatchedPlatforms = append(unmatchedPlatforms, platformStr)
263+
}
264+
}
265+
// Return error with details about unmatched platforms
266+
if len(unmatchedPlatforms) > 0 {
267+
return ocispec.Index{}, nil, fmt.Errorf("some requested platforms were not matched; unmatched platforms: [%s]; available platforms in index: [%s]",
268+
strings.Join(unmatchedPlatforms, ", "), strings.Join(availablePlatforms, ", "))
238269
}
270+
return index, filteredManifests, nil
271+
}
239272

273+
func doMultipleCopy(ctx context.Context, statusHandler status.CopyHandler, metadataHandler metadata.CopyHandler, src oras.ReadOnlyGraphTarget, dst oras.GraphTarget, opts *copyOptions, index ocispec.Index, filteredManifests []ocispec.Descriptor) error {
240274
// Create a new index with only the filtered manifests
241275
newIndex := index
242276
newIndex.Manifests = filteredManifests
@@ -261,7 +295,6 @@ func copyMultiplePlatforms(ctx context.Context, statusHandler status.CopyHandler
261295
extendedCopyGraphOptions.FindPredecessors = func(ctx context.Context, src content.ReadOnlyGraphStorage, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
262296
return registry.Referrers(ctx, src, desc, "")
263297
}
264-
265298
if mountRepo, canMount := getMountPoint(src, dst, opts); canMount {
266299
extendedCopyGraphOptions.MountFrom = func(ctx context.Context, desc ocispec.Descriptor) ([]string, error) {
267300
return []string{mountRepo}, nil
@@ -282,19 +315,19 @@ func copyMultiplePlatforms(ctx context.Context, statusHandler status.CopyHandler
282315
// Copy all matching manifests and their content
283316
for _, manifestDesc := range filteredManifests {
284317
// Copy the manifest itself
285-
if err := oras.CopyGraph(ctx, src, dst, manifestDesc, extendedCopyGraphOptions.CopyGraphOptions); err != nil {
318+
if err = oras.CopyGraph(ctx, src, dst, manifestDesc, extendedCopyGraphOptions.CopyGraphOptions); err != nil {
286319
return fmt.Errorf("failed to copy manifest %s: %w", manifestDesc.Digest, err)
287320
}
288321
}
289322

290323
// Push the new index to the destination
291-
if err := dst.Push(ctx, newIndexDesc, strings.NewReader(string(newIndexContent))); err != nil {
324+
if err = dst.Push(ctx, newIndexDesc, strings.NewReader(string(newIndexContent))); err != nil {
292325
return fmt.Errorf("failed to push new index: %w", err)
293326
}
294327

295328
// Tag the new index if needed
296329
if opts.To.Reference != "" {
297-
if err := dst.Tag(ctx, newIndexDesc, opts.To.Reference); err != nil {
330+
if err = dst.Tag(ctx, newIndexDesc, opts.To.Reference); err != nil {
298331
return fmt.Errorf("failed to tag new index: %w", err)
299332
}
300333
}
@@ -313,37 +346,38 @@ func copyMultiplePlatforms(ctx context.Context, statusHandler status.CopyHandler
313346
if from, err := digest.Parse(opts.From.Reference); err == nil && from != newIndexDesc.Digest {
314347
opts.From.RawReference = fmt.Sprintf("%s@%s", opts.From.Path, newIndexDesc.Digest.String())
315348
}
316-
317-
if err := metadataHandler.OnCopied(&opts.BinaryTarget, newIndexDesc); err != nil {
349+
if err = metadataHandler.OnCopied(&opts.BinaryTarget, newIndexDesc); err != nil {
318350
return err
319351
}
320-
321352
return metadataHandler.Render()
322353
}
323354

324355
// matchesAnyPlatform checks if a manifest platform matches any of the specified platforms
325356
func matchesAnyPlatform(manifestPlatform *ocispec.Platform, platforms []*ocispec.Platform) bool {
326-
for _, platform := range platforms {
327-
if platformMatches(manifestPlatform, platform) {
328-
return true
329-
}
330-
}
331-
return false
357+
return slices.ContainsFunc(platforms, func(platform *ocispec.Platform) bool {
358+
return platformMatches(manifestPlatform, platform)
359+
})
332360
}
333361

334362
// platformMatches checks if two platforms match
335-
func platformMatches(a, b *ocispec.Platform) bool {
336-
if a.OS != b.OS || a.Architecture != b.Architecture {
363+
func platformMatches(manifestPlatform, targetPlatform *ocispec.Platform) bool {
364+
if manifestPlatform.OS != targetPlatform.OS || manifestPlatform.Architecture != targetPlatform.Architecture {
337365
return false
338366
}
339367

340-
// Variant is optional; only treat it as a mismatch if both variants are non-empty and different.
341-
if a.Variant != "" && b.Variant != "" && a.Variant != b.Variant {
368+
// Variant: optional; if specified, must match exactly, otherwise it is ignored
369+
//if targetPlatform.Variant == "" && manifestPlatform.Variant != "" {
370+
// return false
371+
//}
372+
if targetPlatform.Variant != "" && manifestPlatform.Variant == "" {
373+
return false
374+
}
375+
if targetPlatform.Variant != "" && manifestPlatform.Variant != "" && targetPlatform.Variant != manifestPlatform.Variant {
342376
return false
343377
}
344378

345379
// OSVersion is optional; only treat it as a mismatch if both OSVersions are non-empty and different.
346-
if a.OSVersion != "" && b.OSVersion != "" && a.OSVersion != b.OSVersion {
380+
if manifestPlatform.OSVersion != "" && targetPlatform.OSVersion != "" && manifestPlatform.OSVersion != targetPlatform.OSVersion {
347381
return false
348382
}
349383

0 commit comments

Comments
 (0)