From 57ac252482bfe7ccff8c2c10fefa0fd90a57f385 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:04:00 +0200 Subject: [PATCH] feat: check the profiles a branch adds or edits profile_sync gains --changed (added or edited profiles, uncommitted ones included) and --added, both against the merge base of --base (origin/main by default). CONTRIBUTING runs --changed --strict instead of one --emulator call per new profile, and the CI job runs --added in place of its own diff loop. --- .github/workflows/validate.yml | 9 +---- CONTRIBUTING.md | 2 +- scripts/generate_readme.py | 2 +- scripts/profile_sync.py | 57 ++++++++++++++++++++++++++ tests/test_audit_regressions.py | 2 +- tests/test_profile_sync.py | 72 ++++++++++++++++++++++++++++++++- wiki/release-process.md | 7 ++-- wiki/tools.md | 7 +++- 8 files changed, 141 insertions(+), 17 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index c10821e1..80572b13 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -186,14 +186,7 @@ jobs: else base=$BEFORE fi - status=0 - for file in $(git diff --name-only --diff-filter=A "$base" HEAD -- 'emulators/*.yml'); do - name=$(basename "$file" .yml) - echo "::group::$name" - python scripts/profile_sync.py --emulator "$name" --strict || status=1 - echo "::endgroup::" - done - exit $status + python scripts/profile_sync.py --added --base "$base" --strict run-tests: runs-on: ubuntu-latest diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index df37fb8c..fa5b7e12 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -49,7 +49,7 @@ The checks CI runs on a pull request (Python 3.12 or newer): pip install pyyaml "jsonschema[format-nongpl]" markdown python scripts/validate_pr.py --changed # the BIOS files the branch adds python scripts/validate_schemas.py --source-only # profiles and platform files -python scripts/profile_sync.py --emulator NAME --strict # each profile the branch adds +python scripts/profile_sync.py --changed --strict # each profile the branch adds or edits python scripts/refresh_data_dirs.py # data directories the pack-building tests read python -m unittest discover -s tests -t . ``` diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index f1254e51..d901fc18 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -757,7 +757,7 @@ The checks CI runs on a pull request (Python 3.12 or newer): pip install pyyaml "jsonschema[format-nongpl]" markdown python scripts/validate_pr.py --changed # the BIOS files the branch adds python scripts/validate_schemas.py --source-only # profiles and platform files -python scripts/profile_sync.py --emulator NAME --strict # each profile the branch adds +python scripts/profile_sync.py --changed --strict # each profile the branch adds or edits python scripts/refresh_data_dirs.py # data directories the pack-building tests read python -m unittest discover -s tests -t . ``` diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 4d6c929f..603f4303 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -3367,6 +3367,20 @@ def build_parser() -> argparse.ArgumentParser: group = parser.add_mutually_exclusive_group(required=True) group.add_argument("--emulator", help="a single profile") group.add_argument("--all", action="store_true", help="every profile") + group.add_argument( + "--changed", action="store_true", + help="the profiles the branch adds or edits against --base, " + "uncommitted ones included", + ) + group.add_argument( + "--added", action="store_true", + help="the profiles the branch adds against --base, as CI checks them", + ) + parser.add_argument( + "--base", default="origin/main", + help="revision --changed and --added compare with, through its merge " + "base with HEAD (default: %(default)s)", + ) parser.add_argument( "--emulators-dir", default="emulators", help="emulator profile directory (default: %(default)s)", @@ -3481,8 +3495,51 @@ def build_parser() -> argparse.ArgumentParser: return parser +def _git_lines(args: list[str]) -> list[str]: + result = subprocess.run( + ["git", *args], capture_output=True, text=True, check=False + ) + if result.returncode: + detail = result.stderr.strip() or f"exit {result.returncode}" + raise SystemExit(f"ERROR: git {' '.join(args)}: {detail}") + return [line for line in result.stdout.splitlines() if line] + + +def branch_profiles(emulators_dir: str, base: str, added_only: bool) -> list[str]: + """Names of the profiles the branch adds (or edits) against ``base``. + + The diff starts at the merge base, so commits main gained since the + branch point are not counted, and ends at the working tree, so a + profile not yet committed is checked too. A rename reads as an addition. + """ + merge_base = _git_lines(["merge-base", base, "HEAD"])[0] + paths = _git_lines([ + "diff", "--name-only", "--no-renames", + "--diff-filter=A" if added_only else "--diff-filter=AM", + merge_base, "--", emulators_dir, + ]) + paths += _git_lines([ + "ls-files", "--others", "--exclude-standard", "--full-name", + "--", emulators_dir, + ]) + top = Path(_git_lines(["rev-parse", "--show-toplevel"])[0]) + root = Path(emulators_dir).resolve() + return sorted({ + full.stem + for path in paths + if (full := (top / path).resolve()).suffix == ".yml" and full.parent == root + }) + + def select_profiles(profiles: dict, args: argparse.Namespace) -> dict: """Profiles matching the selection flags.""" + if args.changed or args.added: + names = branch_profiles(args.emulators_dir, args.base, args.added) + selected = {name: profiles[name] for name in names if name in profiles} + if not selected: + verb = "adds" if args.added else "adds or edits" + print(f"the branch {verb} no profile against {args.base}", file=sys.stderr) + return selected if args.emulator: if args.emulator not in profiles: # Suggested from what the listing it names shows: counted over the diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 0c392b3a..19cfac28 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -276,7 +276,7 @@ class WorkflowRegressions(unittest.TestCase): self.assertTrue(runs, "no job confronts profiles with their upstream") for name, body in runs: self.assertIn("--strict", body) - self.assertIn("--diff-filter=A", body) + self.assertIn("--added", body) self.assertIsNone(workflow["jobs"][name].get("if"), f"{name} must run on push too") def test_both_events_watch_the_same_paths(self): diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 9b723d35..ffe4d85c 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -4261,8 +4261,9 @@ class TestSelectionNamesWhatMatchedNothing(unittest.TestCase): } def _args(self, **values): - base = {"emulator": None, "system": None, "type_": None, - "stale_before": None, "limit": None} + base = {"emulator": None, "changed": False, "added": False, + "system": None, "type_": None, "stale_before": None, + "limit": None} return argparse.Namespace(**{**base, **values}) def test_a_misspelt_system_stops_and_suggests(self): @@ -4610,5 +4611,72 @@ class TheTriageCountsWhatBlocksThePin(unittest.TestCase): self.assertEqual(len(err.getvalue().splitlines()), 1, err.getvalue()) +class TestBranchProfiles(unittest.TestCase): + """--changed and --added name the profiles a branch brings, not by hand.""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.root = Path(self._tmp.name) + self._cwd = os.getcwd() + self.addCleanup(os.chdir, self._cwd) + os.chdir(self.root) + env = {**os.environ, "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t"} + self.env = env + self._git("init", "-q", "-b", "main") + (self.root / "emulators").mkdir() + (self.root / "emulators" / "kept.yml").write_text("a: 1\n") + (self.root / "emulators" / "edited.yml").write_text("a: 1\n") + self._git("add", "emulators") + self._git("commit", "-q", "--no-verify", "-m", "base") + self._git("checkout", "-q", "-b", "topic") + (self.root / "emulators" / "edited.yml").write_text("a: 2\n") + (self.root / "emulators" / "new.yml").write_text("a: 1\n") + self._git("add", "emulators") + self._git("commit", "-q", "--no-verify", "-m", "topic") + (self.root / "emulators" / "draft.yml").write_text("a: 1\n") + (self.root / "emulators" / "notes.txt").write_text("x\n") + + def _git(self, *args): + subprocess.run(["git", *args], cwd=self.root, env=self.env, check=True) + + def test_changed_takes_added_edited_and_uncommitted(self): + self.assertEqual( + profile_sync.branch_profiles("emulators", "main", added_only=False), + ["draft", "edited", "new"], + ) + + def test_added_leaves_edited_profiles_out(self): + self.assertEqual( + profile_sync.branch_profiles("emulators", "main", added_only=True), + ["draft", "new"], + ) + + def test_the_flag_selects_those_profiles(self): + args = profile_sync.build_parser().parse_args(["--changed", "--base", "main"]) + profiles = {"kept": {}, "edited": {}, "new": {}, "draft": {}} + self.assertEqual( + list(profile_sync.select_profiles(profiles, args)), + ["draft", "edited", "new"], + ) + + +class TestBranchCheckIsWired(unittest.TestCase): + """CONTRIBUTING and CI name the branch's profiles through the flag.""" + + def test_contributing_and_ci_use_the_flags(self): + root = Path(__file__).resolve().parents[1] + sys.path.insert(0, str(root / "scripts")) + import generate_readme # noqa: PLC0415 + + text = generate_readme.generate_contributing() + self.assertIn("profile_sync.py --changed --strict", text) + self.assertNotIn("--emulator NAME --strict", text) + workflow = (root / ".github" / "workflows" / "validate.yml").read_text() + self.assertIn("profile_sync.py --added --base", workflow) + self.assertNotIn('--emulator "$name"', workflow) + + if __name__ == "__main__": unittest.main() diff --git a/wiki/release-process.md b/wiki/release-process.md index 5151da20..864fb59c 100644 --- a/wiki/release-process.md +++ b/wiki/release-process.md @@ -103,9 +103,10 @@ every emulator profile against `schemas/emulator.schema.json`. Both schemas set `additionalProperties: false`, so a typo in a field name fails the job instead of being silently ignored. -**check-new-profiles.** Runs `profile_sync.py --emulator NAME --strict` on -each profile the change adds (the files `git diff --diff-filter=A` names -against the pull request base, or against the commit a push moved main from). +**check-new-profiles.** Runs `profile_sync.py --added --base BASE --strict`, +which checks each profile the change adds (the files `git diff --diff-filter=A` +names against the pull request base, or against the commit a push moved main +from). A profile written from a clone can cite lines that do not hold the value it declares while schemas and tests pass it. This job confronts it with its upstream. An edited profile is not held to it. diff --git a/wiki/tools.md b/wiki/tools.md index e29832d9..ef7244dc 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -401,13 +401,18 @@ and `--limit 20`. Output takes `--markdown` (a report written under `--report-dir`, `reports/` by default), and `--full-diff` takes `--ref PATH` to show one cited file and `--context N` lines around each hunk. `--strict` exits 1 when a profile has refs to review, and CI runs it on every profile a change -adds. `--offline` answers from `--cache-dir` alone. +adds. `--offline` answers from `--cache-dir` alone. `--changed` selects the +profiles the branch adds or edits, uncommitted ones included, and `--added` +the ones it adds, both against the merge base of `--base` (`origin/main` by +default) and HEAD. ```bash python scripts/profile_sync.py --all --system sega-saturn --changed-only python scripts/profile_sync.py --all --stale-before 2026-06-01 --limit 20 --triage python scripts/profile_sync.py --emulator vice --full-diff --ref vice/src/c64/c64rom.h --context 6 python scripts/profile_sync.py --emulator vice --strict +python scripts/profile_sync.py --changed --strict +python scripts/profile_sync.py --added --base upstream/main --strict ``` Per part of a ref: `ANCHORED` (same content, same lines), `SHIFTED` (same