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

Avoid dubious ownership errors with newer versions of git #2890

Merged
merged 1 commit into from
Jan 16, 2023
Merged
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
8 changes: 4 additions & 4 deletions src/ansiblelint/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions src/ansiblelint/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()}"]
8 changes: 4 additions & 4 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions test/test_progressive.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import sys
from pathlib import Path

from ansiblelint.constants import GIT_CMD
from ansiblelint.file_utils import cwd

FAULTY_PLAYBOOK = """---
Expand All @@ -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", "[email protected]"], 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", "[email protected]"], 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]:
Expand Down
7 changes: 2 additions & 5 deletions test/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: "
Expand Down