Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
20 changes: 20 additions & 0 deletions lib/overcommit/hook/pre_push/git_lfs.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
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
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, " \
'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
Copy link
Contributor Author

@detouched detouched Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure: is it possible to output some information in case the hook passes?

In fact, ideally it'd be cool to be able to output something while the hook works cause uploading LFS files might take a while. The original LFS hook reports status interactively, something like:

$ git push
Git LFS: (0 of 1 files) 3.80 MB / 6.85 MB

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know of a good way. Would it make sense to instead of actually uploading just check if there is something to upload? The overcommit hook would then output something like

Checking for files that need to be uploaded..........................[GitLFS] FAILED
./some-file.large hasn't been uploaded. Run `git lfs pre-push ... something` to upload. 

I'm not a lfs user myself, so this idea might not be possible at all. Or it might just be annoying to have to run the upload manually.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would require some refactoring of the display logic for Overcommit. In the past we've kicked around the idea of supporting progress bars or similar, but this would require a much more complicated display controller that utilized more advanced cursor features so you could update multiple lines at the same time (since hooks run in parallel).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely open to the idea, but I think it's well outside the scope of what you're trying to accomplish with this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to instead of actually uploading just check if there is something to upload?

Not really, you'd just want it to be uploaded just with git push, without additionally calling git lfs .... Also, I'd not make overcommit behaviour different from standard (meaning what is installed by LFS client by default).

It's well outside the scope of what you're trying to accomplish with this change.

I totally agree, I was wondering if there's a way to do that, but if not, it's fine. It might be just mentioned in the doc since GitLfs hook is disabled by default and requires the user to add it in the config yaml.

end
end
end
39 changes: 39 additions & 0 deletions spec/overcommit/hook/pre_push/git_lfs_spec.rb
Original file line number Diff line number Diff line change
@@ -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(in_path?: true)
subject.stub(:execute).and_return(result)
end

context 'when git-lfs is not on path' do
before do
subject.stub(in_path?: false)
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
result.stub(success?: false)
result.stub(stderr: 'error: failed to push some refs')
end

it { should fail_hook }
end
end