ci: split jobs and add release workflow#95
Conversation
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| lint: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Install clang-tidy | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y clang-tidy | ||
| - name: Configure | ||
| run: cmake --preset linux | ||
| - name: clang-tidy | ||
| run: run-clang-tidy -p build/linux |
There was a problem hiding this comment.
[P0] Install build dependencies before running lint job
The lint job configures the project with cmake --preset linux but the job only installs clang-tidy. The Linux preset uses Ninja and requires the same development packages (ninja, X11/ALSA/GTK headers, etc.) that the other jobs install; without those packages CMake fails to configure, so the job will never reach run-clang-tidy. Please install the prerequisite packages (or reuse the typecheck setup step) before invoking CMake.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Added a “Setup prerequisites” step to the lint job so it installs Ninja, X11/ALSA/GTK headers, and clang-tidy before running CMake .github/workflows/ci.ymlL13-L18
Testing
- ✅
./actionlint .github/workflows/ci.yml ⚠️ cmake --preset linux -DLIZARD_EMBED_ASSETS=ON(gtk+-3.0 not found)
.github/workflows/release.yml
Outdated
| include: | ||
| - runner: ubuntu-latest | ||
| os: linux | ||
| arch: x64 | ||
| preset: linux | ||
| artifact: LizardHook-linux-x64.tar.gz | ||
| ext: tar.gz | ||
| configure_args: "" | ||
| - runner: ubuntu-latest | ||
| os: linux | ||
| arch: arm64 | ||
| preset: linux | ||
| artifact: LizardHook-linux-arm64.tar.gz | ||
| ext: tar.gz | ||
| configure_args: "-DCMAKE_SYSTEM_PROCESSOR=aarch64" |
There was a problem hiding this comment.
[P1] Avoid publishing x64 build as linux arm64 artifact
The release matrix builds an "linux‑arm64" artifact on an ubuntu-latest runner and only sets CMAKE_SYSTEM_PROCESSOR=aarch64. That runner is x86‑64 and no cross toolchain is configured, so CMake still uses the host GCC and produces an x86‑64 binary while the archive is labelled LizardHook-linux-arm64.tar.gz. Users on ARM64 Linux will receive a binary that cannot execute. Use an ARM64 runner (e.g. ubuntu-24.04-arm64) or install a cross compiler and toolchain file before advertising an arm64 artifact. The same matrix in ci.yml has the same issue.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Route Linux ARM64 artifacts to a native ARM runner and drop the cross-compilation flag in the release workflow, ensuring the published
linux-arm64archive matches its platform .github/workflows/release.ymlL22-L28 - Broaden the Linux setup step in the release workflow to run on any Linux runner rather than only
ubuntu-latest.github/workflows/release.ymlL53-L57 - Mirror the ARM64 runner change in the CI build matrix and switch linkage checks to OS-based logic, so both architectures run with the proper prerequisites and validation .github/workflows/ci.ymlL70-L83 .github/workflows/ci.ymlL108-L112 .github/workflows/ci.ymlL147-L155
Testing
- ✅
./actionlint .github/workflows/release.yml - ✅
./actionlint .github/workflows/ci.yml ⚠️ cmake --preset linux -DLIZARD_EMBED_ASSETS=ON(gtk+-3.0 not found)
…5-09-12 ci: install prerequisites for lint job
|
@codex review |
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| lint: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Setup prerequisites | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y ninja-build xorg-dev libxi-dev libxrandr-dev libxinerama-dev libxcursor-dev libx11-dev libxext-dev libasound2-dev libgtk-3-dev clang-tidy | ||
| - name: Configure | ||
| run: cmake --preset linux | ||
| - name: clang-tidy | ||
| run: run-clang-tidy -p build/linux |
There was a problem hiding this comment.
[P1] Ensure lint job installs tools and compile commands for clang-tidy
The lint workflow invokes run-clang-tidy -p build/linux right after configuration, but the job only installs the clang-tidy package and never generates a compile_commands.json. On ubuntu runners run-clang-tidy is provided by the clang-tools package, so this step will fail before running any checks, and even if the script were present it would abort because no compilation database exists in build/linux. The job will therefore fail on every run. Consider installing clang-tools (or llvm) and enabling CMAKE_EXPORT_COMPILE_COMMANDS=ON or performing a build before calling run-clang-tidy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Ensure the lint workflow installs
clang-toolssorun-clang-tidyis available and generatecompile_commands.jsonby exporting compile commands during configuration .github/workflows/ci.ymlL13-L20
Testing
⚠️ actionlint .github/workflows/ci.yml(command not found: actionlint)⚠️ cmake --preset linux -DLIZARD_EMBED_ASSETS=ON(gtk+-3.0 not found)
.github/workflows/ci.yml
Outdated
| include: | ||
| - os: ubuntu-latest | ||
| - runner: ubuntu-latest | ||
| os: linux | ||
| arch: x64 | ||
| preset: linux | ||
| artifact: LizardHook-linux-x64.tar.gz | ||
| ext: tar.gz | ||
| - os: macos-latest | ||
| configure_args: "" | ||
| - runner: ubuntu-latest | ||
| os: linux | ||
| arch: arm64 | ||
| preset: linux | ||
| artifact: LizardHook-linux-arm64.tar.gz | ||
| ext: tar.gz | ||
| configure_args: "-DCMAKE_SYSTEM_PROCESSOR=aarch64" |
There was a problem hiding this comment.
[P1] Linux arm64 CI artifact is built with x86 toolchain
The build matrix adds a linux/arm64 entry that just passes -DCMAKE_SYSTEM_PROCESSOR=aarch64 while running on the standard ubuntu-latest runner. No cross‑compiler or alternate rootfs is provided, so CMake still uses the host x86_64 GCC/Ninja toolchain and produces an x86 binary that gets uploaded as LizardHook-linux-arm64.tar.gz. This job either fails when linkers reject the unsupported architecture or, worse, publishes an incorrectly labeled artifact that cannot run on arm64 machines. A dedicated arm64 runner or an aarch64 cross toolchain is required before advertising this artifact.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Install
clang-toolsalongsideclang-tidyand export compile commands in the lint job sorun-clang-tidycan locate a compilation database .github/workflows/ci.ymlL13-L18
Testing
- ✅
./actionlint .github/workflows/ci.yml ⚠️ cmake --preset linux -DLIZARD_EMBED_ASSETS=ON(terminated after generating GL bindings)
.github/workflows/release.yml
Outdated
| matrix: | ||
| include: | ||
| - runner: ubuntu-latest | ||
| os: linux | ||
| arch: x64 | ||
| preset: linux | ||
| artifact: LizardHook-linux-x64.tar.gz | ||
| ext: tar.gz | ||
| configure_args: "" | ||
| - runner: ubuntu-latest | ||
| os: linux | ||
| arch: arm64 | ||
| preset: linux | ||
| artifact: LizardHook-linux-arm64.tar.gz | ||
| ext: tar.gz | ||
| configure_args: "-DCMAKE_SYSTEM_PROCESSOR=aarch64" |
There was a problem hiding this comment.
[P1] Release workflow repeats arm64 misconfiguration
The release workflow rebuilds a linux-arm64 artifact on the same ubuntu-latest runner using only -DCMAKE_SYSTEM_PROCESSOR=aarch64. As in CI, no aarch64 toolchain or emulator is installed, so the build runs with the default x86_64 compiler and emits an x86 binary that is packaged and published as LizardHook-linux-arm64.tar.gz. Tagging a release with this workflow will ship a mislabelled binary that will not execute on real arm64 Linux systems. Provision an arm64 runner or install a proper cross compiler before generating this artifact.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Route Linux ARM64 builds in the release workflow to a native
ubuntu-24.04-armrunner and apply Linux prerequisites based on OS rather than a specific runner .github/workflows/release.ymlL22-L28 .github/workflows/release.ymlL53-L56 - Mirror the ARM runner change in CI, removing cross-compilation flags, applying Linux setup generically, and verifying linkage by OS instead of runner label .github/workflows/ci.ymlL70-L83 .github/workflows/ci.ymlL108-L112 .github/workflows/ci.ymlL147-L155
Testing
- ✅
./actionlint .github/workflows/release.yml - ✅
./actionlint .github/workflows/ci.yml ⚠️ cmake --preset linux -DLIZARD_EMBED_ASSETS=ON(terminated after generating GL bindings)
Notes
- The CMake configuration step did not complete; it stopped after generating OpenGL bindings, suggesting missing or misconfigured dependencies in the environment.
Summary
Testing
cmake --preset linux -DLIZARD_EMBED_ASSETS=ONcmake --build build/linux --config Release(fails: deprecated GTK and spdlog warnings)https://chatgpt.com/codex/tasks/task_e_68c315b501f883259ea50282dfde8732