* [bitbake][kirkstone][2.0][PATCH 2/6] bitbake/codeparser.py: address ast module deprecations in py 3.12
2024-02-27 22:04 [bitbake][kirkstone][2.0][PATCH 0/6] Patch review Steve Sakoman
2024-02-27 22:04 ` [bitbake][kirkstone][2.0][PATCH 1/6] codeparser: replace deprecated ast.Str and 's' Steve Sakoman
@ 2024-02-27 22:04 ` Steve Sakoman
2024-02-27 22:04 ` [bitbake][kirkstone][2.0][PATCH 3/6] bitbake/lib/bs4/tests/test_tree.py: python 3.12 regex Steve Sakoman
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Steve Sakoman @ 2024-02-27 22:04 UTC (permalink / raw)
To: bitbake-devel
From: Alexander Kanavin <alex.kanavin@gmail.com>
Specifically:
/srv/work/alex/poky/bitbake/lib/bb/codeparser.py:279: DeprecationWarning: ast.Str is deprecated and will be removed in Python 3.14; use ast.Constant instead
if isinstance(node.args[0], ast.Str):
/srv/work/alex/poky/bitbake/lib/bb/codeparser.py:280: DeprecationWarning: Attribute s is deprecated and will be removed in Python 3.14; use value instead
value = node.args[0].s
Signed-off-by: Alexander Kanavin <alex@linutronix.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
lib/bb/codeparser.py | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/lib/bb/codeparser.py b/lib/bb/codeparser.py
index bb890dddd..6ce0c5182 100644
--- a/lib/bb/codeparser.py
+++ b/lib/bb/codeparser.py
@@ -227,17 +227,17 @@ class PythonParser():
if name and (name.endswith(self.getvars) or name.endswith(self.getvarflags) or name in self.containsfuncs or name in self.containsanyfuncs):
if isinstance(node.args[0], ast.Constant) and isinstance(node.args[0].value, str):
varname = node.args[0].value
- if name in self.containsfuncs and isinstance(node.args[1], ast.Str):
+ if name in self.containsfuncs and isinstance(node.args[1], ast.Constant):
if varname not in self.contains:
self.contains[varname] = set()
- self.contains[varname].add(node.args[1].s)
- elif name in self.containsanyfuncs and isinstance(node.args[1], ast.Str):
+ self.contains[varname].add(node.args[1].value)
+ elif name in self.containsanyfuncs and isinstance(node.args[1], ast.Constant):
if varname not in self.contains:
self.contains[varname] = set()
- self.contains[varname].update(node.args[1].s.split())
+ self.contains[varname].update(node.args[1].value.split())
elif name.endswith(self.getvarflags):
- if isinstance(node.args[1], ast.Str):
- self.references.add('%s[%s]' % (varname, node.args[1].s))
+ if isinstance(node.args[1], ast.Constant):
+ self.references.add('%s[%s]' % (varname, node.args[1].value))
else:
self.warn(node.func, node.args[1])
else:
@@ -245,8 +245,8 @@ class PythonParser():
else:
self.warn(node.func, node.args[0])
elif name and name.endswith(".expand"):
- if isinstance(node.args[0], ast.Str):
- value = node.args[0].s
+ if isinstance(node.args[0], ast.Constant):
+ value = node.args[0].value
d = bb.data.init()
parser = d.expandWithRefs(value, self.name)
self.references |= parser.references
@@ -256,8 +256,8 @@ class PythonParser():
self.contains[varname] = set()
self.contains[varname] |= parser.contains[varname]
elif name in self.execfuncs:
- if isinstance(node.args[0], ast.Str):
- self.var_execs.add(node.args[0].s)
+ if isinstance(node.args[0], ast.Constant):
+ self.var_execs.add(node.args[0].value)
else:
self.warn(node.func, node.args[0])
elif name and isinstance(node.func, (ast.Name, ast.Attribute)):
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [bitbake][kirkstone][2.0][PATCH 4/6] tests/fetch: git-lfs restore _find_git_lfs
2024-02-27 22:04 [bitbake][kirkstone][2.0][PATCH 0/6] Patch review Steve Sakoman
` (2 preceding siblings ...)
2024-02-27 22:04 ` [bitbake][kirkstone][2.0][PATCH 3/6] bitbake/lib/bs4/tests/test_tree.py: python 3.12 regex Steve Sakoman
@ 2024-02-27 22:04 ` Steve Sakoman
2024-02-27 22:04 ` [bitbake][kirkstone][2.0][PATCH 5/6] tests/fetch: Add real git lfs tests and decorator Steve Sakoman
2024-02-27 22:04 ` [bitbake][kirkstone][2.0][PATCH 6/6] fetch2: Ensure that git LFS objects are available Steve Sakoman
5 siblings, 0 replies; 7+ messages in thread
From: Steve Sakoman @ 2024-02-27 22:04 UTC (permalink / raw)
To: bitbake-devel
From: Paulo Neves <paulo@myneves.com>
Not restoring the mocked _find_git_lfs leads to other tests
failing.
(cherry picked from commit 70f848631450bd723c223227c21c60e815ee033d)
Signed-off-by: Paulo Neves <paulo@myneves.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Signed-off-by: Philip Lorenz <philip.lorenz@bmw.de>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
lib/bb/tests/fetch.py | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 7bace415d..743d3e3ff 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -2210,12 +2210,16 @@ class GitLfsTest(FetcherTest):
shutil.rmtree(self.gitdir, ignore_errors=True)
fetcher.unpack(self.d.getVar('WORKDIR'))
- # If git-lfs cannot be found, the unpack should throw an error
- with self.assertRaises(bb.fetch2.FetchError):
- fetcher.download()
- ud.method._find_git_lfs = lambda d: False
- shutil.rmtree(self.gitdir, ignore_errors=True)
- fetcher.unpack(self.d.getVar('WORKDIR'))
+ old_find_git_lfs = ud.method._find_git_lfs
+ try:
+ # If git-lfs cannot be found, the unpack should throw an error
+ with self.assertRaises(bb.fetch2.FetchError):
+ 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
def test_lfs_disabled(self):
import shutil
@@ -2230,17 +2234,21 @@ class GitLfsTest(FetcherTest):
fetcher, ud = self.fetch()
self.assertIsNotNone(ud.method._find_git_lfs)
- # If git-lfs can be found, the unpack should be successful. A
- # live copy of git-lfs is not required for this case, so
- # unconditionally forge its presence.
- ud.method._find_git_lfs = lambda d: True
- shutil.rmtree(self.gitdir, ignore_errors=True)
- fetcher.unpack(self.d.getVar('WORKDIR'))
+ old_find_git_lfs = ud.method._find_git_lfs
+ try:
+ # If git-lfs can be found, the unpack should be successful. A
+ # live copy of git-lfs is not required for this case, so
+ # unconditionally forge its presence.
+ ud.method._find_git_lfs = lambda d: True
+ shutil.rmtree(self.gitdir, ignore_errors=True)
+ fetcher.unpack(self.d.getVar('WORKDIR'))
+ # If git-lfs cannot be found, the unpack should be successful
- # If git-lfs cannot be found, the unpack should be successful
- ud.method._find_git_lfs = lambda d: False
- shutil.rmtree(self.gitdir, ignore_errors=True)
- fetcher.unpack(self.d.getVar('WORKDIR'))
+ 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 = {
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [bitbake][kirkstone][2.0][PATCH 6/6] fetch2: Ensure that git LFS objects are available
2024-02-27 22:04 [bitbake][kirkstone][2.0][PATCH 0/6] Patch review Steve Sakoman
` (4 preceding siblings ...)
2024-02-27 22:04 ` [bitbake][kirkstone][2.0][PATCH 5/6] tests/fetch: Add real git lfs tests and decorator Steve Sakoman
@ 2024-02-27 22:04 ` Steve Sakoman
5 siblings, 0 replies; 7+ messages in thread
From: Steve Sakoman @ 2024-02-27 22:04 UTC (permalink / raw)
To: bitbake-devel
From: Philip Lorenz <philip.lorenz@bmw.de>
The current implementation only performs a git lfs fetch alongside of a
regular git fetch. This causes issues when the downloaded revision is
already part of the fetched repository (e.g. because of moving back in
history or the updated revision already being part of the repository at
the time of the initial clone).
Fix this by explicitly checking whether the required LFS objects are
available in the downloade directory before confirming that a downloaded
repository is up-to-date.
This issue previously went unnoticed as git lfs would silently fetch the
missing objects during the `unpack` task. With network isolation turned
on, this no longer works, and unpacking fails.
(cherry picked from commit cfae1556bf671acec119a6c8bbc4b667a856b9ae)
Signed-off-by: Philip Lorenz <philip.lorenz@bmw.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Signed-off-by: Philip Lorenz <philip.lorenz@bmw.de>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
lib/bb/fetch2/git.py | 45 ++++++++++++++++++++++++++++++++++++--
lib/bb/tests/fetch.py | 51 +++++++++++++++++++++++++++++++++++++++----
2 files changed, 90 insertions(+), 6 deletions(-)
diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index 4d6e57ade..9ecc855af 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -307,7 +307,10 @@ class Git(FetchMethod):
return ud.clonedir
def need_update(self, ud, d):
- return self.clonedir_need_update(ud, d) or self.shallow_tarball_need_update(ud) or self.tarball_need_update(ud)
+ return self.clonedir_need_update(ud, d) \
+ or self.shallow_tarball_need_update(ud) \
+ or self.tarball_need_update(ud) \
+ or self.lfs_need_update(ud, d)
def clonedir_need_update(self, ud, d):
if not os.path.exists(ud.clonedir):
@@ -319,6 +322,15 @@ class Git(FetchMethod):
return True
return False
+ def lfs_need_update(self, ud, d):
+ if self.clonedir_need_update(ud, d):
+ return True
+
+ for name in ud.names:
+ if not self._lfs_objects_downloaded(ud, d, name, ud.clonedir):
+ return True
+ return False
+
def clonedir_need_shallow_revs(self, ud, d):
for rev in ud.shallow_revs:
try:
@@ -406,7 +418,7 @@ class Git(FetchMethod):
if missing_rev:
raise bb.fetch2.FetchError("Unable to find revision %s even from upstream" % missing_rev)
- if self._contains_lfs(ud, d, ud.clonedir) and self._need_lfs(ud):
+ if self.lfs_need_update(ud, d):
# Unpack temporary working copy, use it to run 'git checkout' to force pre-fetching
# of all LFS blobs needed at the srcrev.
#
@@ -649,6 +661,35 @@ class Git(FetchMethod):
raise bb.fetch2.FetchError("The command '%s' gave output with more then 1 line unexpectedly, output: '%s'" % (cmd, output))
return output.split()[0] != "0"
+ def _lfs_objects_downloaded(self, ud, d, name, wd):
+ """
+ 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):
+ return True
+
+ # 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
+ cmd = "%s lfs ls-files -l %s" \
+ % (ud.basecmd, ud.revisions[name])
+ output = runfetchcmd(cmd, d, quiet=True, workdir=wd).rstrip()
+ # Do not do any further matching if no objects are managed by LFS
+ if not output:
+ return True
+
+ # Match all lines beginning with the hexadecimal OID
+ oid_regex = re.compile("^(([a-fA-F0-9]{2})([a-fA-F0-9]{2})[A-Fa-f0-9]+)")
+ for line in output.split("\n"):
+ oid = re.search(oid_regex, line)
+ if not oid:
+ bb.warn("git lfs ls-files output '%s' did not match expected format." % line)
+ if not os.path.exists(os.path.join(wd, "lfs", "objects", oid.group(2), oid.group(3), oid.group(1))):
+ return False
+
+ return True
+
def _need_lfs(self, ud):
return ud.parm.get("lfs", "1") == "1"
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 847a35602..5aa3e464d 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -6,6 +6,7 @@
# SPDX-License-Identifier: GPL-2.0-only
#
+import contextlib
import unittest
import hashlib
import tempfile
@@ -2182,10 +2183,14 @@ class GitLfsTest(FetcherTest):
bb.utils.mkdirhier(self.srcdir)
self.git_init(cwd=self.srcdir)
- with open(os.path.join(self.srcdir, '.gitattributes'), 'wt') as attrs:
- attrs.write('*.mp3 filter=lfs -text')
- self.git(['add', '.gitattributes'], cwd=self.srcdir)
- self.git(['commit', '-m', "attributes", '.gitattributes'], 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:
+ 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()
def fetch(self, uri=None, download=True):
uris = self.d.getVar('SRC_URI').split()
@@ -2205,6 +2210,44 @@ class GitLfsTest(FetcherTest):
unpacked_lfs_file = os.path.join(self.d.getVar('WORKDIR'), 'git', "Cat_poster_1.jpg")
return unpacked_lfs_file
+ @skipIfNoGitLFS()
+ def test_fetch_lfs_on_srcrev_change(self):
+ """Test if fetch downloads missing LFS objects when a different revision within an existing repository is requested"""
+ self.git(["lfs", "install", "--local"], cwd=self.srcdir)
+
+ @contextlib.contextmanager
+ def hide_upstream_repository():
+ """Hide the upstream repository to make sure that git lfs cannot pull from it"""
+ temp_name = self.srcdir + ".bak"
+ os.rename(self.srcdir, temp_name)
+ try:
+ yield
+ finally:
+ os.rename(temp_name, self.srcdir)
+
+ def fetch_and_verify(revision, filename, content):
+ self.d.setVar('SRCREV', revision)
+ fetcher, ud = self.fetch()
+
+ with hide_upstream_repository():
+ workdir = self.d.getVar('WORKDIR')
+ fetcher.unpack(workdir)
+
+ with open(os.path.join(workdir, "git", filename)) as f:
+ self.assertEqual(f.read(), content)
+
+ commit_1 = self.commit_file("a.mp3", "version 1")
+ commit_2 = self.commit_file("a.mp3", "version 2")
+
+ self.d.setVar('SRC_URI', "git://%s;protocol=file;lfs=1;branch=master" % self.srcdir)
+
+ # Seed the local download folder by fetching the latest commit and verifying that the LFS contents are
+ # available even when the upstream repository disappears.
+ fetch_and_verify(commit_2, "a.mp3", "version 2")
+ # Verify that even when an older revision is fetched, the needed LFS objects are fetched into the download
+ # folder.
+ fetch_and_verify(commit_1, "a.mp3", "version 1")
+
@skipIfNoGitLFS()
@skipIfNoNetwork()
def test_real_git_lfs_repo_succeeds_without_lfs_param(self):
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread