-
-
Notifications
You must be signed in to change notification settings - Fork 29.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
subprocess: support undecodable current working directory on POSIX OS #52640
Comments
In py3k, subprocess uses _posixsubprocess.fork_exec() function. This function uses surrogateescape error handler for most arguments, but not for the current working directory (cwd). Attached patch uses PyUnicode_FSConverter() as done for other arguments. I don't know if PyUnicode_FSConverter() result is always a PyBytes, so I added an assertion. It should be fixed. |
See also bpo-8391. |
New patch supporting bytearray type (remove the buggy assertion). |
Commited in py3k (r80135), blocked in 3.1 (r80136). Python 3.1 has no _posixsubprocess module, it uses os.chdir() which accepts bytes, bytearray and str with surrogates. |
It does not work on Windows: >>> subprocess.Popen("c:/windows/notepad.exe", cwd=b'c:/temp')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "D:\afa\python\py3k-1\lib\subprocess.py", line 681, in __init__
restore_signals, start_new_session)
File "D:\afa\python\py3k-1\lib\subprocess.py", line 892, in _execute_child
startupinfo)
TypeError: must be string or None, not bytes The function sp_CreateProcess() in PC/_subprocess.c should probably be fixed. |
I'm thinking on this. I plan to write tests for all my last changes about surrogates. |
I always consider Windows as a special case because Windows uses unicode internally. Byte string are converted quickly to unicode using the current locale. My patch was for UNIX/BSD which uses byte string internally. sp_CreateProcess() in PC/_subprocess.c uses CreateProcessW. To support byte string, we should use the byte string version of CreateProcess ("CreateProcessA" ?) or convert current directory to unicode (using the current locale). The second solution minimize the changes. |
PEP-277 explicitly states that unicode strings should be passed to wide-character functions, whereas byte strings use "standard" functions. The "current locale" is a moving thing. |
CreateProcessW takes a lot of arguments. Should we CreateProcessA or CreateProcessW if some arguments are byte string and other are unicode string?
Don't CreateProcessA do the same thing? Convert byte string to unicode using the current locale. To use the right words, by "current locale" I mean the "mbcs" Windows codec. |
"mbcs" is not a fixed encoding and may change between Windows sessions, see the Rationale in PEP-277 http://www.python.org/dev/peps/pep-0277/ The mixed case is interesting. We could use CreateProcessW when at least one string is Unicode, and CreateProcessA when all strings are bytes. |
Amaury, I fail to see how the error you get on Windows is related to this issue. AFAICT, neither the issue nor the patch affects Windows at all. Closing the issue as fixed. If you think there is also an issue on Windows, please submit a new bug report. However, I don't think the error you get is a bug: what you see is correct behavior. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: