diff mbox series

[master,kirkstone,V2] kernel.bbclass: make KERNEL_DEBUG_TIMESTAMPS work at rebuild

Message ID 20221117041155.22632-1-Qi.Chen@windriver.com
State Accepted, archived
Commit 1b68c2d2d385013a1c535ef81172494302a36d74
Headers show
Series [master,kirkstone,V2] kernel.bbclass: make KERNEL_DEBUG_TIMESTAMPS work at rebuild | expand

Commit Message

ChenQi Nov. 17, 2022, 4:11 a.m. UTC
Currently, the KERNEL_DEBUG_TIMESTAMPS is not working as expected
at rebuild. That is, even if we set it to "1", the kernel build time
is not changed. The problem could be reproduced by the following steps.
  1. bitbake core-image-minimal; start image and check `uname -a` output.
  2. set in local.conf: KERNEL_DEBUG_TIMESTAMPS = "1"
  3. bitbake core-image-minimal; start image and check `uname -a` output.

It's expected that after enabling KERNEL_DEBUG_TIMESTAMPS, the kernel
build time will be set to current date. But it's not. This is because
the compile.h was not re-generated when do_compile task was re-executed.

In mkcompile_h, we have:
"""
 # Only replace the real compile.h if the new one is different,
 # in order to preserve the timestamp and avoid unnecessary
 # recompilations.
 # We don't consider the file changed if only the date/time changed,
 # unless KBUILD_BUILD_TIMESTAMP was explicitly set (e.g. for
 # reproducible builds with that value referring to a commit timestamp).
 # A kernel config change will increase the generation number, thus
 # causing compile.h to be updated (including date/time) due to the
 # changed comment in the
 # first line.
"""
It has made it very clear that it will not be re-generated unless
we have KBUILD_BUILD_TIMESTAMP set explicitly. So we set this variable
explicitly in do_compile to fix this issue.

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes-recipe/kernel.bbclass | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Jose Quaresma Nov. 18, 2022, 11:10 a.m. UTC | #1
Chen Qi <Qi.Chen@windriver.com> escreveu no dia quinta, 17/11/2022 à(s)
04:12:

> Currently, the KERNEL_DEBUG_TIMESTAMPS is not working as expected
> at rebuild. That is, even if we set it to "1", the kernel build time
> is not changed. The problem could be reproduced by the following steps.
>   1. bitbake core-image-minimal; start image and check `uname -a` output.
>   2. set in local.conf: KERNEL_DEBUG_TIMESTAMPS = "1"
>   3. bitbake core-image-minimal; start image and check `uname -a` output.
>
> It's expected that after enabling KERNEL_DEBUG_TIMESTAMPS, the kernel
> build time will be set to current date. But it's not. This is because
> the compile.h was not re-generated when do_compile task was re-executed.
>
> In mkcompile_h, we have:
> """
>  # Only replace the real compile.h if the new one is different,
>  # in order to preserve the timestamp and avoid unnecessary
>  # recompilations.
>  # We don't consider the file changed if only the date/time changed,
>  # unless KBUILD_BUILD_TIMESTAMP was explicitly set (e.g. for
>  # reproducible builds with that value referring to a commit timestamp).
>  # A kernel config change will increase the generation number, thus
>  # causing compile.h to be updated (including date/time) due to the
>  # changed comment in the
>  # first line.
> """
> It has made it very clear that it will not be re-generated unless
> we have KBUILD_BUILD_TIMESTAMP set explicitly. So we set this variable
> explicitly in do_compile to fix this issue.
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  meta/classes-recipe/kernel.bbclass | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/meta/classes-recipe/kernel.bbclass
> b/meta/classes-recipe/kernel.bbclass
> index 3834a42fb9..3f6b40907f 100644
> --- a/meta/classes-recipe/kernel.bbclass
> +++ b/meta/classes-recipe/kernel.bbclass
> @@ -367,6 +367,10 @@ kernel_do_compile() {
>                 export KBUILD_BUILD_TIMESTAMP="$ts"
>                 export KCONFIG_NOTIMESTAMP=1
>                 bbnote "KBUILD_BUILD_TIMESTAMP: $ts"
> +       else
> +               ts=`LC_ALL=C date`
> +               export KBUILD_BUILD_TIMESTAMP="$ts"
> +               bbnote "KBUILD_BUILD_TIMESTAMP: $ts"
>         fi
>         # The $use_alternate_initrd is only set from
>         # do_bundle_initramfs() This variable is specifically for the
> @@ -412,6 +416,10 @@ do_compile_kernelmodules() {
>                 export KBUILD_BUILD_TIMESTAMP="$ts"
>                 export KCONFIG_NOTIMESTAMP=1
>                 bbnote "KBUILD_BUILD_TIMESTAMP: $ts"
> +       else
> +               ts=`LC_ALL=C date`
> +               export KBUILD_BUILD_TIMESTAMP="$ts"
> +               bbnote "KBUILD_BUILD_TIMESTAMP: $ts"
>         fi
>         if (grep -q -i -e '^CONFIG_MODULES=y$' ${B}/.config); then
>                 oe_runmake -C ${B} ${PARALLEL_MAKE} modules
> ${KERNEL_EXTRA_ARGS}
> --
> 2.17.1
>

Hi Chen,

I think this only works on the first time you run it and after that if the
kernel can be taken from the sstate-cache this timestamp will be bypassed.
To generate a new timestamp you need to invalidate the stamp and run the
task again with:

bitbake virtual/linux -C do_compile

Another possible issue is this will make this task indeterministic (not
reproducible) as it uses the command 'date' that generates a
different output in each run.

IMO the bitbake DATETIME is better and it is deterministic:

export KBUILD_BUILD_TIMESTAMP="$DATETIME"

This also maybe introduces a new variable dependency exclusion on the tasks
that uses this variable:

kernel_do_compile[vardepsexclude] += DATETIME
do_compile_kernelmodules[vardepsexclude] += DATETIME

Jose


>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#173401):
> https://lists.openembedded.org/g/openembedded-core/message/173401
> Mute This Topic: https://lists.openembedded.org/mt/95083712/5052612
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> quaresma.jose@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
ChenQi Nov. 18, 2022, 2:06 p.m. UTC | #2
Hi Jose,

Thanks a lot for your review. I’d like to explain the case in more details.

* indeterministic (not reproducible) issue
The use case is: the user wants to disable reproducible build for kernel, and wants to get the actual build time of kernel. In other words, we want it to be indeterministic. This is actually what KERNEL_DEBUG_TIMESTAMPS is expected to do.

* state cache issue
I think if the kernel is taken from the sstate cache and not rebuilt, it’s acceptable. The timestamp in /proc/version (or `uname -a` output) is reflecting the actual kernel build time.
The situation that really confuses users is that when kernel do_compile is re-executed, the timestamp is not updated, even if we want it to (by enabling KERNEL_DEBUG_TIMESTAMPS).
It’s complained that even do_compile is forced to re-run, the timestamp is not updated.
In such case, I only see two choices:
1. delete the compile.h in do_compile (I have to admit that I didn’t try this method out)
2. set KBUILD_BUILD_TIMESTAMP explicitly
I chose the second one because I wanted to output the value of KBUILD_BUILD_TIMESTAMP just like when KERNEL_DEBUG_TIMESTAMPS is disabled.
Also, `date` will be run in one way or another. It will either be run in mkcompile_h or be run in do_compile.

* DATETIME
The DATETIME’s value is the time when bitbake.conf is parsed. By using DATETIME + varexclude, it’s basically achieving the same result as using `date` directly.
The ssate cache issue you mentioned remains the same, although I think it’s actually the expected behavior as explained above. And when source code/config changes and do_compile is re-run, KBUILD_BUILD_TIMESTAMP is set to a different value from the previous run.
The only difference is that DATETIME reflects the time bitbake.conf is parsed, and `date` reflects the time kernel’s do_compile is executed.

Does my explanation make some sense? If I still miss something, please let me know.
Thanks again for your review. Have a good weekend 
Jose Quaresma Nov. 22, 2022, 10:22 a.m. UTC | #3
Hi Qi,

Chen, Qi <Qi.Chen@windriver.com> escreveu no dia sexta, 18/11/2022 à(s)
14:06:

> Hi Jose,
>
>
>
> Thanks a lot for your review. I’d like to explain the case in more details.
>
>
>
> * indeterministic (not reproducible) issue
>
> The use case is: the user wants to disable reproducible build for kernel,
> and wants to get the actual build time of kernel. In other words, we want
> it to be indeterministic. This is actually what KERNEL_DEBUG_TIMESTAMPS is
> expected to do.
>

I don't fully understand the use case when looking at the patch as one of
the primary intentions is make it not reproducible.


>
>
> * state cache issue
>
> I think if the kernel is taken from the sstate cache and not rebuilt, it’s
> acceptable. The timestamp in /proc/version (or `uname -a` output) is
> reflecting the actual kernel build time.
>
> The situation that really confuses users is that when kernel do_compile is
> re-executed, the timestamp is not updated, even if we want it to (by
> enabling KERNEL_DEBUG_TIMESTAMPS).
>
> It’s complained that even do_compile is forced to re-run, the timestamp is
> not updated.
>

Understand so the main idea is to get the timestamp of the last do_compile
task running time even when they will come from the sstate, when it comes
from the sstate the timestamp is the same one that was taken when the
do_compile runs.


> In such case, I only see two choices:
>
> 1. delete the compile.h in do_compile (I have to admit that I didn’t try
> this method out)
>
> 2. set KBUILD_BUILD_TIMESTAMP explicitly
>
> I chose the second one because I wanted to output the value of
> KBUILD_BUILD_TIMESTAMP just like when KERNEL_DEBUG_TIMESTAMPS is disabled.
>
> Also, `date` will be run in one way or another. It will either be run in
> mkcompile_h or be run in do_compile.
>
>
>
> * DATETIME
>
> The DATETIME’s value is the time when bitbake.conf is parsed. By using
> DATETIME + varexclude, it’s basically achieving the same result as using
> `date` directly.
>
> The ssate cache issue you mentioned remains the same, although I think
> it’s actually the expected behavior as explained above. And when source
> code/config changes and do_compile is re-run, KBUILD_BUILD_TIMESTAMP is set
> to a different value from the previous run.
>
> The only difference is that DATETIME reflects the time bitbake.conf is
> parsed, and `date` reflects the time kernel’s do_compile is executed.
>

This suggestion is only to be more bitbake friendly and use a variable that
already exists, using `data` will provide the same and is maybe better in
this case as it does not need the varexclude.


>
>
> Does my explanation make some sense? If I still miss something, please let
> me know.
>

Thanks for explaining the full picture.

Best regards,
Jose


> Thanks again for your review. Have a good weekend 
diff mbox series

Patch

diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
index 3834a42fb9..3f6b40907f 100644
--- a/meta/classes-recipe/kernel.bbclass
+++ b/meta/classes-recipe/kernel.bbclass
@@ -367,6 +367,10 @@  kernel_do_compile() {
 		export KBUILD_BUILD_TIMESTAMP="$ts"
 		export KCONFIG_NOTIMESTAMP=1
 		bbnote "KBUILD_BUILD_TIMESTAMP: $ts"
+	else
+		ts=`LC_ALL=C date`
+		export KBUILD_BUILD_TIMESTAMP="$ts"
+		bbnote "KBUILD_BUILD_TIMESTAMP: $ts"
 	fi
 	# The $use_alternate_initrd is only set from
 	# do_bundle_initramfs() This variable is specifically for the
@@ -412,6 +416,10 @@  do_compile_kernelmodules() {
 		export KBUILD_BUILD_TIMESTAMP="$ts"
 		export KCONFIG_NOTIMESTAMP=1
 		bbnote "KBUILD_BUILD_TIMESTAMP: $ts"
+	else
+		ts=`LC_ALL=C date`
+		export KBUILD_BUILD_TIMESTAMP="$ts"
+		bbnote "KBUILD_BUILD_TIMESTAMP: $ts"
 	fi
 	if (grep -q -i -e '^CONFIG_MODULES=y$' ${B}/.config); then
 		oe_runmake -C ${B} ${PARALLEL_MAKE} modules ${KERNEL_EXTRA_ARGS}