From 4eb3cf79214337d3f45deadc1d00d873bcf2e135 Mon Sep 17 00:00:00 2001 From: immerrr Date: Sun, 5 Apr 2015 00:19:11 +0200 Subject: [PATCH 1/8] Improve virtualenv support & egg-link resolution - add sys_path= kwarg to Script & Evaluator constructors - store sys_path for each evaluator instance - replace get_sys_path with get_venv_path - get_venv_path: use addsitedir to load .pth extension files - get_venv_path: look for egg-link files in all directories in path --- jedi/api/__init__.py | 10 ++- jedi/evaluate/__init__.py | 10 ++- jedi/evaluate/compiled/__init__.py | 12 +-- jedi/evaluate/imports.py | 7 +- jedi/evaluate/sys_path.py | 66 ++++++++++---- test/test_evaluate/test_namespace_package.py | 90 +++++++++---------- test/test_evaluate/test_sys_path.py | 37 ++++++-- .../dir-from-foo-pth/__init__.py | 2 + .../site-packages/egg_link.egg-link | 0 .../lib/python2.7/site-packages/foo.pth | 1 + .../python2.7/site-packages/import_smth.pth | 1 + .../python2.7/site-packages/relative.egg-link | 1 + .../lib/python2.7/site-packages/smth.py | 6 ++ .../dir-from-foo-pth/__init__.py | 2 + .../python3.4/site-packages/egg_link.egg-link | 1 + .../lib/python3.4/site-packages/foo.pth | 1 + .../python3.4/site-packages/import_smth.pth | 1 + .../python3.4/site-packages/relative.egg-link | 1 + .../lib/python3.4/site-packages/smth.py | 6 ++ test/test_regression.py | 5 +- 20 files changed, 174 insertions(+), 86 deletions(-) create mode 100644 test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/dir-from-foo-pth/__init__.py rename test/test_evaluate/{egg-link/venv/lib/python3.4 => venvs/venv27/lib/python2.7}/site-packages/egg_link.egg-link (100%) create mode 100644 test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/foo.pth create mode 100644 test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/import_smth.pth create mode 100644 test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/relative.egg-link create mode 100644 test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/smth.py create mode 100644 test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/dir-from-foo-pth/__init__.py create mode 100644 test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/egg_link.egg-link create mode 100644 test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/foo.pth create mode 100644 test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/import_smth.pth create mode 100644 test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/relative.egg-link create mode 100644 test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/smth.py diff --git a/jedi/api/__init__.py b/jedi/api/__init__.py index a5b0b109..de7580d7 100644 --- a/jedi/api/__init__.py +++ b/jedi/api/__init__.py @@ -34,6 +34,7 @@ from jedi.evaluate.cache import memoize_default from jedi.evaluate.helpers import FakeName, get_module_names from jedi.evaluate.finder import global_names_dict_generator, filter_definition_names from jedi.evaluate import analysis +from jedi.evaluate.sys_path import get_venv_path # Jedi uses lots and lots of recursion. By setting this a little bit higher, we # can remove some "maximum recursion depth" errors. @@ -75,7 +76,8 @@ class Script(object): :type encoding: str """ def __init__(self, source=None, line=None, column=None, path=None, - encoding='utf-8', source_path=None, source_encoding=None): + encoding='utf-8', source_path=None, source_encoding=None, + sys_path=None): if source_path is not None: warnings.warn("Use path instead of source_path.", DeprecationWarning) path = source_path @@ -109,7 +111,11 @@ class Script(object): self._parser = UserContextParser(self._grammar, self.source, path, self._pos, self._user_context, self._parsed_callback) - self._evaluator = Evaluator(self._grammar) + if sys_path is None: + venv = os.getenv('VIRTUAL_ENV') + if venv: + sys_path = list(get_venv_path(venv)) + self._evaluator = Evaluator(self._grammar, sys_path=sys_path) debug.speed('init') def _parsed_callback(self, parser): diff --git a/jedi/evaluate/__init__.py b/jedi/evaluate/__init__.py index a959d05f..37c20fc1 100644 --- a/jedi/evaluate/__init__.py +++ b/jedi/evaluate/__init__.py @@ -61,6 +61,7 @@ that are not used are just being ignored. """ import copy +import sys from itertools import chain from jedi.parser import tree @@ -79,7 +80,7 @@ from jedi.evaluate import helpers class Evaluator(object): - def __init__(self, grammar): + def __init__(self, grammar, sys_path=None): self.grammar = grammar self.memoize_cache = {} # for memoize decorators # To memorize modules -> equals `sys.modules`. @@ -88,6 +89,13 @@ class Evaluator(object): self.recursion_detector = recursion.RecursionDetector() self.execution_recursion_detector = recursion.ExecutionRecursionDetector() self.analysis = [] + if sys_path is None: + sys_path = sys.path + self.sys_path = copy.copy(sys_path) + try: + self.sys_path.remove('') + except ValueError: + pass def wrap(self, element): if isinstance(element, tree.Class): diff --git a/jedi/evaluate/compiled/__init__.py b/jedi/evaluate/compiled/__init__.py index 67d3f371..7224067f 100644 --- a/jedi/evaluate/compiled/__init__.py +++ b/jedi/evaluate/compiled/__init__.py @@ -10,7 +10,6 @@ from functools import partial from jedi._compatibility import builtins as _builtins, unicode from jedi import debug from jedi.cache import underscore_memoization, memoize_method -from jedi.evaluate.sys_path import get_sys_path from jedi.parser.tree import Param, Base, Operator, zero_position_modifier from jedi.evaluate.helpers import FakeName from . import fake @@ -309,15 +308,12 @@ class CompiledName(FakeName): pass # Just ignore this, FakeName tries to overwrite the parent attribute. -def dotted_from_fs_path(fs_path, sys_path=None): +def dotted_from_fs_path(fs_path, sys_path): """ Changes `/usr/lib/python3.4/email/utils.py` to `email.utils`. I.e. compares the path with sys.path and then returns the dotted_path. If the path is not in the sys.path, just returns None. """ - if sys_path is None: - sys_path = get_sys_path() - if os.path.basename(fs_path).startswith('__init__.'): # We are calculating the path. __init__ files are not interesting. fs_path = os.path.dirname(fs_path) @@ -341,13 +337,13 @@ def dotted_from_fs_path(fs_path, sys_path=None): return _path_re.sub('', fs_path[len(path):].lstrip(os.path.sep)).replace(os.path.sep, '.') -def load_module(path=None, name=None): +def load_module(evaluator, path=None, name=None): + sys_path = evaluator.sys_path if path is not None: - dotted_path = dotted_from_fs_path(path) + dotted_path = dotted_from_fs_path(path, sys_path=sys_path) else: dotted_path = name - sys_path = get_sys_path() if dotted_path is None: p, _, dotted_path = path.partition(os.path.sep) sys_path.insert(0, p) diff --git a/jedi/evaluate/imports.py b/jedi/evaluate/imports.py index c14a50c0..3c226866 100644 --- a/jedi/evaluate/imports.py +++ b/jedi/evaluate/imports.py @@ -342,7 +342,7 @@ class Importer(object): module_file.close() if module_file is None and not module_path.endswith('.py'): - module = compiled.load_module(module_path) + module = compiled.load_module(self._evaluator, module_path) else: module = _load_module(self._evaluator, module_path, source, sys_path) @@ -440,12 +440,15 @@ def _load_module(evaluator, path=None, source=None, sys_path=None): with open(path, 'rb') as f: source = f.read() else: - return compiled.load_module(path) + return compiled.load_module(evaluator, path) p = path p = fast.FastParser(evaluator.grammar, common.source_to_unicode(source), p) cache.save_parser(path, p) return p.module + if sys_path is None: + sys_path = evaluator.sys_path + cached = cache.load_parser(path) module = load(source) if cached is None else cached.module module = evaluator.wrap(module) diff --git a/jedi/evaluate/sys_path.py b/jedi/evaluate/sys_path.py index 7cfbd57b..f5043e7e 100644 --- a/jedi/evaluate/sys_path.py +++ b/jedi/evaluate/sys_path.py @@ -1,6 +1,7 @@ import glob import os import sys +from site import addsitedir from jedi._compatibility import exec_function, unicode from jedi.parser import tree @@ -11,24 +12,51 @@ from jedi import common from jedi import cache -def get_sys_path(): - def check_virtual_env(sys_path): - """ Add virtualenv's site-packages to the `sys.path`.""" - venv = os.getenv('VIRTUAL_ENV') - if not venv: - return - venv = os.path.abspath(venv) - p = _get_venv_sitepackages(venv) - if p not in sys_path: - sys_path.insert(0, p) +def get_venv_path(venv): + """Get sys.path for specified virtual environment.""" + sys_path = _get_venv_path_dirs(venv) + with common.ignored(ValueError): + sys_path.remove('') + sys_path = _get_sys_path_with_egglinks(sys_path) + # As of now, get_venv_path_dirs does not scan built-in pythonpath and + # user-local site-packages, let's approximate them using path from Jedi + # interpreter. + return sys_path + sys.path - # Add all egg-links from the virtualenv. + +def _get_sys_path_with_egglinks(sys_path): + """Find all paths including those referenced by egg-links. + + Egg-link-referenced directories are inserted into path immediately after + the directory on which their links were found. Such directories are not + taken into consideration by normal import mechanism, but they are traversed + when doing pkg_resources.require. + """ + result = [] + for p in sys_path: + result.append(p) for egg_link in glob.glob(os.path.join(p, '*.egg-link')): with open(egg_link) as fd: - sys_path.insert(0, fd.readline().rstrip()) + for line in fd: + line = line.strip() + if line: + result.append(os.path.join(p, line)) + # pkg_resources package only interprets the first + # non-empty line in egg-link files. + break + return result - check_virtual_env(sys.path) - return [p for p in sys.path if p != ""] + +def _get_venv_path_dirs(venv): + """Get sys.path for venv without starting up the interpreter.""" + venv = os.path.abspath(venv) + sitedir = _get_venv_sitepackages(venv) + sys.path, old_sys_path = [], sys.path + try: + addsitedir(sitedir) + return sys.path + finally: + sys.path = old_sys_path def _get_venv_sitepackages(venv): @@ -109,7 +137,6 @@ def _paths_from_list_modifications(module_path, trailer1, trailer2): name = trailer1.children[1].value if name not in ['insert', 'append']: return [] - arg = trailer2.children[1] if name == 'insert' and len(arg.children) in (3, 4): # Possible trailing comma. arg = arg.children[2] @@ -117,6 +144,9 @@ def _paths_from_list_modifications(module_path, trailer1, trailer2): def _check_module(evaluator, module): + """ + Detect sys.path modifications within module. + """ def get_sys_path_powers(names): for name in names: power = name.parent.parent @@ -128,10 +158,12 @@ def _check_module(evaluator, module): if isinstance(n, tree.Name) and n.value == 'path': yield name, power - sys_path = list(get_sys_path()) # copy + sys_path = list(evaluator.sys_path) # copy try: possible_names = module.used_names['path'] except KeyError: + # module.used_names is MergedNamesDict whose getitem never throws + # keyerror, this is superfluous. pass else: for name, power in get_sys_path_powers(possible_names): @@ -148,7 +180,7 @@ def sys_path_with_modifications(evaluator, module): if module.path is None: # Support for modules without a path is bad, therefore return the # normal path. - return list(get_sys_path()) + return list(evaluator.sys_path) curdir = os.path.abspath(os.curdir) with common.ignored(OSError): diff --git a/test/test_evaluate/test_namespace_package.py b/test/test_evaluate/test_namespace_package.py index 213576c8..79993f82 100644 --- a/test/test_evaluate/test_namespace_package.py +++ b/test/test_evaluate/test_namespace_package.py @@ -1,55 +1,53 @@ import jedi -import sys from os.path import dirname, join def test_namespace_package(): - sys.path.insert(0, join(dirname(__file__), 'namespace_package/ns1')) - sys.path.insert(1, join(dirname(__file__), 'namespace_package/ns2')) - try: - # goto definition - assert jedi.Script('from pkg import ns1_file').goto_definitions() - assert jedi.Script('from pkg import ns2_file').goto_definitions() - assert not jedi.Script('from pkg import ns3_file').goto_definitions() + sys_path = [join(dirname(__file__), d) + for d in ['namespace_package/ns1', 'namespace_package/ns2']] - # goto assignment - tests = { - 'from pkg.ns2_folder.nested import foo': 'nested!', - 'from pkg.ns2_folder import foo': 'ns2_folder!', - 'from pkg.ns2_file import foo': 'ns2_file!', - 'from pkg.ns1_folder import foo': 'ns1_folder!', - 'from pkg.ns1_file import foo': 'ns1_file!', - 'from pkg import foo': 'ns1!', - } - for source, solution in tests.items(): - ass = jedi.Script(source).goto_assignments() - assert len(ass) == 1 - assert ass[0].description == "foo = '%s'" % solution + def script_with_path(*args, **kwargs): + return jedi.Script(sys_path=sys_path, *args, **kwargs) - # completion - completions = jedi.Script('from pkg import ').completions() - names = [str(c.name) for c in completions] # str because of unicode - compare = ['foo', 'ns1_file', 'ns1_folder', 'ns2_folder', 'ns2_file', - 'pkg_resources', 'pkgutil', '__name__', '__path__', - '__package__', '__file__', '__doc__'] - # must at least contain these items, other items are not important - assert set(compare) == set(names) + # goto definition + assert script_with_path('from pkg import ns1_file').goto_definitions() + assert script_with_path('from pkg import ns2_file').goto_definitions() + assert not script_with_path('from pkg import ns3_file').goto_definitions() - tests = { - 'from pkg import ns2_folder as x': 'ns2_folder!', - 'from pkg import ns2_file as x': 'ns2_file!', - 'from pkg.ns2_folder import nested as x': 'nested!', - 'from pkg import ns1_folder as x': 'ns1_folder!', - 'from pkg import ns1_file as x': 'ns1_file!', - 'import pkg as x': 'ns1!', - } - for source, solution in tests.items(): - for c in jedi.Script(source + '; x.').completions(): - if c.name == 'foo': - completion = c - solution = "statement: foo = '%s'" % solution - assert completion.description == solution + # goto assignment + tests = { + 'from pkg.ns2_folder.nested import foo': 'nested!', + 'from pkg.ns2_folder import foo': 'ns2_folder!', + 'from pkg.ns2_file import foo': 'ns2_file!', + 'from pkg.ns1_folder import foo': 'ns1_folder!', + 'from pkg.ns1_file import foo': 'ns1_file!', + 'from pkg import foo': 'ns1!', + } + for source, solution in tests.items(): + ass = script_with_path(source).goto_assignments() + assert len(ass) == 1 + assert ass[0].description == "foo = '%s'" % solution - finally: - sys.path.pop(0) - sys.path.pop(0) + # completion + completions = script_with_path('from pkg import ').completions() + names = [str(c.name) for c in completions] # str because of unicode + compare = ['foo', 'ns1_file', 'ns1_folder', 'ns2_folder', 'ns2_file', + 'pkg_resources', 'pkgutil', '__name__', '__path__', + '__package__', '__file__', '__doc__'] + # must at least contain these items, other items are not important + assert set(compare) == set(names) + + tests = { + 'from pkg import ns2_folder as x': 'ns2_folder!', + 'from pkg import ns2_file as x': 'ns2_file!', + 'from pkg.ns2_folder import nested as x': 'nested!', + 'from pkg import ns1_folder as x': 'ns1_folder!', + 'from pkg import ns1_file as x': 'ns1_file!', + 'import pkg as x': 'ns1!', + } + for source, solution in tests.items(): + for c in script_with_path(source + '; x.').completions(): + if c.name == 'foo': + completion = c + solution = "statement: foo = '%s'" % solution + assert completion.description == solution diff --git a/test/test_evaluate/test_sys_path.py b/test/test_evaluate/test_sys_path.py index a2600b86..f28c2021 100644 --- a/test/test_evaluate/test_sys_path.py +++ b/test/test_evaluate/test_sys_path.py @@ -1,4 +1,8 @@ import os +from glob import glob +import sys + +import pytest from jedi._compatibility import unicode from jedi.parser import Parser, load_grammar @@ -19,13 +23,30 @@ def test_paths_from_assignment(): assert paths('sys.path, other = ["a"], 2') == [] -def test_get_sys_path(monkeypatch): - monkeypatch.setenv('VIRTUAL_ENV', os.path.join(os.path.dirname(__file__), - 'egg-link', 'venv')) - def sitepackages_dir(venv): - return os.path.join(venv, 'lib', 'python3.4', 'site-packages') +# Currently venv site-packages resolution only seeks pythonX.Y/site-packages +# that belong to the same version as the interpreter to avoid issues with +# cross-version imports. "venvs/" dir contains "venv27" and "venv34" that +# mimic venvs created for py2.7 and py3.4 respectively. If test runner is +# invoked with one of those versions, the test below will be run for the +# matching directory. +CUR_DIR = os.path.dirname(__file__) +VENVS = list(glob(os.path.join(CUR_DIR, + 'venvs/venv%d%d' % sys.version_info[:2]))) - monkeypatch.setattr('jedi.evaluate.sys_path._get_venv_sitepackages', - sitepackages_dir) - assert '/path/from/egg-link' in sys_path.get_sys_path() +@pytest.mark.parametrize('venv', VENVS) +def test_get_venv_path(venv): + pjoin = os.path.join + venv_path = sys_path.get_venv_path(venv) + + site_pkgs = (glob(pjoin(venv, 'lib', 'python*', 'site-packages')) + + glob(pjoin(venv, 'lib', 'site-packages')))[0] + ETALON = [ + site_pkgs, + pjoin(site_pkgs, '.', 'relative', 'egg-link', 'path'), + pjoin('/path', 'from', 'egg-link'), + pjoin(site_pkgs, 'dir-from-foo-pth'), + pjoin('/path', 'from', 'smth.py'), + pjoin('/path', 'from', 'smth.py:extend_path') + ] + assert venv_path[:len(ETALON)] == ETALON diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/dir-from-foo-pth/__init__.py b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/dir-from-foo-pth/__init__.py new file mode 100644 index 00000000..2a1d8700 --- /dev/null +++ b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/dir-from-foo-pth/__init__.py @@ -0,0 +1,2 @@ +# This file is here to force git to create the directory, as *.pth files only +# add existing directories. diff --git a/test/test_evaluate/egg-link/venv/lib/python3.4/site-packages/egg_link.egg-link b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/egg_link.egg-link similarity index 100% rename from test/test_evaluate/egg-link/venv/lib/python3.4/site-packages/egg_link.egg-link rename to test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/egg_link.egg-link diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/foo.pth b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/foo.pth new file mode 100644 index 00000000..88501682 --- /dev/null +++ b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/foo.pth @@ -0,0 +1 @@ +./dir-from-foo-pth diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/import_smth.pth b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/import_smth.pth new file mode 100644 index 00000000..72b006a6 --- /dev/null +++ b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/import_smth.pth @@ -0,0 +1 @@ +import smth; smth.extend_path() \ No newline at end of file diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/relative.egg-link b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/relative.egg-link new file mode 100644 index 00000000..7a9a6156 --- /dev/null +++ b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/relative.egg-link @@ -0,0 +1 @@ +./relative/egg-link/path diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/smth.py b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/smth.py new file mode 100644 index 00000000..3e3008dd --- /dev/null +++ b/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/smth.py @@ -0,0 +1,6 @@ +import sys +sys.path.append('/path/from/smth.py') + + +def extend_path(): + sys.path.append('/path/from/smth.py:extend_path') diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/dir-from-foo-pth/__init__.py b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/dir-from-foo-pth/__init__.py new file mode 100644 index 00000000..2a1d8700 --- /dev/null +++ b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/dir-from-foo-pth/__init__.py @@ -0,0 +1,2 @@ +# This file is here to force git to create the directory, as *.pth files only +# add existing directories. diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/egg_link.egg-link b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/egg_link.egg-link new file mode 100644 index 00000000..dde9b7d5 --- /dev/null +++ b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/egg_link.egg-link @@ -0,0 +1 @@ +/path/from/egg-link diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/foo.pth b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/foo.pth new file mode 100644 index 00000000..88501682 --- /dev/null +++ b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/foo.pth @@ -0,0 +1 @@ +./dir-from-foo-pth diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/import_smth.pth b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/import_smth.pth new file mode 100644 index 00000000..72b006a6 --- /dev/null +++ b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/import_smth.pth @@ -0,0 +1 @@ +import smth; smth.extend_path() \ No newline at end of file diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/relative.egg-link b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/relative.egg-link new file mode 100644 index 00000000..7a9a6156 --- /dev/null +++ b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/relative.egg-link @@ -0,0 +1 @@ +./relative/egg-link/path diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/smth.py b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/smth.py new file mode 100644 index 00000000..3e3008dd --- /dev/null +++ b/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/smth.py @@ -0,0 +1,6 @@ +import sys +sys.path.append('/path/from/smth.py') + + +def extend_path(): + sys.path.append('/path/from/smth.py:extend_path') diff --git a/test/test_regression.py b/test/test_regression.py index fa81c8af..94132acd 100644 --- a/test/test_regression.py +++ b/test/test_regression.py @@ -66,8 +66,9 @@ class TestRegression(TestCase): src1 = "def r(a): return a" # Other fictional modules in another place in the fs. src2 = 'from .. import setup; setup.r(1)' - imports.load_module(os.path.abspath(fname), src2) - result = Script(src1, path='../setup.py').goto_definitions() + script = Script(src1, path='../setup.py') + imports.load_module(script._evaluator, os.path.abspath(fname), src2) + result = script.goto_definitions() assert len(result) == 1 assert result[0].description == 'class int' From f50045710037eccf287a4736c04bfdc57390961d Mon Sep 17 00:00:00 2001 From: immerrr Date: Wed, 21 Oct 2015 08:29:53 +0300 Subject: [PATCH 2/8] sample_venvs: exclude venvs dir from py.test discovery --- pytest.ini | 2 +- .../python2.7/site-packages/dir-from-foo-pth/__init__.py | 0 .../venv27/lib/python2.7/site-packages/egg_link.egg-link | 0 .../venv27/lib/python2.7/site-packages/foo.pth | 0 .../venv27/lib/python2.7/site-packages/import_smth.pth | 0 .../venv27/lib/python2.7/site-packages/relative.egg-link | 0 .../venv27/lib/python2.7/site-packages/smth.py | 0 .../python3.4/site-packages/dir-from-foo-pth/__init__.py | 0 .../venv34/lib/python3.4/site-packages/egg_link.egg-link | 0 .../venv34/lib/python3.4/site-packages/foo.pth | 0 .../venv34/lib/python3.4/site-packages/import_smth.pth | 0 .../venv34/lib/python3.4/site-packages/relative.egg-link | 0 .../venv34/lib/python3.4/site-packages/smth.py | 0 test/test_evaluate/test_sys_path.py | 6 ++++-- 14 files changed, 5 insertions(+), 3 deletions(-) rename test/test_evaluate/{venvs => sample_venvs}/venv27/lib/python2.7/site-packages/dir-from-foo-pth/__init__.py (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv27/lib/python2.7/site-packages/egg_link.egg-link (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv27/lib/python2.7/site-packages/foo.pth (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv27/lib/python2.7/site-packages/import_smth.pth (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv27/lib/python2.7/site-packages/relative.egg-link (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv27/lib/python2.7/site-packages/smth.py (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv34/lib/python3.4/site-packages/dir-from-foo-pth/__init__.py (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv34/lib/python3.4/site-packages/egg_link.egg-link (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv34/lib/python3.4/site-packages/foo.pth (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv34/lib/python3.4/site-packages/import_smth.pth (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv34/lib/python3.4/site-packages/relative.egg-link (100%) rename test/test_evaluate/{venvs => sample_venvs}/venv34/lib/python3.4/site-packages/smth.py (100%) diff --git a/pytest.ini b/pytest.ini index 3a066302..6fafa946 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,7 +2,7 @@ addopts = --doctest-modules # Ignore broken files in blackbox test directories -norecursedirs = .* docs completion refactor absolute_import namespace_package scripts extensions speed static_analysis not_in_sys_path buildout_project egg-link init_extension_module +norecursedirs = .* docs completion refactor absolute_import namespace_package scripts extensions speed static_analysis not_in_sys_path buildout_project sample_venvs init_extension_module # Activate `clean_jedi_cache` fixture for all tests. This should be # fine as long as we are using `clean_jedi_cache` as a session scoped diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/dir-from-foo-pth/__init__.py b/test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/dir-from-foo-pth/__init__.py similarity index 100% rename from test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/dir-from-foo-pth/__init__.py rename to test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/dir-from-foo-pth/__init__.py diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/egg_link.egg-link b/test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/egg_link.egg-link similarity index 100% rename from test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/egg_link.egg-link rename to test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/egg_link.egg-link diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/foo.pth b/test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/foo.pth similarity index 100% rename from test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/foo.pth rename to test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/foo.pth diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/import_smth.pth b/test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/import_smth.pth similarity index 100% rename from test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/import_smth.pth rename to test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/import_smth.pth diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/relative.egg-link b/test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/relative.egg-link similarity index 100% rename from test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/relative.egg-link rename to test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/relative.egg-link diff --git a/test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/smth.py b/test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/smth.py similarity index 100% rename from test/test_evaluate/venvs/venv27/lib/python2.7/site-packages/smth.py rename to test/test_evaluate/sample_venvs/venv27/lib/python2.7/site-packages/smth.py diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/dir-from-foo-pth/__init__.py b/test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/dir-from-foo-pth/__init__.py similarity index 100% rename from test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/dir-from-foo-pth/__init__.py rename to test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/dir-from-foo-pth/__init__.py diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/egg_link.egg-link b/test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/egg_link.egg-link similarity index 100% rename from test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/egg_link.egg-link rename to test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/egg_link.egg-link diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/foo.pth b/test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/foo.pth similarity index 100% rename from test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/foo.pth rename to test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/foo.pth diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/import_smth.pth b/test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/import_smth.pth similarity index 100% rename from test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/import_smth.pth rename to test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/import_smth.pth diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/relative.egg-link b/test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/relative.egg-link similarity index 100% rename from test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/relative.egg-link rename to test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/relative.egg-link diff --git a/test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/smth.py b/test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/smth.py similarity index 100% rename from test/test_evaluate/venvs/venv34/lib/python3.4/site-packages/smth.py rename to test/test_evaluate/sample_venvs/venv34/lib/python3.4/site-packages/smth.py diff --git a/test/test_evaluate/test_sys_path.py b/test/test_evaluate/test_sys_path.py index f28c2021..44997100 100644 --- a/test/test_evaluate/test_sys_path.py +++ b/test/test_evaluate/test_sys_path.py @@ -30,8 +30,8 @@ def test_paths_from_assignment(): # invoked with one of those versions, the test below will be run for the # matching directory. CUR_DIR = os.path.dirname(__file__) -VENVS = list(glob(os.path.join(CUR_DIR, - 'venvs/venv%d%d' % sys.version_info[:2]))) +VENVS = list(glob( + os.path.join(CUR_DIR, 'sample_venvs/venv%d%d' % sys.version_info[:2]))) @pytest.mark.parametrize('venv', VENVS) @@ -49,4 +49,6 @@ def test_get_venv_path(venv): pjoin('/path', 'from', 'smth.py'), pjoin('/path', 'from', 'smth.py:extend_path') ] + # Ensure that none of venv dirs leaked to the interpreter. + assert not set(sys.path).intersection(ETALON) assert venv_path[:len(ETALON)] == ETALON From da4dbe81a94ae90933fd3ac083c734ae36cc464d Mon Sep 17 00:00:00 2001 From: immerrr Date: Wed, 21 Oct 2015 12:03:39 +0300 Subject: [PATCH 3/8] sys_path: order egg-link files for reproducible test results --- jedi/evaluate/sys_path.py | 5 ++++- test/test_evaluate/test_sys_path.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/jedi/evaluate/sys_path.py b/jedi/evaluate/sys_path.py index f5043e7e..e8f46082 100644 --- a/jedi/evaluate/sys_path.py +++ b/jedi/evaluate/sys_path.py @@ -35,7 +35,10 @@ def _get_sys_path_with_egglinks(sys_path): result = [] for p in sys_path: result.append(p) - for egg_link in glob.glob(os.path.join(p, '*.egg-link')): + # pkg_resources does not define a specific order for egg-link files + # using os.listdir to enumerate them, we're sorting them to have + # reproducible tests. + for egg_link in sorted(glob.glob(os.path.join(p, '*.egg-link'))): with open(egg_link) as fd: for line in fd: line = line.strip() diff --git a/test/test_evaluate/test_sys_path.py b/test/test_evaluate/test_sys_path.py index 44997100..ae7b61c0 100644 --- a/test/test_evaluate/test_sys_path.py +++ b/test/test_evaluate/test_sys_path.py @@ -43,8 +43,8 @@ def test_get_venv_path(venv): glob(pjoin(venv, 'lib', 'site-packages')))[0] ETALON = [ site_pkgs, - pjoin(site_pkgs, '.', 'relative', 'egg-link', 'path'), pjoin('/path', 'from', 'egg-link'), + pjoin(site_pkgs, '.', 'relative', 'egg-link', 'path'), pjoin(site_pkgs, 'dir-from-foo-pth'), pjoin('/path', 'from', 'smth.py'), pjoin('/path', 'from', 'smth.py:extend_path') From 90a08794bae11f453728aa3db885e4d381eccb99 Mon Sep 17 00:00:00 2001 From: immerrr Date: Wed, 21 Oct 2015 17:35:28 +0300 Subject: [PATCH 4/8] test_imports: use sys_path --- test/test_evaluate/test_imports.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/test_evaluate/test_imports.py b/test/test_evaluate/test_imports.py index de913a4e..9ebc968d 100644 --- a/test/test_evaluate/test_imports.py +++ b/test/test_evaluate/test_imports.py @@ -46,11 +46,8 @@ def test_flask_ext(script, name): """flask.ext.foo is really imported from flaskext.foo or flask_foo. """ path = os.path.join(os.path.dirname(__file__), 'flask-site-packages') - sys.path.append(path) - try: - assert name in [c.name for c in jedi.Script(script).completions()] - finally: - sys.path.remove(path) + completions = jedi.Script(script, sys_path=[path]).completions() + assert name in [c.name for c in completions] @cwd_at('test/test_evaluate/') From fb592ad02876ca2fd62243b82b99de6c5ce45868 Mon Sep 17 00:00:00 2001 From: immerrr Date: Wed, 21 Oct 2015 17:57:27 +0300 Subject: [PATCH 5/8] test_imports: add test to ensure caching works with sys_path --- test/test_evaluate/test_imports.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/test_evaluate/test_imports.py b/test/test_evaluate/test_imports.py index 9ebc968d..a8825287 100644 --- a/test/test_evaluate/test_imports.py +++ b/test/test_evaluate/test_imports.py @@ -54,3 +54,19 @@ def test_flask_ext(script, name): def test_not_importable_file(): src = 'import not_importable_file as x; x.' assert not jedi.Script(src, path='example.py').completions() + + +def test_cache_works_with_sys_path_param(tmpdir): + foo_path = tmpdir.join('foo') + bar_path = tmpdir.join('bar') + foo_path.join('module.py').write('foo = 123', ensure=True) + bar_path.join('module.py').write('bar = 123', ensure=True) + foo_completions = jedi.Script('import module; module.', + sys_path=[foo_path.strpath]).completions() + bar_completions = jedi.Script('import module; module.', + sys_path=[bar_path.strpath]).completions() + assert 'foo' in [c.name for c in foo_completions] + assert 'bar' not in [c.name for c in foo_completions] + + assert 'bar' in [c.name for c in bar_completions] + assert 'foo' not in [c.name for c in bar_completions] From cc139e8f703c1396b63c8fe172908256eea1f145 Mon Sep 17 00:00:00 2001 From: immerrr Date: Mon, 26 Oct 2015 12:49:12 +0300 Subject: [PATCH 6/8] evaluate.site: copy/adapt site-packages related functionality from stdlib --- jedi/evaluate/site.py | 110 ++++++++++++++++++++++++++++ jedi/evaluate/sys_path.py | 11 +-- test/test_evaluate/test_sys_path.py | 11 ++- 3 files changed, 122 insertions(+), 10 deletions(-) create mode 100644 jedi/evaluate/site.py diff --git a/jedi/evaluate/site.py b/jedi/evaluate/site.py new file mode 100644 index 00000000..bf884fae --- /dev/null +++ b/jedi/evaluate/site.py @@ -0,0 +1,110 @@ +"""An adapted copy of relevant site-packages functionality from Python stdlib. + +This file contains some functions related to handling site-packages in Python +with jedi-specific modifications: + +- the functions operate on sys_path argument rather than global sys.path + +- in .pth files "import ..." lines that allow execution of arbitrary code are + skipped to prevent code injection into jedi interpreter + +""" + +# Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, +# 2011, 2012, 2013, 2014, 2015 Python Software Foundation; All Rights Reserved + +from __future__ import print_function + +import sys +import os + + +def makepath(*paths): + dir = os.path.join(*paths) + try: + dir = os.path.abspath(dir) + except OSError: + pass + return dir, os.path.normcase(dir) + + +def _init_pathinfo(sys_path): + """Return a set containing all existing directory entries from sys_path""" + d = set() + for dir in sys_path: + try: + if os.path.isdir(dir): + dir, dircase = makepath(dir) + d.add(dircase) + except TypeError: + continue + return d + + +def addpackage(sys_path, sitedir, name, known_paths): + """Process a .pth file within the site-packages directory: + For each line in the file, either combine it with sitedir to a path + and add that to known_paths, or execute it if it starts with 'import '. + """ + if known_paths is None: + known_paths = _init_pathinfo(sys_path) + reset = 1 + else: + reset = 0 + fullname = os.path.join(sitedir, name) + try: + f = open(fullname, "r") + except OSError: + return + with f: + for n, line in enumerate(f): + if line.startswith("#"): + continue + try: + if line.startswith(("import ", "import\t")): + # Change by immerrr: don't evaluate import lines to prevent + # code injection into jedi through pth files. + # + # exec(line) + continue + line = line.rstrip() + dir, dircase = makepath(sitedir, line) + if not dircase in known_paths and os.path.exists(dir): + sys_path.append(dir) + known_paths.add(dircase) + except Exception: + print("Error processing line {:d} of {}:\n".format(n+1, fullname), + file=sys.stderr) + import traceback + for record in traceback.format_exception(*sys.exc_info()): + for line in record.splitlines(): + print(' '+line, file=sys.stderr) + print("\nRemainder of file ignored", file=sys.stderr) + break + if reset: + known_paths = None + return known_paths + + +def addsitedir(sys_path, sitedir, known_paths=None): + """Add 'sitedir' argument to sys_path if missing and handle .pth files in + 'sitedir'""" + if known_paths is None: + known_paths = _init_pathinfo(sys_path) + reset = 1 + else: + reset = 0 + sitedir, sitedircase = makepath(sitedir) + if not sitedircase in known_paths: + sys_path.append(sitedir) # Add path component + known_paths.add(sitedircase) + try: + names = os.listdir(sitedir) + except OSError: + return + names = [name for name in names if name.endswith(".pth")] + for name in sorted(names): + addpackage(sys_path, sitedir, name, known_paths) + if reset: + known_paths = None + return known_paths diff --git a/jedi/evaluate/sys_path.py b/jedi/evaluate/sys_path.py index e8f46082..50e34b9d 100644 --- a/jedi/evaluate/sys_path.py +++ b/jedi/evaluate/sys_path.py @@ -1,7 +1,7 @@ import glob import os import sys -from site import addsitedir +from jedi.evaluate.site import addsitedir from jedi._compatibility import exec_function, unicode from jedi.parser import tree @@ -54,12 +54,9 @@ def _get_venv_path_dirs(venv): """Get sys.path for venv without starting up the interpreter.""" venv = os.path.abspath(venv) sitedir = _get_venv_sitepackages(venv) - sys.path, old_sys_path = [], sys.path - try: - addsitedir(sitedir) - return sys.path - finally: - sys.path = old_sys_path + sys_path = [] + addsitedir(sys_path, sitedir) + return sys_path def _get_venv_sitepackages(venv): diff --git a/test/test_evaluate/test_sys_path.py b/test/test_evaluate/test_sys_path.py index ae7b61c0..3c44a99c 100644 --- a/test/test_evaluate/test_sys_path.py +++ b/test/test_evaluate/test_sys_path.py @@ -46,9 +46,14 @@ def test_get_venv_path(venv): pjoin('/path', 'from', 'egg-link'), pjoin(site_pkgs, '.', 'relative', 'egg-link', 'path'), pjoin(site_pkgs, 'dir-from-foo-pth'), - pjoin('/path', 'from', 'smth.py'), - pjoin('/path', 'from', 'smth.py:extend_path') ] + + # Ensure that pth and egg-link paths were added. + assert venv_path[:len(ETALON)] == ETALON + # Ensure that none of venv dirs leaked to the interpreter. assert not set(sys.path).intersection(ETALON) - assert venv_path[:len(ETALON)] == ETALON + + # Ensure that "import ..." lines were ignored. + assert pjoin('/path', 'from', 'smth.py') not in venv_path + assert pjoin('/path', 'from', 'smth.py:extend_path') not in venv_path From f634db7a2081e65d1fdd5cd01a1b14f92e9d02df Mon Sep 17 00:00:00 2001 From: immerrr Date: Mon, 26 Oct 2015 13:37:18 +0300 Subject: [PATCH 7/8] jedi.api.Script: document sys_path parameter and VIRTUAL_ENV variable --- jedi/api/__init__.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/jedi/api/__init__.py b/jedi/api/__init__.py index de7580d7..4e05f3e1 100644 --- a/jedi/api/__init__.py +++ b/jedi/api/__init__.py @@ -59,6 +59,18 @@ class Script(object): You can either use the ``source`` parameter or ``path`` to read a file. Usually you're going to want to use both of them (in an editor). + The script might be analyzed in a different ``sys.path`` than |jedi|: + + - if `sys_path` parameter is not ``None``, it will be used as ``sys.path`` + for the script; + + - if `sys_path` parameter is ``None`` and ``VIRTUAL_ENV`` environment + variable is defined, ``sys.path`` for the specified environment will be + guessed (see :func:`jedi.evaluate.sys_path.get_venv_path`) and used for + the script; + + - otherwise ``sys.path`` will match that of |jedi|. + :param source: The source code of the current file, separated by newlines. :type source: str :param line: The line to perform actions on (starting with 1). @@ -74,6 +86,9 @@ class Script(object): :param source_encoding: The encoding of ``source``, if it is not a ``unicode`` object (default ``'utf-8'``). :type encoding: str + :param sys_path: ``sys.path`` to use during analysis of the script + :type sys_path: list + """ def __init__(self, source=None, line=None, column=None, path=None, encoding='utf-8', source_path=None, source_encoding=None, From 45642cc16c2c1e2f90f740b18911055bd5dfdbe5 Mon Sep 17 00:00:00 2001 From: immerrr Date: Mon, 26 Oct 2015 14:23:24 +0300 Subject: [PATCH 8/8] .coveragerc: exclude imported site.py --- .coveragerc | 1 + 1 file changed, 1 insertion(+) diff --git a/.coveragerc b/.coveragerc index c48bafc2..939a3175 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,6 +1,7 @@ [run] omit = jedi/_compatibility.py + jedi/evaluate/site.py [report] # Regexes for lines to exclude from consideration