Make sure we pass unicode, not bytes to CreateProcess()#830
Conversation
This eliminates exceptions on Python 3 when passing a path argument encoded as bytes.
…spawnProcess on Windows.
opalmer
left a comment
opalmer
left a comment
There was a problem hiding this comment.
Looks good 👍
On Windows file system paths are Unicode by default so it's a bit redundant to do .decode(sys.getfilesystemencoding()). The docs for Python 2 mention that getfilesystemencoding() is mainly intended for string -> byte conversion and for Python 3 it apparently changes to always be 'utf-8'. But based on the docs it shouldn't hurt either so I'd leave it as you have it in case there are edge cases out there.
References:
https://docs.python.org/2/library/sys.html#sys.getfilesystemencoding
https://docs.python.org/3/library/sys.html#sys.getfilesystemencoding
https://www.python.org/dev/peps/pep-0529/
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247.aspx
|
Thank you @altendky ! |
opalmer
left a comment
opalmer
left a comment
There was a problem hiding this comment.
Sorry, forgot to hit approve on my side.
|
What happened to break it again :-( it looks like 95% now. |
|
If I try to look at https://codecov.io/gh/twisted/twisted/pull/830 , I get a 500 error |
|
I see 100% patch coverage here: https://codecov.io/gh/twisted/twisted/pull/830 |
3 participants
https://twistedmatrix.com/trac/ticket/9200