Fix exclude patterns logic

Fixes: #3310

.github/workflows/tox.yml

@@ -59,7 +59,7 @@ jobs:
       WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 795
+      PYTEST_REQPASS: 794
     steps:
       - name: Activate WSL1
         if: "contains(matrix.shell, 'wsl')"

src/ansiblelint/__main__.py

@@ -28,9 +28,7 @@
 import pathlib
 import shutil
 import site
-import subprocess
 import sys
-from contextlib import contextmanager
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Callable, TextIO
 
@@ -49,15 +47,13 @@
     render_yaml,
 )
 from ansiblelint.config import Options, get_version_warning, log_entries, options
-from ansiblelint.constants import GIT_CMD, RC
-from ansiblelint.file_utils import abspath, cwd, normpath
+from ansiblelint.constants import RC
 from ansiblelint.loaders import load_ignore_txt
 from ansiblelint.skip_utils import normalize_tag
 from ansiblelint.version import __version__
 
 if TYPE_CHECKING:
     # RulesCollection must be imported lazily or ansible gets imported too early.
-    from collections.abc import Iterator
 
     from ansiblelint.rules import RulesCollection
     from ansiblelint.runner import LintResult
@@ -286,47 +282,6 @@ def main(argv: list[str] | None = None) -> int:  # noqa: C901
     return app.report_outcome(result, mark_as_success=mark_as_success)
 
 
-@contextmanager
-def _previous_revision() -> Iterator[None]:
-    """Create or update a temporary workdir containing the previous revision."""
-    worktree_dir = f"{options.cache_dir}/old-rev"
-    # Update options.exclude_paths to include use the temporary workdir.
-    rel_exclude_paths = [normpath(p) for p in options.exclude_paths]
-    options.exclude_paths = [abspath(p, worktree_dir) for p in rel_exclude_paths]
-    revision = subprocess.run(
-        [*GIT_CMD, "rev-parse", "HEAD^1"],  # noqa: S603
-        check=True,
-        text=True,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.DEVNULL,
-    ).stdout.strip()
-    _logger.info("Previous revision SHA: %s", revision)
-    path = pathlib.Path(worktree_dir)
-    if path.exists():
-        shutil.rmtree(worktree_dir)
-    path.mkdir(parents=True, exist_ok=True)
-    # Run check will fail if worktree_dir already exists
-    # pylint: disable=subprocess-run-check
-    subprocess.run(
-        [*GIT_CMD, "worktree", "add", "-f", worktree_dir],  # noqa: S603
-        stdout=subprocess.DEVNULL,
-        stderr=subprocess.DEVNULL,
-    )
-    try:
-        with cwd(pathlib.Path(worktree_dir)):
-            subprocess.run(
-                [*GIT_CMD, "checkout", revision],  # noqa: S603
-                stdout=subprocess.DEVNULL,
-                stderr=subprocess.DEVNULL,
-                check=True,
-            )
-            yield
-    finally:
-        options.exclude_paths = [
-            str(Path.cwd().resolve() / p) for p in rel_exclude_paths
-        ]
-
-
 def _run_cli_entrypoint() -> None:
     """Invoke the main entrypoint with current CLI args.
 

src/ansiblelint/cli.py

@@ -34,7 +34,6 @@
 
 _logger = logging.getLogger(__name__)
 _PATH_VARS = [
-    "exclude_paths",
     "rulesdir",
 ]
 
@@ -424,8 +423,9 @@ def get_cli_parser() -> argparse.ArgumentParser:
     parser.add_argument(
         "--exclude",
         dest="exclude_paths",
-        action=AbspathArgAction,
-        type=Path,
+        action="extend",
+        nargs="+",
+        type=str,
         default=[],
         help="path to directories or files to skip. This option is repeatable.",
     )
@@ -594,6 +594,11 @@ def get_config(arguments: list[str]) -> Options:
         msg = f"Failed to determine a valid project_dir: {options.project_dir}"
         raise RuntimeError(msg)
 
+    # expand user home dir in exclude_paths
+    options.exclude_paths = [
+        os.path.expandvars(os.path.expanduser(p)) for p in options.exclude_paths
+    ]
+
     # Compute final verbosity level by subtracting -q counter.
     options.verbosity -= options.quiet
     return config

src/ansiblelint/file_utils.py

@@ -13,7 +13,7 @@
 from typing import TYPE_CHECKING, Any, cast
 
 import wcmatch.pathlib
-from wcmatch.wcmatch import RECURSIVE, WcMatch
+import wcmatch.wcmatch
 from yaml.error import YAMLError
 
 from ansiblelint.config import BASE_KINDS, Options, options
@@ -446,19 +446,22 @@ def discover_lintables(options: Options) -> dict[str, Any]:
         if options.verbosity:
             warn_or_fail(f"Failed to locate command: {exc}")
 
-    if out is None:
-        exclude_pattern = "|".join(str(x) for x in options.exclude_paths)
-        _logger.info("Looking up for files, excluding %s ...", exclude_pattern)
-        # remove './' prefix from output of WcMatch
-        out = {
-            strip_dotslash_prefix(fname)
-            for fname in WcMatch(
-                ".",
-                exclude_pattern=exclude_pattern,
-                flags=RECURSIVE,
-                limit=256,
-            ).match()
-        }
+    # Applying exclude patterns
+    if not out:
+        out = set(".")
+
+    exclude_pattern = "|".join(str(x) for x in options.exclude_paths)
+    _logger.info("Looking up for files, excluding %s ...", exclude_pattern)
+    # remove './' prefix from output of WcMatch
+    out = {
+        strip_dotslash_prefix(fname)
+        for fname in wcmatch.wcmatch.WcMatch(
+            ".",
+            exclude_pattern=exclude_pattern,
+            flags=wcmatch.wcmatch.RECURSIVE,
+            limit=256,
+        ).match()
+    }
 
     return OrderedDict.fromkeys(sorted(out))
 

src/ansiblelint/utils.py

@@ -1043,20 +1043,6 @@ def get_lintables(
     else:
         for filename in discover_lintables(opts):
             path = Path(filename)
-            # skip exclusions
-            try:
-                for file_path in opts.exclude_paths:
-                    if str(path.resolve()).startswith(str(file_path)):
-                        msg = f"File {file_path} matched exclusion entry: {path}"
-                        raise FileNotFoundError(msg)
-            except FileNotFoundError as exc:
-                _logger.debug("Ignored %s due to: %s", path, exc)
-                continue
-
-            if path.is_symlink() and not path.exists():
-                _logger.warning("Ignored broken symlink %s -> %s", path, path.resolve())
-                continue
-
             lintables.append(Lintable(path))
 
         # stage 2: guess roles from current lintables, as there is no unique

test/test_cli.py

@@ -20,41 +20,49 @@ def fixture_base_arguments() -> list[str]:
 
 
 @pytest.mark.parametrize(
-    ("args", "config"),
+    ("args", "config_path"),
     (
-        (["-p"], "test/fixtures/parseable.yml"),
-        (["-q"], "test/fixtures/quiet.yml"),
-        (["-r", "test/fixtures/rules/"], "test/fixtures/rulesdir.yml"),
-        (["-R", "-r", "test/fixtures/rules/"], "test/fixtures/rulesdir-defaults.yml"),
-        (["-s"], "test/fixtures/strict.yml"),
-        (["-t", "skip_ansible_lint"], "test/fixtures/tags.yml"),
-        (["-v"], "test/fixtures/verbosity.yml"),
-        (["-x", "bad_tag"], "test/fixtures/skip-tags.yml"),
-        (["--exclude", "test/"], "test/fixtures/exclude-paths.yml"),
-        (["--show-relpath"], "test/fixtures/show-abspath.yml"),
-        ([], "test/fixtures/show-relpath.yml"),
+        pytest.param(["-p"], "test/fixtures/parseable.yml", id="1"),
+        pytest.param(["-q"], "test/fixtures/quiet.yml", id="2"),
+        pytest.param(
+            ["-r", "test/fixtures/rules/"],
+            "test/fixtures/rulesdir.yml",
+            id="3",
+        ),
+        pytest.param(
+            ["-R", "-r", "test/fixtures/rules/"],
+            "test/fixtures/rulesdir-defaults.yml",
+            id="4",
+        ),
+        pytest.param(["-s"], "test/fixtures/strict.yml", id="5"),
+        pytest.param(["-t", "skip_ansible_lint"], "test/fixtures/tags.yml", id="6"),
+        pytest.param(["-v"], "test/fixtures/verbosity.yml", id="7"),
+        pytest.param(["-x", "bad_tag"], "test/fixtures/skip-tags.yml", id="8"),
+        pytest.param(["--exclude", "../"], "test/fixtures/exclude-paths.yml", id="9"),
+        pytest.param(["--show-relpath"], "test/fixtures/show-abspath.yml", id="10"),
+        pytest.param([], "test/fixtures/show-relpath.yml", id="11"),
     ),
 )
 def test_ensure_config_are_equal(
     base_arguments: list[str],
     args: list[str],
-    config: str,
+    config_path: str,
 ) -> None:
     """Check equality of the CLI options to config files."""
     command = base_arguments + args
     cli_parser = cli.get_cli_parser()
 
     options = cli_parser.parse_args(command)
-    file_config = cli.load_config(config)[0]
-
+    file_config = cli.load_config(config_path)[0]
     for key, val in file_config.items():
         # config_file does not make sense in file_config
         if key == "config_file":
             continue
 
-        if key in {"exclude_paths", "rulesdir"}:
+        if key == "rulesdir":
+            # this is list of Paths
             val = [Path(p) for p in val]
-        assert val == getattr(options, key)
+        assert val == getattr(options, key), f"Mismatch for {key}"
 
 
 @pytest.mark.parametrize(
@@ -175,28 +183,6 @@ def test_path_from_config_do_not_depend_on_cwd(
     assert config1["exclude_paths"].sort() == config2["exclude_paths"].sort()
 
 
-def test_path_from_cli_depend_on_cwd(
-    base_arguments: list[str],
-    monkeypatch: MonkeyPatch,
-) -> None:
-    """Check that CLI-provided paths are relative to CWD."""
-    # Issue 572
-    arguments = [
-        *base_arguments,
-        "--exclude",
-        "test/fixtures/config-with-relative-path.yml",
-    ]
-
-    options1 = cli.get_cli_parser().parse_args(arguments)
-    assert "test/test" not in str(options1.exclude_paths[0])
-
-    test_dir = "test"
-    monkeypatch.chdir(test_dir)
-    options2 = cli.get_cli_parser().parse_args(arguments)
-
-    assert "test/test" in str(options2.exclude_paths[0])
-
-
 @pytest.mark.parametrize(
     "config_file",
     (

test/test_runner.py

@@ -99,8 +99,8 @@ def test_runner_exclude_globs(
     )
 
     matches = runner.run()
-    # we expect to find one error from the only .yaml file we have there.
-    assert len(matches) == 1
+    # we expect to find one 2 matches from the very few .yaml file we have there (most of them have .yml extension)
+    assert len(matches) == 2
 
 
 @pytest.mark.parametrize(

test/test_utils.py

@@ -34,7 +34,7 @@
 from ansiblelint.__main__ import initialize_logger
 from ansiblelint.cli import get_rules_dirs
 from ansiblelint.constants import RC
-from ansiblelint.file_utils import Lintable
+from ansiblelint.file_utils import Lintable, cwd
 from ansiblelint.runner import Runner
 
 if TYPE_CHECKING:
@@ -44,7 +44,6 @@
     from _pytest.logging import LogCaptureFixture
     from _pytest.monkeypatch import MonkeyPatch
 
-    from ansiblelint.config import Options
     from ansiblelint.rules import RulesCollection
 
 
@@ -324,17 +323,25 @@ def test_is_playbook() -> None:
     assert utils.is_playbook("examples/playbooks/always-run-success.yml")
 
 
-def test_auto_detect_exclude(monkeypatch: MonkeyPatch) -> None:
+def test_auto_detect_exclude(tmp_path: Path) -> None:
     """Verify that exclude option can be used to narrow down detection."""
-    options = cli.get_config(["--exclude", "foo"])
-
-    # pylint: disable=unused-argument
-    def mockreturn(options: Options) -> list[str]:  # noqa: ARG001
-        return ["foo/playbook.yml", "bar/playbook.yml"]
-
-    monkeypatch.setattr(utils, "discover_lintables", mockreturn)
-    result = utils.get_lintables(options)
-    assert result == [Lintable("bar/playbook.yml", kind="playbook")]
+    with cwd(tmp_path):
+        subprocess.check_output(
+            "git init",
+            stderr=subprocess.STDOUT,
+            text=True,
+            shell=True,
+            cwd=tmp_path,
+        )
+        (tmp_path / "foo").mkdir()
+        (tmp_path / "bar").mkdir()
+        (tmp_path / "foo" / "playbook.yml").touch()
+        (tmp_path / "bar" / "playbook.yml").touch()
+        options = cli.get_config(["--exclude", "foo"])
+        options.cwd = tmp_path
+
+        result = utils.get_lintables(options)
+        assert result == [Lintable("bar/playbook.yml", kind="playbook")]
 
 
 _DEFAULT_RULEDIRS = [constants.DEFAULT_RULESDIR]