diff mbox series

kernel: Don't fail if Modules.symvers doesn't exist

Message ID 20230802114705.90137-1-joel@jms.id.au
State New
Headers show
Series kernel: Don't fail if Modules.symvers doesn't exist | expand

Commit Message

Joel Stanley Aug. 2, 2023, 11:47 a.m. UTC 
The one liner tests using `[ ] && action` need to have || true after the action,
otherwise the line returns false and the recipie fails.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 meta/classes-recipe/kernel.bbclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Frédéric Martinsons Aug. 2, 2023, 12:45 p.m. UTC | #1 
On Wed, 2 Aug 2023 at 13:47, Joel Stanley <joel@jms.id.au> wrote:

> The one liner tests using `[ ] && action` need to have || true after the
> action,
> otherwise the line returns false and the recipie fails.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  meta/classes-recipe/kernel.bbclass | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/meta/classes-recipe/kernel.bbclass
> b/meta/classes-recipe/kernel.bbclass
> index 247ef4a48aa7..f1bc41e82be8 100644
> --- a/meta/classes-recipe/kernel.bbclass
> +++ b/meta/classes-recipe/kernel.bbclass
> @@ -486,7 +486,7 @@ kernel_do_install() {
>         install -m 0644 System.map
> ${D}/${KERNEL_IMAGEDEST}/System.map-${KERNEL_VERSION}
>         install -m 0644 .config
> ${D}/${KERNEL_IMAGEDEST}/config-${KERNEL_VERSION}
>         install -m 0644 vmlinux
> ${D}/${KERNEL_IMAGEDEST}/vmlinux-${KERNEL_VERSION}
> -       [ -e Module.symvers ] && install -m 0644 Module.symvers
> ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
> +       [ -e Module.symvers ] && install -m 0644 Module.symvers
> ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION} || true
>  }
>
>
By doing that, you simply silent possible error of the action after the
test, below is a little snippet showing that

 fmartinsons@work-pc:~$ touch foo
fmartinsons@work-pc:~$ [ -e foo ] && ls non_existent_file
ls: cannot access 'non_existent_file': No such file or directory
fmartinsons@work-pc:~$ echo $?
2
fmartinsons@work-pc:~$ [ -e foo ] && ls non_existent_file || true
ls: cannot access 'non_existent_file': No such file or directory
fmartinsons@work-pc:~$ echo $?
0
fmartinsons@work-pc:~$ touch existent_file
fmartinsons@work-pc:~$ [ -e foo ] && ls existent_file
existent_file
fmartinsons@work-pc:~$ echo $?
0
fmartinsons@work-pc:~$

I think the origin of your issue is that the action failed , having the log
of kernel_do_install task would help.

  # Must be ran no earlier than after do_kernel_checkout or else Makefile
> won't be in ${S}/Makefile
> @@ -555,7 +555,7 @@ do_shared_workdir () {
>
>         # Copy files required for module builds
>         cp System.map $kerneldir/System.map-${KERNEL_VERSION}
> -       [ -e Module.symvers ] && cp Module.symvers $kerneldir/
> +       [ -e Module.symvers ] && cp Module.symvers $kerneldir/ || true
>         cp .config $kerneldir/
>         mkdir -p $kerneldir/include/config
>         cp include/config/kernel.release
> $kerneldir/include/config/kernel.release
> --
> 2.40.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#185386):
> https://lists.openembedded.org/g/openembedded-core/message/185386
> Mute This Topic: https://lists.openembedded.org/mt/100503145/6213388
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> frederic.martinsons@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Joel Stanley Aug. 2, 2023, 1:15 p.m. UTC | #2 
On Wed, 2 Aug 2023 at 12:45, Frédéric Martinsons
<frederic.martinsons@gmail.com> wrote:
>
>
>
> On Wed, 2 Aug 2023 at 13:47, Joel Stanley <joel@jms.id.au> wrote:
>>
>> The one liner tests using `[ ] && action` need to have || true after the action,
>> otherwise the line returns false and the recipie fails.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> ---
>>  meta/classes-recipe/kernel.bbclass | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
>> index 247ef4a48aa7..f1bc41e82be8 100644
>> --- a/meta/classes-recipe/kernel.bbclass
>> +++ b/meta/classes-recipe/kernel.bbclass
>> @@ -486,7 +486,7 @@ kernel_do_install() {
>>         install -m 0644 System.map ${D}/${KERNEL_IMAGEDEST}/System.map-${KERNEL_VERSION}
>>         install -m 0644 .config ${D}/${KERNEL_IMAGEDEST}/config-${KERNEL_VERSION}
>>         install -m 0644 vmlinux ${D}/${KERNEL_IMAGEDEST}/vmlinux-${KERNEL_VERSION}
>> -       [ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
>> +       [ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION} || true
>>  }
>>
>
> By doing that, you simply silent possible error of the action after the test, below is a little snippet showing that

Sure. But the test is looking for the file, so if it doesn't exist
then the command install won't run.

I can't think of any realistic situation where install would fail if
the file is present.

> I think the origin of your issue is that the action failed , having the log of kernel_do_install task would help.

That isn't the case.

The issue is the Modules.symvers file is not found. This is expected,
because the kernel does not use modules.

Here is the log, with paths trimmed:

DEBUG: Executing shell function do_install
NOTE: no modules to install
WARNING: temp/run.do_install.2584671:225 exit 1 from '[ -e Module.symvers ]'
WARNING: Backtrace (BB generated script):
    #1: do_install, temp/run.do_install.2584671, line 225
    #2: main, temp/run.do_install.2584671, line 283
Mikko Rapeli Aug. 2, 2023, 1:25 p.m. UTC | #3 
Hi,

On Wed, Aug 02, 2023 at 01:15:34PM +0000, Joel Stanley wrote:
> On Wed, 2 Aug 2023 at 12:45, Fr�d�ric Martinsons
> <frederic.martinsons@gmail.com> wrote:
> >
> >
> >
> > On Wed, 2 Aug 2023 at 13:47, Joel Stanley <joel@jms.id.au> wrote:
> >>
> >> The one liner tests using `[ ] && action` need to have || true after the action,
> >> otherwise the line returns false and the recipie fails.
> >>
> >> Signed-off-by: Joel Stanley <joel@jms.id.au>
> >> ---
> >>  meta/classes-recipe/kernel.bbclass | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
> >> index 247ef4a48aa7..f1bc41e82be8 100644
> >> --- a/meta/classes-recipe/kernel.bbclass
> >> +++ b/meta/classes-recipe/kernel.bbclass
> >> @@ -486,7 +486,7 @@ kernel_do_install() {
> >>         install -m 0644 System.map ${D}/${KERNEL_IMAGEDEST}/System.map-${KERNEL_VERSION}
> >>         install -m 0644 .config ${D}/${KERNEL_IMAGEDEST}/config-${KERNEL_VERSION}
> >>         install -m 0644 vmlinux ${D}/${KERNEL_IMAGEDEST}/vmlinux-${KERNEL_VERSION}
> >> -       [ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
> >> +       [ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION} || true
> >>  }
> >>
> >
> > By doing that, you simply silent possible error of the action after the test, below is a little snippet showing that
> 
> Sure. But the test is looking for the file, so if it doesn't exist
> then the command install won't run.
> 
> I can't think of any realistic situation where install would fail if
> the file is present.
> 
> > I think the origin of your issue is that the action failed , having the log of kernel_do_install task would help.
> 
> That isn't the case.
> 
> The issue is the Modules.symvers file is not found. This is expected,
> because the kernel does not use modules.

This crucial bit of information is not in the commit message, please add it.

Then, I think there are better ways to detect if kernel modules are disabled
completely. See for example kernel.bbclass line 430:

if (grep -q -i -e '^CONFIG_MODULES=y$' ${B}/.config); then

Cheers,

-Mikko
Bruce Ashfield Aug. 2, 2023, 1:38 p.m. UTC | #4 
On Wed, Aug 2, 2023 at 9:15 AM Joel Stanley <joel@jms.id.au> wrote:
>
> On Wed, 2 Aug 2023 at 12:45, Frédéric Martinsons
> <frederic.martinsons@gmail.com> wrote:
> >
> >
> >
> > On Wed, 2 Aug 2023 at 13:47, Joel Stanley <joel@jms.id.au> wrote:
> >>
> >> The one liner tests using `[ ] && action` need to have || true after the action,
> >> otherwise the line returns false and the recipie fails.
> >>
> >> Signed-off-by: Joel Stanley <joel@jms.id.au>
> >> ---
> >>  meta/classes-recipe/kernel.bbclass | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
> >> index 247ef4a48aa7..f1bc41e82be8 100644
> >> --- a/meta/classes-recipe/kernel.bbclass
> >> +++ b/meta/classes-recipe/kernel.bbclass
> >> @@ -486,7 +486,7 @@ kernel_do_install() {
> >>         install -m 0644 System.map ${D}/${KERNEL_IMAGEDEST}/System.map-${KERNEL_VERSION}
> >>         install -m 0644 .config ${D}/${KERNEL_IMAGEDEST}/config-${KERNEL_VERSION}
> >>         install -m 0644 vmlinux ${D}/${KERNEL_IMAGEDEST}/vmlinux-${KERNEL_VERSION}
> >> -       [ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
> >> +       [ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION} || true
> >>  }
> >>
> >
> > By doing that, you simply silent possible error of the action after the test, below is a little snippet showing that
>
> Sure. But the test is looking for the file, so if it doesn't exist
> then the command install won't run.

Something doesn't seem quite right, that test for Module.symvers (there
and in the shared workdir code) were put in place for the scenario
you are mentioning .. if Modules are disabled and hence Module.symvers doesn't
exist.

At one point, we even tested that sort of configuration.

I didn't dig more through the logs, but maybe we had lines after that command
previously, so the last return code wasn't a failure and hence no task
failure.

As was mentioned, we could make the test more explicit if the shorthand of
just checking for the symvers file isn't working, since really, if we have an
|| true in the one-liner, we probably can tweak things to not test for
Module.symvers
at all.

Bruce


>
> I can't think of any realistic situation where install would fail if
> the file is present.
>
> > I think the origin of your issue is that the action failed , having the log of kernel_do_install task would help.
>
> That isn't the case.
>
> The issue is the Modules.symvers file is not found. This is expected,
> because the kernel does not use modules.
>
> Here is the log, with paths trimmed:
>
> DEBUG: Executing shell function do_install
> NOTE: no modules to install
> WARNING: temp/run.do_install.2584671:225 exit 1 from '[ -e Module.symvers ]'
> WARNING: Backtrace (BB generated script):
>     #1: do_install, temp/run.do_install.2584671, line 225
>     #2: main, temp/run.do_install.2584671, line 283
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#185391): https://lists.openembedded.org/g/openembedded-core/message/185391
> Mute This Topic: https://lists.openembedded.org/mt/100503145/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Andrew Geissler Aug. 2, 2023, 4:08 p.m. UTC | #5 
On Wed, Aug 2, 2023 at 06:38 AM, Bruce Ashfield wrote:

> 
> On Wed, Aug 2, 2023 at 9:15 AM Joel Stanley <joel@jms.id.au> wrote:
> 
>> On Wed, 2 Aug 2023 at 12:45, Frédéric Martinsons
>> <frederic.martinsons@gmail.com> wrote:
>> 
>>> 
>>> 
>>> On Wed, 2 Aug 2023 at 13:47, Joel Stanley <joel@jms.id.au> wrote:
>>> 
>>>> The one liner tests using `[ ] && action` need to have || true after the
>>>> action,
>>>> otherwise the line returns false and the recipie fails.
>>>> 
>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>> ---
>>>> meta/classes-recipe/kernel.bbclass | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/meta/classes-recipe/kernel.bbclass
>>>> b/meta/classes-recipe/kernel.bbclass
>>>> index 247ef4a48aa7..f1bc41e82be8 100644
>>>> --- a/meta/classes-recipe/kernel.bbclass
>>>> +++ b/meta/classes-recipe/kernel.bbclass
>>>> @@ -486,7 +486,7 @@ kernel_do_install() {
>>>> install -m 0644 System.map
>>>> ${D}/${KERNEL_IMAGEDEST}/System.map-${KERNEL_VERSION}
>>>> install -m 0644 .config ${D}/${KERNEL_IMAGEDEST}/config-${KERNEL_VERSION}
>>>> install -m 0644 vmlinux ${D}/${KERNEL_IMAGEDEST}/vmlinux-${KERNEL_VERSION}
>>>> 
>>>> - [ -e Module.symvers ] && install -m 0644 Module.symvers
>>>> ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
>>>> + [ -e Module.symvers ] && install -m 0644 Module.symvers
>>>> ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION} || true
>>>> }
>>> 
>>> By doing that, you simply silent possible error of the action after the
>>> test, below is a little snippet showing that
>> 
>> Sure. But the test is looking for the file, so if it doesn't exist
>> then the command install won't run.
> 
> Something doesn't seem quite right, that test for Module.symvers (there
> and in the shared workdir code) were put in place for the scenario
> you are mentioning .. if Modules are disabled and hence Module.symvers
> doesn't
> exist.
> 
> At one point, we even tested that sort of configuration.
> 
> I didn't dig more through the logs, but maybe we had lines after that
> command
> previously, so the last return code wasn't a failure and hence no task
> failure.

Yep, this is what happened, there used to be these 2 lines after it:

install -d ${D}${sysconfdir}/modprobe.d

install -d ${D}${sysconfdir}/modules-load.d

https://gerrit.openbmc.org/c/openbmc/openbmc/+/65313/3/poky/meta/classes-recipe/kernel.bbclass

Once these were removed we started hitting this failure over in OpenBMC.
Jose Quaresma Aug. 2, 2023, 5:08 p.m. UTC | #6 
Andrew Geissler <geissonator@gmail.com> escreveu no dia quarta, 2/08/2023
à(s) 17:08:

> On Wed, Aug 2, 2023 at 06:38 AM, Bruce Ashfield wrote:
>
> On Wed, Aug 2, 2023 at 9:15 AM Joel Stanley <joel@jms.id.au> wrote:
>
> On Wed, 2 Aug 2023 at 12:45, Frédéric Martinsons
> <frederic.martinsons@gmail.com> wrote:
>
>
>
> On Wed, 2 Aug 2023 at 13:47, Joel Stanley <joel@jms.id.au> wrote:
>
> The one liner tests using `[ ] && action` need to have || true after the
> action,
> otherwise the line returns false and the recipie fails.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> meta/classes-recipe/kernel.bbclass | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/meta/classes-recipe/kernel.bbclass
> b/meta/classes-recipe/kernel.bbclass
> index 247ef4a48aa7..f1bc41e82be8 100644
> --- a/meta/classes-recipe/kernel.bbclass
> +++ b/meta/classes-recipe/kernel.bbclass
> @@ -486,7 +486,7 @@ kernel_do_install() {
> install -m 0644 System.map
> ${D}/${KERNEL_IMAGEDEST}/System.map-${KERNEL_VERSION}
> install -m 0644 .config ${D}/${KERNEL_IMAGEDEST}/config-${KERNEL_VERSION}
> install -m 0644 vmlinux ${D}/${KERNEL_IMAGEDEST}/vmlinux-${KERNEL_VERSION}
> - [ -e Module.symvers ] && install -m 0644 Module.symvers
> ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
> + [ -e Module.symvers ] && install -m 0644 Module.symvers
> ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION} || true
> }
>
> By doing that, you simply silent possible error of the action after the
> test, below is a little snippet showing that
>
> Sure. But the test is looking for the file, so if it doesn't exist
> then the command install won't run.
>
>
I think using an if solves the issue:

if [ -e Module.symvers ]; then
  install -m 0644 Module.symvers
${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
fi

or something to invert the logic:

! [ -e Module.symvers ] || install -m 0644 Module.symvers
${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}

Jose

Something doesn't seem quite right, that test for Module.symvers (there
> and in the shared workdir code) were put in place for the scenario
> you are mentioning .. if Modules are disabled and hence Module.symvers
> doesn't
> exist.
>
> At one point, we even tested that sort of configuration.
>
> I didn't dig more through the logs, but maybe we had lines after that
> command
> previously, so the last return code wasn't a failure and hence no task
> failure.
>
> Yep, this is what happened, there used to be these 2 lines after it:
>
> install -d ${D}${sysconfdir}/modprobe.d
> install -d ${D}${sysconfdir}/modules-load.d
>
>
>
> https://gerrit.openbmc.org/c/openbmc/openbmc/+/65313/3/poky/meta/classes-recipe/kernel.bbclass
>
> Once these were removed we started hitting this failure over in OpenBMC.
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#185414):
> https://lists.openembedded.org/g/openembedded-core/message/185414
> Mute This Topic: https://lists.openembedded.org/mt/100503145/5052612
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> quaresma.jose@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
diff mbox series

Patch

diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
index 247ef4a48aa7..f1bc41e82be8 100644
--- a/meta/classes-recipe/kernel.bbclass
+++ b/meta/classes-recipe/kernel.bbclass
@@ -486,7 +486,7 @@  kernel_do_install() {
 	install -m 0644 System.map ${D}/${KERNEL_IMAGEDEST}/System.map-${KERNEL_VERSION}
 	install -m 0644 .config ${D}/${KERNEL_IMAGEDEST}/config-${KERNEL_VERSION}
 	install -m 0644 vmlinux ${D}/${KERNEL_IMAGEDEST}/vmlinux-${KERNEL_VERSION}
-	[ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
+	[ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION} || true
 }
 
 # Must be ran no earlier than after do_kernel_checkout or else Makefile won't be in ${S}/Makefile
@@ -555,7 +555,7 @@  do_shared_workdir () {
 
 	# Copy files required for module builds
 	cp System.map $kerneldir/System.map-${KERNEL_VERSION}
-	[ -e Module.symvers ] && cp Module.symvers $kerneldir/
+	[ -e Module.symvers ] && cp Module.symvers $kerneldir/ || true
 	cp .config $kerneldir/
 	mkdir -p $kerneldir/include/config
 	cp include/config/kernel.release $kerneldir/include/config/kernel.release