* [PATCH] fetch2/local: verify checksums for file:// urls
@ 2026-04-30 14:25 Jhonata Poma-Hansen
2026-05-08 14:13 ` [bitbake-devel] " Paul Barker
0 siblings, 1 reply; 20+ messages in thread
From: Jhonata Poma-Hansen @ 2026-04-30 14:25 UTC (permalink / raw)
To: bitbake-devel
Local.urldata_init unconditionally cleared ud.needdonestamp, which
short-circuited the donestamp + verify_checksum cycle for every
file:// url. As a result, a checksum supplied as a url parameter
(for instance UNINATIVE_CHECKSUM expanded into the uninative
tarball's file:// url) was never compared against the file's
actual contents - the fetcher silently used whatever bytes were
at the path.
Keep needdonestamp at the FetchData default (True) when the url
carries any of the known checksum parameters, so the standard
verify_checksum flow runs and a mismatch raises ChecksumError just
like it does for http:// and friends. The check uses CHECKSUM_LIST
and handles both the bare ;<algo>sum= form and the named
;name=foo;foo.<algo>sum= form that recipes (and UNINATIVE_CHECKSUM)
actually use. When no checksum is supplied (the common case for
file:// recipe patches) needdonestamp stays False and behavior is
unchanged - no .done / .lock files appear in DL_DIR for those urls.
For checksummed file:// urls, the donestamp + lockfile do land in
DL_DIR (keyed by basename, alongside the existing http(s) state
for the same basename); this is intentional and matches how every
other fetcher tracks verified content.
An earlier fix (b8b14d975a25 "fetch2: Ensure we don't have file
downloads overwriting each other", 2017) added a short-circuit at
the top of verify_donestamp() so that when origud is a file:// url
with needdonestamp=False, the function returns True early. That
short-circuit was protecting the SSTATE_MIRRORS case where a
file:// url is mapped to an http:// mirror and the http path would
otherwise race against the file:// origud. It still does the
right thing under this change: the only file:// origud whose
needdonestamp now flips to True is one that carries an explicit
checksum parameter, and in that case we actively want the mirror
to run verify_donestamp(), not bypass it. The unchecksummed
file:// origud case (the original SSTATE_MIRRORS scenario) keeps
needdonestamp=False and the 2017 short-circuit fires unchanged.
Two related changes in fetch2 are worth pointing at:
* commit 6424f4b7e9c1 ("fetch2: Ensure mirror tarballs don't enforce
checksum", 2022) marks SSTATE_MIRRORS targets with
newud.ignore_checksums = True. Its commit message states "local
file fetches now validate checksums" - this fix is what finally
makes that statement true. The ignore_checksums flag also acts as
the safety hatch that prevents flipping needdonestamp on for
checksummed file:// urls from propagating verification onto
mirror-of-git-repo tarballs (whose checksums by design do not
match).
* commit 4b8de2e7d126 ("wget: Avoid bad checksum race issues",
2022) established the don't-mutate-on-mismatch doctrine for
downloaded artifacts via the localpath= and fatal_nochecksum=
parameters. The rename_bad_checksum() early-return below applies
the same doctrine to a fetcher whose 'localpath' is the user's
source tree.
A checksum mismatch on a file:// url must not mutate the user's
source tree the way it does for downloaded artifacts in DL_DIR.
The standard error path renames <localpath> to
<localpath>_bad-checksum_<sha> via rename_bad_checksum(), which
for file:// would rename a recipe-shipped patch or an absolute
path the user had supplied. Skip that rename inside
rename_bad_checksum() when ud.type == 'file' so all four call
sites stay consistent and a ChecksumError surfaces cleanly without
on-disk side effects.
Add unit tests in FetcherLocalTest covering: the no-checksum hot
path keeps needdonestamp=False (no regression for recipe patches);
matching and mismatching sha256 and md5 checksums; the named
;name=foo;foo.sha256sum= form; and that a directory url with a
checksum is silently ignored (supports_checksum() is False for
directories). The mismatch tests assert that the source file is
not renamed.
Link: https://bugzilla.yoctoproject.org/show_bug.cgi?id=9993
[YOCTO #9993]
Signed-off-by: Jhonata Poma-Hansen <jhonata.poma@gmail.com>
---
lib/bb/fetch2/__init__.py | 8 ++++
lib/bb/fetch2/local.py | 19 ++++++++-
lib/bb/tests/fetch.py | 104 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 129 insertions(+), 2 deletions(-)
diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 52d5556d..35874e25 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -1075,6 +1075,14 @@ def rename_bad_checksum(ud, suffix):
if ud.localpath is None:
return
+ # For file:// URLs the localpath points at the user's source tree
+ # (e.g. recipe-shipped files under FILESPATH or an absolute path the
+ # user passed in). Renaming those out from under the user is
+ # destructive and surprising; surface the ChecksumError without
+ # mutating their files.
+ if ud.type == 'file':
+ return
+
new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix)
bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath))
if not bb.utils.movefile(ud.localpath, new_localpath):
diff --git a/lib/bb/fetch2/local.py b/lib/bb/fetch2/local.py
index fda56a56..2abcc5eb 100644
--- a/lib/bb/fetch2/local.py
+++ b/lib/bb/fetch2/local.py
@@ -17,7 +17,7 @@ import os
import urllib.request, urllib.parse, urllib.error
import bb
import bb.utils
-from bb.fetch2 import FetchMethod, FetchError, ParameterError
+from bb.fetch2 import CHECKSUM_LIST, FetchMethod, FetchError, ParameterError
from bb.fetch2 import logger
class Local(FetchMethod):
@@ -31,7 +31,22 @@ class Local(FetchMethod):
# We don't set localfile as for this fetcher the file is already local!
ud.basename = os.path.basename(ud.path)
ud.basepath = ud.path
- ud.needdonestamp = False
+ # Without a donestamp the verify_checksum path is bypassed entirely
+ # for file:// urls, so an explicit checksum on the url (for instance
+ # UNINATIVE_CHECKSUM expanded into the uninative tarball file:// url)
+ # is never compared against the file's actual contents. Keep
+ # needdonestamp at the FetchData default (True) when any of the
+ # known checksum parameters is present, so the standard
+ # donestamp + verify_checksum cycle runs and a mismatch raises
+ # ChecksumError. When no checksum is supplied (the common case for
+ # recipe patches under file://) needdonestamp stays False and
+ # behavior is unchanged.
+ name = ud.parm.get("name")
+ ud.needdonestamp = any(
+ (name and "%s.%ssum" % (name, a) in ud.parm)
+ or "%ssum" % a in ud.parm
+ for a in CHECKSUM_LIST
+ )
if "*" in ud.path:
raise bb.fetch2.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url)
return
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 077f741e..3347c60d 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -813,6 +813,110 @@ class FetcherLocalTest(FetcherTest):
tree = self.fetchUnpack(['file://archive.tar.bz2;subdir=bar;striplevel=1'])
self.assertEqual(tree, ['bar/c', 'bar/d', 'bar/subdir/e'])
+ def test_local_no_checksum_no_donestamp(self):
+ # The common case: file:// recipe patches without a checksum
+ # parameter must keep the historical "no donestamp tracking"
+ # behavior so this fix does not regress the hot path. After
+ # download(), no .done / .lock entries must appear in DL_DIR
+ # for the bare file:// url.
+ with open(os.path.join(self.localsrcdir, 'plain'), 'wb') as f:
+ f.write(b"plain\n")
+ fetcher = bb.fetch.Fetch(['file://plain'], self.d)
+ ud = fetcher.ud[fetcher.urls[0]]
+ self.assertFalse(ud.needdonestamp)
+ # When needdonestamp is False, FetchData leaves ud.donestamp
+ # unset (None); we assert that no .done / .lock entries are
+ # written to DL_DIR for the bare file:// url after download.
+ fetcher.download()
+ for entry in os.listdir(self.dldir):
+ self.assertFalse(entry.startswith('plain.done'))
+ self.assertFalse(entry.startswith('plain.lock'))
+
+ def test_local_checksum_match(self):
+ # A correct sha256 must drive the verify_checksum cycle to
+ # completion: download() returns without raising AND the
+ # donestamp lands in DL_DIR. The donestamp is the on-disk
+ # evidence that update_stamp() ran after verify_checksum()
+ # passed - without the fix, needdonestamp=False short-circuits
+ # update_stamp() and no donestamp is written even on "match".
+ import hashlib
+ content = b"file:// checksum match test\n"
+ with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f:
+ f.write(content)
+ good = hashlib.sha256(content).hexdigest()
+ fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d)
+ ud = fetcher.ud[fetcher.urls[0]]
+ self.assertTrue(ud.needdonestamp)
+ fetcher.download()
+ self.assertTrue(os.path.exists(ud.donestamp))
+
+ def test_local_checksum_mismatch(self):
+ content = b"file:// checksum mismatch test\n"
+ srcpath = os.path.join(self.localsrcdir, 'sumfile')
+ with open(srcpath, 'wb') as f:
+ f.write(content)
+ bad = "0" * 64
+ fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d)
+ with self.assertRaises(bb.fetch2.FetchError):
+ fetcher.download()
+ # The user's source tree must not be mutated. rename_bad_checksum
+ # would otherwise leave a <path>_bad-checksum_<sha> sibling and
+ # remove the original.
+ self.assertTrue(os.path.exists(srcpath))
+ with open(srcpath, 'rb') as f:
+ self.assertEqual(f.read(), content)
+ for entry in os.listdir(self.localsrcdir):
+ self.assertNotIn('_bad-checksum_', entry)
+
+ def test_local_checksum_mismatch_md5(self):
+ # Coverage for an algorithm other than sha256 so a regression
+ # that special-cased sha256 detection does not slip through.
+ # Mirror the sha256 mismatch assertions: source file present,
+ # contents unchanged, no _bad-checksum_ sibling left behind.
+ content = b"file:// md5 mismatch test\n"
+ srcpath = os.path.join(self.localsrcdir, 'sumfile_md5')
+ with open(srcpath, 'wb') as f:
+ f.write(content)
+ bad = "0" * 32
+ fetcher = bb.fetch.Fetch(['file://sumfile_md5;md5sum=' + bad], self.d)
+ with self.assertRaises(bb.fetch2.FetchError):
+ fetcher.download()
+ self.assertTrue(os.path.exists(srcpath))
+ with open(srcpath, 'rb') as f:
+ self.assertEqual(f.read(), content)
+ for entry in os.listdir(self.localsrcdir):
+ self.assertNotIn('_bad-checksum_', entry)
+
+ def test_local_checksum_named(self):
+ # UNINATIVE_CHECKSUM (the original reproducer for this bug) and
+ # several other recipe-driven file:// urls use the name= prefix
+ # form: foo.sha256sum= rather than bare sha256sum=. Confirm the
+ # named form flips needdonestamp AND that the verify cycle runs
+ # (donestamp on disk after download).
+ import hashlib
+ content = b"file:// named checksum test\n"
+ with open(os.path.join(self.localsrcdir, 'sumfile_named'), 'wb') as f:
+ f.write(content)
+ good = hashlib.sha256(content).hexdigest()
+ fetcher = bb.fetch.Fetch(
+ ['file://sumfile_named;name=blob;blob.sha256sum=' + good], self.d)
+ ud = fetcher.ud[fetcher.urls[0]]
+ self.assertTrue(ud.needdonestamp)
+ fetcher.download()
+ self.assertTrue(os.path.exists(ud.donestamp))
+
+ def test_local_checksum_directory_ignored(self):
+ # Local supports directory urls; supports_checksum returns False
+ # for directories. A checksum parm on a directory url should
+ # therefore be a no-op rather than a hard error: verify_checksum
+ # short-circuits on supports_checksum() and no ChecksumError is
+ # raised even when the checksum value is bogus.
+ bad = "0" * 64
+ fetcher = bb.fetch.Fetch(['file://dir;sha256sum=' + bad], self.d)
+ ud = fetcher.ud[fetcher.urls[0]]
+ self.assertFalse(ud.method.supports_checksum(ud))
+ fetcher.download()
+
def dummyGitTest(self, suffix):
# Create dummy local Git repo
src_dir = tempfile.mkdtemp(dir=self.tempdir,
---
base-commit: 5d722b5d65e4eef7befe6376983385421e993f86
change-id: 20260429-b4-yocto-9993-cadd9cbed7ce
Best regards,
--
Jhonata Poma-Hansen <jhonata.poma@gmail.com>
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [bitbake-devel] [PATCH] fetch2/local: verify checksums for file:// urls 2026-04-30 14:25 [PATCH] fetch2/local: verify checksums for file:// urls Jhonata Poma-Hansen @ 2026-05-08 14:13 ` Paul Barker 2026-05-11 14:58 ` [PATCH v2] " Jhonata Poma-Hansen 0 siblings, 1 reply; 20+ messages in thread From: Paul Barker @ 2026-05-08 14:13 UTC (permalink / raw) To: Jhonata.poma, bitbake-devel [-- Attachment #1: Type: text/plain, Size: 15165 bytes --] On Thu, 2026-04-30 at 16:25 +0200, Jhonata Poma-Hansen via lists.openembedded.org wrote: > Local.urldata_init unconditionally cleared ud.needdonestamp, which > short-circuited the donestamp + verify_checksum cycle for every > file:// url. As a result, a checksum supplied as a url parameter > (for instance UNINATIVE_CHECKSUM expanded into the uninative > tarball's file:// url) was never compared against the file's > actual contents - the fetcher silently used whatever bytes were > at the path. > > Keep needdonestamp at the FetchData default (True) when the url > carries any of the known checksum parameters, so the standard > verify_checksum flow runs and a mismatch raises ChecksumError just > like it does for http:// and friends. The check uses CHECKSUM_LIST > and handles both the bare ;<algo>sum= form and the named > ;name=foo;foo.<algo>sum= form that recipes (and UNINATIVE_CHECKSUM) > actually use. When no checksum is supplied (the common case for > file:// recipe patches) needdonestamp stays False and behavior is > unchanged - no .done / .lock files appear in DL_DIR for those urls. > > For checksummed file:// urls, the donestamp + lockfile do land in > DL_DIR (keyed by basename, alongside the existing http(s) state > for the same basename); this is intentional and matches how every > other fetcher tracks verified content. > > An earlier fix (b8b14d975a25 "fetch2: Ensure we don't have file > downloads overwriting each other", 2017) added a short-circuit at > the top of verify_donestamp() so that when origud is a file:// url > with needdonestamp=False, the function returns True early. That > short-circuit was protecting the SSTATE_MIRRORS case where a > file:// url is mapped to an http:// mirror and the http path would > otherwise race against the file:// origud. It still does the > right thing under this change: the only file:// origud whose > needdonestamp now flips to True is one that carries an explicit > checksum parameter, and in that case we actively want the mirror > to run verify_donestamp(), not bypass it. The unchecksummed > file:// origud case (the original SSTATE_MIRRORS scenario) keeps > needdonestamp=False and the 2017 short-circuit fires unchanged. > > Two related changes in fetch2 are worth pointing at: > > * commit 6424f4b7e9c1 ("fetch2: Ensure mirror tarballs don't enforce > checksum", 2022) marks SSTATE_MIRRORS targets with > newud.ignore_checksums = True. Its commit message states "local > file fetches now validate checksums" - this fix is what finally > makes that statement true. The ignore_checksums flag also acts as > the safety hatch that prevents flipping needdonestamp on for > checksummed file:// urls from propagating verification onto > mirror-of-git-repo tarballs (whose checksums by design do not > match). > * commit 4b8de2e7d126 ("wget: Avoid bad checksum race issues", > 2022) established the don't-mutate-on-mismatch doctrine for > downloaded artifacts via the localpath= and fatal_nochecksum= > parameters. The rename_bad_checksum() early-return below applies > the same doctrine to a fetcher whose 'localpath' is the user's > source tree. > > A checksum mismatch on a file:// url must not mutate the user's > source tree the way it does for downloaded artifacts in DL_DIR. > The standard error path renames <localpath> to > <localpath>_bad-checksum_<sha> via rename_bad_checksum(), which > for file:// would rename a recipe-shipped patch or an absolute > path the user had supplied. Skip that rename inside > rename_bad_checksum() when ud.type == 'file' so all four call > sites stay consistent and a ChecksumError surfaces cleanly without > on-disk side effects. > > Add unit tests in FetcherLocalTest covering: the no-checksum hot > path keeps needdonestamp=False (no regression for recipe patches); > matching and mismatching sha256 and md5 checksums; the named > ;name=foo;foo.sha256sum= form; and that a directory url with a > checksum is silently ignored (supports_checksum() is False for > directories). The mismatch tests assert that the source file is > not renamed. > > Link: https://bugzilla.yoctoproject.org/show_bug.cgi?id=9993 > [YOCTO #9993] > > Signed-off-by: Jhonata Poma-Hansen <jhonata.poma@gmail.com> Hi, The commit message here is incredibly verbose, to the point that it is difficult to read. While being verbose, it fails to mention that UNINATIVE_URL uses 'https://' in our metadata and so the issue only arises when you replace this with a local 'file://' URL (this is mentioned in the description of issue #9993). Please simplify the commit message and focus on the relevant information. > --- > lib/bb/fetch2/__init__.py | 8 ++++ > lib/bb/fetch2/local.py | 19 ++++++++- > lib/bb/tests/fetch.py | 104 ++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 129 insertions(+), 2 deletions(-) > > diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py > index 52d5556d..35874e25 100644 > --- a/lib/bb/fetch2/__init__.py > +++ b/lib/bb/fetch2/__init__.py > @@ -1075,6 +1075,14 @@ def rename_bad_checksum(ud, suffix): > if ud.localpath is None: > return > > + # For file:// URLs the localpath points at the user's source tree > + # (e.g. recipe-shipped files under FILESPATH or an absolute path the > + # user passed in). Renaming those out from under the user is > + # destructive and surprising; surface the ChecksumError without > + # mutating their files. > + if ud.type == 'file': > + return We should not be adding special cases for specific fetchers in __init__.py. Perhaps we could instead check if ud.localpath is under DL_DIR, or move this function to the FetchMethod class so that the Local fetcher can override it with an empty function. > + > new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) > bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) > if not bb.utils.movefile(ud.localpath, new_localpath): > diff --git a/lib/bb/fetch2/local.py b/lib/bb/fetch2/local.py > index fda56a56..2abcc5eb 100644 > --- a/lib/bb/fetch2/local.py > +++ b/lib/bb/fetch2/local.py > @@ -17,7 +17,7 @@ import os > import urllib.request, urllib.parse, urllib.error > import bb > import bb.utils > -from bb.fetch2 import FetchMethod, FetchError, ParameterError > +from bb.fetch2 import CHECKSUM_LIST, FetchMethod, FetchError, ParameterError > from bb.fetch2 import logger > > class Local(FetchMethod): > @@ -31,7 +31,22 @@ class Local(FetchMethod): > # We don't set localfile as for this fetcher the file is already local! > ud.basename = os.path.basename(ud.path) > ud.basepath = ud.path > - ud.needdonestamp = False > + # Without a donestamp the verify_checksum path is bypassed entirely > + # for file:// urls, so an explicit checksum on the url (for instance > + # UNINATIVE_CHECKSUM expanded into the uninative tarball file:// url) > + # is never compared against the file's actual contents. Keep > + # needdonestamp at the FetchData default (True) when any of the > + # known checksum parameters is present, so the standard > + # donestamp + verify_checksum cycle runs and a mismatch raises > + # ChecksumError. When no checksum is supplied (the common case for > + # recipe patches under file://) needdonestamp stays False and > + # behavior is unchanged. > + name = ud.parm.get("name") > + ud.needdonestamp = any( > + (name and "%s.%ssum" % (name, a) in ud.parm) > + or "%ssum" % a in ud.parm > + for a in CHECKSUM_LIST > + ) The comment here is of about the right level of detail for the commit message. A one or two line comment here to explain the conditional may be useful, but more than that is probably too verbose. > if "*" in ud.path: > raise bb.fetch2.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url) > return > diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py > index 077f741e..3347c60d 100644 > --- a/lib/bb/tests/fetch.py > +++ b/lib/bb/tests/fetch.py > @@ -813,6 +813,110 @@ class FetcherLocalTest(FetcherTest): > tree = self.fetchUnpack(['file://archive.tar.bz2;subdir=bar;striplevel=1']) > self.assertEqual(tree, ['bar/c', 'bar/d', 'bar/subdir/e']) > > + def test_local_no_checksum_no_donestamp(self): > + # The common case: file:// recipe patches without a checksum > + # parameter must keep the historical "no donestamp tracking" > + # behavior so this fix does not regress the hot path. After > + # download(), no .done / .lock entries must appear in DL_DIR > + # for the bare file:// url. > + with open(os.path.join(self.localsrcdir, 'plain'), 'wb') as f: > + f.write(b"plain\n") > + fetcher = bb.fetch.Fetch(['file://plain'], self.d) > + ud = fetcher.ud[fetcher.urls[0]] > + self.assertFalse(ud.needdonestamp) > + # When needdonestamp is False, FetchData leaves ud.donestamp > + # unset (None); we assert that no .done / .lock entries are > + # written to DL_DIR for the bare file:// url after download. > + fetcher.download() > + for entry in os.listdir(self.dldir): > + self.assertFalse(entry.startswith('plain.done')) > + self.assertFalse(entry.startswith('plain.lock')) I don't think this test case adds much value. > + > + def test_local_checksum_match(self): > + # A correct sha256 must drive the verify_checksum cycle to > + # completion: download() returns without raising AND the > + # donestamp lands in DL_DIR. The donestamp is the on-disk > + # evidence that update_stamp() ran after verify_checksum() > + # passed - without the fix, needdonestamp=False short-circuits > + # update_stamp() and no donestamp is written even on "match". > + import hashlib > + content = b"file:// checksum match test\n" > + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: > + f.write(content) > + good = hashlib.sha256(content).hexdigest() > + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) > + ud = fetcher.ud[fetcher.urls[0]] > + self.assertTrue(ud.needdonestamp) > + fetcher.download() > + self.assertTrue(os.path.exists(ud.donestamp)) This test case looks good, but the comment is again too verbose, please simplify it. > + > + def test_local_checksum_mismatch(self): > + content = b"file:// checksum mismatch test\n" > + srcpath = os.path.join(self.localsrcdir, 'sumfile') > + with open(srcpath, 'wb') as f: > + f.write(content) > + bad = "0" * 64 > + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d) > + with self.assertRaises(bb.fetch2.FetchError): > + fetcher.download() > + # The user's source tree must not be mutated. rename_bad_checksum > + # would otherwise leave a <path>_bad-checksum_<sha> sibling and > + # remove the original. > + self.assertTrue(os.path.exists(srcpath)) > + with open(srcpath, 'rb') as f: > + self.assertEqual(f.read(), content) > + for entry in os.listdir(self.localsrcdir): > + self.assertNotIn('_bad-checksum_', entry) This test case is mostly good. I don't think we need to loop through the whole of the directory listing, can we just confirm that the bad-checksum marker for sumfile doesn't exist? > + > + def test_local_checksum_mismatch_md5(self): > + # Coverage for an algorithm other than sha256 so a regression > + # that special-cased sha256 detection does not slip through. > + # Mirror the sha256 mismatch assertions: source file present, > + # contents unchanged, no _bad-checksum_ sibling left behind. > + content = b"file:// md5 mismatch test\n" > + srcpath = os.path.join(self.localsrcdir, 'sumfile_md5') > + with open(srcpath, 'wb') as f: > + f.write(content) > + bad = "0" * 32 > + fetcher = bb.fetch.Fetch(['file://sumfile_md5;md5sum=' + bad], self.d) > + with self.assertRaises(bb.fetch2.FetchError): > + fetcher.download() > + self.assertTrue(os.path.exists(srcpath)) > + with open(srcpath, 'rb') as f: > + self.assertEqual(f.read(), content) > + for entry in os.listdir(self.localsrcdir): > + self.assertNotIn('_bad-checksum_', entry) I don't think this test case adds value, the implementation is not sha256sum specific. > + > + def test_local_checksum_named(self): > + # UNINATIVE_CHECKSUM (the original reproducer for this bug) and > + # several other recipe-driven file:// urls use the name= prefix > + # form: foo.sha256sum= rather than bare sha256sum=. Confirm the > + # named form flips needdonestamp AND that the verify cycle runs > + # (donestamp on disk after download). > + import hashlib > + content = b"file:// named checksum test\n" > + with open(os.path.join(self.localsrcdir, 'sumfile_named'), 'wb') as f: > + f.write(content) > + good = hashlib.sha256(content).hexdigest() > + fetcher = bb.fetch.Fetch( > + ['file://sumfile_named;name=blob;blob.sha256sum=' + good], self.d) > + ud = fetcher.ud[fetcher.urls[0]] > + self.assertTrue(ud.needdonestamp) > + fetcher.download() > + self.assertTrue(os.path.exists(ud.donestamp)) This is a case of testing at the wrong level of detail. Do we already have test coverage that setting foo.some_property for a SRC_URI entry with name=foo results in the property being set for the appropriate entry? If so, we don't need further test coverage here. If not, we should add a generic test case for that. > + > + def test_local_checksum_directory_ignored(self): > + # Local supports directory urls; supports_checksum returns False > + # for directories. A checksum parm on a directory url should > + # therefore be a no-op rather than a hard error: verify_checksum > + # short-circuits on supports_checksum() and no ChecksumError is > + # raised even when the checksum value is bogus. > + bad = "0" * 64 > + fetcher = bb.fetch.Fetch(['file://dir;sha256sum=' + bad], self.d) > + ud = fetcher.ud[fetcher.urls[0]] > + self.assertFalse(ud.method.supports_checksum(ud)) > + fetcher.download() This tests that we suppress an error that we should actually be raising. If the checksum is invalid, then we should raise an error. Best regards, -- Paul Barker [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 252 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] fetch2/local: verify checksums for file:// urls 2026-05-08 14:13 ` [bitbake-devel] " Paul Barker @ 2026-05-11 14:58 ` Jhonata Poma-Hansen 2026-05-11 15:01 ` Jhonata Poma-Hansen 2026-09-23 19:44 ` [PATCH v3] " Savvas Etairidis 0 siblings, 2 replies; 20+ messages in thread From: Jhonata Poma-Hansen @ 2026-05-11 14:58 UTC (permalink / raw) To: bitbake-devel; +Cc: paul, richard.purdie, peter.kjellerstedt, jhonata.poma UNINATIVE_URL ships as an https:// url in metadata and verify_checksum catches a corrupt download. Replace it with a local file:// (the supported form for offline or pinned uninative tarballs) and the checksum supplied via UNINATIVE_CHECKSUM was silently ignored: Local.urldata_init unconditionally cleared ud.needdonestamp, short-circuiting the donestamp + verify_checksum cycle for every file:// url. Recipes that supply ;sha256sum= on a file:// url hit the same hole. Keep needdonestamp at the FetchData default (True) when the url carries any of the known checksum parameters (bare ;<algo>sum= or named ;name=foo;foo.<algo>sum= form), so the standard verify_checksum flow runs and a mismatch raises ChecksumError just like it does for http(s)://. When no checksum is supplied (the common file:// recipe patch case) needdonestamp stays False and behavior is unchanged. A checksum mismatch on a file:// url must not mutate the user's source tree the way it does for downloaded artifacts under DL_DIR. Move rename_bad_checksum onto FetchMethod and override it as a no-op in the Local fetcher so a ChecksumError surfaces without an on-disk <localpath>_bad-checksum_<sha> sibling. Link: https://bugzilla.yoctoproject.org/show_bug.cgi?id=9993 [YOCTO #9993] Signed-off-by: Jhonata Poma-Hansen <jhonata.poma@gmail.com> --- Changes in v2: - EDITME: describe what is new in this series revision. - EDITME: use bulletpoints and terse descriptions. - Link to v1: https://lore.kernel.org/r/20260430-b4-yocto-9993-v1-1-a54d842cd0fd@gmail.com --- lib/bb/fetch2/__init__.py | 36 ++++++++++++++++++------------------ lib/bb/fetch2/local.py | 18 ++++++++++++++++-- lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py index 52d5556d..5f4db608 100644 --- a/lib/bb/fetch2/__init__.py +++ b/lib/bb/fetch2/__init__.py @@ -738,7 +738,7 @@ def verify_donestamp(ud, d, origud=None): logger.warning("Checksum mismatch for local file %s\n" "Cleaning and trying again." % ud.localpath) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) return False @@ -771,7 +771,7 @@ def update_stamp(ud, d): logger.warning("Checksum mismatch for local file %s\n" "Cleaning and trying again." % ud.localpath) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) raise @@ -1067,20 +1067,6 @@ def build_mirroruris(origud, mirrors, ld): return uris, uds -def rename_bad_checksum(ud, suffix): - """ - Renames files to have suffix from parameter - """ - - if ud.localpath is None: - return - - new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) - bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) - if not bb.utils.movefile(ud.localpath, new_localpath): - bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) - - def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url @@ -1156,7 +1142,7 @@ def try_mirror_url(fetch, origud, ud, ld, check = False): logger.warning("Mirror checksum failure for url %s (original url: %s)\nCleaning and trying again." % (ud.url, origud.url)) logger.warning(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: @@ -1472,6 +1458,20 @@ class FetchMethod(object): """ return True + def rename_bad_checksum(self, ud, suffix): + """ + Rename ud.localpath with the given suffix on checksum mismatch. + Local overrides this to a no-op since its localpath points at the + user's source tree. + """ + if ud.localpath is None: + return + + new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) + bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) + if not bb.utils.movefile(ud.localpath, new_localpath): + bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) + def verify_donestamp(self, ud, d): """ Verify the donestamp file @@ -1937,7 +1937,7 @@ class Fetch(object): logger.warning("Checksum failure encountered with download of %s - will attempt other sources if available" % u) logger.debug(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: diff --git a/lib/bb/fetch2/local.py b/lib/bb/fetch2/local.py index fda56a56..f9808e9b 100644 --- a/lib/bb/fetch2/local.py +++ b/lib/bb/fetch2/local.py @@ -17,7 +17,7 @@ import os import urllib.request, urllib.parse, urllib.error import bb import bb.utils -from bb.fetch2 import FetchMethod, FetchError, ParameterError +from bb.fetch2 import CHECKSUM_LIST, FetchMethod, FetchError, ParameterError from bb.fetch2 import logger class Local(FetchMethod): @@ -31,11 +31,25 @@ class Local(FetchMethod): # We don't set localfile as for this fetcher the file is already local! ud.basename = os.path.basename(ud.path) ud.basepath = ud.path - ud.needdonestamp = False + # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe + # ;sha256sum=) by leaving needdonestamp at FetchData's default so + # verify_checksum runs. + name = ud.parm.get("name") + ud.needdonestamp = any( + (name and "%s.%ssum" % (name, a) in ud.parm) + or "%ssum" % a in ud.parm + for a in CHECKSUM_LIST + ) if "*" in ud.path: raise bb.fetch2.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url) return + def rename_bad_checksum(self, ud, suffix): + # file:// urls point at the user's source tree (recipe-shipped files + # under FILESPATH or an absolute path the user passed in); skip the + # rename so a ChecksumError surfaces without mutating their files. + return + def localpath(self, urldata, d): """ Return the local filename of a given url assuming a successful fetch. diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index 077f741e..93e889fd 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -813,6 +813,36 @@ class FetcherLocalTest(FetcherTest): tree = self.fetchUnpack(['file://archive.tar.bz2;subdir=bar;striplevel=1']) self.assertEqual(tree, ['bar/c', 'bar/d', 'bar/subdir/e']) + def test_local_checksum_match(self): + # Correct sha256 must run verify_checksum to completion; donestamp + # lands in DL_DIR. + import hashlib + content = b"file:// checksum match test\n" + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: + f.write(content) + good = hashlib.sha256(content).hexdigest() + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) + ud = fetcher.ud[fetcher.urls[0]] + self.assertTrue(ud.needdonestamp) + fetcher.download() + self.assertTrue(os.path.exists(ud.donestamp)) + + def test_local_checksum_mismatch(self): + # Bad sha256 raises ChecksumError without renaming the user's + # source file to the <localpath>_bad-checksum_<sha> sibling. + content = b"file:// checksum mismatch test\n" + srcpath = os.path.join(self.localsrcdir, 'sumfile') + with open(srcpath, 'wb') as f: + f.write(content) + bad = "0" * 64 + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d) + with self.assertRaises(bb.fetch2.FetchError): + fetcher.download() + self.assertTrue(os.path.exists(srcpath)) + with open(srcpath, 'rb') as f: + self.assertEqual(f.read(), content) + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad)) + def dummyGitTest(self, suffix): # Create dummy local Git repo src_dir = tempfile.mkdtemp(dir=self.tempdir, --- base-commit: 5d722b5d65e4eef7befe6376983385421e993f86 change-id: 20260429-b4-yocto-9993-cadd9cbed7ce Best regards, -- Jhonata Poma-Hansen <jhonata.poma@gmail.com> ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] fetch2/local: verify checksums for file:// urls 2026-05-11 14:58 ` [PATCH v2] " Jhonata Poma-Hansen @ 2026-05-11 15:01 ` Jhonata Poma-Hansen 2026-09-23 19:44 ` [PATCH v3] " Savvas Etairidis 1 sibling, 0 replies; 20+ messages in thread From: Jhonata Poma-Hansen @ 2026-05-11 15:01 UTC (permalink / raw) To: bitbake-devel; +Cc: paul, richard.purdie, peter.kjellerstedt, jhonata.poma Apologies, the "Changes in v2:" section in v2 ships with two literal b4 placeholder lines starting with "EDITME: ..." that I forgot to fill in before sending. Please disregard those two lines. The actual changes from v1, addressing Paul Barker's review (lore.kernel.org/r/162413fca4aa8fa53472aa6ba1e12b56c1a5c5c9.camel@pbarker.dev): - Commit body trimmed; UNINATIVE_URL trigger now leads paragraph 1 (Paul flagged this was missing from v1). - Dropped the long historical commit references (b8b14d975a25, 6424f4b7e9c1, 4b8de2e7d126) from the commit body. - rename_bad_checksum moved from a module-level helper in bb/fetch2/__init__.py onto the FetchMethod class; Local overrides it as a no-op so a ChecksumError surfaces without renaming files under the user's source tree. Four callsites updated to ud.method.rename_bad_checksum(...). No "ud.type == 'file'" special case in __init__.py. - Inline comment in local.py trimmed from ~10 lines to 3. - Tests reduced to two keepers in FetcherLocalTest: - test_local_checksum_match: simpler comment; same assertion. - test_local_checksum_mismatch: dropped the os.listdir loop, replaced with a direct assertFalse on the specific bad-checksum sibling path. Dropped: test_local_no_checksum_no_donestamp, test_local_checksum_mismatch_md5, test_local_checksum_named, test_local_checksum_directory_ignored (the directory-with-bad- checksum case Paul noted should raise; left out of this patch rather than widening scope). Thanks, Jhonata ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] fetch2/local: verify checksums for file:// urls 2026-05-11 14:58 ` [PATCH v2] " Jhonata Poma-Hansen 2026-05-11 15:01 ` Jhonata Poma-Hansen @ 2026-09-23 19:44 ` Savvas Etairidis 2026-09-24 6:36 ` [bitbake-devel] " Antonin Godard 1 sibling, 1 reply; 20+ messages in thread From: Savvas Etairidis @ 2026-09-23 19:44 UTC (permalink / raw) To: bitbake-devel; +Cc: jhonata.poma, paul, Savvas Etairidis Local.urldata_init unconditionally cleared ud.needdonestamp, which skipped the donestamp + verify_checksum cycle for every file:// url. A checksum supplied via a url parameter, such as UNINATIVE_CHECKSUM expanded into the uninative tarball's file:// url, was therefore never compared against the file's actual contents. Keep needdonestamp at its default when the url carries a checksum parameter, and move rename_bad_checksum onto FetchMethod so Local can override it as a no-op, avoiding a rename of the user's source file on mismatch. Based on a patch by Jhonata Poma-Hansen, adapted to this tree's Local.urldata_init(), which now uses ud.decodedurl in place of the ud.path referenced by the original patch. [YOCTO #9993] Signed-off-by: Savvas Etairidis <setairidis@gmail.com> --- bitbake/lib/bb/fetch2/__init__.py | 36 +++++++++++++++---------------- bitbake/lib/bb/fetch2/local.py | 18 ++++++++++++++-- bitbake/lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py index 8f0ed2b9e2..da96a4c399 100644 --- a/bitbake/lib/bb/fetch2/__init__.py +++ b/bitbake/lib/bb/fetch2/__init__.py @@ -701,7 +701,7 @@ def verify_donestamp(ud, d, origud=None): logger.warning("Checksum mismatch for local file %s\n" "Cleaning and trying again." % ud.localpath) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) return False @@ -734,7 +734,7 @@ def update_stamp(ud, d): logger.warning("Checksum mismatch for local file %s\n" "Cleaning and trying again." % ud.localpath) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) raise @@ -1029,20 +1029,6 @@ def build_mirroruris(origud, mirrors, ld): return uris, uds -def rename_bad_checksum(ud, suffix): - """ - Renames files to have suffix from parameter - """ - - if ud.localpath is None: - return - - new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) - bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) - if not bb.utils.movefile(ud.localpath, new_localpath): - bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) - - def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url @@ -1111,7 +1097,7 @@ def try_mirror_url(fetch, origud, ud, ld, check = False): logger.warning("Mirror checksum failure for url %s (original url: %s)\nCleaning and trying again." % (ud.url, origud.url)) logger.warning(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: @@ -1450,6 +1436,20 @@ class FetchMethod(object): """ return True + def rename_bad_checksum(self, ud, suffix): + """ + Rename ud.localpath with the given suffix on checksum mismatch. + Local overrides this to a no-op since its localpath points at the + user's source tree. + """ + if ud.localpath is None: + return + + new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) + bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) + if not bb.utils.movefile(ud.localpath, new_localpath): + bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) + def verify_donestamp(self, ud, d): """ Verify the donestamp file @@ -1887,7 +1887,7 @@ class Fetch(object): logger.warning("Checksum failure encountered with download of %s - will attempt other sources if available" % u) logger.debug(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: diff --git a/bitbake/lib/bb/fetch2/local.py b/bitbake/lib/bb/fetch2/local.py index 7d7668110e..43b8dd25ab 100644 --- a/bitbake/lib/bb/fetch2/local.py +++ b/bitbake/lib/bb/fetch2/local.py @@ -17,7 +17,7 @@ import os import urllib.request, urllib.parse, urllib.error import bb import bb.utils -from bb.fetch2 import FetchMethod, FetchError, ParameterError +from bb.fetch2 import CHECKSUM_LIST, FetchMethod, FetchError, ParameterError from bb.fetch2 import logger class Local(FetchMethod): @@ -32,11 +32,25 @@ class Local(FetchMethod): ud.decodedurl = urllib.parse.unquote(ud.url.split("://")[1].split(";")[0]) ud.basename = os.path.basename(ud.decodedurl) ud.basepath = ud.decodedurl - ud.needdonestamp = False + # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe + # ;sha256sum=) by leaving needdonestamp at FetchData's default so + # verify_checksum runs. + name = ud.parm.get("name") + ud.needdonestamp = any( + (name and "%s.%ssum" % (name, a) in ud.parm) + or "%ssum" % a in ud.parm + for a in CHECKSUM_LIST + ) if "*" in ud.decodedurl: raise bb.fetch2.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url) return + def rename_bad_checksum(self, ud, suffix): + # file:// urls point at the user's source tree (recipe-shipped files + # under FILESPATH or an absolute path the user passed in); skip the + # rename so a ChecksumError surfaces without mutating their files. + return + def localpath(self, urldata, d): """ Return the local filename of a given url assuming a successful fetch. diff --git a/bitbake/lib/bb/tests/fetch.py b/bitbake/lib/bb/tests/fetch.py index 68b2dbba96..4bc82af422 100644 --- a/bitbake/lib/bb/tests/fetch.py +++ b/bitbake/lib/bb/tests/fetch.py @@ -808,6 +808,36 @@ class FetcherLocalTest(FetcherTest): tree = self.fetchUnpack(['file://archive.tar.bz2;subdir=bar;striplevel=1']) self.assertEqual(tree, ['bar/c', 'bar/d', 'bar/subdir/e']) + def test_local_checksum_match(self): + # Correct sha256 must run verify_checksum to completion; donestamp + # lands in DL_DIR. + import hashlib + content = b"file:// checksum match test\n" + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: + f.write(content) + good = hashlib.sha256(content).hexdigest() + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) + ud = fetcher.ud[fetcher.urls[0]] + self.assertTrue(ud.needdonestamp) + fetcher.download() + self.assertTrue(os.path.exists(ud.donestamp)) + + def test_local_checksum_mismatch(self): + # Bad sha256 raises ChecksumError without renaming the user's + # source file to the <localpath>_bad-checksum_<sha> sibling. + content = b"file:// checksum mismatch test\n" + srcpath = os.path.join(self.localsrcdir, 'sumfile') + with open(srcpath, 'wb') as f: + f.write(content) + bad = "0" * 64 + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d) + with self.assertRaises(bb.fetch2.FetchError): + fetcher.download() + self.assertTrue(os.path.exists(srcpath)) + with open(srcpath, 'rb') as f: + self.assertEqual(f.read(), content) + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad)) + def dummyGitTest(self, suffix): # Create dummy local Git repo src_dir = tempfile.mkdtemp(dir=self.tempdir, -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [bitbake-devel] [PATCH v3] fetch2/local: verify checksums for file:// urls 2026-09-23 19:44 ` [PATCH v3] " Savvas Etairidis @ 2026-09-24 6:36 ` Antonin Godard 2026-09-24 20:45 ` [PATCH v4] fetch/local: " Savvas Etairidis 0 siblings, 1 reply; 20+ messages in thread From: Antonin Godard @ 2026-09-24 6:36 UTC (permalink / raw) To: setairidis, bitbake-devel; +Cc: jhonata.poma, paul Hi, On Wed Sep 23, 2026 at 9:44 PM CEST, Savvas Etairidis via lists.openembedded.org wrote: > Local.urldata_init unconditionally cleared ud.needdonestamp, which > skipped the donestamp + verify_checksum cycle for every file:// url. > A checksum supplied via a url parameter, such as UNINATIVE_CHECKSUM > expanded into the uninative tarball's file:// url, was therefore > never compared against the file's actual contents. > > Keep needdonestamp at its default when the url carries a checksum > parameter, and move rename_bad_checksum onto FetchMethod so Local can > override it as a no-op, avoiding a rename of the user's source file > on mismatch. > > Based on a patch by Jhonata Poma-Hansen, adapted to this tree's > Local.urldata_init(), which now uses ud.decodedurl in place of the > ud.path referenced by the original patch. This cannot be applied to the master branch cleanly. Can you rebase and re-send you patch? Antonin ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4] fetch/local: verify checksums for file:// urls 2026-09-24 6:36 ` [bitbake-devel] " Antonin Godard @ 2026-09-24 20:45 ` Savvas Etairidis 2026-09-28 9:30 ` Paul Barker 0 siblings, 1 reply; 20+ messages in thread From: Savvas Etairidis @ 2026-09-24 20:45 UTC (permalink / raw) To: bitbake-devel; +Cc: jhonata.poma, paul, Savvas Etairidis [YOCTO #9993] Local.urldata_init() always cleared needdonestamp, so a checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at a local file://) was never verified. Keep the donestamp when the url carries a checksum so verify_checksum runs, and make rename_bad_checksum() a FetchMethod method that Local overrides as a no-op, so a mismatch doesn't rename the user's source file. Based on a patch by Jhonata Poma-Hansen. Signed-off-by: Savvas Etairidis <setairidis@gmail.com> --- lib/bb/fetch/__init__.py | 36 ++++++++++++++++++------------------ lib/bb/fetch/local.py | 18 ++++++++++++++++-- lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py index 55ab710f3..a9f0add78 100644 --- a/lib/bb/fetch/__init__.py +++ b/lib/bb/fetch/__init__.py @@ -742,7 +742,7 @@ def verify_donestamp(ud, d, origud=None): logger.warning("Checksum mismatch for local file %s\n" "Cleaning and trying again." % ud.localpath) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) return False @@ -775,7 +775,7 @@ def update_stamp(ud, d): logger.warning("Checksum mismatch for local file %s\n" "Cleaning and trying again." % ud.localpath) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) raise @@ -1079,20 +1079,6 @@ def build_mirroruris(origud, mirrors, ld): return uris, uds -def rename_bad_checksum(ud, suffix): - """ - Renames files to have suffix from parameter - """ - - if ud.localpath is None: - return - - new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) - bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) - if not bb.utils.movefile(ud.localpath, new_localpath): - bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) - - def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url @@ -1163,7 +1149,7 @@ def try_mirror_url(fetch, origud, ud, ld, check = False): logger.warning("Mirror checksum failure for url %s (original url: %s)\nCleaning and trying again." % (ud.url, origud.url)) logger.warning(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: @@ -1479,6 +1465,20 @@ class FetchMethod(object): """ return True + def rename_bad_checksum(self, ud, suffix): + """ + Rename ud.localpath with the given suffix on checksum mismatch. + Local overrides this to a no-op since its localpath points at the + user's source tree. + """ + if ud.localpath is None: + return + + new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) + bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) + if not bb.utils.movefile(ud.localpath, new_localpath): + bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) + def verify_donestamp(self, ud, d): """ Verify the donestamp file @@ -1943,7 +1943,7 @@ class Fetch(object): logger.warning("Checksum failure encountered with download of %s - will attempt other sources if available" % u) logger.debug(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: diff --git a/lib/bb/fetch/local.py b/lib/bb/fetch/local.py index dfc5b564c..a92c0d267 100644 --- a/lib/bb/fetch/local.py +++ b/lib/bb/fetch/local.py @@ -17,7 +17,7 @@ import os import urllib.request, urllib.parse, urllib.error import bb import bb.utils -from bb.fetch import FetchMethod, FetchError, ParameterError +from bb.fetch import CHECKSUM_LIST, FetchMethod, FetchError, ParameterError from bb.fetch import logger class Local(FetchMethod): @@ -31,11 +31,25 @@ class Local(FetchMethod): # We don't set localfile as for this fetcher the file is already local! ud.basename = os.path.basename(ud.path) ud.basepath = ud.path - ud.needdonestamp = False + # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe + # ;sha256sum=) by leaving needdonestamp at FetchData's default so + # verify_checksum runs. + name = ud.parm.get("name") + ud.needdonestamp = any( + (name and "%s.%ssum" % (name, a) in ud.parm) + or "%ssum" % a in ud.parm + for a in CHECKSUM_LIST + ) if "*" in ud.path: raise bb.fetch.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url) return + def rename_bad_checksum(self, ud, suffix): + # file:// urls point at the user's source tree (recipe-shipped files + # under FILESPATH or an absolute path the user passed in); skip the + # rename so a ChecksumError surfaces without mutating their files. + return + def localpath(self, urldata, d): """ Return the local filename of a given url assuming a successful fetch. diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index 71b0a63fe..60d356fc6 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -876,6 +876,36 @@ class FetcherLocalTest(FetcherTest): with self.subTest(striplevel=repr(value)): self.assertInvalidStriplevel(value) + def test_local_checksum_match(self): + # Correct sha256 must run verify_checksum to completion; donestamp + # lands in DL_DIR. + import hashlib + content = b"file:// checksum match test\n" + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: + f.write(content) + good = hashlib.sha256(content).hexdigest() + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) + ud = fetcher.ud[fetcher.urls[0]] + self.assertTrue(ud.needdonestamp) + fetcher.download() + self.assertTrue(os.path.exists(ud.donestamp)) + + def test_local_checksum_mismatch(self): + # Bad sha256 raises ChecksumError without renaming the user's + # source file to the <localpath>_bad-checksum_<sha> sibling. + content = b"file:// checksum mismatch test\n" + srcpath = os.path.join(self.localsrcdir, 'sumfile') + with open(srcpath, 'wb') as f: + f.write(content) + bad = "0" * 64 + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d) + with self.assertRaises(bb.fetch.FetchError): + fetcher.download() + self.assertTrue(os.path.exists(srcpath)) + with open(srcpath, 'rb') as f: + self.assertEqual(f.read(), content) + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad)) + def dummyGitTest(self, suffix): # Create dummy local Git repo src_dir = tempfile.mkdtemp(dir=self.tempdir, -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4] fetch/local: verify checksums for file:// urls 2026-09-24 20:45 ` [PATCH v4] fetch/local: " Savvas Etairidis @ 2026-09-28 9:30 ` Paul Barker 2026-09-28 20:59 ` [PATCH v5] " Savvas Etairidis 0 siblings, 1 reply; 20+ messages in thread From: Paul Barker @ 2026-09-28 9:30 UTC (permalink / raw) To: Savvas Etairidis, bitbake-devel; +Cc: jhonata.poma On Thu, 2026-09-24 at 22:45 +0200, Savvas Etairidis wrote: > [YOCTO #9993] > > Local.urldata_init() always cleared needdonestamp, so a checksum given > in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at > a local file://) was never verified. Keep the donestamp when the url > carries a checksum so verify_checksum runs, and make > rename_bad_checksum() a FetchMethod method that Local overrides as a > no-op, so a mismatch doesn't rename the user's source file. > > Based on a patch by Jhonata Poma-Hansen. > > Signed-off-by: Savvas Etairidis <setairidis@gmail.com> Hi, Please include a list of changes since the previous version of the patch after the '---' separator. For an example see 'Changes from v2' in [1]. [1]: https://lore.kernel.org/all/20260607-license-update-v2-1-877d92a588e0@pbarker.dev/ Have you spoken to Jhonata about these patches? It's good practice to ask some questions and engage in discussion before taking over a patch series someone else was working on. Also, please ensure that you're following our policy on AI-assisted contributions [2] if these patches were generated with AI. [2]: https://docs.yoctoproject.org/contributor-guide/submit-changes.html#acceptance-of-ai-generated-code Best regards, -- Paul Barker ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5] fetch/local: verify checksums for file:// urls 2026-09-28 9:30 ` Paul Barker @ 2026-09-28 20:59 ` Savvas Etairidis 2026-09-28 21:02 ` Sawas Etairidis 2026-09-30 10:51 ` Paul Barker 0 siblings, 2 replies; 20+ messages in thread From: Savvas Etairidis @ 2026-09-28 20:59 UTC (permalink / raw) To: bitbake-devel, jhonata.poma; +Cc: Savvas Etairidis [YOCTO #9993] Local.urldata_init() always cleared needdonestamp, so a checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at a local file://) was never verified. Keep the donestamp when the url carries a checksum so verify_checksum runs, and make rename_bad_checksum() a FetchMethod method that Local overrides as a no-op, so a mismatch doesn't rename the user's source file. Based on a patch by Jhonata Poma-Hansen. Signed-off-by: Savvas Etairidis <setairidis@gmail.com> --- changes in v5: - Rebased on top of master, added changelog entry changes in v4: - Rebased on top of master changes in v3: - Cherry-picked the patch from Jhonata Poma-Hansen --- --- lib/bb/fetch/__init__.py | 36 ++++++++++++++++++------------------ lib/bb/fetch/local.py | 18 ++++++++++++++++-- lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py index 55ab710f3..a9f0add78 100644 --- a/lib/bb/fetch/__init__.py +++ b/lib/bb/fetch/__init__.py @@ -742,7 +742,7 @@ def verify_donestamp(ud, d, origud=None): logger.warning("Checksum mismatch for local file %s\n" "Cleaning and trying again." % ud.localpath) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) return False @@ -775,7 +775,7 @@ def update_stamp(ud, d): logger.warning("Checksum mismatch for local file %s\n" "Cleaning and trying again." % ud.localpath) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) raise @@ -1079,20 +1079,6 @@ def build_mirroruris(origud, mirrors, ld): return uris, uds -def rename_bad_checksum(ud, suffix): - """ - Renames files to have suffix from parameter - """ - - if ud.localpath is None: - return - - new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) - bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) - if not bb.utils.movefile(ud.localpath, new_localpath): - bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) - - def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url @@ -1163,7 +1149,7 @@ def try_mirror_url(fetch, origud, ud, ld, check = False): logger.warning("Mirror checksum failure for url %s (original url: %s)\nCleaning and trying again." % (ud.url, origud.url)) logger.warning(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: @@ -1479,6 +1465,20 @@ class FetchMethod(object): """ return True + def rename_bad_checksum(self, ud, suffix): + """ + Rename ud.localpath with the given suffix on checksum mismatch. + Local overrides this to a no-op since its localpath points at the + user's source tree. + """ + if ud.localpath is None: + return + + new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) + bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) + if not bb.utils.movefile(ud.localpath, new_localpath): + bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) + def verify_donestamp(self, ud, d): """ Verify the donestamp file @@ -1943,7 +1943,7 @@ class Fetch(object): logger.warning("Checksum failure encountered with download of %s - will attempt other sources if available" % u) logger.debug(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: diff --git a/lib/bb/fetch/local.py b/lib/bb/fetch/local.py index dfc5b564c..a92c0d267 100644 --- a/lib/bb/fetch/local.py +++ b/lib/bb/fetch/local.py @@ -17,7 +17,7 @@ import os import urllib.request, urllib.parse, urllib.error import bb import bb.utils -from bb.fetch import FetchMethod, FetchError, ParameterError +from bb.fetch import CHECKSUM_LIST, FetchMethod, FetchError, ParameterError from bb.fetch import logger class Local(FetchMethod): @@ -31,11 +31,25 @@ class Local(FetchMethod): # We don't set localfile as for this fetcher the file is already local! ud.basename = os.path.basename(ud.path) ud.basepath = ud.path - ud.needdonestamp = False + # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe + # ;sha256sum=) by leaving needdonestamp at FetchData's default so + # verify_checksum runs. + name = ud.parm.get("name") + ud.needdonestamp = any( + (name and "%s.%ssum" % (name, a) in ud.parm) + or "%ssum" % a in ud.parm + for a in CHECKSUM_LIST + ) if "*" in ud.path: raise bb.fetch.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url) return + def rename_bad_checksum(self, ud, suffix): + # file:// urls point at the user's source tree (recipe-shipped files + # under FILESPATH or an absolute path the user passed in); skip the + # rename so a ChecksumError surfaces without mutating their files. + return + def localpath(self, urldata, d): """ Return the local filename of a given url assuming a successful fetch. diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index 71b0a63fe..60d356fc6 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -876,6 +876,36 @@ class FetcherLocalTest(FetcherTest): with self.subTest(striplevel=repr(value)): self.assertInvalidStriplevel(value) + def test_local_checksum_match(self): + # Correct sha256 must run verify_checksum to completion; donestamp + # lands in DL_DIR. + import hashlib + content = b"file:// checksum match test\n" + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: + f.write(content) + good = hashlib.sha256(content).hexdigest() + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) + ud = fetcher.ud[fetcher.urls[0]] + self.assertTrue(ud.needdonestamp) + fetcher.download() + self.assertTrue(os.path.exists(ud.donestamp)) + + def test_local_checksum_mismatch(self): + # Bad sha256 raises ChecksumError without renaming the user's + # source file to the <localpath>_bad-checksum_<sha> sibling. + content = b"file:// checksum mismatch test\n" + srcpath = os.path.join(self.localsrcdir, 'sumfile') + with open(srcpath, 'wb') as f: + f.write(content) + bad = "0" * 64 + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d) + with self.assertRaises(bb.fetch.FetchError): + fetcher.download() + self.assertTrue(os.path.exists(srcpath)) + with open(srcpath, 'rb') as f: + self.assertEqual(f.read(), content) + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad)) + def dummyGitTest(self, suffix): # Create dummy local Git repo src_dir = tempfile.mkdtemp(dir=self.tempdir, -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v5] fetch/local: verify checksums for file:// urls 2026-09-28 20:59 ` [PATCH v5] " Savvas Etairidis @ 2026-09-28 21:02 ` Sawas Etairidis 2026-09-29 8:22 ` Jhonata Poma-Hansen 2026-09-30 10:51 ` Paul Barker 1 sibling, 1 reply; 20+ messages in thread From: Sawas Etairidis @ 2026-09-28 21:02 UTC (permalink / raw) To: bitbake-devel, jhonata.poma [-- Attachment #1: Type: text/plain, Size: 8986 bytes --] Hi, My bad I assumed that Jhonata stopped working on this based on the ticket comments. I send him a personal email about this. Regards, Savvas Στις Δευ 28 Σεπ 2026 στις 10:59 μ.μ., ο/η Savvas Etairidis < setairidis@gmail.com> έγραψε: > [YOCTO #9993] > > Local.urldata_init() always cleared needdonestamp, so a checksum given > in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at > a local file://) was never verified. Keep the donestamp when the url > carries a checksum so verify_checksum runs, and make > rename_bad_checksum() a FetchMethod method that Local overrides as a > no-op, so a mismatch doesn't rename the user's source file. > > Based on a patch by Jhonata Poma-Hansen. > > Signed-off-by: Savvas Etairidis <setairidis@gmail.com> > > --- > changes in v5: > - Rebased on top of master, added changelog entry > > changes in v4: > - Rebased on top of master > > changes in v3: > - Cherry-picked the patch from Jhonata Poma-Hansen > > --- > --- > lib/bb/fetch/__init__.py | 36 ++++++++++++++++++------------------ > lib/bb/fetch/local.py | 18 ++++++++++++++++-- > lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ > 3 files changed, 64 insertions(+), 20 deletions(-) > > diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py > index 55ab710f3..a9f0add78 100644 > --- a/lib/bb/fetch/__init__.py > +++ b/lib/bb/fetch/__init__.py > @@ -742,7 +742,7 @@ def verify_donestamp(ud, d, origud=None): > logger.warning("Checksum mismatch for local file %s\n" > "Cleaning and trying again." % ud.localpath) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, e.checksum) > bb.utils.remove(ud.donestamp) > return False > > @@ -775,7 +775,7 @@ def update_stamp(ud, d): > logger.warning("Checksum mismatch for local file %s\n" > "Cleaning and trying again." % ud.localpath) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, e.checksum) > bb.utils.remove(ud.donestamp) > raise > > @@ -1079,20 +1079,6 @@ def build_mirroruris(origud, mirrors, ld): > > return uris, uds > > -def rename_bad_checksum(ud, suffix): > - """ > - Renames files to have suffix from parameter > - """ > - > - if ud.localpath is None: > - return > - > - new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) > - bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) > - if not bb.utils.movefile(ud.localpath, new_localpath): > - bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch > to see why" % (ud.localpath, new_localpath)) > - > - > def try_mirror_url(fetch, origud, ud, ld, check = False): > # Return of None or a value means we're finished > # False means try another url > @@ -1163,7 +1149,7 @@ def try_mirror_url(fetch, origud, ud, ld, check = > False): > logger.warning("Mirror checksum failure for url %s (original > url: %s)\nCleaning and trying again." % (ud.url, origud.url)) > logger.warning(str(e)) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, e.checksum) > elif isinstance(e, NoChecksumError): > raise > else: > @@ -1479,6 +1465,20 @@ class FetchMethod(object): > """ > return True > > + def rename_bad_checksum(self, ud, suffix): > + """ > + Rename ud.localpath with the given suffix on checksum mismatch. > + Local overrides this to a no-op since its localpath points at the > + user's source tree. > + """ > + if ud.localpath is None: > + return > + > + new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) > + bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) > + if not bb.utils.movefile(ud.localpath, new_localpath): > + bb.warn("Renaming %s to %s failed, grep movefile in > log.do_fetch to see why" % (ud.localpath, new_localpath)) > + > def verify_donestamp(self, ud, d): > """ > Verify the donestamp file > @@ -1943,7 +1943,7 @@ class Fetch(object): > logger.warning("Checksum failure encountered > with download of %s - will attempt other sources if available" % u) > logger.debug(str(e)) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, > e.checksum) > elif isinstance(e, NoChecksumError): > raise > else: > diff --git a/lib/bb/fetch/local.py b/lib/bb/fetch/local.py > index dfc5b564c..a92c0d267 100644 > --- a/lib/bb/fetch/local.py > +++ b/lib/bb/fetch/local.py > @@ -17,7 +17,7 @@ import os > import urllib.request, urllib.parse, urllib.error > import bb > import bb.utils > -from bb.fetch import FetchMethod, FetchError, ParameterError > +from bb.fetch import CHECKSUM_LIST, FetchMethod, FetchError, > ParameterError > from bb.fetch import logger > > class Local(FetchMethod): > @@ -31,11 +31,25 @@ class Local(FetchMethod): > # We don't set localfile as for this fetcher the file is already > local! > ud.basename = os.path.basename(ud.path) > ud.basepath = ud.path > - ud.needdonestamp = False > + # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe > + # ;sha256sum=) by leaving needdonestamp at FetchData's default so > + # verify_checksum runs. > + name = ud.parm.get("name") > + ud.needdonestamp = any( > + (name and "%s.%ssum" % (name, a) in ud.parm) > + or "%ssum" % a in ud.parm > + for a in CHECKSUM_LIST > + ) > if "*" in ud.path: > raise bb.fetch.ParameterError("file:// urls using globbing > are no longer supported. Please place the files in a directory and > reference that instead.", ud.url) > return > > + def rename_bad_checksum(self, ud, suffix): > + # file:// urls point at the user's source tree (recipe-shipped > files > + # under FILESPATH or an absolute path the user passed in); skip > the > + # rename so a ChecksumError surfaces without mutating their files. > + return > + > def localpath(self, urldata, d): > """ > Return the local filename of a given url assuming a successful > fetch. > diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py > index 71b0a63fe..60d356fc6 100644 > --- a/lib/bb/tests/fetch.py > +++ b/lib/bb/tests/fetch.py > @@ -876,6 +876,36 @@ class FetcherLocalTest(FetcherTest): > with self.subTest(striplevel=repr(value)): > self.assertInvalidStriplevel(value) > > + def test_local_checksum_match(self): > + # Correct sha256 must run verify_checksum to completion; donestamp > + # lands in DL_DIR. > + import hashlib > + content = b"file:// checksum match test\n" > + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: > + f.write(content) > + good = hashlib.sha256(content).hexdigest() > + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], > self.d) > + ud = fetcher.ud[fetcher.urls[0]] > + self.assertTrue(ud.needdonestamp) > + fetcher.download() > + self.assertTrue(os.path.exists(ud.donestamp)) > + > + def test_local_checksum_mismatch(self): > + # Bad sha256 raises ChecksumError without renaming the user's > + # source file to the <localpath>_bad-checksum_<sha> sibling. > + content = b"file:// checksum mismatch test\n" > + srcpath = os.path.join(self.localsrcdir, 'sumfile') > + with open(srcpath, 'wb') as f: > + f.write(content) > + bad = "0" * 64 > + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], > self.d) > + with self.assertRaises(bb.fetch.FetchError): > + fetcher.download() > + self.assertTrue(os.path.exists(srcpath)) > + with open(srcpath, 'rb') as f: > + self.assertEqual(f.read(), content) > + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad)) > + > def dummyGitTest(self, suffix): > # Create dummy local Git repo > src_dir = tempfile.mkdtemp(dir=self.tempdir, > -- > 2.34.1 > > [-- Attachment #2: Type: text/html, Size: 11197 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5] fetch/local: verify checksums for file:// urls 2026-09-28 21:02 ` Sawas Etairidis @ 2026-09-29 8:22 ` Jhonata Poma-Hansen 0 siblings, 0 replies; 20+ messages in thread From: Jhonata Poma-Hansen @ 2026-09-29 8:22 UTC (permalink / raw) To: Sawas Etairidis; +Cc: bitbake-devel [-- Attachment #1: Type: text/plain, Size: 9517 bytes --] Hi, Thank you for taking over the patch series. It has been sitting on my to-do list for quite a while, it's great that somebody else can push it forward. Best regards, Jhonata Il giorno lun 28 set 2026 alle ore 23:02 Sawas Etairidis < setairidis@gmail.com> ha scritto: > Hi, > > My bad I assumed that Jhonata stopped working on this based on the ticket > comments. > I send him a personal email about this. > > Regards, > Savvas > > > Στις Δευ 28 Σεπ 2026 στις 10:59 μ.μ., ο/η Savvas Etairidis < > setairidis@gmail.com> έγραψε: > >> [YOCTO #9993] >> >> Local.urldata_init() always cleared needdonestamp, so a checksum given >> in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at >> a local file://) was never verified. Keep the donestamp when the url >> carries a checksum so verify_checksum runs, and make >> rename_bad_checksum() a FetchMethod method that Local overrides as a >> no-op, so a mismatch doesn't rename the user's source file. >> >> Based on a patch by Jhonata Poma-Hansen. >> >> Signed-off-by: Savvas Etairidis <setairidis@gmail.com> >> >> --- >> changes in v5: >> - Rebased on top of master, added changelog entry >> >> changes in v4: >> - Rebased on top of master >> >> changes in v3: >> - Cherry-picked the patch from Jhonata Poma-Hansen >> >> --- >> --- >> lib/bb/fetch/__init__.py | 36 ++++++++++++++++++------------------ >> lib/bb/fetch/local.py | 18 ++++++++++++++++-- >> lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ >> 3 files changed, 64 insertions(+), 20 deletions(-) >> >> diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py >> index 55ab710f3..a9f0add78 100644 >> --- a/lib/bb/fetch/__init__.py >> +++ b/lib/bb/fetch/__init__.py >> @@ -742,7 +742,7 @@ def verify_donestamp(ud, d, origud=None): >> logger.warning("Checksum mismatch for local file %s\n" >> "Cleaning and trying again." % ud.localpath) >> if os.path.exists(ud.localpath): >> - rename_bad_checksum(ud, e.checksum) >> + ud.method.rename_bad_checksum(ud, e.checksum) >> bb.utils.remove(ud.donestamp) >> return False >> >> @@ -775,7 +775,7 @@ def update_stamp(ud, d): >> logger.warning("Checksum mismatch for local file %s\n" >> "Cleaning and trying again." % ud.localpath) >> if os.path.exists(ud.localpath): >> - rename_bad_checksum(ud, e.checksum) >> + ud.method.rename_bad_checksum(ud, e.checksum) >> bb.utils.remove(ud.donestamp) >> raise >> >> @@ -1079,20 +1079,6 @@ def build_mirroruris(origud, mirrors, ld): >> >> return uris, uds >> >> -def rename_bad_checksum(ud, suffix): >> - """ >> - Renames files to have suffix from parameter >> - """ >> - >> - if ud.localpath is None: >> - return >> - >> - new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) >> - bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) >> - if not bb.utils.movefile(ud.localpath, new_localpath): >> - bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch >> to see why" % (ud.localpath, new_localpath)) >> - >> - >> def try_mirror_url(fetch, origud, ud, ld, check = False): >> # Return of None or a value means we're finished >> # False means try another url >> @@ -1163,7 +1149,7 @@ def try_mirror_url(fetch, origud, ud, ld, check = >> False): >> logger.warning("Mirror checksum failure for url %s (original >> url: %s)\nCleaning and trying again." % (ud.url, origud.url)) >> logger.warning(str(e)) >> if os.path.exists(ud.localpath): >> - rename_bad_checksum(ud, e.checksum) >> + ud.method.rename_bad_checksum(ud, e.checksum) >> elif isinstance(e, NoChecksumError): >> raise >> else: >> @@ -1479,6 +1465,20 @@ class FetchMethod(object): >> """ >> return True >> >> + def rename_bad_checksum(self, ud, suffix): >> + """ >> + Rename ud.localpath with the given suffix on checksum mismatch. >> + Local overrides this to a no-op since its localpath points at the >> + user's source tree. >> + """ >> + if ud.localpath is None: >> + return >> + >> + new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) >> + bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) >> + if not bb.utils.movefile(ud.localpath, new_localpath): >> + bb.warn("Renaming %s to %s failed, grep movefile in >> log.do_fetch to see why" % (ud.localpath, new_localpath)) >> + >> def verify_donestamp(self, ud, d): >> """ >> Verify the donestamp file >> @@ -1943,7 +1943,7 @@ class Fetch(object): >> logger.warning("Checksum failure encountered >> with download of %s - will attempt other sources if available" % u) >> logger.debug(str(e)) >> if os.path.exists(ud.localpath): >> - rename_bad_checksum(ud, e.checksum) >> + ud.method.rename_bad_checksum(ud, >> e.checksum) >> elif isinstance(e, NoChecksumError): >> raise >> else: >> diff --git a/lib/bb/fetch/local.py b/lib/bb/fetch/local.py >> index dfc5b564c..a92c0d267 100644 >> --- a/lib/bb/fetch/local.py >> +++ b/lib/bb/fetch/local.py >> @@ -17,7 +17,7 @@ import os >> import urllib.request, urllib.parse, urllib.error >> import bb >> import bb.utils >> -from bb.fetch import FetchMethod, FetchError, ParameterError >> +from bb.fetch import CHECKSUM_LIST, FetchMethod, FetchError, >> ParameterError >> from bb.fetch import logger >> >> class Local(FetchMethod): >> @@ -31,11 +31,25 @@ class Local(FetchMethod): >> # We don't set localfile as for this fetcher the file is already >> local! >> ud.basename = os.path.basename(ud.path) >> ud.basepath = ud.path >> - ud.needdonestamp = False >> + # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe >> + # ;sha256sum=) by leaving needdonestamp at FetchData's default so >> + # verify_checksum runs. >> + name = ud.parm.get("name") >> + ud.needdonestamp = any( >> + (name and "%s.%ssum" % (name, a) in ud.parm) >> + or "%ssum" % a in ud.parm >> + for a in CHECKSUM_LIST >> + ) >> if "*" in ud.path: >> raise bb.fetch.ParameterError("file:// urls using globbing >> are no longer supported. Please place the files in a directory and >> reference that instead.", ud.url) >> return >> >> + def rename_bad_checksum(self, ud, suffix): >> + # file:// urls point at the user's source tree (recipe-shipped >> files >> + # under FILESPATH or an absolute path the user passed in); skip >> the >> + # rename so a ChecksumError surfaces without mutating their >> files. >> + return >> + >> def localpath(self, urldata, d): >> """ >> Return the local filename of a given url assuming a successful >> fetch. >> diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py >> index 71b0a63fe..60d356fc6 100644 >> --- a/lib/bb/tests/fetch.py >> +++ b/lib/bb/tests/fetch.py >> @@ -876,6 +876,36 @@ class FetcherLocalTest(FetcherTest): >> with self.subTest(striplevel=repr(value)): >> self.assertInvalidStriplevel(value) >> >> + def test_local_checksum_match(self): >> + # Correct sha256 must run verify_checksum to completion; >> donestamp >> + # lands in DL_DIR. >> + import hashlib >> + content = b"file:// checksum match test\n" >> + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: >> + f.write(content) >> + good = hashlib.sha256(content).hexdigest() >> + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], >> self.d) >> + ud = fetcher.ud[fetcher.urls[0]] >> + self.assertTrue(ud.needdonestamp) >> + fetcher.download() >> + self.assertTrue(os.path.exists(ud.donestamp)) >> + >> + def test_local_checksum_mismatch(self): >> + # Bad sha256 raises ChecksumError without renaming the user's >> + # source file to the <localpath>_bad-checksum_<sha> sibling. >> + content = b"file:// checksum mismatch test\n" >> + srcpath = os.path.join(self.localsrcdir, 'sumfile') >> + with open(srcpath, 'wb') as f: >> + f.write(content) >> + bad = "0" * 64 >> + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], >> self.d) >> + with self.assertRaises(bb.fetch.FetchError): >> + fetcher.download() >> + self.assertTrue(os.path.exists(srcpath)) >> + with open(srcpath, 'rb') as f: >> + self.assertEqual(f.read(), content) >> + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + >> bad)) >> + >> def dummyGitTest(self, suffix): >> # Create dummy local Git repo >> src_dir = tempfile.mkdtemp(dir=self.tempdir, >> -- >> 2.34.1 >> >> [-- Attachment #2: Type: text/html, Size: 12006 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5] fetch/local: verify checksums for file:// urls 2026-09-28 20:59 ` [PATCH v5] " Savvas Etairidis 2026-09-28 21:02 ` Sawas Etairidis @ 2026-09-30 10:51 ` Paul Barker 2026-09-30 21:21 ` [PATCH v6] " Savvas Etairidis 1 sibling, 1 reply; 20+ messages in thread From: Paul Barker @ 2026-09-30 10:51 UTC (permalink / raw) To: Savvas Etairidis, bitbake-devel, jhonata.poma Hi Savvas, Some review comments inline. On Mon, 2026-09-28 at 22:59 +0200, Savvas Etairidis wrote: > [YOCTO #9993] > > Local.urldata_init() always cleared needdonestamp, so a checksum given > in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at > a local file://) was never verified. Keep the donestamp when the url > carries a checksum so verify_checksum runs, and make > rename_bad_checksum() a FetchMethod method that Local overrides as a > no-op, so a mismatch doesn't rename the user's source file. This commit message is incredibly terse, and it has changed from v3 without any explanation as to why. > > Based on a patch by Jhonata Poma-Hansen. > > Signed-off-by: Savvas Etairidis <setairidis@gmail.com> > > --- > changes in v5: > - Rebased on top of master, added changelog entry > > changes in v4: > - Rebased on top of master > > changes in v3: > - Cherry-picked the patch from Jhonata Poma-Hansen > > --- > --- > lib/bb/fetch/__init__.py | 36 ++++++++++++++++++------------------ > lib/bb/fetch/local.py | 18 ++++++++++++++++-- > lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ > 3 files changed, 64 insertions(+), 20 deletions(-) > > diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py > index 55ab710f3..a9f0add78 100644 > --- a/lib/bb/fetch/__init__.py > +++ b/lib/bb/fetch/__init__.py > @@ -742,7 +742,7 @@ def verify_donestamp(ud, d, origud=None): > logger.warning("Checksum mismatch for local file %s\n" > "Cleaning and trying again." % ud.localpath) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, e.checksum) > bb.utils.remove(ud.donestamp) > return False > > @@ -775,7 +775,7 @@ def update_stamp(ud, d): > logger.warning("Checksum mismatch for local file %s\n" > "Cleaning and trying again." % ud.localpath) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, e.checksum) > bb.utils.remove(ud.donestamp) > raise > > @@ -1079,20 +1079,6 @@ def build_mirroruris(origud, mirrors, ld): > > return uris, uds > > -def rename_bad_checksum(ud, suffix): > - """ > - Renames files to have suffix from parameter > - """ > - > - if ud.localpath is None: > - return > - > - new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) > - bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) > - if not bb.utils.movefile(ud.localpath, new_localpath): > - bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) > - > - > def try_mirror_url(fetch, origud, ud, ld, check = False): > # Return of None or a value means we're finished > # False means try another url > @@ -1163,7 +1149,7 @@ def try_mirror_url(fetch, origud, ud, ld, check = False): > logger.warning("Mirror checksum failure for url %s (original url: %s)\nCleaning and trying again." % (ud.url, origud.url)) > logger.warning(str(e)) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, e.checksum) > elif isinstance(e, NoChecksumError): > raise > else: > @@ -1479,6 +1465,20 @@ class FetchMethod(object): > """ > return True > > + def rename_bad_checksum(self, ud, suffix): > + """ > + Rename ud.localpath with the given suffix on checksum mismatch. > + Local overrides this to a no-op since its localpath points at the > + user's source tree. > + """ The docstring for a method explaining how another class overrides this function is an AI-generated code smell. It makes no sense to document this here. > + if ud.localpath is None: > + return > + > + new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) > + bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) > + if not bb.utils.movefile(ud.localpath, new_localpath): > + bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) > + > def verify_donestamp(self, ud, d): > """ > Verify the donestamp file > @@ -1943,7 +1943,7 @@ class Fetch(object): > logger.warning("Checksum failure encountered with download of %s - will attempt other sources if available" % u) > logger.debug(str(e)) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, e.checksum) > elif isinstance(e, NoChecksumError): > raise > else: > diff --git a/lib/bb/fetch/local.py b/lib/bb/fetch/local.py > index dfc5b564c..a92c0d267 100644 > --- a/lib/bb/fetch/local.py > +++ b/lib/bb/fetch/local.py > @@ -17,7 +17,7 @@ import os > import urllib.request, urllib.parse, urllib.error > import bb > import bb.utils > -from bb.fetch import FetchMethod, FetchError, ParameterError > +from bb.fetch import CHECKSUM_LIST, FetchMethod, FetchError, ParameterError > from bb.fetch import logger > > class Local(FetchMethod): > @@ -31,11 +31,25 @@ class Local(FetchMethod): > # We don't set localfile as for this fetcher the file is already local! > ud.basename = os.path.basename(ud.path) > ud.basepath = ud.path > - ud.needdonestamp = False > + # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe > + # ;sha256sum=) by leaving needdonestamp at FetchData's default so > + # verify_checksum runs. > + name = ud.parm.get("name") > + ud.needdonestamp = any( > + (name and "%s.%ssum" % (name, a) in ud.parm) > + or "%ssum" % a in ud.parm > + for a in CHECKSUM_LIST > + ) This expression is a bit confusing, can we instead check for the presence of %s_name attributes set by configure_checksum() calls in FetchData.__init__()? The comment can also be simpler, perhaps something like "We need a done stamp file only if a checksum is provided". We should also separate things with a blank line if we need a comment before the ud.needdonestamp assignment. > if "*" in ud.path: > raise bb.fetch.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url) > return > > + def rename_bad_checksum(self, ud, suffix): > + # file:// urls point at the user's source tree (recipe-shipped files > + # under FILESPATH or an absolute path the user passed in); skip the > + # rename so a ChecksumError surfaces without mutating their files. > + return Again the comment can be simpler, perhaps "Don't rename the user's files". > + > def localpath(self, urldata, d): > """ > Return the local filename of a given url assuming a successful fetch. > diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py > index 71b0a63fe..60d356fc6 100644 > --- a/lib/bb/tests/fetch.py > +++ b/lib/bb/tests/fetch.py > @@ -876,6 +876,36 @@ class FetcherLocalTest(FetcherTest): > with self.subTest(striplevel=repr(value)): > self.assertInvalidStriplevel(value) > > + def test_local_checksum_match(self): > + # Correct sha256 must run verify_checksum to completion; donestamp > + # lands in DL_DIR. This comment is also an AI-generated code smell. Please ensure that you're following our AI-assisted contribution policy [1]. If the original patch from Jhonata was AI-generated then you need to document this. [1]: https://docs.yoctoproject.org/contributor-guide/submit-changes.html#acceptance-of-ai-generated-code > + import hashlib > + content = b"file:// checksum match test\n" > + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: > + f.write(content) > + good = hashlib.sha256(content).hexdigest() > + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) > + ud = fetcher.ud[fetcher.urls[0]] > + self.assertTrue(ud.needdonestamp) > + fetcher.download() > + self.assertTrue(os.path.exists(ud.donestamp)) We shouldn't need either of the assertTrue() calls here. The checksum mismatch case will catch failure to check the checksum, and fetcher.download() will raise an error if it fails. > + > + def test_local_checksum_mismatch(self): > + # Bad sha256 raises ChecksumError without renaming the user's > + # source file to the <localpath>_bad-checksum_<sha> sibling. > + content = b"file:// checksum mismatch test\n" > + srcpath = os.path.join(self.localsrcdir, 'sumfile') > + with open(srcpath, 'wb') as f: > + f.write(content) > + bad = "0" * 64 > + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d) > + with self.assertRaises(bb.fetch.FetchError): > + fetcher.download() > + self.assertTrue(os.path.exists(srcpath)) > + with open(srcpath, 'rb') as f: > + self.assertEqual(f.read(), content) Why are we checking the content of the file? We're just looking for a rename. > + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad)) > + > def dummyGitTest(self, suffix): > # Create dummy local Git repo > src_dir = tempfile.mkdtemp(dir=self.tempdir, Best regards, -- Paul Barker ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v6] fetch/local: verify checksums for file:// urls 2026-09-30 10:51 ` Paul Barker @ 2026-09-30 21:21 ` Savvas Etairidis 2026-10-01 8:32 ` Paul Barker 0 siblings, 1 reply; 20+ messages in thread From: Savvas Etairidis @ 2026-09-30 21:21 UTC (permalink / raw) To: bitbake-devel; +Cc: Jhonata Poma-Hansen, Paul Barker, Savvas Etairidis [YOCTO #9993] A checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at a local file://) was never verified, because Local disables the donestamp and so verify_checksum() was never called. Override verify_donestamp() in Local to check the file against those checksums on every fetch, via a new FetchData.verify_checksum_if_expected() helper. A mismatch raises ChecksumError before the rename/mirror handling, so the user's file is never renamed. Hashing directly rather than enabling the donestamp avoids trusting a cached checksum for a file edited with an old mtime, and avoids stamp and lock paths in DL_DIR derived from the url path colliding with other recipes or real downloads. The file is re-hashed on every fetch; this is a known cost, but only for file:// urls that carry a checksum, which are typically small. Based on a patch by Jhonata Poma-Hansen. AI-Generated: Uses Claude Code (Claude Opus 5.5) Signed-off-by: Savvas Etairidis <setairidis@gmail.com> --- changes in v6: - Rebased on top of master - Fixed issues reported by reviewer - Check the file directly in Local.verify_donestamp(), via a new FetchData.verify_checksum_if_expected() helper, instead of enabling the donestamp. Fixes stale checksums for files edited with an old mtime and DL_DIR stamp/lock path collisions - Drop the rename_bad_checksum() refactor, no longer needed - Document the cost of re-hashing on every fetch - Tests: use a correct md5sum and a wrong sha256sum in the mismatch test, add a test for a file edited with an old mtime, and check that nothing is written to DL_DIR - Tested with BB_SKIP_NETTESTS=yes bin/bitbake-selftest bb.tests.fetch changes in v5: - Rebased on top of master, added changelog entry changes in v4: - Rebased on top of master changes in v3: - Cherry-picked the patch from Jhonata Poma-Hansen --- --- lib/bb/fetch/__init__.py | 13 ++++++++++++ lib/bb/fetch/local.py | 16 ++++++++++++++ lib/bb/tests/fetch.py | 46 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py index fb6278439..418a031ec 100644 --- a/lib/bb/fetch/__init__.py +++ b/lib/bb/fetch/__init__.py @@ -1413,6 +1413,19 @@ class FetchData(object): self.donestamp = basepath + '.done' self.lockfile = basepath + '.lock' + def verify_checksum_if_expected(self, d): + """ + Verify localpath against the checksums given in the url, if any. + """ + expected = False + for checksum_id in CHECKSUM_LIST: + if getattr(self, "%s_expected" % checksum_id): + expected = True + break + + if expected: + verify_checksum(self, d) + def setup_revisions(self, d): self.revision = srcrev_internal_helper(self, d, self.name) diff --git a/lib/bb/fetch/local.py b/lib/bb/fetch/local.py index dfc5b564c..d7551221b 100644 --- a/lib/bb/fetch/local.py +++ b/lib/bb/fetch/local.py @@ -36,6 +36,22 @@ class Local(FetchMethod): raise bb.fetch.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url) return + def verify_donestamp(self, ud, d): + # file:// urls have no donestamp; verify any checksum given in the url + # against the file itself on every fetch, since it may be edited in + # place without its mtime changing. A missing file is reported by + # download() with the searched paths, so there's nothing to check here. + if not os.path.exists(ud.localpath): + return True + # Re-hashing on every run is a known cost: verify_checksum() computes + # every algorithm in CHECKSUM_LIST, and Fetch.download() calls + # verify_donestamp() twice, so a 100MB file adds ~1s per fetch. + # This only affects file:// urls that carry a checksum, which are + # typically small (e.g. uninative tarballs), so correctness is + # preferred over caching. + ud.verify_checksum_if_expected(d) + return True + def localpath(self, urldata, d): """ Return the local filename of a given url assuming a successful fetch. diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index 71b0a63fe..2bec8a66a 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -876,6 +876,52 @@ class FetcherLocalTest(FetcherTest): with self.subTest(striplevel=repr(value)): self.assertInvalidStriplevel(value) + def test_local_checksum_match(self): + import hashlib + content = b"file:// checksum match test\n" + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: + f.write(content) + good = hashlib.sha256(content).hexdigest() + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) + fetcher.download() + # No stamp or lock files should be created in DL_DIR for file:// urls + self.assertEqual(os.listdir(self.dldir), []) + + def test_local_checksum_modified_with_old_mtime(self): + import hashlib + srcpath = os.path.join(self.localsrcdir, 'sumfile') + with open(srcpath, 'wb') as f: + f.write(b"file:// checksum original content\n") + good = hashlib.sha256(b"file:// checksum original content\n").hexdigest() + url = 'file://sumfile;sha256sum=' + good + bb.fetch.Fetch([url], self.d).download() + + # Replace the content but keep an old mtime, as cp -p or rsync -a would + old = os.stat(srcpath).st_mtime - 3600 + with open(srcpath, 'wb') as f: + f.write(b"file:// checksum modified content\n") + os.utime(srcpath, (old, old)) + with self.assertRaises(bb.fetch.ChecksumError): + bb.fetch.Fetch([url], self.d).download() + self.assertTrue(os.path.exists(srcpath)) + + def test_local_checksum_mismatch(self): + import hashlib + content = b"file:// checksum mismatch test\n" + srcpath = os.path.join(self.localsrcdir, 'sumfile') + with open(srcpath, 'wb') as f: + f.write(content) + md5 = hashlib.md5(content).hexdigest() + # verify_checksum_if_expected() stops at the first checksum it finds + # (md5); a wrong checksum later in CHECKSUM_LIST must still be caught + url = 'file://sumfile;md5sum=%s;sha256sum=%s' % (md5, "0" * 64) + fetcher = bb.fetch.Fetch([url], self.d) + with self.assertRaises(bb.fetch.ChecksumError) as cm: + fetcher.download() + self.assertIn("has sha256 checksum", str(cm.exception)) + # The user's file must not be renamed to *_bad-checksum_* + self.assertTrue(os.path.exists(srcpath)) + def dummyGitTest(self, suffix): # Create dummy local Git repo src_dir = tempfile.mkdtemp(dir=self.tempdir, -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v6] fetch/local: verify checksums for file:// urls 2026-09-30 21:21 ` [PATCH v6] " Savvas Etairidis @ 2026-10-01 8:32 ` Paul Barker 2026-10-01 20:53 ` [PATCH v7] " Savvas Etairidis 0 siblings, 1 reply; 20+ messages in thread From: Paul Barker @ 2026-10-01 8:32 UTC (permalink / raw) To: Savvas Etairidis, bitbake-devel; +Cc: Jhonata Poma-Hansen On Wed, 2026-09-30 at 23:21 +0200, Savvas Etairidis wrote: > [YOCTO #9993] > > A checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when > UNINATIVE_URL points at a local file://) was never verified, because > Local disables the donestamp and so verify_checksum() was never called. > Override verify_donestamp() in Local to check the file against those > checksums on every fetch, via a new > FetchData.verify_checksum_if_expected() helper. A mismatch raises > ChecksumError before the rename/mirror handling, so the user's file is > never renamed. > > Hashing directly rather than enabling the donestamp avoids trusting a > cached checksum for a file edited with an old mtime, and avoids stamp > and lock paths in DL_DIR derived from the url path colliding with other > recipes or real downloads. > The file is re-hashed on every fetch; this is a known cost, but only > for file:// urls that carry a checksum, which are typically small. We shouldn't assume that anything coming from a file:// URL is "typically small", or that it is on fast local storage (it could be on a HDD array accessed over NFS). > > Based on a patch by Jhonata Poma-Hansen. > > AI-Generated: Uses Claude Code (Claude Opus 5.5) > Signed-off-by: Savvas Etairidis <setairidis@gmail.com> > > --- > changes in v6: > - Rebased on top of master You don't need to mention this if it didn't result in actual change. > - Fixed issues reported by reviewer Such as? This is a completely different approach so it's useful to know which bits of feedback were actually relevant and which were made obsolete by the change in approach. > - Check the file directly in Local.verify_donestamp(), via a new > FetchData.verify_checksum_if_expected() helper, instead of enabling > the donestamp. Fixes stale checksums for files edited with an old > mtime and DL_DIR stamp/lock path collisions What does "edited with an old mtime" mean? > - Drop the rename_bad_checksum() refactor, no longer needed > - Document the cost of re-hashing on every fetch > - Tests: use a correct md5sum and a wrong sha256sum in the mismatch > test, add a test for a file edited with an old mtime, and check that > nothing is written to DL_DIR Why complicate this by using different hash algorithms for the different tests? > - Tested with BB_SKIP_NETTESTS=yes bin/bitbake-selftest bb.tests.fetch > > changes in v5: > - Rebased on top of master, added changelog entry > > changes in v4: > - Rebased on top of master > > changes in v3: > - Cherry-picked the patch from Jhonata Poma-Hansen Best regards, -- Paul Barker ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v7] fetch/local: verify checksums for file:// urls 2026-10-01 8:32 ` Paul Barker @ 2026-10-01 20:53 ` Savvas Etairidis 2026-10-02 8:50 ` Paul Barker 0 siblings, 1 reply; 20+ messages in thread From: Savvas Etairidis @ 2026-10-01 20:53 UTC (permalink / raw) To: bitbake-devel; +Cc: Jhonata Poma-Hansen, Paul Barker, Savvas Etairidis [YOCTO #9993] A checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at a local file://) was never verified, because Local disables the donestamp and so verify_checksum() was never called. Override verify_donestamp() in Local to check the file against those checksums. A mismatch raises ChecksumError before the rename/mirror handling, so the user's file is never renamed. Based on a patch by Jhonata Poma-Hansen. AI-Generated: Uses Claude Code (Claude Opus 5.5) Signed-off-by: Savvas Etairidis <setairidis@gmail.com> --- changes in v7: - We shouldn't assume that anything coming from a file:// URL is "typically small", or that it is on fast local storage (it could be on a HDD array accessed over NFS). * Rework the code to avoid re-hashing the file on every fetch, and instead only re-hash it when the checksum is actually expected to be verified. - You don't need to mention this if it didn't result in actual change. * Wont do in the future. - Such as? This is a completely different approach so it's useful to know which bits of feedback were actually relevant and which were made obsolete by the change in approach. * Hoped I did better this time. - What does "edited with an old mtime" mean? * In that test the file attributes are modified to have an old mtime, which is a corner case that was not handled correctly before. This test was removed in v6 because the new approach doesn't have that issue anymore. - Why complicate this by using different hash algorithms for the different tests? The test wants to cover the case where we have a valid md5sum but an invalid sha256sum or a invalid md5sum but a valid sha256sum. - Added a test to covern new functionality. changes in v6: - Rebased on top of master - Fixed issues reported by reviewer - Check the file directly in Local.verify_donestamp(), via a new FetchData.verify_checksum_if_expected() helper, instead of enabling the donestamp. Fixes stale checksums for files edited with an old mtime and DL_DIR stamp/lock path collisions - Drop the rename_bad_checksum() refactor, no longer needed - Document the cost of re-hashing on every fetch - Tests: use a correct md5sum and a wrong sha256sum in the mismatch test, add a test for a file edited with an old mtime, and check that nothing is written to DL_DIR - Tested with BB_SKIP_NETTESTS=yes bin/bitbake-selftest bb.tests.fetch changes in v5: - Rebased on top of master, added changelog entry changes in v4: - Rebased on top of master changes in v3: - Cherry-picked the patch from Jhonata Poma-Hansen --- --- lib/bb/fetch/__init__.py | 9 ++++ lib/bb/fetch/local.py | 82 +++++++++++++++++++++++++++++ lib/bb/tests/fetch.py | 109 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 200 insertions(+) diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py index fb6278439..d74c20df4 100644 --- a/lib/bb/fetch/__init__.py +++ b/lib/bb/fetch/__init__.py @@ -1413,6 +1413,15 @@ class FetchData(object): self.donestamp = basepath + '.done' self.lockfile = basepath + '.lock' + def checksum_expected(self): + """ + Is any checksum given for this url? + """ + for checksum_id in CHECKSUM_LIST: + if getattr(self, "%s_expected" % checksum_id): + return True + return False + def setup_revisions(self, d): self.revision = srcrev_internal_helper(self, d, self.name) diff --git a/lib/bb/fetch/local.py b/lib/bb/fetch/local.py index dfc5b564c..ca9f91497 100644 --- a/lib/bb/fetch/local.py +++ b/lib/bb/fetch/local.py @@ -14,6 +14,8 @@ BitBake build tools. # import os +import hashlib +import json import urllib.request, urllib.parse, urllib.error import bb import bb.utils @@ -36,6 +38,82 @@ class Local(FetchMethod): raise bb.fetch.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url) return + def checksum_stamp(self, ud, d): + """ + Return the stamp file caching the checksums of ud.localpath, or None. + """ + dldir = d.getVar("DL_DIR") + if not dldir or not ud.localpath: + return None + path = os.path.abspath(ud.localpath) + return os.path.join(dldir, "local-checksums", + hashlib.sha256(path.encode("utf-8")).hexdigest()) + + def checksum_filestat(self, st): + """ + Return a list of the file's stat information that is used to detect changes. + """ + return [st.st_ino, st.st_size, st.st_mtime_ns, st.st_ctime_ns] + + def read_checksum_stamp(self, ud, d, st): + """ + Return the checksums cached in the stamp file for ud.localpath, or {} + if there are none or the file has changed since they were computed. + """ + stamp = self.checksum_stamp(ud, d) + if not stamp: + return {} + try: + with open(stamp) as f: + cached = json.load(f) + if cached["stat"] == self.checksum_filestat(st) and isinstance(cached["checksums"], dict): + return cached["checksums"] + except (OSError, ValueError, KeyError, TypeError): + pass + return {} + + def write_checksum_stamp(self, ud, d, st, checksums): + """ + Cache the checksums computed for ud.localpath, which had the stat + result st before it was hashed, in the stamp file. + """ + stamp = self.checksum_stamp(ud, d) + if not stamp: + return + + # Written via rename as no lock is held for file:// urls. Errors + # aren't fatal, the file is just hashed again next time. + tmpstamp = "%s.%d" % (stamp, os.getpid()) + try: + bb.utils.mkdirhier(os.path.dirname(stamp)) + with open(tmpstamp, "w") as f: + json.dump({"path": ud.localpath, "stat": self.checksum_filestat(st), "checksums": checksums}, f) + os.rename(tmpstamp, stamp) + except OSError: + try: + os.unlink(tmpstamp) + except OSError: + pass + + def verify_donestamp(self, ud, d): + # file:// urls have no donestamp; verify any checksum given in the url + # against the file itself. A missing file is reported by download() + # with the searched paths, so there's nothing to check here. A mismatch + # raises ChecksumError rather than returning False, as there's nothing + # to re-download. + if not ud.checksum_expected(): + return True + try: + st = os.stat(ud.localpath) + except OSError: + return True + + precomputed = self.read_checksum_stamp(ud, d, st) + checksums = bb.fetch.verify_checksum(ud, d, precomputed) + if checksums and checksums != precomputed: + self.write_checksum_stamp(ud, d, st, checksums) + return True + def localpath(self, urldata, d): """ Return the local filename of a given url assuming a successful fetch. @@ -87,5 +165,9 @@ class Local(FetchMethod): return False def clean(self, urldata, d): + # The file is the user's; only remove our cached checksums for it + stamp = self.checksum_stamp(urldata, d) + if stamp: + bb.utils.remove(stamp) return diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index 71b0a63fe..cbb47849c 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -21,6 +21,7 @@ import subprocess import json import tarfile import threading +import time from bb.fetch import URI import bb import bb.utils @@ -876,6 +877,114 @@ class FetcherLocalTest(FetcherTest): with self.subTest(striplevel=repr(value)): self.assertInvalidStriplevel(value) + def writeSumfile(self, content, dirname=None): + path = os.path.join(dirname or self.localsrcdir, 'sumfile') + with open(path, 'wb') as f: + f.write(content) + return path, hashlib.sha256(content).hexdigest() + + def localChecksumStamps(self): + stampdir = os.path.join(self.dldir, 'local-checksums') + return os.listdir(stampdir) if os.path.exists(stampdir) else [] + + def test_local_checksum_match(self): + srcpath, good = self.writeSumfile(b"file:// checksum match test\n") + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) + fetcher.download() + # Only the checksum stamp should be created in DL_DIR + self.assertEqual(os.listdir(self.dldir), ['local-checksums']) + stamps = self.localChecksumStamps() + self.assertEqual(len(stamps), 1) + # The stamp must follow the umask, so a shared DL_DIR can re-use it + umask = os.umask(0) + os.umask(umask) + mode = os.stat(os.path.join(self.dldir, 'local-checksums', stamps[0])).st_mode + self.assertEqual(mode & 0o777, 0o666 & ~umask) + # Cleaning removes the stamp but not the user's file + fetcher.clean() + self.assertEqual(self.localChecksumStamps(), []) + self.assertTrue(os.path.exists(srcpath)) + + def test_local_checksum_no_checksum_no_stamp(self): + self.writeSumfile(b"file:// no checksum test\n") + bb.fetch.Fetch(['file://sumfile'], self.d).download() + self.assertEqual(os.listdir(self.dldir), []) + + def test_local_checksum_stamp_write_failure(self): + # A failure to write the stamp isn't fatal and leaves nothing behind + srcpath, good = self.writeSumfile(b"file:// checksum stamp failure test\n") + with unittest.mock.patch('os.rename', side_effect=OSError("rename failed")): + bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d).download() + self.assertEqual(self.localChecksumStamps(), []) + + def test_local_checksum_unreachable_file(self): + # A path below a regular file can't be stat()ed; this should be + # reported as a missing file, not as an OSError + srcpath, good = self.writeSumfile(b"file:// checksum unreachable test\n") + fetcher = bb.fetch.Fetch(['file://%s/sub;sha256sum=%s' % (srcpath, good)], self.d) + with self.assertRaises(bb.fetch.FetchError): + fetcher.download() + + def test_local_checksum_unchanged_not_rehashed(self): + stampdir, good = self.writeSumfile(b"file:// checksum rehash test\n") + url = 'file://sumfile;sha256sum=' + good + with unittest.mock.patch('bb.utils.sha256_file', + wraps=bb.utils.sha256_file) as sha256_file: + bb.fetch.Fetch([url], self.d).download() + self.assertEqual(sha256_file.call_count, 1) + # A later fetch re-uses the checksums stored in the stamp + bb.fetch.Fetch([url], self.d).download() + self.assertEqual(sha256_file.call_count, 1) + # A wrong checksum in the url is still caught without re-hashing + with self.assertRaises(bb.fetch.ChecksumError): + bb.fetch.Fetch(['file://sumfile;sha256sum=' + "0" * 64], self.d).download() + self.assertEqual(sha256_file.call_count, 1) + + def test_local_checksum_modified_keeping_mtime(self): + srcpath, good = self.writeSumfile(b"file:// checksum original content\n") + url = 'file://sumfile;sha256sum=' + good + bb.fetch.Fetch([url], self.d).download() + + # Replace the content in place with the same size and restore the + # exact original mtime, as rsync -a or touch -r could; only ctime + # changes. Filesystem timestamps may only advance once per timer + # tick, so wait for longer than that first. + st = os.stat(srcpath) + time.sleep(0.05) + with open(srcpath, 'wb') as f: + f.write(b"file:// checksum modified content\n") + os.utime(srcpath, ns=(st.st_atime_ns, st.st_mtime_ns)) + with self.assertRaises(bb.fetch.ChecksumError): + bb.fetch.Fetch([url], self.d).download() + self.assertTrue(os.path.exists(srcpath)) + + def test_local_checksum_mismatch(self): + content = b"file:// checksum mismatch test\n" + srcpath, _ = self.writeSumfile(content) + md5 = hashlib.md5(content).hexdigest() + # A correct checksum must not hide a wrong one + url = 'file://sumfile;md5sum=%s;sha256sum=%s' % (md5, "0" * 64) + fetcher = bb.fetch.Fetch([url], self.d) + with self.assertRaises(bb.fetch.ChecksumError) as cm: + fetcher.download() + self.assertIn("has sha256 checksum", str(cm.exception)) + # The user's file must not be renamed to *_bad-checksum_* + self.assertTrue(os.path.exists(srcpath)) + self.assertEqual(self.localChecksumStamps(), []) + + def test_local_checksum_same_name_different_dirs(self): + # Files with the same name must not share a stamp + sums = {} + for name in ('a', 'b'): + dirname = os.path.join(self.localsrcdir, 'sumdir-' + name) + os.mkdir(dirname) + sums[name] = self.writeSumfile(b"file:// checksum dir %s\n" % name.encode(), dirname) + for srcpath, good in sums.values(): + bb.fetch.Fetch(['file://%s;sha256sum=%s' % (srcpath, good)], self.d).download() + self.assertEqual(len(self.localChecksumStamps()), 2) + with self.assertRaises(bb.fetch.ChecksumError): + bb.fetch.Fetch(['file://%s;sha256sum=%s' % (sums['a'][0], sums['b'][1])], self.d).download() + def dummyGitTest(self, suffix): # Create dummy local Git repo src_dir = tempfile.mkdtemp(dir=self.tempdir, -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v7] fetch/local: verify checksums for file:// urls 2026-10-01 20:53 ` [PATCH v7] " Savvas Etairidis @ 2026-10-02 8:50 ` Paul Barker 2026-10-02 20:48 ` Sawas Etairidis 0 siblings, 1 reply; 20+ messages in thread From: Paul Barker @ 2026-10-02 8:50 UTC (permalink / raw) To: Savvas Etairidis, bitbake-devel; +Cc: Jhonata Poma-Hansen On Thu, 2026-10-01 at 22:53 +0200, Savvas Etairidis wrote: > [YOCTO #9993] > > A checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when > UNINATIVE_URL points at a local file://) was never verified, because > Local disables the donestamp and so verify_checksum() was never called. > Override verify_donestamp() in Local to check the file against those > checksums. A mismatch raises ChecksumError before the rename/mirror > handling, so the user's file is never renamed. > > Based on a patch by Jhonata Poma-Hansen. > > AI-Generated: Uses Claude Code (Claude Opus 5.5) > Signed-off-by: Savvas Etairidis <setairidis@gmail.com> > > --- > changes in v7: > - We shouldn't assume that anything coming from a file:// URL is > "typically small", or that it is on fast local storage (it could be on a > HDD array accessed over NFS). > * Rework the code to avoid re-hashing the file on every fetch, and > instead only re-hash it when the checksum is actually expected to be verified. > - You don't need to mention this if it didn't result in actual change. > * Wont do in the future. > - Such as? This is a completely different approach so it's useful to know > which bits of feedback were actually relevant and which were made > obsolete by the change in approach. > * Hoped I did better this time. > - What does "edited with an old mtime" mean? > * In that test the file attributes are modified to have an old mtime, > which is a corner case that was not handled correctly before. > This test was removed in v6 because the new approach doesn't have > that issue anymore. > - Why complicate this by using different hash algorithms for the different > tests? > The test wants to cover the case where we have a valid md5sum but an > invalid sha256sum or a invalid md5sum but a valid sha256sum. > - Added a test to covern new functionality. This is now inventing a new stamp file mechanism, and the tests are significantly more complicated than they were in the previous version of the patch. I'm sorry to say that this is moving further away from a solution that we can merge, instead of closer to one. This should look like the other fetchers and use the existing done stamps mechanism. That may not be able to detect changes in files if the mtime is moved backwards, but the local filesystem is not a security boundary so this is probably ok. It's also better to get something in place and document the limitations in the bugzilla entry rather than continuing to iterate and add complexity. There is also no need to test with different hash algorithms here. Two simple test cases, one for a matching checksum and one for a mismatch, should be enough, and they should use the same hash algorithm for consistency. The different hash algorithms should be tested in separate unit tests that focus on those features. AI tools really get carried away when adding test cases and some judgement is definitely needed to keep them under control. Best regards, -- Paul Barker ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v7] fetch/local: verify checksums for file:// urls 2026-10-02 8:50 ` Paul Barker @ 2026-10-02 20:48 ` Sawas Etairidis 2026-10-02 20:50 ` [PATCH v8] " Savvas Etairidis 0 siblings, 1 reply; 20+ messages in thread From: Sawas Etairidis @ 2026-10-02 20:48 UTC (permalink / raw) To: Paul Barker; +Cc: bitbake-devel, Jhonata Poma-Hansen [-- Attachment #1: Type: text/plain, Size: 4299 bytes --] Hi, The new stamp file mechanism was there to cover the following case: Developer A builds with file:///mnt/nas/foo.tar.xz;sha256sum=<X>, which caches the file's checksums in a donestamp. Developer B then replaces the file on the NAS with an older version that keeps its older timestamp (e.g. mv, cp -p, rsync -a). On A's next build the file is older than the donestamp, so the cached checksums are trusted and B's file is used in place without being hashed, even though it no longer matches <X>. As that was a larger code change, it needed more unit tests. To me it is better to have one test too many than one too few, with priority given to edge cases, as long as the tests take seconds or less to run. I will send a new version where the existing donestamp is enabled for file:// urls that have a checksum, with the two original unit tests: one for a matching checksum and one for a mismatch. Note that this version does not cover the case above. Regards, Savvas Στις Παρ 2 Οκτ 2026 στις 10:50 π.μ., ο/η Paul Barker <paul@pbarker.dev> έγραψε: > On Thu, 2026-10-01 at 22:53 +0200, Savvas Etairidis wrote: > > [YOCTO #9993] > > > > A checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when > > UNINATIVE_URL points at a local file://) was never verified, because > > Local disables the donestamp and so verify_checksum() was never called. > > Override verify_donestamp() in Local to check the file against those > > checksums. A mismatch raises ChecksumError before the rename/mirror > > handling, so the user's file is never renamed. > > > > Based on a patch by Jhonata Poma-Hansen. > > > > AI-Generated: Uses Claude Code (Claude Opus 5.5) > > Signed-off-by: Savvas Etairidis <setairidis@gmail.com> > > > > --- > > changes in v7: > > - We shouldn't assume that anything coming from a file:// URL is > > "typically small", or that it is on fast local storage (it could be on > a > > HDD array accessed over NFS). > > * Rework the code to avoid re-hashing the file on every fetch, and > > instead only re-hash it when the checksum is actually expected to be > verified. > > - You don't need to mention this if it didn't result in actual change. > > * Wont do in the future. > > - Such as? This is a completely different approach so it's useful to know > > which bits of feedback were actually relevant and which were made > > obsolete by the change in approach. > > * Hoped I did better this time. > > - What does "edited with an old mtime" mean? > > * In that test the file attributes are modified to have an old mtime, > > which is a corner case that was not handled correctly before. > > This test was removed in v6 because the new approach doesn't have > > that issue anymore. > > - Why complicate this by using different hash algorithms for the > different > > tests? > > The test wants to cover the case where we have a valid md5sum but an > > invalid sha256sum or a invalid md5sum but a valid sha256sum. > > - Added a test to covern new functionality. > > This is now inventing a new stamp file mechanism, and the tests are > significantly more complicated than they were in the previous version of > the patch. I'm sorry to say that this is moving further away from a > solution that we can merge, instead of closer to one. > > This should look like the other fetchers and use the existing done > stamps mechanism. That may not be able to detect changes in files if the > mtime is moved backwards, but the local filesystem is not a security > boundary so this is probably ok. It's also better to get something in > place and document the limitations in the bugzilla entry rather than > continuing to iterate and add complexity. > > There is also no need to test with different hash algorithms here. Two > simple test cases, one for a matching checksum and one for a mismatch, > should be enough, and they should use the same hash algorithm for > consistency. The different hash algorithms should be tested in separate > unit tests that focus on those features. AI tools really get carried > away when adding test cases and some judgement is definitely needed to > keep them under control. > > Best regards, > > -- > Paul Barker > > [-- Attachment #2: Type: text/html, Size: 5429 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v8] fetch/local: verify checksums for file:// urls 2026-10-02 20:48 ` Sawas Etairidis @ 2026-10-02 20:50 ` Savvas Etairidis 2026-10-05 14:36 ` Paul Barker 0 siblings, 1 reply; 20+ messages in thread From: Savvas Etairidis @ 2026-10-02 20:50 UTC (permalink / raw) To: bitbake-devel; +Cc: Jhonata Poma-Hansen, Paul Barker, Savvas Etairidis [YOCTO #9993] A checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at a local file://) was never verified, because Local disables the donestamp and so verify_checksum() was never called. Enable the donestamp for file:// urls that carry a checksum. Based on a patch by Jhonata Poma-Hansen. AI-Generated: Uses Claude Code (Claude Opus 5.5) Signed-off-by: Savvas Etairidis <setairidis@gmail.com> --- changes in v8: - Dropped the local-checksums stamps and re-used the regular donestamp, enabled only for file:// urls with a checksum. local.py is unchanged again. - Tests reduced to a checksum match and a mismatch test. - Tested with BB_SKIP_NETTESTS=yes bin/bitbake-selftest bb.tests.fetch changes in v7: - We shouldn't assume that anything coming from a file:// URL is "typically small", or that it is on fast local storage (it could be on a HDD array accessed over NFS). * Rework the code to avoid re-hashing the file on every fetch, and instead only re-hash it when the checksum is actually expected to be verified. - You don't need to mention this if it didn't result in actual change. * Wont do in the future. - Such as? This is a completely different approach so it's useful to know which bits of feedback were actually relevant and which were made obsolete by the change in approach. * Hoped I did better this time. - What does "edited with an old mtime" mean? * In that test the file attributes are modified to have an old mtime, which is a corner case that was not handled correctly before. This test was removed in v6 because the new approach doesn't have that issue anymore. - Why complicate this by using different hash algorithms for the different tests? The test wants to cover the case where we have a valid md5sum but an invalid sha256sum or a invalid md5sum but a valid sha256sum. - Added a test to covern new functionality. changes in v6: - Rebased on top of master - Fixed issues reported by reviewer - Check the file directly in Local.verify_donestamp(), via a new FetchData.verify_checksum_if_expected() helper, instead of enabling the donestamp. Fixes stale checksums for files edited with an old mtime and DL_DIR stamp/lock path collisions - Drop the rename_bad_checksum() refactor, no longer needed - Document the cost of re-hashing on every fetch - Tests: use a correct md5sum and a wrong sha256sum in the mismatch test, add a test for a file edited with an old mtime, and check that nothing is written to DL_DIR - Tested with BB_SKIP_NETTESTS=yes bin/bitbake-selftest bb.tests.fetch changes in v5: - Rebased on top of master, added changelog entry changes in v4: - Rebased on top of master changes in v3: - Cherry-picked the patch from Jhonata Poma-Hansen --- --- lib/bb/fetch/__init__.py | 16 ++++++++++++++++ lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py index eb3483a48..6401ce1b8 100644 --- a/lib/bb/fetch/__init__.py +++ b/lib/bb/fetch/__init__.py @@ -1087,6 +1087,10 @@ def rename_bad_checksum(ud, suffix, localpath=None): if ud.localpath is None: return + # A file:// url refers to the user's own file, leave it in place + if isinstance(ud.method, local.Local): + return + if localpath is None: localpath = ud.localpath @@ -1389,6 +1393,9 @@ class FetchData(object): for checksum_id in CHECKSUM_LIST: configure_checksum(checksum_id) + if isinstance(self.method, local.Local): + self.needdonestamp = self.checksum_expected() + self.ignore_checksums = False if "localpath" in self.parm: @@ -1416,6 +1423,15 @@ class FetchData(object): self.donestamp = basepath + '.done' self.lockfile = basepath + '.lock' + def checksum_expected(self): + """ + Is any checksum given for this url? + """ + for checksum_id in CHECKSUM_LIST: + if getattr(self, "%s_expected" % checksum_id): + return True + return False + def setup_revisions(self, d): self.revision = srcrev_internal_helper(self, d, self.name) diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index 9627b3b1b..a6a993dcd 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -876,6 +876,36 @@ class FetcherLocalTest(FetcherTest): with self.subTest(striplevel=repr(value)): self.assertInvalidStriplevel(value) + def test_local_checksum_match(self): + # Correct sha256 must run verify_checksum to completion; donestamp + # lands in DL_DIR. + import hashlib + content = b"file:// checksum match test\n" + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: + f.write(content) + good = hashlib.sha256(content).hexdigest() + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) + ud = fetcher.ud[fetcher.urls[0]] + self.assertTrue(ud.needdonestamp) + fetcher.download() + self.assertTrue(os.path.exists(ud.donestamp)) + + def test_local_checksum_mismatch(self): + # Bad sha256 raises ChecksumError without renaming the user's + # source file to the <localpath>_bad-checksum_<sha> sibling. + content = b"file:// checksum mismatch test\n" + srcpath = os.path.join(self.localsrcdir, 'sumfile') + with open(srcpath, 'wb') as f: + f.write(content) + bad = "0" * 64 + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d) + with self.assertRaises(bb.fetch2.FetchError): + fetcher.download() + self.assertTrue(os.path.exists(srcpath)) + with open(srcpath, 'rb') as f: + self.assertEqual(f.read(), content) + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad)) + def dummyGitTest(self, suffix): # Create dummy local Git repo src_dir = tempfile.mkdtemp(dir=self.tempdir, -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v8] fetch/local: verify checksums for file:// urls 2026-10-02 20:50 ` [PATCH v8] " Savvas Etairidis @ 2026-10-05 14:36 ` Paul Barker 2026-10-06 16:40 ` Sawas Etairidis 0 siblings, 1 reply; 20+ messages in thread From: Paul Barker @ 2026-10-05 14:36 UTC (permalink / raw) To: Savvas Etairidis, bitbake-devel; +Cc: Jhonata Poma-Hansen On Fri, 2026-10-02 at 22:50 +0200, Savvas Etairidis wrote: > [YOCTO #9993] > > A checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when > UNINATIVE_URL points at a local file://) was never verified, because > Local disables the donestamp and so verify_checksum() was never called. > Enable the donestamp for file:// urls that carry a checksum. > > Based on a patch by Jhonata Poma-Hansen. > > AI-Generated: Uses Claude Code (Claude Opus 5.5) > Signed-off-by: Savvas Etairidis <setairidis@gmail.com> > > --- > changes in v8: > - Dropped the local-checksums stamps and re-used the regular donestamp, > enabled only for file:// urls with a checksum. local.py is unchanged > again. > - Tests reduced to a checksum match and a mismatch test. > - Tested with BB_SKIP_NETTESTS=yes bin/bitbake-selftest bb.tests.fetch > > changes in v7: > - We shouldn't assume that anything coming from a file:// URL is > "typically small", or that it is on fast local storage (it could be on a > HDD array accessed over NFS). > * Rework the code to avoid re-hashing the file on every fetch, and > instead only re-hash it when the checksum is actually expected to be verified. > - You don't need to mention this if it didn't result in actual change. > * Wont do in the future. > - Such as? This is a completely different approach so it's useful to know > which bits of feedback were actually relevant and which were made > obsolete by the change in approach. > * Hoped I did better this time. > - What does "edited with an old mtime" mean? > * In that test the file attributes are modified to have an old mtime, > which is a corner case that was not handled correctly before. > This test was removed in v6 because the new approach doesn't have > that issue anymore. > - Why complicate this by using different hash algorithms for the different > tests? > The test wants to cover the case where we have a valid md5sum but an > invalid sha256sum or a invalid md5sum but a valid sha256sum. > - Added a test to covern new functionality. > > changes in v6: > - Rebased on top of master > - Fixed issues reported by reviewer > - Check the file directly in Local.verify_donestamp(), via a new > FetchData.verify_checksum_if_expected() helper, instead of enabling > the donestamp. Fixes stale checksums for files edited with an old > mtime and DL_DIR stamp/lock path collisions > - Drop the rename_bad_checksum() refactor, no longer needed > - Document the cost of re-hashing on every fetch > - Tests: use a correct md5sum and a wrong sha256sum in the mismatch > test, add a test for a file edited with an old mtime, and check that > nothing is written to DL_DIR > - Tested with BB_SKIP_NETTESTS=yes bin/bitbake-selftest bb.tests.fetch > > changes in v5: > - Rebased on top of master, added changelog entry > > changes in v4: > - Rebased on top of master > > changes in v3: > - Cherry-picked the patch from Jhonata Poma-Hansen > > --- > --- > lib/bb/fetch/__init__.py | 16 ++++++++++++++++ > lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py > index eb3483a48..6401ce1b8 100644 > --- a/lib/bb/fetch/__init__.py > +++ b/lib/bb/fetch/__init__.py > @@ -1087,6 +1087,10 @@ def rename_bad_checksum(ud, suffix, localpath=None): > if ud.localpath is None: > return > > + # A file:// url refers to the user's own file, leave it in place > + if isinstance(ud.method, local.Local): > + return Adding special cases in the base fetcher code is poor design and complicates maintenance. Why have you taken this approach instead of making rename_bad_checksum() a method of the Fetch class and overriding it as you did in v4 of this patch? I'm looking for an explanation not a v9 patch. Best regards, -- Paul Barker ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v8] fetch/local: verify checksums for file:// urls 2026-10-05 14:36 ` Paul Barker @ 2026-10-06 16:40 ` Sawas Etairidis 0 siblings, 0 replies; 20+ messages in thread From: Sawas Etairidis @ 2026-10-06 16:40 UTC (permalink / raw) To: Paul Barker; +Cc: bitbake-devel, Jhonata Poma-Hansen [-- Attachment #1: Type: text/plain, Size: 4449 bytes --] Hi, Path of least resistance: I noticed the isinstance check above and thought it was acceptable. My bad I sends this as a personal response previously. Regards, Savvas Στις Δευ 5 Οκτ 2026 στις 4:36 μ.μ., ο/η Paul Barker <paul@pbarker.dev> έγραψε: > On Fri, 2026-10-02 at 22:50 +0200, Savvas Etairidis wrote: > > [YOCTO #9993] > > > > A checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when > > UNINATIVE_URL points at a local file://) was never verified, because > > Local disables the donestamp and so verify_checksum() was never called. > > Enable the donestamp for file:// urls that carry a checksum. > > > > Based on a patch by Jhonata Poma-Hansen. > > > > AI-Generated: Uses Claude Code (Claude Opus 5.5) > > Signed-off-by: Savvas Etairidis <setairidis@gmail.com> > > > > --- > > changes in v8: > > - Dropped the local-checksums stamps and re-used the regular donestamp, > > enabled only for file:// urls with a checksum. local.py is unchanged > > again. > > - Tests reduced to a checksum match and a mismatch test. > > - Tested with BB_SKIP_NETTESTS=yes bin/bitbake-selftest bb.tests.fetch > > > > changes in v7: > > - We shouldn't assume that anything coming from a file:// URL is > > "typically small", or that it is on fast local storage (it could be on > a > > HDD array accessed over NFS). > > * Rework the code to avoid re-hashing the file on every fetch, and > > instead only re-hash it when the checksum is actually expected to be > verified. > > - You don't need to mention this if it didn't result in actual change. > > * Wont do in the future. > > - Such as? This is a completely different approach so it's useful to know > > which bits of feedback were actually relevant and which were made > > obsolete by the change in approach. > > * Hoped I did better this time. > > - What does "edited with an old mtime" mean? > > * In that test the file attributes are modified to have an old mtime, > > which is a corner case that was not handled correctly before. > > This test was removed in v6 because the new approach doesn't have > > that issue anymore. > > - Why complicate this by using different hash algorithms for the > different > > tests? > > The test wants to cover the case where we have a valid md5sum but an > > invalid sha256sum or a invalid md5sum but a valid sha256sum. > > - Added a test to covern new functionality. > > > > changes in v6: > > - Rebased on top of master > > - Fixed issues reported by reviewer > > - Check the file directly in Local.verify_donestamp(), via a new > > FetchData.verify_checksum_if_expected() helper, instead of enabling > > the donestamp. Fixes stale checksums for files edited with an old > > mtime and DL_DIR stamp/lock path collisions > > - Drop the rename_bad_checksum() refactor, no longer needed > > - Document the cost of re-hashing on every fetch > > - Tests: use a correct md5sum and a wrong sha256sum in the mismatch > > test, add a test for a file edited with an old mtime, and check that > > nothing is written to DL_DIR > > - Tested with BB_SKIP_NETTESTS=yes bin/bitbake-selftest bb.tests.fetch > > > > changes in v5: > > - Rebased on top of master, added changelog entry > > > > changes in v4: > > - Rebased on top of master > > > > changes in v3: > > - Cherry-picked the patch from Jhonata Poma-Hansen > > > > --- > > --- > > lib/bb/fetch/__init__.py | 16 ++++++++++++++++ > > lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ > > 2 files changed, 46 insertions(+) > > > > diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py > > index eb3483a48..6401ce1b8 100644 > > --- a/lib/bb/fetch/__init__.py > > +++ b/lib/bb/fetch/__init__.py > > @@ -1087,6 +1087,10 @@ def rename_bad_checksum(ud, suffix, > localpath=None): > > if ud.localpath is None: > > return > > > > + # A file:// url refers to the user's own file, leave it in place > > + if isinstance(ud.method, local.Local): > > + return > > Adding special cases in the base fetcher code is poor design and > complicates maintenance. Why have you taken this approach instead of > making rename_bad_checksum() a method of the Fetch class and overriding > it as you did in v4 of this patch? I'm looking for an explanation not a > v9 patch. > > Best regards, > > -- > Paul Barker > > [-- Attachment #2: Type: text/html, Size: 5417 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-10-06 16:40 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-30 14:25 [PATCH] fetch2/local: verify checksums for file:// urls Jhonata Poma-Hansen 2026-05-08 14:13 ` [bitbake-devel] " Paul Barker 2026-05-11 14:58 ` [PATCH v2] " Jhonata Poma-Hansen 2026-05-11 15:01 ` Jhonata Poma-Hansen 2026-09-23 19:44 ` [PATCH v3] " Savvas Etairidis 2026-09-24 6:36 ` [bitbake-devel] " Antonin Godard 2026-09-24 20:45 ` [PATCH v4] fetch/local: " Savvas Etairidis 2026-09-28 9:30 ` Paul Barker 2026-09-28 20:59 ` [PATCH v5] " Savvas Etairidis 2026-09-28 21:02 ` Sawas Etairidis 2026-09-29 8:22 ` Jhonata Poma-Hansen 2026-09-30 10:51 ` Paul Barker 2026-09-30 21:21 ` [PATCH v6] " Savvas Etairidis 2026-10-01 8:32 ` Paul Barker 2026-10-01 20:53 ` [PATCH v7] " Savvas Etairidis 2026-10-02 8:50 ` Paul Barker 2026-10-02 20:48 ` Sawas Etairidis 2026-10-02 20:50 ` [PATCH v8] " Savvas Etairidis 2026-10-05 14:36 ` Paul Barker 2026-10-06 16:40 ` Sawas Etairidis
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.