Skip to content

build: replace the Makefiles with a just task runner - #8

Draft
jdsika wants to merge 3 commits into
integration/3.0from
feature/just-task-runner
Draft

jdsika wants to merge 3 commits into
integration/3.0from
feature/just-task-runner

Conversation

@jdsika

@jdsika jdsika commented Oct 23, 2026

Copy link
Copy Markdown

What

Replaces Makefile and tests/drive/Makefile with a just task runner plus a stdlib-only scripts/devtasks.py helper. A thin Makefile shim is kept so make test and make PYTHON=/path/to/python test keep working.

Why

The Makefiles were the only part of the dev workflow that required a POSIX shell. The targets were portable; the recipe bodies were not — mkdir -p, rm -rf, command -v, for…done and $(MAKE) -C meant no suite could be driven from a Windows checkout at all.

Every recipe body is now a single command with no shell operators, so it behaves identically under sh, PowerShell and cmd. The measurable result: the C sim suite now runs natively on Windows, which it could not before.

just is a command runner, not a build system, so Make's prerequisite graph is gone. The staleness checks are reimplemented in devtasks.py with the same mtime semantics, so incrementality is preserved — a warm just test-c compiles nothing (verified: 0.7 s, 0 recompiles).

Honest scoping note for the reviewer: the portability win comes from moving logic into Python, not from just itself. just buys the cross-platform front end (windows-shell, os_family()) and a self-documenting --list; devtasks.py does the real work.

How tested

Command Platform Result
uvx pre-commit run --all-files Windows PASS — all 4 hooks (trailing-whitespace, end-of-file-fixer, ruff-format, clang-format), exit 0, no files modified
just test-c Windows 11 / MinGW gcc PASS — 12/12, loud POSIX-only skip notice shown
just test-c WSL Ubuntu 26.04 / gcc PASS — 14 executed incl. both fork() tests, no skip notice, 0 recompiles
just perf Windows 11 / MinGW gcc PASS — test_simulator_raw_perf, 42117 SPS
just test-unit WSL PASS
just test-eval WSL PASS — "All test groups passed"
just test-notebooks WSL STILL RUNNING when this PR was opened — see Risk/notes
just python=/nonexistent/python check-venv WSL exits 1 (failure propagates)
make JUST=… PYTHON=/nonexistent/python check-venv WSL exits 2 (shim propagates failure)
make JUST=… PYTHON=/usr/bin/python3 check-venv WSL exits 0 (shim forwards the override)
python -m pip install rust-just → just --version WSL 1.59.0 — the exact CI install step, verified in a throwaway venv
  • pre-commit run --all-files
  • make rebuild — run as just build (a dependency of test-unit/test-eval) in WSL. No .c/.h file is touched by this PR, so no new warnings are possible.
  • make test-unit / make test-eval — run as just test-unit / just test-eval, both pass
  • make test-c — run as just test-c on both Windows and WSL
  • make test-notebooks — in progress, not yet green. No notebook is touched by this PR.
  • make test-docker-smoke — not run. No sim dynamics, reward, observation or training-loop code is touched; this PR changes only task-runner plumbing and docs.
  • make -C tests/drive perf — run as just perf; no hot path is touched, so this proves the recipe, not a perf delta
  • make test before marking ready — blocked on notebooks above. This is why the PR is a draft.

New behaviour is covered by a new test / the fix has a test that fails without it:

Not applicable in the usual sense, and I would rather say so than invent a test. This PR adds no simulator behaviour — it replaces the mechanism that runs the tests. The only meaningful verification is executing the existing suites through the new runner on both a POSIX and a Windows host, which is the table above. The one genuinely new behaviour, the Windows skip of the two fork() tests, is asserted by the run itself: Windows reports 12 executed with the skip notice present, WSL reports 14 with it absent.

Stack hygiene

  • exactly one commit (git rev-list --count origin/integration/3.0..HEAD → 1)
  • Conventional Commits subject, rebased on current integration/3.0
  • no fork-only file in the diff (git diff --name-only origin/integration/3.0...HEAD)
  • no fork-only hunk in a shared file — AGENTS.md, CLAUDE.md, and the pull_request
    branch filters of ci.yml / install.yml are untouched in those specific places

ci.yml is in the diff, which is expected and allowed: only its two make run-lines change. The 'integration/3.0' entry in its pull_request branch filter is deliberately untouched (§3.2).

Merge gate

  • Gate 1 — CI green on the final commit (no "green except a flake") — pending, this push is the first CI run
  • Gate 2 — adversarial review posted by someone other than the author; findings fixed or
    explicitly accepted in the thread — not done. I authored this change, so under §5 I must not review it; it needs a fresh-context reviewer or a human.
  • Gate 3 — upstream-readiness proven: mirror/<slug> cherry-picked onto pure
    upstream/3.0, applied without conflict, diff is upstream-clean

Gate 3 result:

Cherry-picked onto a detached worktree at pure upstream/3.0, applied without conflict. Both abort guards passed (no fork-only file, no fork-only hunk):

.github/workflows/ci.yml
Makefile
README.md
justfile
scripts/devtasks.py
tests/drive/Makefile

Upstream disposition

  • Mirror PR to Emerge-Lab/PufferDrive requested now — needs a human to verify the final
    title, body and diff first, and the body must carry the authorship disclosure from
    copilot-instructions.md §6.1
  • Contributable, but we hold it for now, because: Gates 1 and 2 are not yet green, and the fork PR is still a draft. The intent is to upstream this — it was built Gate 3-clean from the start for exactly that reason. Raise the mirror PR once this is stacked, with the human title/body/diff check required by §5.
  • Never upstream — fork-only chore(fork): change, Gate 3 exempt

Risk / notes

Why this is a draft: just test-notebooks was still running when the PR was opened, so just test has not completed end-to-end. Do not mark ready until it has.

Reviewer: the two highest-risk areas.

  1. Lost prerequisite graph. This is the one thing that could silently regress. just has no timestamp graph, so is_stale()/newest_mtime() in devtasks.py re-implement it by hand. If that logic is wrong, builds go stale silently and tests pass against old binaries. Verified warm-run compiles nothing on both platforms, but please attack this specifically.
  2. A quoting bug that was found and fixed here, as a warning. Passing the interpreter as --interpreter "{{ python }}" leaked literal quotes into argv under cmd.exe /c, which made a present venv look missing. It is now passed via an exported PUFFER_PYTHON env var, which bypasses shell quoting entirely. Anything else crossing the just→shell→Python boundary deserves the same suspicion.

Behaviour changes.

  • tests/drive's separate unit and smoke targets are merged into one c-tests command. No caller used them separately.
  • test-c, perf and docker-smoke deliberately use the system interpreter, not .venv. The C suite is the only one that works natively on Windows, so requiring a venv would have broken the one thing this PR enables.
  • ensure-test-deps now passes --python to uv pip install. The old Makefile did not, so make PYTHON=… test-notebooks installed into the wrong environment. Fixed here because this PR promotes that override to documented behaviour.
  • No observation layout, action space, config key or checkpoint format is touched.

Pre-existing README inaccuracies corrected (please sanity-check these rather than take my word): make test-smoke was documented but never existed, and make test was described as running a "replay-HTML smoke test" while omitting test-eval, which it actually runs.

CI. Two Install just steps are added, using pip install rust-just rather than a third-party action — this repo currently uses only actions/checkout and actions/setup-python, and I did not want to widen that. It also means CI genuinely exercises the justfile instead of bypassing it.

Known follow-ups, deliberately not in this PR.

  • A Windows CI job is needed, and its absence is the main weakness here. Nothing currently prevents the Windows path from rotting. Planned as a separate PR adding a runs-on: windows-latest job running just test-c; it will need a human to add it to branch protection's required checks.
  • The fork-only docs (CONTRIBUTING.md, .github/pull_request_template.md, .github/copilot-instructions.md, .github/workflows/sync-mirror.yml) still say make … and are now stale. They are intentionally excluded from this commit — they are fork-only files under §3.1 and must land as a separate chore(fork): change, or this PR would fail Gate 3.
  • Native Windows support for the Python suites is not attempted. setup.py branches only on Linux/Darwin with GCC-only flags, so the extension cannot build with MSVC. WSL/Docker remains the answer; README now says so explicitly.

ASCS-eV/PufferDrive is consumed as a git submodule internally while feeding
features back to Emerge-Lab/PufferDrive. This commit carries everything that
exists only in the fork, and is the single place fork-only content lives.

Branch model:
  3.0              verbatim, fast-forward-only mirror of upstream/3.0
  integration/3.0  default branch; this commit plus a linear stack of
                   single-commit features; what internal consumers pin
  feature/<slug>   one feature, exactly one commit
  mirror/<slug>    cherry-picked onto pure upstream/3.0 for upstream PRs

integration/3.0 is the default branch because GitHub reads the PR template,
the contributing guidelines, the agent instructions and scheduled workflows
only from the default branch, and none of those files may exist on 3.0
without destroying the mirror. A clone checks out the default branch, so the
copy of the instructions here is the process as far as every agent and every
submodule consumer is concerned; a rule that lives only in an open PR has no
effect.

Nothing reaches integration/3.0 until three gates pass:
  1. CI green on the final commit.
  2. Adversarial review by someone other than the author, with the findings
     written down -- including the probes that found nothing.
  3. Upstream-readiness proven rather than asserted: the commit cherry-picks
     onto a branch cut from pure upstream/3.0 and produces an upstream-clean
     diff. We stack only commits we could contribute today.

chore(fork): commits are exempt from gate 3, since fork-only infrastructure
can never be upstream-ready. They carry two orthogonal markers: do-not-mirror
(must never reach upstream) and do-not-merge (the merge button is never
pressed -- merging would create a second chore(fork): commit and break the
single-commit invariant that every sync asserts). Such a PR is folded into
this commit by fixup and closed unmerged.

Contents:
- .github/copilot-instructions.md: normative process for coding agents
- CONTRIBUTING.md: human orientation, same rules
- .github/pull_request_template.md: testing and stack-hygiene gates
- .github/workflows/sync-mirror.yml: fast-forward-only mirror sync that
  refuses any non-fast-forward instead of rewriting the upstream base
- AGENTS.md / CLAUDE.md: fenced pointer so agents find the fork process
  without colliding with upstream-owned content
- ci.yml / install.yml: run CI for PRs targeting integration/3.0, which
  upstream's branch filters would otherwise skip

A standing draft PR from integration/3.0 into 3.0 gives a continuous CI
verdict on the combined stack; it must never be merged.

Everything here is fork-only and must never be cherry-picked upstream.
The Linux CI jobs fail while installing the package:

    [Errno 122] Disk quota exceeded

Errno 122 is EDQUOT, hit while unpacking the CUDA torch stack that
`pip install -e .` resolves by default. torch alone is 554.6 MB and the
accompanying nvidia-* wheels add another ~2.4 GB, which no longer fits
the runner's quota. The failure occurs during "Installing build
dependencies", i.e. the isolated PEP 517 build environment is already
enough to exceed it on its own.

The trigger is a runner image change, not a code change:

  * upstream 11_bernhard_dev, 2026-10-20, Image: ubuntu-24.04 -> success
  * same workflows, 2026-10-23, Image: ubuntu-26.04 -> failure

Both resolve an identical wheel set (torch 2.14.1, 554.6 MB). The
`ubuntu-latest` label began migrating to Ubuntu 26 on 2026-10-19
(actions/runner-images#14748), between those two runs. Both workflows
already carry "Free up disk space" steps, so these jobs were running
close to the limit before the migration.

Fix: install CPU-only torch from download.pytorch.org, then build with
--no-build-isolation so the build reuses it instead of resolving torch
a second time. --no-build-isolation needs the build dependencies present
up front, hence the explicit setuptools/wheel/numpy install; the
`setuptools<81` bound matches the pin already in install_requires.

This loses no coverage: BUILD_CUDA_EXT keys off CUDA_HOME/ROCM_HOME,
which is unset on these runners, so the CUDA sources were never compiled
here. tests/smoke_tests/Dockerfile already installs torch this way.

macOS keeps the plain `pip install -e .` unchanged: its PyPI wheels are
already CPU-only, and leaving it alone keeps the default PEP 517
build-isolation path under test somewhere in the matrix.
The Makefiles were the only part of the dev workflow that required a POSIX
shell. The targets themselves were portable; their recipe bodies were not --
mkdir -p, rm -rf, command -v, for...done and $(MAKE) -C meant no test suite
could be driven from a Windows checkout.

Move the task definitions into a justfile and the logic they invoked into
scripts/devtasks.py (stdlib only, so CI needs no extra dependency). Every
recipe body is now a single command with no shell operators, so it behaves
identically under sh, PowerShell and cmd.

just is a command runner, not a build system, so the prerequisite graph Make
provided is gone. The staleness checks are reimplemented in devtasks.py with
the same mtime semantics: the C extension and each C test binary are rebuilt
only when one of their sources or headers is newer. A warm `just test-c` still
compiles nothing.

tests/drive/Makefile is folded into the same helper; no caller used its unit
and smoke targets separately. The root Makefile is kept as a thin shim that
forwards to just, so `make test` and `make PYTHON=/path/to/python test` keep
working.

The C suite now runs natively on Windows. test_error and test_drive_map_cache
fork() a child to assert on abort paths, which Windows has no equivalent for;
they are skipped there behind an explicit notice so a native run cannot be
mistaken for a full pass. The Python suites still require WSL, Linux or macOS
because setup.py emits GCC/Clang-only flags and drive.h needs unistd.h. README
states that limit instead of leaving it to be discovered by failure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant