[PATCH 0/4] Improve cacheability

[PATCH 0/4] Improve cacheability

[Adriaan Schmidt]

We have recently started analyzing more details on sstate caching, and
identified some potential optimizations. This mainly concerns cases
of absolute paths contained in variables that are used as inputs to
the sstate signature caclulation, which can lead to "false negatives"
when sstate looks for cache hits, e.g., when CI runs on a different
runner (with different local workdir).

This series contains independent patches with fixes for some issues
we found so far.

Adriaan Schmidt (4):
  bitbake.conf: always start with empty SRC_URI
  bitbake.conf: add isar paths to BB_HASHBASE_WHITELIST
  isar-bootstrap: no absolute paths in SRC_URI
  base.bbclass: don't pass absolute paths to root_cleandirs

 meta/classes/base.bbclass                          | 14 +++++++++-----
 meta/conf/bitbake.conf                             |  5 +++--
 .../recipes-core/isar-bootstrap/isar-bootstrap.inc |  5 +++--
 3 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.30.2

[PATCH 1/4] bitbake.conf: always start with empty SRC_URI

[Adriaan Schmidt]

As ${FILE} is an absolute path, having it in SRC_URI breaks caching.
This applies to recipes that don't set SRC_URI (e.g. meta packages
that are only there to pull dependencies), or ones that use "+="
to extend SRC_URI.

The recipe path doesn't need to be part of SRC_URI. Changes/dependencies
are tracked by bitbake, and OE/poky also sets it empty in its bitbake.conf.

Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
---
 meta/conf/bitbake.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index cd1c4a64..fdb7a5c8 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -36,7 +36,7 @@ PR = "${@bb.parse.BBHandler.vars_from_file(d.getVar('FILE', False),d)[2] or 'r0'
 PROVIDES = ""
 S = "${WORKDIR}/${P}"
 AUTOREV = "${@bb.fetch2.get_autorev(d)}"
-SRC_URI = "file://${FILE}"
+SRC_URI = ""
 STAMPS_DIR ?= "${TMPDIR}/stamps"
 STAMP = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}/${PV}-${PR}"
 STAMPCLEAN = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}/*-*"
-- 
2.30.2

[PATCH 2/4] bitbake.conf: add isar paths to BB_HASHBASE_WHITELIST

[Adriaan Schmidt]

There are some absolute paths that should not be included
when caclulating hashes/signatures. Things like TMPDIR or WORKDIR
are part of OE's default, but we also need to add the Isar-specific
paths here.

Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
---
 meta/conf/bitbake.conf | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index fdb7a5c8..db1180a7 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -100,7 +100,8 @@ BB_HASHBASE_WHITELIST ?= "TMPDIR FILE PATH PWD BB_TASKHASH BBPATH BBSERVER DL_DI
     PRSERV_DUMPDIR PRSERV_DUMPFILE PRSERV_LOCKDOWN PARALLEL_MAKE \
     CCACHE_DIR EXTERNAL_TOOLCHAIN CCACHE CCACHE_NOHASHDIR LICENSE_PATH SDKPKGSUFFIX \
     WORKDIR STAMPCLEAN PKGDATA_DIR BUILD_ARCH SSTATE_PKGARCH \
-    BB_WORKERCONTEXT BB_LIMITEDDEPS DEPLOY_DIR"
+    BB_WORKERCONTEXT BB_LIMITEDDEPS DEPLOY_DIR BUILDCHROOT_DIR \
+    REPO_ISAR_DIR REPO_ISAR_DB_DIR REPO_BASE_DIR REPO_BASE_DB_DIR"
 BB_HASHCONFIG_WHITELIST ?= "${BB_HASHBASE_WHITELIST} DATE TIME SSH_AGENT_PID \
     SSH_AUTH_SOCK PSEUDO_BUILD BB_ENV_EXTRAWHITE DISABLE_SANITY_CHECKS \
     BB_NUMBER_THREADS BB_ORIGENV BB_INVALIDCONF BBINCLUDED \
-- 
2.30.2

[PATCH 3/4] isar-bootstrap: no absolute paths in SRC_URI

[Adriaan Schmidt]

Having absolute paths in SRC_URI breaks sstate caching.
So we don't use resolve_file (which searches BBPATH and returns an absolute
path), but instead append the search path to FILESEXTRAPATHS and let the fetcher
find the files.

Note that there is no risk of finding/caching the wrong file, as in addition
to the path in SRC_URI, also the file contents are hashed.

Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
---
 meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
index a6e370e3..145d5e87 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
@@ -29,6 +29,7 @@ DISTRO_BOOTSTRAP_BASE_PACKAGES_append_gnupg = ",gnupg"
 DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "${@https_support(d)}"
 DISTRO_VARS_PREFIX ?= "${@'HOST_' if d.getVar('BOOTSTRAP_FOR_HOST') == '1' else ''}"
 BOOTSTRAP_DISTRO = "${@d.getVar('HOST_DISTRO' if d.getVar('BOOTSTRAP_FOR_HOST') == '1' else 'DISTRO')}"
+FILESEXTRAPATHS_append = ":${BBPATH}"
 
 inherit deb-dl-dir
 
@@ -60,11 +61,11 @@ python () {
 
     distro_apt_sources = d.getVar(d.getVar("DISTRO_VARS_PREFIX") + "DISTRO_APT_SOURCES", True) or ""
     for file in distro_apt_sources.split():
-        d.appendVar("SRC_URI", " file://%s" % bb.parse.resolve_file(file, d))
+        d.appendVar("SRC_URI", " file://%s" % file)
 
     distro_apt_preferences = d.getVar(d.getVar("DISTRO_VARS_PREFIX") + "DISTRO_APT_PREFERENCES", True) or ""
     for file in distro_apt_preferences.split():
-        d.appendVar("SRC_URI", " file://%s" % bb.parse.resolve_file(file, d))
+        d.appendVar("SRC_URI", " file://%s" % file)
 }
 
 def aggregate_files(d, file_list, file_out):
-- 
2.30.2

[PATCH 4/4] base.bbclass: don't pass absolute paths to root_cleandirs

[Adriaan Schmidt]

The directories to be cleaned are contained in the code that is prepended
to the modified task. If those contain absolute paths, it can break caching.
So instead, we pass paths relative to $TMPDIR (code already makes sure
the paths are actually below $TMPDIR).

Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
---
 meta/classes/base.bbclass | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index a7282110..db355875 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -105,14 +105,16 @@ python do_listtasks() {
 root_cleandirs() {
     ROOT_CLEANDIRS_DIRS_PY="${@d.getVar("ROOT_CLEANDIRS_DIRS", True) or ""}"
     ROOT_CLEANDIRS_DIRS="${ROOT_CLEANDIRS_DIRS-${ROOT_CLEANDIRS_DIRS_PY}}"
+    TMPDIR_PY="$@d.getVar("TMPDIR", True) or ""}"
+    TMPDIR="${TMPDIR-${TMPDIR_PY}}"
     for i in $ROOT_CLEANDIRS_DIRS; do
         awk '{ print $2 }' /proc/mounts | grep -q "^${i}\(/\|\$\)" && \
             die "Could not remove $i, because subdir is mounted"
     done
-    if [ -n "$ROOT_CLEANDIRS_DIRS" ]; then
-        sudo rm -rf --one-file-system $ROOT_CLEANDIRS_DIRS
-        mkdir -p $ROOT_CLEANDIRS_DIRS
-    fi
+    for i in $ROOT_CLEANDIRS_DIRS; do
+        sudo rm -rf --one-file-system "$TMPDIR$i"
+        mkdir -p "$TMPDIR$i"
+    done
 }
 
 python() {
@@ -148,8 +150,10 @@ python() {
                     )
 
                 ws = re.match(r"^\s*", d.getVar(e, False)).group()
+                # remove prefix ${TMPDIR}, so we don't have absolute paths in variable e
+                dirs = [dir[len(tmpdir):] for dir in rcleandirs]
                 d.prependVar(
-                    e, cleandir_code.format(ws=ws, dirlist=" ".join(rcleandirs))
+                    e, cleandir_code.format(ws=ws, dirlist=" ".join(dirs))
                 )
 }
 
-- 
2.30.2

RE: [PATCH 4/4] base.bbclass: don't pass absolute paths to root_cleandirs

[Moessbauer, Felix]

Hi Adriaan,

> -----Original Message-----
> From: isar-users@googlegroups.com <isar-users@googlegroups.com> On
> Behalf Of Adriaan Schmidt
> Sent: Monday, April 4, 2022 1:16 PM
> To: isar-users@googlegroups.com
> Cc: Schmidt, Adriaan (T CED SES-DE) <adriaan.schmidt@siemens.com>
> Subject: [PATCH 4/4] base.bbclass: don't pass absolute paths to root_cleandirs
> 
> The directories to be cleaned are contained in the code that is prepended to the
> modified task. If those contain absolute paths, it can break caching.
> So instead, we pass paths relative to $TMPDIR (code already makes sure the
> paths are actually below $TMPDIR).
> 
> Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
> ---
>  meta/classes/base.bbclass | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass index
> a7282110..db355875 100644
> --- a/meta/classes/base.bbclass
> +++ b/meta/classes/base.bbclass
> @@ -105,14 +105,16 @@ python do_listtasks() {
>  root_cleandirs() {
>      ROOT_CLEANDIRS_DIRS_PY="${@d.getVar("ROOT_CLEANDIRS_DIRS", True)
> or ""}"
>      ROOT_CLEANDIRS_DIRS="${ROOT_CLEANDIRS_DIRS-
> ${ROOT_CLEANDIRS_DIRS_PY}}"
> +    TMPDIR_PY="$@d.getVar("TMPDIR", True) or ""}"

There is a typo in that line.
By that, the inline python function is not invoked, but instead the line is passed as-is to the TMPDIR_PY.
This leads to a dir "d.getVar(TMPDIR, True) or }" being created in /build.

Felix

> +    TMPDIR="${TMPDIR-${TMPDIR_PY}}"
>      for i in $ROOT_CLEANDIRS_DIRS; do
>          awk '{ print $2 }' /proc/mounts | grep -q "^${i}\(/\|\$\)" && \
>              die "Could not remove $i, because subdir is mounted"
>      done
> -    if [ -n "$ROOT_CLEANDIRS_DIRS" ]; then
> -        sudo rm -rf --one-file-system $ROOT_CLEANDIRS_DIRS
> -        mkdir -p $ROOT_CLEANDIRS_DIRS
> -    fi
> +    for i in $ROOT_CLEANDIRS_DIRS; do
> +        sudo rm -rf --one-file-system "$TMPDIR$i"
> +        mkdir -p "$TMPDIR$i"
> +    done
>  }
> 
>  python() {
> @@ -148,8 +150,10 @@ python() {
>                      )
> 
>                  ws = re.match(r"^\s*", d.getVar(e, False)).group()
> +                # remove prefix ${TMPDIR}, so we don't have absolute paths in
> variable e
> +                dirs = [dir[len(tmpdir):] for dir in rcleandirs]
>                  d.prependVar(
> -                    e, cleandir_code.format(ws=ws, dirlist=" ".join(rcleandirs))
> +                    e, cleandir_code.format(ws=ws, dirlist="
> + ".join(dirs))
>                  )
>  }
> 
> --
> 2.30.2
> 
> --
> You received this message because you are subscribed to the Google Groups
> "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email
> to isar-users+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.g
> oogle.com%2Fd%2Fmsgid%2Fisar-users%2F20220404111613.2159786-5-
> adriaan.schmidt%2540siemens.com&amp;data=04%7C01%7Cfelix.moessbauer
> %40siemens.com%7C59454b1a80a746e5e34008da162c9418%7C38ae3bcd9579
> 4fd4addab42e1495d55a%7C1%7C0%7C637846678701794447%7CUnknown%7C
> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> VCI6Mn0%3D%7C2000&amp;sdata=eqpc14UYg7%2FH4z2xJnu%2F3GLthvMv67s
> Xj4NLt65eIR0%3D&amp;reserved=0.

RE: [PATCH 4/4] base.bbclass: don't pass absolute paths to root_cleandirs

[Schmidt, Adriaan]

Moessbauer, Felix, Freitag, 8. April 2022 17:41:
> Hi Adriaan,
> 
> > -----Original Message-----
> > From: isar-users@googlegroups.com <isar-users@googlegroups.com> On
> > Behalf Of Adriaan Schmidt
> > Sent: Monday, April 4, 2022 1:16 PM
> > To: isar-users@googlegroups.com
> > Cc: Schmidt, Adriaan (T CED SES-DE) <adriaan.schmidt@siemens.com>
> > Subject: [PATCH 4/4] base.bbclass: don't pass absolute paths to
> root_cleandirs
> >
> > The directories to be cleaned are contained in the code that is prepended
> to the
> > modified task. If those contain absolute paths, it can break caching.
> > So instead, we pass paths relative to $TMPDIR (code already makes sure the
> > paths are actually below $TMPDIR).
> >
> > Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com>
> > ---
> >  meta/classes/base.bbclass | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass index
> > a7282110..db355875 100644
> > --- a/meta/classes/base.bbclass
> > +++ b/meta/classes/base.bbclass
> > @@ -105,14 +105,16 @@ python do_listtasks() {
> >  root_cleandirs() {
> >      ROOT_CLEANDIRS_DIRS_PY="${@d.getVar("ROOT_CLEANDIRS_DIRS", True)
> > or ""}"
> >      ROOT_CLEANDIRS_DIRS="${ROOT_CLEANDIRS_DIRS-
> > ${ROOT_CLEANDIRS_DIRS_PY}}"
> > +    TMPDIR_PY="$@d.getVar("TMPDIR", True) or ""}"
> 
> There is a typo in that line.
> By that, the inline python function is not invoked, but instead the line is
> passed as-is to the TMPDIR_PY.
> This leads to a dir "d.getVar(TMPDIR, True) or }" being created in /build.

Thanks! Fixed in v2.
Adriaan

> Felix
> 
> > +    TMPDIR="${TMPDIR-${TMPDIR_PY}}"
> >      for i in $ROOT_CLEANDIRS_DIRS; do
> >          awk '{ print $2 }' /proc/mounts | grep -q "^${i}\(/\|\$\)" && \
> >              die "Could not remove $i, because subdir is mounted"
> >      done
> > -    if [ -n "$ROOT_CLEANDIRS_DIRS" ]; then
> > -        sudo rm -rf --one-file-system $ROOT_CLEANDIRS_DIRS
> > -        mkdir -p $ROOT_CLEANDIRS_DIRS
> > -    fi
> > +    for i in $ROOT_CLEANDIRS_DIRS; do
> > +        sudo rm -rf --one-file-system "$TMPDIR$i"
> > +        mkdir -p "$TMPDIR$i"
> > +    done
> >  }
> >
> >  python() {
> > @@ -148,8 +150,10 @@ python() {
> >                      )
> >
> >                  ws = re.match(r"^\s*", d.getVar(e, False)).group()
> > +                # remove prefix ${TMPDIR}, so we don't have absolute paths
> in
> > variable e
> > +                dirs = [dir[len(tmpdir):] for dir in rcleandirs]
> >                  d.prependVar(
> > -                    e, cleandir_code.format(ws=ws, dirlist="
> ".join(rcleandirs))
> > +                    e, cleandir_code.format(ws=ws, dirlist="
> > + ".join(dirs))
> >                  )
> >  }
> >
> > --
> > 2.30.2
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "isar-users" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to isar-users+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.g
> > oogle.com%2Fd%2Fmsgid%2Fisar-users%2F20220404111613.2159786-5-
> > adriaan.schmidt%2540siemens.com&amp;data=04%7C01%7Cfelix.moessbauer
> > %40siemens.com%7C59454b1a80a746e5e34008da162c9418%7C38ae3bcd9579
> > 4fd4addab42e1495d55a%7C1%7C0%7C637846678701794447%7CUnknown%7C
> > TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> > VCI6Mn0%3D%7C2000&amp;sdata=eqpc14UYg7%2FH4z2xJnu%2F3GLthvMv67s
> > Xj4NLt65eIR0%3D&amp;reserved=0.