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

Update xattr.py to not throw hard errors on some Linux systems. #784

Merged
merged 4 commits into from
Sep 15, 2022

Conversation

jgstew
Copy link
Contributor

@jgstew jgstew commented Jan 5, 2022

xattr does not work in some Linux file systems, or in Linux running under WSL, or in some docker containers in general. This will make the failures of xattr no longer hard errors in these cases.

Use of xattr for storing etag data is not ideal in the first place and is already solved in URLDownloaderPython by using a json metadata file instead, but this will help with compatibility in general for older methods. This will have the drawback of not detecting that the already downloaded file is the same correctly due to lack of metadata for any recipe currently relying on xattr. That said, redownloading the file seems better than throwing hard errors.

I believe the behavior should be similar to how these older methods work on Windows today since on Windows xattr is no-op within autopkg.

xattr does not work in some Linux file systems, or in Linux running under WSL, or in some docker containers in general. This will make the failures of xattr no longer hard errors in these cases.

Use of xattr for storing etag data is not ideal in the first place and is already solved in URLDownloaderPython by using a json metadata file instead, but this will help with compatibility in general for older methods. This will have the drawback of not detecting that the already downloaded file is the same correctly due to lack of metadata for any recipe currently relying on xattr. That said, redownloading the file seems better than throwing hard errors.
@jgstew
Copy link
Contributor Author

jgstew commented Jan 5, 2022

This should prevent hard errors in the case of "attempting to store extended attributes on files on NFS mounts is known to still fail": #778

minor tweak, want to see if this kicks off unittesting
Comment on lines 89 to 90
print("WARNING: xattr.getxattr threw OSError.")
print(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

You could combine this:

print(f"WARNING: xattr.getxattr threw OSError: {e}")

@nmcspadden
Copy link
Contributor

nmcspadden commented Apr 18, 2022

EDIT: I'm going clinically insane

combine error messaging
combine error messages
@jgstew
Copy link
Contributor Author

jgstew commented Aug 5, 2022

combined xatter error messaging as requested @nmcspadden

@nmcspadden nmcspadden merged commit 5b582b2 into autopkg:dev Sep 15, 2022
nmcspadden pushed a commit that referenced this pull request Sep 24, 2022
* Update xattr.py

xattr does not work in some Linux file systems, or in Linux running under WSL, or in some docker containers in general. This will make the failures of xattr no longer hard errors in these cases.

Use of xattr for storing etag data is not ideal in the first place and is already solved in URLDownloaderPython by using a json metadata file instead, but this will help with compatibility in general for older methods. This will have the drawback of not detecting that the already downloaded file is the same correctly due to lack of metadata for any recipe currently relying on xattr. That said, redownloading the file seems better than throwing hard errors.

* Update xattr.py

minor tweak, want to see if this kicks off unittesting

* Update xattr.py

combine error messaging

* Update xattr.py

combine error messages
nmcspadden pushed a commit that referenced this pull request Jul 24, 2023
* Update xattr.py

xattr does not work in some Linux file systems, or in Linux running under WSL, or in some docker containers in general. This will make the failures of xattr no longer hard errors in these cases.

Use of xattr for storing etag data is not ideal in the first place and is already solved in URLDownloaderPython by using a json metadata file instead, but this will help with compatibility in general for older methods. This will have the drawback of not detecting that the already downloaded file is the same correctly due to lack of metadata for any recipe currently relying on xattr. That said, redownloading the file seems better than throwing hard errors.

* Update xattr.py

minor tweak, want to see if this kicks off unittesting

* Update xattr.py

combine error messaging

* Update xattr.py

combine error messages
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.

None yet

2 participants