-
Notifications
You must be signed in to change notification settings - Fork 36
make CI jobs faster, and other improvements #181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
CMake Error: Running '/usr/local/bin/ninja' '-C' '/home/runner/work/curl-fuzzer/curl-fuzzer/build' '-t' 'recompact' failed with: ninja: error: build.ninja:1174: bad $-escape (literal $ must be written as $$) CMake Generate step failed. Build files cannot be regenerated correctly. https://github.com/curl/curl-fuzzer/actions/runs/16584395005/job/46906820771?pr=181
``` CMake Error: Running '/usr/local/bin/ninja' '-C' '/home/runner/work/curl-fuzzer/curl-fuzzer/build' '-t' 'recompact' failed with: ninja: error: build.ninja:1174: bad $-escape (literal $ must be written as $$) ``` https://github.com/curl/curl-fuzzer/actions/runs/16755832030/job/47438071308#step:4:57
… the orchestrator which is incompatible with ninja due to using Externalproject
wget already installed, suggestions/recommendations not install in the docker machine by default.
esp configure speed
This reverts commit 06ac61a. Roughly the same.
They are shown via compile_target.sh a couple of lines later.
This reverts commit d321b8b. BuildFuzzers went up from 7.5 minutes to 10.
@cmeister2 What do you think of these updates? |
Before this patch the GNU make command-line was: /usr/bin/gmake -s -j4 --jobserver-auth=3,4 After moving to Ninja this changed implicitly to: make -w -j4 --jobserver-auth=3,4 as seen in JustDependencies and Mainline jobs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broadly happy but I would like the commits to be squashed or otherwise rebased into logical blocks when merging.
I always squash before merge, as the last step, to not lose track of the local history till there may be a need for changes. The total diff in the meantime: https://github.com/curl/curl-fuzzer/pull/181.diff (In this case I'm not keen to split into multiple PRs, because the number of touched lines is fairly small, but also very prone to conflicts if torn apart.) |
After this patch BuildFuzzers and Testi386 jobs need about 7.5 minutes
per job, down from 15-16m, for a >2x speed-up. There should noticeable
improvements in the other jobs, too:
Before: https://github.com/curl/curl/actions/runs/16769657727
After: https://github.com/curl/curl-fuzzer/actions/runs/16775268062?pr=181
Also reduce log noise and fix minor issues found along the way. Add
Ninja support.
Details:
apt
commands.env:
beforerun:
to match control flow. (style)performance across platforms, and more readable logs.
BUILD_COMMAND $(MAKE)
andINSTALL_COMMAND $(MAKE) install
and use the cmake defaults.The defaults work with both GNU Make and Ninja generators. This also
avoids passing a GNU Make macro to the generator, which broke with and
error when used with Ninja.
Ref: https://cmake.org/cmake/help/latest/module/ExternalProject.html
$(MAKE)
use with${MAKE}
, and initializethe latter from the calling env. This is necessary to keep avoiding
the super lengthy OpenSSL manual install phase, which needs a custom
install target (
install_sw
) unfortunately.ExternalProject_Add()
commands.Necessary for Ninja to figure out what output is generated, to resolve
them as dependencies.
.tar.xz
where available, for smallerdownloads.
(esp. better configure performance.)
it).
openssl, zstd, nghttp2, libidn2. (e.g. tools, docs, examples, tests.)
--enable-websockets
when building curl.It became the default after leaving the experimental status.
performance.
MAKEFLAGS
, which became-j4-j4
during ossfuzz builds.This made all builds run without parallelism.
Also show this env in the log, for visibility.
DOWNLOAD_EXTRACT_TIMESTAMP
option for Git.BUILD_IN_SOURCE
andSOURCE_SUBDIR
options.
groff-base
. Turns out to be a tiny patch; it's not worththe manual workaround.
Follow-up to d0bf19b
MAKEFLAGS
settings and env dumps.-s
inMAKEFLAGS
to keep the low verbosity of sub-builds asbefore this patch. Without it, CMake seems to add
-w
when usingNinja.
Due to the way ossfuzz works (?), these changes cannot be tested within
this PR, because part of the repo is pulled from curl/curl-fuzzer master
while testing changes pushed in this PR. This makes the scripts miss to
install Ninja. This setup surprised me. Also not fully from curl
upstream, because some of the parts seem to always come from master.
Helper patch: d0bf19b
Partial test from curl: curl/curl#18202
Bug: curl/curl#18140 (comment)
Possible future improvements?:
or at least for PR pushes. These jobs test the curl master branch, not
the PR branch. AFAIU it's meant to be testing the fuzzer sources with
curl and deps. Maybe a daily build could work for this? Or restrict
runs to curl master pushes?
JustDependencies only tests this repo and have no dependency on curl.
Testi386
for each curl push? 32-bit Linuxis already tested in curl CI, the rest is ensuring that the dependencies
and the fuzzer code builds for i386. It seems overkill to test these so
often, when curl is not likely to cause breakage that we'd not see anyway.
Perhaps do this as a daily build?
"Build check passed" entry. I wonder what's happening there, and if it
could be made faster?
https://github.com/curl/curl-fuzzer/actions/runs/16771441073/job/47487297844?pr=181#step:3:10581
https://github.com/google/oss-fuzz/blob/f0f9b221190c6063a773bea606d192ebfc3d00cf/infra/cifuzz/build_fuzzers.py#L207
https://github.com/google/oss-fuzz/blob/f0f9b221190c6063a773bea606d192ebfc3d00cf/infra/base-images/base-runner/test_all.py
This seems to be checking the build, which seem overkill when called
up for PR pushes?
for each curl PR or master push, taking quite a bit of time, even
though these dependencies rarely change. With caching it'd be possible
to save most of the build time, by only building curl and the fuzzer
itself.
Ref: Build Caching #183