From 6463fc3d79ba347d20a23f3439f9c5fcc4687f1f Mon Sep 17 00:00:00 2001 From: janmondry <73887656+janmondry@users.noreply.github.com> Date: Wed, 5 Jun 2024 15:10:48 +0200 Subject: [PATCH] Edit path_inject() to compare Paths instead of strings --- src/ansiblelint/__main__.py | 28 +++++++++++++++------------- test/test_main.py | 20 ++++++++++++++++++++ 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/ansiblelint/__main__.py b/src/ansiblelint/__main__.py index 76b5fe9d65..413c1be0b7 100755 --- a/src/ansiblelint/__main__.py +++ b/src/ansiblelint/__main__.py @@ -427,13 +427,13 @@ def path_inject(own_location: str = "") -> None: # # This must be run before we do run any subprocesses, and loading config # does this as part of the ansible detection. - paths = [x for x in os.environ.get("PATH", "").split(os.pathsep) if x] + paths = [Path(x) for x in os.environ.get("PATH", "").split(os.pathsep) if x] # Expand ~ in PATH as it known to break many tools expanded = False for idx, path in enumerate(paths): - if path.startswith("~"): # pragma: no cover - paths[idx] = str(Path(path).expanduser()) + if str(path).startswith("~"): # pragma: no cover + paths[idx] = Path(path).expanduser() expanded = True if expanded: # pragma: no cover print( # noqa: T201 @@ -446,35 +446,37 @@ def path_inject(own_location: str = "") -> None: userbase_bin_path = Path(site.getuserbase()) / "bin" if ( - str(userbase_bin_path) not in paths + userbase_bin_path not in paths and (userbase_bin_path / "bin" / "ansible").exists() ): - inject_paths.append(str(userbase_bin_path)) + inject_paths.append(userbase_bin_path) py_path = Path(sys.executable).parent pipx_path = os.environ.get("PIPX_HOME", "pipx") if ( - str(py_path) not in paths + py_path not in paths and (py_path / "ansible").exists() and pipx_path not in str(py_path) ): - inject_paths.append(str(py_path)) + inject_paths.append(py_path) # last option, if nothing else is found, just look next to ourselves... if own_location: own_location = os.path.realpath(own_location) parent = Path(own_location).parent - if (parent / "ansible").exists() and str(parent) not in paths: - inject_paths.append(str(parent)) + if (parent / "ansible").exists() and parent not in paths: + inject_paths.append(parent) - if not os.environ.get("PYENV_VIRTUAL_ENV", None): - if inject_paths and not all("pipx" in p for p in inject_paths): + if not os.environ.get("PYENV_VIRTUAL_ENV"): + if inject_paths and not all("pipx" in str(p) for p in inject_paths): print( # noqa: T201 - f"WARNING: PATH altered to include {', '.join(inject_paths)} :: This is usually a sign of broken local setup, which can cause unexpected behaviors.", + f"WARNING: PATH altered to include {', '.join(map(str, inject_paths))} :: This is usually a sign of broken local setup, which can cause unexpected behaviors.", file=sys.stderr, ) if inject_paths or expanded: - os.environ["PATH"] = os.pathsep.join([*inject_paths, *paths]) + os.environ["PATH"] = os.pathsep.join( + [*map(str, inject_paths), *map(str, paths)], + ) # We do know that finding ansible in PATH does not guarantee that it is # functioning or that is in fact the same version that was installed as diff --git a/test/test_main.py b/test/test_main.py index d2e48dc060..f7cefa1a35 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -2,6 +2,7 @@ import os import shutil +import site import subprocess import sys import tempfile @@ -12,6 +13,7 @@ import pytest from pytest_mock import MockerFixture +from ansiblelint.__main__ import path_inject from ansiblelint.config import get_version_warning, options from ansiblelint.constants import RC @@ -149,3 +151,21 @@ def test_broken_ansible_cfg() -> None: "Invalid type for configuration option setting: CACHE_PLUGIN_TIMEOUT" in proc.stderr ) + + +@pytest.mark.parametrize("tested_path", ("/app/bin/", "/app/bin")) +def test_path_inject(mocker: MockerFixture, tested_path: str) -> None: + """Asserts PATH is not changed when it contains paths with trailing slashes.""" + own_location = Path(tested_path) / "ansible-lint" + + # ensure inject_paths is empty before searching around "own_location" + userbase_bin_path = Path(site.getuserbase()) / "bin" + py_path = Path(sys.executable).parent + mocked_path = f"{userbase_bin_path}:{py_path}:{tested_path}" + + mocker.patch("os.environ", {"PATH": mocked_path}) + mocker.patch("Path.exists", return_value=True) + + path_inject(str(own_location)) + + assert os.environ["PATH"] == mocked_path