Move plugins into their own project

[osrf-migration]

Original report (archived issue) by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

While I've been advocating for this for a while, there now seems to be a tangible motive for spinning the plugin framework of ign-common into its own project.

Talking with @wjwwood and @mikaelarguedas , they have expressed a serious interest in using the ignition plugin framework as a replacement for class_loader in ROS2. Our plugin library will provide (pending some backlogged pull requests) a number of powerful features and safety measures that are currently unobtainable in class_loader due to legacy design issues.

The only barrier to adoption is that the plugin framework needs to be provided as its own library, independent from ign-common. I think this is a reasonable and desirable change to make, and it may motivate even wider adoption of the library, going beyond the ROS and Gazebo communities.

I can easily do this right away and migrate the open plugin PRs over to a new project, but first I wanted to get a gauge of the ignition developers' feelings on this.

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

After copying the plugin code, could we retain the current API's but mark them deprecated and point to the new project location?

Also, none of our released code using the ignition plugin code, right? I think it's just ign-gui right now.

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

Yeah, rather than removing the existing released plugin framework, we could mark it as deprecated and refer to the new library. We could also just decline the current open PRs instead of worrying about merging them in. That would probably make for the smoothest transition.

And yes, none of the released code is using the ignition plugin framework yet.

[osrf-migration]

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).

---

1. How much does the final version of the plugin framework rely on other ign-common stuff such as StringUtils and Filesystem? Would we duplicate that code?

2. Would putting the plugins in their own component be enough to address that use case? (see issue #14)

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

[1] The final version of the plugin system does not use StringUtils (the current version on ign-common1 does, I believe, but the version in the latest PR does not). It has no dependence on Filesystem, because the lookup of plugin paths is done by the SystemPaths class which would remain in ign-common. The PluginLoader class requires that it be given the full, exact path of the plugin library, so it does not have any use for additional Filesystem features. This was actually one of the selling points for the ROS folk.

It does currently use some warning suppression features of ign-common, and while I would hate to duplicate those, I don't consider it to be a dealbreaker.

[2] While I am a fan of the components concept, I think this particular case would be an abuse of components, and it would do little if anything to help with broader adoption. The components concept works best when the components are extending the functionality of the base library, but that wouldn't really be the case here. We'd still be submersing the plugin framework into a mess of other, completely unrelated libraries which do nothing to help or extend the features of the plugin framework.

To maximize adoption and usefulness, the plugin framework should be completely free of baggage. Just think about how troublesome it can be to try to use just one component of Boost. While Boost is technically modular, it can be difficult and confusing for users to figure out how to install and link to just one component/module.

[osrf-migration]

Original comment by Ian Chen (Bitbucket: Ian Chen, GitHub: iche033).

---

I like the idea of spinning into something that'll be used by ROS2. Is the problem to do with ign-common's dependencies? The components approach may reduce the number of dependencies when compiling ign-common from source but not sure about the deb versions of ign-common.

Edit: I was late to the discussion. Just saw the reply above which answers my question

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

Right the dependency issue is the main thing, but also it's more of a mess to maintain when it's encased in a project with a miscellaneous group of other libraries that might be getting maintained orthogonally to the plugins. The stability of the plugin framework shouldn't be coupled with the stability of, for example, our audio-visual component. As a component, the version number on the library will be forcibly kept in sync with the version number of ign-common as a whole.

Besides maintainability, I also have concerns about end-user experience:

1. Building from source is more of a chore for users who want to build it from source (and there may be any number of legitimate reasons that they want to / need to do this).

2. The debian would still depend on the base ign-common library, even though it shouldn't be necessary.

(2) can be solved in the Components system by explicitly decoupling the plugin component from the main library (this is an option in the ign_add_component(~) function), but that just begs the question of why not make it its own project if there isn't even a dependency on the base library?

(1) Doesn't really have a solution besides putting it in its own project.

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

It may also be worth pointing out that our plugin framework is soon to be orders of magnitude more sophisticated (for better or worse) than it was originally designed to be when it was first added to the ign-common repo. I suspect that if we had known the level of sophistication it would end up having, we would have created it in its own project from the start.

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

There's been some talk about components. If this isn't a good use case for components, then what is? Maybe ign-math-eigen that we talked about in this issue? I'm genuinely curious, since the Components functionality is still under review.

[osrf-migration]

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).

---

@scpeters , my understanding is that it is suitable for example in the case of ign-rendering, where there is a some core functionality, and other components like ign-rendering-ogre and ign-rendering-optix would extend that. ign-math-eigen also seems to fall into this category for me.

I think the main difference here is that there isn't really an ign-common core which ties all the other components together.

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

We have several examples of good component use in the works. An easy rule of thumb is to split components based on dependencies. The core library should consist of whatever can be implemented with the minimum number of dependencies while still being a useful piece of software. If adding a dependency could extend or expand the usefulness of the core library, then an optional component should be created that leverages that dependency.

ign-math-eigen would be a good use of components, since it introduces a new dependency while expanding the functionality of the core library.

ign-rendering-optix, ign-rendering-ogre, and ign-rendering-<other_rendering_library> would be good uses of components, because the core library provides the value of being a shared interface, and then the components allow you to choose which features or backend(s) you want to use with that interface.

ign-physics-dart vs ign-physics-ode is a good use of components, for reasons similar to ign-rendering.

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

@chapulina 's explanation is spot-on, but I typed mine out before I saw hers.

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

Thanks for clarifying about components.

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

It would be nice if we could preserve history of the plugin files in the new repository, but I'm not sure if that's possible.

EDIT: it sounds like it is possible, though probably tricky.

• https://confluence.atlassian.com/bitbucket/split-a-repository-in-two-313464964.html#Splitarepositoryintwo-UsingMercurialtoSplit-outaDirectory

[osrf-migration]

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

---

Can we call feature-freeze on plugins, review the existing pull requests, and then split it out into a separate library?

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

For preserving version history, there may be some straightforward methods if we do it file-by-file, which I think would be my preferred approach.

I'm a bit conflicted about whether it would be preferable to merge and then split or split and then merge. I can see good arguments for either approach. I would lean more towards splitting before merging because if we start merging these open pull requests before splitting, then these PRs will introduce new features and new behaviors into the ign-common codebase while also breaking ABI, and then we're going to immediately deprecate those features. So we'd be introducing the need for current users of ign-common plugins to update their code, even if they just continue to use the ign-common implementation. That being said, we don't have very many current users of plugins, so maybe that's not much of a concern.

To be clear, if we do the split-before-merging approach, I would recreate all the open pull requests on the new repo, and I would create references to any unaddressed reviewer feedback in the new repo's pull requests.

[osrf-migration]

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

---

We don't need to worry about breaking ABI/API and deprecating code, as long as we don't make a new major release before the split. Your PRs target default. We can merge those in, and then remove the code if we want. We would have to deprecate the plugin code in ign-common1.

I would base the split/merge decision on the amount of time spent reviewing the PRs. If we haven't spent a lot of time reviewing these PRs, then the cost of splitting now and recreating the PRs in a new repo is low. If some or all of the PRs are close to being approved, then we should wrap them up and then split.

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

I think the oldest open PR has had a considerable amount of attention, so it would make a lot of sense to merge that one before splitting if people are willing to expedite some final reviews and approvals for it.

The other three PRs have had relatively little reviewing done (I don't think anyone has been looking at the most recent one yet), so maybe those would be worth deferring until after the split.

[osrf-migration]

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

---

Sounds good. Do you have a name suggestion for the new plugin library?

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

I would recommend ignition-plugin or ignition-plugins. I don't have a strong preference between the two, and I'm totally open to other suggestions.

[osrf-migration]

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

---

The standalone word plugin conveys that the library contains a single plugin or multiple plugins. How about ignition-pluginloader, or some extra word that adds a bit more description to the name?

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

I definitely agree that plugin or plugins alone might make it sound like a repository of plugins, which is the wrong thing to convey.

I was also considering ignition-dl, since the library is building on top of libdl (even on Windows, we build on top of dlfcn-win32, which is a Windows wrapper library that provides the libdl interface). But I'm afraid that name is not very communicative for anyone who isn't familiar with libdl.

With respect to ignition-pluginloader, I had some plans to split the library into ignition-plugin[s], ignition-plugin[s]-loader, and ignition-plugin[s]-register where -loader and -register are components of the main library. Libraries that need to load plugins would link against ignition-plugin[s]-loader while libraries that want to be plugins would link against ignition-plugin[s]-register. That should help users to understand what parts of the plugin framework are meant for loading plugins versus becoming a plugin, and it would allow plugin creators to generate plugins without having a libdl or dlfcn-win32 dependency (it would also help us deal with an issue in GCC, but I won't get into the weeds on that in this post).

Still, if the project name were ignition-plugin[s], then these component names wouldn't help with communicating the intent of the project at the surface level.

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

how about plugnition

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

plugnite

[osrf-migration]

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

---

In that case, ignition-plugin would be better than ignition-plugins as the latter conveys a collection of plugins.

[osrf-migration]

Original comment by Michael Grey (Bitbucket: mxgrey, GitHub: mxgrey).

---

I'm good with ignition-plugin. I'll wait until Monday to create the new project, just in case someone comes up with an objection or a better proposal before then.

[osrf-migration]

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).

---

ignition-plug

[osrf-migration]

Original comment by Shane Loretz (Bitbucket: Shane Loretz, GitHub: sloretz).

---

spark-plugins?

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

• changed state from "new" to "resolved"

!/ignitionrobotics/ign-plugin