From cf54c418cafef2c065d12ad3736b4a1bcde0c15e Mon Sep 17 00:00:00 2001 From: Sean Millichamp Date: Fri, 23 Apr 2021 08:04:46 -0400 Subject: [PATCH] Add Gem hook plugin support Support loading Overcommit hooks supplied by Gems in the same fashion as hooks that can be provided in Repo-Specific hooks. --- README.md | 36 +++++++++++ config/default.yml | 13 ++++ lib/overcommit.rb | 1 + lib/overcommit/configuration.rb | 20 ++++++ lib/overcommit/configuration_validator.rb | 38 ++++++++++++ lib/overcommit/hook_loader/gem_hook_loader.rb | 14 +++++ lib/overcommit/hook_runner.rb | 5 ++ spec/overcommit/configuration_spec.rb | 58 +++++++++++++++++ .../configuration_validator_spec.rb | 62 ++++++++++++++++++- .../hook_loader/gem_hook_loader_spec.rb | 31 ++++++++++ spec/overcommit/hook_runner_spec.rb | 56 +++++++++++++++++ 11 files changed, 333 insertions(+), 1 deletion(-) create mode 100644 lib/overcommit/hook_loader/gem_hook_loader.rb create mode 100644 spec/overcommit/hook_loader/gem_hook_loader_spec.rb create mode 100644 spec/overcommit/hook_runner_spec.rb diff --git a/README.md b/README.md index 36126a2c..153902cd 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,7 @@ any Ruby code. * [PreRebase](#prerebase) * [Repo-Specific Hooks](#repo-specific-hooks) * [Adding Existing Git Hooks](#adding-existing-git-hooks) +* [Gem-provided Hooks](#gem-provided-hooks) * [Security](#security) * [Contributing](#contributing) * [Community](#community) @@ -671,6 +672,41 @@ of hook, see the [git-hooks documentation][GHD]. [GHD]: https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks +## Gem-provided Hooks + +For hooks which should be available from multiple repositories, but which do not +make sense to contribute back to Overcommit, you can package them as a Ruby gem +and `overcommit` can use them as if they were repo-specific hooks. This would be +useful for an organization that has custom hooks they wish to apply to many +repositories, but manage the hooks in a central way. + +To enable gem-provided hooks you need to set the configuration option +`gem_plugins_enabled` to `true`. + +If you are using the [`gemfile`](#gemfile) configuration option or otherwise +invoking Overcommit via Bundler then you must ensure the gems are listed +in your Bundler configuration and Bundler will ensure that the gems are in +Ruby's `$LOAD_PATH` so that Overcommit can find and load them. + +If you are not using Bundler (the gems are installed directly in your Ruby +environment) then you must provide a path that Overcommit can `require` so +the gem will be added to the `$LOAD_PATH`. + +```yaml +gem_plugins_require: ['overcommit_site_hooks'] +``` + +The above would result in a `require 'overcommit_site_hooks'`. The file does +not have to implement any functional code, but merely be a placeholder sufficient +to allow the `require` to work. + +The hooks themselves are implemented similarly to Repo-Specific Hooks, but need +to be placed in a gem in the normal `overcommit` hook path within the gem. For +example, to add `MyCustomHook` as a pre_commit hook you would put it here: +`./lib/overcommit/hooks/pre_commit/my_custom_hook.rb` within the gem file structure. + +See [Repo-Specific Hooks](#repo-specific-hooks) for details. + ## Security While Overcommit can make managing Git hooks easier and more convenient, diff --git a/config/default.yml b/config/default.yml index bfd91e08..d533241d 100644 --- a/config/default.yml +++ b/config/default.yml @@ -34,6 +34,19 @@ # (Generate lock file by running `bundle install --gemfile=.overcommit_gems.rb`) gemfile: false +# If enabled, Overcommit will look for plugins provided anywhere in the +# load path and load them as it would with built-in or repo-specific +# hooks, allowing plugins to be provided by gems. +# Note: If you are providing them as system gems (and not using `gemfile` +# or a Bundler context where they are specified) then you must also set the +# `gem_plugins_require` option. +gem_plugins_enabled: true + +# If `gem_plugins_enabled` is true and you are using system gems (not via a +# Bundler context), in order to allow Ruby to find the gems, you must list a +# valid `require` path for each gem providing custom hooks here: +# gem_plugins_require: [ 'overcommit_custom_hook' ] + # Where to store hook plugins specific to a repository. These are loaded in # addition to the default hooks Overcommit comes with. The location is relative # to the root of the repository. diff --git a/lib/overcommit.rb b/lib/overcommit.rb index 15fef94f..bb6a3e7e 100644 --- a/lib/overcommit.rb +++ b/lib/overcommit.rb @@ -17,6 +17,7 @@ require 'overcommit/hook_signer' require 'overcommit/hook_loader/base' require 'overcommit/hook_loader/built_in_hook_loader' +require 'overcommit/hook_loader/gem_hook_loader' require 'overcommit/hook_loader/plugin_hook_loader' require 'overcommit/interrupt_handler' require 'overcommit/printer' diff --git a/lib/overcommit/configuration.rb b/lib/overcommit/configuration.rb index ae3efa50..73d7782c 100644 --- a/lib/overcommit/configuration.rb +++ b/lib/overcommit/configuration.rb @@ -119,6 +119,15 @@ def enabled_builtin_hooks(hook_context) select { |hook_name| hook_enabled?(hook_context, hook_name) } end + # Returns the gem-provided hooks that have been enabled for a hook type. + def enabled_gem_hooks(hook_context) + @hash[hook_context.hook_class_name].keys. + reject { |hook_name| hook_name == 'ALL' }. + reject { |hook_name| built_in_hook?(hook_context, hook_name) }. + select { |hook_name| gem_hook?(hook_context, hook_name) }. + select { |hook_name| hook_enabled?(hook_context, hook_name) } + end + # Returns the ad hoc hooks that have been enabled for a hook type. def enabled_ad_hoc_hooks(hook_context) @hash[hook_context.hook_class_name].keys. @@ -259,6 +268,7 @@ def ad_hoc_hook?(hook_context, hook_name) # Ad hoc hooks are neither built-in nor have a plugin file written but # still have a `command` specified to be run !built_in_hook?(hook_context, hook_name) && + !gem_hook?(hook_context, hook_name) && !plugin_hook?(hook_context, hook_name) && (ad_hoc_conf['command'] || ad_hoc_conf['required_executable']) end @@ -270,8 +280,18 @@ def built_in_hook?(hook_context, hook_name) hook_context.hook_type_name, "#{hook_name}.rb")) end + def gem_hook?(hook_context, hook_name) + hook_name = Overcommit::Utils.snake_case(hook_name) + + $LOAD_PATH.any? do |path| + File.exist?(File.join(path, 'overcommit', 'hook', + hook_context.hook_type_name, "#{hook_name}.rb")) + end + end + def hook_exists?(hook_context, hook_name) built_in_hook?(hook_context, hook_name) || + gem_hook?(hook_context, hook_name) || plugin_hook?(hook_context, hook_name) || ad_hoc_hook?(hook_context, hook_name) end diff --git a/lib/overcommit/configuration_validator.rb b/lib/overcommit/configuration_validator.rb index ce1eb317..55d15d04 100644 --- a/lib/overcommit/configuration_validator.rb +++ b/lib/overcommit/configuration_validator.rb @@ -23,6 +23,7 @@ def validate(config, hash, options) check_for_missing_enabled_option(hash) unless @options[:default] check_for_too_many_processors(config, hash) check_for_verify_plugin_signatures_option(hash) + check_for_gem_plugins(hash) hash end @@ -181,6 +182,43 @@ def check_for_verify_plugin_signatures_option(hash) @log.newline end end + + # check for presence of Gems specified in gem_plugins by trying to load them + def check_for_gem_plugins(hash) + return unless @log + return unless hash['gem_plugins_enabled'] == true + return unless hash.key?('gem_plugins_require') + + required = hash['gem_plugins_require'] + + unless required.is_a?(Array) + @log.error 'gem_plugins_require expects a list value to be set' + raise Overcommit::Exceptions::ConfigurationError, + 'gem_plugins_require expects a list value' + end + + errors = [] + + required.each do |path| + begin + require path + rescue LoadError + errors << "Unable to require path '#{path}' listed in gem_plugins_require." + end + @log.debug "Successfully loaded gem_plugins_require: #{path}" + end + + return unless errors.any? + + errors << 'Ensure that the gems providing requested gem_plugins_require are ' \ + 'installed on the system or specify them via the gemfile ' \ + 'configuration option' + + @log.error errors.join("\n") + @log.newline + raise Overcommit::Exceptions::ConfigurationError, + 'One or more gems specified in gem_plugins_require could not be loaded' + end end end # rubocop:enable Metrics/ClassLength, Metrics/CyclomaticComplexity, Metrics/MethodLength diff --git a/lib/overcommit/hook_loader/gem_hook_loader.rb b/lib/overcommit/hook_loader/gem_hook_loader.rb new file mode 100644 index 00000000..9e57d3f3 --- /dev/null +++ b/lib/overcommit/hook_loader/gem_hook_loader.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Overcommit::HookLoader + # Responsible for loading custom hooks that users install via Gems + class GemHookLoader < Base + def load_hooks + @config.enabled_gem_hooks(@context).map do |hook_name| + underscored_hook_name = Overcommit::Utils.snake_case(hook_name) + require "overcommit/hook/#{@context.hook_type_name}/#{underscored_hook_name}" + create_hook(hook_name) + end + end + end +end diff --git a/lib/overcommit/hook_runner.rb b/lib/overcommit/hook_runner.rb index 8ac01e99..c0b01420 100644 --- a/lib/overcommit/hook_runner.rb +++ b/lib/overcommit/hook_runner.rb @@ -200,6 +200,11 @@ def load_hooks @hooks += HookLoader::BuiltInHookLoader.new(@config, @context, @log).load_hooks + # Load Gem-based hooks next, if gemfile in use or gem_plugins is explicitly enabled: + if @config['gem_plugins_enabled'] == true + @hooks += HookLoader::GemHookLoader.new(@config, @context, @log).load_hooks + end + # Load plugin hooks after so they can subclass existing hooks @hooks += HookLoader::PluginHookLoader.new(@config, @context, @log).load_hooks rescue LoadError => ex diff --git a/spec/overcommit/configuration_spec.rb b/spec/overcommit/configuration_spec.rb index d9860400..ee1c6f75 100644 --- a/spec/overcommit/configuration_spec.rb +++ b/spec/overcommit/configuration_spec.rb @@ -294,4 +294,62 @@ end end end + + describe '#enabled_gem_hooks' do + let(:hash) do + { + 'PreCommit' => { + 'MyCustomHook' => { + 'enabled' => true + }, + 'MyOtherHook' => { + 'enabled' => false + }, + } + } + end + + let(:context) { double('context') } + subject { config.enabled_gem_hooks(context) } + + before do + context.stub(hook_class_name: 'PreCommit', + hook_type_name: 'pre_commit') + end + + it 'excludes hooks that are not found' do + subject.should_not include 'MyCustomHook' + subject.should_not include 'MyOtherHook' + end + + context 'when custom hooks are found' do + before do + $LOAD_PATH.unshift('/my/custom/path/lib') + allow(File).to receive(:exist?). + with('/my/custom/path/lib/overcommit/hook/pre_commit/my_custom_hook.rb'). + and_return(true) + allow(File).to receive(:exist?). + with(File.join(Overcommit::HOME, 'lib/overcommit/hook/pre_commit/my_custom_hook.rb')). + and_return(false) + allow(File).to receive(:exist?). + with('/my/custom/path/lib/overcommit/hook/pre_commit/my_other_hook.rb'). + and_return(true) + allow(File).to receive(:exist?). + with(File.join(Overcommit::HOME, 'lib/overcommit/hook/pre_commit/my_other_hook.rb')). + and_return(false) + end + + after do + $LOAD_PATH.shift + end + + it 'includes hooks that are enabled and found' do + subject.should include 'MyCustomHook' + end + + it 'excludes hooks that are not enable but found' do + subject.should_not include 'MyOtherHook' + end + end + end end diff --git a/spec/overcommit/configuration_validator_spec.rb b/spec/overcommit/configuration_validator_spec.rb index 52320633..dcd3654a 100644 --- a/spec/overcommit/configuration_validator_spec.rb +++ b/spec/overcommit/configuration_validator_spec.rb @@ -7,8 +7,9 @@ let(:logger) { Overcommit::Logger.new(output) } let(:options) { { logger: logger } } let(:config) { Overcommit::Configuration.new(config_hash, validate: false) } + let(:instance) { described_class.new } - subject { described_class.new.validate(config, config_hash, options) } + subject { instance.validate(config, config_hash, options) } context 'when hook has an invalid name' do let(:config_hash) do @@ -115,4 +116,63 @@ end end end + + context 'when gem_plugins_require is set' do + let(:plugins_enabled) { true } + let(:plugins_require) { nil } + + let(:config_hash) do + { + 'gem_plugins_enabled' => plugins_enabled, + 'gem_plugins_require' => plugins_require, + } + end + + context 'when plugins_enabled is true' do + let(:plugins_enabled) { true } + + context 'and it is not an array' do + let(:plugins_require) { true } + + it 'raises an error' do + expect { subject }.to raise_error Overcommit::Exceptions::ConfigurationError + end + end + + context 'and one does not load' do + let(:plugins_require) { %w[mygem missinggem] } + + before do + allow(instance).to receive(:require).with('mygem').and_return(true) + allow(instance).to receive(:require).with('missinggem').and_raise(LoadError) + end + + it 'raises an error' do + expect(logger).to receive(:error).with(/installed on the system/) + + expect { subject }.to raise_error Overcommit::Exceptions::ConfigurationError + end + end + + context 'and the gems load' do + let(:plugins_require) { ['mygem'] } + + it 'is valid' do + expect(instance).to receive(:require).with('mygem').and_return(true) + + expect { subject }.not_to raise_error + end + end + end + + context 'when plugins_enabled is false' do + let(:plugins_enabled) { false } + let(:plugins_require) { ['one'] } + + it 'loads nothing' do + expect(instance).not_to receive(:require) + subject + end + end + end end diff --git a/spec/overcommit/hook_loader/gem_hook_loader_spec.rb b/spec/overcommit/hook_loader/gem_hook_loader_spec.rb new file mode 100644 index 00000000..4f72290e --- /dev/null +++ b/spec/overcommit/hook_loader/gem_hook_loader_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Overcommit::HookLoader::GemHookLoader do + let(:hash) { {} } + let(:config) { Overcommit::Configuration.new(hash) } + let(:logger) { double('logger') } + let(:context) { double('context') } + let(:loader) { described_class.new(config, context, logger) } + + describe '#load_hooks' do + subject(:load_hooks) { loader.send(:load_hooks) } + + before do + context.stub(hook_class_name: 'PreCommit', + hook_type_name: 'pre_commit') + end + + it 'loads enabled gem hooks' do + allow(config).to receive(:enabled_gem_hooks).with(context).and_return(['MyCustomHook']) + + allow(loader).to receive(:require). + with('overcommit/hook/pre_commit/my_custom_hook'). + and_return(true) + allow(loader).to receive(:create_hook).with('MyCustomHook') + expect(loader).to receive(:require).with('overcommit/hook/pre_commit/my_custom_hook') + load_hooks + end + end +end diff --git a/spec/overcommit/hook_runner_spec.rb b/spec/overcommit/hook_runner_spec.rb new file mode 100644 index 00000000..d2ce4bf3 --- /dev/null +++ b/spec/overcommit/hook_runner_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Overcommit::HookRunner do + let(:hash) { {} } + let(:config) { Overcommit::Configuration.new(hash) } + let(:logger) { double('logger') } + let(:context) { double('context') } + let(:printer) { double('printer') } + let(:runner) { described_class.new(config, logger, context, printer) } + + describe '#load_hooks' do + subject(:load_hooks) { runner.send(:load_hooks) } + + before do + context.stub(hook_class_name: 'PreCommit', + hook_type_name: 'pre_commit') + allow_any_instance_of(Overcommit::HookLoader::BuiltInHookLoader). + to receive(:load_hooks).and_return([]) + allow_any_instance_of(Overcommit::HookLoader::PluginHookLoader). + to receive(:load_hooks).and_return([]) + end + + context 'when gem_plugins_enabled is disabled' do + let(:hash) { { 'gem_plugins_enabled' => false } } + + it 'expects not to load gem hooks' do + expect_any_instance_of(Overcommit::HookLoader::GemHookLoader). + not_to receive(:load_hooks) + load_hooks + end + end + + context 'when gem_plugins_enabled is not specified' do + let(:gemhookloader) { Overcommit::HookLoader::GemHookLoader.new(config, context, logger) } + + it 'expects not to load gem hooks' do + expect_any_instance_of(Overcommit::HookLoader::GemHookLoader). + not_to receive(:load_hooks) + load_hooks + end + end + + context 'when gem_plugins_enabled is enabled' do + let(:hash) { { 'gem_plugins_enabled' => true } } + let(:gemhookloader) { Overcommit::HookLoader::GemHookLoader.new(config, context, logger) } + + it 'expects to load gem hooks' do + expect_any_instance_of(Overcommit::HookLoader::GemHookLoader). + to receive(:load_hooks).and_call_original + load_hooks + end + end + end +end