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
29 changes: 27 additions & 2 deletions .github/scripts/release_attribution.py
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,27 @@ def unresolved_coauthor_identities(
return sorted(unresolved.values(), key=lambda item: (item["email"], item["sha"]))


def is_clean_main_sync(github, repository, sha, parents, main_sha) -> bool:
if len(parents) != 2 or not main_sha:
return False
try:
ancestry = github.get(f"repos/{repository}/compare/{parents[1]}...{main_sha}")
if ancestry.get("status") not in {"ahead", "identical"}:
return False
changed_paths = []
for parent in parents:
files = github.get(f"repos/{repository}/compare/{parent}...{sha}").get("files")
if files is None or len(files) >= 300:
return False
changed_paths.append(
{file.get(key) for file in files for key in ("filename", "previous_filename")}
- {None}
)
return changed_paths[0].isdisjoint(changed_paths[1])
except (ReleaseError, OSError, ValueError, KeyError, TypeError):
return False


def validate_pr_attribution(
*,
repository: str,
Expand All @@ -527,6 +548,7 @@ def validate_pr_attribution(
base_config: Mapping[str, Any],
identity_changes: Mapping[str, str | None],
github: GitHubClient,
main_sha: str = "",
) -> None:
"""Fail closed when preserved human authorship cannot reach a GitHub login.

Expand Down Expand Up @@ -624,9 +646,11 @@ def record_unresolved(
#
# A base synchronization merge imports a parent outside the PR commit set,
# so it is not salvaged contributor work. Its distinct author must still
# have explicit coauthor credit elsewhere in the PR.
# have explicit coauthor credit elsewhere in the PR unless it is a clean main sync.
if is_base_sync_merge and distinct_external_author:
if author_login.lower() not in credited_coauthors:
if author_login.lower() not in credited_coauthors and not is_clean_main_sync(
github, repository, sha, parents, main_sha
):
trailer = f"Co-authored-by: {author_name} <{author_email}>"
early_errors.append(
f"base synchronization merge {sha} is authored by @{author_login}, "
Expand Down Expand Up @@ -1467,6 +1491,7 @@ def validate_pr_command(args: argparse.Namespace) -> None:
base_config=base_config,
identity_changes=identity_changes,
github=client,
main_sha=trusted_sha,
)
print(f"contributor attribution is merge-ready for pull request #{number}")

Expand Down
79 changes: 77 additions & 2 deletions .github/scripts/tests/test_pr_attribution.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,16 @@ def commit(


class SourceGitHub:
def __init__(self, authors, source_emails):
def __init__(self, authors, source_emails, comparisons=None):
self.authors = authors
self.source_emails = source_emails
self.comparisons = comparisons or {}

def get(self, path):
value = self.comparisons[path]
if isinstance(value, Exception):
raise value
return value

def pull(self, repository: str, number: int):
assert repository == "trycua/cua"
Expand All @@ -81,6 +88,8 @@ def validate(
changes=None,
authors=None,
source_emails=None,
comparisons=None,
main_sha="",
):
base = base or config()
validate_pr_attribution(
Expand All @@ -89,7 +98,8 @@ def validate(
commits=commits or [],
base_config=base,
identity_changes=changes or {},
github=SourceGitHub(authors or {}, source_emails or {}),
github=SourceGitHub(authors or {}, source_emails or {}, comparisons),
main_sha=main_sha,
)


Expand Down Expand Up @@ -456,3 +466,68 @@ def test_compare_pages_that_stop_short_are_refused():
client = PagedCompareGitHub(total=250, reported_total=300)
with pytest.raises(release_attribution.ReleaseError, match="refusing partial"):
client.all_pull_commits("trycua/cua-staging", large_pull())


@pytest.mark.parametrize(
"status, branch_files, main_files, failure, clean",
[
("ahead", ["main-change"], ["branch-change"], False, True),
("identical", ["main-change"], ["branch-change"], False, True),
("ahead", ["conflict"], ["conflict"], False, False),
("ahead", [{"filename": "q", "previous_filename": "p"}], ["p"], False, False),
("diverged", ["main-change"], ["branch-change"], False, False),
("ahead", [str(i) for i in range(300)], ["branch-change"], False, False),
("ahead", ["main-change"], [str(i) for i in range(300)], False, False),
("ahead", ["main-change"], ["branch-change"], True, False),
(None, ["main-change"], ["branch-change"], False, False),
("ahead", ["main-change"], ["branch-change"], "missing-files", False),
],
)
def test_main_sync_merge_authorship(status, branch_files, main_files, failure, clean):
comparisons = {
"repos/trycua/cua/compare/main-tip...trusted-main": {"status": status} if status else {},
"repos/trycua/cua/compare/feature-tip...sync-merge": {
"files": [
name if isinstance(name, dict) else {"filename": name} for name in branch_files
]
},
"repos/trycua/cua/compare/main-tip...sync-merge": (
ReleaseError("API failed")
if failure is True
else (
{}
if failure == "missing-files"
else {"files": [{"filename": name} for name in main_files]}
)
),
}
values = dict(
commits=[
commit(sha="feature-tip", login="landing-author"),
commit(
sha="sync-merge",
login="injaneity",
email="44902825+injaneity@users.noreply.github.com",
parent_shas=("feature-tip", "main-tip"),
),
],
comparisons=comparisons,
main_sha="trusted-main",
)
if clean:
validate(**values)
unlinked = commit(
sha="sync-merge",
email="unlinked@institution.example",
parent_shas=("feature-tip", "main-tip"),
)
with pytest.raises(ReleaseError, match="unlinked commit author"):
validate(**{**values, "commits": [values["commits"][0], unlinked]})
values["commits"][1]["commit"][
"message"
] += "\n\nCo-authored-by: Pair <pair@institution.example>"
with pytest.raises(ReleaseError, match="has no explicit same-repository source PR"):
validate(**values)
else:
with pytest.raises(ReleaseError, match="base synchronization merge"):
validate(**values)
Loading