| Message ID | 46e8b3a2-128f-c63a-cbfc-6d38a1792a35@siemens.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [isar-cip-core] squashfs: Fix vardeps for SQUASHFS_THREADS and SQUASHFS_MEMLIMIT | expand |
On Sat, 2023-05-27 at 18:47 +0200, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > While SQUASHFS_DEFAULTS was decoupled from its potentially unstable > inputs, SQUASHFS_THREADS and SQUASHFS_MEMLIMIT, the consumer of > SQUASHFS_DEFAULTS was not. Rework this moving the vardepsexclude to > IMAGE_CMD:squashfs. This also obsoletes the related vardepvalue and > vardepsexclude for SQUASHFS_THREADS and SQUASHFS_MEMLIMIT. Why was this required? The consumer does not expand the variables and by that changes of e.g SQUASHFS_THREADS should not affect the hashes. > > As we are now expecting only generation limits via that ignored > variable, rename it to SQUASHFS_CREATION_LIMITS and deny overwriting. > Only SQUASHFS_THREADS and SQUASHFS_MEMLIMIT are supposed to be tuned > to the build environment - if at all. I'm not against this change, but I don't understand what issue is solved here. Felix > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > classes/squashfs.bbclass | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/classes/squashfs.bbclass b/classes/squashfs.bbclass > index b14768c2..1f2a7595 100644 > --- a/classes/squashfs.bbclass > +++ b/classes/squashfs.bbclass > @@ -1,7 +1,7 @@ > # > # CIP Core, generic profile > # > -# Copyright (c) Siemens AG, 2021-2022 > +# Copyright (c) Siemens AG, 2021-2023 > # > # Authors: > # Quirin Gylstorff <quirin.gylstorff@siemens.com> > @@ -16,11 +16,9 @@ SQUASHFS_CONTENT ?= "${PP_ROOTFS}" > SQUASHFS_CREATION_ARGS ?= "" > > SQUASHFS_THREADS ?= "${@oe.utils.cpu_count(at_least=2)}" > -SQUASHFS_THREADS[vardepvalue] = "1" > # default according to mksquasfs docs > SQUASHFS_MEMLIMIT ?= "7982M" > -SQUASHFS_DEFAULTS ?= "-mem ${SQUASHFS_MEMLIMIT} -processors > ${SQUASHFS_THREADS}" > -SQUASHFS_DEFAULTS[vardepsexclude] += "SQUASHFS_MEMLIMIT > SQUASHFS_THREADS" > +SQUASHFS_CREATION_LIMITS = "-mem ${SQUASHFS_MEMLIMIT} -processors > ${SQUASHFS_THREADS}" > > python __anonymous() { > exclude_directories = d.getVar('SQUASHFS_EXCLUDE_DIRS').split() > @@ -35,8 +33,9 @@ python __anonymous() { > } > > IMAGE_CMD:squashfs[depends] = "${PN}:do_transform_template" > +IMAGE_CMD:squashfs[vardepsexclude] += "SQUASHFS_CREATION_LIMITS" > IMAGE_CMD:squashfs() { > ${SUDO_CHROOT} /bin/mksquashfs \ > '${SQUASHFS_CONTENT}' '${IMAGE_FILE_CHROOT}' \ > - -noappend ${SQUASHFS_DEFAULTS} ${SQUASHFS_CREATION_ARGS} > + -noappend ${SQUASHFS_CREATION_LIMITS} > ${SQUASHFS_CREATION_ARGS} > }
On 29.05.23 12:24, MOESSBAUER, Felix (T CED INW-CN) wrote: > On Sat, 2023-05-27 at 18:47 +0200, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> While SQUASHFS_DEFAULTS was decoupled from its potentially unstable >> inputs, SQUASHFS_THREADS and SQUASHFS_MEMLIMIT, the consumer of >> SQUASHFS_DEFAULTS was not. Rework this moving the vardepsexclude to >> IMAGE_CMD:squashfs. This also obsoletes the related vardepvalue and >> vardepsexclude for SQUASHFS_THREADS and SQUASHFS_MEMLIMIT. > > Why was this required? The consumer does not expand the variables and > by that changes of e.g SQUASHFS_THREADS should not affect the hashes. > >> >> As we are now expecting only generation limits via that ignored >> variable, rename it to SQUASHFS_CREATION_LIMITS and deny overwriting. >> Only SQUASHFS_THREADS and SQUASHFS_MEMLIMIT are supposed to be tuned >> to the build environment - if at all. > > I'm not against this change, but I don't understand what issue is > solved here. The current code broke as soon as patch 2 ("Calculate a smarter default SQUASHFS_MEMLIMIT") got applied and actually brought variation into the variables, and that already during runtime (changing free memory -> unstable task hashes). I'm sure you can reproduce the issue as well by varying the number of available CPUs between builds. Jan
diff --git a/classes/squashfs.bbclass b/classes/squashfs.bbclass index b14768c2..1f2a7595 100644 --- a/classes/squashfs.bbclass +++ b/classes/squashfs.bbclass @@ -1,7 +1,7 @@ # # CIP Core, generic profile # -# Copyright (c) Siemens AG, 2021-2022 +# Copyright (c) Siemens AG, 2021-2023 # # Authors: # Quirin Gylstorff <quirin.gylstorff@siemens.com> @@ -16,11 +16,9 @@ SQUASHFS_CONTENT ?= "${PP_ROOTFS}" SQUASHFS_CREATION_ARGS ?= "" SQUASHFS_THREADS ?= "${@oe.utils.cpu_count(at_least=2)}" -SQUASHFS_THREADS[vardepvalue] = "1" # default according to mksquasfs docs SQUASHFS_MEMLIMIT ?= "7982M" -SQUASHFS_DEFAULTS ?= "-mem ${SQUASHFS_MEMLIMIT} -processors ${SQUASHFS_THREADS}" -SQUASHFS_DEFAULTS[vardepsexclude] += "SQUASHFS_MEMLIMIT SQUASHFS_THREADS" +SQUASHFS_CREATION_LIMITS = "-mem ${SQUASHFS_MEMLIMIT} -processors ${SQUASHFS_THREADS}" python __anonymous() { exclude_directories = d.getVar('SQUASHFS_EXCLUDE_DIRS').split() @@ -35,8 +33,9 @@ python __anonymous() { } IMAGE_CMD:squashfs[depends] = "${PN}:do_transform_template" +IMAGE_CMD:squashfs[vardepsexclude] += "SQUASHFS_CREATION_LIMITS" IMAGE_CMD:squashfs() { ${SUDO_CHROOT} /bin/mksquashfs \ '${SQUASHFS_CONTENT}' '${IMAGE_FILE_CHROOT}' \ - -noappend ${SQUASHFS_DEFAULTS} ${SQUASHFS_CREATION_ARGS} + -noappend ${SQUASHFS_CREATION_LIMITS} ${SQUASHFS_CREATION_ARGS} }