Skip to content

Commit 94e43cf

Browse files
fix(ci_visibility): use ||| as git show format separator when fetching user info [backport 2.5] (#8606)
Backport b887ed9 from #8559 to 2.5. Fixes an issue where git committer or author names containing a comma (eg: "Lastname, Firstname" instead of "Firstname Lastname") would cause the user info function to fail to unpack properly. This changes the separator for the git show command format to "|||" which should be far less common in committer or author names. ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. - [x] If change touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Romain Komorn <[email protected]>
1 parent db87313 commit 94e43cf

File tree

3 files changed

+19
-2
lines changed

3 files changed

+19
-2
lines changed

ddtrace/ext/git.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,10 @@ def extract_user_info(cwd=None):
179179
# type: (Optional[str]) -> Dict[str, Tuple[str, str, str]]
180180
"""Extract commit author info from the git repository in the current directory or one specified by ``cwd``."""
181181
# Note: `git show -s --format... --date...` is supported since git 2.1.4 onwards
182-
stdout = _git_subprocess_cmd("show -s --format=%an,%ae,%ad,%cn,%ce,%cd --date=format:%Y-%m-%dT%H:%M:%S%z", cwd=cwd)
183-
author_name, author_email, author_date, committer_name, committer_email, committer_date = stdout.split(",")
182+
stdout = _git_subprocess_cmd(
183+
"show -s --format=%an|||%ae|||%ad|||%cn|||%ce|||%cd --date=format:%Y-%m-%dT%H:%M:%S%z", cwd=cwd
184+
)
185+
author_name, author_email, author_date, committer_name, committer_email, committer_date = stdout.split("|||")
184186
return {
185187
"author": (author_name, author_email, author_date),
186188
"committer": (committer_name, committer_email, committer_date),
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
CI Visibility: fixes an issue where git author or committer names containing commas (eg: "Lastname, Firstname")
5+
would not work (and log an error) due to the use of comma as a separator.

tests/tracer/test_ci.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@ def test_git_extract_user_info(git_repo):
6060
assert extracted_users["committer"] == expected_committer
6161

6262

63+
def test_git_extract_user_info_with_commas():
64+
with mock.patch(
65+
"ddtrace.ext.git._git_subprocess_cmd",
66+
return_value="Do, Jo|||[email protected]|||2021-01-19T09:24:53-0400|||Do, Ja|||[email protected]|||2021-01-20T04:37:21-0400",
67+
):
68+
extracted_users = git.extract_user_info()
69+
assert extracted_users["author"] == ("Do, Jo", "[email protected]", "2021-01-19T09:24:53-0400")
70+
assert extracted_users["committer"] == ("Do, Ja", "[email protected]", "2021-01-20T04:37:21-0400")
71+
72+
6373
def test_git_extract_user_info_error(git_repo_empty):
6474
"""On error, the author/committer tags should not be extracted, and should internally raise an error."""
6575
with pytest.raises(ValueError):

0 commit comments

Comments
 (0)