Skip to content

Conversation

@dosyfier
Copy link

@dosyfier dosyfier commented Sep 19, 2022

This pull request is an appendix to #815 (which was prematurely merged).

It concludes the plugin migration towards Redmine 5 initiated by #813.

It corrects:

  • the loading of all Redmine Git Hosting classes, triggered by RedmineGitHosting.load_plugin!, which wasn't called anymore;
  • the loading of Redmine Git Hosting's Commands and Config modules, which failed in an infinite loading loop.

The block registered by redmine_git_hosting plugin to
Rails.application.config.to_prepare wasn't actually called (at least, in
a test context).

This appears to be due to the fact that the plugin's init.rb file is
already loaded from within a Rails "to_prepare" initialization event
block. Cf. Redmine's PluginLoader class and its "load" method defined in
lib/redmine/plugin_loader.rb file: a Rails "to_prepare" block iterates
over all plugin directory to find and load an "init.rb" file.

As a consequence, there's no need to rewrap some code from the plugin's
init.rb file into a Rails "to_prepare" event handling block.
Maybe due to upgrading to Ruby 3.x, the code of these modules couldn't
be loaded anymore, due to an infinite loading loop.

For instance, for the Commands module:
* Loading of commands.rb is triggered,
  * As its first instruction, the module extends Commands::Base,
  * So the loading of commands/base.rb is triggered,
    * But then, in that file, the Base module is included in the
      Commands module, which has to be loaded first,
      * And so, we're back to the beginning...

To work around this, the "extend Xx::Xx" instructions are moved to the
submodules themselves, so that each submodule can be loaded
independently and add its contributions to the main module (Commands or
Config).
@dosyfier dosyfier force-pushed the redmine5 branch 2 times, most recently from b838f30 to f628fb9 Compare September 19, 2022 18:16
After upgrading to Ruby 3.1, following error raises when running
redmine_git_hosting tests depending on loading a fixture based on a YAML
file:

    Psych::DisallowedClass:
           Tried to load unspecified class: Time
@PowerKiKi PowerKiKi merged commit d3a1c45 into redmine-git-hosting:redmine5 Sep 26, 2022
@PowerKiKi
Copy link
Collaborator

This is great, thank you !

Is there anything else you would like to do before we cut a release ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants