Re: [PATCH v8] fetch/local: verify checksums for file:// urls

[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