Skip to content
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

cmake: Update use of MbedTLS to support versions 3.6.0 and beyond #11056

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

PatTheMav
Copy link
Member

Description

Update MbedTLS find module and usage in CMake files to prepare codebase for later update to CMake version 3.6.0 and beyond.

Motivation and Context

MbedTLS changed a lot of internals with their LTS version 3.6.0, which are incompatible with the find module currently shipped with OBS Studio.

The solution requires several changes to be applied at once:

  • Rename the generated target name to MbedTLS::mbedtls to match the name used by MbedTLS' own CMake package
  • Update find module to use the updated target name(s)
  • Set CMAKE_FIND_PACKAGE_PREFER_CONFIG to TRUE before trying to find MbedTLS to ensure that CMake package files are used with priority (Those are shipped only with MbedTLS 3.6.0 in obs-deps).
  • A deprecation warning is emitted if the find module is used with MbedTLS 3.6.0 available

How Has This Been Tested?

Tested on macOS with either current MbedTLS version provided by obs-deps as well as MbedTLS 3.6.0 provided via its own CMake package.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@tytan652
Copy link
Collaborator

tytan652 commented Jul 31, 2024

3.6.0 caused a breakage (which break RTMPS), until MbedTLS fix their mess with a bugfix release.
I don't think this PR should be "ready for review" since 3.6.0 can not be supported.

@PatTheMav
Copy link
Member Author

3.6.0 caused a breakage (which break RTMPS), until MbedTLS fix their mess with a bugfix release. I don't think this PR should be "ready for review"

It is, and also it needs to be merged before we update MbedTLS in obs-deps as that will immediately break the current codebase.

Thus we need to merge a forward-compatible change that will work with the current release of obs-deps as well as the future release which will be based of 3.6.0 or beyond and which creates CMake packages. There is currently no suggestion that the updated version of MbedTLS will not continue using its updated build system.

@tytan652
Copy link
Collaborator

tytan652 commented Jul 31, 2024

I, at least, think we should not advertise any support of version ==3.6.0 in any capacity.

@RytoEX RytoEX self-assigned this Jul 31, 2024
@WizardCM WizardCM added the Bug Fix Non-breaking change which fixes an issue label Aug 3, 2024
@RytoEX
Copy link
Member

RytoEX commented Aug 20, 2024

This needs to be rebased and get CI to pass. IIRC, building SRT 1.5.x against mbedTLS 3.5.0+ fails because of changes in mbedTLS that needed addressed in libsrt, but there is no new libsrt tag with the needed changes. That being the case, we currently cannot update to mbedTLS 3.5+, but we can attempt to lay the foundation to allow building against newer versions so we don't have to juggle those changes in the future.

@PatTheMav
Copy link
Member Author

Requires a new obs-deps release with obsproject/obs-deps#261 merged to fix macOS.

MbedTLS changed a lot of internals with their LTS version 3.6.0, which
are incompatible with the find module currently shipped with
OBS Studio.

The solution requires several changes to be applied at once:

* Rename the generated target name to MbedTLS::mbedtls to match the
  name used by MbedTLS' own CMake package
* Update find module to use the updated target name(s)
* Set CMAKE_FIND_PACKAGE_PREFER_CONFIG to TRUE before trying to find
  MbedTLS to ensure that CMake package files are used with priority
  (Those are shipped only with MbedTLS 3.6.0 in obs-deps).
* A deprecation warning is emitted if the find module is used with
  MbedTLS 3.6.0 available
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine. We can merge this to unblock ourselves for future updates when it's possible for us to update libsrt and MbedTLS without declaring explicit support for MbedTLS 3.6.0+.

@RytoEX RytoEX merged commit f9f974f into obsproject:master Sep 13, 2024
14 checks passed
@PatTheMav PatTheMav deleted the obs-deps-update branch September 13, 2024 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants