-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
civetweb: Add missing CMake options #9303
Conversation
Signed-off-by: Jean-Baptiste Louazel <jb.louazel@gmail.com>
This comment has been minimized.
This comment has been minimized.
I don't know if this is caused by something I did. If someone wants to jump in and explain this to me, I would genuinely appreciate it. |
Signed-off-by: Jean-Baptiste Louazel <jb.louazel@gmail.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Jean-Baptiste Louazel <jb.louazel@gmail.com>
Signed-off-by: Jean-Baptiste Louazel <jb.louazel@gmail.com>
Signed-off-by: Jean-Baptiste Louazel <jb.louazel@gmail.com>
This comment has been minimized.
This comment has been minimized.
@@ -77,18 +101,32 @@ def _configure_cmake(self): | |||
if self._cmake: | |||
return self._cmake | |||
self._cmake = CMake(self) | |||
self._cmake.definitions["CIVETWEB_ENABLE_SSL"] = self.options.with_ssl | |||
|
|||
if self.options.with_ssl: | |||
openssl_version = tools.Version(self.deps_cpp_info["openssl"].version[:-1]) |
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.
This would break with openssl greater then 3.x 🤔 maybe a validate check?
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.
It was already there before my PR, so I didn't touch it. I am not against taking the opportunity to do that as well. What would you recommend?
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.
It's the first PR since... let's not block this good work, maybe follow up with another PR 🙏
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.
@prince-chrismc Sure, will do!
Specify library name and version: civetweb/1.15
This pull request aims to add more options available in
civetweb
's CMake. This change is confirmed to fix the issue I already reported and bring more modularity to this package.This resolves #9302.
Note for reviewers
As this is the first time I am contributing, I am not sure how to deal with the versioning of my pull request. Technically speaking, there is only an update in the recipe itself as the code behind it didn't change, so I didn't change anything regarding the version. Please tell me if there is something I missed in the procedure. I appreciate any help you can provide.
conan-center hook activated.