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

feat: add Git LFS #15

Closed
wants to merge 5 commits into from
Closed

feat: add Git LFS #15

wants to merge 5 commits into from

Conversation

fgblomqvist
Copy link
Contributor

@fgblomqvist fgblomqvist commented Apr 7, 2021

This adds Git LFS as a tool.
I'm not too sure what kinds of tests you'd expect/want? Or where to add the install-tool line. It seems like by default, only git is installed in this base image?

Finally, worth noting is the discussion in renovatebot/renovate#7894.

Closes #13.

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

@rarkins
Copy link
Member

rarkins commented Apr 7, 2021

@viceice I was thinking we wouldn't want it in the base image (that all images share) and instead only in Renovate?

@viceice viceice changed the title Add Git LFS feat: add Git LFS Apr 7, 2021
@viceice
Copy link
Member

viceice commented Apr 7, 2021

@viceice I was thinking we wouldn't want it in the base image (that all images share) and instead only in Renovate?

Ok, so it should be installed to Dockerfile.bionic and test/latest/Dockerfile for testing.
We need to prepare a simple git test repo with one lfs file and unse it in tests to verify it works for root and user

@fgblomqvist
Copy link
Contributor Author

Found this: https://github.com/git-lfs/git-lfs/wiki/Installation#debian-and-ubuntu
It's essentially possible to install git-lfs and get the latest version from apt if we want. Up to you if you still prefer the tarball for speed.

@viceice
Copy link
Member

viceice commented Apr 7, 2021

Please stay at tarball, so we have control over installed version

@fgblomqvist
Copy link
Contributor Author

Made some changes to the install script, to more-so mimmick the dotnet one you linked @viceice.
The questions that remain are:

  • How do we wanna test this?
  • What do we do about git lfs install? Right now, I install git-lfs by running the install script, which automatically runs git lfs install as well. However, it's user-specific which means that it only gets installed/setup for root. But yeah, not sure how we wanna go about it (e.g. what if some people using Renovate don't want git lfs?).

@fgblomqvist
Copy link
Contributor Author

Bump @viceice @rarkins.

@rarkins
Copy link
Member

rarkins commented May 6, 2021

If git lfs is installed by default for the container, does that cause problems for those who don't want it or need it? I had expected not, but perhaps not aware of the edge cases.

@viceice
Copy link
Member

viceice commented May 6, 2021

i think it's save to install lfs, because lfs hooks are disabled by renovate renovatebot/renovate#9673

But it think we should only have this installed to main renovate image to not have any side effects on sidecar container starting to pull huge lfs objects

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

exit 0
fi

LFS_FILE="git-lfs-linux-amd64-${TOOL_VERSION}.tar.gz"
Copy link
Member

Choose a reason for hiding this comment

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

fi

LFS_FILE="git-lfs-linux-amd64-${TOOL_VERSION}.tar.gz"
wget "https://github.com/git-lfs/git-lfs/releases/download/${TOOL_VERSION}/${LFS_FILE}"
Copy link
Member

Choose a reason for hiding this comment

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

All other tools are usiing curl, why are you using wget ?

@jtbandes
Copy link

jtbandes commented Jul 7, 2021

Hi, what's the status of this work? Seems like this is a necessary step to make progress on renovatebot/renovate#6842.

@viceice
Copy link
Member

viceice commented Jul 7, 2021

currently blocked by renovatebot/renovate#10748

@viceice
Copy link
Member

viceice commented Jul 30, 2021

Superseeded by #104

@viceice viceice closed this Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add git-lfs
4 participants