Re: [Buildroot] [PATCH 1/1] download/git: support Git LFS

From: John Keeping <john@metanate.com>
To: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	"Yann E . MORIN" <yann.morin.1998@free.fr>,
	Vincent Fazio <vfazio@xes-inc.com>,
	Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH 1/1] download/git: support Git LFS
Date: Thu, 6 Jan 2022 15:03:50 +0000	[thread overview]
Message-ID: <YdcE1oBg+tvkNeXs@donbot> (raw)
In-Reply-To: <d28ff21d-c2bf-f489-2d09-9f01936f1337@mind.be>

Hi Arnout,

On Thu, Jan 06, 2022 at 11:35:21AM +0100, Arnout Vandecappelle wrote:
>  I finally applied this to master, with a few minor changes.

Thanks for merging this!

> On 21/01/2020 21:33, Vincent Fazio wrote:
> > From: John Keeping <john@metanate.com>
> > 
> > Git Large File Storage replaces large files with text pointers in the
> > Git repository while storing the contents on a remote server.  If a
> > repository is using this extension, then git-lfs must be used to
> > checkout the large files before the source archive is generated.
> > 
> > Signed-off-by: John Keeping <john@metanate.com>
> > [vfazio:
> >    - add git-lfs to DL_TOOLS_DEPENDENCIES
> >    - fixup for 5a0d6813948fea2cdb88a2e35984520eec856dec
> >      ("infra/pkg-download: make the DOWNLOAD macro fully parameterised")
> > ]
> > Signed-off-by: Vincent Fazio <vfazio@xes-inc.com>
> 
> [snip]
> 
> > +# If there are large files then fetch them.
> > +if [ ${large_file} -eq 1 ]; then
> > +    _git lfs install
> 
>  AFAICS an lfs install is not needed. IMHO it's pretty bad to do that
> because it updates ~/.gitconfig, which Buildroot shouldn't do. We could do
> lfs install --local, but even that is not needed. The lfs install just makes
> sure that lfs is done automatically whenever you do commit or pull or
> something like that, but in Buildroot we always do it explicitly so the
> smudge filter isn't needed.

I had a local update with --local.  Unfortunately it seems that install
is necessary as when testing the committed version (having removed the
filter config from ~/.gitconfig) I get:

	Cannot checkout LFS objects, Git LFS is not installed.

I'll send a patch to do `git lfs install --local` here.

> > +    _git lfs fetch
> > +    _git lfs checkout
> 
>  As already mentioned by Yann, this doesn't work when combined with
> submodules. The solution is pretty trivial:
> 
>     # If there are also submodules, recurse into them,
>     if [ ${recurse} -eq 1 ]; then
>         _git submodule foreach --recursive ${GIT} lfs fetch
>         _git submodule foreach --recursive ${GIT} lfs checkout
>     fi
> 
>  I haven't actually tested this though.

I have a test repo with LFS submodules and I can confirm this works
(modulo the install concern above).

>  Vincent (or anybody), it would be nice to have a runtime test for this
> feature. support/testing/tests/download/br2-external/git-refs/package/git-submodule-{enabled,disabled}
> can be a good source of inspiration. Only I'm not sure if it's possible to
> set up an LFS repository that is purely file-based.

Yes, I think you're right that LFS requires HTTPS.  It's possible to
combine that with any Git protocol, but the LFS transfer is always
HTTPS, so any runtime tests would require a temporary server.

John
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot