The Plugin::Instance.find_all method incorrectly treats every file with the «plugin.rb» name as a Discourse plugin

The incorrect code:

https://github.com/discourse/discourse/blob/v1.7.0.beta10/lib/plugin/instance.rb#L42-L42

It will fail if a plugin requires a gem, which contain a file with the plugin.rb name.
An example of problem gem is airbrake.

If a plugin requires the airbrake gem, then Discourse incorrectly theats the lib/airbrake/delayed_job/plugin.rb file as a Discourse plugin, and fails with the message:

/vagrant/plugins/df-core/gems/2.3.1/gems/airbrake-5.6.1/lib/airbrake/delayed_job/plugin.rb:6:in <module:Plugins>': uninitialized constant Delayed (NameError) from /vagrant/plugins/df-core/gems/2.3.1/gems/airbrake-5.6.1/lib/airbrake/delayed_job/plugin.rb:2:in module:Delayed
from /vagrant/plugins/df-core/gems/2.3.1/gems/airbrake-5.6.1/lib/airbrake/delayed_job/plugin.rb:1:in activate!' from /vagrant/lib/plugin/instance.rb:312:in instance_eval’
from /vagrant/lib/plugin/instance.rb:312:in activate!' from /vagrant/lib/discourse.rb:108:in block in activate_plugins!’
from /vagrant/lib/discourse.rb:105:in each' from /vagrant/lib/discourse.rb:105:in activate_plugins!’
from /vagrant/config/application.rb:165:in <class:Application>' from /vagrant/config/application.rb:19:in module:Discourse
from /vagrant/config/application.rb:18:in <top (required)>' from /home/vagrant/.rvm/gems/ruby-2.3.1/gems/railties-4.2.7.1/lib/rails/commands/commands_tasks.rb:78:in require’
from /home/vagrant/.rvm/gems/ruby-2.3.1/gems/railties-4.2.7.1/lib/rails/commands/commands_tasks.rb:78:in block in server' from /home/vagrant/.rvm/gems/ruby-2.3.1/gems/railties-4.2.7.1/lib/rails/commands/commands_tasks.rb:75:in tap’

6 Likes

good point, this is a legit issue.

Moreover, looks like the buggy code is also unefficient, because it searches for the «plugin.rb» files in all the filesystem tree inside the «plugins» directory, despite the Discourse’s «plugin.rb» files are always on the second level in this tree.

btw. PR totally welcome to fix this, should be simple.

3 Likes

As I have checked in the Ruby debugger, the following code works correctly for both the real directories and symlinks:

Dir["#{parent_path}/*/plugin.rb"].sort.each do |path|

4 Likes

Fixed per:

https://github.com/discourse/discourse/commit/6a1f579c6e6f93579a9269acf58e7109072b943f

3 Likes