tool_help_check: a tool that answers --help by WRITING is a failure too

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")