SCENARIO_USE_SYSTEM_TPL does not work

[traversaro]

The SCENARIO_USE_SYSTEM_TPL option (see https://github.com/robotology/gym-ignition/blob/v1.0beta2/deps/CMakeLists.txt#L11) assumes that tiny-process-library installs a CMake config for the tiny-process-library CMake package, and that this CMake package exports a tiny-process-library imported target, but none of these two hypothesis are true for the current version of tiny-process-library.

[diegoferigo]

Thanks for reporting @traversaro! Yes that's right, at the moment there's nothing exported by the upstream tiny-process-library. What approach would you recommend in this case? At first though, the quickest workaround would be developing a Find<>.cmake that tries to locate the library in the default system folders.

[traversaro]

At first though, the quickest workaround would be developing a Find<>.cmake that tries to locate the library in the default system folders.

In theory yes, but let's see if we can fix the problem upstream (see https://gitlab.com/eidheim/tiny-process-library/-/merge_requests/38/diffs) so that we don't have complexity downstream.

[traversaro]

At first though, the quickest workaround would be developing a Find<>.cmake that tries to locate the library in the default system folders.

In theory yes, but let's see if we can fix the problem upstream (see https://gitlab.com/eidheim/tiny-process-library/-/merge_requests/38/diffs) so that we don't have complexity downstream.

The MR has been merged, so I think that we can fix the issue also here now.

[diegoferigo]

Awesome! I had a quick look to the current situation, and I want to explain and double check the situation with this dependency. Please @traversaro correct me if you notice anything wrong.

To avoid any problem with the export of ScenarIO, we compile the vendored tiny-process-library as OBJECT library:

https://github.com/robotology/gym-ignition/blob/ae7b5f4f600913c303ca478d78876c5b833ae994/deps/CMakeLists.txt#L81-L83

Then, all our targets that link against this object will produce a library / executable with that code included.

This being said, let's analyze the following two cases:

1. TPL is not found is the system. Our targets link to the OBJECT library as explained above and nothing TPL-related is installed. Downstream projects that import ScenarIO do not need to have TPL installed in their system.
2. TPL is found in the system as SHARED library. Our targets link to the SHARED library, and since TPL is a PRIVATE dependency, downstream projects do not need to link against it. However, their system must have TLP installed (I guess that our EXPORT should define TPL as (private?) dependency in this case).

The modification required to achieve it are minimal.

Then, when packaging, we will set SCENARIO_USE_SYSTEM_TPL=OFF, so that downstream projects do not have to install separately TPL. This is the most delicate point. Sometimes there are problem if an application loads 1) ScenarIO with TPL included and 2) another library that loads a system TPL. It's not clear to me how to address this specific use case.

[traversaro]

Sometimes there are problem if an application loads 1) ScenarIO with TPL included and 2) another library that loads a system TPL. It's not clear to me how to address this specific use case.

That is the classical problem when vendoring dependencies. On ELF system everything is fine when both TPLs have the same ABI, while you may get a mysterious runtime crash if their ABI are different. The only way to avoid this problems is either hide the TPL symbols, or to change the symbols name to add a namespace, so they don't get mixed with the real TPL symbols. See robotology/how-to-export-cpp-library#46 (comment) for more details. However I do not know if the complexity of the additional machinery for hiding symbols or changing their name is worth the benefit.

[diegoferigo]

That is the classical problem when vendoring dependencies. On ELF system everything is fine when both TPLs have the same ABI, while you may get a mysterious runtime crash if their ABI are different. The only way to avoid this problems is either hide the TPL symbols, or to change the symbols name to add a namespace, so they don't get mixed with the real TPL symbols. See robotology/how-to-export-cpp-library#46 (comment) for more details. However I do not know if the complexity of the additional machinery for hiding symbols or changing their name is worth the benefit.

Mmh if I understood, hiding / scoping the symbols has to be done in the external library, right? On the fly I cannot see how it can be done without editing those sources.

Since the library is small, we're bypassing their CMake project and create a new target on the fly in the vendored case. Here in theory we can actually hide all symbols of this target by default since we control it. This is the problematic case, do you think this is the right way and is worth spending some time on it? I never tried to hide symbols and I'm not sure it can be done on a single-target basis without operating on the sources.

[traversaro]

This is the problematic case, do you think this is the right way and is worth spending some time on it?

Personal evaluation: I think that the effort require to tackle this corner case is more then the benefit of the problems that we can avoid.

[diegoferigo]

This is the problematic case, do you think this is the right way and is worth spending some time on it?

Personal evaluation: I think that the effort require to tackle this corner case is more then the benefit of the problems that we can avoid.

Ok then, I'll fix what I can fix after the MR and let's keep this caveat in mind in case related problems emerge.