All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] image.bbclass: Correct chaining compression support
@ 2017-07-21 22:06 Tom Rini
  2017-07-21 22:06 ` [PATCH 2/2] image_types.bbclass: Make u-boot signed images more versatile Tom Rini
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Tom Rini @ 2017-07-21 22:06 UTC (permalink / raw)
  To: openembedded-core

When chaining of compression/conversion types was added, we had a new
way to handle doing things like "ext4.bz2.sha256sum" or
"ext2.gz.u-boot".  However, because the U-Boot image class isn't
included normally, it wasn't properly converted at the time.  After the
support was added the "clean" argument that the .u-boot code uses no
longer functions.  The fix for this inadvertently broke chaining
compression/conversion.  First, correct the u-boot conversion code.

Fixes: 46bc438374de ("image.bbclass: do exact match for rootfs type")
Cc: Zhenhua Luo <zhenhua.luo@nxp.com>
Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
Cc: Patrick Ohly <patrick.ohly@intel.com>
Signed-off-by: Tom Rini <trini@konsulko.com>
---
This change is fairly important and, imho, innocuous and should be
populated to pyro/etc, once merged to master.  The next part of the
series is clean-up and while with my U-Boot hat on, I would say should
be pushed more widely, I am biased.
---
 meta/classes/image.bbclass             |  2 +-
 meta/classes/image_types_uboot.bbclass | 13 +++++--------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index de535ce6fcff..bd6a5b7b810a 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -453,7 +453,7 @@ python () {
         rm_tmp_images = set()
         def gen_conversion_cmds(bt):
             for ctype in ctypes:
-                if bt[bt.find('.') + 1:] == ctype:
+                if bt.endswith("." + ctype):
                     type = bt[0:-len(ctype) - 1]
                     if type.startswith("debugfs_"):
                         type = type[8:]
diff --git a/meta/classes/image_types_uboot.bbclass b/meta/classes/image_types_uboot.bbclass
index 5dfa39287dab..8db436efb14b 100644
--- a/meta/classes/image_types_uboot.bbclass
+++ b/meta/classes/image_types_uboot.bbclass
@@ -3,9 +3,6 @@ inherit image_types kernel-arch
 oe_mkimage () {
     mkimage -A ${UBOOT_ARCH} -O linux -T ramdisk -C $2 -n ${IMAGE_NAME} \
         -d ${IMGDEPLOYDIR}/$1 ${IMGDEPLOYDIR}/$1.u-boot
-    if [ x$3 = x"clean" ]; then
-        rm $1
-    fi
 }
 
 CONVERSIONTYPES += "gz.u-boot bz2.u-boot lz4.u-boot lzma.u-boot lzo.u-boot u-boot"
@@ -14,19 +11,19 @@ CONVERSION_DEPENDS_u-boot = "u-boot-mkimage-native"
 CONVERSION_CMD_u-boot      = "oe_mkimage ${IMAGE_NAME}.rootfs.${type} none"
 
 CONVERSION_DEPENDS_gz.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_gz.u-boot      = "${CONVERSION_CMD_gz}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.gz gzip clean"
+CONVERSION_CMD_gz.u-boot      = "${CONVERSION_CMD_gz}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.gz gzip"
 
 CONVERSION_DEPENDS_bz2.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_bz2.u-boot      = "${CONVERSION_CMD_bz2}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.bz2 bzip2 clean"
+CONVERSION_CMD_bz2.u-boot      = "${CONVERSION_CMD_bz2}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.bz2 bzip2"
 
 CONVERSION_DEPENDS_lz4.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_lz4.u-boot      = "${CONVERSION_CMD_lz4_legacy}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lz4 lz4 clean"
+CONVERSION_CMD_lz4.u-boot      = "${CONVERSION_CMD_lz4_legacy}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lz4 lz4"
 
 CONVERSION_DEPENDS_lzma.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_lzma.u-boot      = "${CONVERSION_CMD_lzma}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzma lzma clean"
+CONVERSION_CMD_lzma.u-boot      = "${CONVERSION_CMD_lzma}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzma lzma"
 
 CONVERSION_DEPENDS_lzo.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_lzo.u-boot      = "${CONVERSION_CMD_lzo}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzo lzo clean"
+CONVERSION_CMD_lzo.u-boot      = "${CONVERSION_CMD_lzo}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzo lzo"
 
 IMAGE_TYPES += "ext2.u-boot ext2.gz.u-boot ext2.bz2.u-boot ext2.lzma.u-boot ext3.gz.u-boot ext4.gz.u-boot cpio.gz.u-boot"
 
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] image_types.bbclass: Make u-boot signed images more versatile
  2017-07-21 22:06 [PATCH 1/2] image.bbclass: Correct chaining compression support Tom Rini
@ 2017-07-21 22:06 ` Tom Rini
  2017-07-24  8:35 ` [PATCH 1/2] image.bbclass: Correct chaining compression support Patrick Ohly
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2017-07-21 22:06 UTC (permalink / raw)
  To: openembedded-core

With the introduction of chaining compression/conversion support we can
convert the old image_types_uboot.bbclass code that did a hand-chaining
of a set of ${filesystem}.${compression} into generic and arbitrary
support to sign whatever the user wants to sign for their image.

This, for the record, does remove setting a valid compression type in
the record in favour of just saying none.  This is not a generally
useful feature in U-Boot and I believe being versatile in terms of being
able to pass in arbitrary compressions is more important.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
 meta/classes/image.bbclass             |  9 ---------
 meta/classes/image_types.bbclass       |  6 ++++--
 meta/classes/image_types_uboot.bbclass | 29 -----------------------------
 3 files changed, 4 insertions(+), 40 deletions(-)
 delete mode 100644 meta/classes/image_types_uboot.bbclass

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index bd6a5b7b810a..b095bca7c12a 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -144,15 +144,6 @@ inherit ${IMAGE_TYPE_vm}
 IMAGE_TYPE_container = '${@bb.utils.contains("IMAGE_FSTYPES", "container", "image-container", "", d)}'
 inherit ${IMAGE_TYPE_container}
 
-def build_uboot(d):
-    if 'u-boot' in (d.getVar('IMAGE_FSTYPES') or ''):
-        return "image_types_uboot"
-    else:
-        return ""
-
-IMAGE_TYPE_uboot = "${@build_uboot(d)}"
-inherit ${IMAGE_TYPE_uboot}
-
 IMAGE_TYPE_wic = "image_types_wic"
 inherit ${IMAGE_TYPE_wic}
 
diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index 8f8d79cd3d70..4fdcde25b4f5 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -214,7 +214,7 @@ IMAGE_CMD_ubifs = "mkfs.ubifs -r ${IMAGE_ROOTFS} -o ${IMGDEPLOYDIR}/${IMAGE_NAME
 
 EXTRA_IMAGECMD = ""
 
-inherit siteinfo
+inherit siteinfo kernel-arch
 JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
 JFFS2_ERASEBLOCK ?= "0x40000"
 EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
@@ -269,7 +269,7 @@ IMAGE_TYPES = " \
 # CONVERSION_CMD/DEPENDS.
 COMPRESSIONTYPES ?= ""
 
-CONVERSIONTYPES = "gz bz2 lzma xz lz4 lzo zip sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum bmap ${COMPRESSIONTYPES}"
+CONVERSIONTYPES = "gz bz2 lzma xz lz4 lzo zip sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum bmap u-boot ${COMPRESSIONTYPES}"
 CONVERSION_CMD_lzma = "lzma -k -f -7 ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
 CONVERSION_CMD_gz = "gzip -f -9 -c ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.gz"
 CONVERSION_CMD_bz2 = "pbzip2 -f -k ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
@@ -286,6 +286,7 @@ CONVERSION_CMD_sha256sum = "sha256sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}
 CONVERSION_CMD_sha384sum = "sha384sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha384sum"
 CONVERSION_CMD_sha512sum = "sha512sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha512sum"
 CONVERSION_CMD_bmap = "bmaptool create ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} -o ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.bmap"
+CONVERSION_CMD_u-boot = "mkimage -A ${UBOOT_ARCH} -O linux -T ramdisk -C none -n ${IMAGE_NAME} -d ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.u-boot"
 CONVERSION_DEPENDS_lzma = "xz-native"
 CONVERSION_DEPENDS_gz = "pigz-native"
 CONVERSION_DEPENDS_bz2 = "pbzip2-native"
@@ -295,6 +296,7 @@ CONVERSION_DEPENDS_lzo = "lzop-native"
 CONVERSION_DEPENDS_zip = "zip-native"
 CONVERSION_DEPENDS_sum = "mtd-utils-native"
 CONVERSION_DEPENDS_bmap = "bmap-tools-native"
+CONVERSION_DEPENDS_u-boot = "u-boot-mkimage-native"
 
 RUNNABLE_IMAGE_TYPES ?= "ext2 ext3 ext4"
 RUNNABLE_MACHINE_PATTERNS ?= "qemu"
diff --git a/meta/classes/image_types_uboot.bbclass b/meta/classes/image_types_uboot.bbclass
deleted file mode 100644
index 8db436efb14b..000000000000
--- a/meta/classes/image_types_uboot.bbclass
+++ /dev/null
@@ -1,29 +0,0 @@
-inherit image_types kernel-arch
-
-oe_mkimage () {
-    mkimage -A ${UBOOT_ARCH} -O linux -T ramdisk -C $2 -n ${IMAGE_NAME} \
-        -d ${IMGDEPLOYDIR}/$1 ${IMGDEPLOYDIR}/$1.u-boot
-}
-
-CONVERSIONTYPES += "gz.u-boot bz2.u-boot lz4.u-boot lzma.u-boot lzo.u-boot u-boot"
-
-CONVERSION_DEPENDS_u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_u-boot      = "oe_mkimage ${IMAGE_NAME}.rootfs.${type} none"
-
-CONVERSION_DEPENDS_gz.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_gz.u-boot      = "${CONVERSION_CMD_gz}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.gz gzip"
-
-CONVERSION_DEPENDS_bz2.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_bz2.u-boot      = "${CONVERSION_CMD_bz2}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.bz2 bzip2"
-
-CONVERSION_DEPENDS_lz4.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_lz4.u-boot      = "${CONVERSION_CMD_lz4_legacy}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lz4 lz4"
-
-CONVERSION_DEPENDS_lzma.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_lzma.u-boot      = "${CONVERSION_CMD_lzma}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzma lzma"
-
-CONVERSION_DEPENDS_lzo.u-boot = "u-boot-mkimage-native"
-CONVERSION_CMD_lzo.u-boot      = "${CONVERSION_CMD_lzo}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzo lzo"
-
-IMAGE_TYPES += "ext2.u-boot ext2.gz.u-boot ext2.bz2.u-boot ext2.lzma.u-boot ext3.gz.u-boot ext4.gz.u-boot cpio.gz.u-boot"
-
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] image.bbclass: Correct chaining compression support
  2017-07-21 22:06 [PATCH 1/2] image.bbclass: Correct chaining compression support Tom Rini
  2017-07-21 22:06 ` [PATCH 2/2] image_types.bbclass: Make u-boot signed images more versatile Tom Rini
@ 2017-07-24  8:35 ` Patrick Ohly
  2017-07-24 11:19   ` Tom Rini
  2017-07-26  7:16 ` Ed Bartosh
  2017-09-19  9:23 ` Martin Hundebøll
  3 siblings, 1 reply; 9+ messages in thread
From: Patrick Ohly @ 2017-07-24  8:35 UTC (permalink / raw)
  To: Tom Rini, openembedded-core

On Fri, 2017-07-21 at 18:06 -0400, Tom Rini wrote:
> The fix for this inadvertently broke chaining
> compression/conversion.  First, correct the u-boot conversion code.
> 
> Fixes: 46bc438374de ("image.bbclass: do exact
> match for rootfs type")
> Cc: Zhenhua Luo <zhenhua.luo@nxp.com>
> Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
> Cc: Patrick Ohly <patrick.ohly@intel.com>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> This change is fairly important and, imho, innocuous and should be
> populated to pyro/etc, once merged to master.  The next part of the
> series is clean-up and while with my U-Boot hat on, I would say
> should
> be pushed more widely, I am biased.
> ---
>  meta/classes/image.bbclass             |  2 +-
>  meta/classes/image_types_uboot.bbclass | 13 +++++--------
>  2 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index de535ce6fcff..bd6a5b7b810a 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -453,7 +453,7 @@ python () {
>          rm_tmp_images = set()
>          def gen_conversion_cmds(bt):
>              for ctype in ctypes:
> -                if bt[bt.find('.') + 1:] == ctype:
> +                if bt.endswith("." + ctype)

This reverts 46bc438374de and thus restores the code as I had
originally written it.

Looking at 46bc438374de, it's not clear to me how the commit message
matches the code, i.e. I don't understand the commit. So it was an
incorrect fix for the problem described in that commit message, and the
right one are your changes to the u-boot conversion command?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] image.bbclass: Correct chaining compression support
  2017-07-24  8:35 ` [PATCH 1/2] image.bbclass: Correct chaining compression support Patrick Ohly
@ 2017-07-24 11:19   ` Tom Rini
  2017-07-25 11:44     ` Patrick Ohly
  0 siblings, 1 reply; 9+ messages in thread
From: Tom Rini @ 2017-07-24 11:19 UTC (permalink / raw)
  To: Patrick Ohly; +Cc: openembedded-core

[-- Attachment #1: Type: text/plain, Size: 2322 bytes --]

On Mon, Jul 24, 2017 at 10:35:37AM +0200, Patrick Ohly wrote:
> On Fri, 2017-07-21 at 18:06 -0400, Tom Rini wrote:
> > The fix for this inadvertently broke chaining
> > compression/conversion.  First, correct the u-boot conversion code.
> > 
> > Fixes: 46bc438374de ("image.bbclass: do exact
> > match for rootfs type")
> > Cc: Zhenhua Luo <zhenhua.luo@nxp.com>
> > Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
> > Cc: Patrick Ohly <patrick.ohly@intel.com>
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> > This change is fairly important and, imho, innocuous and should be
> > populated to pyro/etc, once merged to master.  The next part of the
> > series is clean-up and while with my U-Boot hat on, I would say
> > should
> > be pushed more widely, I am biased.
> > ---
> >  meta/classes/image.bbclass             |  2 +-
> >  meta/classes/image_types_uboot.bbclass | 13 +++++--------
> >  2 files changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > index de535ce6fcff..bd6a5b7b810a 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -453,7 +453,7 @@ python () {
> >          rm_tmp_images = set()
> >          def gen_conversion_cmds(bt):
> >              for ctype in ctypes:
> > -                if bt[bt.find('.') + 1:] == ctype:
> > +                if bt.endswith("." + ctype)
> 
> This reverts 46bc438374de and thus restores the code as I had
> originally written it.
> 
> Looking at 46bc438374de, it's not clear to me how the commit message
> matches the code, i.e. I don't understand the commit. So it was an
> incorrect fix for the problem described in that commit message, and the
> right one are your changes to the u-boot conversion command?

Ah, so the important bit is the other half of this patch, which
addresses the problem 46bc438374de was intended to deal with, correctly.
Prior to the chaining compression/conversion support, the "u-boot"
targets would clean up their intermediate files.  With your patch those
files get cleaned up automatically and that the mkimage calling function
was also doing it lead to build failures.  But since we no longer need
to have a manual cleaning step, we can just drop it.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] image.bbclass: Correct chaining compression support
  2017-07-24 11:19   ` Tom Rini
@ 2017-07-25 11:44     ` Patrick Ohly
  0 siblings, 0 replies; 9+ messages in thread
From: Patrick Ohly @ 2017-07-25 11:44 UTC (permalink / raw)
  To: Tom Rini; +Cc: openembedded-core

On Mon, 2017-07-24 at 07:19 -0400, Tom Rini wrote:
> On Mon, Jul 24, 2017 at 10:35:37AM +0200, Patrick Ohly wrote:
> > On Fri, 2017-07-21 at 18:06 -0400, Tom Rini wrote:
> > > The fix for this inadvertently broke chaining
> > > compression/conversion.  First, correct the u-boot conversion
> > > code.
> > > 
> > > Fixes: 46bc438374de ("image.bbclass: do exact
> > > match for rootfs type")
> > > Cc: Zhenhua Luo <zhenhua.luo@nxp.com>
> > > Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
> > > Cc: Patrick Ohly <patrick.ohly@intel.com>
> > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > ---
> > > This change is fairly important and, imho, innocuous and should
> > > be
> > > populated to pyro/etc, once merged to master.  The next part of
> > > the
> > > series is clean-up and while with my U-Boot hat on, I would say
> > > should
> > > be pushed more widely, I am biased.
> > > ---
> > >  meta/classes/image.bbclass             |  2 +-
> > >  meta/classes/image_types_uboot.bbclass | 13 +++++--------
> > >  2 files changed, 6 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/meta/classes/image.bbclass
> > > b/meta/classes/image.bbclass
> > > index de535ce6fcff..bd6a5b7b810a 100644
> > > --- a/meta/classes/image.bbclass
> > > +++ b/meta/classes/image.bbclass
> > > @@ -453,7 +453,7 @@ python () {
> > >          rm_tmp_images = set()
> > >          def gen_conversion_cmds(bt):
> > >              for ctype in ctypes:
> > > -                if bt[bt.find('.') + 1:] == ctype:
> > > +                if bt.endswith("." + ctype)
> > 
> > This reverts 46bc438374de and thus restores the code as I had
> > originally written it.
> > 
> > Looking at 46bc438374de, it's not clear to me how the commit
> > message
> > matches the code, i.e. I don't understand the commit. So it was an
> > incorrect fix for the problem described in that commit message, and
> > the
> > right one are your changes to the u-boot conversion command?
> 
> Ah, so the important bit is the other half of this patch, which
> addresses the problem 46bc438374de was intended to deal with,
> correctly.
> Prior to the chaining compression/conversion support, the "u-boot"
> targets would clean up their intermediate files.  With your patch
> those
> files get cleaned up automatically and that the mkimage calling
> function
> was also doing it lead to build failures.  But since we no longer
> need
> to have a manual cleaning step, we can just drop it.

Makes sense.

Acked-by: Patrick Ohly <patrick.ohly@intel.com>

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] image.bbclass: Correct chaining compression support
  2017-07-21 22:06 [PATCH 1/2] image.bbclass: Correct chaining compression support Tom Rini
  2017-07-21 22:06 ` [PATCH 2/2] image_types.bbclass: Make u-boot signed images more versatile Tom Rini
  2017-07-24  8:35 ` [PATCH 1/2] image.bbclass: Correct chaining compression support Patrick Ohly
@ 2017-07-26  7:16 ` Ed Bartosh
  2017-07-26 11:10   ` Tom Rini
  2017-09-19  9:23 ` Martin Hundebøll
  3 siblings, 1 reply; 9+ messages in thread
From: Ed Bartosh @ 2017-07-26  7:16 UTC (permalink / raw)
  To: Tom Rini; +Cc: openembedded-core

On Fri, Jul 21, 2017 at 06:06:33PM -0400, Tom Rini wrote:
> When chaining of compression/conversion types was added, we had a new
> way to handle doing things like "ext4.bz2.sha256sum" or
> "ext2.gz.u-boot".  However, because the U-Boot image class isn't
> included normally, it wasn't properly converted at the time.  After the
> support was added the "clean" argument that the .u-boot code uses no
> longer functions.  The fix for this inadvertently broke chaining
> compression/conversion.  First, correct the u-boot conversion code.
> 
> Fixes: 46bc438374de ("image.bbclass: do exact match for rootfs type")
> Cc: Zhenhua Luo <zhenhua.luo@nxp.com>
> Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
> Cc: Patrick Ohly <patrick.ohly@intel.com>
> Signed-off-by: Tom Rini <trini@konsulko.com>

Acked-by: Ed Bartosh <ed.bartosh@linux.intel.com>

Any chance to have this functionality covered by oe-selftest?
That would help to ensure it will not be broken again.

> ---
> This change is fairly important and, imho, innocuous and should be
> populated to pyro/etc, once merged to master.  The next part of the
> series is clean-up and while with my U-Boot hat on, I would say should
> be pushed more widely, I am biased.
> ---
>  meta/classes/image.bbclass             |  2 +-
>  meta/classes/image_types_uboot.bbclass | 13 +++++--------
>  2 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index de535ce6fcff..bd6a5b7b810a 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -453,7 +453,7 @@ python () {
>          rm_tmp_images = set()
>          def gen_conversion_cmds(bt):
>              for ctype in ctypes:
> -                if bt[bt.find('.') + 1:] == ctype:
> +                if bt.endswith("." + ctype):
>                      type = bt[0:-len(ctype) - 1]
>                      if type.startswith("debugfs_"):
>                          type = type[8:]
> diff --git a/meta/classes/image_types_uboot.bbclass b/meta/classes/image_types_uboot.bbclass
> index 5dfa39287dab..8db436efb14b 100644
> --- a/meta/classes/image_types_uboot.bbclass
> +++ b/meta/classes/image_types_uboot.bbclass
> @@ -3,9 +3,6 @@ inherit image_types kernel-arch
>  oe_mkimage () {
>      mkimage -A ${UBOOT_ARCH} -O linux -T ramdisk -C $2 -n ${IMAGE_NAME} \
>          -d ${IMGDEPLOYDIR}/$1 ${IMGDEPLOYDIR}/$1.u-boot
> -    if [ x$3 = x"clean" ]; then
> -        rm $1
> -    fi
>  }
>  
>  CONVERSIONTYPES += "gz.u-boot bz2.u-boot lz4.u-boot lzma.u-boot lzo.u-boot u-boot"
> @@ -14,19 +11,19 @@ CONVERSION_DEPENDS_u-boot = "u-boot-mkimage-native"
>  CONVERSION_CMD_u-boot      = "oe_mkimage ${IMAGE_NAME}.rootfs.${type} none"
>  
>  CONVERSION_DEPENDS_gz.u-boot = "u-boot-mkimage-native"
> -CONVERSION_CMD_gz.u-boot      = "${CONVERSION_CMD_gz}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.gz gzip clean"
> +CONVERSION_CMD_gz.u-boot      = "${CONVERSION_CMD_gz}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.gz gzip"
>  
>  CONVERSION_DEPENDS_bz2.u-boot = "u-boot-mkimage-native"
> -CONVERSION_CMD_bz2.u-boot      = "${CONVERSION_CMD_bz2}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.bz2 bzip2 clean"
> +CONVERSION_CMD_bz2.u-boot      = "${CONVERSION_CMD_bz2}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.bz2 bzip2"
>  
>  CONVERSION_DEPENDS_lz4.u-boot = "u-boot-mkimage-native"
> -CONVERSION_CMD_lz4.u-boot      = "${CONVERSION_CMD_lz4_legacy}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lz4 lz4 clean"
> +CONVERSION_CMD_lz4.u-boot      = "${CONVERSION_CMD_lz4_legacy}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lz4 lz4"
>  
>  CONVERSION_DEPENDS_lzma.u-boot = "u-boot-mkimage-native"
> -CONVERSION_CMD_lzma.u-boot      = "${CONVERSION_CMD_lzma}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzma lzma clean"
> +CONVERSION_CMD_lzma.u-boot      = "${CONVERSION_CMD_lzma}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzma lzma"
>  
>  CONVERSION_DEPENDS_lzo.u-boot = "u-boot-mkimage-native"
> -CONVERSION_CMD_lzo.u-boot      = "${CONVERSION_CMD_lzo}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzo lzo clean"
> +CONVERSION_CMD_lzo.u-boot      = "${CONVERSION_CMD_lzo}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzo lzo"
>  
>  IMAGE_TYPES += "ext2.u-boot ext2.gz.u-boot ext2.bz2.u-boot ext2.lzma.u-boot ext3.gz.u-boot ext4.gz.u-boot cpio.gz.u-boot"
>  
> -- 
> 1.9.1
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

--
Regards,
Ed


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] image.bbclass: Correct chaining compression support
  2017-07-26  7:16 ` Ed Bartosh
@ 2017-07-26 11:10   ` Tom Rini
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2017-07-26 11:10 UTC (permalink / raw)
  To: Ed Bartosh; +Cc: openembedded-core

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]

On Wed, Jul 26, 2017 at 10:16:39AM +0300, Ed Bartosh wrote:
> On Fri, Jul 21, 2017 at 06:06:33PM -0400, Tom Rini wrote:
> > When chaining of compression/conversion types was added, we had a new
> > way to handle doing things like "ext4.bz2.sha256sum" or
> > "ext2.gz.u-boot".  However, because the U-Boot image class isn't
> > included normally, it wasn't properly converted at the time.  After the
> > support was added the "clean" argument that the .u-boot code uses no
> > longer functions.  The fix for this inadvertently broke chaining
> > compression/conversion.  First, correct the u-boot conversion code.
> > 
> > Fixes: 46bc438374de ("image.bbclass: do exact match for rootfs type")
> > Cc: Zhenhua Luo <zhenhua.luo@nxp.com>
> > Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
> > Cc: Patrick Ohly <patrick.ohly@intel.com>
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> 
> Acked-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> 
> Any chance to have this functionality covered by oe-selftest?
> That would help to ensure it will not be broken again.

Give me a pointer to adding stuff to oe-selftest and I'll take a look.
Thanks!

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] image.bbclass: Correct chaining compression support
  2017-07-21 22:06 [PATCH 1/2] image.bbclass: Correct chaining compression support Tom Rini
                   ` (2 preceding siblings ...)
  2017-07-26  7:16 ` Ed Bartosh
@ 2017-09-19  9:23 ` Martin Hundebøll
  2017-09-20  0:22   ` Tom Rini
  3 siblings, 1 reply; 9+ messages in thread
From: Martin Hundebøll @ 2017-09-19  9:23 UTC (permalink / raw)
  To: Tom Rini; +Cc: openembedded-core

Hi Tom,

On 2017-07-22 00:06, Tom Rini wrote:
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index de535ce6fcff..bd6a5b7b810a 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -453,7 +453,7 @@ python () {
>           rm_tmp_images = set()
>           def gen_conversion_cmds(bt):
>               for ctype in ctypes:
> -                if bt[bt.find('.') + 1:] == ctype:
> +                if bt.endswith("." + ctype):
>                       type = bt[0:-len(ctype) - 1]
>                       if type.startswith("debugfs_"):
>                           type = type[8:]

This is the change that in fact messes with our cpio.gz.u-boot image 
types, causing base hash changes.

I suspect the changed if-check to be too permissive. In our case, it now 
matches any ctype that ends with ".u-boot", and not just "gz.u-boot" as 
set in IMAGE_FSTYPES.

Wouldn't it be correct to match on the entire bt variable? I.e.

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass 

index ef2b38aeaf..818932f7f1 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -454,7 +454,7 @@ python () {
          rm_tmp_images = set()
          def gen_conversion_cmds(bt):
              for ctype in ctypes:
-                if bt.endswith("." + ctype):
+                if bt.split('.', 1) == [t, ctype]:
                      type = bt[0:-len(ctype) - 1]
                      if type.startswith("debugfs_"):
                          type = type[8:]

This works for me, preliminary tested with the following fs types 
configured:

	IMAGE_FSTYPES = "cpio.gz.u-boot"
	IMAGE_FSTYPES = "tar cpio cpio.gz cpio.gz.u-boot"
	IMAGE_FSTYPES = "tar cpio cpio.gz cpio.gz.u-boot tar.bz2 jffs2 wic 
wic.bmap"

// Martin


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] image.bbclass: Correct chaining compression support
  2017-09-19  9:23 ` Martin Hundebøll
@ 2017-09-20  0:22   ` Tom Rini
  0 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2017-09-20  0:22 UTC (permalink / raw)
  To: Martin Hundebøll; +Cc: openembedded-core

[-- Attachment #1: Type: text/plain, Size: 2196 bytes --]

On Tue, Sep 19, 2017 at 11:23:33AM +0200, Martin Hundebøll wrote:
> Hi Tom,
> 
> On 2017-07-22 00:06, Tom Rini wrote:
> >diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> >index de535ce6fcff..bd6a5b7b810a 100644
> >--- a/meta/classes/image.bbclass
> >+++ b/meta/classes/image.bbclass
> >@@ -453,7 +453,7 @@ python () {
> >          rm_tmp_images = set()
> >          def gen_conversion_cmds(bt):
> >              for ctype in ctypes:
> >-                if bt[bt.find('.') + 1:] == ctype:
> >+                if bt.endswith("." + ctype):
> >                      type = bt[0:-len(ctype) - 1]
> >                      if type.startswith("debugfs_"):
> >                          type = type[8:]
> 
> This is the change that in fact messes with our cpio.gz.u-boot image
> types, causing base hash changes.
> 
> I suspect the changed if-check to be too permissive. In our case, it
> now matches any ctype that ends with ".u-boot", and not just
> "gz.u-boot" as set in IMAGE_FSTYPES.
> 
> Wouldn't it be correct to match on the entire bt variable? I.e.
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> 
> index ef2b38aeaf..818932f7f1 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -454,7 +454,7 @@ python () {
>          rm_tmp_images = set()
>          def gen_conversion_cmds(bt):
>              for ctype in ctypes:
> -                if bt.endswith("." + ctype):
> +                if bt.split('.', 1) == [t, ctype]:
>                      type = bt[0:-len(ctype) - 1]
>                      if type.startswith("debugfs_"):
>                          type = type[8:]
> 
> This works for me, preliminary tested with the following fs types
> configured:
> 
> 	IMAGE_FSTYPES = "cpio.gz.u-boot"
> 	IMAGE_FSTYPES = "tar cpio cpio.gz cpio.gz.u-boot"
> 	IMAGE_FSTYPES = "tar cpio cpio.gz cpio.gz.u-boot tar.bz2 jffs2 wic
> wic.bmap"

Good catch.  Does it also work with say cpio.u-boot.gz ?  And does
oe-selftest still pass for the image related parts?  I think those are
spelled out in the thread too (I've forgotten the incantation myself
now, sorry).

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-09-20  0:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-21 22:06 [PATCH 1/2] image.bbclass: Correct chaining compression support Tom Rini
2017-07-21 22:06 ` [PATCH 2/2] image_types.bbclass: Make u-boot signed images more versatile Tom Rini
2017-07-24  8:35 ` [PATCH 1/2] image.bbclass: Correct chaining compression support Patrick Ohly
2017-07-24 11:19   ` Tom Rini
2017-07-25 11:44     ` Patrick Ohly
2017-07-26  7:16 ` Ed Bartosh
2017-07-26 11:10   ` Tom Rini
2017-09-19  9:23 ` Martin Hundebøll
2017-09-20  0:22   ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.