Skip to content

bpo-24564: shutil.copystat(): ignore EINVAL on os.setxattr() #13369

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.

Sign up for GitHub

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 5 commits into from
May 30, 2019
Merged

bpo-24564: shutil.copystat(): ignore EINVAL on os.setxattr() #13369

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6138e35
BPO-24564 shutil.copystat(): ignore invalid argument error number whe…
yingw787 May 16, 2019
181e059
📜🤖 Added by blurb_it.
blurb-it[bot] May 16, 2019
17ae022
BPO-24564 Update NEWS entry to use markup.p.
yingw787 May 18, 2019
875d07e
Merge branch 'master' of https://github.com/python/cpython into bpo-2…
yingw787 May 29, 2019
5e731d2
Update shutil.py update syntax for conditional statement according to…
yingw787 May 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Lib/shutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,15 +309,16 @@ def _copyxattr(src, dst, *, follow_symlinks=True):
try:
names = os.listxattr(src, follow_symlinks=follow_symlinks)
except OSError as e:
if e.errno not in (errno.ENOTSUP, errno.ENODATA):
if e.errno not in (errno.ENOTSUP, errno.ENODATA, errno.EINVAL):
raise
return
for name in names:
try:
value = os.getxattr(src, name, follow_symlinks=follow_symlinks)
os.setxattr(dst, name, value, follow_symlinks=follow_symlinks)
except OSError as e:
if e.errno not in (errno.EPERM, errno.ENOTSUP, errno.ENODATA):
if e.errno not in (errno.EPERM,
errno.ENOTSUP, errno.ENODATA, errno.EINVAL):
Copy link
Contributor

@giampaolo giampaolo May 28, 2019

Choose a reason for hiding this comment

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

just a cosmetic/style nitpick - it's better to do this:

                if e.errno not in (errno.EPERM, errno.ENOTSUP, errno.ENODATA,
                                   errno.EINVAL):

Copy link
Contributor Author

@yingw787 yingw787 May 29, 2019

Choose a reason for hiding this comment

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

Should be updated!

raise
else:
def _copyxattr(*args, **kwargs):
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2019-05-16-23-40-36.bpo-24564.lIwV_7.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
shutil.copystat(): ignore EINVAL on os.setxattr() which may occur when copying files on NFS filesystems.
Copy link
Contributor

@BoboTiG BoboTiG May 18, 2019

Choose a reason for hiding this comment

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

Do you mind adding markup and reword the NEWs entry? I think something like that could be good (think to use blurb to format correctly the lines):

:func:`shutil.copystat` now ignores :const:`errno.EINVAL` on :func:`os.setxattr` which may occur when copying files on filesystems without extended attributes support. Original patch by Giampaolo Rodola, updated by Ying Wang.

Copy link
Contributor Author

@yingw787 yingw787 May 18, 2019

Choose a reason for hiding this comment

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

Sounds great, should be pushed 👍


(patch by Giampaolo Rodola, pull request update by Ying Wang)