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.20:18:30
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <87zjc45bja.fsf@gmail.com>
In-reply-to <1415298314.32.0.0430576457537.issue16353@psf.upfronthosting.co.za> (Matt Frank's message of "Thu, 06 Nov 2014 18:25:14 +0000")
Content
> Matt Frank added the comment:
>
> In msg230720 Akira Li (akira) wrote:
>> 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.
>
> I understand this issue is about the path to the standard shell.
> My argument is that os.defpath is the wrong tool for finding that
> path, and os.confstr('CS_PATH') is the correct tool, as you originally
> suggested in msg224514.
>

I'll repeat os.defpath definition that I've cited in the same message
msg224514:

  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.

1. os.defpath should work (contain the path to the standard shell)
   everywhere where os.confstr("CS_PATH") works.

2. And it might be easier to customize e.g., on systems where there is
   no os.confstr or where changing CS_PATH involves recompiling crucial
   system processes.

If these two statements are not true then os.defpath could be dropped.

>> The patch [1] has tests. Have you tried to run them?
>> The tests *pass* at least on one system ;)
>>
>> [...]
>>
>> os.get_shell_executable() does not use cwd. Neither the documentation
>> nor the code in the patch suggest that.
>
> I ran the tests (and the entire Python test suite) before I wrote my
> first message.  But you didn't test what happens when there is a bogus
> 'sh' somewhere along the path.  You also have a bug in your path
> searching loop that interprets an empty element on the path as "/".
> Here's the current failure mode:
>
> Go to root on your Posix system.  With "su" or "sudo" create an
> executable file named sh.  (for example "cd /; sudo touch sh; sudo
> chmod +x sh").  Then go back to your python interpreter (with the
> patch applied) and run "os.get_shell_executable()".  The returned
> result will be '/sh'.
> But if you fix the loop to iterate over the path correctly (including
> 'cwd') then you will find that putting an executable named 'sh' in the
> current working directory will make os.get_shell_executable() return
> the "sh" in the current working directory (because os.defpath says it
> should).
>

Comment in the patch explicitly says that '' is interpreted as '/'. The
intent is to exclude the current working directory, thinking that /sh
executable can create only root. And root can do anything to the machine
anyway.

I agree. It is a misfeature. I've uploaded a new patch that excludes ''
completely. The previous code tried to be too cute.

> The loop _should_ be treating empty path elements as current working
> directory.  (In the glibc sysdeps/posix/spawni.c file you pointed to
> look around line 281, by the comment that says "/* Two adjacent
> colons, or a colon at the beginning or the end of 'PATH' means to
> search the current directory.*/")

yes. Empty path element stands for the current working directory e.g.,

   $ PATH= python
   $ PATH=: python
   $ PATH=:/ python
   $ PATH=/: python

run ./python on my system.

The current working directory is intentionally excluded (in the original
and in the current patches) otherwise os.get_exec_path(env={}) would be
used.

> Note also, that glibc's spawni() uses two different "default paths".
> One is for if the user didn't specify their PATH variable (that's the
> one where they use ":/bin:/usr/bin") and a completely different (built
> in path is used in the spawn*p* version) if it turns out that the file
> is a script that needs to run under the system sh.  (On line 290 they
> call maybe_script_execute(), which calls script_execute(), which uses
> _PATH_BSHELL.)

glibc hardcodes the path (like subprocess module does currently):

  #define	_PATH_BSHELL	"/bin/sh"

os.get_shell_executable() is an enhancement that allows
(Unix) systems to configure the path via os.defpath.

> But os.defpath doesn't work on Android either (because it is hardcoded
> to ':/bin:/usr/bin').

As I've mentioned in msg224514 before posting my original patch:

- "Andriod uses /system/bin/sh"
- "os.defpath could be tweaked on Android"

i.e., yes. Default os.defpath doesn't work there and it should be
configured on Android to include the path to the standard shell.
History
Date User Action Args
2014-11-06 20:18:31akirasetrecipients: + 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 20:18:31akiralinkissue16353 messages
2014-11-06 20:18:30akiracreate