Author akira
Recipients akira, asvetlov, chris.jerdonek, christian.heimes, eric.araujo, eryksun, ezio.melotti, gregory.p.smith, lyapun, neologix, pitrou, r.david.murray, serhiy.storchaka
Date 2014-08-01.19:11:53
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1406920314.29.0.837965155557.issue16353@psf.upfronthosting.co.za>
In-reply-to
Content
> Should it be function? Why not use just a variable initialized at
> import time? The path to the default shell shouldn't change during
> the time of program execution.

> if sys.platform == 'win32':
>     default_shell = 'cmd.exe'
> else:
>    default_shell = '/bin/sh'

Andriod uses /system/bin/sh (and /sbin/sh if we believe some adb
source on the internet). See issue16255: "subrocess.Popen needs
/bin/sh but Android only has /system/bin/sh"

POSIX recommends [1] `getconf PATH` (os.confstr('CS_PATH')):

  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.

  For example, to determine the location of the standard sh utility:

    $ command -v sh

i.e. shell executable is shutil.which('sh', os.defpath) on POSIX.
os.defpath could be tweaked on Android.

To avoid dependency on shutil something like msg174628 could be
adopted:

  if sys.platform == "win32": # use sys.platform to accommodate java
      def get_shell_executable():
          """Return path to the command processor specified by %ComSpec%.

          Default: %SystemRoot%\system32\cmd.exe
          """
          return (environ.get('ComSpec') or
                  path.join(environ.get('SystemRoot', r'C:\Windows'),
                            r'system32\cmd.exe'))
  else: # POSIX
      def get_shell_executable():
          """Return path to the standard system shell executable.

          Default: '/bin/sh'
          """
          for dirpath in defpath.split(pathsep):
              sh = dirpath + sep + 'sh' #NOTE: interpret '' as '/'
              if access(sh, F_OK | X_OK) and path.isfile(sh):
                  #NOTE: allow symlinks e.g., /bin/sh on Ubuntu may be dash
                  return sh
          return '/bin/sh' #XXX either OS or python are broken if we got here

Windows part is based on msg224512. If r'C:\Windows' or '/bin/sh' code
is reached then it means that either OS or python are
broken/misconfigured.

system(), popen() are described on POSIX as [2]:

  # fork()
  execl(shell path, "sh", "-c", command, (char *)0);

subprocess module that replaces system, popen, spawn*p* calls may use
os.get_shell_executable() to implement shell=True.

os.defpath is [3]:

  The default search path used by exec*p* and spawn*p* if the
  environment doesn’t have a 'PATH' key. Also available via os.path.

In the absense of os.confstr('CS_PATH') on Android (msg175006),
os.defpath seems appropriate than os.environ['PATH'] to expand 'sh'
basename to the full path to be used by subprocess later.

os.get_shell() name might imply that a shell object is returned that
can run commands like for example os.popen() returns a file-like
object that is not true (the intent is to return a path -- a simple
string).

os.get_shell_executable() is similar to sys.executable that returns
path to python executable.

os.get_shell_executable() is not necessary for every python run
therefore it is better to keep it a function to avoid unnecessary stat
calls on startup. Though the result should not change during the
program run.

> On Windows the ComSpec environment variable could change during
> program execution.

Does it? The docs for os.environ say [4]:

  This mapping is captured the first time the os module is imported,
  typically during Python startup as part of processing
  site.py. Changes to the environment made after this time are not
  reflected in os.environ, except for changes made by modifying
  os.environ directly.

I've uploaded a patch with the described implementation (plus docs, tests).

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sh.html
[2] http://pubs.opengroup.org/onlinepubs/9699919799/functions/popen.html
[3] https://docs.python.org/3.4/library/os.html#os.defpath
[4] https://docs.python.org/3.4/library/os.html#os.environ
History
Date User Action Args
2014-08-01 19:11:54akirasetrecipients: + akira, gregory.p.smith, pitrou, christian.heimes, ezio.melotti, eric.araujo, r.david.murray, asvetlov, chris.jerdonek, neologix, serhiy.storchaka, eryksun, lyapun
2014-08-01 19:11:54akirasetmessageid: <1406920314.29.0.837965155557.issue16353@psf.upfronthosting.co.za>
2014-08-01 19:11:54akiralinkissue16353 messages
2014-08-01 19:11:53akiracreate