search

bus1 / dbus-broker

Linux D-Bus Message Broker
https://github.com/bus1/dbus-broker/wiki
Apache License 2.0
667 stars 78 forks source link

ci: turn on CIFuzz #295

Closed evverx closed 2 years ago

evverx commented 2 years ago

https://google.github.io/oss-fuzz/getting-started/continuous-integration/

Depends on https://github.com/google/oss-fuzz/pull/7870

dvdhrm commented 2 years ago

Thanks a lot! Looks good! I will wait for the fuzzer changes to be merged, then take another look at this one.

evverx commented 2 years ago

@dvdhrm now that the fuzz target is merged I think this PR should be good to go. I also rolled out the alignment check in https://github.com/google/oss-fuzz/pull/8055 just in case.

dvdhrm commented 2 years ago

Thank you very much for going through with this! I now merged this and pushed a follow-up where I did minor coding-style adjustments (eg., we do not indent arrays in yml), and I moved it to fuzzer.yml rather than cifuzz.yml.

I wonder whether it is worth it running this on every PR. We do not get unlimited time on the github CI, and I don't mind too much catching things a bit late on main. But lets deal with that if CI times become an issue.

Again, thanks a lot for working on this and keeping an eye on the upstream PR!

evverx commented 2 years ago

We do not get unlimited time on the github CI

dbus-broker is a public repository so it should receive as many free minutes as it needs. The only limit is the number of concurrent jobs but I don't think the repository should hit it very often. Things get complicated when public repositories are forked and moved to private repositories (where https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions kicks in) but this scenario is already covered by skipping CIFuzz outside of the dbus-broker repository.

dvdhrm commented 2 years ago

We do not get unlimited time on the github CI

dbus-broker is a public repository so it should receive as many free minutes as it needs. The only limit is the number of concurrent jobs but I don't think the repository should hit it very often.

Yes, yes, this was what I meant. The concurrency is pretty low (4?) and if you have them all occupied with 1h jobs, trivial unit-tests in other repositories needlessly have to wait for 1h. This can be a bit annoying. But lets deal with that when it becomes a problem.

evverx commented 2 years ago

The concurrency is pretty low (4?)

I think it should be 20 by default.

if you have them all occupied with 1h jobs, trivial unit-tests in other repositories needlessly have to wait for 1h. This can be a bit annoying.

Agreed. It's annoying when it happens. CIFuzz should run for 10 minutes though. Let me double-check just in case.

But lets deal with that when it becomes a problem.

Agreed.

FWIW dbus-broker (or, more precisely, OSS-Fuzz) appears to have hit some limit somewhere but it should be addressed in https://github.com/google/oss-fuzz/issues/8067.

dvdhrm commented 2 years ago

The concurrency is pretty low (4?)

I think it should be 20 by default.

Indeed it is 20. I had remembered 4, but that might be from the initial beta phase still. This explains why I barely saw concurrency issues. 20 seems fine for now.

if you have them all occupied with 1h jobs, trivial unit-tests in other repositories needlessly have to wait for 1h. This can be a bit annoying.

Agreed. It's annoying when it happens. CIFuzz should run for 10 minutes though. Let me double-check just in case.

Yes, they stop after 10min. That should be fine!