Skip to content

Conversation

@J-jxr
Copy link
Contributor

@J-jxr J-jxr commented Oct 20, 2025

Overview

This PR is an update based on the original PR #677 (submitted by BraveY). It addresses some review comments from the original PR, retains the full commit history , and ensures the core function—reconverting nydus-format images to OCI format—works as expected.

Related Issues

PR#677
Related dragonflyoss/nydus#1682 (follows the same upstream issue as the original PR)

Change Details

  1. Added defer cw.Close() for blob writer (cw)
  2. Added "uncompressed" option to the Compressor parameter (default remains "gzip") to cover uncompressed OCI image scenarios
  3. Cleaned up unused code: Removed unused comments (e.g., LayerAnnotationOciSourceDigest)
  4. Optimized memory usage: Modified the logic for writing large OCI tar packages

Change Type

  • Feature Addition (aligns with the original PR's feature type, focusing on completing the nydus-to-OCI reconversion feature)

@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 0% with 164 lines in your changes missing coverage. Please review.
✅ Project coverage is 20.59%. Comparing base (0d09129) to head (5e354ba).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/converter/reconvert_unix.go 0.00% 164 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
- Coverage   20.89%   20.59%   -0.31%     
==========================================
  Files         122      123       +1     
  Lines       11068    11232     +164     
==========================================
  Hits         2313     2313              
- Misses       8436     8600     +164     
  Partials      319      319              
Files with missing lines Coverage Δ
pkg/converter/types.go 0.00% <ø> (ø)
pkg/converter/reconvert_unix.go 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@BraveY BraveY left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. LGTM!

Copy link
Contributor

@Zephyrcf Zephyrcf left a comment

Choose a reason for hiding this comment

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

LGTM!

@J-jxr J-jxr force-pushed the jxr-pr677-update branch 9 times, most recently from 79ccf90 to 62c9f24 Compare November 6, 2025 13:14
@J-jxr J-jxr force-pushed the jxr-pr677-update branch 7 times, most recently from 9e0c04b to b958d78 Compare November 18, 2025 15:28

func MergeLayers(ctx context.Context, cs content.Store, descs []ocispec.Descriptor, opt MergeOption) (*ocispec.Descriptor, []ocispec.Descriptor, error) {
panic("not implemented")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep blank lines

return nil, nil
}

if !images.IsManifestType(newDesc.MediaType) && !images.IsIndexType(newDesc.MediaType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This if statement seems unnecessary; we only need to handle the Manifest type, and all others can be returned directly.

return newManifestDesc, nil
}

func ReconvertHookFunc() converter.ConvertHookFunc {
Copy link
Contributor

Choose a reason for hiding this comment

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

move this func to reconvert_unix.go

}
}

func LayerReconvertFunc(opt UnpackOption) ConvertFunc {
Copy link
Contributor

Choose a reason for hiding this comment

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

move this func to reconvert_unix.go

@J-jxr J-jxr force-pushed the jxr-pr677-update branch 2 times, most recently from 1a2649c to d21315b Compare November 21, 2025 09:30
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.

3 participants