Akira Li added the comment: > 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. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue16353> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com