@@ -324,6 +324,9 @@ class Git(FetchMethod):
return False
def lfs_need_update(self, ud, d):
+ if not self._need_lfs(ud):
+ return False
+
if self.clonedir_need_update(ud, d):
return True
@@ -507,7 +510,9 @@ class Git(FetchMethod):
def lfs_fetch(self, ud, d, clonedir, revision, fetchall=False, progresshandler=None):
"""Helper method for fetching Git LFS data"""
try:
- if self._need_lfs(ud) and self._contains_lfs(ud, d, clonedir) and self._find_git_lfs(d) and len(revision):
+ if self._need_lfs(ud) and self._contains_lfs(ud, d, clonedir) and len(revision):
+ self._ensure_git_lfs(d, ud)
+
# Using worktree with the revision because .lfsconfig may exists
worktree_add_cmd = "%s worktree add wt %s" % (ud.basecmd, revision)
runfetchcmd(worktree_add_cmd, d, log=progresshandler, workdir=clonedir)
@@ -740,11 +745,11 @@ class Git(FetchMethod):
runfetchcmd("%s remote set-url origin %s" % (ud.basecmd, shlex.quote(repourl)), d, workdir=destdir)
if self._contains_lfs(ud, d, destdir):
- if need_lfs and not self._find_git_lfs(d):
- 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:
+ if not need_lfs:
bb.note("Repository %s has LFS content but it is not being fetched" % (repourl))
else:
+ self._ensure_git_lfs(d, ud)
+
runfetchcmd("%s lfs install --local" % ud.basecmd, d, workdir=destdir)
if not ud.nocheckout:
@@ -807,9 +812,11 @@ class Git(FetchMethod):
Verifies whether the LFS objects for requested revisions have already been downloaded
"""
# Bail out early if this repository doesn't use LFS
- if not self._need_lfs(ud) or not self._contains_lfs(ud, d, wd):
+ if not self._contains_lfs(ud, d, wd):
return True
+ self._ensure_git_lfs(d, ud)
+
# The Git LFS specification specifies ([1]) the LFS folder layout so it should be safe to check for file
# existence.
# [1] https://github.com/git-lfs/git-lfs/blob/main/docs/spec.md#intercepting-git
@@ -859,11 +866,14 @@ class Git(FetchMethod):
pass
return False
- def _find_git_lfs(self, d):
+ def _ensure_git_lfs(self, d, ud):
"""
- Return True if git-lfs can be found, False otherwise.
+ Ensures that git-lfs is available, raising a FetchError if it isn't.
"""
- return shutil.which("git-lfs", path=d.getVar('PATH')) is not None
+ if shutil.which("git-lfs", path=d.getVar('PATH')) is None:
+ raise bb.fetch2.FetchError(
+ "Repository %s has LFS content, install git-lfs on host to download (or set lfs=0 "
+ "to ignore it)" % self._get_repo_url(ud))
def _get_repo_url(self, ud):
"""
@@ -9,6 +9,7 @@
import contextlib
import shutil
import unittest
+import unittest.mock
import urllib.parse
import hashlib
import tempfile
@@ -2292,12 +2293,18 @@ class GitLfsTest(FetcherTest):
self.git_init(cwd=self.srcdir)
self.commit_file('.gitattributes', '*.mp3 filter=lfs -text')
- def commit_file(self, filename, content):
- with open(os.path.join(self.srcdir, filename), "w") as f:
+ def commit(self, *, cwd=None):
+ cwd = cwd or self.srcdir
+ self.git(["commit", "-m", "Change"], cwd=cwd)
+ return self.git(["rev-parse", "HEAD"], cwd=cwd).strip()
+
+ def commit_file(self, filename, content, *, cwd=None):
+ cwd = cwd or self.srcdir
+
+ with open(os.path.join(cwd, filename), "w") as f:
f.write(content)
- self.git(["add", filename], cwd=self.srcdir)
- self.git(["commit", "-m", "Change"], cwd=self.srcdir)
- return self.git(["rev-parse", "HEAD"], cwd=self.srcdir).strip()
+ self.git(["add", filename], cwd=cwd)
+ return self.commit(cwd=cwd)
def fetch(self, uri=None, download=True):
uris = self.d.getVar('SRC_URI').split()
@@ -2413,18 +2420,17 @@ class GitLfsTest(FetcherTest):
# Careful: suppress initial attempt at downloading
fetcher, ud = self.fetch(uri=None, download=False)
- # Artificially assert that git-lfs is not installed, so
- # we can verify a failure to unpack in it's absence.
- old_find_git_lfs = ud.method._find_git_lfs
- try:
- # If git-lfs cannot be found, the unpack should throw an error
+ # If git-lfs cannot be found, the download should throw an error
+ with unittest.mock.patch("shutil.which", return_value=None):
with self.assertRaises(bb.fetch2.FetchError):
fetcher.download()
- ud.method._find_git_lfs = lambda d: False
+
+ fetcher.download()
+ # If git-lfs cannot be found, the unpack should throw an error
+ with self.assertRaises(bb.fetch2.FetchError):
+ with unittest.mock.patch("shutil.which", return_value=None):
shutil.rmtree(self.gitdir, ignore_errors=True)
fetcher.unpack(self.d.getVar('WORKDIR'))
- finally:
- ud.method._find_git_lfs = old_find_git_lfs
def test_lfs_disabled_not_installed(self):
uri = 'git://%s;protocol=file;lfs=0;branch=master' % self.srcdir
@@ -2433,17 +2439,30 @@ class GitLfsTest(FetcherTest):
# Careful: suppress initial attempt at downloading
fetcher, ud = self.fetch(uri=None, download=False)
- # Artificially assert that git-lfs is not installed, so
- # we can verify a failure to unpack in it's absence.
- old_find_git_lfs = ud.method._find_git_lfs
- try:
- # Even if git-lfs cannot be found, the unpack should be successful
+ # Even if git-lfs cannot be found, the download / unpack should be successful
+ with unittest.mock.patch("shutil.which", return_value=None):
+ fetcher.download()
+ shutil.rmtree(self.gitdir, ignore_errors=True)
+ fetcher.unpack(self.d.getVar('WORKDIR'))
+
+ def test_lfs_enabled_not_installed_but_not_needed(self):
+ srcdir = os.path.join(self.tempdir, "emptygit")
+ bb.utils.mkdirhier(srcdir)
+ self.git_init(srcdir)
+ self.commit_file("test", "test content", cwd=srcdir)
+
+ uri = 'git://%s;protocol=file;lfs=1;branch=master' % srcdir
+ self.d.setVar('SRC_URI', uri)
+
+ # Careful: suppress initial attempt at downloading
+ fetcher, ud = self.fetch(uri=None, download=False)
+
+ # It shouldnt't matter that git-lfs cannot be found as the repository configuration does not
+ # specify any LFS filters.
+ with unittest.mock.patch("shutil.which", return_value=None):
fetcher.download()
- ud.method._find_git_lfs = lambda d: False
shutil.rmtree(self.gitdir, ignore_errors=True)
fetcher.unpack(self.d.getVar('WORKDIR'))
- finally:
- ud.method._find_git_lfs = old_find_git_lfs
class GitURLWithSpacesTest(FetcherTest):
test_git_urls = {
So far, existence of `git-lfs` was only checked during unpacking. As the binary is also used in earlier steps also check for its existence there. Additionally, factor out the LFS existence check into a dedicated function and call it wherever git-lfs is used for the first time. Signed-off-by: Philip Lorenz <philip.lorenz@bmw.de> --- V2 -> V3: * Only check for git-lfs existence after verifying that repository is using it * Add test case to verify that git-lfs is not required when the repository does not have a single LFS filter configured --- lib/bb/fetch2/git.py | 26 ++++++++++++------ lib/bb/tests/fetch.py | 61 ++++++++++++++++++++++++++++--------------- 2 files changed, 58 insertions(+), 29 deletions(-)