easybuild-framework icon indicating copy to clipboard operation
easybuild-framework copied to clipboard

change copy_file to ignore failing copy of file metadata by using shutil.copyfile + shutil.copystat rather than shutil.copy2

Open boegel opened this issue 3 years ago • 4 comments

boegel avatar Dec 09 '21 23:12 boegel

When reading the docco on shutil.copy2 I don't see the point in this change:

copy2() will preserve all the metadata it can; copy2() never raises an exception because it cannot preserve file metadata.

copy2() uses copystat() to copy the file metadata. Please see copystat() for more information about platform support for modifying symbolic link metadata.

Note the part where copy2 won't raise an exception reg metadata...

akesandgren avatar Dec 10 '21 07:12 akesandgren

Hmm, that confuses me a bit, since the problem report in #3910 does show that an error was raised. It's a No space left on device error though, due to an inode issue, I guess that would also be raised by copyfile?

@ofisette Any chance you can test the changes proposed here to see if it actually helps for the problem you're seeing? Seems like it won't...

cc @bartoldeman

boegel avatar Dec 10 '21 07:12 boegel

@boegel The copy2 function definitely raises exceptions when trying to copy metadata, contradicting the doc:

>>> from shutil import copy2
>>> copy2("/cvmfs/soft.computecanada.ca/easybuild/site-packages/easybuild-easyblocks/easybuild/easyblocks/generic/cmakemake.py", "/mnt/testfs/tmp/")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib/python3.7/shutil.py", line 267, in copy2
    copystat(src, dst, follow_symlinks=follow_symlinks)
  File "/cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib/python3.7/shutil.py", line 209, in copystat
    _copyxattr(src, dst, follow_symlinks=follow)
  File "/cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib/python3.7/shutil.py", line 165, in _copyxattr
    os.setxattr(dst, name, value, follow_symlinks=follow_symlinks)
OSError: [Errno 28] No space left on device: '/mnt/testfs/tmp/cmakemake.py'

Looking at the shutil module, exceptions are only suppressed in copystat when metadata type is unsupported, EOPNOTSUPP. (And copy2 is just copyfile followed by copystat.) I hesitate to report this as a Python bug because it seems like a good choice, but the doc could be clearer.

ofisette avatar Dec 10 '21 15:12 ofisette

copystat does

  1. utime
  2. _copyxattr
  3. chmod so if _copyxattr fails chmod will not be called. one can try copymode however if copystat fails, and then just the permission bits are copied.

There's another issue with this patch is that the if/else logic could possibly be simplified since there's another copyfile already a little further up. I'll need to look carefully to determine that.

bartoldeman avatar Dec 10 '21 18:12 bartoldeman