Record a request in pins.json before it queues

.github/workflows/publish.yml

@@ -5,8 +5,18 @@ name: Publish the index
 # `publish-package-release` dispatch here (PyDevices/.github's
 # reusable-request-mip-publication.yml), naming its profile, repository and
 # tag; the run records the tag in pins.json, rebuilds every package from the
-# pins, and deploys. Runs queue one behind another: each rebuilds everything,
-# so the last one in wins, and none is cancelled for a later one.
+# pins, and deploys.
+#
+# Recording comes first and is not queued. GitHub's concurrency group holds one
+# running job and one pending job, and a newer pending job replaces the older
+# one, so a request that waited in the queue could be replaced by a later one
+# and its pin never written. Instead the `record` job commits the pin to main
+# the moment the request arrives; only the build and the deploy queue. Each
+# build starts by checking out main, so it carries every pin recorded before
+# it started, and a build that replaces a pending one still publishes the
+# pending one's release. (pins.json therefore says what was asked for; a
+# build that fails leaves its pin there, and the next build fails the same way
+# until the pin is fixed.)
 #
 # A merge to pydevices' main that touches board_configs/ or drivers/ sends the
 # same dispatch with the `boards` profile and the merge commit. The run lays
@@ -37,10 +47,6 @@ on:
         default: ""
         type: string
 
-concurrency:
-  group: omnipython-lib-publication-queue
-  cancel-in-progress: false
-
 permissions:
   contents: read
 
@@ -53,7 +59,28 @@ env:
   MPY_CROSS_PIP: mpy-cross==1.29.0.post2
 
 jobs:
+  record:
+    # Not in the queue: every request writes its pin before it waits.
+    if: github.event.client_payload.profile != '' || (inputs.profile != '' && inputs.test-root-ref == '')
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    permissions:
+      contents: write
+    steps:
+      - uses: actions/checkout@v7
+        with:
+          ref: main
+      - name: Record the release in pins.json on main
+        run: python tools/pins.py record --profile "$PROFILE" --repository "$REPOSITORY" --ref "$REF"
+
   build:
+    needs: record
+    # The record job is skipped for a rebuild or a test build, which is fine;
+    # it failing (a request pins.py refuses) is not.
+    if: ${{ !failure() && !cancelled() }}
+    concurrency:
+      group: omnipython-lib-publication-queue
+      cancel-in-progress: false
     runs-on: ubuntu-latest
     timeout-minutes: 30
     steps:
@@ -65,10 +92,6 @@ jobs:
         with:
           python-version: '3.13'
 
-      - name: Record the release
-        if: env.PROFILE != '' && env.TEST_ROOT_REF == ''
-        run: python tools/pins.py apply --profile "$PROFILE" --repository "$REPOSITORY" --ref "$REF"
-
       - name: Check out the pinned sources
         run: |
           args=(--out src)
@@ -89,23 +112,19 @@ jobs:
       - name: Check the index against the pins
         run: python tools/pins.py verify build/index
 
-      - name: Keep what the record job commits
-        run: cp pins.json "$RUNNER_TEMP/pins.json"
-
       - name: Prepare the Pages artifact
         run: touch build/index/.nojekyll
 
       - uses: actions/upload-pages-artifact@v5
         with:
           path: build/index
 
-      - uses: actions/upload-artifact@v7
-        with:
-          name: pins
-          path: ${{ runner.temp }}/pins.json
-
   deploy:
     needs: build
+    # The same queue as the build, so two deploys never overlap.
+    concurrency:
+      group: omnipython-lib-publication-queue
+      cancel-in-progress: false
     runs-on: ubuntu-latest
     permissions:
       pages: write
@@ -116,40 +135,3 @@ jobs:
     steps:
       - id: deployment
         uses: actions/deploy-pages@v5
-
-  record:
-    # pins.json says what is live, so it changes only once the deploy worked.
-    needs: deploy
-    if: github.event.client_payload.profile != '' || (inputs.profile != '' && inputs.test-root-ref == '')
-    runs-on: ubuntu-latest
-    permissions:
-      contents: write
-    steps:
-      - uses: actions/checkout@v7
-        with:
-          ref: main
-      - uses: actions/download-artifact@v8
-        with:
-          name: pins
-          path: ${{ runner.temp }}/pins
-      - name: Commit pins.json
-        run: |
-          cp "$RUNNER_TEMP/pins/pins.json" pins.json
-          git config user.name 'github-actions[bot]'
-          git config user.email 'github-actions[bot]@users.noreply.github.com'
-          git add pins.json
-          if git diff --cached --quiet; then
-            echo "pins.json already records ${PROFILE} ${REF}."
-            exit 0
-          fi
-          if [ "${PROFILE}" = boards ]; then
-            git commit -m "Publish boards and drivers from ${REPOSITORY#*/} ${REF:0:12}"
-          else
-            git commit -m "Publish ${REPOSITORY#*/} ${REF}"
-          fi
-          for attempt in 1 2 3; do
-            if git push origin HEAD:main; then exit 0; fi
-            git pull --rebase origin main
-          done
-          echo "push failed after 3 attempts" >&2
-          exit 1

.github/workflows/test.yml

@@ -0,0 +1,28 @@
+name: Tests
+
+on:
+  pull_request:
+    paths:
+      - 'tools/**'
+      - 'tests/**'
+      - '.github/workflows/test.yml'
+  push:
+    branches: [main]
+    paths:
+      - 'tools/**'
+      - 'tests/**'
+      - '.github/workflows/test.yml'
+
+permissions:
+  contents: read
+
+jobs:
+  unittest:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+      - uses: actions/setup-python@v7
+        with:
+          python-version: '3.13'
+      - name: tools/pins.py
+        run: python -m unittest discover -s tests -v

README.md

@@ -58,7 +58,10 @@ too, naming the commit: the board and driver files come from that commit and
 everything else from the last pydevices release, so a board or driver is
 published when it merges. The next pydevices release carries them and clears
 the commit. [`publish.yml`](.github/workflows/publish.yml) records the request
-in [`pins.json`](pins.json), rebuilds every package from the releases pinned
-there with pydevices' `tools/libbuild.py`, and deploys the result. Requests
-queue, one run at a time. A run can also be started by hand from the Actions
-tab to rebuild from the pins as they are.
+in [`pins.json`](pins.json) as soon as it arrives, rebuilds every package from
+the releases pinned there with pydevices' `tools/libbuild.py`, and deploys the
+result. Builds queue, one at a time, and a build waiting behind another may be
+replaced by a newer one; that costs nothing, because every build starts from
+the pins as recorded so far, so each request is in the index by the next build
+that finishes. A run can also be started by hand from the Actions tab to
+rebuild from the pins as they are.

tests/test_pins.py

@@ -0,0 +1,178 @@
+"""tools/pins.py: applying a request to pins.json, and recording it on main.
+
+Everything runs against throwaway git repositories; nothing is fetched.
+"""
+
+import importlib.util
+import json
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parent.parent
+
+spec = importlib.util.spec_from_file_location("pins", ROOT / "tools" / "pins.py")
+pins = importlib.util.module_from_spec(spec)
+sys.modules["pins"] = pins
+spec.loader.exec_module(pins)
+
+BOARDS = "1" * 40
+BOARDS2 = "2" * 40
+PINS = {
+    "pydevices": {"repository": "PyDevices/pydevices", "ref": "v0.7.7rc1"},
+    "boards": {"repository": "PyDevices/pydevices", "ref": None},
+    "palettes": {"repository": "PyDevices/pydevices", "ref": "palettes-v0.0.15rc1"},
+    "pdwidgets": {"repository": "PyDevices/pydevices", "ref": None},
+    "pygraphics": {"repository": "PyDevices/pygraphics", "ref": "v0.1.0"},
+}
+
+
+def git(directory, *args):
+    return subprocess.run(["git", "-C", str(directory), *args], check=True, capture_output=True, text=True).stdout.strip()
+
+
+class Checkout(unittest.TestCase):
+    """A bare remote with pins.json on main, and clones of it."""
+
+    def setUp(self):
+        self._tmp = tempfile.TemporaryDirectory()
+        self.addCleanup(self._tmp.cleanup)
+        self.dir = Path(self._tmp.name)
+        self.remote = self.dir / "remote.git"
+        git(self.dir, "init", "-q", "--bare", "-b", "main", str(self.remote))
+        seed = self.clone("seed")
+        (seed / "pins.json").write_text(json.dumps(PINS, indent=2) + "\n")
+        git(seed, "add", "pins.json")
+        git(seed, "-c", "user.name=t", "-c", "user.email=t@example.com", "commit", "-q", "-m", "pins")
+        git(seed, "push", "-q", "origin", "HEAD:main")
+        self.addCleanup(self.restore_pins_path)
+        self.saved = pins.PINS
+
+    def restore_pins_path(self):
+        pins.PINS = self.saved
+
+    def clone(self, name):
+        path = self.dir / name
+        git(self.dir, "clone", "-q", str(self.remote), str(path))
+        return path
+
+    def use(self, clone):
+        pins.PINS = clone / "pins.json"
+
+    def remote_pins(self):
+        return json.loads(git(self.remote, "show", "main:pins.json"))
+
+
+class Applying(Checkout):
+    def test_a_release_sets_its_pin_and_clears_the_boards_pin(self):
+        clone = self.clone("a")
+        self.use(clone)
+        pins.apply("boards", "PyDevices/pydevices", BOARDS)
+        self.assertEqual(pins.load()["boards"]["ref"], BOARDS)
+        pins.apply("pydevices", "PyDevices/pydevices", "v0.7.7")
+        self.assertEqual(pins.load()["pydevices"]["ref"], "v0.7.7")
+        self.assertIsNone(pins.load()["boards"]["ref"])
+
+    def test_refuses_what_is_not_a_request(self):
+        self.use(self.clone("a"))
+        for args in (("boards", "PyDevices/pydevices", "v0.7.7"),
+                     ("pydevices", "PyDevices/pydevices", BOARDS),
+                     ("palettes", "PyDevices/pydevices", "v0.0.15"),
+                     ("pygraphics", "PyDevices/pydevices", "v0.1.1"),
+                     ("nothing", "PyDevices/pydevices", "v0.7.7")):
+            with self.assertRaises(SystemExit, msg=args):
+                pins.apply(*args)
+
+
+class Recording(Checkout):
+    def test_pushes_the_pin_to_the_remote(self):
+        self.use(self.clone("a"))
+        pins.record("palettes", "PyDevices/pydevices", "palettes-v0.0.14")
+        self.assertEqual(self.remote_pins()["palettes"]["ref"], "palettes-v0.0.14")
+        self.assertEqual(git(self.remote, "log", "-1", "--format=%s", "main"), "Publish pydevices palettes-v0.0.14")
+
+    def test_boards_message_names_the_short_commit(self):
+        self.use(self.clone("a"))
+        pins.record("boards", "PyDevices/pydevices", BOARDS)
+        self.assertEqual(git(self.remote, "log", "-1", "--format=%s", "main"),
+                         "Publish boards and drivers from pydevices 111111111111")
+
+    def test_the_same_request_twice_commits_once(self):
+        self.use(self.clone("a"))
+        pins.record("palettes", "PyDevices/pydevices", "palettes-v0.0.14")
+        before = git(self.remote, "rev-parse", "main")
+        pins.record("palettes", "PyDevices/pydevices", "palettes-v0.0.14")
+        self.assertEqual(git(self.remote, "rev-parse", "main"), before)
+
+    def test_a_request_recorded_after_a_stale_checkout_keeps_the_earlier_one(self):
+        # The lost-release case: two runs check out main together, the release's
+        # record lands first, and the boards request records from a checkout that
+        # does not have it. Both must be in main afterwards.
+        release, boards = self.clone("release"), self.clone("boards")
+        self.use(release)
+        pins.record("palettes", "PyDevices/pydevices", "palettes-v0.0.14")
+        self.use(boards)  # still at the commit before the release
+        pins.record("boards", "PyDevices/pydevices", BOARDS)
+        final = self.remote_pins()
+        self.assertEqual(final["palettes"]["ref"], "palettes-v0.0.14")
+        self.assertEqual(final["boards"]["ref"], BOARDS)
+
+    def test_a_push_that_loses_a_race_starts_again(self):
+        mine, other = self.clone("mine"), self.clone("other")
+        self.use(other)
+        real_run = subprocess.run
+        raced = []
+
+        def run(cmd, *args, **kwargs):
+            # Just before this run's first push, another run's commit lands.
+            if cmd[:2] == ["git", "push"] and not raced:
+                raced.append(True)
+                pins.PINS = other / "pins.json"
+                pins.record("pygraphics", "PyDevices/pygraphics", "v0.1.1")
+                pins.PINS = mine / "pins.json"
+            return real_run(cmd, *args, **kwargs)
+
+        self.use(mine)
+        original = pins.subprocess.run
+        pins.subprocess.run = run
+        original_sleep = pins.time.sleep
+        pins.time.sleep = lambda seconds: None
+        self.addCleanup(setattr, pins.subprocess, "run", original)
+        self.addCleanup(setattr, pins.time, "sleep", original_sleep)
+        pins.record("boards", "PyDevices/pydevices", BOARDS)
+        self.assertTrue(raced)
+        final = self.remote_pins()
+        self.assertEqual(final["boards"]["ref"], BOARDS)
+        self.assertEqual(final["pygraphics"]["ref"], "v0.1.1")
+
+    def test_two_records_in_a_row_from_stale_checkouts(self):
+        clones = [self.clone(f"c{i}") for i in range(3)]
+        requests = [("palettes", "palettes-v0.0.14"), ("boards", BOARDS), ("pygraphics", "v0.1.1")]
+        for clone, (profile, ref) in zip(clones, requests):
+            self.use(clone)
+            repo = "PyDevices/pygraphics" if profile == "pygraphics" else "PyDevices/pydevices"
+            pins.record(profile, repo, ref)
+        final = self.remote_pins()
+        self.assertEqual(final["palettes"]["ref"], "palettes-v0.0.14")
+        self.assertEqual(final["boards"]["ref"], BOARDS)
+        self.assertEqual(final["pygraphics"]["ref"], "v0.1.1")
+
+    def test_refuses_a_checkout_with_uncommitted_changes(self):
+        clone = self.clone("a")
+        self.use(clone)
+        (clone / "pins.json").write_text("{}\n")
+        with self.assertRaises(SystemExit):
+            pins.record("boards", "PyDevices/pydevices", BOARDS)
+
+    def test_a_refused_request_pushes_nothing(self):
+        self.use(self.clone("a"))
+        before = git(self.remote, "rev-parse", "main")
+        with self.assertRaises(SystemExit):
+            pins.record("boards", "PyDevices/pydevices", "v0.7.7")
+        self.assertEqual(git(self.remote, "rev-parse", "main"), before)
+
+
+if __name__ == "__main__":
+    unittest.main()