Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support platform selection on Copy #244

Merged
merged 8 commits into from
Aug 8, 2022

Conversation

lizMSFT
Copy link
Contributor

@lizMSFT lizMSFT commented Jul 20, 2022

Support platform selection on Copy by adding AddPlatformFilter func that configure the MapRoot option for clients.

Platform property ref: OCI Image Index Specification

The given platform matches the target platform if all of the below conditions are met:

  • Architecture and OS exactly match.
  • Variant and OSVersion exactly match if target platform provided.
  • OSFeatures of the target platform are the subsets of the OSFeatures array of the given platform.
  • Features property is reserved for future versions of the specification, will skip this for now. ref: descriptor.go

Note: Variant, OSVersion, OSFeatures and Features are optional fields, will skip the comparison if the target platform does not provide specfic value.

If multiple manifests match a client or runtime's requirements, the first matching entry will be returned.

Resolves #210

Signed-off-by: REDMOND\zoeyli zoeyli@microsoft.com

internal/platform/platform.go Outdated Show resolved Hide resolved
internal/platform/platform.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy.go Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
internal/platform/platform.go Outdated Show resolved Hide resolved
internal/platform/platform.go Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
internal/platform/platform.go Outdated Show resolved Hide resolved
@qweeah
Copy link
Contributor

qweeah commented Jul 20, 2022

NIT: function comments should alway ends with a dot. https://tip.golang.org/doc/comment#Funcs

internal/platform/platform.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy_test.go Outdated Show resolved Hide resolved
copy_test.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
@lizMSFT lizMSFT force-pushed the liz/210 branch 3 times, most recently from 6cbedba to 29bf7e4 Compare July 21, 2022 03:15
@qweeah qweeah mentioned this pull request Jul 21, 2022
copy.go Outdated Show resolved Hide resolved
copy_test.go Outdated Show resolved Hide resolved
content.go Outdated Show resolved Hide resolved
content.go Show resolved Hide resolved
content.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy_test.go Show resolved Hide resolved
content.go Outdated Show resolved Hide resolved
internal/docker/mediatype.go Outdated Show resolved Hide resolved
content.go Outdated Show resolved Hide resolved
content.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
@lizMSFT lizMSFT force-pushed the liz/210 branch 3 times, most recently from 3557557 to 6aabee8 Compare August 3, 2022 17:30
@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2022

Codecov Report

Merging #244 (c98f3d9) into main (588379d) will increase coverage by 0.18%.
The diff coverage is 75.92%.

@@            Coverage Diff             @@
##             main     #244      +/-   ##
==========================================
+ Coverage   70.50%   70.68%   +0.18%     
==========================================
  Files          35       36       +1     
  Lines        3109     3217     +108     
==========================================
+ Hits         2192     2274      +82     
- Misses        682      700      +18     
- Partials      235      243       +8     
Impacted Files Coverage Δ
content.go 56.25% <63.33%> (+11.80%) ⬆️
copy.go 60.78% <72.22%> (+3.07%) ⬆️
internal/platform/platform.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

copy.go Outdated Show resolved Hide resolved
content.go Outdated Show resolved Hide resolved
content.go Show resolved Hide resolved
copy.go Outdated Show resolved Hide resolved
@lizMSFT lizMSFT force-pushed the liz/210 branch 2 times, most recently from c0c03fc to 8be0e77 Compare August 4, 2022 08:01
content.go Outdated Show resolved Hide resolved
Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

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

LGTM

@lizMSFT lizMSFT force-pushed the liz/210 branch 3 times, most recently from aa3013c to e2af366 Compare August 5, 2022 06:59
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

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

LGTM with suggestions on documentation

copy.go Outdated Show resolved Hide resolved
Signed-off-by: REDMOND\zoeyli <zoeyli@microsoft.com>
…g error instead of ErrNotFound]

Signed-off-by: Zoey Li <zoeyli@microsoft.com>
…add test]

Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

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

LGTM

@Wwwsylvia Wwwsylvia merged commit e24e247 into oras-project:main Aug 8, 2022
return false
}

set := make(map[string]bool)

Choose a reason for hiding this comment

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

Looks like you want to use it as a set. using struct{} instead of bool can save memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support platform selection on Copy
8 participants