Skip to content
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
40 changes: 23 additions & 17 deletions git/objects/commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,14 +450,7 @@ def trailers_list(self) -> List[Tuple[str, str]]:
:return:
List containing key-value tuples of whitespace stripped trailer information.
"""
cmd = ["git", "interpret-trailers", "--parse"]
proc: Git.AutoInterrupt = self.repo.git.execute( # type: ignore[call-overload]
cmd,
as_process=True,
istream=PIPE,
)
trailer: str = proc.communicate(str(self.message).encode())[0].decode("utf8")
trailer = trailer.strip()
trailer = self._interpret_trailers(self.repo, self.message, ["--parse"], encoding=self.encoding).strip()

if not trailer:
return []
Expand All @@ -469,6 +462,27 @@ def trailers_list(self) -> List[Tuple[str, str]]:

return trailer_list

@classmethod
def _interpret_trailers(
cls,
repo: "Repo",
message: Union[str, bytes],
trailer_args: Sequence[str],
encoding: str = default_encoding,
) -> str:
message_bytes = message if isinstance(message, bytes) else message.encode(encoding, errors="strict")
cmd = [repo.git.GIT_PYTHON_GIT_EXECUTABLE, "interpret-trailers", *trailer_args]
proc: Git.AutoInterrupt = repo.git.execute( # type: ignore[call-overload]
cmd,
as_process=True,
istream=PIPE,
)
try:
stdout_bytes, _ = proc.communicate(message_bytes)
return stdout_bytes.decode(encoding, errors="strict")
finally:
finalize_process(proc)
Comment on lines +480 to +484
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In _interpret_trailers, proc.communicate() captures stderr but it’s discarded, and then finalize_process(proc) may raise GitCommandError without including the subprocess stderr (pipes may already be drained/closed). Capture stderr_bytes from communicate() and pass it into finalize_process(proc, stderr=stderr_bytes) so failures surface with a useful error message.

Suggested change
try:
stdout_bytes, _ = proc.communicate(message_bytes)
return stdout_bytes.decode(encoding, errors="strict")
finally:
finalize_process(proc)
stderr_bytes = None
try:
stdout_bytes, stderr_bytes = proc.communicate(message_bytes)
return stdout_bytes.decode(encoding, errors="strict")
finally:
finalize_process(proc, stderr=stderr_bytes)

Copilot uses AI. Check for mistakes.

@property
def trailers_dict(self) -> Dict[str, List[str]]:
"""Get the trailers of the message as a dictionary.
Expand Down Expand Up @@ -699,15 +713,7 @@ def create_from_tree(
trailer_args.append("--trailer")
trailer_args.append(f"{key}: {val}")

cmd = [repo.git.GIT_PYTHON_GIT_EXECUTABLE, "interpret-trailers"] + trailer_args
proc: Git.AutoInterrupt = repo.git.execute( # type: ignore[call-overload]
cmd,
as_process=True,
istream=PIPE,
)
stdout_bytes, _ = proc.communicate(str(message).encode())
finalize_process(proc)
message = stdout_bytes.decode("utf8")
message = cls._interpret_trailers(repo, str(message), trailer_args)
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_from_tree has already computed conf_encoding from i18n.commitencoding, but trailer application calls _interpret_trailers without using it. To keep trailer I/O aligned with how the commit will be serialized, pass conf_encoding (or have _interpret_trailers derive it from the repo) when applying trailers.

Suggested change
message = cls._interpret_trailers(repo, str(message), trailer_args)
message = cls._interpret_trailers(
repo,
str(message),
trailer_args,
conf_encoding=conf_encoding,
)

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In create_from_tree(), trailers are applied via _interpret_trailers() without passing the repo-configured commit encoding (conf_encoding). This means interpret-trailers I/O is always UTF-8 even when the commit will be serialized using a different encoding, which is inconsistent with trailers_list() and can produce different on-disk bytes. Pass encoding=conf_encoding when calling _interpret_trailers here (and avoid double str() conversion if possible).

Suggested change
message = cls._interpret_trailers(repo, str(message), trailer_args)
if not isinstance(message, str):
message = str(message)
message = cls._interpret_trailers(repo, message, trailer_args, encoding=conf_encoding)

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_from_tree reads i18n.commitencoding into conf_encoding, but the trailer application path calls _interpret_trailers(...) without passing that encoding (so it always encodes/decodes as UTF-8). To keep trailer I/O aligned with the configured commit encoding (and avoid byte-length differences affecting wrapping/formatting), pass encoding=conf_encoding when calling _interpret_trailers here.

Suggested change
message = cls._interpret_trailers(repo, str(message), trailer_args)
message = cls._interpret_trailers(repo, str(message), trailer_args, encoding=conf_encoding)

Copilot uses AI. Check for mistakes.
# END apply trailers

# CREATE NEW COMMIT
Expand Down
65 changes: 65 additions & 0 deletions test/test_commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,71 @@ def test_create_from_tree_with_trailers_list(self, rw_dir):
"Issue": ["456"],
}

@with_rw_directory
def test_create_from_tree_with_non_utf8_trailers(self, rw_dir):
"""Test that trailer creation and parsing respect the configured commit encoding."""
rw_repo = Repo.init(osp.join(rw_dir, "test_trailers_non_utf8"))
with rw_repo.config_writer() as writer:
writer.set_value("i18n", "commitencoding", "ISO-8859-1")

path = osp.join(str(rw_repo.working_tree_dir), "hello.txt")
touch(path)
rw_repo.index.add([path])
tree = rw_repo.index.write_tree()

commit = Commit.create_from_tree(
rw_repo,
tree,
"Résumé",
head=True,
trailers={"Reviewed-by": "André <andre@example.com>"},
)

assert commit.encoding == "ISO-8859-1"
assert "Résumé" in commit.message
assert "Reviewed-by: André <andre@example.com>" in commit.message
assert commit.trailers_list == [("Reviewed-by", "André <andre@example.com>")]

@with_rw_directory
def test_trailers_list_with_non_utf8_message_bytes(self, rw_dir):
"""Test that trailer parsing handles non-UTF-8 commit message bytes."""
rw_repo = Repo.init(osp.join(rw_dir, "test_trailers_non_utf8_bytes"))
with rw_repo.config_writer() as writer:
writer.set_value("i18n", "commitencoding", "ISO-8859-1")

path = osp.join(str(rw_repo.working_tree_dir), "hello.txt")
touch(path)
rw_repo.index.add([path])
tree = rw_repo.index.write_tree()

commit = Commit.create_from_tree(
rw_repo,
tree,
"Résumé",
head=True,
trailers={"Reviewed-by": "André <andre@example.com>"},
)

bytes_commit = Commit(
rw_repo,
commit.binsha,
message=commit.message.encode(commit.encoding),
encoding=commit.encoding,
)

assert bytes_commit.trailers_list == [("Reviewed-by", "André <andre@example.com>")]

def test_interpret_trailers_encodes_before_launching_process(self):
"""Test that encoding failures happen before spawning interpret-trailers."""
repo = Mock()
repo.git = Mock()
repo.git.GIT_PYTHON_GIT_EXECUTABLE = "git"

with self.assertRaises(UnicodeEncodeError):
Commit._interpret_trailers(repo, "Euro: €", ["--parse"], encoding="ISO-8859-1")

repo.git.execute.assert_not_called()

@with_rw_directory
def test_index_commit_with_trailers(self, rw_dir):
"""Test that IndexFile.commit() supports adding trailers."""
Expand Down
Loading