diff --git a/scripts/check_freshness.py b/scripts/check_freshness.py index 604fcfcb..ed6c3b52 100644 --- a/scripts/check_freshness.py +++ b/scripts/check_freshness.py @@ -1253,6 +1253,8 @@ def check_profiles(run: bool, offline: bool) -> list[Finding]: "--all", "--json", "--check-version", + "--detect-new-files", + "--watch-hashes", ] if offline: cmd.append("--offline") @@ -1273,7 +1275,9 @@ def summarize_profile_sync(report: object) -> list[Finding]: The JSON carries the ref verdicts and the pin against the tip. A profile whose refs need review is stale, one the forge could not serve is unknown, and one whose upstream moved while every ref still anchors is reported as - such without being counted as stale. + such without being counted as stale. A cited file that gained a file name + since the pin, or a hash the profile does not declare, is stale too: the + upstream may load a file the profile does not document. """ if not isinstance(report, list): return [Finding("profiles", "profile_sync", ERROR, detail="report is not a list")] @@ -1303,7 +1307,18 @@ def summarize_profile_sync(report: object) -> list[Finding]: continue review = row.get("needs_review") or 0 pin, head = str(row.get("pin") or ""), str(row.get("head") or "") - if review: + added = sorted({ + str(item.get("name")) + for item in row.get("new_files") or () + if isinstance(item, dict) and item.get("since_pin") + }) + hashes = len(row.get("new_hashes") or ()) + reasons = [f"{review} refs to review"] if review else [] + if added: + reasons.append(f"new file names since the pin: {', '.join(added)}") + if hashes: + reasons.append(f"{hashes} undeclared hashes added since the pin") + if reasons: findings.append( Finding( "profiles", @@ -1311,7 +1326,7 @@ def summarize_profile_sync(report: object) -> list[Finding]: STALE, local=pin[:8], remote=head[:8], - detail=f"{review} refs to review", + detail=", ".join(reasons), ) ) elif pin and head and pin != head: diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 802cadd7..bec43bcc 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -966,6 +966,14 @@ class ProfileReport: # release, kept on the report so every output carries it. version: VersionReport | None = None version_warning: str | None = None + # --detect-new-files and --watch-hashes, kept on the report so --json + # carries them to check_freshness. None when not asked. A new file is + # {path, line, name, since_pin}: since_pin when the pinned revision of + # the same file does not carry the name. A new hash is {path, kind, value}. + new_files: list[dict] | None = None + new_files_elided: dict[str, int] = field(default_factory=dict) + new_hashes: list[dict] | None = None + detection_skipped: str | None = None # Skips that say the profile cannot be checked as written: a repository # that no longer answers, no pin to read, a death the forge contradicts. @@ -3756,37 +3764,84 @@ def _cited_paths(report: ProfileReport) -> list[str]: ) -def _print_detection( +def _attach_detection( + args: argparse.Namespace, profile: dict, report: ProfileReport +) -> None: + """Record undeclared names and hashes of the cited files on the report. + + Kept on the report rather than printed, so --json carries them as the + text report does. A forge that refuses is named on the report: the ref + report already muted it, and one refusal must not end a pass over every + profile. A quota signal still stops everything. + """ + if not report.head or not (args.detect_new_files or args.watch_hashes): + return + repo = select_repo(profile) + if repo is None: + return + try: + _detect(args, profile, report, repo) + except upstream.RateLimitError: + raise + except upstream.UpstreamError as exc: + report.detection_skipped = str(exc) + + +def _detect( args: argparse.Namespace, profile: dict, report: ProfileReport, repo: upstream.Repo, ) -> None: names, hashes = declared_names(profile), declared_hashes(profile) + new_files: list[dict] = [] + new_hashes: list[dict] = [] for path in _cited_paths(report): head_lines = upstream.fetch_file( repo, report.head, path, args.cache_dir, args.offline ) if head_lines is None: continue + pin_lines = ( + upstream.fetch_file(repo, report.pin, path, args.cache_dir, args.offline) + or [] + ) if args.detect_new_files: found, elided = detect_new_files(head_lines, names) - for number, name in found: - print(f" new file: {path}:{number} {name}") - if elided: - print(f" new file: {elided} further matches not shown") - if args.watch_hashes: - pin_lines = ( - upstream.fetch_file( - repo, report.pin, path, args.cache_dir, args.offline - ) - or [] + at_pin = { + name.casefold() + for _, name in detect_new_files(pin_lines, names, cap=sys.maxsize)[0] + } + new_files.extend( + {"path": path, "line": number, "name": name, + "since_pin": name.casefold() not in at_pin} + for number, name in found ) + if elided: + report.new_files_elided[path] = elided + if args.watch_hashes: added = [ line for line in difflib.unified_diff(pin_lines, head_lines, n=0) if line.startswith("+") and not line.startswith("+++") ] - for kind, value in watch_hashes(added, hashes): - print(f" new {kind}: {path} {value}") + new_hashes.extend( + {"path": path, "kind": kind, "value": value} + for kind, value in watch_hashes(added, hashes) + ) + if args.detect_new_files: + report.new_files = new_files + if args.watch_hashes: + report.new_hashes = new_hashes + + +def _print_detection(report: ProfileReport) -> None: + if report.detection_skipped: + print(f" extras skipped: {report.detection_skipped}") + for item in report.new_files or (): + print(f" new file: {item['path']}:{item['line']} {item['name']}") + for path, elided in report.new_files_elided.items(): + print(f" new file: {path}: {elided} further matches not shown") + for item in report.new_hashes or (): + print(f" new {item['kind']}: {item['path']} {item['value']}") def _attach_version( @@ -3820,13 +3875,8 @@ def _print_extras( instead of ending a pass over every profile. A quota signal still stops everything. """ - wants = ( - args.detect_new_files - or args.watch_hashes - or args.full_diff - or args.tree_diff - ) - if not wants or not report.head: + _print_detection(report) + if not (args.full_diff or args.tree_diff) or not report.head: return repo = select_repo(profile) if repo is None: @@ -3843,8 +3893,6 @@ def _print_extras_from( args: argparse.Namespace, profile: dict, report: ProfileReport, repo: upstream.Repo, ) -> None: - if args.detect_new_files or args.watch_hashes: - _print_detection(args, profile, report, repo) if args.full_diff: for path in _cited_paths(report): if args.ref and args.ref not in path: @@ -3991,13 +4039,15 @@ def _refuse_flag_combinations(args: argparse.Namespace) -> None: ] # The version check lives on the report, so the outputs built from the # report (text, --json, --markdown) carry it. A URL list and a ranking - # do not. + # do not. Detection lives on the report too, and --json carries it for + # check_freshness; the markdown table has no column for it. lists_only = args.fetch_plan or args.triage + off_json = lists_only or args.markdown extras = [ flag for flag, on in ( ("--check-version", args.check_version and lists_only), - ("--detect-new-files", args.detect_new_files), - ("--watch-hashes", args.watch_hashes), + ("--detect-new-files", args.detect_new_files and off_json), + ("--watch-hashes", args.watch_hashes and off_json), ("--full-diff", args.full_diff), ("--tree-diff", args.tree_diff), ) if on @@ -4094,6 +4144,7 @@ def main() -> None: profile["version_source"].get("no_genuine_copy") or "") or None if args.check_version: _attach_version(args, profile, report) + _attach_detection(args, profile, report) reports.append(report) if writes: try: diff --git a/tests/test_check_freshness.py b/tests/test_check_freshness.py index 664fc636..d99e330b 100644 --- a/tests/test_check_freshness.py +++ b/tests/test_check_freshness.py @@ -388,6 +388,29 @@ class ProfileSummaryTests(unittest.TestCase): self.assertNotIn("a", by_subject) self.assertNotIn("b", by_subject) + def test_new_upstream_names_and_hashes_are_stale(self): + """A file name the cited code gained since the pin, or a hash nobody + declares, was printed only by the text pass that --profiles never ran.""" + report = [ + {"name": "a", "pin": "1" * 40, "head": "2" * 40, "needs_review": 0, + "new_files": [ + {"path": "x.c", "line": 4, "name": "old.rom", "since_pin": False}, + {"path": "x.c", "line": 9, "name": "new.rom", "since_pin": True}, + ], "new_hashes": []}, + {"name": "b", "pin": "1" * 40, "head": "2" * 40, "needs_review": 0, + "new_files": [], "new_hashes": [{"path": "x.c", "kind": "md5", "value": "0" * 32}]}, + {"name": "c", "pin": "1" * 40, "head": "2" * 40, "needs_review": 0, + "new_files": [ + {"path": "x.c", "line": 4, "name": "old.rom", "since_pin": False}, + ], "new_hashes": []}, + ] + by_subject = {f.subject: f for f in cf.summarize_profile_sync(report)} + self.assertEqual(by_subject["a"].status, cf.STALE) + self.assertIn("new.rom", by_subject["a"].detail) + self.assertNotIn("old.rom", by_subject["a"].detail) + self.assertEqual(by_subject["b"].status, cf.STALE) + self.assertNotIn("c", by_subject) + def test_profile_sync_accepts_the_command_it_is_given(self): """--json with the detection flags was refused, so --profiles read an empty stdout and reported "unreadable report" on every run.""" diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 576796db..11dd3bd9 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -2023,13 +2023,42 @@ class TestExtrasSurviveARefusingForge(unittest.TestCase): upstream.fetch_file = refuse try: + report = _sample_report() + profile_sync._attach_detection(self._args(), self._profile(), report) buffer = io.StringIO() with contextlib.redirect_stdout(buffer): - profile_sync._print_extras(self._args(), self._profile(), _sample_report()) + profile_sync._print_extras(self._args(), self._profile(), report) finally: upstream.fetch_file = original + self.assertEqual(report.detection_skipped, "https://x/y: HTTP 403") self.assertIn("extras skipped: https://x/y: HTTP 403", buffer.getvalue()) + def test_detection_lands_on_the_report(self): + """A name the pinned file already carried is listed, but only a name + the pin did not carry is new since the pin.""" + files = { + "pinsha0000": ['load("old.rom");'], + "headsha000": ['load("old.rom");', 'load("added.rom"); /* 0xdeadbeef */'], + } + original = upstream.fetch_file + upstream.fetch_file = lambda repo, rev, path, *a, **k: files[rev] + args = self._args() + args.watch_hashes = True + report = _sample_report() + try: + profile_sync._attach_detection(args, self._profile(), report) + finally: + upstream.fetch_file = original + by_name = {(i["path"], i["name"]): i["since_pin"] for i in report.new_files} + self.assertEqual( + by_name, + {("a.c", "old.rom"): False, ("a.c", "added.rom"): True, + ("b.c", "old.rom"): False, ("b.c", "added.rom"): True}, + ) + self.assertIn( + {"path": "a.c", "kind": "crc32", "value": "deadbeef"}, report.new_hashes + ) + def test_a_quota_signal_still_stops_the_pass(self): original = upstream.fetch_file @@ -2039,7 +2068,9 @@ class TestExtrasSurviveARefusingForge(unittest.TestCase): upstream.fetch_file = quota try: with self.assertRaises(upstream.RateLimitError): - profile_sync._print_extras(self._args(), self._profile(), _sample_report()) + profile_sync._attach_detection( + self._args(), self._profile(), _sample_report() + ) finally: upstream.fetch_file = original @@ -4112,11 +4143,35 @@ class TestOutputModesApplyTheirFlags(unittest.TestCase): self.assertIn("clean", out) self.assertIn("prose under an older pin", out) - def test_extras_are_refused_with_json(self): - code, _, err = self._main("--json", "--detect-new-files") + def test_diffs_are_refused_with_json(self): + code, _, err = self._main("--json", "--full-diff") + self.assertEqual(code, 1) + self.assertIn("--full-diff", err) + + def test_markdown_refuses_detection(self): + code, _, err = self._main("--markdown", "--detect-new-files") self.assertEqual(code, 1) self.assertIn("--detect-new-files", err) + def test_json_carries_detection(self): + """check_freshness reads --json: a new upstream file or hash printed + only in the text report stayed a human reading.""" + from unittest import mock + + def detect(args, profile, report): + report.new_files = [ + {"path": "a.c", "line": 3, "name": "new.rom", "since_pin": True} + ] + report.new_hashes = [{"path": "a.c", "kind": "md5", "value": "0" * 32}] + + with mock.patch.object(profile_sync, "_attach_detection", side_effect=detect): + code, out, err = self._main("--json", "--detect-new-files", "--watch-hashes") + self.assertEqual(code, 0, err) + rows = json.loads(out) + self.assertEqual(rows[0]["new_files"][0]["name"], "new.rom") + self.assertTrue(rows[0]["new_files"][0]["since_pin"]) + self.assertEqual(rows[0]["new_hashes"][0]["kind"], "md5") + def test_json_carries_the_version_check(self): """check_freshness asks for --json --check-version: refused, its profile row read an empty report as unreadable on every run.""" diff --git a/wiki/tools.md b/wiki/tools.md index 5a42d3ac..d01a2f9c 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -527,8 +527,11 @@ reported as `RENAMED` so `--rebase-refs` cleans the profile. `--check-version` compares `core_version` with the latest upstream tag and release. `--detect-new-files` lists filename literals at HEAD the profile does not declare. `--watch-hashes` lists hash literals added upstream that -match no entry. `--tree-diff` shows added, removed and renamed files in the -directories the refs point at. +match no entry. Both land in the `--json` report (`new_files`, with +`since_pin` true for a name the pinned file did not carry, and +`new_hashes`), which `check_freshness.py --profiles` reads: a name gained +since the pin or an undeclared hash makes the profile stale. `--tree-diff` +shows added, removed and renamed files in the directories the refs point at. Writes are explicit and mechanical only. `--backfill-commits` fills a missing `source_commit`, `--rebase-refs` recales `SHIFTED` and `RENAMED`