Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add ign plugin --info to print plugin info #32

Merged
merged 12 commits into from
Feb 4, 2021

Conversation

scpeters
Copy link
Member

This adds an ign plugin --info command to the loader component that attempts to load a specified shared library and print information about the plugins and interfaces in that file. My motivation for this is to support development of ignition-physics Implementation plugins, though it is useful for other plugins as well. I've added libignition-tools-dev to the packages.apt in this PR and to the homebrew formula in osrf/homebrew-simulation#1213.

$ ign plugin --help
Print information about plugins.

  ign plugin [options]

Options:

  -i [ --info ]              Get info about a plugin.
                             Requires the -p option.

  -p [ --plugin ] arg        Name of a plugin.
                             Required with -i.

  -v [ --verbose ]           Print verbose info.

  -h [ --help ]              Print this help message.
                                                    
  --force-version <VERSION>  Use a specific library version.
                                                    
  --versions                 Show the available versions.

For example, given a path to the DummyPlugins shared library used in unit tests for the loader component, ign plugin --info --plugin lib/libIGNDummyPlugins.dylib generates the following output.

$ ign plugin --info --plugin lib/libIGNDummyPlugins.dylib
Loading plugin library file [lib/libIGNDummyPlugins.dylib]
* Found 3 plugins in library file:
  - test::util::DummyMultiPlugin
  - test::util::DummyNoAliasPlugin
  - test::util::DummySinglePlugin
* Found 7 interfaces in library file:
  - test::util::DummyNameBase
  - test::util::DummyGetObjectBase
  - test::util::DummyIntBase
  - test::util::DummyGetPluginInstancePtr
  - test::util::DummyDoubleBase
  - test::util::DummySetterBase
  - ignition::plugin::EnablePluginFromThis

More verbose output from the Loader::PrettyStr() function can be printed with the --verbose flag.

$ ign plugin --info --plugin lib/libIGNDummyPlugins.dylib --verbose
Loading plugin library file [lib/libIGNDummyPlugins.dylib]
Loader State
	Known Interfaces: 7
		test::util::DummyNameBase
		test::util::DummyGetObjectBase
		test::util::DummyIntBase
		test::util::DummyGetPluginInstancePtr
		test::util::DummyDoubleBase
		test::util::DummySetterBase
		ignition::plugin::EnablePluginFromThis
	Known Plugins: 3
		[test::util::DummyMultiPlugin]
			has 3 aliases:
				[Bar]
				[Baz]
				[Foo]
			implements 7 interfaces:
				ignition::plugin::EnablePluginFromThis
				test::util::DummyDoubleBase
				test::util::DummyGetObjectBase
				test::util::DummyGetPluginInstancePtr
				test::util::DummyIntBase
				test::util::DummyNameBase
				test::util::DummySetterBase
		[test::util::DummyNoAliasPlugin]
			has no aliases
			implements 1 interface:
				test::util::DummyNameBase
		[test::util::DummySinglePlugin]
			has 3 aliases:
				[Alternative name]
				[Bar]
				[Baz]
			implements 1 interface:
				test::util::DummyNameBase
	There are 2 aliases with a name collision:
		[Bar] collides between:
			[test::util::DummyMultiPlugin]
			[test::util::DummySinglePlugin]
		[Baz] collides between:
			[test::util::DummyMultiPlugin]
			[test::util::DummySinglePlugin]


Given a library file, `ign plugin --info` will print the
names of plugins exported within that library and the
interfaces exposed. The --verbose flag can provide
extra information.

Signed-off-by: Steve Peters <[email protected]>
@osrf-triage osrf-triage added this to Inbox in Core development Nov 30, 2020
@github-actions github-actions bot added 🏢 edifice Ignition Edifice 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels Nov 30, 2020
@scpeters scpeters moved this from Inbox to In review in Core development Nov 30, 2020
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider adding a new component for the command line tools, so that users of the plugin loader aren't required to bring ign-tools? The Ruby dependency is often undesired.

loader/CMakeLists.txt Show resolved Hide resolved
loader/src/cmd/cmdplugin.rb.in Outdated Show resolved Hide resolved
Also follow suggestions from review comments.

Signed-off-by: Steve Peters <[email protected]>
@scpeters
Copy link
Member Author

scpeters commented Dec 1, 2020

Did you consider adding a new component for the command line tools, so that users of the plugin loader aren't required to bring ign-tools? The Ruby dependency is often undesired.

From a build perspective, ign-tools isn't a required dependency; it's merely an optional dependency for tests. (For comparison, ruby is a required build dependency for libsdformat, since it is used to generate the schema files). With this PR, we will be configuring and installing some extra files, and one needs to install ignition-tools in order to use them

The only impact it has on the loader is adding one symbol to the loader component library, but that seems small to me compared to the extra effort of adding another component. I think we could create a separate debian package in the metadata to install the ign files without modifying the cmake code any further here

Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
There are some issues with loading plugins from `ign` when run
with /usr/bin/ruby on macOS due to System Integrity Protection.
If a plugin ending in ".dylib" fails to load, print a workaround
suggestion using ruby from brew.

Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
@chapulina
Copy link
Contributor

I think we could create a separate debian package in the metadata to install the ign files without modifying the cmake code any further here

Ok, I think this could be a good separation. So just to clarify, this will be the situation, right?

  • Installing libignition-plugin(-dev) won't install ignition-tools
  • ign_find_package(ignition-plugin1 REQUIRED COMPONENTS loader) won't fail if ignition-tools isn't installed

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me! I tried it with plugins from several libraries.

@@ -3,6 +3,11 @@
# "tests" variable
ign_get_libsources_and_unittests(sources tests)

# Disable ign_TEST if ignition-tools is not found
if (MSVC OR NOT IGNITION-TOOLS_BINARY_DIRS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we open a ticket to track command line support on Windows?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think gazebosim/gz-tools#7 will help with windows support eventually

@scpeters scpeters moved this from In review to In progress in Core development Jan 5, 2021
@chapulina
Copy link
Contributor

@scpeters , is this PR pending anything else?

@scpeters
Copy link
Member Author

scpeters commented Feb 3, 2021

@scpeters , is this PR pending anything else?

hm...I think I was waiting to see how gazebosim/gz-transport#216 turned out, but I guess we would need ign-utils to follow that pattern

I think this could be fine as is since I don't think we would add ign-utils as a dependency on ign-plugin1

@chapulina
Copy link
Contributor

Yeah we could do the current approach on ign-plugin1 and update it to use ign-utils on ign-plugin2 if that's desired.

@scpeters scpeters merged commit 744c3b9 into gazebosim:ign-plugin1 Feb 4, 2021
Core development automation moved this from In progress to Done Feb 4, 2021
@scpeters scpeters deleted the ign_plugin_loader branch February 4, 2021 21:58
@chapulina
Copy link
Contributor

Oops, this broke downstream CI:

https://build.osrfoundation.org/job/ign_gui-pr-win/625/console

CMake Error in loader/src/cmd/CMakeLists.txt:
  Evaluation file to be written multiple times with different content.  This
  is generally caused by the content evaluating the configuration type,
  language, or location of object files:

   D:/Jenkins/workspace/ign_gui-pr-win/ws/build/ignition-plugin1/test/lib/ruby/ignition/cmdplugin1.rb

@scpeters
Copy link
Member Author

scpeters commented Feb 5, 2021

Oops, this broke downstream CI:

I just noticed this. I'll revert unless I find a quick fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants