3.6.1rc1 adds the current directory to sys.path when running a subdirectory's __main__.py; previous versions did not #73909
Comments
|
3.6.1rc1 adds the current directory to sys.path when running a subdirectory's __main__.py $ pwd
/Users/ned/foo
$ cat main361/__main__.py
import pprint, sys
pprint.pprint(sys.path)
$ for ver in 2.7.13 3.4.6 3.5.3 3.6.0 3.6.1rc1; do
> py=/usr/local/pythonz/pythons/CPython-$ver/bin/python
> $py -V
> $py main361
> done
Python 2.7.13
['main361',
'/usr/local/pythonz/pythons/CPython-2.7.13/lib/python27.zip',
'/usr/local/pythonz/pythons/CPython-2.7.13/lib/python2.7',
'/usr/local/pythonz/pythons/CPython-2.7.13/lib/python2.7/plat-darwin',
'/usr/local/pythonz/pythons/CPython-2.7.13/lib/python2.7/plat-mac',
'/usr/local/pythonz/pythons/CPython-2.7.13/lib/python2.7/plat-mac/lib-scriptpackages',
'/usr/local/pythonz/pythons/CPython-2.7.13/lib/python2.7/lib-tk',
'/usr/local/pythonz/pythons/CPython-2.7.13/lib/python2.7/lib-old',
'/usr/local/pythonz/pythons/CPython-2.7.13/lib/python2.7/lib-dynload',
'/usr/local/pythonz/pythons/CPython-2.7.13/lib/python2.7/site-packages']
Python 3.4.6
['main361',
'/usr/local/pythonz/pythons/CPython-3.4.6/lib/python34.zip',
'/usr/local/pythonz/pythons/CPython-3.4.6/lib/python3.4',
'/usr/local/pythonz/pythons/CPython-3.4.6/lib/python3.4/plat-darwin',
'/usr/local/pythonz/pythons/CPython-3.4.6/lib/python3.4/lib-dynload',
'/usr/local/pythonz/pythons/CPython-3.4.6/lib/python3.4/site-packages']
Python 3.5.3
['main361',
'/usr/local/pythonz/pythons/CPython-3.5.3/lib/python35.zip',
'/usr/local/pythonz/pythons/CPython-3.5.3/lib/python3.5',
'/usr/local/pythonz/pythons/CPython-3.5.3/lib/python3.5/plat-darwin',
'/usr/local/pythonz/pythons/CPython-3.5.3/lib/python3.5/lib-dynload',
'/usr/local/pythonz/pythons/CPython-3.5.3/lib/python3.5/site-packages']
Python 3.6.0
['main361',
'/usr/local/pythonz/pythons/CPython-3.6.0/lib/python36.zip',
'/usr/local/pythonz/pythons/CPython-3.6.0/lib/python3.6',
'/usr/local/pythonz/pythons/CPython-3.6.0/lib/python3.6/lib-dynload',
'/usr/local/pythonz/pythons/CPython-3.6.0/lib/python3.6/site-packages']
Python 3.6.1rc1
['main361',
'/Users/ned/foo',
'/usr/local/pythonz/pythons/CPython-3.6.1rc1/lib/python36.zip',
'/usr/local/pythonz/pythons/CPython-3.6.1rc1/lib/python3.6',
'/usr/local/pythonz/pythons/CPython-3.6.1rc1/lib/python3.6/lib-dynload',
'/usr/local/pythonz/pythons/CPython-3.6.1rc1/lib/python3.6/site-packages']
$ |
|
BTW, I don't know if this is relevant, but PyPy has added the current directory in this situation for a long time. |
|
I think this is a result of the changes introduced with bpo-29319. Perhaps a more prominent NEWS entry is needed? Nick? Ned, does this change in behavior cause problems? |
|
That's not the current directory. It's the 'script' directory (i.e. the parent of main361), as set by PySys_SetArgv when Py_IsolatedFlag isn't set. The old behavior was for RunMainFromImporter to unconditionally replace this directory in sys.path with the import source for "__main__.py", which is the correct behavior, except I think it should try to get an absolute path. The resolution for bpo-29319 changed this such that when sys.path[0] is non-empty it now inserts the import source at index 0 rather than replace index 0. It seems to me that this new logic should only be used in isolated mode. Otherwise it should be replacing this bogus script directory like it used to. |
|
I don't have a specific problem that it causes. It's just a difference from any previous CPython version. About the PyPy thing: it has always behaved this way: $ for ver in 4.0.1 5.1.1 5.4.0 5.6.0; do py=/usr/local/pythonz/pythons/PyPy-$ver/bin/pypy; $py -V; $py main361; done
Python 2.7.10 (5f8302b8bf9f, Nov 18 2015, 10:38:03)
[PyPy 4.0.1 with GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.40)]
['main361',
'/Users/ned/foo',
'/usr/local/pythonz/pythons/PyPy-4.0.1/lib_pypy/__extensions__',
'/usr/local/pythonz/pythons/PyPy-4.0.1/lib_pypy',
'/usr/local/pythonz/pythons/PyPy-4.0.1/lib-python/2.7',
'/usr/local/pythonz/pythons/PyPy-4.0.1/lib-python/2.7/lib-tk',
'/usr/local/pythonz/pythons/PyPy-4.0.1/lib-python/2.7/plat-darwin',
'/usr/local/pythonz/pythons/PyPy-4.0.1/lib-python/2.7/plat-mac',
'/usr/local/pythonz/pythons/PyPy-4.0.1/lib-python/2.7/plat-mac/lib-scriptpackages',
'/usr/local/pythonz/pythons/PyPy-4.0.1/site-packages']
Python 2.7.10 (b0a649e90b66, Apr 28 2016, 08:33:07)
[PyPy 5.1.1 with GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.40)]
['main361',
'/Users/ned/foo',
'/usr/local/pythonz/pythons/PyPy-5.1.1/lib_pypy/__extensions__',
'/usr/local/pythonz/pythons/PyPy-5.1.1/lib_pypy',
'/usr/local/pythonz/pythons/PyPy-5.1.1/lib-python/2.7',
'/usr/local/pythonz/pythons/PyPy-5.1.1/lib-python/2.7/lib-tk',
'/usr/local/pythonz/pythons/PyPy-5.1.1/lib-python/2.7/plat-darwin',
'/usr/local/pythonz/pythons/PyPy-5.1.1/lib-python/2.7/plat-mac',
'/usr/local/pythonz/pythons/PyPy-5.1.1/lib-python/2.7/plat-mac/lib-scriptpackages',
'/usr/local/pythonz/pythons/PyPy-5.1.1/site-packages']
Python 2.7.10 (77392ad26350, Aug 28 2016, 05:07:44)
[PyPy 5.4.0 with GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.40)]
['main361',
'/Users/ned/foo',
'/usr/local/pythonz/pythons/PyPy-5.4.0/lib_pypy/__extensions__',
'/usr/local/pythonz/pythons/PyPy-5.4.0/lib_pypy',
'/usr/local/pythonz/pythons/PyPy-5.4.0/lib-python/2.7',
'/usr/local/pythonz/pythons/PyPy-5.4.0/lib-python/2.7/lib-tk',
'/usr/local/pythonz/pythons/PyPy-5.4.0/lib-python/2.7/plat-darwin',
'/usr/local/pythonz/pythons/PyPy-5.4.0/lib-python/2.7/plat-mac',
'/usr/local/pythonz/pythons/PyPy-5.4.0/lib-python/2.7/plat-mac/lib-scriptpackages',
'/usr/local/pythonz/pythons/PyPy-5.4.0/site-packages']
Python 2.7.12 (aff251e54385, Nov 09 2016, 17:25:49)
[PyPy 5.6.0 with GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.40)]
['main361',
'/Users/ned/foo',
'/usr/local/pythonz/pythons/PyPy-5.6.0/lib_pypy/__extensions__',
'/usr/local/pythonz/pythons/PyPy-5.6.0/lib_pypy',
'/usr/local/pythonz/pythons/PyPy-5.6.0/lib-python/2.7',
'/usr/local/pythonz/pythons/PyPy-5.6.0/lib-python/2.7/lib-tk',
'/usr/local/pythonz/pythons/PyPy-5.6.0/lib-python/2.7/plat-darwin',
'/usr/local/pythonz/pythons/PyPy-5.6.0/lib-python/2.7/plat-mac',
'/usr/local/pythonz/pythons/PyPy-5.6.0/lib-python/2.7/plat-mac/lib-scriptpackages',
'/usr/local/pythonz/pythons/PyPy-5.6.0/site-packages']
$ for ver in 2.4.0 5.5.0; do py=/usr/local/pythonz/pythons/PyPy3-$ver/bin/pypy3; $py -V; $py main361; done
Python 3.2.5 (b2091e973da6, Oct 19 2014, 18:30:58)
[PyPy 2.4.0 with GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.51)]
['main361',
'/Users/ned/foo',
'/usr/local/pythonz/pythons/PyPy3-2.4.0/lib_pypy/__extensions__',
'/usr/local/pythonz/pythons/PyPy3-2.4.0/lib_pypy',
'/usr/local/pythonz/pythons/PyPy3-2.4.0/lib-python/3',
'/usr/local/pythonz/pythons/PyPy3-2.4.0/lib-python/3/lib-tk',
'/usr/local/pythonz/pythons/PyPy3-2.4.0/lib-python/3/plat-darwin',
'/usr/local/pythonz/pythons/PyPy3-2.4.0/lib-python/3/plat-mac',
'/usr/local/pythonz/pythons/PyPy3-2.4.0/lib-python/3/plat-mac/lib-scriptpackages',
'/usr/local/pythonz/pythons/PyPy3-2.4.0/site-packages']
Python 3.3.5 (619c0d5af0e5, Oct 08 2016, 22:08:19)
[PyPy 5.5.0-alpha0 with GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.40)]
['main361',
'/Users/ned/foo',
'/usr/local/pythonz/pythons/PyPy3-5.5.0/lib_pypy/__extensions__',
'/usr/local/pythonz/pythons/PyPy3-5.5.0/lib_pypy',
'/usr/local/pythonz/pythons/PyPy3-5.5.0/lib-python/3',
'/usr/local/pythonz/pythons/PyPy3-5.5.0/lib-python/3/lib-tk',
'/usr/local/pythonz/pythons/PyPy3-5.5.0/lib-python/3/plat-darwin',
'/usr/local/pythonz/pythons/PyPy3-5.5.0/lib-python/3/plat-mac',
'/usr/local/pythonz/pythons/PyPy3-5.5.0/lib-python/3/plat-mac/lib-scriptpackages',
'/usr/local/pythonz/pythons/PyPy3-5.5.0/site-packages']
$ |
Adding an arbitrary directory ahead of the standard library in sys.path is a problem waiting to happen. This also happens when the import source is a zip file, e.g. a .pyz app. |
|
I think Eryk's diagnosis is correct: this is a straight up bug in the original patch, where it's missing the check to only use the new logic when in isolated mode (as it's compensating for the fact that there is no extra sys.path[0] entry inserted in that case). I mentioned that on the original issue http://bugs.python.org/issue29319#msg285904 but hadn't noticed that the relevant check was missing from the applied patch. |
|
Thanks for the analysis, Eryk and Nick. Then this sounds like a release blocker problem that should be fixed for 3.6.1. Steve, can you take a look, please? |
|
It's actually "adding" the current directory by not replacing the empty string that's normally there, presumably because it's already been resolved into a path at this stage. The behavior on Windows is correct, so I expect it's actually a difference between getpath.c and getpathp.c, rather than an isolated/non-isolated mode issue. I'll try and take a look - my plate is fairly full over the next few weeks though, so if anyone is able to help out I'd appreciate it, especially since it's not my usual platform. |
It's the script directory, which gets added on all platforms when PySys_SetArgv is called with Py_IsolatedFlag == 0. In this case we're running "__main__.py" from a directory or zip file, and we don't want its parent directory in sys.path.
Here's what I see on a current build of 3.6: C:\Temp>python_d -i main361
>>> import sys
>>> sys.version
'3.6.0+ (default, Mar 8 2017, 06:51:22) [MSC v.1900 64 bit (AMD64)]'
>>> sys.path[:2]
['main361', 'C:\\Temp']C:\Temp doesn't belong in sys.path in this case. When we're not in isolated mode, RunMainFromImporter should set the __main__.py import source as index 0 rather than insert it. |
|
Ah, interesting, I didn't know there was a difference between the platforms in when the placeholder got resolved to a full path. However, after browsing the code and running some local tests, it seems that injecting sys.path[0] isn't handled by Py_GetPath() or PySys_SetPath(), regardless of OS. Instead, it's handled as a side effect of calling PySys_SetArgV(), with PySys_SetArgVEx() adding a flag to disable the side effect (for the benefit of the isolated mode implementation). The actual side effect itself is implemented in sys_update_path: https://github.com/python/cpython/blob/3.6/Python/sysmodule.c#L2162 So in the case of running The first line is from In this scenario, we want the "Tools" entry to *overwrite* sys.path[0]. While in isolated mode I get: In this scenario, we want the "Tools" entry to be inserted *before* sys.path[0]. Note that this has been buggy since the -I switch was introduced, but the entry we've been overwriting has been the one for the stdlib-as-a-zip-archive, so it didn't cause any problems for anyone using the default directory-based installation layout. However, we can't reliably figure out which to do based on the contents of sys.path, we need to look at Py_IsolatedFlag instead. (I'm not sure why the current check is working on Windows, as sys_update_path() attempts to resolve an absolute path entry from the archive or directory name there as well) |
|
So a potentially more robust fix here would be to always call That way RunMainFromImporter could just *always* insert at the front, and not have to try to guess whether or not a different sys.path[0] entry had already been populated. This would involve splitting RunMainFromImporter into two pieces (one to check whether the given filename is a valid sys.path entry, the second to actually modify sys.path and run __main__), but that's entirely feasible. |
Hang on, why not? If I were running a module.py then it would be, so why is a package\main.py different (and not able to import itself or its siblings)? The package is the "script" being run here, yes? Or is __main__.py different from __init__.py in this regard? (That is, it isn't *part* of the package, it's just a loose script that happens to inherit the name of its directory) |
|
main361 can be run as a package from the current directory via -m because an empty string is in sys.path, i.e. the current directory. It imports the package, executing __init__.py and then __main__.py. In this case, the main361 directory/zip import source is not added to sys.path. When we run main361 as a script, the main361 package is not imported, i.e. __init__.py is not executed. The siblings of the __main__.py script are in the main361 directory (or zip file). Effectively this is the script directory, and it does belong in sys.path -- even in isolated mode because we trust the directory or zip file that contains __main__.py. A potential problem, however, is that we don't ensure the __main__ import source is added as an absolute path. So if the script changes the working directory, then it loses this import source and possibly (unlikely) gains a new import source if the relative path exists relative to the new working directory. |
|
As Eryk notes, this has revealed a separate bug in that we don't make the path absolute when adding to sys.path. However, that's *not* directly related to the regression, so I'm resisting the temptation to change it here. Instead, the new test case I'm adding will just run a script directory both with and without -I, and make sure sys.path ends up the same in both cases. |
|
Work in progress PR at #571 I also came up with a reasonably straightforward way of defining the desired "sys.path" initialisation behaviour, which is that the following should all get the *same* sys.path entries: None of those should have the current directory on sys.path, but they should all have They all differ from the |
|
Slightly correction, as I didn't have the isolated mode equivalents quite correct: Isolated mode still runs the system site.py, which turns out to be signficant in a development checkout (getpath includes the Lib/ directory twice, and site.py then cleans out the duplicate). So I think this PR is the right fix, but need confirmation from Steve that it still solves the original problem reported in bpo-29319. (Note the current PR is open against the 3.6 branch - I'm going to make a new single-commit one against master, flagging it for cherry-picking, then close this one) |
|
OK, #575 is the new PR that covers everything needed to fix the root cause of the problem (which was the entirely unnecessary add-and-overwrite dance that zipfile and directory executation was doing for sys.path[0]) |
|
Paul Moore reporting on trying out the new PR in http://bugs.python.org/issue29319#msg289357 and confirmed that it still solves the originally reported problem in bpo-29319. |
|
I've merged PR 575 to master, and 636 is pending a successful Travis run for 3.6. Both branches still need NEWS entries- I'll do those as independent PRs rather than cherry-picking (since cherry picking NEWS changes is currently still doomed to fail) |
|
Noting for the record: Steve reviewed & approved the original PR before I merged that and did the backport to 3.6. With the new test case ensuring sys.path configuration consistency and Paul's report of success when checking the original problem from bpo-29319 was resolved, I think this is OK to just go out with 3.6.1 final rather than needing an rc2 release. |
|
Thanks, Nick! I agree that this alone isn't worth a second RC, though if we fix another "nearly worth it" (especially anything related to the github transition) we may want to consider it still. Ned - I'd be okay to sneak out another release this week if we want, without slipping the final release. |
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: