mbox series

[v3,0/8] Fix gitsm LFS support

Message ID 20250425144138.4089681-1-philip.lorenz@bmw.de
Headers show
Series Fix gitsm LFS support | expand

Message

Philip Lorenz April 25, 2025, 2:41 p.m. UTC 
This patch series fixes a number of issues we observed when using the
gitsm fetcher in conjunction with git-lfs.

I'd like to highlight two patches in this series:
* 0004-fetch2-Simplify-git-LFS-detection: I could not come up with a
  good reason on why the code so far preferred the content of
 `.gitattributes` on the repository's branch instead of always
  considering the state of the target revision. Please let me know
  if there's something I didn't take into account.
* 0005-fetch2-Use-git-lfs-fetch-to-download-objects.patch: I could not
  replicate the `git-lfs fetch` behaviour in versions going back to
  2.13.2 released in 2021. This leads me to believe that the issue
  leading to this workaround is no longer in place. However, if anyone
  remembers the actual issue / versions involved I can also retest to
  confirm that there are no regression.

---
V1 -> V2:
* The correct parameter is now passed for each submodule URL when LFS is
  explicitly disabled
* Smudging for submodules is now skipped when LFS is explcitly disabled.
  This ensures consistent behaviour in setups where git-lfs is installed
  but should not be used.
* Test cases are extended to test the LFS disabled state.
* Explicitly install git lfs into submodule in the test cases to make
  sure that git-lfs is actually used for storing the files.

V2 -> V3:
* Only check for git-lfs existence after confirming whether it is really
  neded

Philip Lorenz (8):
  fetch2: Clean up no longer used name parameter
  tests/fetch: Move commonly used imports to top
  fetch2: Check for git-lfs existence before using it
  fetch2: Simplify git LFS detection
  fetch2: Use git-lfs fetch to download objects
  fetch2: Fix incorrect lfs parametrization for submodules
  fetch2: Fix LFS object checkout in submodules
  tests/fetch: Test gitsm with LFS

 lib/bb/fetch2/git.py   |  69 +++++---------
 lib/bb/fetch2/gitsm.py |  13 ++-
 lib/bb/tests/fetch.py  | 202 +++++++++++++++++++++++++++++++----------
 3 files changed, 182 insertions(+), 102 deletions(-)

Comments

Mathieu Dubois-Briand April 28, 2025, 4:25 p.m. UTC | #1 
On Fri Apr 25, 2025 at 4:41 PM CEST, Philip Lorenz wrote:
> This patch series fixes a number of issues we observed when using the
> gitsm fetcher in conjunction with git-lfs.
>
> I'd like to highlight two patches in this series:
> * 0004-fetch2-Simplify-git-LFS-detection: I could not come up with a
>   good reason on why the code so far preferred the content of
>  `.gitattributes` on the repository's branch instead of always
>   considering the state of the target revision. Please let me know
>   if there's something I didn't take into account.
> * 0005-fetch2-Use-git-lfs-fetch-to-download-objects.patch: I could not
>   replicate the `git-lfs fetch` behaviour in versions going back to
>   2.13.2 released in 2021. This leads me to believe that the issue
>   leading to this workaround is no longer in place. However, if anyone
>   remembers the actual issue / versions involved I can also retest to
>   confirm that there are no regression.
>
> ---

Hi Philip,

Thanks for the new version.

It works much better than previously. I still have some issues with
bitbake selftests on my branch:

ERROR: test_lfs_enabled_not_installed (bb.tests.fetch.GitLfsTest.test_lfs_enabled_not_installed)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/pokybuild/yocto-worker/oe-selftest-debian/build/bitbake/lib/bb/tests/fetch.py", line 2528, in test_lfs_enabled_not_installed
    fetcher.download()
    ~~~~~~~~~~~~~~~~^^
  File "/srv/pokybuild/yocto-worker/oe-selftest-debian/build/bitbake/lib/bb/fetch2/__init__.py", line 1929, in download
    raise FetchError("Unable to fetch URL from any source.", u)
bb.fetch2.FetchError: Fetcher failure for URL: 'git:///tmp/bitbake-fetch-m30azy7_/gitsource;protocol=file;lfs=1;branch=master'. Unable to fetch URL from any source.

Stdout:
Cloning into bare repository '/tmp/bitbake-fetch-m30azy7_/download/git2/tmp.bitbake-fetch-m30azy7_.gitsource'...
remote: Enumerating objects: 3, done.
remote: Counting objects: 100% (3/3), done.
remote: Total 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
Receiving objects:  33% (1/3)
Receiving objects: 100% (3/3), done.
Failed to fetch URL git:///tmp/bitbake-fetch-m30azy7_/gitsource;protocol=file;lfs=1;branch=master, attempting MIRRORS if available
Fetcher failure: Repository file:///tmp/bitbake-fetch-m30azy7_/gitsource has LFS content, install git-lfs on host to download (or set lfs=0 to ignore it)
Failed to fetch URL git:///tmp/bitbake-fetch-m30azy7_/gitsource;protocol=file;lfs=1;branch=master, attempting MIRRORS if available
Fetcher failure: Repository file:///tmp/bitbake-fetch-m30azy7_/gitsource has LFS content, install git-lfs on host to download (or set lfs=0 to ignore it)

https://autobuilder.yoctoproject.org/valkyrie/#/builders/35/builds/1440/steps/12/logs/stdio
Richard Purdie April 28, 2025, 9:09 p.m. UTC | #2 
On Mon, 2025-04-28 at 18:25 +0200, Mathieu Dubois-Briand wrote:
> On Fri Apr 25, 2025 at 4:41 PM CEST, Philip Lorenz wrote:
> > This patch series fixes a number of issues we observed when using the
> > gitsm fetcher in conjunction with git-lfs.
> > 
> > I'd like to highlight two patches in this series:
> > * 0004-fetch2-Simplify-git-LFS-detection: I could not come up with a
> >   good reason on why the code so far preferred the content of
> >  `.gitattributes` on the repository's branch instead of always
> >   considering the state of the target revision. Please let me know
> >   if there's something I didn't take into account.
> > * 0005-fetch2-Use-git-lfs-fetch-to-download-objects.patch: I could not
> >   replicate the `git-lfs fetch` behaviour in versions going back to
> >   2.13.2 released in 2021. This leads me to believe that the issue
> >   leading to this workaround is no longer in place. However, if anyone
> >   remembers the actual issue / versions involved I can also retest to
> >   confirm that there are no regression.
> > 
> > ---
> 
> Hi Philip,
> 
> Thanks for the new version.
> 
> It works much better than previously. I still have some issues with
> bitbake selftests on my branch:
> 
> ERROR: test_lfs_enabled_not_installed (bb.tests.fetch.GitLfsTest.test_lfs_enabled_not_installed)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/srv/pokybuild/yocto-worker/oe-selftest-debian/build/bitbake/lib/bb/tests/fetch.py", line 2528, in test_lfs_enabled_not_installed
>     fetcher.download()
>     ~~~~~~~~~~~~~~~~^^
>   File "/srv/pokybuild/yocto-worker/oe-selftest-debian/build/bitbake/lib/bb/fetch2/__init__.py", line 1929, in download
>     raise FetchError("Unable to fetch URL from any source.", u)
> bb.fetch2.FetchError: Fetcher failure for URL: 'git:///tmp/bitbake-fetch-m30azy7_/gitsource;protocol=file;lfs=1;branch=master'. Unable to fetch URL from any source.
> 
> Stdout:
> Cloning into bare repository '/tmp/bitbake-fetch-m30azy7_/download/git2/tmp.bitbake-fetch-m30azy7_.gitsource'...
> remote: Enumerating objects: 3, done.
> remote: Counting objects: 100% (3/3), done.
> remote: Total 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
> Receiving objects:  33% (1/3)
> Receiving objects: 100% (3/3), done.
> Failed to fetch URL git:///tmp/bitbake-fetch-m30azy7_/gitsource;protocol=file;lfs=1;branch=master, attempting MIRRORS if available
> Fetcher failure: Repository file:///tmp/bitbake-fetch-m30azy7_/gitsource has LFS content, install git-lfs on host to download (or set lfs=0 to ignore it)
> Failed to fetch URL git:///tmp/bitbake-fetch-m30azy7_/gitsource;protocol=file;lfs=1;branch=master, attempting MIRRORS if available
> Fetcher failure: Repository file:///tmp/bitbake-fetch-m30azy7_/gitsource has LFS content, install git-lfs on host to download (or set lfs=0 to ignore it)
> 
> https://autobuilder.yoctoproject.org/valkyrie/#/builders/35/builds/1440/steps/12/logs/stdio

My bitbake master-next branch has a fix for this. We need to mark an
additional test as requiring git-lfs...

I've tested it and it works, I just need to clean up the patch and send
it which I didn't get to yet.

Cheers,

Richard
Philip Lorenz April 29, 2025, 6:12 a.m. UTC | #3 
Hi Richard,

On 28.04.25 23:09, Richard Purdie wrote:
> On Mon, 2025-04-28 at 18:25 +0200, Mathieu Dubois-Briand wrote:
>> On Fri Apr 25, 2025 at 4:41 PM CEST, Philip Lorenz wrote:
>>> This patch series fixes a number of issues we observed when using the
>>> gitsm fetcher in conjunction with git-lfs.
>>>
>>> I'd like to highlight two patches in this series:
>>> * 0004-fetch2-Simplify-git-LFS-detection: I could not come up with a
>>>    good reason on why the code so far preferred the content of
>>>   `.gitattributes` on the repository's branch instead of always
>>>    considering the state of the target revision. Please let me know
>>>    if there's something I didn't take into account.
>>> * 0005-fetch2-Use-git-lfs-fetch-to-download-objects.patch: I could not
>>>    replicate the `git-lfs fetch` behaviour in versions going back to
>>>    2.13.2 released in 2021. This leads me to believe that the issue
>>>    leading to this workaround is no longer in place. However, if anyone
>>>    remembers the actual issue / versions involved I can also retest to
>>>    confirm that there are no regression.
>>>
>>> ---
>> Hi Philip,
>>
>> Thanks for the new version.
>>
>> It works much better than previously. I still have some issues with
>> bitbake selftests on my branch:
>>
>> ERROR: test_lfs_enabled_not_installed (bb.tests.fetch.GitLfsTest.test_lfs_enabled_not_installed)
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>    File "/srv/pokybuild/yocto-worker/oe-selftest-debian/build/bitbake/lib/bb/tests/fetch.py", line 2528, in test_lfs_enabled_not_installed
>>      fetcher.download()
>>      ~~~~~~~~~~~~~~~~^^
>>    File "/srv/pokybuild/yocto-worker/oe-selftest-debian/build/bitbake/lib/bb/fetch2/__init__.py", line 1929, in download
>>      raise FetchError("Unable to fetch URL from any source.", u)
>> bb.fetch2.FetchError: Fetcher failure for URL: 'git:///tmp/bitbake-fetch-m30azy7_/gitsource;protocol=file;lfs=1;branch=master'. Unable to fetch URL from any source.
>>
>> Stdout:
>> Cloning into bare repository '/tmp/bitbake-fetch-m30azy7_/download/git2/tmp.bitbake-fetch-m30azy7_.gitsource'...
>> remote: Enumerating objects: 3, done.
>> remote: Counting objects: 100% (3/3), done.
>> remote: Total 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
>> Receiving objects:  33% (1/3)
>> Receiving objects: 100% (3/3), done.
>> Failed to fetch URL git:///tmp/bitbake-fetch-m30azy7_/gitsource;protocol=file;lfs=1;branch=master, attempting MIRRORS if available
>> Fetcher failure: Repository file:///tmp/bitbake-fetch-m30azy7_/gitsource has LFS content, install git-lfs on host to download (or set lfs=0 to ignore it)
>> Failed to fetch URL git:///tmp/bitbake-fetch-m30azy7_/gitsource;protocol=file;lfs=1;branch=master, attempting MIRRORS if available
>> Fetcher failure: Repository file:///tmp/bitbake-fetch-m30azy7_/gitsource has LFS content, install git-lfs on host to download (or set lfs=0 to ignore it)
>>
>> https://autobuilder.yoctoproject.org/valkyrie/#/builders/35/builds/1440/steps/12/logs/stdio
> My bitbake master-next branch has a fix for this. We need to mark an
> additional test as requiring git-lfs...
>
> I've tested it and it works, I just need to clean up the patch and send
> it which I didn't get to yet.
>
Are you referring to [1]? I had a brief look based on Mathieu's comment 
and the test case will need to be split into two parts (one which can be 
executed even when git-lfs isn't on the host, the second requiring 
git-lfs to be installed so the download step can happen before 
continuing correct behaviour during unpack). I'll send an update series 
today after actually executing all test cases in all test environments 
later today.

Philip


[1] 
https://git.openembedded.org/bitbake/commit/?h=master-next&id=3a219c691e37ec2f8d26e8de302616a3f94efee2