From fac1324a1e08a49f75de1b4cd8ee03d4faaf9119 Mon Sep 17 00:00:00 2001 From: Daniil Penkin Date: Thu, 23 Mar 2017 15:51:46 +1100 Subject: [PATCH 1/2] Add Git LFS pre-push hook Git LFS installs pre-push hook which: - checks that git-lfs utility is on PATH, and if no, warns that Git LFS hook should probably be removed - calls it by executing `git lfs pre-push ` The introduced hook is a Ruby port of the same algorithm modulo hook disabling note in the warning message. --- README.md | 1 + config/default.yml | 4 ++ lib/overcommit/hook/pre_push/git_lfs.rb | 21 ++++++++++ spec/overcommit/hook/pre_push/git_lfs_spec.rb | 39 +++++++++++++++++++ 4 files changed, 65 insertions(+) create mode 100644 lib/overcommit/hook/pre_push/git_lfs.rb create mode 100644 spec/overcommit/hook/pre_push/git_lfs_spec.rb diff --git a/README.md b/README.md index e94a5406..69e2e1c8 100644 --- a/README.md +++ b/README.md @@ -549,6 +549,7 @@ but before any objects have been transferred. If a hook fails, the push is aborted. * [Brakeman](lib/overcommit/hook/pre_push/brakeman.rb) +* [GitLfs](lib/overcommit/hook/pre_push/git_lfs.rb) * [Minitest](lib/overcommit/hook/pre_push/minitest.rb) * [ProtectedBranches](lib/overcommit/hook/pre_push/protected_branches.rb) * [Pytest](lib/overcommit/hook/pre_push/pytest.rb) diff --git a/config/default.yml b/config/default.yml index 0ef15b95..1c5086ec 100644 --- a/config/default.yml +++ b/config/default.yml @@ -945,6 +945,10 @@ PrePush: flags: ['--exit-on-warn', '--quiet', '--summary'] install_command: 'gem install brakeman' + GitLfs: + enabled: false + description: 'Upload files tracked by Git LFS' + # Hooks that run during `git rebase`, before any commits are rebased. # If a hook fails, the rebase is aborted. PreRebase: diff --git a/lib/overcommit/hook/pre_push/git_lfs.rb b/lib/overcommit/hook/pre_push/git_lfs.rb new file mode 100644 index 00000000..18363c88 --- /dev/null +++ b/lib/overcommit/hook/pre_push/git_lfs.rb @@ -0,0 +1,21 @@ +module Overcommit::Hook::PrePush + # Invokes Git LFS command that uploads files tracked by Git LFS to the LFS storage + # + # @see https://git-lfs.github.com/ + class GitLfs < Base + def run + result = execute(['command', '-v', 'git-lfs']) + unless result.success? + return :warn, 'This repository is configured for Git LFS but \'git-lfs\' ' \ + 'was not found on your path.\nIf you no longer wish to use Git LFS, ' \ + 'disable this hook by removing or setting \'enabled: false\' for GitLFS ' \ + 'hook in your .overcommit.yml file' + end + + result = execute(['git', 'lfs', 'pre-push', remote_name, remote_url]) + return :fail, result.stderr unless result.success? + + :pass + end + end +end diff --git a/spec/overcommit/hook/pre_push/git_lfs_spec.rb b/spec/overcommit/hook/pre_push/git_lfs_spec.rb new file mode 100644 index 00000000..c72c1d02 --- /dev/null +++ b/spec/overcommit/hook/pre_push/git_lfs_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe Overcommit::Hook::PrePush::GitLfs do + let(:config) { Overcommit::ConfigurationLoader.default_configuration } + let(:context) { double('context', remote_name: 'remote_name', remote_url: 'remote_url') } + subject { described_class.new(config, context) } + + let(:result) { double('result') } + + before do + subject.stub(:execute).and_return(result) + end + + context 'when git-lfs is not on path' do + before do + result.stub(success?: false, stderr: '') + end + + it { should warn } + end + + context 'when git lfs hook exits successfully' do + before do + result.stub(success?: true, stderr: '') + end + + it { should pass } + end + + context 'when git lfs hook exits unsuccessfully' do + before do + # First for checking that git-lfs is on path, second for calling the hook itself + result.stub(:success?).and_return(true, false) + result.stub(:stderr).and_return('', 'error: failed to push some refs') + end + + it { should fail_hook } + end +end From ba71f99643bf51d9b816a1567edf955d0523f3cd Mon Sep 17 00:00:00 2001 From: Daniil Penkin Date: Sat, 1 Apr 2017 01:39:01 +1100 Subject: [PATCH 2/2] PR review - Refactor checking LFS executable on PATH - Use `in_path` to check LFS binary presence on PATH - Fix newline in the warning message when LFS binary is not on PATH --- lib/overcommit/hook/pre_push/git_lfs.rb | 5 ++--- spec/overcommit/hook/pre_push/git_lfs_spec.rb | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/overcommit/hook/pre_push/git_lfs.rb b/lib/overcommit/hook/pre_push/git_lfs.rb index 18363c88..f29c3437 100644 --- a/lib/overcommit/hook/pre_push/git_lfs.rb +++ b/lib/overcommit/hook/pre_push/git_lfs.rb @@ -4,10 +4,9 @@ module Overcommit::Hook::PrePush # @see https://git-lfs.github.com/ class GitLfs < Base def run - result = execute(['command', '-v', 'git-lfs']) - unless result.success? + unless in_path?('git-lfs') return :warn, 'This repository is configured for Git LFS but \'git-lfs\' ' \ - 'was not found on your path.\nIf you no longer wish to use Git LFS, ' \ + "was not found on your path.\nIf you no longer wish to use Git LFS, " \ 'disable this hook by removing or setting \'enabled: false\' for GitLFS ' \ 'hook in your .overcommit.yml file' end diff --git a/spec/overcommit/hook/pre_push/git_lfs_spec.rb b/spec/overcommit/hook/pre_push/git_lfs_spec.rb index c72c1d02..1d85fc3f 100644 --- a/spec/overcommit/hook/pre_push/git_lfs_spec.rb +++ b/spec/overcommit/hook/pre_push/git_lfs_spec.rb @@ -8,12 +8,13 @@ let(:result) { double('result') } before do + subject.stub(in_path?: true) subject.stub(:execute).and_return(result) end context 'when git-lfs is not on path' do before do - result.stub(success?: false, stderr: '') + subject.stub(in_path?: false) end it { should warn } @@ -29,9 +30,8 @@ context 'when git lfs hook exits unsuccessfully' do before do - # First for checking that git-lfs is on path, second for calling the hook itself - result.stub(:success?).and_return(true, false) - result.stub(:stderr).and_return('', 'error: failed to push some refs') + result.stub(success?: false) + result.stub(stderr: 'error: failed to push some refs') end it { should fail_hook }