orocos-toolchain / rtt

Orocos Real-Time Toolkit
http://www.orocos.org
Other
72 stars 79 forks source link

Inconsistent RTT_COMPONENT_PATH overlay semantics #161

Closed ahoarau closed 5 years ago

ahoarau commented 8 years ago

When I import a component from an overlay workspace it always takes the last on the ROS_PACKAGE_PATH list. The order of priority is reversed !

if I load component A (import("A")) it will output :

# blabla loading from component A from workspace2 --> success !
# but then, loading component A from workspace 1 (why ??) --> success
 [ Warning][ComponentLoader::import(path_list)] Component type name A already used: overriding.

This seems to be the culprit.

meyerj commented 8 years ago

This is a known problem and RTT can simply not deal with overlay workspaces that provide the same component types or plugins with the same name at the moment.

I somehow remember the discussion back in 2014 about whether to reverse paths in the RTT_COMPONENT_PATH or not, but I cannot find a reference now. The real problem is that the ComponentLoader, the PluginLoader and the TypeInfoRepository, the classes who load the plugins or hold information about known components, services and typekits, have inconsistent behaviors. Without reversing the entries in the RTT_COMPONENT_PATH given as "/path/to/overlay:/path/to/underlay"...

Last but not least the prefer overlay vs. prefer underlay behavior should be compatible with the build system's and the linker's notion of variables like CMAKE_PREFIX_PATH, LD_LIBRARY_PATH, PATH, PKG_CONFIG_PATH etc., which is typically prefer overlay. We do not want to link to a typekit plugin library in an overlay but load the plugin from the underlay during run-time.

The import process itself in intermingled (e.g. the ComponentLoader calls into the PluginLoader singleton during the process of importing a package), so that reversing the paths in some cases only is also not an option.

You could consider any of the above behaviors as a bug, but it has been decided to not break the pre-catkin behavior of RTT only to fully support catkin overlays. Probably a clean rewrite of the PluginLoader and ComponentLoader classes, which contain lot of redundant code for historic reasons, and a previous design phase would be the best viable option here.

ahoarau commented 8 years ago

1) we could return true here and it's back to prefer overlay. 3) isn't that a very rare special case ?

But also stop the search once a good one is found.

guihomework commented 8 years ago

Just fell on the exact same issue. As a workaround, we enforced the full path to the overlay devel/lib/orocos in import during the development phase, but this is not a nice development setup.

Hope a clean/consistent solution can be found some day.

meyerj commented 8 years ago

It would be an option to simply load a package from the first path found in ComponentLoader::importInstalledPackage(). This should fix the overlay semantics of the import() operation and would not break the component or typekit overwrite behavior of a subsequent import call of another package. Not sure what has been the original use case to import a single package from multiple locations found. @psoetens?

ahoarau commented 6 years ago

@meyerj @psoetens gentle ping on this issue. Is there any real world applications were you need to load the last found component/plugin ?

willcbaker commented 6 years ago

can confirm issue

ahoarau commented 6 years ago

It would be an option to simply load a package from the first path found in ComponentLoader::importInstalledPackage(). This should fix the overlay semantics of the import() operation and would not break the component or typekit overwrite behavior of a subsequent import call of another package. Not sure what has been the original use case to import a single package from multiple locations found. @psoetens?

meyerj commented 5 years ago

Fixed in #288.