diff --git a/.github/workflows/promote-baseline.yml b/.github/workflows/promote-baseline.yml index 476d81f4e..06e229149 100644 --- a/.github/workflows/promote-baseline.yml +++ b/.github/workflows/promote-baseline.yml @@ -340,8 +340,8 @@ jobs: set -euo pipefail # The PR head's objects, to read its tree: fetched, never checked out or run. git fetch --quiet origin "$BASE_REF" "+refs/pull/$PR_NUMBER/head:refs/remotes/origin/pr-head" - if ! git merge-base --is-ancestor "$MERGE_SHA" "origin/$BASE_REF"; then - echo "::notice::merge commit $MERGE_SHA is not on origin/$BASE_REF; skipping" + if ! git merge-base --is-ancestor "$MERGE_SHA" "refs/remotes/origin/$BASE_REF"; then + echo "::notice::merge commit $MERGE_SHA is not on refs/remotes/origin/$BASE_REF; skipping" echo "ok=false" >> "$GITHUB_OUTPUT"; exit 0 fi merge_tree=$(git rev-parse "$MERGE_SHA^{tree}") diff --git a/scripts/check_doc_reverts.py b/scripts/check_doc_reverts.py index c8886ef51..0b13a7cd7 100644 --- a/scripts/check_doc_reverts.py +++ b/scripts/check_doc_reverts.py @@ -52,16 +52,16 @@ Without ``--head`` the gate walks the non-merge commits of ``base..HEAD`` one by one, each against its parent and with its own message. ``base`` is ``--base``, else ``$PINEFORGE_DOC_REVERT_BASE``, else the merge base with -``origin/main`` or ``main``, else -- in a checkout with no branch refs, as the -lab's remote hosts have -- the newest first-parent ancestor of ``HEAD`` whose -subject ends in a pull request number, `` (#N)``: the squash or merge commit -GitHub wrote on ``main``, which is the merge base CI would find (its branch's -commits were judged one by one, with their own messages, before it merged; a -squash commit's message is the pull request's title and cannot name them -again). Where none resolves, the walk starts at ``HEAD`` and stops at the first -commit that predates this gate. A commit whose own tree -has no ``scripts/check_doc_reverts.py`` is never judged: the rule binds from -the commit that introduced it on. At most ``MAX_COMMITS`` commits are walked. +``refs/remotes/origin/main`` or ``refs/heads/main``, else -- in a checkout with +no branch refs, as the lab's remote hosts have -- the newest first-parent +ancestor of ``HEAD`` whose subject ends in a pull request number, `` (#N)``: +the squash or merge commit GitHub wrote on ``main``, which is the merge base +CI would find (its branch's commits were judged one by one, with their own +messages, before it merged; a squash commit's message is the pull request's +title and cannot name them again). Where none resolves, the walk starts at +``HEAD`` and stops at the first commit that predates this gate. A commit whose +own tree has no ``scripts/check_doc_reverts.py`` is never judged: the rule binds +from the commit that introduced it on. At most ``MAX_COMMITS`` commits are walked. Exit 0 when every revert and deletion is named, 1 when one is not, 2 when git or a named ref cannot be read. @@ -352,7 +352,7 @@ def default_base(root: Path) -> str | None: env = os.environ.get('PINEFORGE_DOC_REVERT_BASE') if env: return resolve(env, root) - for ref in ('origin/main', 'main'): + for ref in ('refs/remotes/origin/main', 'refs/heads/main'): tip = git('rev-parse', '--verify', '--quiet', ref + '^{commit}', root=root, check=False).strip() if tip: diff --git a/scripts/ci_preflight.py b/scripts/ci_preflight.py index 9f3e43c71..26d6e7ff2 100644 --- a/scripts/ci_preflight.py +++ b/scripts/ci_preflight.py @@ -1884,7 +1884,7 @@ def ci_workflow_findings(ci: str, native: str, promote: str, cmake: str, or 'BASE_REF: main' not in exacttree or 'git fetch --quiet origin "$BASE_REF" "+refs/pull/$PR_NUMBER/head:refs/remotes/origin/pr-head"' not in exacttree - or 'git merge-base --is-ancestor "$MERGE_SHA" "origin/$BASE_REF"' not in exacttree + or 'git merge-base --is-ancestor "$MERGE_SHA" "refs/remotes/origin/$BASE_REF"' not in exacttree or '--is-ancestor "$HEAD_SHA"' in promote or 'merge_tree=$(git rev-parse "$MERGE_SHA^{tree}")' not in exacttree or 'head_tree=$(git rev-parse "$HEAD_SHA^{tree}")' not in exacttree diff --git a/scripts/test_check_doc_reverts.py b/scripts/test_check_doc_reverts.py index f6536c727..3b2adc1f6 100644 --- a/scripts/test_check_doc_reverts.py +++ b/scripts/test_check_doc_reverts.py @@ -18,11 +18,13 @@ import contextlib import io +import os from pathlib import Path import shutil import subprocess import tempfile import unittest +from unittest import mock import check_doc_reverts as guard @@ -91,6 +93,73 @@ def with_keep_handle(page: str = BASE_PAGE) -> str: return page + '\n' + KEEP_HANDLE + '\n' +class DefaultBase(unittest.TestCase): + def setUp(self) -> None: + env = mock.patch.dict(os.environ, {'PINEFORGE_DOC_REVERT_BASE': ''}) + env.start() + self.addCleanup(env.stop) + + def test_tags_cannot_shadow_branch_refs(self) -> None: + for ref, tag in (('refs/remotes/origin/main', 'origin/main'), + ('refs/heads/main', 'main')): + with self.subTest(ref=ref), repo() as r: + common = r.commit(BASE_PAGE, 'docs: common ancestor') + r.git('checkout', '-q', '-b', 'tag-history') + tag_tip = r.commit(with_keep_handle(), 'docs: unrelated tag history') + r.git('tag', tag, tag_tip) + r.git('checkout', '-q', 'lane') + base = r.commit(BASE_PAGE + '\nThe main branch adds its own sentence.\n', + 'docs: main branch') + r.git('update-ref', ref, base) + r.commit(with_keep_handle(), 'docs: lane after main') + self.assertEqual(r.git('merge-base', 'HEAD', f'refs/tags/{tag}'), common) + self.assertEqual(r.git('merge-base', 'HEAD', ref), base) + self.assertNotEqual(common, base) + self.assertEqual(guard.default_base(r.root), base) + + def test_remote_tracking_main_precedes_local_main(self) -> None: + with repo() as r: + remote = r.commit(BASE_PAGE, 'docs: remote main') + r.git('update-ref', 'refs/remotes/origin/main', remote) + local = r.commit(with_keep_handle(), 'docs: local main') + r.git('update-ref', 'refs/heads/main', local) + r.commit(BASE_PAGE + '\nThe lane adds a new sentence after local main.\n', + 'docs: lane') + self.assertEqual(guard.default_base(r.root), remote) + + def test_local_main_is_used_without_remote_tracking_main(self) -> None: + with repo() as r: + base = r.commit(BASE_PAGE, 'docs: local main') + r.git('update-ref', 'refs/heads/main', base) + r.commit(with_keep_handle(), 'docs: lane') + self.assertEqual(guard.default_base(r.root), base) + + def test_environment_base_precedes_branch_refs(self) -> None: + with repo() as r: + override = r.commit(BASE_PAGE, 'docs: explicit base') + remote = r.commit(with_keep_handle(), 'docs: remote main') + r.git('update-ref', 'refs/remotes/origin/main', remote) + local = r.commit(BASE_PAGE + '\nLocal main adds another sentence.\n', + 'docs: local main') + r.git('update-ref', 'refs/heads/main', local) + r.commit(with_keep_handle(), 'docs: lane') + with mock.patch.dict(os.environ, {'PINEFORGE_DOC_REVERT_BASE': override}): + self.assertEqual(guard.default_base(r.root), override) + + def test_merged_pull_request_is_used_without_branch_refs(self) -> None: + with repo() as r: + r.commit(BASE_PAGE, 'docs: initial guide') + base = r.commit(with_keep_handle(), 'docs: merged guide (#12)') + r.commit(BASE_PAGE + '\nThe lane adds a new sentence after the merge.\n', + 'docs: lane') + self.assertEqual(guard.default_base(r.root), base) + + def test_no_branch_ref_or_merged_pull_request_has_no_base(self) -> None: + with repo() as r: + r.commit(BASE_PAGE, 'docs: initial guide') + self.assertIsNone(guard.default_base(r.root)) + + class MustFail(unittest.TestCase): def setup_history(self, r: Repo) -> tuple[str, str]: base = r.commit(BASE_PAGE, 'docs: the guide') diff --git a/scripts/test_ci_preflight.py b/scripts/test_ci_preflight.py index 220b7328a..1729b1523 100644 --- a/scripts/test_ci_preflight.py +++ b/scripts/test_ci_preflight.py @@ -868,6 +868,27 @@ def test_ci_contract_pins_parallel_pr_light_and_full_events(self): self.assertNotEqual(changed[index], original[index]) self.assertNotEqual(ci_workflow_findings(*changed), []) + def test_baseline_promotion_accepts_qualified_base_ref(self): + sources = self.ci_sources() + short = 'git merge-base --is-ancestor "$MERGE_SHA" "origin/$BASE_REF"' + qualified = ('git merge-base --is-ancestor "$MERGE_SHA" ' + '"refs/remotes/origin/$BASE_REF"') + # Supply the same qualified input before and after the workflow fix. + sources[2] = sources[2].replace(short, qualified) + self.assertEqual(sources[2].count(qualified), 1) + self.assertEqual(ci_workflow_findings(*sources, others=self.other_sources()), []) + + def test_baseline_promotion_rejects_short_base_ref(self): + sources = self.ci_sources() + short = 'git merge-base --is-ancestor "$MERGE_SHA" "origin/$BASE_REF"' + qualified = ('git merge-base --is-ancestor "$MERGE_SHA" ' + '"refs/remotes/origin/$BASE_REF"') + # Supply the old command even when the workflow already uses the fix. + sources[2] = sources[2].replace(qualified, short) + self.assertEqual(sources[2].count(short), 1) + self.assertIn('baseline promotion must admit a squash merge by tree equality only', + ci_workflow_findings(*sources, others=self.other_sources())) + def test_baseline_promotion_rejects_later_pr_head_checkout(self): finding = ("baseline promotion must run main's copy and require " "the latest two head statuses")