classification
Title: copystat on symlinks fails for alpine -- faulty lchmod implementation?
Type: Stage: patch review
Components: Build, Library (Lib), Tests Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Anthony Sottile, giampaolo.rodola, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2017-11-03 23:51 by Anthony Sottile, last changed 2018-08-07 11:43 by giampaolo.rodola.

Pull Requests
URL Status Linked Edit
PR 4267 open Anthony Sottile, 2017-11-04 04:39
PR 4783 open Anthony Sottile, 2017-12-10 19:37
Messages (8)
msg305527 - (view) Author: Anthony Sottile (Anthony Sottile) * Date: 2017-11-03 23:51
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:

```dockerfile
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 https://github.com/pre-commit/pre-commit/issues/655
msg305528 - (view) Author: Anthony Sottile (Anthony Sottile) * Date: 2017-11-04 00:14
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: #31940
+        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)
msg305689 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-11-06 23:29
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.)
msg305697 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-11-07 00:35
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")
msg305732 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-11-07 10:05
I don't have any strong opinion, but I think doing the check at runtime is better.
msg308394 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-12-15 14:37
Dummy question. If lchmod() is not supported by the libc, would it be possible to reimplement it using readlink()?
msg308404 - (view) Author: Anthony Sottile (Anthony Sottile) * Date: 2017-12-15 15:28
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)
msg308408 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-12-15 15:41
> 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 :-)
History
Date User Action Args
2018-08-07 11:43:06giampaolo.rodolasetnosy: + giampaolo.rodola
2018-04-20 21:23:16martin.panterlinkissue28627 superseder
2017-12-15 15:41:14vstinnersetmessages: + msg308408
2017-12-15 15:28:02Anthony Sottilesetmessages: + msg308404
2017-12-15 14:37:59vstinnersetmessages: + msg308394
2017-12-10 19:37:06Anthony Sottilesetpull_requests: + pull_request4684
2017-11-07 10:05:42pitrousetmessages: + msg305732
2017-11-07 00:35:43vstinnersetnosy: + pitrou
messages: + msg305697
2017-11-06 23:29:50vstinnersetnosy: + vstinner
messages: + msg305689
2017-11-04 04:39:30Anthony Sottilesetkeywords: + patch
stage: patch review
pull_requests: + pull_request4230
2017-11-04 00:14:49Anthony Sottilesetmessages: + msg305528
2017-11-03 23:51:13Anthony Sottilecreate