diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml index ac25e31..719600a 100644 --- a/.github/workflows/dco.yml +++ b/.github/workflows/dco.yml @@ -21,4 +21,7 @@ jobs: env: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} - run: python scripts/check_dco.py "$BASE_SHA" "$HEAD_SHA" + # DCO was introduced on dev by PR #208. Commits already reachable + # from this baseline predate enforcement and remain exempt. + DCO_BASELINE_SHA: 1d9469e162fce788bca839cfaf8a3e66ca35cf9b + run: python scripts/check_dco.py "$BASE_SHA" "$HEAD_SHA" "$DCO_BASELINE_SHA" diff --git a/docs/dco.md b/docs/dco.md index 9876244..e2a8068 100644 --- a/docs/dco.md +++ b/docs/dco.md @@ -17,6 +17,13 @@ different authorized signer. Every non-merge commit introduced by a pull request is checked; a sign-off only in the pull request description is insufficient. +DCO enforcement began with commit +`1d9469e162fce788bca839cfaf8a3e66ca35cf9b` (PR #208). Commits already +reachable from that baseline—or recorded before it—are exempt, so release pull +requests and still-open legacy branches do not retroactively reject work created +before the policy. New commits remain subject to DCO even when they are added to +a branch created before the baseline. + Merge commits (e.g. from running `git merge origin/dev` to bring your branch up to date) are exempt — they carry Git's own default message, not your authorship, so there is nothing for you to sign off on. Only commits you diff --git a/scripts/check_dco.py b/scripts/check_dco.py index 1e5eb8a..e77162d 100644 --- a/scripts/check_dco.py +++ b/scripts/check_dco.py @@ -14,19 +14,35 @@ def has_signoff(message: str) -> bool: return SIGNOFF.search(message) is not None -def commits_between(base: str, head: str) -> list[str]: +def commit_timestamp(commit: str) -> int: + """Return a commit's recorded committer timestamp.""" + return int( + subprocess.check_output( + ["git", "show", "--no-patch", "--format=%ct", commit], + text=True, + ).strip() + ) + + +def commits_between(base: str, head: str, baseline: str) -> list[str]: """Return commits introduced between the pull request base and head. + Commits reachable from ``baseline`` and commits recorded before that policy + commit are exempt because they existed before DCO enforcement. The timestamp + check covers still-open legacy branches whose commits are not part of the + baseline's ancestry, while newer work on those branches remains enforced. + Excludes merge commits: a `git merge origin/dev` inside a long-running PR branch produces a commit with Git's default merge message and no Signed-off-by trailer, through no fault of the author's own commits. GitHub's own DCO app skips merge commits for the same reason. """ output = subprocess.check_output( - ["git", "rev-list", "--reverse", "--no-merges", f"{base}..{head}"], + ["git", "rev-list", "--reverse", "--no-merges", f"{base}..{head}", f"^{baseline}"], text=True, ) - return [item for item in output.splitlines() if item] + baseline_timestamp = commit_timestamp(baseline) + return [item for item in output.splitlines() if item and commit_timestamp(item) > baseline_timestamp] def commit_message(commit: str) -> str: @@ -43,14 +59,14 @@ def unsigned_commits(commits: Iterable[str]) -> list[str]: def main() -> int: - if len(sys.argv) != 3: - print("Usage: check_dco.py ", file=sys.stderr) + if len(sys.argv) != 4: + print("Usage: check_dco.py ", file=sys.stderr) return 2 - commits = commits_between(sys.argv[1], sys.argv[2]) + commits = commits_between(sys.argv[1], sys.argv[2], sys.argv[3]) if not commits: - print("No pull request commits found.", file=sys.stderr) - return 1 + print("No DCO-eligible pull request commits found.") + return 0 missing = unsigned_commits(commits) if missing: diff --git a/tests/test_dco_check.py b/tests/test_dco_check.py index c062c29..2c4745f 100644 --- a/tests/test_dco_check.py +++ b/tests/test_dco_check.py @@ -1,8 +1,9 @@ """Tests for the dependency-free DCO enforcement helper.""" +import subprocess from unittest.mock import patch -from scripts.check_dco import commits_between, has_signoff, unsigned_commits +from scripts.check_dco import commit_timestamp, commits_between, has_signoff, main, unsigned_commits def test_has_signoff_accepts_standard_dco_trailer(): @@ -24,13 +25,75 @@ def test_unsigned_commits_checks_each_commit(): assert unsigned_commits(["a", "b"]) == ["b"] +def test_main_passes_when_every_commit_is_exempt(): + with ( + patch("scripts.check_dco.sys.argv", ["check_dco.py", "base", "head", "baseline"]), + patch("scripts.check_dco.commits_between", return_value=[]), + ): + assert main() == 0 + + def test_commits_between_excludes_merge_commits(): """A `git merge origin/dev` inside a PR branch has no Signed-off-by trailer and isn't the author's own commit — it must never be checked, or a legitimate PR gets blocked for merging the base branch in.""" - with patch("scripts.check_dco.subprocess.check_output", return_value="abc123\ndef456\n") as mock_run: - result = commits_between("base-sha", "head-sha") + timestamps = {"baseline-sha": 100, "abc123": 101, "def456": 102} + with ( + patch("scripts.check_dco.subprocess.check_output", return_value="abc123\ndef456\n") as mock_run, + patch("scripts.check_dco.commit_timestamp", side_effect=timestamps.get), + ): + result = commits_between("base-sha", "head-sha", "baseline-sha") assert result == ["abc123", "def456"] called_args = mock_run.call_args.args[0] assert "--no-merges" in called_args + assert "^baseline-sha" in called_args + + +def test_commits_between_exempts_legacy_side_branch_but_checks_new_work(): + timestamps = {"baseline": 100, "legacy": 90, "new": 110} + with ( + patch("scripts.check_dco.subprocess.check_output", return_value="legacy\nnew\n"), + patch("scripts.check_dco.commit_timestamp", side_effect=timestamps.get), + ): + assert commits_between("base", "head", "baseline") == ["new"] + + +def test_commit_timestamp_reads_committer_epoch(): + with patch("scripts.check_dco.subprocess.check_output", return_value="1234567890\n") as run: + assert commit_timestamp("abc123") == 1234567890 + assert run.call_args.args[0] == ["git", "show", "--no-patch", "--format=%ct", "abc123"] + + +def _git(repository, *args): + return subprocess.check_output( + ["git", "-C", str(repository), *args], + text=True, + ).strip() + + +def _commit(repository, message): + (repository / "history.txt").write_text(message, encoding="utf-8") + _git(repository, "add", "history.txt") + _git(repository, "commit", "-m", message) + return _git(repository, "rev-parse", "HEAD") + + +def test_baseline_exempts_history_but_checks_new_work_from_old_branch(tmp_path, monkeypatch): + repository = tmp_path / "repository" + repository.mkdir() + _git(repository, "init") + _git(repository, "config", "user.name", "Test User") + _git(repository, "config", "user.email", "test@example.com") + + old_commit = _commit(repository, "old unsigned commit") + baseline = _commit(repository, "introduce DCO") + _git(repository, "switch", "-c", "old-feature", old_commit) + new_commit = _commit(repository, "new unsigned work") + + monkeypatch.chdir(repository) + with patch( + "scripts.check_dco.commit_timestamp", + side_effect={baseline: 100, new_commit: 101}.get, + ): + assert commits_between(baseline, new_commit, baseline) == [new_commit]