CI: Align workflow with configure.ac (recursive submodules, v2 Linux job, drop x32)#3504
CI: Align workflow with configure.ac (recursive submodules, v2 Linux job, drop x32)#3504Easton97-Jens wants to merge 11 commits intoowasp-modsecurity:v3/masterfrom
Conversation
…lementation ci: install pcre1 dev package for explicit --with-pcre jobs
|
Hi @Easton97-Jens, many thanks for this PR. The only one thing that I've done that a bit reformatted your description for the better visibility. Please take a look at that, I hope I put lines (lists, headings) to the right place. If you think everything is okay, just let me know, or feel free to correct the description. I can merge this one soon. Beside of that, I like this PR. If everything will be okay, we can remove the existing one later - what do you think? |
|
You can go ahead and merge the PR. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new, parallel CI workflow (ci_new.yml) alongside the existing ci.yml. Its primary goals are: aligning the CI environment with what configure.ac requires (recursive submodule initialization), upgrading runner environments (Ubuntu 24.04, macOS 15, Windows 2025), dropping x32/i386 support, and adding a Linux-based static analysis job using a debian:sid container.
Changes:
- Adds a new
ci_new.ymlworkflow with updated runner versions (Ubuntu 24.04, macOS 15, Windows 2025) and explicit recursive submodule initialization. - Introduces a new
cppcheck-linuxstatic analysis job using adebian:sidcontainer. - Drops x32/i386 matrix entries from the Linux build.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/ci_new.yml
Outdated
| cppcheck-linux: | ||
| name: Static analysis (cppcheck, Linux, debian:sid) | ||
| runs-on: ubuntu-latest | ||
| container: debian:sid |
There was a problem hiding this comment.
debian:sid (unstable) is a rolling release that receives continuous package updates without stability guarantees. Using it as a CI container base image means the build environment can break at any time due to upstream package updates, library ABI changes, or temporary package unavailability — completely unrelated to any code changes in the repository. Consider using a pinned Debian release (e.g., debian:bookworm or debian:trixie) to get a stable, reproducible build environment.
| container: debian:sid | |
| container: debian:bookworm |
There was a problem hiding this comment.
Good idea, I've implemented it, although it then differs from v2/master.
.github/workflows/ci_new.yml
Outdated
| submodules: true | ||
|
|
||
| - name: Ensure submodules are fully initialized (recursive) | ||
| run: git submodule update --init --recursive | ||
|
|
There was a problem hiding this comment.
The checkout step specifies submodules: true, which performs a non-recursive shallow submodule checkout, and then a separate step runs git submodule update --init --recursive. While this is functional, using submodules: recursive directly in the checkout action (instead of submodules: true) would handle recursive initialization in one step and make the subsequent manual git submodule update --init --recursive step redundant. This applies to all five checkout instances in the file (lines 38–41, 103–106, 171–174, 223–226, 254–257).
| submodules: true | |
| - name: Ensure submodules are fully initialized (recursive) | |
| run: git submodule update --init --recursive | |
| submodules: recursive |
There was a problem hiding this comment.
Good idea, I checked the behavior and it behaves the same as the other one.
.github/workflows/ci_new.yml
Outdated
| liblua5.2-dev \ | ||
| libmaxminddb-dev \ | ||
| libpcre2-dev \ | ||
| libgeoip-dev \ |
There was a problem hiding this comment.
The libgeoip-dev package has been removed from Debian Sid (unstable). Debian dropped the legacy GeoIP C library (libgeoip-dev) as it is deprecated and unmaintained upstream. Attempting to install it on debian:sid will fail with a package-not-found error, breaking the cppcheck-linux job. This package should be removed from the dependency list for this job, which implicitly means GeoIP support won't be analyzed by cppcheck (it will be configured away).
| libgeoip-dev \ |
There was a problem hiding this comment.
It's removed, he's right about that, I also completely removed it on macOS.
.github/workflows/ci_new.yml
Outdated
| liblua5.2-dev \ | ||
| libmaxminddb-dev \ | ||
| libpcre2-dev \ | ||
| libpcre3-dev \ |
There was a problem hiding this comment.
libpcre3-dev was dropped from Ubuntu 24.04 (Noble Numbat). The package no longer exists in the Ubuntu 24.04 APT repositories, so this apt-get install step will fail with a package-not-found error for every matrix job in the Linux build. Since the new workflow targets ubuntu-24.04 (line 13), libpcre3-dev should be removed from the dependency list. The PCRE (v1) --with-pcre configure option can use the libpcre3-dev-equivalent package libpcre2-dev is already listed; for PCRE v1 support, note that Ubuntu 24.04 still provides libpcre3 (runtime) but no longer ships the -dev headers package. You may need to build or bundle PCRE v1 separately if PCRE v1 support is still needed, or conditionally install it only for the with pcre matrix variant.
| libpcre3-dev \ |
| - uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 0 | ||
| submodules: true | ||
|
|
||
| - name: Mark repo as safe for git | ||
| run: git config --global --add safe.directory $GITHUB_WORKSPACE | ||
|
|
There was a problem hiding this comment.
In the cppcheck-linux job running inside a debian:sid container, the git config --global --add safe.directory step is placed after the actions/checkout@v6 step. The checkout action itself internally invokes git operations within the workspace directory. In a container context where the workspace is owned by a different UID than the runner user, git may refuse to operate on it during checkout itself. The safe.directory configuration should be set before the checkout step — or alternatively, install git and run the safe.directory config before the checkout action runs.
| - uses: actions/checkout@v6 | |
| with: | |
| fetch-depth: 0 | |
| submodules: true | |
| - name: Mark repo as safe for git | |
| run: git config --global --add safe.directory $GITHUB_WORKSPACE | |
| - name: Mark repo as safe for git | |
| run: git config --global --add safe.directory "$GITHUB_WORKSPACE" | |
| - uses: actions/checkout@v6 | |
| with: | |
| fetch-depth: 0 | |
| submodules: true |
Updated CI workflow to use recursive submodules and changed container from debian:sid to debian:trixie. Removed GeoIP library build steps and unnecessary dependencies.
|
| name: Static analysis (cppcheck, Linux, debian:sid) | ||
| runs-on: ubuntu-latest | ||
| container: debian:sid | ||
| container: debian:trixie |
There was a problem hiding this comment.
trixie is the current stable release of Debian (Debian 13). It contains 2.17.1, see the packages site.
Sid contains 2.19.0, and here that's the point: we definitely want to use the last cppcheck version, as soon as possible.
I saw Copilot suggestion, but I'm afraid in this case you shouldn't follow it :).
.github/workflows/ci_new.yml
Outdated
| - { label: "without ssdeep", opt: "--without-ssdeep" } | ||
| - { label: "with lmdb", opt: "--with-lmdb" } | ||
| - { label: "with pcre2 (default)", opt: "" } | ||
| - { label: "with pcre", opt: "--with-pcre" } |
There was a problem hiding this comment.
You removed the old PCRE library's check, but I think while the used OS supports PCRE3, we must check that too.
It seems you use ubuntu24.04, which still contains pcre3, so please keep this case too.
(Unfortunately we don't have any information about how many uses uses the old and the new PCRE lib)
.github/workflows/ci_new.yml
Outdated
| liblua5.2-dev \ | ||
| libmaxminddb-dev \ | ||
| libpcre2-dev \ | ||
| libgeoip-dev \ |
There was a problem hiding this comment.
libgeoip-dev was removed, but there is not a replacement, but a new library which can be used: maxminddb. I see that's also installed (two lines above), but for some reason the configure script does not recongise it. You can see what happens during the job (check the output, for eg. like this one).
Also, if you want to use LMDB, then you should pass that to configure script: --with-lmdb. Beside of that, you could try to pass --with-maxmind too.
Summary
This PR introduces a new separate CI workflow (
ci_new.yml).The existing workflow remains unchanged.
The primary goal is to align CI behavior with the expectations defined in configure.ac, while modernizing the Linux environment and simplifying architecture handling.
1. Alignment with
configure.acThe build system assumes that required components provided via git submodules are fully and recursively initialized.
configure.acexplicitly documents this expectation (e.g., requiring recursive submodule initialization for bundled modules).To ensure CI matches this requirement, the new workflow:
Upgrades actions/checkout from v4 to v6
Explicitly runs:
This guarantees:
This is the primary structural change introduced by this workflow.
2. Integration of v2 Linux Static Analysis Flow
The workflow adds a dedicated Linux
cppcheckjob using adebian:sidcontainer.This structure is derived from the previous v2 CI logic and restores Linux-based static analysis in addition to the macOS job.
Benefits:
3. Removal of x32 (i386)
The new workflow drops 32-bit (x32/i386) builds:
Ubuntu upgraded from 22.04 to 24.04
Ubuntu 24.04 no longer provides native i386 runner support
Removes multilib setup and architecture-specific dependency branches
Simplifies the matrix and dependency logic
This reduces CI complexity and aligns the build matrix with current runner support.
Additional Updates
macOS 14 → macOS 15
Windows 2022 → Windows 2025
Minor matrix cleanup and clearer job structure
Rationale for Separate Workflow
The existing workflow is intentionally preserved to: