copystat on symlinks fails for alpine -- faulty lchmod implementation?

[asottile]

BPO	31940
Nosy	@pitrou, @vstinner, @giampaolo, @asottile
PRs	bpo-31940: Reject faulty lchown implementations #4267 bpo-31940: Reject faulty lchmod implementations #4783
Superseder	bpo-34652: never enable lchmod on Linux

^{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 2019-02-11.01:52:46.619>
created_at = <Date 2017-11-03.23:51:13.581>
labels = ['3.7', 'tests', '3.8', 'build', 'library']
title = 'copystat on symlinks fails for alpine -- faulty lchmod implementation?'
updated_at = <Date 2019-02-11.01:52:46.618>
user = 'https://github.com/asottile'

bugs.python.org fields:

activity = <Date 2019-02-11.01:52:46.618>
actor = 'benjamin.peterson'
assignee = 'none'
closed = True
closed_date = <Date 2019-02-11.01:52:46.619>
closer = 'benjamin.peterson'
components = ['Build', 'Library (Lib)', 'Tests']
creation = <Date 2017-11-03.23:51:13.581>
creator = 'Anthony Sottile'
dependencies = []
files = []
hgrepos = []
issue_num = 31940
keywords = ['patch']
message_count = 8.0
messages = ['305527', '305528', '305689', '305697', '305732', '308394', '308404', '308408']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'giampaolo.rodola', 'Anthony Sottile']
pr_nums = ['4267', '4783']
priority = 'normal'
resolution = 'duplicate'
stage = 'resolved'
status = 'closed'
superseder = '34652'
type = None
url = 'https://bugs.python.org/issue31940'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[asottile]

Fortunately, this can be reproduced with the testsuite:

======================================================================
ERROR: test_copystat_symlinks (__main__.TestShutil)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.6/test/test_shutil.py", line 366, in test_copystat_symlinks
    os.lchmod(src_link, stat.S_IRWXO)
OSError: [Errno 95] Not supported: '/tmp/tmplfli9msi/baz'

My simplest reproduction involves docker:

FROM alpine
RUN apk update && apk add curl python3

RUN mkdir foo && ln -s /dev/null foo/bar

CMD [ \
    "python3", "-c", \
    "import shutil; shutil.copytree('foo', 'bar', symlinks=True)" \
]

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python3.6/shutil.py", line 359, in copytree
    raise Error(errors)
shutil.Error: [('foo/bar', 'bar/bar', "[Errno 95] Not supported: 'bar/bar'")]

By looking at pyconfig, I get the following:

/ # grep -E '(HAVE_FCHMODAT|HAVE_LCHMOD)' /usr/include/python3.6m/pyconfig.h 
#define HAVE_FCHMODAT 1
#define HAVE_LCHMOD 1

But it seems lchmod is actually nonfunctional in this case.

I think the fix is to augment configure to detect faulty lchmod and not set HAVE_LCHMOD? I'm not terribly familiar with the autotools pipeline but that's where I'm going to take a stab at it!

I'm originally finding this issue via pre-commit/pre-commit#655

[asottile]

Here's one idea for a patch (inspired by the rest of the function):

diff --git a/Lib/shutil.py b/Lib/shutil.py
index 464ee91..2099289 100644
--- a/Lib/shutil.py
+++ b/Lib/shutil.py
@@ -213,6 +213,13 @@ def copystat(src, dst, *, follow_symlinks=True):
         # symlink.  give up, suppress the error.
         # (which is what shutil always did in this circumstance.)
         pass
+    except OSError as why:
+        # lchmod on alpine will raise this with symlinks: python/cpython#76121
+        for err in 'EOPNOTSUPP', 'ENOTSUP':
+            if hasattr(errno, err) and why.errno == getattr(errno, err):
+                break
+        else:
+            raise
     if hasattr(st, 'st_flags'):
         try:
             lookup("chflags")(dst, st.st_flags, follow_symlinks=follow)

However lchmod seems to be just broken on alpine so the tests continue to fail (however my usecase is fixed)

[vstinner]

I would prefer to catch os.lchmod() exception and reminds that lchmod() doesn't work. Search for _O_TMPFILE_WORKS in Lib/tempfile.py for a Python example of code trying to use a function, or falls back on something else.

(I gave other examples for C code in a comment on PR 4267.)

[vstinner]

If lchmod() fails with [Errno 95] Not supported, IMHO chmod(path, mode, follow_symlinks=False) should behaves as lchmod() doesn't exist and falls back to the next case, as if HAVE_LCHMOD wasn't defined, but implement such falls back at runtime.

So the "broken" lchmod() issue should be fixed a multiple places:

• os.lchmod()
• os.chmod()
• shutil.copymode()
• pathlib.Path.lchmod()

Oh wow, this issue impacts much more functions than what I expected.

Maybe the compromise of a configure check avoids to overengineer this issue :-)

Antoine: What do you think? Should we check if lchmod() works in configure (as already implemented in the PR 4267), or implement a runtime check as I proposed.

pathlib has an interesting long comment:

    # Some platforms don't support lchmod().  Often the function exists
    # anyway, as a stub that always returns ENOSUP or perhaps EOPNOTSUPP.
    # (No, I don't know why that's a good design.)  ./configure will detect
    # this and reject it--so HAVE_LCHMOD still won't be defined on such
    # platforms.  This is Very Helpful.
    #
    # However, sometimes platforms without a working lchmod() *do* have
    # fchmodat().  (Examples: Linux kernel 3.2 with glibc 2.15,
    # OpenIndiana 3.x.)  And fchmodat() has a flag that theoretically makes
    # it behave like lchmod().  So in theory it would be a suitable
    # replacement for lchmod().  But when lchmod() doesn't work, fchmodat()'s
    # flag doesn't work *either*.  Sadly ./configure isn't sophisticated
    # enough to detect this condition--it only determines whether or not
    # fchmodat() minimally works.
    #
    # Therefore we simply ignore fchmodat() when deciding whether or not
    # os.chmod supports follow_symlinks.  Just checking lchmod() is
    # sufficient.  After all--if you have a working fchmodat(), your
    # lchmod() almost certainly works too.
    #
    # _add("HAVE_FCHMODAT",   "chmod")
    _add("HAVE_FCHOWNAT",   "chown")

[pitrou]

I don't have any strong opinion, but I think doing the check at runtime is better.

[vstinner]

Dummy question. If lchmod() is not supported by the libc, would it be possible to reimplement it using readlink()?

[asottile]

if I'm reading the manpage correctly: `readlink` tells the filename that the symlink points to.  lchmod is concerned with setting the `stat` on the link itself (which only some platforms actually support)

[vstinner]

if I'm reading the manpage correctly: readlink tells the filename that the symlink points to. lchmod is concerned with setting the stat on the link itself (which only some platforms actually support)

Oops, my bad. Ignore my comment :-)