Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edit path_inject() to compare Paths instead of strings #4210

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions src/ansiblelint/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
20 changes: 20 additions & 0 deletions test/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import os
import shutil
import site
import subprocess
import sys
import tempfile
Expand All @@ -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

Expand Down Expand Up @@ -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
Loading