[PATCHv2 0/2] sstate bug fix

[PATCHv2 0/2] sstate bug fix

[Henning Schild]

changes since v1:
 - add p2
 - fix typo in p1
 - move code around in p1, later touched again in p2

This is just a single patch but i think it fixes bugs and should be
considered before a release ... i usually do not do cover letters for
single patches. So here i still do it to point out that it might be
important.


Henning Schild (2):
  sstate: change rootfs task to cache
  sstate: fix task order and deps

 meta/classes/rootfs.bbclass | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

-- 
2.34.1

[PATCHv2 1/2] sstate: change rootfs task to cache

[Henning Schild]

This patch moves sstate caching from the task "do_rootfs" to
"do_rootfs_install". Which is earlier in the task chain.

There are two "bugs" that we fix by doing so, while hopefully not
giving up on too much caching speed.
 - rootfs_postprocess:
   - image_postprocess_mark: potentially outdated BUILD_ID and friends
   - rootfs_generate_manifest: manifest file gone missing in DEPLOY_DIR

Since rootfs_postprocess is kind of an open field it might also fix
issues in ROOTFS_POSTPROCESS_COMMAND functions in layers. Any function
deriving stuff (like a manifest) or changing stuff (like the mark) could
potentially benefit from "not caching".

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 meta/classes/rootfs.bbclass | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index 1fc487896f4d..78fb2cc71350 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -270,29 +270,28 @@ python do_rootfs() {
 }
 addtask rootfs before do_build
 
-do_rootfs[depends] = "base-apt:do_cache isar-apt:do_cache_config"
-
-SSTATETASKS += "do_rootfs"
+SSTATETASKS += "do_rootfs_install"
 ROOTFS_SSTATE = "${WORKDIR}/rootfs-sstate"
-do_rootfs[dirs] += "${ROOTFS_SSTATE} ${WORKDIR}/mnt/rootfs"
-do_rootfs[cleandirs] += "${ROOTFS_SSTATE}"
-do_rootfs[sstate-plaindirs] = "${ROOTFS_SSTATE}"
-do_rootfs[sstate-interceptfuncs] = "rootfs_sstate_prepare"
+do_rootfs_install[depends] += " base-apt:do_cache isar-apt:do_cache_config"
+do_rootfs_install[dirs] += "${ROOTFS_SSTATE} ${WORKDIR}/mnt/rootfs"
+do_rootfs_install[cleandirs] += "${ROOTFS_SSTATE}"
+do_rootfs_install[sstate-plaindirs] = "${ROOTFS_SSTATE}"
+do_rootfs_install[sstate-interceptfuncs] = "rootfs_install_sstate_prepare"
 
 # the buildchroot is owned by root, so we need some sudoing to pack and unpack
-rootfs_sstate_prepare() {
+rootfs_install_sstate_prepare() {
     sudo mount --bind ${WORKDIR}/rootfs ${WORKDIR}/mnt/rootfs -o ro
     sudo tar -C ${WORKDIR}/mnt -cpf ${ROOTFS_SSTATE}/rootfs.tar --one-file-system rootfs
     sudo umount ${WORKDIR}/mnt/rootfs
 }
-do_rootfs_sstate_prepare[lockfiles] = "${REPO_ISAR_DIR}/isar.lock"
+do_rootfs_install_sstate_prepare[lockfiles] = "${REPO_ISAR_DIR}/isar.lock"
 
-rootfs_sstate_finalize() {
+rootfs_install_sstate_finalize() {
     sudo tar -C ${WORKDIR} -xpf ${ROOTFS_SSTATE}/rootfs.tar
 }
 
-python do_rootfs_setscene() {
+python do_rootfs_install_setscene() {
     sstate_setscene(d)
-    bb.build.exec_func('rootfs_sstate_finalize', d)
+    bb.build.exec_func('rootfs_install_sstate_finalize', d)
 }
-addtask do_rootfs_setscene
+addtask do_rootfs_install_setscene
-- 
2.34.1

[PATCHv2 2/2] sstate: fix task order and deps

[Henning Schild]

With do_rootfs_install being the new sstate task we need to be more
precise on what needs to happen before do_rootfs_postprocess.

Some postprocess functions need files downloaded so we add "after unpack"
 - sdkchroot_install_files
 - buildchroot_install_files
And do_rootfs_postprocess needs the apt caches for rootfs_do_mounts.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 meta/classes/rootfs.bbclass | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index 78fb2cc71350..9f0d345dae03 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -262,7 +262,7 @@ python do_rootfs_postprocess() {
         bb.build.exec_func(cmd, d)
         progress_reporter.update(int(i / len(cmds) * 100))
 }
-addtask rootfs_postprocess before do_rootfs
+addtask rootfs_postprocess before do_rootfs after do_unpack
 
 python do_rootfs() {
     """Virtual task"""
@@ -270,9 +270,10 @@ python do_rootfs() {
 }
 addtask rootfs before do_build
 
+do_rootfs_postprocess[depends] = "base-apt:do_cache isar-apt:do_cache_config"
+
 SSTATETASKS += "do_rootfs_install"
 ROOTFS_SSTATE = "${WORKDIR}/rootfs-sstate"
-do_rootfs_install[depends] += " base-apt:do_cache isar-apt:do_cache_config"
 do_rootfs_install[dirs] += "${ROOTFS_SSTATE} ${WORKDIR}/mnt/rootfs"
 do_rootfs_install[cleandirs] += "${ROOTFS_SSTATE}"
 do_rootfs_install[sstate-plaindirs] = "${ROOTFS_SSTATE}"
-- 
2.34.1

Re: [PATCHv2 2/2] sstate: fix task order and deps

[Henning Schild]

Am Fri, 28 Jan 2022 10:00:10 +0100
schrieb Henning Schild <henning.schild@siemens.com>:

> With do_rootfs_install being the new sstate task we need to be more
> precise on what needs to happen before do_rootfs_postprocess.
> 
> Some postprocess functions need files downloaded so we add "after
> unpack"
>  - sdkchroot_install_files
>  - buildchroot_install_files
> And do_rootfs_postprocess needs the apt caches for rootfs_do_mounts.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  meta/classes/rootfs.bbclass | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index 78fb2cc71350..9f0d345dae03 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -262,7 +262,7 @@ python do_rootfs_postprocess() {
>          bb.build.exec_func(cmd, d)
>          progress_reporter.update(int(i / len(cmds) * 100))
>  }
> -addtask rootfs_postprocess before do_rootfs
> +addtask rootfs_postprocess before do_rootfs after do_unpack
>  
>  python do_rootfs() {
>      """Virtual task"""
> @@ -270,9 +270,10 @@ python do_rootfs() {
>  }
>  addtask rootfs before do_build
>  
> +do_rootfs_postprocess[depends] = "base-apt:do_cache
> isar-apt:do_cache_config" +

i do understand why this needs to be here, and it is in the commit
message.

>  SSTATETASKS += "do_rootfs_install"
>  ROOTFS_SSTATE = "${WORKDIR}/rootfs-sstate"
> -do_rootfs_install[depends] += " base-apt:do_cache
> isar-apt:do_cache_config" do_rootfs_install[dirs] +=

However i do not fully understand why it seems ok to drop this again.
All test i did so far look promising. And i rather not have deps
overspecified, that is why i drop it here.

Henning

> "${ROOTFS_SSTATE} ${WORKDIR}/mnt/rootfs" do_rootfs_install[cleandirs]
> += "${ROOTFS_SSTATE}" do_rootfs_install[sstate-plaindirs] =
> "${ROOTFS_SSTATE}"