Configure datachannel target_properties for APPLE

[Sean-Der]

After 7591b96 libdatachannel stopped working with OBS on macOS[0].
XCode only resolves one level of symlinks [1]. libdatachannel
is generating two levels.

This commit updates the shared object to only use only the major version
in the name.

[0] https://stackoverflow.com/questions/29946961/xcode-copy-files-build-phase-and-symlinks
[1] obsproject/obs-deps#204 (comment)

[Sean-Der]

@PatTheMav does this look ok?

[Sean-Der]

@PatTheMav I am reading the CMake docs and it seems to recommend NOT setting NO_SONAME

use this property only for leaf private libraries or plugins. If you use it on normal shared libraries which other targets link against, on some platforms a linker will insert a full path to the library (as specified at link time) into the dynamic section of the dependent binary.

When I set NO_SONAME in libdatachannel's CMakeLists.txt it also breaks the test. I have no idea what other impact it will have on dependent projects. I don't understand the benefit of these changes, but I do see some negatives.

@paullouisageneau what do you think is best here?

[PatTheMav]

@PatTheMav I am reading the CMake docs and it seems to recommend NOT setting NO_SONAME

use this property only for leaf private libraries or plugins. If you use it on normal shared libraries which other targets link against, on some platforms a linker will insert a full path to the library (as specified at link time) into the dynamic section of the dependent binary.

When I set NO_SONAME in libdatachannel's CMakeLists.txt it also breaks the test. I have no idea what other impact it will have on dependent projects. I don't understand the benefit of these changes, but I do see some negatives.

@paullouisageneau what do you think is best here?

The SONAME is only used to set the install_name on the generated library by CMake, which it prepends with the @rpath/ prefix. It is expected that an app binary has a set of rpath entries it uses to replace the prefix with when resolving library paths.

Xcode does that by itself, but if you do not use Xcode to build the library for macOS, you need to add that manually: target_link_options(datachannel PRIVATE -install_name "@rpath/libdatachannel.dylib"). We dropped all support for non-Xcode generators on macOS for OBS Studio, which is why we can omit that step.

FWIW, omitting the NO_SONAME property and ensuring a single version token is used (e.g. libdatachannel.5.dylib) is fine and should work as long as there is no additional symlink redirection. Setting the MACHO properties is still recommended though (as those are the actual properties checked by the library loader).

[PatTheMav]

I took a look at the dyld source code (https://opensource.apple.com/source/dyld/dyld-852.2/src/ImageLoader.cpp.auto.html) and it seems that contrary to my understanding (and what the manpage states), checks of the compatibility version are skipped since macOS 10.14 (macOS Mojave - check bool MachOFile::enforceCompatVersion() const in https://opensource.apple.com/source/dyld/dyld-852.2/dyld3/MachOFile.cpp.auto.html).

As such I stand corrected that using a single version-token SONAME (e.g. libdatachannel.1.dylib) and incrementing that single number on an ABI break is the correct approach (alternatively A/B/C can be used similar to how Frameworks are versioned, but that's entirely semantics). The MACHO ELF headers will still work on macOS 10.13 and older.

Annoying, but the more you know...

[paullouisageneau]

@PatTheMav MacOS 10.13 is legacy now, right? Is there still a point in setting the MACHO_ properties?

[PatTheMav]

@PatTheMav MacOS 10.13 is legacy now, right? Is there still a point in setting the MACHO_ properties?

See #1091 (comment) - the MACHO_ properties can be kept if the exact version numbers are desired to be visible via otool on the library and consumers of the library, but as far as breaking the link on ABI change is required, it doesn't seem necessary anymore.

[paullouisageneau]

@PatTheMav Thank you for testing and clarifying!

@Sean-Der If you want to keep the MACHO_ properties even if they are not strictly necessary, they were introduced in CMake 3.17, so you must bump cmake_minimum_required to 3.17.

[Sean-Der]

@PatTheMav can/should we drop the MACHO values?

I don’t care what we do, my hope/goal is

• Works with OBS
• Doesn’t impact other libdatachannel users

@paullouisageneau would you be ok with a new var? MACOS_SO_VERSION and we start it at 0. I would put it next to VERSION in CmakeLists.txt. From this thread that seems idiomatic?

When a v0.20.0 is ready will then bump OBS and resolve issues.

[PatTheMav]

@PatTheMav can/should we drop the MACHO values?

I don’t care what we do, my hope/goal is

• Works with OBS
• Doesn’t impact other libdatachannel users

@paullouisageneau would you be ok with a new var? MACOS_SO_VERSION and we start it at 0. I would put it next to VERSION in CmakeLists.txt. From this thread that seems idiomatic?

When a v0.20.0 is ready will then bump OBS and resolve issues.

You can drop them, the values will be inferred from VERSION/SOVERSION then, so they'll always be limited to ..0 but that shouldn't introduce major issues.

[Sean-Der]

@PatTheMav @paullouisageneau @RytoEX How does this look? I think it hits everyones concerns.

• It doesn't require setting/maintaining a new variable
• We only have one symlink
• Just puts the major version number in the name
• For other platforms (Linux) we don't effect the naming scheme (major/minor) etc..

[paullouisageneau]

@Sean-Der It looks good to me. Would you like the change on 0.19? If so, could you please rebase on the v0.19 branch?

[Sean-Der]

@paullouisageneau no thanks! v0.20 only (if that is ok)

That way I can fix both bugs