Migrate ./build to a Makefile and add smoke tests#70
Merged
Conversation
- Replace ./build with a Makefile exposing help/build/test/push/clean targets. Default target is help. - Add tests/ with four hello-world sources (Java Grinder, BasTo6809, mcbasic, CMOC OS-9). `make test` mounts them read-only into a freshly-built container, runs each toolchain, and verifies outputs exist. - Update README to use `make build` instead of `./build`. - Update build-docker.yml workflow: checkout, build with load:true, run smoke tests, then push image only on tag pushes (build-push-action v6 doesn't allow load+push together). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- .dockerignore: excludes everything not needed by the build
(Dockerfile only consumes utils/basto6809todsk from the context).
- tests/ reorganized into per-tool subfolders:
tests/java_grinder/, tests/basto6809/, tests/mcbasic/, tests/cmoc-os9/
- Makefile additions:
make shell one-off bash shell in the image
make lint shellcheck via koalaman/shellcheck-alpine
make size prints image size in MB
- Fix shellcheck issues in coco-dev and utils/basto6809todsk:
- Use "$@" array instead of "$*" so args with spaces are preserved
- Quote variable expansions
- Replace legacy backticks with $(...)
- Add `set -eu` to basto6809todsk
- /bin/env -> /usr/bin/env shebang (POSIX)
- CI: add a fast `lint` job and an Image size step after smoke tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pushing to a branch with an open PR was firing both `push` and `pull_request: synchronize` — double the CI minutes for the same commit. Drop the pull_request trigger; push-trigger covers every same-repo branch. Concurrency group cancels superseded runs when multiple commits land in quick succession. Tradeoff: PRs from forks no longer auto-trigger CI (they wouldn't have had access to DOCKERHUB_TOKEN anyway). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both matrix jobs (ubuntu-24.04 and ubuntu-24.04-arm) were writing to the default GHA cache slot in parallel; the second to start would race and fail with `error writing layer blob: failed to reserve cache`. Giving each arch its own scope eliminates the collision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
./buildwith aMakefileexposinghelp(default),build,test,shell,lint,size,push, andcleantargets.tests/organized by tool (java_grinder/,basto6809/,mcbasic/,cmoc-os9/) with hello-world sources for each..dockerignoresodocker compose buildonly sends what the Dockerfile actually needs.make test,make size, andmake lintinto CI.coco-devandutils/basto6809todsk.Release notes
New developer commands
make build— builds the docker image (replaces./build).make test— runs four smoke tests inside the just-built container:javac+java_grinder+naken_asm→ CoCo Java binarybasto6809todsk→ CoCo.DSKfrom BASIC sourcemcbasic→ MC-10.c10from BASIC sourcecmoc --os9→ OS-9 module from C sourcemake shell— drops you into a one-off bash shell in the image.make lint— runs shellcheck againstcoco-devandutils/basto6809todskvia thekoalaman/shellcheck-alpineimage (no host install required).make size— prints the size of the built image (e.g.jamieleecho/coco-dev:0.79: 1271.08 MB).make push— pushes the image to Docker Hub.make clean— removes the local image.make test TAG=jamieleecho/coco-dev:0.79.Bug fixes (shellcheck)
coco-dev: arguments with spaces are now preserved correctly. Previouslycoco-dev cmd "foo bar"would splitfoo barinto two separate arguments inside the container because the script joined args via$*and re-split on IFS. Switched to"$@"and quoted array expansions.utils/basto6809todsk: quoted all variable expansions, replaced legacy backticks with$(...), addedset -eu, fixed shebang to/usr/bin/env bash.CI
lintjob runsmake linton every PR/push (fast, single ubuntu runner).build_and_push_image: now runsmake testandmake sizeafter each per-arch build, before pushing.docker/build-push-action@v6doesn't allowload: trueandpush: truetogether, so the workflow now builds withload: true(image available locally for smoke tests), then a separatedocker pushstep runs on tag pushes.Image
.dockerignoreexcludes.git,.github,tests,Makefile,README.md, etc. from the build context. Fasterdocker compose buildinvocations and avoids accidentally shipping dev files.Test plan
make helpprints target list with computed tag frompackage.json.make buildsucceeds with the new.dockerignore.make testpasses locally (all 4 smoke tests).make lintpasses (no shellcheck warnings).make sizeprints1271.08 MBfor the locally-built image.🤖 Generated with Claude Code