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: CMake Build System 3.0 overhaul for future feature compatibility #8247

Merged
merged 5 commits into from Mar 26, 2023

Conversation

PatTheMav
Copy link
Member

@PatTheMav PatTheMav commented Feb 10, 2023

Description

This PR overhauls the existing CMake build system, utilising new features added to CMake in the year since the last update and fixing/integrating issues that arose from the prior update.

Due to increased code quality requirements by macOS builds via Xcode, additional long-existing warnings and code issues have been fixed as well.

Note: This first draft retains every change split up into separate commits to make it easier to fix bugs in specific modules. Appropriate squashing will happen before a final merge.

Dependencies: This PR requires the following PRs to also be merged:

In its current state, the code will compile, but without the obs-deps update macOS builds will not run (due to dynamic linker lookup incompatibilities).

Motivation and Context

Due to macOS 14 removing DAL Plugin functionality entirely, a new virtual camera based on a system extension is required. System extensions not only require code signing and notarisation but also attaching a provisioning profile to the application.

For this (and many other reasons) it became easier to have distributable builds of OBS be built by Xcode directly (as Ninja is not able to handle codesigning, bundling, and applying profiles automatically).

To achieve this, a lot of functionality currently spread out between build scripts (dependency management) the build system itself (generating the Xcode project) and installation scripts (bundling the application) have been integrated into the main build system:

  • System requirements (Xcode, CMake) still need to be fulfilled by maintainers or CI build scripts
  • Build requirements (obs-deps, CEF, Sparkle, VLC) are fulfilled by CMake itself (no need for build scripts)
  • Bundling is handled by Xcode itself (as the CMake build system now informs Xcode directly about all binary dependencies)
  • Codesigning is handled by Xcode itself
  • Packaging is handled by CPack
  • Notarization still needs to be done manually (or via the packaging scripts)

For a transitional period Windows and Linux will still use the existing build scripts (aka "CMake Build System 2.0"), with follow-up PRs for Windows, Linux, GitHub Actions Workflows and build scripts are ready (and will be published after this PR has been stabilised enough - details on those changes will be explained in the respective PRs).

How Has This Been Tested?

Successful builds were created on macOS 13, Ubuntu 22.10, and Windows 11, as well as on CI.

As this PR only changes the way macOS builds are generated, checks on Linux and Windows should focus on regressions.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Code cleanup (non-breaking change which makes code smaller or more readable)

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.

@PatTheMav
Copy link
Member Author

PatTheMav commented Feb 10, 2023

Additional Information

Instead of ballooning the OP, I'll add some additional features/highlights in the comments.

These changes only apply when the 3.0 path is used:

CMake

  • The new build script understands a cache variable named OBS_CMAKE_VERSION which will allow switching between the updated and legacy systems for a while. Version 3.0 (or more recent) triggers the new system
  • Almost all build settings are now managed via CMake presets and the buildspec.json file which contains versions and hashes of required dependencies
  • For Linux compatibility reasons, the minimum required CMake version is still 3.16 in the main CMake files (as otherwise the legacy scripts couldn't be loaded), but the actual minimum requirement for version 3.0 is CMake 3.22 or more recent
    • Windows and macOS requirement is CMake 3.24 or more recent
  • CMake recursively resolves all dependencies of all build targets and either copies them into the binary directory as a POST_BUILD CMake action (Windows) or adds them to the Xcode project as bundle frameworks and plugins (macOS)
    • This allows Xcode to automatically copy and code sign all dependent libraries and OBS plugins into the bundle without additional code
  • Qt plugins are detected and resolved as best as possible, including Qt frameworks+plugins not in use by OBS yet
  • cmake-lint was used on all files to check for issues - some errors had to be disabled because the cmakelang package hasn't kept up with recent CMake releases and doesn't recognise updated function signatures and new functions
  • cmake-format was used on all files with an updated rules file, increasing the line-width to 100 characters
  • The framework itself is modular and based on including setting/feature-specific files:
    • OS-specific functionality is included by using common names for modules and adding only the OS-specific module path
    • Larger components have their CMake list files broken up and using includes to improve readability and maintainability
    • Each component contains its own cmake subdirectory containing resource files for specific OSes and included files
    • The existing CMake list files are retained as legacy.cmake for each component

CMake finders

  • All CMake finders have been rewritten from scratch
    • Doc headers as used by CMake's official finders have been added
    • Legacy cache variables (<LIB>_LIBRARIES, <LIB>_INCLUDE_DIRS) as used up to CMake 2.8 have been removed to force use of targets (CMake 3.0+ style)
    • If debug and non-debug variants of a library a found, both configurations are made available (CMake will fall back to any available non-debug variant for other configurations)
    • Every find modules attempts to find a valid version number for a dependency
      • FFmpeg's finder was updated to support FFmpeg's new version headers (major version is kept in a different header file version_major.h)
    • Find modules will attempt to set the SONAME for macOS and Linux from the dynamic library
    • Find modules will attempt to discover a matching DLL file for a discovered import library
      • If no DLL is found, the static library property is explicitly set
      • If a DLL is found, the import library will be referred as the IMPLIB and the DLL as the actual LIBRARY on the target
    • FeatureSummary is used when debug output is enabled to print more information about libraries discovered and used in the project

macOS

  • Minimum macOS deployment target is macOS 11.0
    • Default (primary) architecture is Apple Silicon, secondary architecture is Intel x86_64
  • Compatibility version of all dynamic libraries is fixed to 1.0.0 (canonical on macOS)
  • Dynamic library identifiers (used to lookup linked libraries) do not contain version numbers anymore (as the version is maintained via metadata and not via the filename as common on Linux/Unix).
    • This will require additional testing with plugins, as unfortunately OBS has carried version numbers for obs-frontend-api in the past. Otherwise plugins need to be updated for whichever version this is merged in again.
    • This (possibly breaking) change is required to make the Xcode-based bundling work however
  • There is a possible slowdown of Xcode builds when using ccache (vs Ninja) as its settings have been tweaked to work more "correctly" with the advanced build features Xcode uses (e.g. ObjC modules), I'm still investigating this one
  • Builds use xcbeautify to make Xcode build output easier to parse/read

@PatTheMav PatTheMav added the Seeking Testers Build artifacts on CI label Feb 10, 2023
@PatTheMav PatTheMav force-pushed the macos-build-update branch 19 times, most recently from f89c250 to f62806b Compare February 16, 2023 17:02
@PatTheMav PatTheMav force-pushed the macos-build-update branch 5 times, most recently from 00c5355 to 7b7821a Compare February 19, 2023 01:49
@derrod derrod mentioned this pull request Feb 19, 2023
6 tasks
libobs/util/platform.h Outdated Show resolved Hide resolved
@RytoEX
Copy link
Member

RytoEX commented Mar 16, 2023

Something I noticed is that I'm now seeing over 100 warnings in syphon that I didn't see before, are those meant to not be suppressed?

That's on purpose - warnings are supposed to be fixed and not suppressed. The syphon module is severely outdated and needs to be rewritten from scratch or at the very least updated to use a more recent version of the Syphon framework (if there is any).

There is a newer version, yes, which is still maintained. If the newer version for some reason does not meet our needs, we should work on that.
https://github.com/Syphon/Syphon-Framework

@PatTheMav
Copy link
Member Author

Something I noticed is that I'm now seeing over 100 warnings in syphon that I didn't see before, are those meant to not be suppressed?

That's on purpose - warnings are supposed to be fixed and not suppressed. The syphon module is severely outdated and needs to be rewritten from scratch or at the very least updated to use a more recent version of the Syphon framework (if there is any).

There is a newer version, yes, which is still maintained. If the newer version for some reason does not meet our needs, we should work on that. https://github.com/Syphon/Syphon-Framework

Correct - updating the plugin to use that would fix the warnings by virtue of being a proper refactor. So I'm not concerned about leaving those warnings as a reminder to get around to that at some point.

cmake/finders/FindMbedTLS.cmake Outdated Show resolved Hide resolved
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.

Some questions, some nits, and some items to fix.

I am perfectly okay with some things being deferred to a later PR (sweeping for c_std_11, cxx_std_11, etc.) if that creates less work here or keeps the diff minimal.

UI/CMakeLists.txt Outdated Show resolved Hide resolved
UI/CMakeLists.txt Show resolved Hide resolved
UI/cmake/os-linux.cmake Outdated Show resolved Hide resolved
UI/cmake/os-windows.cmake Outdated Show resolved Hide resolved
UI/cmake/os-windows.cmake Outdated Show resolved Hide resolved
plugins/CMakeLists.txt Show resolved Hide resolved
plugins/mac-virtualcam/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/obs-filters/cmake/rnnoise.cmake Show resolved Hide resolved
plugins/vlc-video/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/win-capture/CMakeLists.txt Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
UI/cmake/os-windows.cmake Outdated Show resolved Hide resolved
UI/win-update/updater/CMakeLists.txt Outdated Show resolved Hide resolved
cmake/Modules/CompilerConfig.cmake Show resolved Hide resolved
cmake/common/bootstrap.cmake Outdated Show resolved Hide resolved
libobs/CMakeLists.txt Outdated Show resolved Hide resolved
libobs/CMakeLists.txt Outdated Show resolved Hide resolved
plugins/CMakeLists.txt Outdated Show resolved Hide resolved
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.

Just a few items.

UI/win-update/updater/CMakeLists.txt Outdated Show resolved Hide resolved
cmake/common/bootstrap.cmake Show resolved Hide resolved
libobs/CMakeLists.txt Outdated Show resolved Hide resolved
@PatTheMav PatTheMav force-pushed the macos-build-update branch 2 times, most recently from 58418bd to f882db0 Compare March 24, 2023 16:26
UI/win-update/updater/CMakeLists.txt Outdated Show resolved Hide resolved
@PatTheMav PatTheMav force-pushed the macos-build-update branch 4 times, most recently from 5d6d69c to 8465aa7 Compare March 25, 2023 21:18
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.

Seems to pass all of my local tests. Looks good to me. Pat or I will handle merge order and submodule updates.

@PatTheMav PatTheMav force-pushed the macos-build-update branch 2 times, most recently from 9c6278a to cb086ca Compare March 26, 2023 02:14
OBS CMake build framework 3.0 is a minor overhaul of version 2.0. Due
to close proximity of the 2.0 rework, the amount of actual changes to
project files are minimal and mostly concern application generation.

This commit contains the bootstrap elements only and requires
OS-specific implementations to be functional.
Adds necessary macOS-specific implementation of the framework, focusing
on native Xcode builds.
New code path only taken if OBS_CMAKE_VERSION is set to 3.0.0 or
greater, old functionality remains unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Seeking Testers Build artifacts on CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants