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 |
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] > -=-=-=-=-=-=-=-=-=-=-=- > >
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
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
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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.
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 --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
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(-)