Improve CI/CD #101
Merged
Improve CI/CD #101
Conversation
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
This replaces 'find' with 'git ls-files' to avoid checking source files from submodules like portaudio, mini-gdbstub, and minislirp. Root cause: The find command recursively searches the entire directory tree, including initialized submodules. This causes CI failures when third-party code does not match the project's formatting standards.
This adds explicit permissions block with contents:read to follow the principle of least privilege. By default, GitHub Actions tokens have broad write access to the repository, which poses unnecessary security risks.
Implement two-layer caching strategy to significantly reduce CI runtime:
1. External downloads cache (Image, rootfs.cpio):
- Caches prebuilt Linux kernel (~72MB) and initrd
- Key: external-${{ hashFiles('mk/external.mk') }}
- Invalidates only when SHA1 checksums change
- Shared across all OS platforms
2. Submodule builds cache (libgdbstub.a, libslirp.a):
- Caches compiled third-party libraries
- Key: ${{ runner.os }}-submodules-${{ hashFiles(...) }}
- Per-platform caching for binary compatibility
- Invalidates when submodule commits change
1. Create shared '.ci/common.sh':
- Extracted common functions: cleanup, ASSERT, print_success/error
- Centralized platform detection and timeout configuration
- Added bash strict mode (set -euo pipefail)
- Registered trap for guaranteed cleanup (EXIT INT TERM)
2. Simplify format checker (check-format.sh):
- Use clang-format -n --Werror for direct validation
- Eliminate temporary file generation (expected-format)
3. Create composite action ('.github/actions/setup-semu'):
- Centralize dependency installation logic
- Support both Linux (apt) and macOS (brew)
- Apply optimizations: DEBIAN_FRONTEND=noninteractive,
HOMEBREW_NO_AUTO_UPDATE
This enhances the coding_style job to provide inline pull request
comments for formatting violations using reviewdog. Workflow behavior:
- On push: Only runs check-format.sh (strict validation)
- On PR: Runs reviewdog (inline suggestions) +
check-format.sh (validation)
Benefits:
- Developers see exact formatting issues inline in PR
- Faster iteration on style fixes
- Non-blocking suggestions with blocking validation
This properly quotes all variable expansions and uses bash arrays for
file lists to prevent word splitting issues:
- Use mapfile to populate SOURCES array instead of command substitution
- Quote all command substitutions and variable assignments
- Use "${SOURCES[@]}" for proper array expansion
- Quote exit codes and function parameters
This replaces '==' with '=' in bash test expressions for POSIX compliance. While '==' works in bash, '=' is the POSIX standard and ensures better portability across different shells.
This adds quotes around all integer variable usages to follow defensive
programming best practices:
- Quote variable assignments: ret="$?"
- Quote in numerical comparisons: [ "$ret" -eq 0 ]
- Quote in array indexing: ${MESSAGES["$ret"]}
- Quote in exit statements: exit "$ret"
While unquoted variables work in numeric contexts, quoting ensures:
1. Consistent coding style across the codebase
2. Protection against unexpected word splitting
3. Better shellcheck compliance
4. Clearer intent that these are variable expansions
Implement workflow-level concurrency control to optimize CI resource usage: - Group by workflow name and git ref (branch/tag) - Cancel in-progress runs when new commits are pushed - Prevents multiple CI runs for the same branch running simultaneously This addresses the issue where rapid pushes to the same branch would spawn multiple workflow instances, wasting GitHub Actions runner time and delaying feedback on the latest commit. Example behavior: - Push A triggers workflow run 1 - Push B to same branch triggers workflow run 2 - Run 1 is automatically cancelled, run 2 proceeds Concurrency group format: "CI-refs/heads/branch-name"
This adds timeout-minutes to all workflow steps to prevent individual steps from hanging and consuming runner resources for extended periods (default job timeout is 6 hours). This prevents scenarios where a hung step blocks the runner for hours, improving overall CI responsiveness and resource utilization.
This commit implements APT package caching for Linux jobs to avoid re-downloading packages on every workflow run, reducing network usage and improving CI performance.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 6 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name=".github/workflows/main.yml">
<violation number="1" location=".github/workflows/main.yml:9">
Limiting the workflow token to contents: read revokes the actions permission required by actions/cache, so every cache step will now fail with “Resource not accessible by integration.” Please add actions: write (or at least actions: read) alongside contents: read.</violation>
</file>
<file name=".ci/common.sh">
<violation number="1" location=".ci/common.sh:72">
Wrap the message argument in a `%s` placeholder so user-provided percent signs are printed literally and cannot break the format string.</violation>
<violation number="2" location=".ci/common.sh:77">
Use a `%s` placeholder when printing the error message so percent signs in the input are handled safely and the output remains correct.</violation>
</file>
<file name=".ci/check-format.sh">
<violation number="1" location=".ci/check-format.sh:6">
`mapfile` is unavailable in the default macOS bash (3.2), so this script now fails on macOS hosts.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
This commit fixes the CI timeout issue where netdev tests would hang for 10 minutes and fail. The root cause was the ASSERT function capturing command output, which broke expect's TTY interaction. The original implementation executed commands directly without capture, which is essential for interactive commands like expect. Capturing output broke the TTY connection that expect requires.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 by cubic
Speeds up and hardens CI/CD with caching, timeouts, least-privilege tokens, and unified CI scripts. Adds inline formatting feedback while enforcing stricter style checks.
Refactors
Dependencies