build: make the tree compile on gcc 16, and add a CI lane so it stays that way - #945
Open
jimmykarily wants to merge 1 commit into
Open
build: make the tree compile on gcc 16, and add a CI lane so it stays that way#945jimmykarily wants to merge 1 commit into
jimmykarily wants to merge 1 commit into
Conversation
…it that way Five files call ::getpid() without including <unistd.h> and compiled only because an older libstdc++ pulled the header in transitively. gcc 16 does not, so they fail. One of them is src/vllm/entrypoints/openai/server_main.cpp, i.e. a SHIPPED binary that does not build on a current distro. This is the second time this exact break has landed. The first three files were fixed after a gfx1151 report on mudler#41; the pattern came back in five more, because every Linux CI lane installs the distro g++ (gcc 13 on ubuntu-latest) and no lane uses anything newer. A point fix does not hold, so this adds the missing coverage rather than only the includes: * build-newest-gcc: a COMPILE-ONLY lane in the gcc:16 container, tests configured ON so the test tree is compiled too. It deliberately does not run ctest -- build-test-cpu already owns execution, and this lane exists to catch compile-time portability for roughly one extra build per PR. * cmake/CompilerWarnings.cmake: on gcc >= 16 only, -Warray-bounds is reported but no longer fatal. gcc 16 emits it inside libstdc++ and vendored nlohmann for correct code, and no change to the calling code avoids it: after inlining, _Sp_counted_base::_M_release() is identical for every shared_ptr type, so GCC attributes one instantiation's destructor to another's allocation size. libstdc++ carries its own #pragma suppressions around that destructor; upstream is GCC PR tree-optimization/122197. gcc <= 15 is unchanged and still fails the build on a real out-of-bounds. * docs/USAGE.md: a Host compilers section stating which compilers build the tree and what the gcc 16 warning policy is. Required by check-doc-checkpoint, which classifies src/vllm/entrypoints/ as a user-facing surface and has no bypass by design. Verified on gcc 16.1.1 (Arch): full CPU configure + build with tests ON, no command-line workaround, 0 errors, and build-gcc16/examples/vllm-server runs (`vllm.cpp 0.0.3 c-abi=20`). check-doc-checkpoint, check-public-doc-tables, check-readme-structure and check-env-doc all pass. The workflow file itself is NOT verified -- there is no way to execute a GitHub Actions lane locally, so this PR's own CI is its first run. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode] Signed-off-by: Dimitris Karakasilis <dimitris@karakasilis.me>
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.
What this fixes
The project does not currently build with gcc 16, which is what current Linux distributions ship. Five files use
getpid()without including the header that declares it. They compiled anyway on older compilers, because some other standard header happened to include it for them. Newer compilers do not, so the build stops:One of those five is
src/vllm/entrypoints/openai/server_main.cpp. That is the OpenAI server's own entry point, so this is not just a test problem. A binary the project ships cannot be built on a current distribution.Why it keeps happening
This is the second time. Three files were fixed a week ago after a report on #41. The same mistake then appeared in five new files, because each new model-loader test copies the same helper.
The reason nobody notices is that CI cannot see it. There are eighteen CI jobs, including Windows and ARM, but every Linux job installs the distribution's default
g++, which onubuntu-latestis gcc 13. No lane uses anything newer. So the mistake is invisible to CI and to anyone developing on Ubuntu, and it only shows up for people on newer distributions.Fixing the five files again would not stop a third time. So this adds the missing coverage instead.
What is in this PR
1. The five missing includes. One line each.
2. A new CI job,
build-newest-gcc. It builds the project in the officialgcc:16container. It only compiles, it does not run the tests, because the existingbuild-test-cpujob already runs them and this one exists to catch compile problems. Cost is roughly one extra build per pull request. It follows the same pattern as the existing CUDA job, which also runs in a container.3. One warning is downgraded, on gcc 16 and newer only. Without this the new job cannot pass. gcc 16 reports an
array-boundsproblem inside the standard library and the bundled JSON library, on code that is correct. The warning still appears, it just no longer stops the build. On gcc 15 and older nothing changes at all, and a genuine out-of-bounds still fails the build there.Worth knowing about item 3: this is not something the code can be rewritten to avoid. The warning appears after the compiler merges the shared-pointer cleanup function, which is identical for every shared pointer type, and then blames one type's destructor for another type's size. The standard library itself silences this same warning in the same place. The upstream bug is
tree-optimization/122197, and Eigen, assimp and CMSSW all handle it the same way.4. A
Host compilerssection indocs/USAGE.md. Not originally planned.check-doc-checkpointclassifiessrc/vllm/entrypoints/as a user-facing surface and has no bypass by design, so rather than write a filler documentation update, this states the truthful user-facing fact: which compilers build the tree, and what the gcc 16 warning policy is.What I checked, and what I did not
Checked on gcc 16.1.1 (Arch Linux):
vllm.cpp 0.0.3 c-abi=20.check-doc-checkpoint,check-public-doc-tables,check-readme-structureandcheck-env-docall pass.Not checked: the CI file itself. There is no way to run a GitHub Actions job locally, so this pull request's own CI run will be the first time that file executes. If it needs adjusting, say so and I will fix it.
If you would prefer something different
Item 3 is a policy decision about the
-Werrorrule, so it is yours to make. Alternatives, and I am happy to switch to any of them:-Werroruntouched and mark the new job as non-blocking, so it reports the problem without failing the build.Note
This change was prepared with the help of Claude AI and reviewed by the contributor before submission.