[llhttp] Fix Build Issues & Coverage Failure #12933
Merged
+12
−12
Conversation
This file contains 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
Fixes several issues in the build which have been breaking the runtime code coverage collection. ### Key Changes: - Upgraded Node.js to the latest LTS release (v22.x) for compatibility with the upstream project's requirements. - Refined the build commands to prefer the upstream project's documentation. - Adjusted compiler flags to ensure only the executable fuzzing harness is placed in the `$OUT` directory so unnecessary build artifacts can't break anything in unexpected ways. ### Impact: - Resolves the broken coverage collection. - Marginally improves overall build times.
|
DaveLak is a new contributor to projects/llhttp. The PR must be approved by known contributors before it can be merged. The past contributors are: DonggeLiu |
DaveLak
force-pushed
the
projects/llhttp/update-build-script-and-dependencies
branch
2 times, most recently
from
296933c to
d588b5f
Compare
Replaces the third party JavaScript toolchain install script with the installation script included in OSS-Fuzz.
DaveLak
force-pushed
the
projects/llhttp/update-build-script-and-dependencies
branch
from
d588b5f to
4504217
Compare
oliverchang
approved these changes
Jan 13, 2025
DaveLak
deleted the
projects/llhttp/update-build-script-and-dependencies
branch
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.
Fixes several issues in the build which have been breaking the runtime code coverage collection since at least 2024-11-24.
Key Changes:
install_javascriot.shscript for compatibility with the upstream project's requirements.$OUTdirectory so unnecessary build artifacts can't break anything in unexpected ways.Impact:
Note Nodejs Install Script Slowdowns
The Dockerfile, both before and after this PR, uses a script hosted by https://deb.nodesource\.com to install a more recent version of Nodejs than the (extremely) old version available in the container by default.
That has been working just fine, however while I was reviewing the build logs I noticed the install script prints a warning about the deprecation of this node 14 version, but more importantly:
It deliberately sleeps for 80 seconds after printing the warnings, halting the build before continuing.
Unfortunately this is not easy to spot when reviewing the raw log directly: https://oss-fuzz-build-logs.storage.googleapis.com/log-fdce3d72-fe01-4a10-9f84-98617dc5110b.txt
It is a bit easier in the build status dashboard though: https://oss-fuzz-build-logs.storage.googleapis.com/index.html#llhttp
See below:
Log excerpt