diff --git a/tools/ci/tool_help_check.py b/tools/ci/tool_help_check.py index be886afa8..e1ee3e25b 100644 --- a/tools/ci/tool_help_check.py +++ b/tools/ci/tool_help_check.py @@ -30,6 +30,7 @@ """ import ast +import hashlib import os import pathlib import subprocess @@ -58,6 +59,61 @@ def is_cli(path): return has_main and 'sys.argv' in pathlib.Path(path).read_text(errors='replace') +def tree_state(root, cwd): + """A digest of the tree a tool could write into, and the untracked paths inside it. + + `git status` when the cwd is inside a work tree -- a tool writes relative to the cwd, not to the + directory it lives in, so scanning `root` alone misses the exact defect this is for: the two + `oga_*` tools wrote a file named `--help` (172 KB of JSON) into the repo ROOT and a directory + `--help/dd_metastate_...` beside it. git reports both as untracked, and it costs one process + instead of hashing every path. + """ + try: + # git in the SAME directory the tool runs in -- a write lands relative to that cwd, and asking + # git from the checker's own cwd reports the wrong tree (the self-test arm caught this). + r = subprocess.run(['git', 'status', '--porcelain', '--untracked-files=all'], + cwd=str(cwd), capture_output=True, text=True, timeout=30) + if r.returncode == 0: + untracked = {line[3:] for line in r.stdout.splitlines() if line.startswith('?? ')} + return 'git:' + hashlib.sha256(r.stdout.encode()).hexdigest()[:16], untracked + except (OSError, subprocess.TimeoutExpired): + pass + h = hashlib.sha256() + scan = pathlib.Path(cwd) + paths = set() + for f in sorted(scan.rglob('*')): + rel = str(f.relative_to(scan)) + if '.git' in rel.split('/'): + continue + paths.add(rel) + h.update(rel.encode()) + if f.is_file(): + try: + h.update(str(f.stat().st_size).encode()) + except OSError: + h.update(b'?') + return 'tree:' + h.hexdigest()[:16], paths + + +def remove_created(cwd, created): + """Delete what a misbehaving tool wrote, deepest first, and only inside the tree it was run in. + + A checker that reports "this tool writes a file while answering --help" and then leaves that file in the + developer's tree is only half a check: the next `git status` blames them, and a second run reports the same + tool against a tree that is now dirty for a reason nobody chose. + """ + scan = pathlib.Path(cwd) + for rel in sorted(created, key=lambda r: r.count('/'), reverse=True): + p = scan / rel + try: + if p.is_file() or p.is_symlink(): + p.unlink() + elif p.is_dir(): + p.rmdir() # only succeeds when the tool's own contents are gone/empty + except OSError: + pass + + def tools_under(root): for f in sorted(pathlib.Path(root).rglob('*.py')): if any(part in SKIP_DIR_NAMES for part in f.parts): @@ -118,7 +174,16 @@ def check(root): for f in tools_under(root): rel = f.relative_to(root) n += 1 + before, before_paths = tree_state(root, pathlib.Path.cwd()) state, detail = crashes(f, root) + after, after_paths = tree_state(root, pathlib.Path.cwd()) + if before != after: + remove_created(pathlib.Path.cwd(), after_paths - before_paths) + # A tool that writes while answering `--help` is a defect whether or not it also printed + # something: the file it wrote is named after an argument, and nothing will clean it up. + bad.append((rel, f'wrote into the tree while answering --help ' + f'({before} -> {after})')) + continue if state == 'silent': if is_cli(f): bad.append((rel, detail)) @@ -192,6 +257,22 @@ def chk(want, got, what): 'a file with __main__ and sys.argv is a CLI; a plain module is not') (root / 'dep.py').write_text("import not_a_real_module_xyz\n") + # The side-effect rule, in the shape that produced it: a tool whose argv[1] is an output path. + (root / 'writer.py').write_text( + "import sys, pathlib\nif __name__ == '__main__':\n" + " pathlib.Path(sys.argv[1]).write_text('x')\n") + before, before_paths = tree_state(root, root) + crashes(root / 'writer.py', root, cwd=root) + after, after_paths = tree_state(root, root) + chk(False, before == after, + 'a tool that writes its argument while answering --help changes the tree digest') + # And the check undoes the write: the file the tool created is gone, and so is its digest change. + remove_created(root, after_paths - before_paths) + chk(False, (root / '--help').exists(), + 'the file such a tool wrote is removed, so the tree is left as it was found') + chk(True, before == tree_state(root, root)[0], + 'after the removal the tree digest is the one from before the run') + chk('skip', crashes(root / 'dep.py', root)[0], 'a missing dependency is not asked, not a defect') (root / 'other.py').write_text("open('data-that-is-not-here.bin').read()\n")