Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Git LFS support #872

Merged
merged 4 commits into from
Mar 27, 2020
Merged

Add Git LFS support #872

merged 4 commits into from
Mar 27, 2020

Conversation

remilapeyre
Copy link
Contributor

Closes #869

@remilapeyre
Copy link
Contributor Author

@lkysow I made some tests and this seems to work, I've some trouble to write a unit test for it though as Git LFS expect having an HTTP server to read the file from and does not support the file:// protocol yet.

@codecov
Copy link

codecov bot commented Dec 4, 2019

Codecov Report

Merging #872 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #872   +/-   ##
=======================================
  Coverage   71.87%   71.87%           
=======================================
  Files          65       65           
  Lines        5244     5244           
=======================================
  Hits         3769     3769           
  Misses       1188     1188           
  Partials      287      287

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b6541a...d09a63c. Read the comment docs.

Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

I just tested a git lfs repo on an Atlantis Docker image where git-lfs is installed and git lfs install was run and everything works fine. Why do we need the additional git lfs pull command?

@lkysow lkysow added the waiting-on-response Waiting for a response from the user label Jan 20, 2020
@remilapeyre
Copy link
Contributor Author

Just checked and it seems that recent packages install git-lfs in the global Git configuration so it should work without manually pulling indeed. I'm not sure why the documentation at https://github.com/git-lfs/git-lfs/wiki/Installation says that git lfs install must be run, it seems unnecessary. Do you want me to revert the changes on server/events/working_dir.go?

@lkysow
Copy link
Member

lkysow commented Jan 20, 2020

Do you want me to revert the changes on server/events/working_dir.go?

Yes please.

Also test that git lfs install isn't necessary. If it is, it needs to be run on the Docker image.

@remilapeyre
Copy link
Contributor Author

remilapeyre commented Jan 21, 2020

Also test that git lfs install isn't necessary

I tried on alpine, Debian and Centos and it seems to run on install now. It seems to be like this for Debian since 2016 (https://github.com/git-lfs/git-lfs/blob/master/debian/postinst).

@Charg
Copy link

Charg commented Mar 26, 2020

Thanks for creating the PR @remilapeyre. Do we require any additional testing? My environment depends on LFS making this a really nice addition to Atlantis for us.

@rguevara84
Copy link

rguevara84 commented Mar 26, 2020

This Is great, we depend on this change too, would be nice to get it merged 💯

@lkysow lkysow merged commit f4dd828 into runatlantis:master Mar 27, 2020
@lkysow
Copy link
Member

lkysow commented Mar 27, 2020

Sorry for the delay here.

@lkysow lkysow added feature New functionality/enhancement and removed waiting-on-response Waiting for a response from the user labels Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git LFS is not supported
4 participants