Skip to content

feat: option to have File store preserve permissions when extracting #891

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

Merged
merged 2 commits into from
Feb 26, 2025

Conversation

sammy-da
Copy link
Contributor

@sammy-da sammy-da commented Feb 14, 2025

This PR takes care of #886, or rather part of it.
It adds an option to the File store that's similar to tar's --preserve-permissions.

closes #886

@sammy-da sammy-da force-pushed the file-store-preserve-perms branch 3 times, most recently from 9f93f59 to f9f1baa Compare February 14, 2025 05:44
@sammy-da sammy-da marked this pull request as ready for review February 14, 2025 05:44
@sammy-da sammy-da force-pushed the file-store-preserve-perms branch 4 times, most recently from 830c0a9 to 0b55f28 Compare February 14, 2025 05:57
Copy link

codecov bot commented Feb 20, 2025

Codecov Report

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

Project coverage is 80.19%. Comparing base (cb6d75b) to head (89a8845).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
content/file/utils.go 70.00% 2 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (72.72%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #891      +/-   ##
==========================================
- Coverage   80.27%   80.19%   -0.08%     
==========================================
  Files          63       63              
  Lines        6047     6053       +6     
==========================================
  Hits         4854     4854              
- Misses        856      860       +4     
- Partials      337      339       +2     

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

@sammy-da sammy-da force-pushed the file-store-preserve-perms branch 2 times, most recently from bafe923 to 5560c4d Compare February 20, 2025 14:24
@sammy-da sammy-da force-pushed the file-store-preserve-perms branch 4 times, most recently from 26443ab to 9bf6f31 Compare February 22, 2025 09:33
@sammy-da sammy-da requested a review from Wwwsylvia February 22, 2025 10:07
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. Just a nit.

@Wwwsylvia Wwwsylvia changed the title option to have File store preserve permsissions when extracting feat: option to have File store preserve permissions when extracting Feb 24, 2025
@Wwwsylvia
Copy link
Member

@sammy-da Could you also link the PR to the issue using GitHub keywords?

@sammy-da
Copy link
Contributor Author

Could you also link the PR to the issue using GitHub keywords?

It doesn't fully close it though. This will only take care of files that end up getting packed.
I guess i'll mark this as closing the issue anyway 🤷

@sammy-da sammy-da requested a review from Wwwsylvia February 24, 2025 10:41
@shizhMSFT
Copy link
Contributor

@sammy-da DCO checking is failing. Could you fix it?

@sammy-da sammy-da force-pushed the file-store-preserve-perms branch from f7118da to 46736fd Compare February 24, 2025 23:16
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
Copy link
Member

@sammy-da Could you help to rebase?

@sammy-da sammy-da force-pushed the file-store-preserve-perms branch from 0ae429f to 89a8845 Compare February 25, 2025 14:55
@sammy-da
Copy link
Contributor Author

@Wwwsylvia rebased

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

@Wwwsylvia Wwwsylvia merged commit d92df9d into oras-project:main Feb 26, 2025
8 of 9 checks passed
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.

file permissions are not retained
4 participants