From c85cb41f65cde155663b0953569c90e8de8b6c1e Mon Sep 17 00:00:00 2001 From: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Date: Sun, 28 Mar 2021 20:40:52 +0200 Subject: Fix regression with sys.path filter (#4258) * Fix regression with sys.path filter --- ChangeLog | 4 ++ pylint/__init__.py | 16 ++++-- tests/test_self.py | 151 ++++++++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 141 insertions(+), 30 deletions(-) diff --git a/ChangeLog b/ChangeLog index 407b79b3c..c85071544 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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? =========================== diff --git a/pylint/__init__.py b/pylint/__init__.py index 20bb554d2..7d2341396 100644 --- a/pylint/__init__.py +++ b/pylint/__init__.py @@ -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__"] diff --git a/tests/test_self.py b/tests/test_self.py index b3d8927b9..af78f6f3c 100644 --- a/tests/test_self.py +++ b/tests/test_self.py @@ -45,7 +45,7 @@ import warnings 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 @@ -724,61 +724,131 @@ class TestRunTC: 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" @@ -838,7 +908,7 @@ class TestRunTC: # 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, @@ -849,8 +919,39 @@ class TestRunTC: ], 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" -- cgit v1.2.1