Drop architecture_independent from rqt_py_common

[cottsay]

When this flag was added back in 2014 (ros-visualization/rqt_common_plugins#254), the package was indeed architecture-independent. Now this package includes shared object libraries which were created as part of message generation, making it architecture-specific.

I'm not sure if these messages need to be distributed with the package, but if they are only used for tests, we should find a way to remove them from installation. That would make this package architecture-independent once again.

[dirk-thomas]

I'm not sure if these messages need to be distributed with the package, but if they are only used for tests, we should find a way to remove them from installation.

The messages seem to be only used for the tests. Therefore I think their installation should be skipped using the option SKIP_INSTALL.

[cottsay]

I think their installation should be skipped using the option SKIP_INSTALL.

I agree, that's a better option. It looks like ros1_bridge is currently dependent on this package, probably because of these messages, so that will likely need to change.

I also need to figure out how to make the messages discoverable to the tests when they're not installed.

Directly dependent packages seem to compile and test fine without the messages, which seems to confirm your analysis. Stay tuned - I'll try to figure out how to make SKIP_INSTALL work.

[dirk-thomas]

It looks like ros1_bridge is currently dependent on this package, probably because of these messages, so that will likely need to change.

If the package doesn't install the interface files it neither needs to be part of the rosidl_interface_packages group nor does it export the interface files. See https://github.com/ros2/rosidl/blob/3988597605a332b6b4254745969524972ffe5683/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake#L222-L236

[cottsay]

This doesn't appear to be a trivial change.

rosidl_generator_py uses the PROJECT_NAME as the module name and doesn't have any way to change it. Since we aren't installing, the generated rqt_py_common module never gets merged with the REAL rqt_py_common module. I can't add the generated module to PYTHONPATH because we can't have two modules with the same name - only one will win.

I also can't reference by going up one directory because the parent directory is called rosidl_generator_py, which is ALSO a real module already.

So unless we plumb through a way to tell rosidl_generator_py to use an alternate module name, I don't see how we can expose the generated messages to the tests cleanly.

It doesn't look like rosidl_generator_py uses the LIBRARY_NAME value at all. The other generators just use it a prefix on the generated .so files. Maybe that is possible?

I'm open to suggestions, but it might be worth just removing the <architecture_independent> tag until we can devise a clean solution for using non-installed generated messages.

[dirk-thomas]

I don't see how we can expose the generated messages to the tests cleanly.

Have you checked how other packages which use interfaces only in their tests use them? Why do the approaches used in the existing cases not work here?

[cottsay]

Have you checked how other packages which use interfaces only in their tests use them? Why do the approaches used in the existing cases not work here?

Here are all of the uses of rosidl_generate_interfaces(... SKIP_INSTALL) I'm aware of:

• rosidl_generator_cpp: Doesn't reference generated messages using Python at all
• rosidl_generator_c: Doesn't reference generated messages using Python at all
• rosidl_generator_py: Doesn't reference both generated messages AND the real module
• test_rclcpp: Doesn't reference generated messages using Python at all
• test_communication: Doesn't reference generated messages using Python at all

rosidl_generator_py did, however, leverage the python module search order's preference toward the current working directory to effectively "prefix" the PYTHONPATH instead of appending to it, meaning that the generated messages will take precedence over the real module.

Unfortunately, that breaks the other test that references the real module. So I'd guess splitting the tests up into ones that require the messages and ones that require the real module will work in this case, but only because there is no intersection between those.

So to answer your question, none of the other approaches work here, but I think it's possible to change this package so that one of the patterns will work here (at least for now).

[cottsay]

...but only because there is no intersection

Nope, I was wrong, the message tests also reference the module itself. Nix that idea.

rqt/rqt_py_common/test/test_rqt_common_unit.py

Lines 40 to 41 in 5549d88

	from rqt_py_common.message_tree_model import MessageTreeModel
	from rqt_py_common.msg import Val, ArrayVal

[cottsay]

@dirk-thomas, do you have any suggestions for how to make the tests work without installing the messages?

If not, I'd like to proceed with dropping <architecture_independent/>.

[cottsay]

@dirk-thomas ping.

do you have any suggestions for how to make the tests work without installing the messages?

[dirk-thomas]

@cottsay Please see #228 which skips the installation of the interfaces.

[dirk-thomas]

Resolved by #228.