Skip to content

Conversation

wangxiaoxuan273
Copy link
Contributor

@wangxiaoxuan273 wangxiaoxuan273 commented Jun 4, 2025

What this PR does / why we need it:

Added the following test case to the E2E test suite. On copying v.1.3.8, the expected behavior would be that all six nodes in the graph are copied, including the index, two images and three referrers.
image

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1728

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

Xiaoxuan Wang added 4 commits June 4, 2025 10:55
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Copy link

codecov bot commented Jun 4, 2025

Codecov Report

Attention: Patch coverage is 91.42857% with 3 lines in your changes missing coverage. Please review.

Project coverage is 85.61%. Comparing base (2d31c5f) to head (3679831).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/oras/root/cp.go 91.42% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1749      +/-   ##
==========================================
+ Coverage   85.58%   85.61%   +0.03%     
==========================================
  Files         137      137              
  Lines        5993     6006      +13     
==========================================
+ Hits         5129     5142      +13     
  Misses        613      613              
  Partials      251      251              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Xiaoxuan Wang added 5 commits June 5, 2025 12:51
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
@wangxiaoxuan273 wangxiaoxuan273 marked this pull request as ready for review June 5, 2025 05:40
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
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 with minor suggestions.

@qweeah
Copy link
Contributor

qweeah commented Jun 5, 2025

IMO fix here is still hacky. But it's good enough for a bug fix to unblock users.

Implementations in this PR are duplicated to SDK-side work like finding DAG roots. Would be ideal to fix via oras-project/oras-go#957.

Xiaoxuan Wang added 2 commits June 9, 2025 11:05
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Xiaoxuan Wang added 7 commits June 13, 2025 13:05
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Xiaoxuan Wang added 6 commits July 1, 2025 17:39
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Xiaoxuan Wang added 2 commits July 2, 2025 15:28
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Xiaoxuan Wang added 4 commits July 10, 2025 14:33
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@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 with a minor suggestion

Xiaoxuan Wang added 2 commits July 14, 2025 12:57
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@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

@shizhMSFT shizhMSFT requested a review from Copilot July 14, 2025 09:10
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Ensures oras cp -r includes the root index when it has no referrers by adjusting the copy entrypoint and expanding tests.

  • Change prepareCopyOption to return a custom entrypoint (copyRoot) for indexes without referrers.
  • Update recursiveCopy to invoke ExtendedCopyGraph on the returned copyRoot and properly tag the original index.
  • Add E2E tests, testdata JSON, and diagram updates for both registry and OCI layout scenarios.

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
cmd/oras/root/cp.go Extend prepareCopyOption to return copyRoot and update recursiveCopy logic
cmd/oras/root/cp_test.go Update tests to handle the new prepareCopyOption signature
test/e2e/testdata/zot/command/artifacts/index.json Expand index manifests and annotations for the new test case
test/e2e/suite/command/cp.go Add E2E tests for copying root index and its referrers in registry and OCI layout
test/e2e/README.md Update ASCII diagram to reflect the new index and referrer relationships
Comments suppressed due to low confidence (1)

cmd/oras/root/cp.go:242

  • [nitpick] Add a doc comment explaining the new copyRoot return descriptor and how it affects the copy entrypoint when the root index has no referrers.
func prepareCopyOption(ctx context.Context, src oras.ReadOnlyGraphTarget, dst oras.Target, root ocispec.Descriptor, opts oras.ExtendedCopyOptions) (oras.ExtendedCopyOptions, ocispec.Descriptor, error) {

Xiaoxuan Wang added 2 commits July 14, 2025 18:08
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
Signed-off-by: Xiaoxuan Wang <xiaoxuanwang@microsoft.com>
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

@Wwwsylvia Wwwsylvia merged commit 3c7e4f9 into oras-project:main Jul 14, 2025
8 checks passed
@wangxiaoxuan273 wangxiaoxuan273 deleted the cp-r branch August 25, 2025 02:59
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.

Resursively copying a multi-plarform image might fail when the index manifest has no referrer
4 participants