Reraise sensible errors from auto_chmod #4593
Conversation
Avasam
force-pushed
the
Reraise-sensible-errors-from-auto_chmod
branch
from
51cd3b3
to
450a962
Compare
setuptools/command/easy_install.py
Outdated
| def auto_chmod(func: Callable[..., _T], arg: str, exc: BaseException) -> _T: | ||
| """shutils onexc callback to automatically call chmod for certain functions.""" | ||
| if os.name != 'nt': | ||
| raise OSError(f"Can't call auto_chmod on non-nt os {os.name!r}") from exc |
There was a problem hiding this comment.
This is an interesting aspect of the previous implementation (not related to your changes): why can we not call chmod(path, stat.S_IWRITE) on non Windows OSs?
I had a look on the git blame history and found that this check was introduced in d675527, but I cannot guess from the commit message why it is restricted to Windows only.
This thread https://bugs.python.org/issue22040 seems to suggest that rather than the workaround not being applicable to other OSs, it is the problem that seems to only appear on Windows...
I wonder if we actually need this OS-specific check...
I can see other projects in the community just going for it, without checking for windows:
The most complex handling I saw were from pre-commit and pip, but still there is no OS-specific check:
https://github.com/pypa/pip/blob/main/src/pip/_internal/utils/misc.py#L153-L191
There was a problem hiding this comment.
I don't see why either, just ran os.chmod(".", stat.S_IWRITE) in WSL and it doesn't immediately error out or whatever.
seems to suggest that rather than the workaround not being applicable to other OSs, it is the problem that seems to only appear on Windows...
I'd make more sense to just pass-through if not Windows, or call os.chmod indescriminately.
There was a problem hiding this comment.
Here's another way of seeing it: if we know the error only happens on Windows, there's no point in "fixing and retrying", so it was just immediately rearising to keep the traceback to a minimum.
My suggestion here would be closer to that: #4593 (comment) (with a comment # Only retry for scenarios known to have an issue)
Avasam
force-pushed
the
Reraise-sensible-errors-from-auto_chmod
branch
from
79b06aa
to
ef2957a
Compare
|
Thank you very much! |
Summary of changes
My best understanding is that this is old Python2 code that was trying to re-raise an error with additional information in the message. (back when raise could take a tuple of strings)
This PR changes the nonsensical
TypeError: 'Exception' object is not subscriptablethat would always happen now for for sensible and complete messages with the old error as part of the trace. Optionally we could just reraise.Reduces changes in #4192
Pull Request Checklist
newsfragments/.(See documentation for details)