diff mbox series

bitbake: fetch2: git: Add git-lfs install step

Message ID CAJZje+tdOZPp73THMk6pZNLyzncLQcqTRcdyN=v99dJL_=430g@mail.gmail.com
State New
Headers show
Series bitbake: fetch2: git: Add git-lfs install step | expand

Commit Message

Desone Burns II Oct. 24, 2023, 8:27 p.m. UTC
When pulling a Git repo that included LFS, it was possible that
the LFS download would fail with a message regarding the LFS being
installed in the repo. This change runs the LFS install step prior
to checkout, to ensure the download step is included in the Git hooks.

Signed-off-by: Desone Burns II <dburnsii@live.com>
---
 bitbake/lib/bb/fetch2/git.py | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)


 # Copyright (C) 2005 Richard Purdie
@@ -630,6 +635,8 @@ class Git(FetchMethod):
                 raise bb.fetch2.FetchError("Repository %s has LFS
content, install git-lfs on host to download (or set lfs=0 to ignore
it)" % (repourl))
             elif not need_lfs:
                 bb.note("Repository %s has LFS content but it is not
being fetched" % (repourl))
+            else:
+                runfetchcmd("%s lfs install" % ud.basecmd, d,
workdir=destdir)

         if not ud.nocheckout:
             if subpath:
@@ -689,13 +696,15 @@ class Git(FetchMethod):
         Check if the repository has 'lfs' (large file) content
         """

-        # The bare clonedir doesn't use the remote names; it has the
branch immediately.
-        if wd == ud.clonedir:
+        if ud.nobranch:
+            refname = self._build_revision(ud, d, ud.names[0])
+        elif wd == ud.clonedir:
+            # The bare clonedir doesn't use the remote names; it has
the branch immediately.
             refname = ud.branches[ud.names[0]]
         else:
             refname = "origin/%s" % ud.branches[ud.names[0]]

-        cmd = "%s grep lfs %s:.gitattributes | wc -l" % (
+        cmd = "%s lfs ls-files %s | wc -l" % (
             ud.basecmd, refname)

         try:

Comments

Richard Purdie Oct. 25, 2023, 9:27 a.m. UTC | #1
On Tue, 2023-10-24 at 16:27 -0400, Desone Burns wrote:
> When pulling a Git repo that included LFS, it was possible that
> the LFS download would fail with a message regarding the LFS being
> installed in the repo. This change runs the LFS install step prior
> to checkout, to ensure the download step is included in the Git
> hooks.
> 
> Signed-off-by: Desone Burns II <dburnsii@live.com>
> ---
>  bitbake/lib/bb/fetch2/git.py | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Firstly, the patch didn't come through correctly and has line wrapping
and whitespace issues which would prevent it being applied.

I'm also very unclear which lfs issues this patch fixes. There have
been a few changes around lfs recently and I can't tell if this is a
new issue, a regression with one of the recent changes or something
else. Which branch/revision was the original issue seen against?

In general we'd expect a user to have git-lfs available in advance of
the builds and not to download and create a local install in the repo
during the fetch. I'm unclear what an "lfs install" does though so
perhaps I don't understand that.

The patch also doesn't change any of our tests which ideally need
updating to include whatever issue this patch is fixing.

Cheers,

Richard
Desone Burns II Oct. 25, 2023, 4:30 p.m. UTC | #2
My mistake regarding the format of the patch, I can reupload as a v2 with
the formatting corrected.

With regards to this patch, I was having issues pulling a git repo
containing an LFS, the smaller files would come through but the files
actually stored under the LFS did not make it across. In my case, I was
attempting to pull images stored in an LFS for a plymouth splash screen.
The `git lfs install`, as far as I understand, sets up the cloned repo to
also download the large files. Namely, it sets up the git hooks to allow
the download to take place. When cloning the repo without running this
install step, the files populate the `git` directory in the following
format:

version https://git-lfs.github.com/spec/v1
oid sha256:<File SHA>
size 1442

This instructs the LFS application to download the large file, but without
installing the LFS download step into the git hooks, the file remains in
this format. This is all running under the assumption that the user does
already have git-lfs installed on their system, although it may be worth
investigating what minimum version of the application is required for this
to work.

I would be more than willing to write a test to cover this functionality,
is there an available Yocto repo that contains an LFS I could test against?

On Wed, Oct 25, 2023 at 5:27 AM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Tue, 2023-10-24 at 16:27 -0400, Desone Burns wrote:
> > When pulling a Git repo that included LFS, it was possible that
> > the LFS download would fail with a message regarding the LFS being
> > installed in the repo. This change runs the LFS install step prior
> > to checkout, to ensure the download step is included in the Git
> > hooks.
> >
> > Signed-off-by: Desone Burns II <dburnsii@live.com>
> > ---
> >  bitbake/lib/bb/fetch2/git.py | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
>
> Firstly, the patch didn't come through correctly and has line wrapping
> and whitespace issues which would prevent it being applied.
>
> I'm also very unclear which lfs issues this patch fixes. There have
> been a few changes around lfs recently and I can't tell if this is a
> new issue, a regression with one of the recent changes or something
> else. Which branch/revision was the original issue seen against?
>
> In general we'd expect a user to have git-lfs available in advance of
> the builds and not to download and create a local install in the repo
> during the fetch. I'm unclear what an "lfs install" does though so
> perhaps I don't understand that.
>
> The patch also doesn't change any of our tests which ideally need
> updating to include whatever issue this patch is fixing.
>
> Cheers,
>
> Richard
>
>
Martin Jansa Oct. 25, 2023, 4:49 p.m. UTC | #3
I guess you needed git lfs install to configure lfs in your gitconfig like
e.g. git-lfs postinst does in ubuntu, for details see:
https://bugzilla.yoctoproject.org/show_bug.cgi?id=14938

there is also a link to new lfs selftest, but the expected behavior is
still unknown.

On Wed, Oct 25, 2023 at 6:30 PM Desone Burns <dburns@seegrid.com> wrote:

> My mistake regarding the format of the patch, I can reupload as a v2 with
> the formatting corrected.
>
> With regards to this patch, I was having issues pulling a git repo
> containing an LFS, the smaller files would come through but the files
> actually stored under the LFS did not make it across. In my case, I was
> attempting to pull images stored in an LFS for a plymouth splash screen.
> The `git lfs install`, as far as I understand, sets up the cloned repo to
> also download the large files. Namely, it sets up the git hooks to allow
> the download to take place. When cloning the repo without running this
> install step, the files populate the `git` directory in the following
> format:
>
> version https://git-lfs.github.com/spec/v1
> oid sha256:<File SHA>
> size 1442
>
> This instructs the LFS application to download the large file, but without
> installing the LFS download step into the git hooks, the file remains in
> this format. This is all running under the assumption that the user does
> already have git-lfs installed on their system, although it may be worth
> investigating what minimum version of the application is required for this
> to work.
>
> I would be more than willing to write a test to cover this functionality,
> is there an available Yocto repo that contains an LFS I could test against?
>
> On Wed, Oct 25, 2023 at 5:27 AM Richard Purdie <
> richard.purdie@linuxfoundation.org> wrote:
>
>> On Tue, 2023-10-24 at 16:27 -0400, Desone Burns wrote:
>> > When pulling a Git repo that included LFS, it was possible that
>> > the LFS download would fail with a message regarding the LFS being
>> > installed in the repo. This change runs the LFS install step prior
>> > to checkout, to ensure the download step is included in the Git
>> > hooks.
>> >
>> > Signed-off-by: Desone Burns II <dburnsii@live.com>
>> > ---
>> >  bitbake/lib/bb/fetch2/git.py | 15 ++++++++++++---
>> >  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> Firstly, the patch didn't come through correctly and has line wrapping
>> and whitespace issues which would prevent it being applied.
>>
>> I'm also very unclear which lfs issues this patch fixes. There have
>> been a few changes around lfs recently and I can't tell if this is a
>> new issue, a regression with one of the recent changes or something
>> else. Which branch/revision was the original issue seen against?
>>
>> In general we'd expect a user to have git-lfs available in advance of
>> the builds and not to download and create a local install in the repo
>> during the fetch. I'm unclear what an "lfs install" does though so
>> perhaps I don't understand that.
>>
>> The patch also doesn't change any of our tests which ideally need
>> updating to include whatever issue this patch is fixing.
>>
>> Cheers,
>>
>> Richard
>>
>>
>
> --
> Desone Burns II
> Software Engineer III
> seegrid.com <https://www.seegrid.com/>  *•*  412-379-4500
> m: 623-258-7361
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#15285):
> https://lists.openembedded.org/g/bitbake-devel/message/15285
> Mute This Topic: https://lists.openembedded.org/mt/102165795/3617156
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> martin.jansa@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
diff mbox series

Patch

diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 33895e09b2..e4da7de377 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -52,6 +52,11 @@  Supported SRC_URI options are:
    For local git:// urls to use the current branch HEAD as the
revision for use with
    AUTOREV. Implies nobranch.

+- lfs
+    Enable the checkout to use LFS for large files. This will
download all LFS files
+    in the download step, as the unpack step does not have network access.
+    The default is "1", set lfs=0 to skip.
+
 """