diff mbox series

[RFC] fetch: `touch` mirror tarball stamp files on download

Message ID 20260902083232.825826-1-philip.lorenz@bmw.de
State New
Headers show
Series [RFC] fetch: `touch` mirror tarball stamp files on download | expand

Commit Message

Philip Lorenz Sept. 2, 2026, 8:32 a.m. UTC
The mirror tarball stamp file's mtime is currently only updated on
creation / initial download. This makes it difficult to detect whether a
mirror tarball is still relevant in a given build.

Improve this by also `touch`ing the mirror tarball stamp files alongside
the sources primary stamp file.

Signed-off-by: Philip Lorenz <philip.lorenz@bmw.de>
---
 lib/bb/fetch2/__init__.py | 17 ++++++++++++++---
 lib/bb/tests/fetch.py     | 15 +++++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

Comments

Richard Purdie Sept. 2, 2026, 9:06 a.m. UTC | #1
On Wed, 2026-09-02 at 10:32 +0200, Philip Lorenz via lists.openembedded.org wrote:
> The mirror tarball stamp file's mtime is currently only updated on
> creation / initial download. This makes it difficult to detect whether a
> mirror tarball is still relevant in a given build.
> 
> Improve this by also `touch`ing the mirror tarball stamp files alongside
> the sources primary stamp file.

This gets a bit tricky as we don't touch every download file at every
download, we only touch the end stamp file.

This would mean we touch mirror files and stamp files but not main
download files and I think brings things into more disparity?

I'm reluctant on more complexity in this area, we already have a lot of
problems with the mirror tarballs and code structure.

I can also think of cases where updating the timestamp may cause things
to re-download the tarballs too since you can't know the content in
advance with many of them. This is perhaps the stronger reason not to
do this.

I was also going to say there shouldn't be hardcoded DL_DIR references
in there but I notice the fetcher has a few too many of them already.
That would be a separate bug/issue :(.

Cheers,

Richard
Philip Lorenz Sept. 2, 2026, 12:43 p.m. UTC | #2
Hi Richard,

On Wed, Sep 2, 2026 at 11:06 AM, Richard Purdie wrote:

> 
> On Wed, 2026-09-02 at 10:32 +0200, Philip Lorenz via
> lists.openembedded.org wrote:
> 
>> The mirror tarball stamp file's mtime is currently only updated on
>> creation / initial download. This makes it difficult to detect whether a
>> mirror tarball is still relevant in a given build.
>> 
>> Improve this by also `touch`ing the mirror tarball stamp files alongside
>> the sources primary stamp file.
> 
> This gets a bit tricky as we don't touch every download file at every
> download, we only touch the end stamp file.
> 
> This would mean we touch mirror files and stamp files but not main
> download files and I think brings things into more disparity?

This patch extends bitbake to touch the mirror tarball's `.done` file (not the mirror tarball itself) to bring it inline with regular stamp file handling.
> 
> 
> I can also think of cases where updating the timestamp may cause things
> to re-download the tarballs too since you can't know the content in
> advance with many of them. This is perhaps the stronger reason not to
> do this.

I'm not completely familiar with all of the fetcher details (and also couldn't find anything obvious) but in which cases would changing the mtime lead to a redownload? I'm not sure if this is still an issue given that this only updates the `.done` files of the mirror tarballs though.

The rationale behind this change is that we'd like to clean up our download caches and only keep the inputs relevant to the latest build configuration around. Using `.done` files (after executing a `bitbake --runall=fetch`) works for this as long as the mirror tarballs are freshly created. On consecutive runs their `.done` file is not touched and they are therefore deemed out of date.

Philip
Richard Purdie Sept. 2, 2026, 3:26 p.m. UTC | #3
On Wed, 2026-09-02 at 05:43 -0700, Philip Lorenz via lists.openembedded.org wrote:
> On Wed, Sep 2, 2026 at 11:06 AM, Richard Purdie wrote:
> > On Wed, 2026-09-02 at 10:32 +0200, Philip Lorenz via lists.openembedded.org wrote:
> > > The mirror tarball stamp file's mtime is currently only updated on
> > > creation / initial download. This makes it difficult to detect whether a
> > > mirror tarball is still relevant in a given build.
> > > 
> > > Improve this by also `touch`ing the mirror tarball stamp files alongside
> > > the sources primary stamp file.
> > This gets a bit tricky as we don't touch every download file at every
> > download, we only touch the end stamp file.
> > 
> > This would mean we touch mirror files and stamp files but not main
> > download files and I think brings things into more disparity?
> This patch extends bitbake to touch the mirror tarball's `.done` file (not the 
> mirror tarball itself) to bring it inline with regular stamp file handling.
> > I can also think of cases where updating the timestamp may cause things
> > to re-download the tarballs too since you can't know the content in
> > advance with many of them. This is perhaps the stronger reason not to
> > do this.
> I'm not completely familiar with all of the fetcher details (and also couldn't 
> find anything obvious) but in which cases would changing the mtime lead to a 
> redownload? I'm not sure if this is still an issue given that this only updates 
> the `.done` files of the mirror tarballs though.
>  
> The rationale behind this change is that we'd like to clean up our download caches 
> and only keep the inputs relevant to the latest build configuration around. Using 
> `.done` files (after executing a `bitbake --runall=fetch`) works for this as long 
> as the mirror tarballs are freshly created. On consecutive runs their `.done` file 
> is not touched and they are therefore deemed out of date.

I hadn't fully taken in the fact it was just the done stamps, sorry.
This does still leave me with questions though.

For example we have cases where it wouldn't try and download mirror
tarballs. If the main source repo is up to date or even just present,
it could fetch to there and the mirror tarballs wouldn't be touched.
They could even be out of date compared to the main source repo unless
the tarball generation options are set.

So in the context of having mirror tarball generation set, the patch
could make sense but probably not outside of that.

I have thoughts about changing the way mirror tarballs work in the next
release anyway so I'm still leaning towards not adding more complexity
right now...

Cheers,

Richard
diff mbox series

Patch

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 9cb268be5..7b981b0f3 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -755,13 +755,24 @@  def update_stamp(ud, d):
     if not ud.needdonestamp:
         return
 
-    if os.path.exists(ud.donestamp):
-        # Touch the done stamp file to show active use of the download
+    def bump_utime(path):
         try:
-            os.utime(ud.donestamp, None)
+            os.utime(path, None)
         except:
             # Errors aren't fatal here
             pass
+
+    if os.path.exists(ud.donestamp):
+        # Touch the done stamp file to show active use of the download
+        bump_utime(ud.donestamp)
+
+        # Also mark any mirror tarballs related to the download as being active.
+        dl_dir = d.getVar("DL_DIR")
+        for mirrortarball in ud.mirrortarballs:
+            mirrortarball = os.path.join(dl_dir, mirrortarball) + ".done"
+            # There's no need to check if the file exists as `bump_utime` will
+            # ignore non-existent files.
+            bump_utime(mirrortarball)
     else:
         try:
             checksums = verify_checksum(ud, d)
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index a1e4b45f8..b29d5909f 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.fetch2 import URI
 import bb
 import bb.utils
@@ -1101,18 +1102,32 @@  class FetcherNetworkTest(FetcherTest):
                                 cwd=os.path.join(self.unpackdir, 'git')).strip()
             self.assertEqual(revision, "270a05b0b4ba0959fe0624d2a4885d7b70426da5")
 
+        def checkdonestamp(fetcher, url, download_timestamp):
+            ud = fetcher.ud[url]
+            self.assertGreaterEqual(os.stat(ud.donestamp).st_mtime, download_timestamp)
+
+            dldir = self.d.getVar("DL_DIR")
+            for mirrortarball in ud.mirrortarballs:
+                mirrortarball = os.path.join(dldir, mirrortarball) + ".done"
+                if os.path.exists(mirrortarball):
+                    self.assertGreaterEqual(os.stat(mirrortarball).st_mtime, download_timestamp)
+
         self.d.setVar("BB_GENERATE_MIRROR_TARBALLS", "1")
         self.d.setVar("SRCREV", "270a05b0b4ba0959fe0624d2a4885d7b70426da5")
         fetcher = bb.fetch.Fetch([url1], self.d)
+        download_timestamp = time.time()
         fetcher.download()
         checkrevision(self, fetcher)
+        checkdonestamp(fetcher, url1, download_timestamp)
         # Wipe out the dldir clone and the unpacked source, turn off the network and check mirror tarball works
         bb.utils.prunedir(self.dldir + "/git2/")
         bb.utils.prunedir(self.unpackdir)
         self.d.setVar("BB_NO_NETWORK", "1")
         fetcher = bb.fetch.Fetch([url2], self.d)
+        download_timestamp = time.time()
         fetcher.download()
         checkrevision(self, fetcher)
+        checkdonestamp(fetcher, url2, download_timestamp)
 
     @skipIfNoNetwork()
     def test_gitfetch(self):