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

Bug
1

The incorrect code:

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
2

good point, this is a legit issue.

3

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.

4

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

3 Likes
5

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
6

Fixed per:

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

3 Likes