[GOLDEN SOLUTION] Add CMake MbedTLS Component Support with Fail-to-Pass Integration Tests#2375
[GOLDEN SOLUTION] Add CMake MbedTLS Component Support with Fail-to-Pass Integration Tests#2375Teja3804 wants to merge 2 commits intoyhirose:masterfrom
Conversation
|
While I completly neglected the tests, I dislike the ideas of tests for CMake in general and the approach of programming a "...Config.cmake.in" for a dependency in case that the system does not provide it. For the tests, I do not like Docker. I assume the tests cover one possible environment. What about linkage? Static and shared builds? Every combination of build options? Even different operating systems, Windows, GNU/Linux, Mac? I would not test CMake. That is my opinion. The tests are just a side note but the main contribution of this pull request is the individual "...Config.cmake.in" for MbedTLS. The project itself (MbedTLS) should provide its exported CMake target and does so. Imagine every consuming project wrote its own "...Config.cmake.in" for every dependency. That would result in different behaviours all over the place. Despite that, the burden of maintenance increases a lot. I am aware that Brotli has its own "...Config.cmake.in" but Brotli does not provide it itself but a pkgconfig. Even there I would propose using https://cmake.org/cmake/help/latest/module/FindPkgConfig.html to decrease the maintenance. Being consistent with your approach would require every dependency to have its own "...Config.cmake.in". I would even support the idea of removing Summary, I do not like complicating CMake more than necessary because CMake itself is annoying at developing and maintaining. Moreover, leave the responsibility of the exported targets at the origin source. |
| @@ -0,0 +1,79 @@ | |||
| # A lightweight find module for Mbed TLS that works with older CMake versions. | |||
There was a problem hiding this comment.
how is this better than pkg-config?
Solution Approach
I used the existing solution idea from PR #2360 (
[CMake] New compoment MbedTLS) and implemented a production-ready version on top of currentmainwith one[sol]commit.Changes in the
[sol]commit:HTTPLIB_REQUIRE_MBEDTLSHTTPLIB_USE_MBEDTLS_IF_AVAILABLECMakeLists.txt:find_package(MbedTLS REQUIRED/QUIET)HTTPLIB_IS_USING_MBEDTLSstate handlingMbedTLS::mbedtlsCPPHTTPLIB_MBEDTLS_SUPPORTcmake/httplibConfig.cmake.in:HTTPLIB_IS_USING_MBEDTLSfind_dependency(MbedTLS)httplib_MbedTLS_FOUNDcmake/FindMbedTLS.cmakeand installed it with package config so component discovery works even whenMbedTLSConfig.cmakeis not provided by the distro.Testing Approach
I added one fail-to-pass integration test script in the
[f2p]commit:test/cmake_mbedtls_component_test.shWhat it validates:
httplibwith-DHTTPLIB_REQUIRE_MBEDTLS=ON.find_package(httplib REQUIRED COMPONENTS MbedTLS)and linkshttplib::httplib.HTTPLIB_IS_USING_MBEDTLS,httplib_MbedTLS_FOUND).I also updated
run_tests.shto:cmake,ninja-build,libmbedtls-dev, etc.).test/cmake_mbedtls_component_test.shfrom the comma-separated file argument.Test Execution Steps
docker.io/ccrawford4/python-base-amd64:v1.0.9BASE_COMMIT.run_script.shfromTEST_COMMIT:run_tests.sh.[f2p]patch.[sol]patch.Test Execution + Results