Refuse a pin move that would roll the source back

Rehearsing the bot against the real upstreams found newlib pinned to
the head of `newlib-4.6.0` while `vita`, its default branch, sits three
major versions behind it: following `vita` would have quietly undone
the 4.6 upgrade on the first nightly, and the only sign would have been
whatever broke afterwards. The config now names the branch the work is
actually on.

That was luck -- somebody happened to look. A pin only moves to a
revision that contains it, so a force-push upstream, a branch that is
not where the work happens, or a pin taken from somewhere else stops
the bot and names both revisions. The check asks the remote for commits
and nothing else: the whole history of the largest leaf is a few
megabytes and about three seconds, and only a pin that is actually
moving pays for it.

Author: frangarcj

cmake/pin-tracking.json

@@ -2,7 +2,7 @@
   "schema": 1,
   "_comment": "Which git-pinned components the pin-bump bot moves forward, and which it leaves alone. Every component pinned by repository plus revision must appear in exactly one of the two maps: scripts/bump-pins.py refuses to run otherwise, so a new pin cannot go unnoticed in either direction.",
   "tracked": {
-    "newlib": {"branch": "vita"},
+    "newlib": {"branch": "newlib-4.6.0"},
     "samples": {"branch": "master"},
     "headers": {"branch": "master"},
     "toolchain": {"branch": "master"},

scripts/bump-pins.py

@@ -3,10 +3,11 @@
 
 import argparse
 import json
-import os
 import re
+import shutil
 import subprocess
 import sys
+import tempfile
 
 COMPONENTS_PATH = "cmake/Components.cmake"
 TRACKING_PATH = "cmake/pin-tracking.json"
@@ -116,6 +117,41 @@ def resolve_branch(repository, branch):
     return result.stdout.split()[0]
 
 
+def contains(repository, pin, branch):
+    """Whether what the branch holds now was built on top of the pin.
+
+    A pin that moves to a revision not descended from it is a rollback:
+    a force-push upstream, a branch that is not where the work happens,
+    a pin taken from somewhere else. The bot refuses and a person looks.
+    """
+
+    probe = tempfile.mkdtemp(prefix="bump-pins-")
+    try:
+        subprocess.run(["git", "init", "--quiet", probe], check=True)
+        # tree:0 asks for commits and nothing else: the whole history of
+        # the largest leaf arrives in a few megabytes and a few seconds.
+        fetch = subprocess.run(
+            [
+                "git", "-C", probe, "fetch", "--quiet", "--filter=tree:0",
+                "--no-tags", repository,
+                f"+refs/heads/{branch}:refs/heads/tracked", pin,
+            ],
+            capture_output=True,
+            text=True,
+        )
+        if fetch.returncode != 0:
+            raise BumpError(
+                f"{repository} cannot serve {pin} and {branch} together: "
+                f"{fetch.stderr.strip() or 'git fetch failed'}"
+            )
+        ancestor = subprocess.run(
+            ["git", "-C", probe, "merge-base", "--is-ancestor", pin, "refs/heads/tracked"]
+        )
+        return ancestor.returncode == 0
+    finally:
+        shutil.rmtree(probe, ignore_errors=True)
+
+
 def rewrite_pin(text, variable, old, new):
     pattern = re.compile(
         r"(set\(\s*" + re.escape(variable) + r"_TAG\s+)" + re.escape(old) + r"(?=[\s)])"
@@ -160,6 +196,12 @@ def bump(components_path, tracking_path, overrides, dry_run):
         head = resolve_branch(repository, tracked[name])
         if head == component["pin"]:
             continue
+        if not contains(repository, component["pin"], tracked[name]):
+            raise BumpError(
+                f"{name}: {tracked[name]} is at {head}, which does not contain "
+                f"the pinned {component['pin']}; moving it would roll the "
+                "source back"
+            )
         text = rewrite_pin(text, component["variable"], component["pin"], head)
         moves.append(
             {

tests/ci/test-bump-pins.sh

@@ -20,6 +20,22 @@ bump() {
 		--components "$components" --tracking "$tracking" "$@"
 }
 
+seed_pin() {
+	python3 - "$components" "$1" "$2" <<'PYEOF'
+import re, sys
+path, name, revision = sys.argv[1], sys.argv[2], sys.argv[3]
+variable = name.upper().replace("-", "_")
+text, replaced = re.subn(
+    r"(set\(\s*" + variable + r"_TAG\s+)\S+",
+    lambda match: match.group(1) + revision,
+    open(path).read(),
+    count=1,
+)
+assert replaced == 1, f"no {variable}_TAG to seed"
+open(path, "w").write(text)
+PYEOF
+}
+
 # The upstreams the bot follows, straight from the file under test: a
 # component added to the config is exercised here without editing this test.
 mapfile -t tracked < <(python3 - "$tracking" <<'PYEOF'
@@ -48,6 +64,10 @@ for entry in "${tracked[@]}"; do
 	git -C "$upstream" commit --quiet -m "first"
 	upstream_of[$name]=$upstream
 	overrides+=(--repository "$name=$upstream")
+	# The pins start where these fixtures start: the real ones name
+	# revisions no fixture can serve, and a bump is a move from something
+	# the upstream actually holds.
+	seed_pin "$name" "$(git -C "$upstream" rev-parse HEAD)"
 done
 
 pin_of() {
@@ -61,6 +81,11 @@ PYEOF
 }
 
 # Everything moves once, to exactly what the upstream branches hold.
+for entry in "${tracked[@]}"; do
+	read -r name _ <<< "$entry"
+	echo "moved" > "${upstream_of[$name]}/file"
+	git -C "${upstream_of[$name]}" commit --quiet -am "moved"
+done
 output=$(bump "${overrides[@]}")
 grep -q "bump-pins: ${#tracked[@]} pin(s) moved" <<< "$output" || {
 	printf 'the first bump did not move every tracked pin: %s\n' "$output" >&2
@@ -142,6 +167,30 @@ cmp -s "$components" "$temporary_root/before-dry" || {
 	exit 1
 }
 
+# A branch that no longer contains the pin is a rollback, whatever the
+# reason -- a force-push, or a pin that lives on another branch. The bot
+# refuses and names both revisions instead of quietly going backwards.
+git -C "$moved_upstream" branch keep
+git -C "$moved_upstream" reset --quiet --hard HEAD~2
+cp "$components" "$temporary_root/before-rollback"
+if output=$(bump "${overrides[@]}" 2>&1); then
+	printf 'the bot accepted a branch that had gone backwards\n' >&2
+	exit 1
+fi
+grep -q "$moved_name" <<< "$output" || {
+	printf 'the refusal does not name the rolled back component: %s\n' "$output" >&2
+	exit 1
+}
+grep -q "$(pin_of "$moved_name")" <<< "$output" || {
+	printf 'the refusal does not name the pin it kept: %s\n' "$output" >&2
+	exit 1
+}
+cmp -s "$components" "$temporary_root/before-rollback" || {
+	printf 'a refused bump rewrote Components.cmake\n' >&2
+	exit 1
+}
+git -C "$moved_upstream" reset --quiet --hard keep
+
 # The pins the config leaves alone stay exactly as the file declares them,
 # and are never resolved: no override is given for any of them here.
 for name in $(python3 -c 'import json,sys; print(" ".join(sorted(json.load(open(sys.argv[1]))["untracked"])))' "$tracking"); do