diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d9c0d29..8514c7c 100644 --- a/.github/workflows/publish.yml +++ b/.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,9 +112,6 @@ 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 @@ -99,13 +119,12 @@ jobs: 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 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..1bd64d5 --- /dev/null +++ b/.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 diff --git a/README.md b/README.md index 92c861f..c6aba05 100644 --- a/README.md +++ b/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. diff --git a/tests/test_pins.py b/tests/test_pins.py new file mode 100644 index 0000000..55f52f1 --- /dev/null +++ b/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() diff --git a/tools/pins.py b/tools/pins.py index 275a65d..2d13f08 100644 --- a/tools/pins.py +++ b/tools/pins.py @@ -21,7 +21,11 @@ A publication request names a profile, the repository it came from and the tag (the commit, for ``boards``). ``apply`` checks them against ``pins.json`` -and records it. +and records it. ``record`` is ``apply`` plus a commit pushed to main, retried +against whatever else landed meanwhile; the workflow runs it the moment a +request arrives, before the request waits its turn to build, so a request that +GitHub later replaces in the queue is still in ``pins.json`` for the build +that replaces it. ``fetch`` checks the pinned sources out, ``lock`` writes the pinned tags into the checkout's ``lib-lock.json`` for its builder, and ``verify`` checks a built index has every package at the version pinned. @@ -37,6 +41,7 @@ import shutil import subprocess import sys +import time from pathlib import Path PINS = Path(__file__).resolve().parent.parent / "pins.json" @@ -95,6 +100,41 @@ def _git(*args: str, cwd: Path | None = None) -> None: subprocess.run(["git", *args], cwd=cwd, check=True, stdout=subprocess.DEVNULL) +def _message(profile: str, repository: str, ref: str) -> str: + if profile == BOARDS_PROFILE: + return f"Publish boards and drivers from {repository.split('/')[-1]} {ref[:12]}" + return f"Publish {repository.split('/')[-1]} {ref}" + + +def record(profile: str, repository: str, ref: str, remote: str = "origin", branch: str = "main", attempts: int = 6) -> None: + """Record a request in pins.json and push the commit to ``remote``'s ``branch``. + + Meant for the workflow's own checkout: each attempt first sets the checkout + to the remote's branch, so a push that lost a race to another request's + commit starts again from the merged ``pins.json`` rather than conflicting. + It refuses to start in a checkout with uncommitted changes. + """ + root = PINS.parent + if subprocess.run(["git", "status", "--porcelain"], cwd=root, check=True, capture_output=True, text=True).stdout.strip(): + raise SystemExit("record resets the checkout to the remote branch, and this one has uncommitted changes") + identity = ["-c", "user.name=github-actions[bot]", "-c", "user.email=github-actions[bot]@users.noreply.github.com"] + for attempt in range(1, attempts + 1): + _git("fetch", "-q", remote, branch, cwd=root) + _git("reset", "-q", "--hard", "FETCH_HEAD", cwd=root) + apply(profile, repository, ref) + _git("add", PINS.name, cwd=root) + if subprocess.run(["git", "diff", "--cached", "--quiet"], cwd=root).returncode == 0: + print(f"pins.json already records {profile} {ref}.") + return + _git(*identity, "commit", "-q", "-m", _message(profile, repository, ref), cwd=root) + if subprocess.run(["git", "push", "-q", remote, f"HEAD:{branch}"], cwd=root).returncode == 0: + print(f"recorded {profile} {ref}") + return + print(f"push lost a race (attempt {attempt}); starting again from {remote}/{branch}", file=sys.stderr) + time.sleep(min(attempt, 3)) + raise SystemExit(f"could not push pins.json after {attempts} attempts") + + def _fetch(repository: str, ref: str, destination: Path) -> None: destination.mkdir(parents=True) _git("init", "-q", cwd=destination) @@ -189,6 +229,10 @@ def main() -> None: a.add_argument("--profile", required=True) a.add_argument("--repository", required=True) a.add_argument("--ref", required=True) + r = commands.add_parser("record", help="apply a request and push the commit to main, retrying when another request lands first") + r.add_argument("--profile", required=True) + r.add_argument("--repository", required=True) + r.add_argument("--ref", required=True) f = commands.add_parser("fetch", help="check out the pinned sources") f.add_argument("--out", required=True, type=Path) f.add_argument("--root-ref", help="build the library repository at this ref instead of the pinned release (a test build)") @@ -199,6 +243,8 @@ def main() -> None: args = parser.parse_args() if args.command == "apply": apply(args.profile, args.repository, args.ref) + elif args.command == "record": + record(args.profile, args.repository, args.ref) elif args.command == "fetch": fetch(args.out, args.root_ref) elif args.command == "lock":