-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Fix compilation errors and add fuzzers to CircleCI #9420
Conversation
35915f2
to
3fa8373
Compare
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
d48fdd9
to
cf12e97
Compare
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
cf12e97
to
c6ef1ca
Compare
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
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.
LGTM
git clone --single-branch --branch master --depth 1 git@github.com:google/libprotobuf-mutator.git ~/libprotobuf-mutator | ||
cd ~/libprotobuf-mutator && mkdir build && cd build | ||
cmake .. -GNinja -DCMAKE_C_COMPILER=clang-13 -DCMAKE_CXX_COMPILER=clang++-13 -DCMAKE_BUILD_TYPE=Release -DLIB_PROTO_MUTATOR_DOWNLOAD_PROTOBUF=ON | ||
ninja && sudo ninja install |
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.
Ideally, we should ping libprotobuf to a fixed version instead of master (but seems they only have one master branch).
is there a pre-built package we could install?
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.
I searched online but did not find a pre-built package for ubuntu or centos.
The commit history suggests that updates are rare, thus I guess we are fine for now.
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.
Looks like it's available on SUSE https://packagehub.suse.com/packages/libprotobuf-mutator/
@@ -20,10 +20,10 @@ PROTO_IN = $(ROOT_DIR)/fuzz/proto | |||
PROTO_OUT = $(ROOT_DIR)/fuzz/proto/gen | |||
|
|||
ifneq ($(FUZZ_ENV), ossfuzz) | |||
CC = clang++ |
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.
wish we could have cmake support too.
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.
I am going to create a task/issue for this.
This PR does the following:
Test plan:
CI