Skip to content

Commit 6463fc3

Browse files
janmondryssbarnea
authored andcommitted
Edit path_inject() to compare Paths instead of strings
1 parent 712b530 commit 6463fc3

File tree

2 files changed

+35
-13
lines changed

2 files changed

+35
-13
lines changed

src/ansiblelint/__main__.py

+15-13
Original file line numberDiff line numberDiff line change
@@ -427,13 +427,13 @@ def path_inject(own_location: str = "") -> None:
427427
#
428428
# This must be run before we do run any subprocesses, and loading config
429429
# does this as part of the ansible detection.
430-
paths = [x for x in os.environ.get("PATH", "").split(os.pathsep) if x]
430+
paths = [Path(x) for x in os.environ.get("PATH", "").split(os.pathsep) if x]
431431

432432
# Expand ~ in PATH as it known to break many tools
433433
expanded = False
434434
for idx, path in enumerate(paths):
435-
if path.startswith("~"): # pragma: no cover
436-
paths[idx] = str(Path(path).expanduser())
435+
if str(path).startswith("~"): # pragma: no cover
436+
paths[idx] = Path(path).expanduser()
437437
expanded = True
438438
if expanded: # pragma: no cover
439439
print( # noqa: T201
@@ -446,35 +446,37 @@ def path_inject(own_location: str = "") -> None:
446446
userbase_bin_path = Path(site.getuserbase()) / "bin"
447447

448448
if (
449-
str(userbase_bin_path) not in paths
449+
userbase_bin_path not in paths
450450
and (userbase_bin_path / "bin" / "ansible").exists()
451451
):
452-
inject_paths.append(str(userbase_bin_path))
452+
inject_paths.append(userbase_bin_path)
453453

454454
py_path = Path(sys.executable).parent
455455
pipx_path = os.environ.get("PIPX_HOME", "pipx")
456456
if (
457-
str(py_path) not in paths
457+
py_path not in paths
458458
and (py_path / "ansible").exists()
459459
and pipx_path not in str(py_path)
460460
):
461-
inject_paths.append(str(py_path))
461+
inject_paths.append(py_path)
462462

463463
# last option, if nothing else is found, just look next to ourselves...
464464
if own_location:
465465
own_location = os.path.realpath(own_location)
466466
parent = Path(own_location).parent
467-
if (parent / "ansible").exists() and str(parent) not in paths:
468-
inject_paths.append(str(parent))
467+
if (parent / "ansible").exists() and parent not in paths:
468+
inject_paths.append(parent)
469469

470-
if not os.environ.get("PYENV_VIRTUAL_ENV", None):
471-
if inject_paths and not all("pipx" in p for p in inject_paths):
470+
if not os.environ.get("PYENV_VIRTUAL_ENV"):
471+
if inject_paths and not all("pipx" in str(p) for p in inject_paths):
472472
print( # noqa: T201
473-
f"WARNING: PATH altered to include {', '.join(inject_paths)} :: This is usually a sign of broken local setup, which can cause unexpected behaviors.",
473+
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.",
474474
file=sys.stderr,
475475
)
476476
if inject_paths or expanded:
477-
os.environ["PATH"] = os.pathsep.join([*inject_paths, *paths])
477+
os.environ["PATH"] = os.pathsep.join(
478+
[*map(str, inject_paths), *map(str, paths)],
479+
)
478480

479481
# We do know that finding ansible in PATH does not guarantee that it is
480482
# functioning or that is in fact the same version that was installed as

test/test_main.py

+20
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import os
44
import shutil
5+
import site
56
import subprocess
67
import sys
78
import tempfile
@@ -12,6 +13,7 @@
1213
import pytest
1314
from pytest_mock import MockerFixture
1415

16+
from ansiblelint.__main__ import path_inject
1517
from ansiblelint.config import get_version_warning, options
1618
from ansiblelint.constants import RC
1719

@@ -149,3 +151,21 @@ def test_broken_ansible_cfg() -> None:
149151
"Invalid type for configuration option setting: CACHE_PLUGIN_TIMEOUT"
150152
in proc.stderr
151153
)
154+
155+
156+
@pytest.mark.parametrize("tested_path", ("/app/bin/", "/app/bin"))
157+
def test_path_inject(mocker: MockerFixture, tested_path: str) -> None:
158+
"""Asserts PATH is not changed when it contains paths with trailing slashes."""
159+
own_location = Path(tested_path) / "ansible-lint"
160+
161+
# ensure inject_paths is empty before searching around "own_location"
162+
userbase_bin_path = Path(site.getuserbase()) / "bin"
163+
py_path = Path(sys.executable).parent
164+
mocked_path = f"{userbase_bin_path}:{py_path}:{tested_path}"
165+
166+
mocker.patch("os.environ", {"PATH": mocked_path})
167+
mocker.patch("Path.exists", return_value=True)
168+
169+
path_inject(str(own_location))
170+
171+
assert os.environ["PATH"] == mocked_path

0 commit comments

Comments
 (0)