From 4eb3cf79214337d3f45deadc1d00d873bcf2e135 Mon Sep 17 00:00:00 2001 From: immerrr Date: Sun, 5 Apr 2015 00:19:11 +0200 Subject: [PATCH] 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'