Re: [yocto-patches] [yocto-autobuilder-helper][PATCH v2 01/10] scripts/utils: fix stale extraction dir when tarball is updated

Paul Barker <[email protected]> Tue, 02 Jun 2026 11:22:14 +0100
Newsgroups	org.yoctoproject.lists.yocto-patches
Message-ID	<[email protected]>
On Mon, 2026-06-01 at 16:18 -0700, Tim Orling via lists.yoctoproject.org
wrote:
> From: Tim Orling <[email protected]>
> 
> Previously the entire download/cache-validation block in
> setup_tools_tarball() was guarded by `if not os.path.exists(btdir)`.
> Once the extraction directory existed from a prior build, every
> subsequent call was a no-op: the cached tarball was never re-validated,
> so a freshly-published SDK (e.g. vcontainer-tarball-latest) was silently
> ignored and the stale btdir kept being used.
> 
> Fix by moving the lock/download block outside the btdir existence guard
> so cache validation always runs. Track tarball_updated to know when the
> cached download was actually replaced, then remove the stale btdir when
> True so the fresh tarball is re-extracted.
> 
> Also add an mtime-based staleness check for local-path (cp) sources so
> that a newer source file automatically invalidates the cached copy.
> 
> AI-Generated: Claude Cowork Sonnet 4.6
> Signed-off-by: Tim Orling <[email protected]>
> ---
>  scripts/utils.py | 64 +++++++++++++++++++++++++++++++-----------------
>  1 file changed, 41 insertions(+), 23 deletions(-)
> 
> diff --git a/scripts/utils.py b/scripts/utils.py
> index 88842f0..87acad6 100644
> --- a/scripts/utils.py
> +++ b/scripts/utils.py
> @@ -481,31 +481,49 @@ def setup_tools_tarball(ourconfig, btdir, bttarball, name="buildtools"):
>          if ";" in bttarball:
>              bttarball, sha256 = bttarball.split(";")
>          btdir = os.path.abspath(btdir)
> +        btdlpath = getconfig("BASE_SHAREDDIR", ourconfig) + "/cluster-downloads-cache/" + os.path.basename(bttarball)
> +        btlock = btdlpath + ".lock"
> +        if not os.path.exists(os.path.dirname(btdlpath)):
> +            os.makedirs(os.path.dirname(btdlpath), exist_ok=True)
> +        # Always run the cache-validation / download step so that a freshly
> +        # published tarball is detected even when btdir already exists from a
> +        # previous build.  tarball_updated is set to True whenever the cached
> +        # download is replaced, which triggers removal of the stale btdir.
> +        tarball_updated = False
> +        while True:
> +            try:
> +                with open(btlock, 'a+') as lf:
> +                    fileno = lf.fileno()
> +                    fcntl.flock(fileno, fcntl.LOCK_EX)
> +                    if sha256 and os.path.exists(btdlpath):
> +                        dl_sha256 = sha256_file(btdlpath)
> +                        if dl_sha256 != sha256:
> +                            os.unlink(btdlpath)
> +                    elif bttarball.startswith("/") and os.path.exists(btdlpath):
> +                        # For local-path sources (e.g. vcontainer-tarball-latest)
> +                        # invalidate the cached copy when the source is newer so
> +                        # that a freshly-published tarball is always picked up.
> +                        if os.path.getmtime(bttarball) > os.path.getmtime(btdlpath):
> +                            os.unlink(btdlpath)
> +                        os.unlink(btdlpath)
> +                    if not os.path.exists(btdlpath):
> +                        if bttarball.startswith("/"):
> +                            subprocess.check_call(["cp", bttarball, btdlpath])
> +                        else:
> +                            subprocess.check_call(["wget", "-O", btdlpath, bttarball])
> +                        os.chmod(btdlpath, 0o775)
> +                        tarball_updated = True
> +                break
> +            except OSError:
> +                # We raced with someone else, try again
> +                pass

Hi Tim,

This loop allows us to handle random OSError conditions caused by a
race, but it turns a deterministic OSError into an infinite loop. The
logic already exists, you're just moving it around, but now I have seen
it I cannot unsee it!

Should we add a timeout while we are here?

> +        # If the underlying tarball changed, remove any stale extraction
> +        # directory so it is re-extracted below.
> +        if tarball_updated and os.path.exists(btdir):
> +            print("Removing stale %s extraction at %s" % (name, btdir))
> +            subprocess.check_call(["rm", "-rf", btdir])
>          if not os.path.exists(btdir):
> -            btdlpath = getconfig("BASE_SHAREDDIR", ourconfig) + "/cluster-downloads-cache/" + os.path.basename(bttarball)
>              print("Extracting %s %s" % (name, bttarball))
> -            btlock = btdlpath + ".lock"
> -            if not os.path.exists(os.path.dirname(btdlpath)):
> -                os.makedirs(os.path.dirname(btdlpath), exist_ok=True)
> -            while True:
> -                try:
> -                    with open(btlock, 'a+') as lf:
> -                        fileno = lf.fileno()
> -                        fcntl.flock(fileno, fcntl.LOCK_EX)
> -                        if sha256 and os.path.exists(btdlpath):
> -                            dl_sha256 = sha256_file(btdlpath)
> -                            if dl_sha256 != sha256:
> -                                os.unlink(btdlpath)
> -                        if not os.path.exists(btdlpath):
> -                            if bttarball.startswith("/"):
> -                                subprocess.check_call(["cp", bttarball, btdlpath])
> -                            else:
> -                                subprocess.check_call(["wget", "-O", btdlpath, bttarball])
> -                            os.chmod(btdlpath, 0o775)
> -                    break
> -                except OSError:
> -                    # We raced with someone else, try again
> -                    pass
>              subprocess.check_call(["bash", btdlpath, "-d", btdir, "-y"])
>          enable_tools_tarball(btdir, name)

Best regards,

-- 
Paul Barker
signature.asc (application/pgp-signature, 252 B)