msg173093 - (view) |
Author: Ben Rousch (brousch) |
Date: 2012-10-16 21:00 |
The subprocess.Popen function uses /bin/sh in Unix environments. Android is detected as a Unix environemnt, but has moved that executable to /system/bin/sh. This can be worked around by adding a parameter "executable='/system/bin/sh'" to the call, but it is impractical to do this for every call to Popen in every library and codebase. For subprocess.Popen to work on Android, Popen needs to be able to detect if /bin/sh is not there and try /system/bin/sh instead.
|
msg173100 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2012-10-16 21:24 |
Android really should not be breaking the standards that way. We do want to support Android, but have you submitted a bug report to them?
|
msg173107 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2012-10-16 22:31 |
...Applications should note that the standard PATH to the shell cannot be assumed to be either /bin/sh or /usr/bin/sh, and should be determined by interrogation of the PATH returned by getconf PATH , ensuring that the returned pathname is an absolute pathname and not a shell built-in....
|
msg173109 - (view) |
Author: Ben Rousch (brousch) |
Date: 2012-10-16 23:16 |
@rpetrov thanks for finding that - I was having trouble hunting down what the standard is. I think you're quoting from http://pubs.opengroup.org/onlinepubs/009695399/utilities/sh.html
|
msg173115 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2012-10-17 00:06 |
We should do that, then, if /bin/sh doesn't exist.
|
msg173117 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2012-10-17 00:45 |
I wouldn't blame android for this; I doubt Android claims to support whatever standard you are holding it to.
It seems simple enough for us to make the default configurable (a public module level constant that anyone can override in their code after importing the module to change the default) or to otherwise query the system for where the default shell is at import time.
|
msg173120 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2012-10-17 01:12 |
Well, posix; but I was wrong about what posix required, as Roumen pointed out.
|
msg173361 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-19 22:44 |
Is there some document describing procedure for building/installing/running_tests at Android device?
I have Samsung Galaxy Tab and would to work on the issue.
But my android experience is limited to installing Scripting Layer for Android from Google Play and running python3 scripts.
|
msg173362 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-19 22:59 |
> It seems simple enough for us to make the default configurable
This seems good to do in any case (as opposed to having the string be "hard-coded"), and we can test it independently of Android. There is a recently added test in the 3.x branches that can be modified: test method "test_executable_replaces_shell" which checks that the executable argument replaces the default shell for non-Windows.
I would suggest implementing Gregory's two suggested measures as two separate patches.
|
msg173363 - (view) |
Author: Ben Rousch (brousch) |
Date: 2012-10-19 23:29 |
> Is there some document describing procedure for building/installing/running_tests at Android device?
I'm using the android-python27 project, but you can also reproduce it under SL4A:
Start SL4A
Menu -> View -> Interpreters
Python 2.6.2 or Python 3
>>> from subprocess import Popen
>>> p = Popen("ls", shell=True)
*Fails* OSError: [Error 2] No such file or directory
>>> p = Popen("ls" shell=True, executable="/system/bin/sh")
*Works*
|
msg173477 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-21 20:12 |
It occurs to me that logic for detecting the shell might make sense for being part of the os module, e.g. os.getdefaultshell().
|
msg173658 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2012-10-24 09:08 |
Chris, agree with you.
|
msg174094 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-10-29 01:27 |
I created issue 16353 for adding a function to the os module for getting the path to the default shell.
The current issue 16255 could be addressed in 3.4 by calling such a function from the subprocess module. For earlier versions (since enhancements are not allowed), the current issue could perhaps be addressed by backporting the logic in such a function directly into the subprocess module (i.e. without adding a new function to the os module).
|
msg224453 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2014-07-31 23:14 |
Why not simply use $SHELL?
|
msg224474 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2014-08-01 08:50 |
$SHELL is up to the user and not guaranteed to be anything remotely /bin/sh
compatible.
On Jul 31, 2014 4:14 PM, "Antoine Pitrou" <report@bugs.python.org> wrote:
>
> Antoine Pitrou added the comment:
>
> Why not simply use $SHELL?
>
> ----------
> nosy: +pitrou
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue16255>
> _______________________________________
>
|
msg230757 - (view) |
Author: Matt Frank (WanderingLogic) * |
Date: 2014-11-06 19:08 |
Assuming issue16353 is fixed using http://bugs.python.org/file36196/os.get_shell_executable.patch the appropriate way to find the path to the default shell is by calling os.get_shell_executable().
This is the 1-liner patch that uses os.get_shell_executable() in Popen() instead of the hard-coded string "/bin/sh".
|
msg264332 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2016-04-26 22:19 |
It seems that Android is the only known platform that deviates from
/bin/sh. So perhaps we should simply set a variable to either
/bin/sh or /system/bin/sh rather than waiting for #16353 to settle.
|
msg266084 - (view) |
Author: Xavier de Gaye (xdegaye) * |
Date: 2016-05-22 15:51 |
The list of locations where '/bin/sh' is hard coded in the standard library:
Lib/distutils/tests/test_build_scripts.py|68 col 31| ("#!/bin/sh\n"
Lib/distutils/tests/test_install_scripts.py|57 col 38| write_script("shell.sh", ("#!/bin/sh\n"
Lib/distutils/tests/test_spawn.py|34 col 37| self.write_file(exe, '#!/bin/sh\nexit 1')
Lib/distutils/tests/test_spawn.py|45 col 37| self.write_file(exe, '#!/bin/sh\nexit 0')
Lib/subprocess.py|611 col 24| >>> check_output(["/bin/sh", "-c",
Lib/subprocess.py|1450 col 26| args = ["/bin/sh", "-c"] + args
Lib/test/test__osx_support.py|46 col 24| f.write("#!/bin/sh\n/bin/echo OK\n")
Lib/test/test__osx_support.py|58 col 24| f.write("#!/bin/sh\n/bin/echo ExpectedOutput\n")
Lib/test/test__osx_support.py|149 col 28| f.write("#!/bin/sh\n/bin/echo " + c_output)
Lib/test/test__osx_support.py|205 col 24| f.write("#!/bin/sh\nexit 255")
Lib/test/test_os.py|673 col 42| @unittest.skipUnless(os.path.exists('/bin/sh'), 'requires /bin/sh')
Lib/test/test_os.py|677 col 24| with os.popen("/bin/sh -c 'echo $HELLO'") as popen:
Lib/test/test_os.py|681 col 42| @unittest.skipUnless(os.path.exists('/bin/sh'), 'requires /bin/sh')
Lib/test/test_os.py|684 col 14| "/bin/sh -c 'echo \"line1\nline2\nline3\"'") as popen:
Lib/test/test_subprocess.py|1563 col 31| fobj.write("#!/bin/sh\n")
Lib/test/test_subprocess.py|1611 col 31| fobj.write("#!/bin/sh\n")
Lib/test/test_subprocess.py|1629 col 15| sh = '/bin/sh'
|
msg266089 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2016-05-22 16:30 |
I've also opened a feature request here:
https://code.google.com/p/android/issues/detail?id=210812
|
msg270047 - (view) |
Author: Xavier de Gaye (xdegaye) * |
Date: 2016-07-09 15:18 |
> The list of locations where '/bin/sh' is hard coded in the standard library:
I have entered issue 27472.
|
msg270834 - (view) |
Author: Xavier de Gaye (xdegaye) * |
Date: 2016-07-19 16:41 |
The attached patch fixes the issue. There is at least already an existing test case: test_subprocess.MiscTests.test_getoutput calls subprocess.getstatusoutput() that runs a Popen instance with shell=True.
|
msg270865 - (view) |
Author: Stefan Krah (skrah) * |
Date: 2016-07-20 11:37 |
Apparently sysconfig.get_config_var() is already called in site.py anyway,
so in principle I'm fine with using it here.
In the stdlib it seems to be the first use outside site.py though, and
site.py can be disabled by using "python -S", so perhaps we should be
extra careful, set UNIX_SHELL unconditionally to "/bin/sh" and move
the Android conditional inside a try/except.
But perhaps I'm being irrational here. Victor, what do you think?
|
msg270919 - (view) |
Author: Xavier de Gaye (xdegaye) * |
Date: 2016-07-21 13:08 |
New patch that does not call sysconfig.get_config_var() on platforms that have '/bin/sh'.
|
msg283017 - (view) |
Author: Xavier de Gaye (xdegaye) * |
Date: 2016-12-12 16:00 |
New patch using sys.getandroidapilevel() that is only defined for Android.
getandroidapilevel() is only available in 3.7, so only 3.7 is being fixed.
|
msg283116 - (view) |
Author: Xavier de Gaye (xdegaye) * |
Date: 2016-12-13 16:16 |
For some reason the following notifications have not all been received (yet):
remote: added 1 changesets with 2 changes to 2 files
remote: buildbot: change(s) sent successfully
remote: sent email to roundup at report@bugs.python.org
remote: notified python-checkins@python.org of incoming changeset 96a9992d1003
But the buildbots have processed the build request.
Closing the issue.
|
msg283117 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-12-13 16:20 |
New changeset 96a9992d1003 by Xavier de Gaye in branch 'default':
Issue #16255: subrocess.Popen uses /system/bin/sh on Android as the shell,
https://hg.python.org/cpython/rev/96a9992d1003
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:37 | admin | set | github: 60459 |
2017-03-31 16:36:16 | dstufft | set | pull_requests:
+ pull_request905 |
2016-12-13 16:20:30 | python-dev | set | nosy:
+ python-dev messages:
+ msg283117
|
2016-12-13 16:16:50 | xdegaye | set | status: open -> closed resolution: fixed messages:
+ msg283116
stage: patch review -> resolved |
2016-12-12 16:00:24 | xdegaye | set | files:
+ unix_shell_16255_3.patch versions:
+ Python 3.7, - Python 3.6 messages:
+ msg283017
assignee: xdegaye dependencies:
- add function to os module for getting path to default shell |
2016-07-21 13:08:43 | xdegaye | set | files:
+ unix_shell_16255_2.patch
messages:
+ msg270919 |
2016-07-20 11:37:24 | skrah | set | messages:
+ msg270865 |
2016-07-19 16:49:14 | xdegaye | set | nosy:
+ vstinner
|
2016-07-19 16:41:20 | xdegaye | set | files:
+ unix_shell_16255.patch
messages:
+ msg270834 stage: patch review |
2016-07-09 15:18:28 | xdegaye | set | messages:
+ msg270047 |
2016-05-22 16:30:05 | skrah | set | messages:
+ msg266089 |
2016-05-22 15:58:14 | xdegaye | link | issue26865 dependencies |
2016-05-22 15:51:10 | xdegaye | set | nosy:
+ xdegaye messages:
+ msg266084
|
2016-04-27 10:37:46 | Roman.Evstifeev | set | nosy:
+ Roman.Evstifeev
|
2016-04-26 22:19:42 | skrah | set | nosy:
+ skrah
messages:
+ msg264332 versions:
+ Python 3.6, - Python 3.4 |
2015-11-25 15:12:44 | yan12125 | set | nosy:
+ yan12125
|
2015-08-23 22:30:27 | r.david.murray | link | issue24919 superseder |
2014-11-06 19:08:33 | WanderingLogic | set | files:
+ popen-no-hardcode-bin-sh.patch keywords:
+ patch messages:
+ msg230757
|
2014-11-05 17:56:13 | WanderingLogic | set | nosy:
+ WanderingLogic
|
2014-08-01 08:50:40 | gregory.p.smith | set | messages:
+ msg224474 |
2014-07-31 23:14:01 | pitrou | set | nosy:
+ pitrou messages:
+ msg224453
|
2013-08-19 16:52:27 | asvetlov | set | versions:
- Python 2.7, Python 3.2, Python 3.3 |
2013-08-19 16:52:03 | asvetlov | set | dependencies:
+ add function to os module for getting path to default shell |
2012-11-02 18:59:49 | ezio.melotti | set | nosy:
+ ezio.melotti
|
2012-10-29 01:27:47 | chris.jerdonek | set | messages:
+ msg174094 |
2012-10-24 09:08:41 | asvetlov | set | messages:
+ msg173658 |
2012-10-21 20:12:49 | chris.jerdonek | set | messages:
+ msg173477 |
2012-10-19 23:29:36 | brousch | set | messages:
+ msg173363 |
2012-10-19 22:59:21 | chris.jerdonek | set | messages:
+ msg173362 |
2012-10-19 22:44:21 | asvetlov | set | messages:
+ msg173361 |
2012-10-19 16:18:49 | eric.araujo | set | nosy:
+ eric.araujo
|
2012-10-17 01:12:12 | r.david.murray | set | messages:
+ msg173120 |
2012-10-17 00:45:41 | gregory.p.smith | set | messages:
+ msg173117 |
2012-10-17 00:22:38 | chris.jerdonek | set | nosy:
+ asvetlov, chris.jerdonek
|
2012-10-17 00:06:31 | r.david.murray | set | versions:
+ Python 3.2, Python 3.3, Python 3.4 nosy:
+ gregory.p.smith
messages:
+ msg173115
type: crash -> behavior |
2012-10-16 23:16:29 | brousch | set | messages:
+ msg173109 |
2012-10-16 22:31:07 | rpetrov | set | nosy:
+ rpetrov messages:
+ msg173107
|
2012-10-16 21:24:06 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg173100
|
2012-10-16 21:00:02 | brousch | create | |