bpo-44205: Ignore out of space errors in shutil.copystat

[chrisburr]

https://bugs.python.org/issue44205

https://bugs.python.org/issue44205

[nanjekyejoannah]

@chrisburr, is it possible for you to verify that your new changes work by adding a test?

[chrisburr]

I don't see an easy way of testing as it is dependent on having two file systems available that have different limits on the size of the extended attributes. I could probably mock something but I'm not convinced that would be very useful for testing anything other than if it was mocked correctly.

Do you have any thoughts?

[github-actions]

This PR is stale because it has been open for 30 days with no activity.

[zlmarshall]

Hi @nanjekyejoannah ,

I've run into this as well now – thanks to Chris for diagnosing it! I agree with his comment that having two file systems available is difficult for a test. This is a file system dependent error, not a file dependent error. I hope this can be merged soon, or addressed in another way if needed.

Cheers,
Zach

[nanjekyejoannah]

The change makes sense to me cc @vstinner for a second eye before we merge this.

[vstinner]

I'm not used to extended attributes. Ignoring errors is usually not a good idea.

The bare minimum would be to document which errors are ignored by copystat() on purpose.

How does the Unix "cp" command handle this case? Does it also ignore ENOSPC errors on setxattr()?

[chrisburr]

Thanks for taking a look @vstinner.

I'm not used to extended attributes. Ignoring errors is usually not a good idea.

In general I would agree however most errors are already ignored in copystat, such as if extended attributes not being supported at all. There is also nothing actionable that can be done about it.

The bare minimum would be to document which errors are ignored by copystat() on purpose.

The documentation already states On Linux, copystat() also copies the “extended attributes” where possible. so this PR fixes a bug with respect to the expected behaviour.

How does the Unix "cp" command handle this case? Does it also ignore ENOSPC errors on setxattr()?

By default it doesn't try to copy them, if I pass --preserve=all it shows errors but still exits as if it was successful:

[cburr@lxplus792]~% cp -p --preserve=all /cvmfs/lhcb.cern.ch/etc/grid-security/vomses/lhcb /tmp/lhcb
cp: setting attribute 'user.root_hash' for 'user.root_hash': No space left on device
cp: setting attribute 'user.speed' for 'user.speed': No space left on device
cp: setting attribute 'user.tag' for 'user.tag': No space left on device
cp: setting attribute 'user.timeout' for 'user.timeout': No space left on device
cp: setting attribute 'user.timeout_direct' for 'user.timeout_direct': No space left on device
cp: setting attribute 'user.uptime' for 'user.uptime': No space left on device
cp: setting attribute 'user.useddirp' for 'user.useddirp': No space left on device
cp: setting attribute 'user.usedfd' for 'user.usedfd': No space left on device
cp: setting attribute 'user.version' for 'user.version': No space left on device
[cburr@lxplus792]~% echo $?
0

[vstinner]

cp: setting attribute 'user.root_hash' for 'user.root_hash': No space left on device

Can we do something similar in Python?

[chrisburr]

Can we do something similar in Python?

I feel like that would be against the precedent set by the shutil module and the standard library as a whole as I can't think of any other similar functions that print warnings for the expected/documented behaviour.

The top of the shutil documentation also clearly states this limitation (shutil.copy2 uses shutil.copystat behind the scenes). Anyone who has specific requirements for extended attributes (e.g. backup tools) should be using the lower-level primitives in the os module.

The shutil module offers a number of high-level operations on files and collections of files. In particular, functions are provided which support file copying and removal. For operations on individual files, see also the os module.

Warning: Even the higher-level file copying functions (shutil.copy(), shutil.copy2()) cannot copy all file metadata.

On POSIX platforms, this means that file owner and group are lost as well as ACLs. On Mac OS, the resource fork and other metadata are not used. This means that resources will be lost and file type and creator codes will not be correct. On Windows, file owners, ACLs and alternate data streams are not copied.

[vstinner]

I would prefer to give the control to the caller to decide how setxattr() errors are handled. For example, see the onerror parameter of shutil.rmtree().

[chrisburr]

I think this PR is a bugfix that should be eligible for backporting to Python 3.9 and Python 3.10 similarly to how #13369 was.

I'm not very familiar with cpython's release process but I think adding an onerror keyword argument would be a new feature and therefore only eligible for Python 3.11? How about I open a new issue/PR to add an onerror parameter to various functions in shutil where it makes sense?

[vstinner]

I'm not used to the shutil module. I looked at this PR because @nanjekyejoannah asked me to review it.

I'm not sure that the current behavior is a bug. As I wrote, I'm not used to extended attributes. I don't know what "smaller limit on the size of the extended attributes" means. Where do these limits come from? What are limited on EXT4 and XFS for example?

[chrisburr]

I'm not used to the shutil module. I looked at this PR because @nanjekyejoannah asked me to review it.

I completely understand and sympathise. Thank you very much for taking the time to review it.

Where do these limits come from? What are limited on EXT4 and XFS for example?

I believe these both limit the size of extended attributes to the filesystem block size (often 4KB) so the only way to fix the issue is to reformat the destination filesystem to use a larger block size.

In my situation the source filesystem is a FUSE-based filesystem (CVMFS) which uses extended attributes to expose behind-the-scenes information and can be arbitrarily large as most of it is actually per-mount rather than per-file. The issue comes as shutil.copytree defaults to copying the (meaningless) extended attributes causing a crash.

Reading the shutil.copystat docs again I've also noticed it also specifically states "copystat() never returns failure".

[vstinner]

copystat() documentation says: "On Linux, copystat() also copies the “extended attributes” where possible."

Can you please document that ENOSPC errors on setxattr() are ignored on purpose? You can explain that this error can happen when the destination filesystem uses smaller limits for extended attributes thant the source filesystem.

[MaxwellDupre]

Running test_shutil produces:
Ran 159 tests in 2.168s
OK (skipped=24)
Looks ok.

[hauntsaninja]

While the added comment is welcome, I think Victor wanted a change to the documentation for shutil.copystat (on the line that says "On Linux, copystat() also copies the “extended attributes” where possible.")

[vstinner]

Oops right, please complete Doc/library/shutil.rst documentation ;-)