Simplify loading of plugins

[justfalter]

I ran into the issue that is ultimately the cause of #5. This commit (3754f57de3ad5082b2e85a7d0292be43a92eaef7) forces the requirement that all plugins be contained within a rubygem. So, pluginator would fail to load any plugins that are in $LOAD_PATH but not in a gem.

I removed the gem activation code as it was duplicating what is already being done by the rubygems' wrapper for 'require'. You can see that there is considerably more logic to the require wrapper than was present in the previous version of 'load_plugin': https://github.com/rubygems/rubygems/blob/master/lib/rubygems/core_ext/kernel_require.rb

This change modifies load_plugin so that it returns true if the path was successfully required, and false if a LoadError occurred. This works for the situation described by #3, which asks that pluginator only load files from activated gems.

Fixes #5

[mpapis]

I am really concerned about https://github.com/rubygems/rubygems/blob/master/lib/rubygems/core_ext/kernel_require.rb#L103 - this will forbid migrating between gem names, maybe put the old code in the rescue block.

[justfalter]

The feature that you're linking to is pretty important, otherwise you end up activating arbitrary versions of gems.

I'm going to attempt to describe the situation that I think you're describing:

You have three versions of 'widget-plugin' installed: 0.1, 0.2, and 0.3.

Here are the plugin files, by version:

• widget-plugin 0.1
  • plugins/group/type/a.rb
  • plugins/group/type/b.rb
• widget-plugin 0.2
  • plugins/group/type/b.rb
  • plugins/group/type/c.rb
• widget-plugin 0.3
  • plugins/group/type/c.rb

Pluginator will find 3 candidate plugins. Things change depending on which file is loaded first:

• plugins/group/type/a.rb loaded first
  • Version 0.1 is activated.
  • a.rb and b.rb load
  • c.rb doesn't load
• plugins/group/type/b.rb loaded first
  • Version 0.2 is activated
  • b.rb and c.rb load
  • a.rb doesn't load
• plugins/group/type/c.rb loaded first
  • Version 0.3 activated
  • c.rb loads
  • a.rb and b.rb don't load.

Looking at this, the most predictable behavior would be to simply use latest available version of a given gem (if one hasn't already activated). So, in the above case, unless something already activated an earlier version of 'widget-plugin', pluginator would look at version 0.3 and only try loading 'plugins/group/type/c.rb'.

So, ultimately the solution would be to:

• Use the result of Gem.find_files only to find all gems that might provide plugins.
• For each unique gem name:
  • Select the activated version of the gem spec, otherwise select the latest version of the gem spec.
  • For each plugin file in the gem specification
  • Kernel#require plugin file

What do you think, @mpapis? At the end of the day, there should be unit tests that validate any of these behaviors.

[mpapis]

between gems:

• develop plugins in let say abc
• split the plugins to a, b & c

[mpapis]

any thoughts on the gem name changing, I already have example for that like => https://github.com/jish/pre-commit/pull/180

[justfalter]

I wouldn't want to do anything special. If you move something from one gem to another, just make sure you release a new version of the original gem that doesn't have the conflicting code.

I would discourage you from trying to have pluginator "handle" such a situation. This is no different than moving code from one gem to another. Any project depending on the code needs to put its big-boy pants on and make sure they are not depending upon gems that have conflicting code.

Mine

On Sep 14, 2014, at 7:32 PM, Michal Papis notifications@github.com wrote:

any thoughts on the gem name changing, I already have example for that like => jish/pre-commit#180

— Reply to this email directly or view it on GitHub.

[mpapis]

assuming new version is released, but in case when you do not bundle exec then you get the old version already installed, cleanup of old versions is not done when installing / updating gems

[justfalter]

Then, like I said in an earlier comment, I would expect pluginator to use the latest versions of any gem that offers plugins.

On Sep 14, 2014, at 8:06 PM, Michal Papis notifications@github.com wrote:

assuming new version is released, but in case when you do not bundle exec then you get the old version already installed, cleanup of old versions is not done when installing / updating gems

— Reply to this email directly or view it on GitHub.

[mpapis]

your code would not allow it as it would fail with

raise Gem::LoadError, "#{path} found in multiple gems: #{names.join ', '}"

from https://github.com/rubygems/rubygems/blob/master/lib/rubygems/core_ext/kernel_require.rb#L103

[justfalter]

See the logic I laid out in my comment, here. My code does not currently implement such a thing.

I tried to create some unit tests for you using https://github.com/rubygems/rubygems/blob/master/lib/rubygems/test_case.rb, but found that bundler interferes with the ability to generate and "install" gems.

I don't know that I can really help you out farther than I have, already. I'm not actively using your gem, so I'm not really vested in seeing this issue through, for you. You need unit tests to validate your plugin-loading behavior otherwise it will be a perpetual pain-point for your project.

[mpapis]

yep, I will add the tests first and then try to improve it

[mpapis]

one more c90191e