Skip to content
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

add 'libprotobuf-mutator' recipe #8578

Merged
merged 8 commits into from
Mar 12, 2022

Conversation

n-bes
Copy link
Contributor

@n-bes n-bes commented Dec 29, 2021

Specify library name and version: libprotobuf-mutator

https://www.youtube.com/watch?v=U60hC16HEDY


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@n-bes n-bes changed the title init libprotobuf-mutator add 'libprotobuf-mutator' recipe Dec 29, 2021
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution, nice project, I didn't know about.

Please, take a look on my review. Happy new year!

recipes/libprotobuf-mutator/all/conandata.yml Show resolved Hide resolved
recipes/libprotobuf-mutator/all/conanfile.py Show resolved Hide resolved
recipes/libprotobuf-mutator/all/conanfile.py Show resolved Hide resolved
"""# (disabled by conan) add_subdirectory(examples EXCLUDE_FROM_ALL)""")

def validate(self):
if self.settings.compiler != "clang":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a reference about it? I only found:

https://github.com/google/libprotobuf-mutator/blob/54742e781e572a72214f3d76f4d2f9793dde9106/README.md#quick-start-on-debianubuntu

Clang is only needed for libFuzzer integration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

up!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still didn't answer!

recipes/libprotobuf-mutator/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libprotobuf-mutator/all/conanfile.py Show resolved Hide resolved
recipes/libprotobuf-mutator/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot

This comment has been minimized.

@SSE4 SSE4 requested a review from uilianries January 12, 2022 07:34
@conan-center-bot conan-center-bot requested a review from SSE4 January 27, 2022 12:06
SSE4
SSE4 previously approved these changes Feb 21, 2022
@n-bes
Copy link
Contributor Author

n-bes commented Feb 23, 2022

Do you have a reference about it?

@uilianries can't answer in the topic without reason. I don't know any compiler which support libfuzzer.

@uilianries
Copy link
Member

Do you have a reference about it?

@uilianries can't answer in the topic without reason. I don't know any compiler which support libfuzzer.

You are saying this recipe could work with other compilers but you did not test? If yes, remove that privation. Let's run Linux and Windows build. We skip some setting when we have some information from the upstream, or the project doesn't work and the upstream doesn't know and a hotfix is costly.

@n-bes
Copy link
Contributor Author

n-bes commented Feb 25, 2022

You are saying this recipe could work with other compilers but you did not test?

No. I mean that libfuzzer is part (unique feature) of clang.

@SSE4
Copy link
Contributor

SSE4 commented Feb 25, 2022

You are saying this recipe could work with other compilers but you did not test? If yes, remove that privation. Let's run Linux and Windows build. We skip some setting when we have some information from the upstream, or the project doesn't work and the upstream doesn't know and a hotfix is costly.

let's move forward. it's not strictly necessary to add all the possible configurations at the very first iteration.
providing some configurations is an improvement over having no package at all.
adding additional configurations could be done in follow-up PRs, on-demand.

I think we're totally fine if someone wants to contribute some recipe, and doesn't want to test e.g. Windows, so puts if self.settings.os == "Windows": raise ConanInvalidConfiguration into the recipe, if it makes his contribution faster/easier.
I believe asking people to support everything at once might only distract people from contributions.

Co-authored-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

@conan-center-bot conan-center-bot requested a review from SSE4 March 10, 2022 12:10
@conan-center-bot
Copy link
Collaborator

All green in build 1 (48a7aca05a30370671fbc57fbafda42a050b11a2):

  • libprotobuf-mutator/cci.20210831@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 2341f93 into conan-io:master Mar 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants