Skip to content
Open
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
1 change: 1 addition & 0 deletions changelog/51157.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Suppress noisy ERROR log messages when git.is_worktree probes a directory that is not a git repository.
11 changes: 9 additions & 2 deletions salt/modules/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,9 @@ def _git_run(
return result


def _get_toplevel(path, user=None, password=None, output_encoding=None):
def _get_toplevel(
path, user=None, password=None, ignore_retcode=False, output_encoding=None
):
"""
Use git rev-parse to return the top level of a repo
"""
Expand All @@ -417,6 +419,7 @@ def _get_toplevel(path, user=None, password=None, output_encoding=None):
cwd=path,
user=user,
password=password,
ignore_retcode=ignore_retcode,
output_encoding=output_encoding,
)["stdout"]

Expand Down Expand Up @@ -2419,7 +2422,11 @@ def is_worktree(cwd, user=None, password=None, output_encoding=None):
cwd = _expand_path(cwd, user)
try:
toplevel = _get_toplevel(
cwd, user=user, password=password, output_encoding=output_encoding
cwd,
user=user,
password=password,
ignore_retcode=True,
output_encoding=output_encoding,
)
except CommandExecutionError:
return False
Expand Down
88 changes: 88 additions & 0 deletions tests/pytests/unit/modules/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,3 +285,91 @@ def test_tag_rejects_message_in_opts(tmp_path):
git_mod.tag(str(tmp_path), "v1.2", opts="-m 'sneaky'")

git_run_mock.assert_not_called()


def test_is_worktree_probe_ignores_retcode():
"""
Regression guard for #51157.

``git.is_worktree`` probes ``cwd`` with ``git rev-parse --show-toplevel``
and expects that command to fail (retcode 128) when ``cwd`` is not a git
repository. That expected failure must be run with ``ignore_retcode=True``
so the noisy ERROR-level logging is suppressed while still returning False.
"""
cmd_run_mock = MagicMock(
return_value={
"stdout": "",
"stderr": (
"fatal: not a git repository (or any of the parent "
"directories): .git"
),
"retcode": 128,
"pid": 12345,
}
)
with patch.dict(git_mod.__salt__, {"cmd.run_all": cmd_run_mock}), patch.object(
git_mod, "_expand_path", lambda cwd, user: str(cwd)
):
assert git_mod.is_worktree("/not/a/repo") is False

cmd_run_mock.assert_called_once()
assert cmd_run_mock.call_args.kwargs.get("ignore_retcode") is True


def test_get_toplevel_forwards_ignore_retcode_51157():
"""
Regression test for #51157.

``_get_toplevel`` must accept ``ignore_retcode`` and forward it to
``cmd.run_all`` so that an expected rev-parse failure is not logged at
ERROR level. This calls the helper directly with ``ignore_retcode=True``,
which is exactly what its production caller ``is_worktree`` passes when
probing a path that may not be a git repository.
"""
cmd_run_mock = MagicMock(
return_value={
"stdout": "/some/repo",
"stderr": "",
"retcode": 0,
"pid": 12345,
}
)
with patch.dict(git_mod.__salt__, {"cmd.run_all": cmd_run_mock}):
# ignore_retcode=True is the decisive flag; is_worktree passes it
# because the probe is expected to fail on non-repo paths.
result = git_mod._get_toplevel("/some/repo", ignore_retcode=True)

assert result == "/some/repo"
cmd_run_mock.assert_called_once()
assert cmd_run_mock.call_args.kwargs.get("ignore_retcode") is True


def test_get_toplevel_default_stays_loud_51157():
"""
Overcorrection guard for #51157.

Only the ``is_worktree`` probe opts in to ``ignore_retcode``. Other
production callers such as ``list_worktrees`` invoke ``_get_toplevel``
without it, and a genuine rev-parse failure there must NOT be silenced
by this fix: ``cmd.run_all`` must still receive ``ignore_retcode=False``
(so the failure is logged) and ``_git_run`` must still raise
``CommandExecutionError``. This test passes both with and without the
fix applied; it guards against the default flipping to True.
"""
cmd_run_mock = MagicMock(
return_value={
"stdout": "",
"stderr": (
"fatal: not a git repository (or any of the parent "
"directories): .git"
),
"retcode": 128,
"pid": 12345,
}
)
with patch.dict(git_mod.__salt__, {"cmd.run_all": cmd_run_mock}):
with pytest.raises(git_mod.CommandExecutionError):
git_mod._get_toplevel("/some/repo")

cmd_run_mock.assert_called_once()
assert cmd_run_mock.call_args.kwargs.get("ignore_retcode") is False
Loading