-
Couldn't load subscription status.
- Fork 2.1k
Add initial recipe for boringssl #28672
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
base: master
Are you sure you want to change the base?
Conversation
7aa1be3 to
3c9278f
Compare
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.
@bringhurst Hello! Thank you for your interest in bringing BoringSSL into Conan Center Index. I see this project has good potential as is adopted by other package managers and has a popularity level (+2k stars in GitHub).
I would ask you to simplify as much as possible your recipe, including removing most of the options. For instance, sanitizers have a different approach using Conan, and it would only increase the future maintenance of this recipe. I would suggest only keeping shared, fPIC, no_asm and small, as most of the package managers sustain as well.
Regards!
|
Works for me. :) I just stripped it down quite a bit and also verified that it still builds with |
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.
@bringhurst Thank you again for taking your time in this PR. I made a second review round; this time, I built this project locally to check its results, so I suggested some changes. Regards!
|
|
||
| def generate(self): | ||
| tc = CMakeToolchain(self) | ||
| tc.cache_variables["CMAKE_FIND_PACKAGE_PREFER_CONFIG"] = True |
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.
| tc.cache_variables["CMAKE_FIND_PACKAGE_PREFER_CONFIG"] = True |
That's a good catch by informing CMake to prefer Config, but it's managed already by Conan CMakeToolchain: https://docs.conan.io/2/reference/tools/cmake/cmaketoolchain.html#extending-and-advanced-customization
| tc.cache_variables["OPENSSL_NO_ASM"] = bool(self.options.openssl_no_asm) | ||
| tc.cache_variables["OPENSSL_SMALL"] = bool(self.options.openssl_small) |
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.
| tc.cache_variables["OPENSSL_NO_ASM"] = bool(self.options.openssl_no_asm) | |
| tc.cache_variables["OPENSSL_SMALL"] = bool(self.options.openssl_small) | |
| tc.cache_variables["OPENSSL_NO_ASM"] = self.options.openssl_no_asm | |
| tc.cache_variables["OPENSSL_SMALL"] = self.options.openssl_small |
Just to let you know, options are converted to boolean automatically, so you don't need to use bool(). Just an information for a future PR, don't need to change these lines :)
| set(CMAKE_FIND_PACKAGE_PREFER_CONFIG ON) | ||
|
|
||
| # Use the OpenSSL config package installed by BoringSSL |
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.
| set(CMAKE_FIND_PACKAGE_PREFER_CONFIG ON) | |
| # Use the OpenSSL config package installed by BoringSSL |
Let Conan CMakeToolchain manage it.
|
|
||
| add_executable(test_package test_package.cpp) | ||
| target_link_libraries(test_package PRIVATE OpenSSL::SSL OpenSSL::Crypto) | ||
| target_compile_features(test_package PRIVATE cxx_std_17) |
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.
Dont need to remove, just for a future PR: Conan injects the CMAKE_CXX_STANDARD always, based on the settings.compiler.cppstd, so you do not need to configure it in the test package.
| cmake.build() | ||
|
|
||
| def test(self): | ||
| if not self.conf.get("tools.build:skip_test", default=False): |
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.
| if not self.conf.get("tools.build:skip_test", default=False): | |
| if can_run(self): |
The conf tools.build:skip_test is mainly directed to unit test, but regarding the project itself, not the test package. The test package, as the name indicates, validates if the package is all in order.
|
|
||
| # Intentionally DO NOT remove lib/cmake to preserve the upstream OpenSSL | ||
| # config package so consumers can use: find_package(OpenSSL CONFIG) |
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.
| # Intentionally DO NOT remove lib/cmake to preserve the upstream OpenSSL | |
| # config package so consumers can use: find_package(OpenSSL CONFIG) | |
| rmdir(self, os.path.join(self.package_folder, "lib", "cmake")) |
By the opposite, consumers should use the Conan generator CMakeDeps that will provide proper CMake files using the same name and targets.
|
|
||
| # Remove fPIC when shared=True |
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.
| # Remove fPIC when shared=True | |
| provides = "openssl" |
As it will collide with regular OpenSSL, we should avoid both in the same graph. https://docs.conan.io/2/reference/conanfile/attributes.html#provides
| elif str(self.settings.os) not in ("Android", "Generic"): | ||
| # Upstream links Threads::Threads on most non-Android/embedded platforms | ||
| crypto.system_libs.append("pthread") |
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.
| elif str(self.settings.os) not in ("Android", "Generic"): | |
| # Upstream links Threads::Threads on most non-Android/embedded platforms | |
| crypto.system_libs.append("pthread") | |
| elif self.settings.os in ["Linux", "FreeBSD"]: | |
| crypto.system_libs = ["pthread"] |
This is largely used in ConanCenterIndex. At least we are on safe ground when restricting to these OS, so it may avoid future issues.
| #include <openssl/base.h> | ||
| #include <openssl/crypto.h> | ||
| #include <openssl/ssl.h> | ||
|
|
||
| #include <cassert> | ||
| #include <cstring> | ||
| #include <iostream> | ||
|
|
||
| int main() { | ||
| #if !defined(OPENSSL_IS_BORINGSSL) | ||
| #error \ | ||
| "This test must be compiled against BoringSSL headers (OPENSSL_IS_BORINGSSL not defined)." | ||
| #endif | ||
|
|
||
| // Runtime verification that we linked against BoringSSL. | ||
| const char *version = OpenSSL_version(OPENSSL_VERSION); | ||
| std::cout << "OpenSSL_version: " << version << "\n"; | ||
| if (!version || std::strstr(version, "BoringSSL") == nullptr) { | ||
| std::cerr << "ERROR: The loaded libcrypto/libssl does not appear to be " | ||
| "BoringSSL.\n"; | ||
| return 1; | ||
| } | ||
|
|
||
| // Basic sanity to ensure libssl is usable. | ||
| SSL_CTX *ctx = SSL_CTX_new(TLS_method()); | ||
| assert(ctx != nullptr); | ||
| SSL_CTX_free(ctx); | ||
|
|
||
| std::cout << "BoringSSL test executable linked and ran.\n"; | ||
| return 0; | ||
| } |
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.
| #include <openssl/base.h> | |
| #include <openssl/crypto.h> | |
| #include <openssl/ssl.h> | |
| #include <cassert> | |
| #include <cstring> | |
| #include <iostream> | |
| int main() { | |
| #if !defined(OPENSSL_IS_BORINGSSL) | |
| #error \ | |
| "This test must be compiled against BoringSSL headers (OPENSSL_IS_BORINGSSL not defined)." | |
| #endif | |
| // Runtime verification that we linked against BoringSSL. | |
| const char *version = OpenSSL_version(OPENSSL_VERSION); | |
| std::cout << "OpenSSL_version: " << version << "\n"; | |
| if (!version || std::strstr(version, "BoringSSL") == nullptr) { | |
| std::cerr << "ERROR: The loaded libcrypto/libssl does not appear to be " | |
| "BoringSSL.\n"; | |
| return 1; | |
| } | |
| // Basic sanity to ensure libssl is usable. | |
| SSL_CTX *ctx = SSL_CTX_new(TLS_method()); | |
| assert(ctx != nullptr); | |
| SSL_CTX_free(ctx); | |
| std::cout << "BoringSSL test executable linked and ran.\n"; | |
| return 0; | |
| } | |
| #include <cstdlib> | |
| #include <openssl/ssl.h> | |
| #include <iostream> | |
| int main() { | |
| OPENSSL_init_ssl(0, NULL); | |
| std::cout << "OpenSSL version: " << OpenSSL_version(OPENSSL_VERSION) << "\n"; | |
| return EXIT_SUCCESS; | |
| } |
Let's simplify it as much as possible, as the test package only needs to validate the package itself. Plus, it will only be used for this package, not for regular OpenSSL, so mixing them is not something that will happen.
| def generate(self): | ||
| tc = CMakeToolchain(self) | ||
| tc.cache_variables["CMAKE_FIND_PACKAGE_PREFER_CONFIG"] = True | ||
|
|
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.
| tc.cache_variables["BUILD_TESTING"] = False |
It's currently building unit tests, increasing by around +200 files
Summary
Changes to recipe: boringssl/0.20251002.0
fixes #28634
Motivation
This recipe does not currently exist in the index.
Details
This is a brand new recipe. Code was formatted using default black (python) and clang-tidy (cpp) settings.
Note: Please pay special attention to the unusual handling of cmake files in
def package(self).