Issue31940
Created on 2017-11-03 23:51 by Anthony Sottile, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 4267 | closed | Anthony Sottile, 2017-11-04 04:39 | |
| PR 4783 | closed | 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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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) * ![]() |
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 |
| 2022-04-11 14:58:54 | admin | set | github: 76121 |
| 2019-02-11 01:52:46 | benjamin.peterson | set | status: open -> closed superseder: never enable lchmod on Linux resolution: duplicate stage: patch review -> resolved |
| 2018-08-07 11:43:06 | giampaolo.rodola | set | nosy:
+ giampaolo.rodola |
| 2018-04-20 21:23:16 | martin.panter | link | issue28627 superseder |
| 2017-12-15 15:41:14 | vstinner | set | messages: + msg308408 |
| 2017-12-15 15:28:02 | Anthony Sottile | set | messages: + msg308404 |
| 2017-12-15 14:37:59 | vstinner | set | messages: + msg308394 |
| 2017-12-10 19:37:06 | Anthony Sottile | set | pull_requests: + pull_request4684 |
| 2017-11-07 10:05:42 | pitrou | set | messages: + msg305732 |
| 2017-11-07 00:35:43 | vstinner | set | nosy:
+ pitrou messages: + msg305697 |
| 2017-11-06 23:29:50 | vstinner | set | nosy:
+ vstinner messages: + msg305689 |
| 2017-11-04 04:39:30 | Anthony Sottile | set | keywords:
+ patch stage: patch review pull_requests: + pull_request4230 |
| 2017-11-04 00:14:49 | Anthony Sottile | set | messages: + msg305528 |
| 2017-11-03 23:51:13 | Anthony Sottile | create | |
