Re: [OE-core] [PATCH] kernel: Use KERNEL_IMAGETYPE_FOR_MAKE in do_bundle_initramfs

2018-01-22 Thread Andre McCurdy
On Sun, Jan 21, 2018 at 2:02 AM, Richard Purdie
 wrote:
> On Wed, 2018-01-17 at 13:31 -0800, Andre McCurdy wrote:
>> On Wed, Jan 17, 2018 at 1:21 AM, Thomas Perrot > > wrote:
>> >
>> > In the case of a fitImage the this step should be done on the
>> > image use to assemble the fitImage.
>> >
>> > Signed-off-by: Thomas Perrot 
>> > ---
>> >  meta/classes/kernel.bbclass | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/meta/classes/kernel.bbclass
>> > b/meta/classes/kernel.bbclass
>> > index c42f85c665..ff09d4c4c7 100644
>> > --- a/meta/classes/kernel.bbclass
>> > +++ b/meta/classes/kernel.bbclass
>> > @@ -231,7 +231,7 @@ do_bundle_initramfs () {
>> > copy_initramfs
>> > # Backing up kernel image relies on its
>> > type(regular file or symbolic link)
>> > tmp_path=""
>> > -   for type in ${KERNEL_IMAGETYPES} ; do
>> > +   for type in ${KERNEL_IMAGETYPE_FOR_MAKE} ; do
>> The net result of this change is to no longer backup the non-
>> initramfs
>> vmlinux.gz kernel image before creating the kernel image(s)
>> containing
>> bundled initramfs (plus some other corner cases, depending on whether
>> the legacy KERNEL_IMAGETYPE and KERNEL_ALT_IMAGETYPE variables are
>> used and whether or not KERNEL_IMAGETYPES contains both vmlinux and
>> vmlinux.gz or vmlinux.gz only).
>>
>> Can you explain a little how that's related to building fitImage
>> kernels?
>>
>> This code in do_bundle_initramfs() is extremely fragile and changes
>> may only break less well testing targets such as MIPS, so any changes
>> need to be very carefully reviewed and tested.
>
> Actually, I think it means we need better tests.

Yes. This bug was spotted via code review though (unfortunately too
late to prevent the change being merged to master...).

> I'm seriously considering removing a lot of the stuff in kernel.bbclass
> and then rebuilding things in hopefully a cleaner way. The test for
> anything being needed would be whether our regression tests pass.
>
> I appreciate the idea will fill some with horror but there are a few
> places the code code is turning into an unmaintainable (and
> unreviewable) mess, the kernel classes are getting there :(
>
> Cheers,
>
> Richard
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


Re: [OE-core] [PATCH] kernel: Use KERNEL_IMAGETYPE_FOR_MAKE in do_bundle_initramfs

2018-01-21 Thread Richard Purdie
On Wed, 2018-01-17 at 13:31 -0800, Andre McCurdy wrote:
> On Wed, Jan 17, 2018 at 1:21 AM, Thomas Perrot  > wrote:
> > 
> > In the case of a fitImage the this step should be done on the
> > image use to assemble the fitImage.
> > 
> > Signed-off-by: Thomas Perrot 
> > ---
> >  meta/classes/kernel.bbclass | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes/kernel.bbclass
> > b/meta/classes/kernel.bbclass
> > index c42f85c665..ff09d4c4c7 100644
> > --- a/meta/classes/kernel.bbclass
> > +++ b/meta/classes/kernel.bbclass
> > @@ -231,7 +231,7 @@ do_bundle_initramfs () {
> > copy_initramfs
> > # Backing up kernel image relies on its
> > type(regular file or symbolic link)
> > tmp_path=""
> > -   for type in ${KERNEL_IMAGETYPES} ; do
> > +   for type in ${KERNEL_IMAGETYPE_FOR_MAKE} ; do
> The net result of this change is to no longer backup the non-
> initramfs
> vmlinux.gz kernel image before creating the kernel image(s)
> containing
> bundled initramfs (plus some other corner cases, depending on whether
> the legacy KERNEL_IMAGETYPE and KERNEL_ALT_IMAGETYPE variables are
> used and whether or not KERNEL_IMAGETYPES contains both vmlinux and
> vmlinux.gz or vmlinux.gz only).
> 
> Can you explain a little how that's related to building fitImage
> kernels?
> 
> This code in do_bundle_initramfs() is extremely fragile and changes
> may only break less well testing targets such as MIPS, so any changes
> need to be very carefully reviewed and tested.

Actually, I think it means we need better tests.

I'm seriously considering removing a lot of the stuff in kernel.bbclass
and then rebuilding things in hopefully a cleaner way. The test for
anything being needed would be whether our regression tests pass.

I appreciate the idea will fill some with horror but there are a few
places the code code is turning into an unmaintainable (and
unreviewable) mess, the kernel classes are getting there :(

Cheers,

Richard
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


Re: [OE-core] [PATCH] kernel: Use KERNEL_IMAGETYPE_FOR_MAKE in do_bundle_initramfs

2018-01-17 Thread Andre McCurdy
On Wed, Jan 17, 2018 at 1:21 AM, Thomas Perrot  wrote:
> In the case of a fitImage the this step should be done on the
> image use to assemble the fitImage.
>
> Signed-off-by: Thomas Perrot 
> ---
>  meta/classes/kernel.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index c42f85c665..ff09d4c4c7 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -231,7 +231,7 @@ do_bundle_initramfs () {
> copy_initramfs
> # Backing up kernel image relies on its type(regular file or 
> symbolic link)
> tmp_path=""
> -   for type in ${KERNEL_IMAGETYPES} ; do
> +   for type in ${KERNEL_IMAGETYPE_FOR_MAKE} ; do

The net result of this change is to no longer backup the non-initramfs
vmlinux.gz kernel image before creating the kernel image(s) containing
bundled initramfs (plus some other corner cases, depending on whether
the legacy KERNEL_IMAGETYPE and KERNEL_ALT_IMAGETYPE variables are
used and whether or not KERNEL_IMAGETYPES contains both vmlinux and
vmlinux.gz or vmlinux.gz only).

Can you explain a little how that's related to building fitImage kernels?

This code in do_bundle_initramfs() is extremely fragile and changes
may only break less well testing targets such as MIPS, so any changes
need to be very carefully reviewed and tested.

> if [ -h ${KERNEL_OUTPUT_DIR}/$type ] ; then
> linkpath=`readlink -n 
> ${KERNEL_OUTPUT_DIR}/$type`
> realpath=`readlink -fn 
> ${KERNEL_OUTPUT_DIR}/$type`
> --
> 2.13.6
>
> --
> ___
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core