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

Fixed xattr reading and writing #2762

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tsvietOK
Copy link

@tsvietOK tsvietOK commented Jul 26, 2024

Fixes #2761
Based on https://stackoverflow.com/questions/60893372/why-must-a-user-be-prepended-to-the-name-when-setting-a-files-xattr-with-os and https://go.googlesource.com/sys.git/+/master/unix/xattr_bsd.go#31 looks like "user" prefix is needed when saving/reading xattr to/from the file.
I don't know the Go language, so feel free to add new changes to my branch or create a new one.

@tsvietOK
Copy link
Author

tsvietOK commented Jul 26, 2024

Seems like the test (from main branch, not from mine) on Linux is reporting not supported result:
image

@tsvietOK
Copy link
Author

Tested my change:
image

@tsvietOK tsvietOK changed the title Add prefix to the xattr Fixed xattr reading and writing Aug 19, 2024
@tsvietOK
Copy link
Author

tsvietOK commented Aug 19, 2024

Maybe also there is needed some back compatibility mode (if user used old version of azcopy on Mac OS and file already has xattr without "user." prefix) for Mac OS where xattr works fine (according to test execution).
image

Copy link
Member

@adreed-msft adreed-msft left a comment

Choose a reason for hiding this comment

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

This is good, but it comes within a catch. This changes the metadata name, and somebody could have come to rely upon that. Two things need to happen to allow this through

  1. When reading, let's check for both.
  2. When writing, let's check that the "oldschool" version doesn't exist; if it does, delete it and replace it with the new attribute.

@tsvietOK
Copy link
Author

Seems like the requested changes are way above my knowledge of the Go. So, feel free to create a new PR or push to this one. By the way, wouldn't it be better to use this module https://github.com/pkg/xattr which covers support/error handling for some specific OSs like FreeBSD, NetBSD and MacOS?

@adreed-msft
Copy link
Member

Seems like the requested changes are way above my knowledge of the Go. So, feel free to create a new PR or push to this one. By the way, wouldn't it be better to use this module https://github.com/pkg/xattr which covers support/error handling for some specific OSs like FreeBSD, NetBSD and MacOS?

Sounds like I'll be picking this up if I get some spare time.

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.

AzCopy fails to write xattr
2 participants