From 2d2d97a24687b5273ea6d19d8017669b360fc03f Mon Sep 17 00:00:00 2001 From: Sean Millichamp Date: Mon, 19 Apr 2021 11:43:43 -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 | 19 +++++++ config/default.yml | 5 ++ lib/overcommit.rb | 1 + lib/overcommit/configuration.rb | 19 +++++++ lib/overcommit/hook_loader/gem_hook_loader.rb | 14 +++++ lib/overcommit/hook_runner.rb | 5 ++ spec/overcommit/configuration_spec.rb | 48 +++++++++++++++++ .../hook_loader/gem_hook_loader_spec.rb | 31 +++++++++++ spec/overcommit/hook_runner_spec.rb | 54 +++++++++++++++++++ 9 files changed, 196 insertions(+) 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..107ac931 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,24 @@ 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. + +These are implemented similarly to Repo-Specific Hooks, but need to be placed +in a Gem in the normal `overcommit` hook path. For example, to add `MyCustomHook` +as a pre_commit hook you would put it here: +`./lib/overcommit/hooks/pre_commit/my_custom_hook.rb` + +You must ensure that the Gem is available in the environment where `overcommit` is +being run. + +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..e22af649 100644 --- a/config/default.yml +++ b/config/default.yml @@ -34,6 +34,11 @@ # (Generate lock file by running `bundle install --gemfile=.overcommit_gems.rb`) gemfile: false +# If enabled, Overcommit will look for plugins provided anywhere in the +# standard load path and load them as it would with built-in or repo-specific +# hooks. +gem_plugins: true + # 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..c333ed04 100644 --- a/lib/overcommit/configuration.rb +++ b/lib/overcommit/configuration.rb @@ -119,6 +119,14 @@ 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' }. + 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 +267,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 +279,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/hook_loader/gem_hook_loader.rb b/lib/overcommit/hook_loader/gem_hook_loader.rb new file mode 100644 index 00000000..b8132694 --- /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 hooks that ship with Overcommit. + 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..7ca13519 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 gem_plugins is enabled: + if @config['gem_plugins'] + @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..82f066ff 100644 --- a/spec/overcommit/configuration_spec.rb +++ b/spec/overcommit/configuration_spec.rb @@ -294,4 +294,52 @@ 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('/my/custom/path/lib/overcommit/hook/pre_commit/my_other_hook.rb'). + and_return(true) + 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/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..d779e040 --- /dev/null +++ b/spec/overcommit/hook_runner_spec.rb @@ -0,0 +1,54 @@ +# 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 is disabled' do + let(:hash) do + { + 'gem_plugins' => false + } + end + + 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 is enabled' do + let(:hash) do + { + 'gem_plugins' => true + } + end + 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