mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-12 06:23:23 -05:00
fix: report the writes a dry run would make
--backfill-commits and --realign-prose have always printed what they would write. --rebase-refs and --bump-commit took the flag and printed nothing, so the only way to read a plan was to let it happen, and the recale and the pin had to be done in two full network passes with --force in between. Both now plan. The plan runs the production write path over a throwaway copy rather than a parallel branch, so it cannot drift from the write, and the planned bump reads the text the recale would have left: a pin held back by prose the same pass would move is no longer reported as blocked. One pass does both, recale before bump on each profile. bump_commit also stopped announcing a rewrite of the pin to the value it already held. On the corpus that was 126 of 232 announcements, which buried the 106 profiles that did move.
This commit is contained in:
1 parent
1a96853aee
commit
3891d897a6
3 files changed
+162
-9
No files matched your search
+24
-9
@@ -9,6 +9,7 @@ recalable from what needs the code read again.
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import contextlib
|
||||
import difflib
|
||||
import json
|
||||
import os
|
||||
@@ -16,6 +17,7 @@ import posixpath
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
from dataclasses import asdict, dataclass
|
||||
from datetime import date
|
||||
from pathlib import Path
|
||||
@@ -2175,6 +2177,10 @@ def bump_commit(
|
||||
if pending_recale(report, accept_changed, text):
|
||||
return False
|
||||
document = yaml.safe_load(text)
|
||||
if document.get("source_commit") == report.head:
|
||||
# Rewriting the pin to the value it already holds is not a bump, and
|
||||
# announcing it buries the profiles that did move.
|
||||
return False
|
||||
expected = dict(document)
|
||||
expected["source_commit"] = report.head
|
||||
if document.get("source_commit"):
|
||||
@@ -2525,15 +2531,24 @@ def _apply_writes(args, name: str, profile: dict, report: ProfileReport) -> None
|
||||
print(f"{name}: would write source_commit {report.pin[:7]}")
|
||||
elif backfill_commit(path, report.pin):
|
||||
print(f"{name}: source_commit {report.pin[:7]}")
|
||||
if args.rebase_refs and not args.dry_run and not report.skipped:
|
||||
for line in rebase_refs(path, report, args.accept_changed):
|
||||
print(f"{name}: {line}")
|
||||
if (
|
||||
args.bump_commit
|
||||
and not args.dry_run
|
||||
and bump_commit(path, report, args.accept_changed)
|
||||
):
|
||||
print(f"{name}: source_commit -> {report.head[:7]}")
|
||||
if not (args.rebase_refs or args.bump_commit):
|
||||
return
|
||||
with contextlib.ExitStack() as stack:
|
||||
if args.dry_run:
|
||||
# Plan on a copy through the production write path rather than a
|
||||
# parallel branch: the bump then reads the text the rebase would
|
||||
# have left, which is the only text under which it is reachable.
|
||||
scratch = stack.enter_context(tempfile.TemporaryDirectory())
|
||||
target = Path(scratch) / path.name
|
||||
target.write_bytes(path.read_bytes())
|
||||
recale, bumped = "would recale ", "would set source_commit ->"
|
||||
else:
|
||||
target, recale, bumped = path, "", "source_commit ->"
|
||||
if args.rebase_refs and not report.skipped:
|
||||
for line in rebase_refs(target, report, args.accept_changed):
|
||||
print(f"{name}: {recale}{line}")
|
||||
if args.bump_commit and bump_commit(target, report, args.accept_changed):
|
||||
print(f"{name}: {bumped} {report.head[:7]}")
|
||||
|
||||
|
||||
def _cited_paths(report: ProfileReport) -> list[str]:
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import contextlib
|
||||
import io
|
||||
import json
|
||||
@@ -1834,6 +1835,128 @@ class TestRebaseRefs(unittest.TestCase):
|
||||
self.assertIn('source_ref: "b.c:5"', text)
|
||||
|
||||
|
||||
class TestWriteDryRun(unittest.TestCase):
|
||||
"""--dry-run has to plan a write, not stay silent about it.
|
||||
|
||||
--backfill-commits and --realign-prose have always reported what they
|
||||
would write. --rebase-refs and --bump-commit accepted the flag and said
|
||||
nothing, so the only way to read the plan was to let it happen.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
self.tmp = tempfile.TemporaryDirectory()
|
||||
self.dir = Path(self.tmp.name)
|
||||
self.path = self.dir / "p.yml"
|
||||
self.path.write_text(SAMPLE, encoding="utf-8")
|
||||
|
||||
def tearDown(self):
|
||||
self.tmp.cleanup()
|
||||
|
||||
def _args(self, **over):
|
||||
base = dict(
|
||||
emulators_dir=str(self.dir), backfill_commits=False,
|
||||
rebase_refs=False, bump_commit=False, accept_changed=False,
|
||||
dry_run=True,
|
||||
)
|
||||
base.update(over)
|
||||
return argparse.Namespace(**base)
|
||||
|
||||
def _run(self, args, report, sample=SAMPLE):
|
||||
buffer = io.StringIO()
|
||||
with contextlib.redirect_stdout(buffer):
|
||||
profile_sync._apply_writes(
|
||||
args, "p", yaml.safe_load(sample), report
|
||||
)
|
||||
return buffer.getvalue()
|
||||
|
||||
def _shifted(self):
|
||||
part = PartResult(
|
||||
RefPart("a.c", 10, 12, "a.c:10-12"), "SHIFTED", None, 30, 32, []
|
||||
)
|
||||
return ProfileReport(
|
||||
name="p", repo="o/n", pin="pin", head="newhead",
|
||||
entries=[EntryReport("a.bin", "a.c:10-12", "SHIFTED", [part])],
|
||||
counts={"SHIFTED": 1},
|
||||
)
|
||||
|
||||
def test_rebase_states_the_move_and_leaves_the_file(self):
|
||||
before = self.path.read_text()
|
||||
output = self._run(self._args(rebase_refs=True), self._shifted())
|
||||
self.assertIn("would recale", output)
|
||||
self.assertIn("a.c:10-12 -> a.c:30-32", output)
|
||||
self.assertEqual(self.path.read_text(), before)
|
||||
|
||||
def test_a_pin_already_at_head_is_not_announced(self):
|
||||
"""A write that changes nothing must not be reported as one.
|
||||
|
||||
126 of 232 announced bumps on the corpus rewrote the pin to the
|
||||
value it already held, so the plan overstated the work by more than
|
||||
double and no reader could tell which entries meant anything.
|
||||
"""
|
||||
part = PartResult(
|
||||
RefPart("a.c", 10, 12, "a.c:10-12"), "ANCHORED", None, 10, 12, []
|
||||
)
|
||||
self.path.write_text(
|
||||
insert_after_line(SAMPLE, "profiled_date", 'source_commit: "newhead"'),
|
||||
encoding="utf-8",
|
||||
)
|
||||
report = ProfileReport(
|
||||
name="p", repo="o/n", pin="newhead", head="newhead",
|
||||
entries=[EntryReport("a.bin", "a.c:10-12", "ANCHORED", [part])],
|
||||
counts={"ANCHORED": 1},
|
||||
)
|
||||
self.assertFalse(bump_commit(self.path, report))
|
||||
self.assertEqual(self._run(self._args(bump_commit=True), report), "")
|
||||
|
||||
def test_bump_states_the_pin_and_leaves_the_file(self):
|
||||
before = self.path.read_text()
|
||||
part = PartResult(
|
||||
RefPart("a.c", 10, 12, "a.c:10-12"), "ANCHORED", None, 10, 12, []
|
||||
)
|
||||
report = ProfileReport(
|
||||
name="p", repo="o/n", pin="pin", head="newhead",
|
||||
entries=[EntryReport("a.bin", "a.c:10-12", "ANCHORED", [part])],
|
||||
counts={"ANCHORED": 1},
|
||||
)
|
||||
output = self._run(self._args(bump_commit=True), report)
|
||||
self.assertIn("would set source_commit -> newhead", output)
|
||||
self.assertEqual(self.path.read_text(), before)
|
||||
|
||||
def test_writes_still_happen_without_the_flag(self):
|
||||
output = self._run(
|
||||
self._args(rebase_refs=True, dry_run=False), self._shifted()
|
||||
)
|
||||
self.assertNotIn("would", output)
|
||||
self.assertIn('source_ref: "a.c:30-32"', self.path.read_text())
|
||||
|
||||
def test_a_planned_bump_reads_the_prose_the_rebase_would_have_left(self):
|
||||
"""The pin is blocked by prose until the rebase moves it.
|
||||
|
||||
Planning the bump against the file on disk would report a refusal
|
||||
that the real run, where the rebase lands first, never produces.
|
||||
"""
|
||||
self.path.write_text(PROSE_SAMPLE, encoding="utf-8")
|
||||
before = self.path.read_text()
|
||||
part = PartResult(RefPart("a.c", 10, 10, "10"), "SHIFTED", None, 14, 14, [])
|
||||
entry = EntryReport(
|
||||
"notes", "a.c:10", "SHIFTED", [part], "prose", "notes"
|
||||
)
|
||||
report = ProfileReport(
|
||||
name="p", repo="o/n", pin="pin", head="newhead",
|
||||
entries=[entry], counts={"SHIFTED": 1},
|
||||
)
|
||||
self.assertEqual(
|
||||
profile_sync.pending_recale(report, False, before), 1,
|
||||
"the prose still describes the pinned revision",
|
||||
)
|
||||
output = self._run(
|
||||
self._args(rebase_refs=True, bump_commit=True), report, PROSE_SAMPLE
|
||||
)
|
||||
self.assertIn("would recale", output)
|
||||
self.assertIn("would set source_commit -> newhead", output)
|
||||
self.assertEqual(self.path.read_text(), before)
|
||||
|
||||
|
||||
class TestBumpCommit(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.tmp = tempfile.TemporaryDirectory()
|
||||
|
||||
@@ -282,6 +282,21 @@ recaling is all or nothing per profile: a profile that still needs a
|
||||
re-read is left untouched and moves as a whole once resolved. Rewrites
|
||||
preserve the prose of annotated refs, replacing only the location.
|
||||
|
||||
Those two writes belong in one pass, because a profile whose refs moved
|
||||
without its pin describes two revisions at once:
|
||||
|
||||
```bash
|
||||
python scripts/profile_sync.py --all --rebase-refs --bump-commit --dry-run
|
||||
python scripts/profile_sync.py --all --rebase-refs --bump-commit
|
||||
```
|
||||
|
||||
The first prints the plan and changes nothing: `would recale` per ref and
|
||||
`would set source_commit` per profile. It reaches that plan by running the
|
||||
real write path over a throwaway copy, so the planned bump reads the text
|
||||
the recale would have left rather than the one on disk; a pin held back by
|
||||
prose that the same pass would have moved is not reported as blocked. Drop
|
||||
`--dry-run` and the same pass writes, recale before bump on each profile.
|
||||
|
||||
`--accept-changed` recales `CHANGED` refs too, for a profile whose diff
|
||||
has been read and judged benign. It applies to one profile at a time and
|
||||
is refused with `--all`.
|
||||
|
||||
Reference in new issue
Block a user