Add trailer support for commit creation#2116
Add trailer support for commit creation#2116Byron merged 2 commits intogitpython-developers:mainfrom
Conversation
Add a `trailers` parameter to `Commit.create_from_tree()` and `IndexFile.commit()` that allows appending trailer key-value pairs (e.g. Signed-off-by, Issue) to the commit message at creation time. Trailers can be passed as either a dict or a list of (key, value) tuples, the latter being useful when duplicate keys are needed. The implementation uses `git interpret-trailers` for proper formatting, consistent with the existing trailer parsing in `Commit.trailers_list`. Closes gitpython-developers#1998
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Adds support for creating commit trailers when programmatically generating commits, aligning commit creation with GitPython’s existing trailer parsing capabilities.
Changes:
- Add a
trailersparameter toCommit.create_from_tree()to append trailers viagit interpret-trailers. - Thread
trailersthroughIndexFile.commit()so index commits can include trailers. - Add tests covering dict-based trailers, list-of-tuples trailers (duplicate keys), and the index commit plumbing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
git/objects/commit.py |
Adds trailers support in Commit.create_from_tree() by invoking git interpret-trailers. |
git/index/base.py |
Exposes trailers in IndexFile.commit() and forwards it to Commit.create_from_tree(). |
test/test_commit.py |
Adds round-trip tests validating trailer creation and parsing for both APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot, this looks good to me.
The Copilot comments should be addressed and then it can be merged.
|
@Byron addressed those comments, can you please review? |
|
It doesn't look like they are resolved, nor was anything pushed. Please also resolve them. Thank you. |
4da170c to
af0933c
Compare
|
@Byron sorry, missed pushing them. Can you review now? |
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot, looks good to me!
It's a shame that it's shelling out to build a string, but better to have a slow but correct version, than nothing at all in that regard.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stdout_bytes, _ = proc.communicate(str(message).encode()) | ||
| finalize_process(proc) | ||
| message = stdout_bytes.decode("utf8") |
There was a problem hiding this comment.
interpret-trailers I/O is hard-coded to UTF-8 (.encode() default + .decode('utf8')), but this method already resolves conf_encoding for the commit’s declared encoding. If i18n.commitencoding is not UTF-8, this can raise decode errors or corrupt non-ASCII characters; use the resolved encoding for both encoding stdin and decoding stdout (and consider an explicit error strategy).
| stdout_bytes, _ = proc.communicate(str(message).encode()) | |
| finalize_process(proc) | |
| message = stdout_bytes.decode("utf8") | |
| stdout_bytes, _ = proc.communicate(str(message).encode(conf_encoding, errors="strict")) | |
| finalize_process(proc) | |
| message = stdout_bytes.decode(conf_encoding, errors="strict") |
| ) | ||
| stdout_bytes, _ = proc.communicate(str(message).encode()) | ||
| finalize_process(proc) |
There was a problem hiding this comment.
This uses as_process=True, which makes Git.execute(..., with_exceptions=True) ineffective (see Git.execute docs), but the code never checks proc.returncode / stderr after communicate(). If interpret-trailers fails (e.g., unsupported --trailer on an older Git), message may be replaced with empty/partial stdout and the commit is still created; please detect non-zero exit status and raise (or preserve the original message on failure).
| ) | |
| stdout_bytes, _ = proc.communicate(str(message).encode()) | |
| finalize_process(proc) | |
| stderr=PIPE, | |
| ) | |
| stdout_bytes, stderr_bytes = proc.communicate(str(message).encode()) | |
| finalize_process(proc) | |
| if proc.returncode: | |
| stderr = stderr_bytes.decode("utf8", errors="replace") if stderr_bytes else "" | |
| raise RuntimeError( | |
| "git interpret-trailers failed with exit code " | |
| f"{proc.returncode}: {stderr or 'no error output'}" | |
| ) |
| cmd = [repo.git.GIT_PYTHON_GIT_EXECUTABLE, "interpret-trailers"] + trailer_args | ||
| proc: Git.AutoInterrupt = repo.git.execute( # type: ignore[call-overload] | ||
| cmd, |
There was a problem hiding this comment.
The command uses repo.git.GIT_PYTHON_GIT_EXECUTABLE here, but Commit.trailers_list/trailers_dict currently invoke interpret-trailers via a hard-coded ['git', ...]. If a caller overrides GIT_PYTHON_GIT_EXECUTABLE, trailer creation and trailer parsing can end up using different Git binaries; consider aligning both paths on the same executable selection approach for consistent round-trips.
| cmd = [repo.git.GIT_PYTHON_GIT_EXECUTABLE, "interpret-trailers"] + trailer_args | |
| proc: Git.AutoInterrupt = repo.git.execute( # type: ignore[call-overload] | |
| cmd, | |
| proc: Git.AutoInterrupt = repo.git._call_process( # type: ignore[attr-defined] | |
| "interpret-trailers", | |
| *trailer_args, |
This MR contains the following updates: | Package | Type | Update | Change | OpenSSF | |---|---|---|---|---| | [gitpython](https://github.com/gitpython-developers/GitPython) | dev | patch | `3.1.46` → `3.1.47` | [](https://securityscorecards.dev/viewer/?uri=github.com/gitpython-developers/GitPython) | --- ### Release Notes <details> <summary>gitpython-developers/GitPython (gitpython)</summary> ### [`v3.1.47`](https://github.com/gitpython-developers/GitPython/releases/tag/3.1.47): - with security fixes [Compare Source](gitpython-developers/GitPython@3.1.46...3.1.47) #### Advisories - <GHSA-rpm5-65cw-6hj4> - <GHSA-x2qx-6953-8485> #### What's Changed - Prepare next release by [@​Byron](https://github.com/Byron) in [#​2095](gitpython-developers/GitPython#2095) - Bump git/ext/gitdb from `335c0f6` to `4c63ee6` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​2096](gitpython-developers/GitPython#2096) - DOC: README Add urls and updated a relative url by [@​Timour-Ilyas](https://github.com/Timour-Ilyas) in [#​2098](gitpython-developers/GitPython#2098) - Fix GitConfigParser ignoring multiple \[include] path entries by [@​daniel7an](https://github.com/daniel7an) in [#​2100](gitpython-developers/GitPython#2100) - Switch back from Alpine to Debian for WSL by [@​EliahKagan](https://github.com/EliahKagan) in [#​2108](gitpython-developers/GitPython#2108) - Bump git/ext/gitdb from `4c63ee6` to `5c1b303` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​2106](gitpython-developers/GitPython#2106) - Run `gc.collect()` twice in `test_rename` on Python 3.12 by [@​EliahKagan](https://github.com/EliahKagan) in [#​2109](gitpython-developers/GitPython#2109) - fix: guard AutoInterrupt terminate during interpreter shutdown by [@​lweyrich1](https://github.com/lweyrich1) in [#​2105](gitpython-developers/GitPython#2105) - Improve CI infrastructure for pre-commit by [@​EliahKagan](https://github.com/EliahKagan) in [#​2110](gitpython-developers/GitPython#2110) - Bump the pre-commit group with 5 updates by [@​dependabot](https://github.com/dependabot)\[bot] in [#​2111](gitpython-developers/GitPython#2111) - Upgrade Sphinx for 3.14 support; drop doc build support on 3.8; test 3.14 by [@​EliahKagan](https://github.com/EliahKagan) in [#​2112](gitpython-developers/GitPython#2112) - Fix `Repo.active_branch` resolution for reftable-backed repositories by [@​Copilot](https://github.com/Copilot) in [#​2114](gitpython-developers/GitPython#2114) - docs: warn about GitDB performance with large commits by [@​mvanhorn](https://github.com/mvanhorn) in [#​2115](gitpython-developers/GitPython#2115) - cmd: fix kwarg formatting in docstring example by [@​UweSchwaeke](https://github.com/UweSchwaeke) in [#​2117](gitpython-developers/GitPython#2117) - Bump <https://github.com/astral-sh/ruff-pre-commit> from v0.15.5 to 0.15.8 in the pre-commit group by [@​dependabot](https://github.com/dependabot)\[bot] in [#​2122](gitpython-developers/GitPython#2122) - Add trailer support for commit creation by [@​Krishnachaitanyakc](https://github.com/Krishnachaitanyakc) in [#​2116](gitpython-developers/GitPython#2116) - Harden commit trailer subprocess handling and align trailer I/O paths by [@​Copilot](https://github.com/Copilot) in [#​2125](gitpython-developers/GitPython#2125) - git.cmd.Git.execute(..): fix `with_stdout=False` by [@​ngie-eign](https://github.com/ngie-eign) in [#​2126](gitpython-developers/GitPython#2126) - Make sure that multi-options are checked after splitting them with `shlex` by [@​Byron](https://github.com/Byron) in [#​2130](gitpython-developers/GitPython#2130) - Block unsafe underscored git kwargs / Fix for GHSA-rpm5-65cw-6hj4 by [@​WesR](https://github.com/WesR) in [#​2131](gitpython-developers/GitPython#2131) #### New Contributors - [@​Timour-Ilyas](https://github.com/Timour-Ilyas) made their first contribution in [#​2098](gitpython-developers/GitPython#2098) - [@​daniel7an](https://github.com/daniel7an) made their first contribution in [#​2100](gitpython-developers/GitPython#2100) - [@​lweyrich1](https://github.com/lweyrich1) made their first contribution in [#​2105](gitpython-developers/GitPython#2105) - [@​Copilot](https://github.com/Copilot) made their first contribution in [#​2114](gitpython-developers/GitPython#2114) - [@​mvanhorn](https://github.com/mvanhorn) made their first contribution in [#​2115](gitpython-developers/GitPython#2115) - [@​UweSchwaeke](https://github.com/UweSchwaeke) made their first contribution in [#​2117](gitpython-developers/GitPython#2117) - [@​Krishnachaitanyakc](https://github.com/Krishnachaitanyakc) made their first contribution in [#​2116](gitpython-developers/GitPython#2116) - [@​ngie-eign](https://github.com/ngie-eign) made their first contribution in [#​2126](gitpython-developers/GitPython#2126) - [@​WesR](https://github.com/WesR) made their first contribution in [#​2131](gitpython-developers/GitPython#2131) **Full Changelog**: <gitpython-developers/GitPython@3.1.46...3.1.47> </details> --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xNDEuNSIsInVwZGF0ZWRJblZlciI6IjQzLjE0MS41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJkZXBlbmRlbmNpZXMiLCJyZW5vdmF0ZSJdfQ==--> See merge request swiss-armed-forces/cyber-command/cea/loom!486 Co-authored-by: Loom MR Pipeline Trigger <group_103951964_bot_9504bb8dead6d4e406ad817a607f24be@noreply.gitlab.com>
Summary
trailersparameter toCommit.create_from_tree()andIndexFile.commit()enabling trailer creation (e.g.Signed-off-by,Issue) when committing programmaticallydictor a list of(key, value)tuples (for duplicate keys like multipleSigned-off-by)git interpret-trailersfor formatting, consistent with existing trailer parsing inCommit.trailers_listCloses #1998
Motivation
GitPython already supports parsing trailers from existing commits, but did not support adding them when creating new commits. The
git commitCLI supports this via--trailer, e.g.:This change brings equivalent functionality to GitPython's Python API.
Usage
Test plan
test_create_from_tree_with_trailers_dict- verifies dict-based trailer creation and round-trip parsingtest_create_from_tree_with_trailers_list- verifies list-of-tuples trailer creation with duplicate keystest_index_commit_with_trailers- verifiesIndexFile.commit()passes trailers through correctly