Fix linking RialtoServer for UnitTests

[dwrobel]

Fixes linking error:
[ 81%] Linking CXX executable RialtoServer
/usr/bin/ld: main/libRialtoServerMain.a(MediaKeysServerInternal.cpp.o):
in function firebolt::rialto::server::MediaKeysServerInternalFactory:: createMediaKeysServerInternal(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const': rialto/media/server/main/source/MediaKeysServerInternal.cpp:70: undefined reference to firebolt::rialto::server::IOcdmSystemFactory::createFactory()'
/usr/bin/ld: main/libRialtoServerMain.a(MediaKeysCapabilities.cpp.o):
in function firebolt::rialto::MediaKeysCapabilitiesFactory:: getMediaKeysCapabilities() const': rialto/media/server/main/source/MediaKeysCapabilities.cpp:76: undefined reference to firebolt::rialto::server::IOcdmSystemFactory::createFactory()' /usr/bin/ld: rialto/media/server/main/source/MediaKeysCapabilities.cpp:75:
undefined reference to `firebolt::rialto::server::IOcdmFactory::createFactory()'

Signed-off-by: Damian Wrobel dwrobel@ertelnet.rybnik.pl

[skywojciechowskim]

Review rejected. Please close this pull request.
RialtoServer is not UT binary, we build it only using bitbake and run it on STB.
It doesn't link, because OCDM library is not present in UT environment. Please use yocto if you want to build and link this binary. Yocto env has got ocdm dependency added, RialtoServer will build and link successfully there.

[dwrobel]

we build it only using bitbake and run it on STB.

Unfortunately using the following simple steps:

$ cmake -DCMAKE_BUILD_FLAG=UnitTests
$ make

generates a linking error for RialtoServer binary.

So, it needs to be fixed and I don't see a reason why you see a problem witt this PR.

[skywojciechowskim]

-DCMAKE_BUILD_FLAG=UnitTests tells us, that we want to build test binaries. RialtoServer is not UnitTest binary. You don't build it for UTs. UT binaries for rialto server are: RialtoServerMainUnitTests, RialtoServerGstPlayerUnitTests, RialtoServerIpcUnitTests, RialtoServerServiceUnitTests. Those executables build and link normally with -DCMAKE_BUILD_FLAG=UnitTests flag.
It's obvious that we should not link mocks to production code. It would introduce unnecessary mess. RialtoServer won't work with mocks anyway. RialtoServer will work only with real OCDM library. It is available, when you build RialtoServer with Yocto. If you want to build it locally, you have to provide ocdm library in your local environment and build it without -DCMAKE_BUILD_FLAG=UnitTests cmake option.

[skywojciechowskim]

btw. to build and run UTs you can use build_ut script. It contains all possible UT targets :)

[dwrobel]

You don't build it for UTs.

If you would like not to build it, then please consider removing it from building when -DCMAKE_BUILD_FLAG=UnitTests is specified.

It's obvious that we should not link mocks to production code. It would introduce unnecessary mess.

It's equally messy when make is failing.

[skywojciechowskim]

ok, so you can remove it from building, when this flag is enabled (in this MR). It's much better solution than linking mocks to production code.

[dwrobel]

It's much better solution than linking mocks to production code.

Surprisingly, it wouldn't be better. As the current approach allows you to specify -DCMAKE_BUILD_FLAG=UnitTests and you can still compile the code (including RialtoServer) using much newer toolchain available on the development platform (PC) which might not be easily available on the embedded platform. Taking advantage of that, we could catch much more warnings/errors then it would have been possible using embedded toolchain.

[skywojciechowskim]

I don't agree. I'll try to explain, why (for me) it's obvious, why we shouldn't link test code to production code. Let's imagine such situation: In November 2023 Rialto is much bigger project than it's now and it consists of (let's say) 30 libraries. A tester or developer has got an access to real OCDM library for x86-64 and wants to test rialto on his/her computer. He compiles the code, runs it, but surprisingly videos don't decode. He writes the e-mail to our team with request to fix that problem. We spend few hours to find the problem and we finally discover, that the developer used wrong cmake flag by mistake and linked mocks instead of real ocdm library. With current solution we can treat compilation as a first test (for example Python devs don't have such possibility :)), and the developer/tester from my story above is able to find out that he made a mistake much earlier. You don't have to believe me, but I worked in many projects, much bigger than this one and finding such problem (especially in case when library linked by mistake is very small and don't have debug prints) may take some time. I don't have time to investigate such problems, so I won't accept this review (You can ask Adam, Luke or Stuart about that if you really want to deliver it). BUT if you want to remove RialtoServer for building when UnitTests flag is enabled, I am ok with that idea.

[skywojciechowskim]

And I also don't agree with the argument about catching additional warnings/errors. RialtoServer has got only one additional file - main.cpp. Rest of files can be compiled and linked in UT executables.

[github-actions]

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 71.4%
Functions coverage stays unchanged and is: 67.7%