Refuse a pin move that would roll the source back

Rehearsing the bot against the real upstreams found newlib's `vita`
branch 2445 commits behind the pinned revision, missing the 4.3, 4.4,
4.5 and 4.6 merges the SDK is actually built from. Following it would
have undone the 4.6 upgrade on the first nightly, at 03:00, and the only
sign would have been whatever broke afterwards. That is fixed where it
was broken -- `vita` now holds what the SDK builds -- but it was luck
that somebody looked.

So a pin only moves to a revision that contains it. 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

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,50 @@ 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"],
+            capture_output=True,
+            text=True,
+        )
+        # 1 is the answer "no"; anything else is git saying it could not
+        # work out the question, which must not read as a rollback.
+        if ancestor.returncode not in (0, 1):
+            raise BumpError(
+                f"{repository}: cannot tell whether {branch} contains {pin}: "
+                f"{ancestor.stderr.strip() or 'git merge-base failed'}"
+            )
+        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 +205,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,43 @@ 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
+
+# A pin git cannot resolve is git failing to answer, not an answer: it
+# must not be reported as a source going backwards.
+seed_pin "$moved_name" "not-a-revision"
+if output=$(bump "${overrides[@]}" 2>&1); then
+	printf 'the bot ran with a pin that is not a revision\n' >&2
+	exit 1
+fi
+if grep -q 'roll the source back' <<< "$output"; then
+	printf 'an unresolvable pin was reported as a rollback: %s\n' "$output" >&2
+	exit 1
+fi
+seed_pin "$moved_name" "$moved_head"
+
 # 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