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

Don't use the unofficial libevent component #1837

Merged
merged 1 commit into from Dec 23, 2022

Conversation

fredizzimo
Copy link
Contributor

The official libevent cmake file, does not declare the libevent component, which is a combination of core and extra, see https://github.com/libevent/libevent/blob/master/cmake/LibeventConfig.cmake.in, so it should not be used here, even if the embedded FindLibevent.cmake supports it.

This causes problems for me in combination with the Conan package manager, which sets CMAKE_FIND_PACKAGE_PREFER_CONFIG to true, which makes find_package use the system version instead of the embedded version, and therefore fails to find the libevent component. See this discussion and other linked issues for more information about that conan-io/conan#12488

This is a problem only if the configuration is set to not need libevent, since find_package is called unconditionally. When libevent is actually needed, the Conan version will be used instead, which does not cause any problems.

The fix is simple, just request core and extra instead of libevent. I did not make any changes to FindLibevent.cmake, since this change works with the version in this repository without any changes. But I do believe that in the long run it would be better to replace it with something much closer to the official LibeventConfig.cmake.

@tatsuhiro-t tatsuhiro-t merged commit 252c425 into nghttp2:master Dec 23, 2022
@tatsuhiro-t tatsuhiro-t added this to the v1.52.0 milestone Dec 23, 2022
@tatsuhiro-t
Copy link
Member

Thank you for PR. Merged now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants