From 273a63d9d78724a173658f08aadc78a8061ffb03 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Wed, 5 Aug 2026 13:16:25 +0100 Subject: [PATCH 1/2] fix(ci): apply DCO enforcement baseline Signed-off-by: Tanvir Farhad --- .github/workflows/dco.yml | 5 ++++- docs/dco.md | 7 +++++++ scripts/check_dco.py | 18 +++++++++------- tests/test_dco_check.py | 44 +++++++++++++++++++++++++++++++++++++-- 4 files changed, 64 insertions(+), 10 deletions(-) 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..1448d4b 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 are exempt, so release pull requests do not +retroactively reject project history 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..fc10e17 100644 --- a/scripts/check_dco.py +++ b/scripts/check_dco.py @@ -14,16 +14,20 @@ def has_signoff(message: str) -> bool: return SIGNOFF.search(message) is not None -def commits_between(base: str, head: str) -> list[str]: +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`` are exempt because they existed before + DCO enforcement. Subtracting the baseline's ancestry, instead of filtering + by date, still checks new work committed on a branch created before DCO. + 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] @@ -43,14 +47,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..aeda1d3 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 commits_between, has_signoff, main, unsigned_commits def test_has_signoff_accepts_standard_dco_trailer(): @@ -24,13 +25,52 @@ 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") + 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 _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) + assert commits_between(baseline, new_commit, baseline) == [new_commit] From 619aa33e25445fd193e473ab6a8387d63e84d618 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Wed, 5 Aug 2026 13:50:43 +0100 Subject: [PATCH 2/2] fix(ci): grandfather legacy branch commits Signed-off-by: Tanvir Farhad --- docs/dco.md | 8 ++++---- scripts/check_dco.py | 20 ++++++++++++++++---- tests/test_dco_check.py | 29 ++++++++++++++++++++++++++--- 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/docs/dco.md b/docs/dco.md index 1448d4b..e2a8068 100644 --- a/docs/dco.md +++ b/docs/dco.md @@ -19,10 +19,10 @@ insufficient. DCO enforcement began with commit `1d9469e162fce788bca839cfaf8a3e66ca35cf9b` (PR #208). Commits already -reachable from that baseline are exempt, so release pull requests do not -retroactively reject project history created before the policy. New commits -remain subject to DCO even when they are added to a branch created before the -baseline. +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 diff --git a/scripts/check_dco.py b/scripts/check_dco.py index fc10e17..e77162d 100644 --- a/scripts/check_dco.py +++ b/scripts/check_dco.py @@ -14,12 +14,23 @@ def has_signoff(message: str) -> bool: return SIGNOFF.search(message) is not None +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`` are exempt because they existed before - DCO enforcement. Subtracting the baseline's ancestry, instead of filtering - by date, still checks new work committed on a branch created before DCO. + 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 @@ -30,7 +41,8 @@ def commits_between(base: str, head: str, baseline: str) -> list[str]: ["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: diff --git a/tests/test_dco_check.py b/tests/test_dco_check.py index aeda1d3..2c4745f 100644 --- a/tests/test_dco_check.py +++ b/tests/test_dco_check.py @@ -3,7 +3,7 @@ import subprocess from unittest.mock import patch -from scripts.check_dco import commits_between, has_signoff, main, unsigned_commits +from scripts.check_dco import commit_timestamp, commits_between, has_signoff, main, unsigned_commits def test_has_signoff_accepts_standard_dco_trailer(): @@ -37,7 +37,11 @@ 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: + 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"] @@ -46,6 +50,21 @@ def test_commits_between_excludes_merge_commits(): 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], @@ -73,4 +92,8 @@ def test_baseline_exempts_history_but_checks_new_work_from_old_branch(tmp_path, new_commit = _commit(repository, "new unsigned work") monkeypatch.chdir(repository) - assert commits_between(baseline, new_commit, baseline) == [new_commit] + with patch( + "scripts.check_dco.commit_timestamp", + side_effect={baseline: 100, new_commit: 101}.get, + ): + assert commits_between(baseline, new_commit, baseline) == [new_commit]