diff --git a/subcmds/cherry_pick.py b/subcmds/cherry_pick.py index 3d2193339..4e7b978c8 100644 --- a/subcmds/cherry_pick.py +++ b/subcmds/cherry_pick.py @@ -14,6 +14,7 @@ import re import sys +from typing import Tuple from command import Command from error import GitError @@ -43,36 +44,8 @@ change id will be added. def Execute(self, opt, args): reference = args[0] - - p = GitCommand( - None, - ["rev-parse", "--verify", reference], - capture_stdout=True, - capture_stderr=True, - verify_command=True, - ) - try: - p.Wait() - except GitError: - logger.error(p.stderr) - raise - - sha1 = p.stdout.strip() - - p = GitCommand( - None, - ["cat-file", "commit", sha1], - capture_stdout=True, - verify_command=True, - ) - - try: - p.Wait() - except GitError: - logger.error("error: Failed to retrieve old commit message") - raise - - old_msg = self._StripHeader(p.stdout) + sha1, commit = self._ResolveReference(reference) + old_msg = self._StripHeader(commit) p = GitCommand( None, @@ -117,6 +90,48 @@ change id will be added. logger.error("error: Failed to update commit message") raise + def _ResolveReference(self, reference: str) -> Tuple[str, str]: + """Resolve a commit and read it through one cat-file batch request.""" + expression = f"{reference}^{{commit}}" + p = GitCommand( + None, + ["cat-file", "--batch"], + input=expression + "\n", + capture_stdout=True, + capture_stderr=True, + verify_command=True, + ) + try: + p.Wait() + header, separator, output = p.stdout.partition("\n") + if not separator: + raise ValueError("missing cat-file header") + if header.endswith(" missing") or header.endswith(" ambiguous"): + raise GitError(f"commit {reference} not found") + + parts = header.split(" ", 2) + if len(parts) != 3 or parts[1] != "commit": + raise ValueError( + f"unexpected object type {parts[1]!r}" + if len(parts) >= 2 + else "invalid header" + ) + sha1, _object_type, _size = parts + + if not output.endswith("\n"): + raise ValueError("truncated cat-file object") + + commit = output[:-1] + except (GitError, ValueError) as e: + logger.error( + "error: Failed to resolve or read commit %s", reference + ) + if isinstance(e, GitError): + raise + raise GitError(str(e)) from e + + return sha1, commit + def _IsChangeId(self, line): return CHANGE_ID_RE.match(line) diff --git a/tests/test_subcmds_cherry_pick.py b/tests/test_subcmds_cherry_pick.py new file mode 100644 index 000000000..b52efc86b --- /dev/null +++ b/tests/test_subcmds_cherry_pick.py @@ -0,0 +1,85 @@ +# Copyright (C) 2026 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Unittests for subcmds/cherry_pick.py.""" + +from unittest import mock + +import pytest + +from error import GitError +from git_command import GitCommand +from subcmds import cherry_pick + + +def test_resolve_reference_uses_one_typed_batch_request( + monkeypatch: pytest.MonkeyPatch, +) -> None: + oid = "1" * 40 + commit = "tree " + "2" * 40 + "\n\nSubject 🚀\n\nBody\n" + output = oid + " commit " + str(len(commit.encode("utf-8"))) + "\n" + output += commit + "\n" + command = mock.create_autospec(GitCommand, instance=True) + command.stdout = output + command.stderr = "" + command.Wait.return_value = 0 + run_git = mock.create_autospec(GitCommand, return_value=command) + monkeypatch.setattr(cherry_pick, "GitCommand", run_git) + + resolved, contents = cherry_pick.CherryPick()._ResolveReference("topic") + + assert resolved == oid + assert contents == commit + run_git.assert_called_once_with( + None, + ["cat-file", "--batch"], + input="topic^{commit}\n", + capture_stdout=True, + capture_stderr=True, + verify_command=True, + ) + + +def test_resolve_reference_rejects_missing_object( + monkeypatch: pytest.MonkeyPatch, +) -> None: + command = mock.create_autospec(GitCommand, instance=True) + command.stdout = "topic^{commit} missing\n" + command.stderr = "" + command.Wait.return_value = 0 + monkeypatch.setattr( + cherry_pick, + "GitCommand", + mock.create_autospec(GitCommand, return_value=command), + ) + + with pytest.raises(GitError, match="commit topic not found"): + cherry_pick.CherryPick()._ResolveReference("topic") + + +def test_resolve_reference_rejects_ambiguous_object( + monkeypatch: pytest.MonkeyPatch, +) -> None: + command = mock.create_autospec(GitCommand, instance=True) + command.stdout = "topic^{commit} ambiguous\n" + command.stderr = "" + command.Wait.return_value = 0 + monkeypatch.setattr( + cherry_pick, + "GitCommand", + mock.create_autospec(GitCommand, return_value=command), + ) + + with pytest.raises(GitError, match="commit topic not found"): + cherry_pick.CherryPick()._ResolveReference("topic")