New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IDLE sys.path does not contain Current Working Directory #57715
Comments
|
The IDLE shell sys.path does not contains any entry for the Current Working Directory ('' or '.' or '.\'); without it, when changing the CWD with os.chdir(), the shell cannot find, execute or import scripts or module in it. I can start the standard Python interactive shell and os.chdir to any dev or test directories and import or call scripts and modules, or I can cd first to any of those directories from any shell (sh, batch, cmd.com), then start Python interactive shell and import or call scripts and modules, because there is the CWD available in the sys.path as ''. I tried to manually add it to IDLE: from cli by calling idle.py or idle.pyw with "-cimport sys;sys.path.insert(0,"");del sys" or by making a IDLESTARTUP.py script import sys
sys.path.insert(0,"")
del sysand setting the IDLESTARTUP env var pointing at it but these work in adding the CWD to sys.path only for the first run (start) of IDLE shell; but when it get restarted (ex.: the Shell/Restart Shell toolbar option) the CWD get reset without the CWD entry, and again it has the same problem I also tried to cd from shell (sh, bash, cmd.com) first and then start IDLE and it worked for importing and calling modules and scripts in the specific dev/test directory, but it failed to import or call other standard scripts that actually are in the sys.path (ex.: win32ui). This is needed to use IDLE to develop and test new scripts and modules not yet ready to be included into the standard libraries and paths, so momentarily modifying the path as above that also does not survive restarts made within the application, or permanently modifying the path statically to include any dev and test dir by adding the absolute path(s ) to env var PYTHONPATH are both not correct way. And anyway it is not consistent with the behavior of the standard Python interactive shell that includes the CWD in sys.path (as '' right at the beginning), therefore os.chdir('to any non in sys.path dirs') works correctly in contrast with IDLE shell behavior... and who knows what else it breaks. I am not an expert on python environment, but I have 20+ years professional experience in many other high profile QA dev and testing project (just Google me), so I just thing it should be fixed, since I see this problem since 2008 and I know others that have the same issue for long time before that. If there is anything that I don't see just let me know, |
|
... I think it could be fixed by adding sys.path.insert(0,"")on the # process sys.argv and sys.path: section in PyShell.py after (~about) line 1383 sys.path.insert(0, dir)
sys.path.insert(0,"") <----HERE
# check the IDLE settings configuration (but command line overrides)(Unified diff/patch file attached) |
|
+1 The proposed patch works as described. I do agree with Marco that IDLE does need some more QA. |
On 3.2.2, I get
>>> import sys
>>> sys.path
['C:\\Programs\\Python32\\Lib\\idlelib', 'C:\\Windows\\system32\\python32.zip', 'C:\\Programs\\Python32\\DLLs', 'C:\\Programs\\Python32\\lib', 'C:\\Programs\\Python32', 'C:\\Programs\\Python32\\lib\\site-packages', 'F:\\Python']so I presume the fix should include 3.2 and 3.3. |
|
I have given this issue some further consideration, and realized that my "+1" recommendation was not correct. Starting the interactive python interpreter will automatically include [''] in sys.path. A fresh restart of IDLE's shell does NOT include ''. However, running a python script will have sys.path include the absolute path to the script. IDLE's shell does do this in conjunction with ScriptBinding's "Run Module". The new patch preserves the existing correct behavior, but also fixes the issue. |
|
At first I did no see the difference on preserving the existing correct behavior and fixing the issue between the two patches... and I thought less is more, so mine was better... But, I checked again and by: I can see the reasoning for it and since that IS the standard Python behavior it should be also transposed to IDLE. So yes, Roger (serwy) patch presents a more accurate correction. +1 |
|
I test Roger patch on 2.7.2.1 and 3.2.1.1 fresh installs on Win7 Pro 32bit and I confirm it is fixing the issue and it also keeps the existing correct behavior. ----- patching file PyShell.py |
|
I tested on 3.2, Win 7. sys.path starts with '' on startup, after restart, and after ^c interrupt of 'while True: pass'. It start with absolute path when running a file from the editor. I believe this is as should be. I simplified the patch a bit. Its basic idea is to add a parameter with_cwd to PyShell.ModifiedInterpreter.transfer_path. That method is called by methods start_subprocess and restart_subprocess. start_process is only called by PyShell.PyShell.begin. So I think there is no need to propagate the new parameter back past the call in start_subprocess. .restart_subprocess is called in .poll_subprocess, .runcode, PyShell.cancel_process, and PyShell.restart_shell. (I am a little puzzled why ModifiedInterpreter.runcode calls self.interp.restart_subprocess, as in the PyShell methods, instead of self.restart_subprocess, as in poll_subprocess, but moving on...) restart_process needs the new parameter to pass to transfer_path because it should be True for F6 Restart Shell and not for F5 Run Module. Roger's patch also left the default False for the other calls. I do not know what they do so I cannot judge them. Roger, do you know, and did you consider each? (Leaving the current behavior for those will at worst not fix something that could be, so it is safe.) restart_shell is *bound* to F6 Restart Shell (which passes the ignored even), so the default with_cwd must be True for that binding. It is *called* by a function ScriptBinding.py that gets bound, so the call can be altered to pass the correct False. I prefer that to making two functions. That call could be replaces by a call directly to .interp.restart_subprocess(False) (and restart_shell slightly simplified). I retested after these changes to the patch. It is from TortoiseHG, so should apply (after I modified the file paths). Since hacking on idlelib is new for me, I would like other eyes and tests before I apply. |
|
Whoops, hit submit too soon, before saving tab to space change. |
|
I tested your patch and it works. For the sake of completeness, here's what I did: Test 1: Start IDLE in shell mode and run "import sys; print(sys.path)". The first entry should be '' This is consistent with the behavior of the regular python shell. Test 2: Restart the shell with Ctrl+F6 (and View->Restart Shell) and repeat Test 1. This should give the same result. Test 3: Create a script in /tmp/a.py with "import sys; print(sys.path)" as its contents. Running this script from the editor gives '/tmp' as the first entry. This is consistent with the behavior of running "python3 /tmp/a.py". I'd make one minor change to the patch for sake of being explicit in ScriptBinding.py: My rationale was to modify only PyShell.py to get the correct behavior. Clearly, making a simple modification to ScriptBinding.py gives a much shorter solution. I also like your simplification for "start_subprocess". I did consider each case of default arguments. I developed my patch iteratively starting by having "with_cwd=False" for all function calls that called "transfer_path" so that the existing (incorrect) behavior remained. I then modified how these functions were called, setting "with_cwd=True" in the cases where '' needed to be transferred, namely on the shell start and restart from within the PyShell window. That is why I introduced "restart_shell_interactive". I do agree with your reasoning that one function call is better than two. |
|
New changeset 1b5abba0c808 by Terry Jan Reedy in branch '2.7': New changeset 1993aa091d89 by Terry Jan Reedy in branch '3.2': New changeset acedd92086c5 by Terry Jan Reedy in branch 'default': |
|
I noticed that the script in ScriptBinding.py already dereferences interp and calls 3 interp functions directly, so I decided that calling a 4th on interp was fine. That lets restart_shell be specialized to the one purpose of being bound to Restart Shell on the menu. I added a docstring to that effect so no will will try to generalize it again to use when running a module. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: