Author akira
Recipients WanderingLogic, akira, asvetlov, chris.jerdonek, christian.heimes, eric.araujo, eryksun, ezio.melotti, gregory.p.smith, lyapun, ned.deily, neologix, pitrou, r.david.murray, serhiy.storchaka
Date 2014-11-06.00:47:47
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <8761et6tqg.fsf@gmail.com>
In-reply-to <1415217675.7.0.0765393995642.issue16353@psf.upfronthosting.co.za> (Matt Frank's message of "Wed, 05 Nov 2014 20:01:15 +0000")
Content
> Matt Frank added the comment:
>
> Unfortunately os.defpath seems to be hardcoded.  And hardcoded to the
> wrong value on every system I have looked at, including Linux.

os.defpath is supposed to be ':'+CS_PATH, e.g., look at glibc (C library
used on Linux) sysdeps/posix/spawni.c I don't know whether it is
possible to change CS_PATH without recompiling every statically linked
executable on a system, sysdeps/unix/confstr.h:

  #define CS_PATH "/bin:/usr/bin"

Though this issue is about the path to the standard shell sh/cmd.exe.
It is not about os.defpath.

The patch [1] has tests. Have you tried to run them?
The tests *pass* at least on one system ;)

[1] http://bugs.python.org/review/16353/#ps12569

To run the tests, download the patch into your python checkout:

  $ curl -O \
  http://bugs.python.org/file36196/os.get_shell_executable.patch

and apply it:

  $ patch -p1< os.get_shell_executable.patch

to run only test_os.py:

  $ ./python -mtest test_os

> Lib/posixpath.py sets defpath=':/bin:/usr/bin' which is _not_ what
> getconf CS_PATH` returns on my Linux (the extra ':' at the beginning
> means "include the cwd" which is almost certainly not what anyone
> wants.) 

os.get_shell_executable() does not use cwd. Neither the documentation
nor the code in the patch suggest that.

> The hardcoded value '/bin:/usr/bin' would also be wrong for
> Solaris and AIX installations that are trying to be POSIX (I think
> they use /usr/xpg4/bin/sh).

os.defpath is the default search path used by exec*p* and spawn*p*
i.e., if os.defpath is incorrect for these system then os.exec*p* and
os.spawn*p* functions could also be broken.

I expect that Windows, OS X, Linux work as is. If the tests fail on
Solaris, AIX then os.defpath should be tweaked on these systems if it
hasn't been already. Note: os.defpath is a very conservative value: it
should be set at python's installation time at the very
latest. Henceforth it should remain the same.

> And Lib/ntpath.py sets defpath='.;C:\\bin', which doesn't resemble a
> path that even works (as pointed out in now closed
> http://bugs.python.org/issue5717).  (The correct value may be
> %SystemRoot%\system32;%SystemRoot%;%SystemRoot%\System32\Wbem;%SYSTEMROOT%\System32\WindowsPowerShell\v1.0\'.)

Please, look at the patch. Neither the documentation nor the code
suggest that os.defpath is used on Windows.

> So I don't know where to go next.  I'm happy to cook up a different
> patch, but I have no idea what it should be.  Here are some
> possibilities:
>
> 1. Kick the can down the road: file a new bug against os.defpath and
> (somehow) fix Lib/*path.py so they do something more reasonable in
> more cases.  One of which might be to have posixpath.py try to call
> os.confstr() (and then, perhaps, special-code something in
> Modules/posixmodule.c in the case that HAVE_CONFSTR is not defined.)
>
> 2. Modify @akira's patch to call os.confstr('CS_PATH') instead of
> os.defpath, and then deal with the fact that very few systems actually
> implement confstr() (perhaps by special-coding something in
> Modules/posixmodule.c as described above.)

Note I'm the author of http://bugs.python.org/issue16353#msg224514
message that references the POSIX recommendation about `getconf PATH`
and *explicitly* mentions os.confstr('CS_PATH').

I don't remember exactly the motivation to use os.defpath instead of
os.confstr('CS_PATH').

A guess: the result is the same (a lone `:` is ignored in the patch) but
os.defpath is easier to modify for uncommon systems where os.confstr
might not be available.

I expect that the tests in the patch will pass on all stable buildbots
[2] successfully without any modifications (with os.defpath as is).

[2] http://buildbot.python.org/all/waterfall?category=3.x.stable

Other systems should probably configure os.defpath appropriately (it
should include the path to the standard shell).

> 3. Modify this patch to fall back to `PATH` if `sh` can't be found on
> os.defpath (or os.confstr('CS_PATH') fails).

The standard shell should not depend on PATH envvar. The tests show that
os.get_shell_executable() works even with an empty environment. The
motivation is the same as why `getconf PATH` exists in the first place.

The documentation for os.get_shell_executable() (from the patch):

   Return a path to the standard system shell executable -- ``sh``
   program on POSIX (default: ``'/bin/sh'``) or ``%ComSpec%`` command
   processor on Windows (default: ``%SystemRoot%\system32\cmd.exe``).

   Availability: Unix, Windows

The intent is that os.get_shell_executable() returns the same *shell
path* as used by system(3) on POSIX i.e., (ignoring signals,
etc) os.system(command) could be implemented:

  subprocess.call(command, shell=True, executable=os.get_shell_executable())

(presumably subprocess can use get_shell_executable() internally instead
of /bin/sh but it is another issue)
History
Date User Action Args
2014-11-06 00:47:47akirasetrecipients: + akira, gregory.p.smith, pitrou, christian.heimes, ned.deily, ezio.melotti, eric.araujo, r.david.murray, asvetlov, chris.jerdonek, neologix, serhiy.storchaka, eryksun, lyapun, WanderingLogic
2014-11-06 00:47:47akiralinkissue16353 messages
2014-11-06 00:47:47akiracreate