Skip to content

Conversation

@dosyfier
Copy link

@dosyfier dosyfier commented Sep 10, 2022

With the works of @PowerKiKi and @prahal, I managed to make redmine_git_hosting plugin work with Redmine 5.

Here is a recap of what this PR brings to the existing "redmine5" branch:

With these changes, here's what I was able to test:

  • start a Redmine 5 instance with the redmine_git_hosting plugin,
  • browse the redmine_git_hosting settings page,
  • register an SSH key into Redmine,
  • create / delete a gitolite repository,
  • push changes to a gitolite repository and see the result in Redmine afterwards.

Nota: This PR is based upon the work done in following PRs:

…ents

Ruby 3.0 does not implicitly destructure a Hash into keyword arguments
if it was the last parameter to a method call.

Also handle explicitely delegate keyword arguments.
Seems we only want an optional hash not keyword arguments so
remove the double sprat and set a default empty hash value.
…Cache>::Timezone

Typo Timezone.new instead of Time.zone.now
@PowerKiKi
Copy link
Collaborator

Thanks a lot for this work. I see all tests are still failing when running bundle exec rake db:test:prepare. Would you be able to make tests pass, so we can have more confidence that the plugin would still work completely ?

@dosyfier
Copy link
Author

Thanks a lot for this work. I see all tests are still failing when running bundle exec rake db:test:prepare. Would you be able to make tests pass, so we can have more confidence that the plugin would still work completely ?

Agreed, that's a must have!

I'll work on it. Not sure though how long the build has been broken, but at first look, it looks related to some dependencies (like redmine_sidekiq) not being compatible with Rails 6.

This new gitolite-rugged version brings rugged v1.5.x instead of v1.1.x,
which allows gitolite-rugged to specify credentials when pushing to a
git (which is expected for it to work correctly).

By the way:
* pkg-config is necessary to build rugged native C extensions;
* starting from v1.5, rugged expects an explicit setting to build with
  SSH support, which is expected by redmine_git_hosting to work
  correctly (see: https://github.com/libgit2/rugged/tree/v1.5.0#options)
The options hash passed to repository constructor is duplicated.

See lib/redmine_git_hosting/gitolite_handlers/repositories/base.rb#13

Thus, options should be updated by accessing the class' attribute
directly.
@PowerKiKi
Copy link
Collaborator

Damned, I accidentally merged the commit into redmine5 branch, even though the work is not finished yet.

@dosyfier would you mind opening a new (draft) PR with the rest of you work please ?

@dosyfier
Copy link
Author

Hi @PowerKiKi,
No worries for the merge. Yet, I haven't gone further with the tests to fix.
I think I solved the issue with redmine-sidekiq (thanks for the PR btw). Now I can go to the testing step of the pipeline, but there are lots of tests failing...

It looks like the patches brought by redmine_git_hosting (from the lib/redmine_git_hosting/patches/ folder) are not loaded at Rails startup. There's some code at the end of lib/redmine_git_hosting.rb which purpose is to load these patches:

# Set up autoload of patches                                                                                                                                                                                                                                                  
Rails.config.to_prepare do
  # Redmine Git Hosting Libs and Patches
  RedmineGitHosting.load_plugin!

  # Redmine SCM adapter
  require_dependency 'redmine/scm/adapters/xitolite_adapter'

  require 'hrack/bundle'
end

but it isn't called at Redmine startup.

I attempted to change Rails.config.to_prepare into Rails.application.config.to_prepare (my last commit on the PR), but it didn't change anything.

I'm no rails expert, so it may take me some time to investigate, but I'll try again probably next weekend, and hopefully I'll open a new PR :)

@prahal
Copy link

prahal commented Sep 14, 2022

Not an answer but clues:

in redmine_plugin_loader

        def required_lib_dirs
	  plugin_lib_dir plugin_name, '**', '*.rb'
	end

        def plugin_dir(*dirs)
          Rails.root.join('plugins', plugin_name, *dirs)
        end

        def plugin_lib_dir(*dirs)
          plugin_dir('lib', *dirs)
        end

	def load_plugin!
	  autoload_libs!
	  autoload_paths!
	  autoload_locales!
	end


	def autoload_libs!
	  Dir.glob(required_lib_dirs).each do |file|
		require_dependency file unless skip_lib_file? file
	  end
	end
    def plugin_hooks_dir
      plugin_lib_dir plugin_name, 'hooks'
    end

    def hook_file?(file)
      File.dirname(file) == plugin_hooks_dir.to_s
    end

    def skip_lib_file?(file)
      # Exclude Redmine Views Hooks from Rails loader to avoid multiple calls to hooks on reload in dev environment.
      true if hook_file?(file) || (file.include?('journal_logger.rb') && !Object.const_defined?('Account'))
    end

autoload_libs is the function that copes with lib/redmine_git_hosting/ subdirs.
It loads all ruby files in all the subdirs of /lib/redmine_git_hosting/ except journal_logger.rb like files when Account is already defined, and except /lib/redmine_git_hosting/hooks (the latter do not exists anymore since https://github.com/jbox-web/redmine_git_hosting/commits/master/lib/redmine_git_hosting/hooks ie commit 4201717 , this is at best a leftover).

As far as I know rails now uses zeitwerk autoloaders which autoload all ruby classes matching their file hierarchy (ie /lib/redmine_git_hosting/test.rb when it contains the definition of RedmineGitHosting::test.rb.

ie from https://guides.rubyonrails.org/autoloading_and_reloading_constants.html

Rails manages a couple of Zeitwerk loaders on your behalf.
In a Rails application file names have to match the constants they define, with directories acting as namespaces.
but also
Idiomatic Rails applications only issue require calls to load stuff from their lib directory, the Ruby standard library, Ruby gems, etc. That is, anything that does not belong to their autoload paths, explained below.

I did add /lib to the autoloader path in my patchset. You might want to try with the code for that too, ie https://github.com/jbox-web/redmine_git_hosting/pull/807/files#diff-4afdb3c3c34a069c37756ebaee5883f21565d29c7730ba4565b88aa809d226cc .

haru/redmine_wiki_extensions@91499cc
ie in the plugin init.rb
$LOAD_PATH.unshift "#{File.dirname(FILE)}/lib"
(add plugin lib to search path).

It fixed a:
LoadError: cannot load such file -- redmine_git_hosting issue.

@dosyfier
Copy link
Author

Hi @prahal,
Thanks for the hint.

Actually, your $LOAD_PATH.unshift "#{File.dirname(FILE)}/lib" was already added to the plugin's init.rb, but there were still things that weren't loaded. Maybe it's what the autoload_paths! function was meant for?

Anyway, I think I managed to make it work with 23d1541.

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.

3 participants