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

os.setxattr PermissionError on panfs propagates up causing copystat, copytree, and pip install . to fail unhepfully #68726

Closed
gerritholl mannequin opened this issue Jun 30, 2015 · 10 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@gerritholl
Copy link
Mannequin

gerritholl mannequin commented Jun 30, 2015

BPO 24538
Nosy @giampaolo, @tarekziade, @bitdancer, @gerritholl, @obilaniu, @iritkatriel
PRs
  • bpo-24538: Fix bug in shutil involving the copying of xattrs to read-only files. #13212
  • [3.7] bpo-24538: Fix bug in shutil involving the copying of xattrs to read-only files. (PR-13212) #13234
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2020-10-20.10:57:24.809>
    created_at = <Date 2015-06-30.12:07:21.714>
    labels = ['3.8', '3.7', 'library', 'type-crash']
    title = 'os.setxattr PermissionError on panfs propagates up causing `copystat`, `copytree`, and `pip install .` to fail unhepfully'
    updated_at = <Date 2020-10-20.10:57:24.809>
    user = 'https://github.com/gerritholl'

    bugs.python.org fields:

    activity = <Date 2020-10-20.10:57:24.809>
    actor = 'iritkatriel'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-10-20.10:57:24.809>
    closer = 'iritkatriel'
    components = ['Library (Lib)']
    creation = <Date 2015-06-30.12:07:21.714>
    creator = 'Gerrit.Holl'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 24538
    keywords = ['patch']
    message_count = 10.0
    messages = ['245985', '245987', '245989', '301493', '342022', '342051', '342052', '342433', '378292', '378300']
    nosy_count = 8.0
    nosy_names = ['giampaolo.rodola', 'tarek', 'r.david.murray', 'Gerrit.Holl', 'Bart Oldeman', 'Maxime Boissonneault', 'obilaniu', 'iritkatriel']
    pr_nums = ['13212', '13234']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue24538'
    versions = ['Python 2.7', 'Python 3.7', 'Python 3.8']

    @gerritholl
    Copy link
    Mannequin Author

    gerritholl mannequin commented Jun 30, 2015

    shutil.copystat fails on panfs if the source file lacks u+w, because setting extended attributes results in a PermissionError. This leads to higher end functions such as shutil.copytree to fail. More seriously, it leads to pip install . to fail, as I reported here:

    In [55]: shutil.copystat("/src/on/panfs", "/tmp/fubar")
    \---------------------------------------------------------------------------
    
        PermissionError                           Traceback (most recent call last)
        <ipython-input-55-139c9fc77184> in <module>()
        ----> 1 shutil.copystat("/home/users/gholl/checkouts/pyatmlab/.git/objects/pack/pack-c1449559ec4287b3830efe2913608dddf2f21391.pack", "/tmp/fubar")
        
        /home/users/gholl/lib/python3.4/shutil.py in copystat(src, dst, follow_symlinks)
            211             else:
            212                 raise
        --> 213     _copyxattr(src, dst, follow_symlinks=follow)
            214
            215 def copy(src, dst, *, follow_symlinks=True):
        
        /home/users/gholl/lib/python3.4/shutil.py in _copyxattr(src, dst, follow_symlinks)
            151             try:
            152                 value = os.getxattr(src, name, follow_symlinks=follow_symlinks)
        --> 153                 os.setxattr(dst, name, value, follow_symlinks=follow_symlinks)
            154             except OSError as e:
            155                 if e.errno not in (errno.EPERM, errno.ENOTSUP, errno.ENODATA):
        
        PermissionError: [Errno 13] Permission denied: '/tmp/fubar'

    This occurs for any source file where the user write bit is not set.

    Now, I'm not sure if this should be addressed in pip, shutil.copytree, shutil.copystat, setxattr, panfs, or in none of the above. I'm reporting it in different places; please close this issue if this is the wrong place.

    @gerritholl gerritholl mannequin added stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump labels Jun 30, 2015
    @bitdancer
    Copy link
    Member

    There are a couple of related open issues. I think there is an stdlib problem here, but I"m not sure what the solution is.

    @gerritholl
    Copy link
    Mannequin Author

    gerritholl mannequin commented Jun 30, 2015

    Perhaps the solution would be some kind of flag, at least for copytree and possibly others, on what to do when attributes cannot be completely copied — a bit like numpys options to raise, warn, or ignore?

    @BartOldeman
    Copy link
    Mannequin

    BartOldeman mannequin commented Sep 6, 2017

    The issue can be avoided by calling _copyxattr *before* instead of after os.chmod in shutil.copystat. That way the file is still writable.

    The same issue happens on the Lustre parallel file system, with the lustre.lov extended attribute.

    @obilaniu
    Copy link
    Mannequin

    obilaniu mannequin commented May 10, 2019

    This old bug now has a PR on Github authored by myself that may be reviewed.

    @giampaolo
    Copy link
    Contributor

    Patch LGTM. I'd like to point out one thing for posterity. Current shutil code catches EPERM but not EACCES. While reviewing the patch I wondered whether simply catching (and ignoring) both error codes instead, but it turns out they are supposed to have 2 different meanings:
    https://stackoverflow.com/a/35879961/376587
    Especially after _copyxattr is moved before chmod() EACCES (permission denied) should propagate instead of being silenced, and EPERM should be left in place because in this case EPERM acts more like an ENOTSUP (operation not supported) than EACCES (permission denied) and as such should rightly be ignored.

    @giampaolo
    Copy link
    Contributor

    New changeset 79efbb7 by Giampaolo Rodola (Olexa Bilaniuk) in branch 'master':
    bpo-24538: Fix bug in shutil involving the copying of xattrs to read-only files. (PR-13212)
    79efbb7

    @giampaolo giampaolo added 3.7 (EOL) end of life 3.8 only security fixes labels May 10, 2019
    @giampaolo
    Copy link
    Contributor

    New changeset 0a5b88e by Giampaolo Rodola (Miss Islington (bot)) in branch '3.7':
    bpo-24538: Fix bug in shutil involving the copying of xattrs to read-only files. (PR-13212) (bpo-13234)
    0a5b88e

    @iritkatriel
    Copy link
    Member

    This seems complete, can it be closed?

    @obilaniu
    Copy link
    Mannequin

    obilaniu mannequin commented Oct 9, 2020

    Yes, it may be closed. The fix was merged in time to make it into Python 3.7.4 and 3.8.0, and solved the problems we were facing with fancy filesystem xattrs on read-only files.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants