Only do the ensure_text() dance on Windows #8666
Conversation
|
Not including this in 20.2.1, since it's Python 2 and I'm not super sure I can review this unilaterally. :) |
|
Do you think this could be included in the next release? It seems like a fairly straightforward fix and the issue does impact quite a few users. |
src/pip/_internal/utils/temp_dir.py
Outdated
| # errors caused by the environment configuring encodings incorrectly. | ||
| if WINDOWS: | ||
| encoding = sys.getfilesystemencoding() | ||
| rmtree(ensure_text(self._path, encoding=encoding)) |
There was a problem hiding this comment.
Why do we have an explicit encoding here when previously we didn't? IMO, we should limit this to the absolute bare minimum change, as we won't ever have a chance to fix any bugs we might introduce (after 20.3, we'll be dropping Python 2 support, and we don't want to introduce new Python 2 bugs just before doing so).
There was a problem hiding this comment.
ensure_text uses utf-8 by default, which is wrong on Windows in most of the cases. But fair enough—nobody has ever reported this, Python 2 is going away, and has enough problems on non-English Windows this probably does not matter.
uranusjr
force-pushed
the
rmtree-unicode-use-file-system-encoding
branch
from
240ed7d
to
5a0118c
Compare
|
Not merging just yet, this probably needs a rebase once the CI issues are sorted. |
POSIX is problematic when the environment is not configured properly.
uranusjr
force-pushed
the
rmtree-unicode-use-file-system-encoding
branch
from
5a0118c
to
94fbb6c
Compare
|
Thanks for the nudge here @lkollar! |
POSIX is problematic when the environment is not configured properly. Fix #8658.