ci(credit): accept contributor authorship on the merged side of a harvest merge
A merge commit that lands a contributor's PR (`git merge --no-ff`) keeps the contributor as author of the merged commits, which is exactly the credit the gate asks for. When that merge also carries a `Harvested from PR #N by @login` line for the auto-close workflow, the checker now looks at the authors of `first-parent..merge` before demanding a duplicate Co-authored-by trailer on the merge commit itself. Non-merge harvested commits and merges that do not carry the contributor's commits are unchanged. Covered by a new unit test.
This commit is contained in:
@@ -206,6 +206,35 @@ def lookup_identity(aliases: dict[str, Identity], *values: str) -> Identity | No
|
||||
return None
|
||||
|
||||
|
||||
def merged_author_emails(commit: Commit) -> set[str]:
|
||||
"""Author emails of the commits a merge commit brings in.
|
||||
|
||||
A merge commit that lands a contributor's PR preserves the contributor as
|
||||
author of the merged commits themselves. When such a merge also carries a
|
||||
`Harvested from PR #N by @login` line (so the auto-close workflow credits
|
||||
the PR), the contributor's authorship on the second-parent side is the
|
||||
machine-readable credit; the merge commit does not need a duplicate
|
||||
trailer.
|
||||
"""
|
||||
parents = commit.parents.split()
|
||||
if len(parents) < 2:
|
||||
return set()
|
||||
try:
|
||||
raw = subprocess.check_output(
|
||||
[
|
||||
"git",
|
||||
"log",
|
||||
"--format=%ae",
|
||||
f"{parents[0]}..{commit.sha}",
|
||||
],
|
||||
cwd=ROOT,
|
||||
text=True,
|
||||
)
|
||||
except subprocess.CalledProcessError:
|
||||
return set()
|
||||
return {norm_key(line) for line in raw.splitlines() if line.strip()}
|
||||
|
||||
|
||||
def is_preserved_mapped_identity(commit: Commit, role: str, identity: Identity) -> bool:
|
||||
return (
|
||||
commit.sha.strip().lower(),
|
||||
@@ -285,6 +314,7 @@ def validate(commits: list[Commit], aliases: dict[str, Identity], check_authors:
|
||||
expected = lookup_identity(aliases, coauthor.email, coauthor.name)
|
||||
if expected and is_preserved_mapped_identity(commit, "coauthor", coauthor):
|
||||
coauthor_emails.add(norm_key(expected.email))
|
||||
merged_emails = merged_author_emails(commit) if harvested_logins else set()
|
||||
for login in harvested_logins:
|
||||
expected = lookup_identity(aliases, login)
|
||||
if expected is None:
|
||||
@@ -295,6 +325,7 @@ def validate(commits: list[Commit], aliases: dict[str, Identity], check_authors:
|
||||
if (
|
||||
norm_key(commit.author_email) != norm_key(expected.email)
|
||||
and norm_key(expected.email) not in coauthor_emails
|
||||
and norm_key(expected.email) not in merged_emails
|
||||
):
|
||||
errors.append(
|
||||
f"{prefix}: `Harvested from PR ... by @{login}` needs machine-readable "
|
||||
|
||||
@@ -6,6 +6,7 @@ from __future__ import annotations
|
||||
import importlib.util
|
||||
import sys
|
||||
import unittest
|
||||
import unittest.mock
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
@@ -54,6 +55,28 @@ class CheckCoauthorTrailersTests(unittest.TestCase):
|
||||
errors = mod.validate([commit("human credit", body)], self.aliases, False)
|
||||
self.assertEqual(errors, [])
|
||||
|
||||
def test_merge_of_contributor_pr_carries_harvest_credit_by_authorship(self) -> None:
|
||||
merge = mod.Commit(
|
||||
sha="feedface" * 5,
|
||||
parents="1111111111111111111111111111111111111111 2222222222222222222222222222222222222222",
|
||||
author_name="CodeWhale Bot",
|
||||
author_email="bot@codewhale.net",
|
||||
subject="Merge PR #5423: test(tui): isolate background verifier from rustup",
|
||||
body="Merge PR #5423\n\nHarvested from PR #5423 by @wuisabel-gif",
|
||||
)
|
||||
with unittest.mock.patch.object(
|
||||
mod,
|
||||
"merged_author_emails",
|
||||
return_value={"231155141+wuisabel-gif@users.noreply.github.com"},
|
||||
):
|
||||
errors = mod.validate([merge], self.aliases, False)
|
||||
self.assertEqual(errors, [])
|
||||
# Without the contributor on the merged side the same merge still needs a trailer.
|
||||
with unittest.mock.patch.object(mod, "merged_author_emails", return_value=set()):
|
||||
errors = mod.validate([merge], self.aliases, False)
|
||||
self.assertEqual(len(errors), 1)
|
||||
self.assertIn("needs machine-readable credit", errors[0])
|
||||
|
||||
def test_allows_merge_commit_with_bot_trailer(self) -> None:
|
||||
merge = mod.Commit(
|
||||
sha="cafebabe" * 5,
|
||||
|
||||
Reference in New Issue
Block a user