Improve OpenSSL KDF check #546
Conversation
|
Can someone review this, please? It is a pretty trivial, but also important change |
|
Looks beautiful, but does it work? I have no idea. Who is the Meson expert? |
| openssl_dep.type_name() != 'internal' and | ||
| get_option('crypto-library-kdf').disabled() != true and | ||
| cc.has_function('kdf_srtp', dependencies: openssl_dep) | ||
| ) |
There was a problem hiding this comment.
Looks fine to me, but
- why do we have to exclude openssl as meson subproject here? I mean, is there a technical reason or is it just a shortcut because you know your openssl wrap doesn't provide this? (But it could in future, if a new version of openssl starts shipping this, or someone makes a modified subproject with it included)
- use
not get_option(..).disabled()
There was a problem hiding this comment.
As I mentioned in the very first comment cc.has_function() doesn't work with dependencies that are subprojects.
Applied second suggestion.
There was a problem hiding this comment.
Ah right, of course. Sorry, only started reading from the most recent email onwards :)
|
@tp-m can you confirm this looks good to you? As to whether it works, it definitely does in versatica/mediasoup#622 where CI runs on Windows, Linux and macOS with various compilers and all builds/runs fine. And there is really nothing so significant here to break anything. |
Yes, looks good to me. Looking forward to be able to use this in GStreamer as well! |
|
OpenSSL KDF check should probably be removed, I do not know of any actual use of OpenSSL KDF with libSRTP but that is a different issue. |
Without this change libsrtp2 usage with OpenSSL as subproject will fail with:
I had to include 2 additional conditions:
cc.has_function()will fail as error above suggestscrypto-library-kdfoption (in which case there is no need to check anything and configuration incorrectly didn't take that into account)P.S. OpenSSL as subproject/wrap will hopefully soon be a thing, see mesonbuild/wrapdb#113