From 030f151335b85dc2d56f7b4cfb7e57f80f3b4af9 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Thu, 12 Jan 2023 14:31:42 +0000 Subject: [PATCH] Avoid dubious ownership errors with newer versions of git Related: https://github.com/ansible/ansible-lint-action/issues/138 --- src/ansiblelint/__main__.py | 8 ++++---- src/ansiblelint/constants.py | 7 +++++++ src/ansiblelint/file_utils.py | 8 ++++---- test/test_progressive.py | 11 ++++++----- test/test_utils.py | 7 ++----- 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/ansiblelint/__main__.py b/src/ansiblelint/__main__.py index d1de1b4867..2865edc04a 100755 --- a/src/ansiblelint/__main__.py +++ b/src/ansiblelint/__main__.py @@ -48,7 +48,7 @@ render_yaml, ) from ansiblelint.config import get_version_warning, options -from ansiblelint.constants import EXIT_CONTROL_C_RC, LOCK_TIMEOUT_RC +from ansiblelint.constants import EXIT_CONTROL_C_RC, GIT_CMD, LOCK_TIMEOUT_RC from ansiblelint.file_utils import abspath, cwd, normpath from ansiblelint.skip_utils import normalize_tag from ansiblelint.version import __version__ @@ -304,7 +304,7 @@ def _previous_revision() -> Iterator[None]: 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", "rev-parse", "HEAD^1"], + [*GIT_CMD, "rev-parse", "HEAD^1"], check=True, text=True, stdout=subprocess.PIPE, @@ -318,14 +318,14 @@ def _previous_revision() -> Iterator[None]: # Run check will fail if worktree_dir already exists # pylint: disable=subprocess-run-check subprocess.run( - ["git", "worktree", "add", "-f", worktree_dir], + [*GIT_CMD, "worktree", "add", "-f", worktree_dir], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, ) try: with cwd(worktree_dir): subprocess.run( - ["git", "checkout", revision], + [*GIT_CMD, "checkout", revision], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=True, diff --git a/src/ansiblelint/constants.py b/src/ansiblelint/constants.py index b359e86fa8..48c0d19b6a 100644 --- a/src/ansiblelint/constants.py +++ b/src/ansiblelint/constants.py @@ -145,3 +145,10 @@ def main(): "import_role", "include_role", } + +# Newer versions of git might fail to run when different file ownership is +# found of repo. One example is on GHA runners executing containerized +# reusable actions, where the mounted volume might have different owner. +# +# https://github.com/ansible/ansible-lint-action/issues/138 +GIT_CMD = ["git", "-c", f"safe.directory={os.getcwd()}"] diff --git a/src/ansiblelint/file_utils.py b/src/ansiblelint/file_utils.py index 24562a7394..041a988a76 100644 --- a/src/ansiblelint/file_utils.py +++ b/src/ansiblelint/file_utils.py @@ -19,7 +19,7 @@ from wcmatch.wcmatch import RECURSIVE, WcMatch from ansiblelint.config import BASE_KINDS, options -from ansiblelint.constants import FileType +from ansiblelint.constants import GIT_CMD, FileType if TYPE_CHECKING: # https://github.com/PyCQA/pylint/issues/3979 @@ -368,14 +368,14 @@ def discover_lintables(options: Namespace) -> dict[str, Any]: """ # git is preferred as it also considers .gitignore git_command_present = [ - "git", + *GIT_CMD, "ls-files", "--cached", "--others", "--exclude-standard", "-z", ] - git_command_absent = ["git", "ls-files", "--deleted", "-z"] + git_command_absent = [*GIT_CMD, "ls-files", "--deleted", "-z"] out = None try: @@ -437,7 +437,7 @@ def guess_project_dir(config_file: str | None) -> str: if path is None: try: result = subprocess.run( - ["git", "rev-parse", "--show-toplevel"], + [*GIT_CMD, "rev-parse", "--show-toplevel"], capture_output=True, text=True, check=True, diff --git a/test/test_progressive.py b/test/test_progressive.py index bde8079d35..f540ed15af 100644 --- a/test/test_progressive.py +++ b/test/test_progressive.py @@ -6,6 +6,7 @@ import sys from pathlib import Path +from ansiblelint.constants import GIT_CMD from ansiblelint.file_utils import cwd FAULTY_PLAYBOOK = """--- @@ -29,16 +30,16 @@ def git_init() -> None: """Init temporary git repository.""" - subprocess.run(["git", "init", "--initial-branch=main"], check=True) - subprocess.run(["git", "config", "user.email", "test@example.com"], check=True) - subprocess.run(["git", "config", "user.name", "test"], check=True) + subprocess.run([*GIT_CMD, "init", "--initial-branch=main"], check=True) + subprocess.run([*GIT_CMD, "config", "user.email", "test@example.com"], check=True) + subprocess.run([*GIT_CMD, "config", "user.name", "test"], check=True) def git_commit(filename: Path, content: str) -> None: """Create and commit a file.""" filename.write_text(content) - subprocess.run(["git", "add", filename], check=True) - subprocess.run(["git", "commit", "-a", "-m", f"Commit {filename}"], check=True) + subprocess.run([*GIT_CMD, "add", filename], check=True) + subprocess.run([*GIT_CMD, "commit", "-a", "-m", f"Commit {filename}"], check=True) def run_lint(cmd: list[str]) -> subprocess.CompletedProcess[str]: diff --git a/test/test_utils.py b/test/test_utils.py index 9ed405b17a..5099a420a1 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -288,11 +288,8 @@ def test_cli_auto_detect(capfd: CaptureFixture[str]) -> None: out, err = capfd.readouterr() # Confirmation that it runs in auto-detect mode - assert ( - "Discovered files to lint using: git ls-files --cached --others --exclude-standard -z" - in err - ) - assert "Excluded removed files using: git ls-files --deleted -z" in err + assert "Discovered files to lint using: git" in err + assert "Excluded removed files using: git" in err # An expected rule match from our examples assert ( "examples/playbooks/empty_playbook.yml:1:1: "