srt icon indicating copy to clipboard operation
srt copied to clipboard

fix: link bcrypt on Windows when mbedtls >=v3.5.0

Open tommyvct opened this issue 1 year ago • 2 comments

PR #2842 doens't fix the problem in all cases. This PR will only link to bcrypt if mbedtls is not found by find_package(mbedtls).

tommyvct avatar Jan 20 '24 09:01 tommyvct

CC @maxsharabayko @wangyoucao577

tommyvct avatar Jan 20 '24 09:01 tommyvct

Oh, I didn't cover that branch because I saw it exist for WIN32 already, see https://github.com/Mbed-TLS/mbedtls/blob/fb12d9204dec811d0277a09bd93f9d745f44ce9f/library/CMakeLists.txt#L218-L220 which has been added in PR https://github.com/Mbed-TLS/mbedtls/pull/8047. On the other hand, I saw it in the generated cmake config file MbedTLSTargets.cmake:
image

But I indeed didn't test it. If it's not work on find_package, maybe it's better to fix it on MbedTLS side?

wangyoucao577 avatar Jan 20 '24 11:01 wangyoucao577

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.37%. Comparing base (2eb47e3) to head (daff78e). Report is 21 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2860      +/-   ##
==========================================
+ Coverage   66.56%   67.37%   +0.81%     
==========================================
  Files         103      103              
  Lines       20443    20538      +95     
==========================================
+ Hits        13608    13838     +230     
+ Misses       6835     6700     -135     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 07 '24 09:03 codecov[bot]

The Android NDK-R23 github action failed, but on downloading OpenSSL. Not related to this PR. And I already can't re-run it. As there were no objection to this fix, merging.

maxsharabayko avatar Mar 07 '24 09:03 maxsharabayko