diff mbox series

[v3,09/11] image_types_wic.bbclass: capture verbose wic output by default

Message ID 20250404162932.447699-10-mikko.rapeli@linaro.org
State New
Headers show
Series systemd based initrd and modular kernel support | expand

Commit Message

Mikko Rapeli April 4, 2025, 4:29 p.m. UTC
Call wic with --debug to capture logs from wic internals
so that it's clear which partitions get created and which
files get copied where. wic plugins contain for example
race conditions which don't install files at all and thus
images fail to boot and it's not possible to debug these without
something in wic task logs.

For example core-image-initramfs-boot do_image_wic
log is now 576 lines which is not excessive but very
important when debugging problems, especially race
conditions which are only hit in some builds in CI.

Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 meta/classes-recipe/image_types_wic.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Trevor Woerner April 14, 2025, 8:43 p.m. UTC | #1
On Fri 2025-04-04 @ 07:29:30 PM, Mikko Rapeli via lists.openembedded.org wrote:
> Call wic with --debug to capture logs from wic internals
> so that it's clear which partitions get created and which
> files get copied where. wic plugins contain for example
> race conditions which don't install files at all and thus
> images fail to boot and it's not possible to debug these without
> something in wic task logs.
> 
> For example core-image-initramfs-boot do_image_wic
> log is now 576 lines which is not excessive but very
> important when debugging problems, especially race
> conditions which are only hit in some builds in CI.
> 
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> ---
>  meta/classes-recipe/image_types_wic.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes-recipe/image_types_wic.bbclass b/meta/classes-recipe/image_types_wic.bbclass
> index 1b422b6280..10888bc12b 100644
> --- a/meta/classes-recipe/image_types_wic.bbclass
> +++ b/meta/classes-recipe/image_types_wic.bbclass
> @@ -72,7 +72,7 @@ IMAGE_CMD:wic () {
>  	if [ -z "$wks" ]; then
>  		bbfatal "No kickstart files from WKS_FILES were found: ${WKS_FILES}. Please set WKS_FILE or WKS_FILES appropriately."
>  	fi
> -	BUILDDIR="${TOPDIR}" PSEUDO_UNLOAD=1 wic create "$wks" --vars "${STAGING_DIR}/${MACHINE}/imgdata/" -e "${IMAGE_BASENAME}" -o "$build_wic/" -w "$tmp_wic" ${WIC_CREATE_EXTRA_ARGS}
> +	BUILDDIR="${TOPDIR}" PSEUDO_UNLOAD=1 wic create --debug "$wks" --vars "${STAGING_DIR}/${MACHINE}/imgdata/" -e "${IMAGE_BASENAME}" -o "$build_wic/" -w "$tmp_wic" ${WIC_CREATE_EXTRA_ARGS}
>  
>  	# look to see if the user specifies a custom imager
>  	IMAGER=direct

When I need to debug a wic issue, I add "--debug" to WIC_CREATE_EXTRA_ARGS in
my conf/local.conf, debug the issue, then submit the patch which modifies the
layer without any debugging remnants left behind. I can then either leave wic
debugging turned on, or turn it off for my builds as I see fit. Turning on
debugging by default for everyone using wic is not what we should be doing.
It will slow down builds, and will inflate log files... by miniscule amounts,
sure, but not the direction we should be taking.

If you feel there are messages that should be logged on every wic invocation
(on which I doubt there will be consensus) then they should be moved out from
behind a --debug flag. The whole point of a debug flag is to identify messages
that do not need to be logged on every build, but only under special
circumstances. Lots of software projects have debug flags; I can't think of any
that turn their debug flags on by default.
Mikko Rapeli April 15, 2025, 5:19 a.m. UTC | #2
Hi,

On Mon, Apr 14, 2025 at 04:43:58PM -0400, Trevor Woerner wrote:
> On Fri 2025-04-04 @ 07:29:30 PM, Mikko Rapeli via lists.openembedded.org wrote:
> > Call wic with --debug to capture logs from wic internals
> > so that it's clear which partitions get created and which
> > files get copied where. wic plugins contain for example
> > race conditions which don't install files at all and thus
> > images fail to boot and it's not possible to debug these without
> > something in wic task logs.
> > 
> > For example core-image-initramfs-boot do_image_wic
> > log is now 576 lines which is not excessive but very
> > important when debugging problems, especially race
> > conditions which are only hit in some builds in CI.
> > 
> > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> > ---
> >  meta/classes-recipe/image_types_wic.bbclass | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes-recipe/image_types_wic.bbclass b/meta/classes-recipe/image_types_wic.bbclass
> > index 1b422b6280..10888bc12b 100644
> > --- a/meta/classes-recipe/image_types_wic.bbclass
> > +++ b/meta/classes-recipe/image_types_wic.bbclass
> > @@ -72,7 +72,7 @@ IMAGE_CMD:wic () {
> >  	if [ -z "$wks" ]; then
> >  		bbfatal "No kickstart files from WKS_FILES were found: ${WKS_FILES}. Please set WKS_FILE or WKS_FILES appropriately."
> >  	fi
> > -	BUILDDIR="${TOPDIR}" PSEUDO_UNLOAD=1 wic create "$wks" --vars "${STAGING_DIR}/${MACHINE}/imgdata/" -e "${IMAGE_BASENAME}" -o "$build_wic/" -w "$tmp_wic" ${WIC_CREATE_EXTRA_ARGS}
> > +	BUILDDIR="${TOPDIR}" PSEUDO_UNLOAD=1 wic create --debug "$wks" --vars "${STAGING_DIR}/${MACHINE}/imgdata/" -e "${IMAGE_BASENAME}" -o "$build_wic/" -w "$tmp_wic" ${WIC_CREATE_EXTRA_ARGS}
> >  
> >  	# look to see if the user specifies a custom imager
> >  	IMAGER=direct
> 
> When I need to debug a wic issue, I add "--debug" to WIC_CREATE_EXTRA_ARGS in
> my conf/local.conf, debug the issue, then submit the patch which modifies the
> layer without any debugging remnants left behind. I can then either leave wic
> debugging turned on, or turn it off for my builds as I see fit. Turning on
> debugging by default for everyone using wic is not what we should be doing.
> It will slow down builds, and will inflate log files... by miniscule amounts,
> sure, but not the direction we should be taking.

I just disagree. Default logging should not be excessive but logs should
show at high level what the task and tools executed there are doing.
wic logs without --debug are completely useless in understanding what
happened when image was created. wic logs with --debug are not excessive
but show which partitions got created and which files copied where.
Full rootfs content is not shown so logging is not excessive.

> If you feel there are messages that should be logged on every wic invocation
> (on which I doubt there will be consensus) then they should be moved out from
> behind a --debug flag. The whole point of a debug flag is to identify messages
> that do not need to be logged on every build, but only under special
> circumstances. Lots of software projects have debug flags; I can't think of any
> that turn their debug flags on by default.

I want to see which files wic copies and where, e.g. ESP boot partition, and
which partitions it created. Without --debug wic does not log these.

Some people want all logs to be empty by default, I want them to be usable
when things go wrong in builds which I did not start or even fully control.

Cheers,

-Mikko
Alexander Kanavin April 22, 2025, 2:25 p.m. UTC | #3
On Tue, 15 Apr 2025 at 07:20, Mikko Rapeli via lists.openembedded.org
<mikko.rapeli=linaro.org@lists.openembedded.org> wrote:

> Some people want all logs to be empty by default, I want them to be usable
> when things go wrong in builds which I did not start or even fully control.

Then maybe some of the debug messages should have their level raised to info?

Alex
diff mbox series

Patch

diff --git a/meta/classes-recipe/image_types_wic.bbclass b/meta/classes-recipe/image_types_wic.bbclass
index 1b422b6280..10888bc12b 100644
--- a/meta/classes-recipe/image_types_wic.bbclass
+++ b/meta/classes-recipe/image_types_wic.bbclass
@@ -72,7 +72,7 @@  IMAGE_CMD:wic () {
 	if [ -z "$wks" ]; then
 		bbfatal "No kickstart files from WKS_FILES were found: ${WKS_FILES}. Please set WKS_FILE or WKS_FILES appropriately."
 	fi
-	BUILDDIR="${TOPDIR}" PSEUDO_UNLOAD=1 wic create "$wks" --vars "${STAGING_DIR}/${MACHINE}/imgdata/" -e "${IMAGE_BASENAME}" -o "$build_wic/" -w "$tmp_wic" ${WIC_CREATE_EXTRA_ARGS}
+	BUILDDIR="${TOPDIR}" PSEUDO_UNLOAD=1 wic create --debug "$wks" --vars "${STAGING_DIR}/${MACHINE}/imgdata/" -e "${IMAGE_BASENAME}" -o "$build_wic/" -w "$tmp_wic" ${WIC_CREATE_EXTRA_ARGS}
 
 	# look to see if the user specifies a custom imager
 	IMAGER=direct