Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Inconsistent RTT_COMPONENT_PATH overlay semantics #161

Closed
ahoarau opened this issue Jul 22, 2016 · 8 comments
Closed

Inconsistent RTT_COMPONENT_PATH overlay semantics #161

ahoarau opened this issue Jul 22, 2016 · 8 comments
Milestone

Comments

@ahoarau
Copy link
Contributor

ahoarau commented Jul 22, 2016

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 !

  • workspace 1 contains component A
  • workspace 2 extends workspace 1, and also contains a different copy of component A

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
Copy link
Member

meyerj commented Jul 25, 2016

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"...

  • component libraries loaded later override existing component type names (see @ahoarau's example) --> prefer underlay
  • service plugins providing the same service can be loaded at the same time, but when loading a service by name the first factory function found is called --> prefer overlay
  • typekit plugins that provide the same type can add alias names to types loaded earlier, but the factories installed in TypeInfoGenerator::installTypeInfoObject(TypeInfo *) (e.g. in TemplateTypeInfo). This behavior is closer to prefer underlay unless the typekits are fully compatible or complementary.
  • transport plugins that provide a transport for the same type and with the same id cannot override a transport registered earlier --> prefer overlay

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.

@meyerj meyerj changed the title import components override Inconsistent RTT_COMPONENT_PATH overlay semantics Jul 25, 2016
@ahoarau
Copy link
Contributor Author

ahoarau commented Jul 25, 2016

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

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

@guihomework
Copy link

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
Copy link
Member

meyerj commented Aug 26, 2016

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
Copy link
Contributor Author

ahoarau commented Dec 1, 2017

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

@willcbaker
Copy link

willcbaker commented Apr 17, 2018

can confirm issue

@ahoarau
Copy link
Contributor Author

ahoarau commented Apr 23, 2018

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?

  • faster loading time I guess. +100 for this issue

@meyerj
Copy link
Member

meyerj commented May 6, 2019

Fixed in #288.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants