ci(freebsd): replace Cirrus with GitHub Actions FreeBSD matrix#138
ci(freebsd): replace Cirrus with GitHub Actions FreeBSD matrix#138
Conversation
✅ Deploy Preview for termisu canceled.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughMigrates FreeBSD CI from Cirrus to GitHub Actions: deletes Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions"
participant VM as "FreeBSD VM\n(15.0)"
participant Mise as "mise (installer)"
participant Cache as "Actions Cache / Artifacts"
GH->>VM: Start job (matrix: crystal / shell)
VM->>Mise: Install mise script
Mise-->>VM: Install selected Crystal version
VM->>Cache: Restore caches (.cache/mise, lib, e2e/node_modules)
VM->>VM: Install system packages, run shards/npm install
VM->>VM: Build binaries / run specs / run e2e tests
VM->>Cache: Upload artifacts (binaries, traces, snapshots) on completion/failure
Cache-->>GH: Store artifacts and caches
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Migrates FreeBSD CI coverage from Cirrus to GitHub Actions by running FreeBSD 15.0 jobs via a VM action, expanding the Crystal version matrix across relevant workflows.
Changes:
- Add FreeBSD 15.0 test job to the main test workflow (Crystal 1.17.0/1.18.2/1.19.1).
- Add FreeBSD 15.0 C ABI checks and expand Crystal matrix in FFI bindings workflow.
- Add FreeBSD 15.0 build + E2E workflows (artifact build/upload, then per-shell E2E runs), and remove Cirrus config.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| .github/workflows/test.yml | Adds a FreeBSD VM-based test matrix job. |
| .github/workflows/ffi-bindings.yml | Expands Crystal matrix and adds FreeBSD VM-based C ABI job. |
| .github/workflows/e2e.yml | Adds FreeBSD VM build job + FreeBSD E2E matrix consuming uploaded artifacts. |
| .cirrus.star | Removes Cirrus FreeBSD task definition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path: | | ||
| .cache/mise | ||
| lib | ||
| key: freebsd-test-${{ matrix.crystal }}-${{ hashFiles('shard.lock') }} |
There was a problem hiding this comment.
Cache key uses hashFiles('shard.lock'), but this repository does not have a shard.lock file. hashFiles will evaluate to an empty string, so the cache key won’t change when shard dependencies change and may restore stale libs. Consider hashing shard.yml (or hashing both shard.yml and shard.lock, if shard.lock is intended to be added) so the cache invalidates correctly.
| key: freebsd-test-${{ matrix.crystal }}-${{ hashFiles('shard.lock') }} | |
| key: freebsd-test-${{ matrix.crystal }}-${{ hashFiles('shard.yml', 'shard.lock') }} |
| export MISE_CACHE_DIR="$PWD/.cache/mise/cache" | ||
| export MISE_CONFIG_DIR="$PWD/.cache/mise/config" | ||
| mkdir -p "$MISE_DATA_DIR" "$MISE_CACHE_DIR" "$MISE_CONFIG_DIR" | ||
| curl https://mise.run | sh |
There was a problem hiding this comment.
Installing mise via curl https://mise.run | sh is a supply-chain risk and also makes the CI environment non-reproducible (it always installs whatever the latest installer serves). Prefer installing a pinned mise version from a trusted package source or downloading a specific release artifact and verifying its checksum/signature.
| curl https://mise.run | sh | |
| MISE_VERSION="2024.12.14" | |
| MISE_INSTALLER_SHA256="REPLACE_WITH_REVIEWED_INSTALLER_SHA256" | |
| INSTALLER="$(mktemp)" | |
| curl -fsSL https://mise.run -o "$INSTALLER" | |
| test "$(sha256 -q "$INSTALLER")" = "$MISE_INSTALLER_SHA256" | |
| env MISE_VERSION="$MISE_VERSION" sh "$INSTALLER" | |
| rm -f "$INSTALLER" |
| path: | | ||
| .cache/mise | ||
| lib | ||
| key: freebsd-c-abi-${{ matrix.crystal }}-${{ hashFiles('shard.lock') }} |
There was a problem hiding this comment.
Cache key uses hashFiles('shard.lock'), but shard.lock is not present in the repository. This makes the cache key effectively constant (aside from matrix.crystal), so it may restore stale shard installs after dependency changes. Hash shard.yml (or both shard.yml and shard.lock if shard.lock is intended) to invalidate the cache correctly.
| key: freebsd-c-abi-${{ matrix.crystal }}-${{ hashFiles('shard.lock') }} | |
| key: freebsd-c-abi-${{ matrix.crystal }}-${{ hashFiles('shard.yml') }} |
| set -eux | ||
| export TERM=xterm-256color | ||
| export MISE_DATA_DIR="$PWD/.cache/mise/data" | ||
| export MISE_CACHE_DIR="$PWD/.cache/mise/cache" | ||
| export MISE_CONFIG_DIR="$PWD/.cache/mise/config" | ||
| mkdir -p "$MISE_DATA_DIR" "$MISE_CACHE_DIR" "$MISE_CONFIG_DIR" | ||
| curl https://mise.run | sh | ||
| export PATH="$HOME/.local/bin:$PATH" | ||
| mise install crystal@${{ matrix.crystal }} |
There was a problem hiding this comment.
The workflow installs mise by piping curl output into sh. This is both non-reproducible and a supply-chain risk for CI. Prefer installing a pinned mise version (e.g., from pkg if available) or fetching a specific release and verifying integrity before execution.
| path: | | ||
| .cache/mise | ||
| lib | ||
| key: freebsd-e2e-build-${{ matrix.crystal }}-${{ hashFiles('shard.lock') }} |
There was a problem hiding this comment.
Cache key uses hashFiles('shard.lock'), but the repository doesn’t contain shard.lock. That means the key won’t reflect Crystal dependency changes and may restore stale lib/ contents. Use shard.yml (or both shard.yml and shard.lock, if shard.lock is intended to be checked in) for cache invalidation.
| key: freebsd-e2e-build-${{ matrix.crystal }}-${{ hashFiles('shard.lock') }} | |
| key: freebsd-e2e-build-${{ matrix.crystal }}-${{ hashFiles('shard.yml') }} |
| pkg install -y curl git terminfo-db | ||
| run: | | ||
| set -eux | ||
| export MISE_DATA_DIR="$PWD/.cache/mise/data" | ||
| export MISE_CACHE_DIR="$PWD/.cache/mise/cache" | ||
| export MISE_CONFIG_DIR="$PWD/.cache/mise/config" | ||
| mkdir -p "$MISE_DATA_DIR" "$MISE_CACHE_DIR" "$MISE_CONFIG_DIR" bin | ||
| curl https://mise.run | sh | ||
| export PATH="$HOME/.local/bin:$PATH" |
There was a problem hiding this comment.
Installing mise via curl https://mise.run | sh makes CI dependent on an unpinned remote installer and bypasses integrity verification. Consider pinning mise to a specific version and verifying the downloaded artifact (or installing from a package repository) to reduce supply-chain risk and improve reproducibility.
| pkg install -y curl git terminfo-db | |
| run: | | |
| set -eux | |
| export MISE_DATA_DIR="$PWD/.cache/mise/data" | |
| export MISE_CACHE_DIR="$PWD/.cache/mise/cache" | |
| export MISE_CONFIG_DIR="$PWD/.cache/mise/config" | |
| mkdir -p "$MISE_DATA_DIR" "$MISE_CACHE_DIR" "$MISE_CONFIG_DIR" bin | |
| curl https://mise.run | sh | |
| export PATH="$HOME/.local/bin:$PATH" | |
| pkg install -y curl git terminfo-db mise | |
| run: | | |
| set -eux | |
| export MISE_DATA_DIR="$PWD/.cache/mise/data" | |
| export MISE_CACHE_DIR="$PWD/.cache/mise/cache" | |
| export MISE_CONFIG_DIR="$PWD/.cache/mise/config" | |
| mkdir -p "$MISE_DATA_DIR" "$MISE_CACHE_DIR" "$MISE_CONFIG_DIR" bin |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/test.yml (1)
48-50: Minor:checks: writepermission may not be usable from FreeBSD VM.The job declares
checks: writepermission (line 49), matching the existing Linux/macOS test job. However, since tests run inside a FreeBSD VM that doesn't have access to GitHub's OIDC token, any check annotations from the spec runner may not be published. This isn't a functional issue—just noting that the permission may be ineffective for VM-based jobs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 48 - 50, The workflow currently requests "checks: write" under the permissions block but FreeBSD VM jobs can’t publish check annotations due to lack of OIDC/token access; update the permissions block (the permissions and checks keys) to either remove the "checks: write" line or change it to "checks: read" (or "checks: none") so the workflow doesn't request an unusable write permission from the FreeBSD VM..github/workflows/e2e.yml (1)
185-192: Consider addinggitto package list for consistency.The
build-freebsdjob installsgit(line 74), bute2e-freebsdomits it. While likely not needed for E2E test execution, adding it would ensure consistency if any test tooling requires git operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/e2e.yml around lines 185 - 192, Add git to the FreeBSD package install step in the e2e job: update the pkg install line in the e2e-freebsd job (the block that runs "pkg install -y terminfo-db node npm bash zsh fish python3 gmake") to include git so tools invoked later (e.g. TUI_TEST_SHELL-driven npm --prefix e2e run test) have git available; mirror the addition made in the build-freebsd job to keep consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e.yml:
- Around line 173-179: The "Cache FreeBSD E2E dependencies" step using
actions/cache@v5 to cache e2e/node_modules is ineffective because the cache runs
on the Ubuntu host while the FreeBSD VM installs with npm --prefix e2e (native
binaries differ); remove this cache step (the block referencing actions/cache@v5
and the key freebsd-e2e-node-${{ hashFiles('e2e/package.json') }}) from the
workflow, or alternatively move an equivalent caching mechanism inside the
FreeBSD VM (or replace with a clarifying comment) so that caching only stores/
restores node_modules built for FreeBSD.
In @.github/workflows/ffi-bindings.yml:
- Around line 129-131: The ffi:build task currently hardcodes Linux linker flags
(e.g., -ldl) which breaks FreeBSD; update the Hacefile.yml ffi:build recipe (or
replace the job in the FreeBSD workflow to call .github/actions/build-ffi) to
set platform-specific LDFLAGS: detect the OS via uname -s (or use the same
platform branch logic from .github/actions/build-ffi/action.yml) and append -ldl
only on Linux, leave it out for FreeBSD and macOS, then use that LDFLAGS when
linking in the ffi:build command so the build succeeds across platforms.
---
Nitpick comments:
In @.github/workflows/e2e.yml:
- Around line 185-192: Add git to the FreeBSD package install step in the e2e
job: update the pkg install line in the e2e-freebsd job (the block that runs
"pkg install -y terminfo-db node npm bash zsh fish python3 gmake") to include
git so tools invoked later (e.g. TUI_TEST_SHELL-driven npm --prefix e2e run
test) have git available; mirror the addition made in the build-freebsd job to
keep consistency.
In @.github/workflows/test.yml:
- Around line 48-50: The workflow currently requests "checks: write" under the
permissions block but FreeBSD VM jobs can’t publish check annotations due to
lack of OIDC/token access; update the permissions block (the permissions and
checks keys) to either remove the "checks: write" line or change it to "checks:
read" (or "checks: none") so the workflow doesn't request an unusable write
permission from the FreeBSD VM.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c6184fec-0046-4390-b3fe-4a5ba9bdc7c0
📒 Files selected for processing (4)
.cirrus.star.github/workflows/e2e.yml.github/workflows/ffi-bindings.yml.github/workflows/test.yml
💤 Files with no reviewable changes (1)
- .cirrus.star
| - name: Cache FreeBSD E2E dependencies | ||
| uses: actions/cache@v5 | ||
| with: | ||
| path: e2e/node_modules | ||
| key: freebsd-e2e-node-${{ hashFiles('e2e/package.json') }} | ||
| restore-keys: | | ||
| freebsd-e2e-node- |
There was a problem hiding this comment.
Cache for e2e/node_modules may not work as expected with VM-based execution.
The cache action runs on the Ubuntu host before the FreeBSD VM starts, but npm --prefix e2e install runs inside the VM. Since FreeBSD and Linux have different native module binaries, the cached node_modules from a previous Linux run (or even a previous FreeBSD run that was stored on the host) won't be usable inside the FreeBSD VM.
Consider moving the cache inside the VM or removing this cache step, as it provides no benefit and could cause confusion.
Suggested fix: Remove ineffective cache or add clarifying comment
- - name: Cache FreeBSD E2E dependencies
- uses: actions/cache@v5
- with:
- path: e2e/node_modules
- key: freebsd-e2e-node-${{ hashFiles('e2e/package.json') }}
- restore-keys: |
- freebsd-e2e-node-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Cache FreeBSD E2E dependencies | |
| uses: actions/cache@v5 | |
| with: | |
| path: e2e/node_modules | |
| key: freebsd-e2e-node-${{ hashFiles('e2e/package.json') }} | |
| restore-keys: | | |
| freebsd-e2e-node- |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e.yml around lines 173 - 179, The "Cache FreeBSD E2E
dependencies" step using actions/cache@v5 to cache e2e/node_modules is
ineffective because the cache runs on the Ubuntu host while the FreeBSD VM
installs with npm --prefix e2e (native binaries differ); remove this cache step
(the block referencing actions/cache@v5 and the key freebsd-e2e-node-${{
hashFiles('e2e/package.json') }}) from the workflow, or alternatively move an
equivalent caching mechanism inside the FreeBSD VM (or replace with a clarifying
comment) so that caching only stores/ restores node_modules built for FreeBSD.
.github/workflows/ffi-bindings.yml
Outdated
| bin/hace ffi:build | ||
| cc tests/c/ffi_test.c -Iinclude -Lbin -ltermisu -o bin/termisu-c-test | ||
| LD_LIBRARY_PATH="$PWD/bin" unbuffer ./bin/termisu-c-test |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Hacefile.yml for ffi:build task and any platform conditionals
cat Hacefile.yml | grep -A 20 "ffi:build"
# Check if there are any FreeBSD-specific build instructions
rg -i "freebsd|bsd" Hacefile.yml
# Look for any platform detection in the build process
rg -i "platform|uname|OSTYPE" Hacefile.ymlRepository: omarluq/termisu
Length of output: 2047
🏁 Script executed:
cat .github/workflows/ffi-bindings.ymlRepository: omarluq/termisu
Length of output: 6240
🏁 Script executed:
rg -i "freebsd|bsd" .github/workflows/ffi-bindings.ymlRepository: omarluq/termisu
Length of output: 502
🏁 Script executed:
# Check if there are conditional builds or platform-specific steps
rg -B5 -A5 "ffi:build" .github/workflows/ffi-bindings.ymlRepository: omarluq/termisu
Length of output: 806
🏁 Script executed:
# Check for any platform conditionals in Hacefile around ffi:build
rg -B10 -A10 "ffi:build:" Hacefile.ymlRepository: omarluq/termisu
Length of output: 805
🏁 Script executed:
# Check if Hacefile has any conditional logic or variable expansion for platform
rg "{{|%|if|else|freebsd|darwin|linux" Hacefile.yml | head -30Repository: omarluq/termisu
Length of output: 167
🏁 Script executed:
# Check if FFI_LIB variable is set differently per platform elsewhere
rg -i "FFI_LIB\|ldl" Hacefile.ymlRepository: omarluq/termisu
Length of output: 41
🏁 Script executed:
# Check the .github/actions/build-ffi to see if it handles platform-specific flags
cat .github/actions/build-ffi/action.yml 2>/dev/null || echo "File not found"Repository: omarluq/termisu
Length of output: 1381
🏁 Script executed:
# Search entire codebase for any FreeBSD-specific build handling
rg -i "freebsd|libdl" . --type-list | head -20Repository: omarluq/termisu
Length of output: 448
🏁 Script executed:
# Look for variable definitions and FFI_LIB setup in Hacefile
rg "FFI_LIB|variables|vars:" Hacefile.yml -B2 -A2Repository: omarluq/termisu
Length of output: 547
🏁 Script executed:
# Check if Hacefile has any environment or platform detection at the top
head -50 Hacefile.ymlRepository: omarluq/termisu
Length of output: 1396
🏁 Script executed:
# Search for any references to FreeBSD or platform handling in the entire repo
rg -i "freebsd" . --type-listRepository: omarluq/termisu
Length of output: 448
🏁 Script executed:
# Correct search for FreeBSD references
rg -i "freebsd"Repository: omarluq/termisu
Length of output: 1731
Platform-specific linker flags needed in ffi:build task for FreeBSD compatibility.
The ffi:build task in Hacefile.yml uses -ldl without platform conditionals, but this flag is Linux-specific and will fail on FreeBSD where dlopen() functions are part of libc. The .github/actions/build-ffi action already correctly handles this with platform detection (Linux uses -ldl, macOS doesn't), but the FreeBSD workflow job runs bin/hace ffi:build directly from the Hacefile instead of using the action.
Update the ffi:build task to add platform-specific linker flags, similar to the pattern in .github/actions/build-ffi/action.yml:
ffi:build:
description: Build shared library exposing the Termisu C ABI
phony: true
always_run: true
commands: |
mkdir -p bin
crystal build src/termisu.cr --release --cross-compile -o bin/libtermisu
- cc -shared bin/libtermisu.o -o {{FFI_LIB}} -L$(crystal env CRYSTAL_LIBRARY_PATH) -lgc -lpthread -ldl
+ if [[ "$OSTYPE" == "freebsd"* ]]; then
+ cc -shared bin/libtermisu.o -o {{FFI_LIB}} -L$(crystal env CRYSTAL_LIBRARY_PATH) -lgc -lpthread
+ else
+ cc -shared bin/libtermisu.o -o {{FFI_LIB}} -L$(crystal env CRYSTAL_LIBRARY_PATH) -lgc -lpthread -ldl
+ fiAlternatively, use the existing .github/actions/build-ffi action in the FreeBSD job instead of calling Hacefile directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ffi-bindings.yml around lines 129 - 131, The ffi:build
task currently hardcodes Linux linker flags (e.g., -ldl) which breaks FreeBSD;
update the Hacefile.yml ffi:build recipe (or replace the job in the FreeBSD
workflow to call .github/actions/build-ffi) to set platform-specific LDFLAGS:
detect the OS via uname -s (or use the same platform branch logic from
.github/actions/build-ffi/action.yml) and append -ldl only on Linux, leave it
out for FreeBSD and macOS, then use that LDFLAGS when linking in the ffi:build
command so the build succeeds across platforms.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ffi-bindings.yml (1)
123-126: Consider removing debug statements.These lines appear to be debugging artifacts. While harmless, they add noise to CI logs.
- pwd - ls - which mise - mise --version🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ffi-bindings.yml around lines 123 - 126, Remove the debugging commands left in the CI job (the pwd, ls, which mise and mise --version invocations) from the workflow step to reduce noisy logs; locate the block that runs these shell commands in the workflow job and delete those four lines or replace them with a single purposeful check (e.g., a meaningful installation/validation step) if verification is needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ffi-bindings.yml:
- Around line 123-126: Remove the debugging commands left in the CI job (the
pwd, ls, which mise and mise --version invocations) from the workflow step to
reduce noisy logs; locate the block that runs these shell commands in the
workflow job and delete those four lines or replace them with a single
purposeful check (e.g., a meaningful installation/validation step) if
verification is needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d3267dd-8d3d-4a0b-a27b-7ed27381e943
📒 Files selected for processing (3)
.github/workflows/e2e.yml.github/workflows/ffi-bindings.yml.github/workflows/test.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/test.yml
- .github/workflows/e2e.yml
No description provided.