Skip to content

Commit

Permalink
Fix regression with sys.path filter (#4258)
Browse files Browse the repository at this point in the history
* Fix regression with sys.path filter
  • Loading branch information
cdce8p committed Mar 28, 2021
1 parent 95daeca commit c85cb41
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 30 deletions.
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ Release date: TBA

Closes #3202

* Fix regression with plugins on PYTHONPATH if latter is cwd

Closes #4252


What's New in Pylint 2.7.2?
===========================
Expand Down
16 changes: 11 additions & 5 deletions pylint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,22 @@ def modify_sys_path() -> None:
CPython issue: https://bugs.python.org/issue33053
- Remove the first entry. This will always be either "" or the working directory
- Remove the working directory from the second and third entries. This can
occur if PYTHONPATH includes a ":" at the beginning or the end.
- Remove the working directory from the second and third entries
if PYTHONPATH includes a ":" at the beginning or the end.
https://github.com/PyCQA/pylint/issues/3636
Don't remove it if PYTHONPATH contains the cwd or '.' as the entry will
only be added once.
- Don't remove the working directory from the rest. It will be included
if pylint is installed in an editable configuration (as the last item).
https://github.com/PyCQA/pylint/issues/4161
"""
sys.path = [
p for i, p in enumerate(sys.path) if i > 0 and not (i < 3 and p == os.getcwd())
]
sys.path.pop(0)
env_pythonpath = os.environ.get("PYTHONPATH", "")
cwd = os.getcwd()
if env_pythonpath.startswith(":") and env_pythonpath not in (f":{cwd}", ":."):
sys.path.pop(0)
elif env_pythonpath.endswith(":") and env_pythonpath not in (f"{cwd}:", ".:"):
sys.path.pop(1)


__all__ = ["__version__"]
151 changes: 126 additions & 25 deletions tests/test_self.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
from copy import copy
from io import StringIO
from os.path import abspath, dirname, join
from typing import Generator
from typing import Generator, Optional
from unittest import mock
from unittest.mock import patch

Expand Down Expand Up @@ -724,61 +724,131 @@ def test_sys_path() -> Generator[None, None, None]:
finally:
sys.path = original_path

@contextlib.contextmanager
def test_environ_pythonpath(
new_pythonpath: Optional[str],
) -> Generator[None, None, None]:
original_pythonpath = os.environ.get("PYTHONPATH")
if new_pythonpath:
os.environ["PYTHONPATH"] = new_pythonpath
elif new_pythonpath is None and original_pythonpath is not None:
# If new_pythonpath is None, make sure to delete PYTHONPATH if present
del os.environ["PYTHONPATH"]
try:
yield
finally:
if original_pythonpath:
os.environ["PYTHONPATH"] = original_pythonpath
elif new_pythonpath is not None:
# Only delete PYTHONPATH if new_pythonpath wasn't None
del os.environ["PYTHONPATH"]

with test_sys_path(), patch("os.getcwd") as mock_getcwd:
cwd = "/tmp/pytest-of-root/pytest-0/test_do_not_import_files_from_0"
mock_getcwd.return_value = cwd

paths = [
cwd,
cwd,
default_paths = [
"/usr/local/lib/python39.zip",
"/usr/local/lib/python3.9",
"/usr/local/lib/python3.9/lib-dynload",
"/usr/local/lib/python3.9/site-packages",
]

paths = [
cwd,
*default_paths,
]
sys.path = copy(paths)
modify_sys_path()
assert sys.path == paths[2:]
with test_environ_pythonpath(None):
modify_sys_path()
assert sys.path == paths[1:]

paths = [
cwd,
cwd,
*default_paths,
]
sys.path = copy(paths)
with test_environ_pythonpath("."):
modify_sys_path()
assert sys.path == paths[1:]

paths = [
cwd,
"/custom_pythonpath",
*default_paths,
]
sys.path = copy(paths)
with test_environ_pythonpath("/custom_pythonpath"):
modify_sys_path()
assert sys.path == paths[1:]

paths = [
cwd,
"/usr/local/lib/python39.zip",
"/usr/local/lib/python3.9",
"/usr/local/lib/python3.9/lib-dynload",
"/usr/local/lib/python3.9/site-packages",
"/custom_pythonpath",
cwd,
*default_paths,
]
sys.path = copy(paths)
modify_sys_path()
with test_environ_pythonpath("/custom_pythonpath:"):
modify_sys_path()
assert sys.path == [paths[1]] + paths[3:]

paths = [
"",
cwd,
"/custom_pythonpath",
"/usr/local/lib/python39.zip",
"/usr/local/lib/python3.9",
"/usr/local/lib/python3.9/lib-dynload",
"/usr/local/lib/python3.9/site-packages",
*default_paths,
]
sys.path = copy(paths)
modify_sys_path()
with test_environ_pythonpath(":/custom_pythonpath"):
modify_sys_path()
assert sys.path == paths[2:]

paths = [
"",
cwd,
"/usr/local/lib/python39.zip",
"/usr/local/lib/python3.9",
"/usr/local/lib/python3.9/lib-dynload",
"/usr/local/lib/python3.9/site-packages",
cwd,
"/custom_pythonpath",
*default_paths,
]
sys.path = copy(paths)
modify_sys_path()
with test_environ_pythonpath(":/custom_pythonpath:"):
modify_sys_path()
assert sys.path == paths[2:]

paths = [
cwd,
cwd,
*default_paths,
]
sys.path = copy(paths)
with test_environ_pythonpath(":."):
modify_sys_path()
assert sys.path == paths[1:]
sys.path = copy(paths)
with test_environ_pythonpath(f":{cwd}"):
modify_sys_path()
assert sys.path == paths[1:]

sys.path = copy(paths)
with test_environ_pythonpath(".:"):
modify_sys_path()
assert sys.path == paths[1:]
sys.path = copy(paths)
with test_environ_pythonpath(f"{cwd}:"):
modify_sys_path()
assert sys.path == paths[1:]

paths = [
"",
cwd,
*default_paths,
cwd,
]
sys.path = copy(paths)
with test_environ_pythonpath(cwd):
modify_sys_path()
assert sys.path == paths[1:]

@staticmethod
def test_do_not_import_files_from_local_directory(tmpdir):
p_astroid = tmpdir / "astroid.py"
Expand Down Expand Up @@ -838,7 +908,7 @@ def test_do_not_import_files_from_local_directory_with_pythonpath(tmpdir):
# https://github.com/PyCQA/pylint/issues/3636
with tmpdir.as_cwd():
orig_pythonpath = os.environ.get("PYTHONPATH")
os.environ["PYTHONPATH"] = os.environ.get("PYTHONPATH", "") + ":"
os.environ["PYTHONPATH"] = f"{(orig_pythonpath or '').strip(':')}:"
subprocess.check_output(
[
sys.executable,
Expand All @@ -849,8 +919,39 @@ def test_do_not_import_files_from_local_directory_with_pythonpath(tmpdir):
],
cwd=str(tmpdir),
)
if orig_pythonpath is not None:
if orig_pythonpath:
os.environ["PYTHONPATH"] = orig_pythonpath
else:
del os.environ["PYTHONPATH"]

@staticmethod
def test_import_plugin_from_local_directory_if_pythonpath_cwd(tmpdir):
p_plugin = tmpdir / "plugin.py"
p_plugin.write("# Some plugin content")

with tmpdir.as_cwd():
orig_pythonpath = os.environ.get("PYTHONPATH")
os.environ["PYTHONPATH"] = "."
process = subprocess.run(
[
sys.executable,
"-m",
"pylint",
"--load-plugins",
"plugin",
],
cwd=str(tmpdir),
stderr=subprocess.PIPE,
check=False,
)
assert (
"AttributeError: module 'plugin' has no attribute 'register'"
in process.stderr.decode()
)
if orig_pythonpath:
os.environ["PYTHONPATH"] = orig_pythonpath
else:
del os.environ["PYTHONPATH"]

def test_allow_import_of_files_found_in_modules_during_parallel_check(self, tmpdir):
test_directory = tmpdir / "test_directory"
Expand Down

0 comments on commit c85cb41

Please sign in to comment.