[OE-core] [PATCH 3/5] image_types.bbclass: fix EXTRA_IMAGECMD_sum.jffs2
Richard Purdie
richard.purdie at linuxfoundation.org
Tue Oct 29 15:33:28 UTC 2013
On Tue, 2013-10-29 at 13:17 +0100, Andrea Adami wrote:
> On Tue, Oct 29, 2013 at 12:45 PM, Richard Purdie
> <richard.purdie at linuxfoundation.org> wrote:
> > On Tue, 2013-10-29 at 12:17 +0100, Andrea Adami wrote:
> >> On Tue, Oct 29, 2013 at 12:01 PM, Richard Purdie
> >> <richard.purdie at linuxfoundation.org> wrote:
> >> > On Mon, 2013-10-21 at 00:34 +0200, Andrea Adami wrote:
> >> >> When overriding EXTRA_IMAGE_CMD_jffs2 = "--pad=foo ..."
> >> >> we are passing a malformed option to sumtool:
> >> >>
> >> >> sumtool: option '--pad' doesn't allow an argument
> >> >>
> >> >> Fix this by declaring a separate variable for the purpose.
> >> >>
> >> >> Signed-off-by: Andrea Adami <andrea.adami at gmail.com>
> >> >> ---
> >> >> meta/classes/image_types.bbclass | 5 +++--
> >> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
> >> >> index b8779e0..21391e8 100644
> >> >> --- a/meta/classes/image_types.bbclass
> >> >> +++ b/meta/classes/image_types.bbclass
> >> >> @@ -141,8 +141,8 @@ XZ_INTEGRITY_CHECK ?= "crc32"
> >> >> XZ_THREADS ?= "-T 0"
> >> >>
> >> >> IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
> >> >> -IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
> >> >> - -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
> >> >> +IMAGE_CMD_sum.jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD_jffs2} \
> >> >> + && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
> >> >>
> >> >> IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
> >> >>
> >> >> @@ -212,6 +212,7 @@ inherit siteinfo
> >> >> JFFS2_ENDIANNESS ?= "${@base_conditional('SITEINFO_ENDIANNESS', 'le', '-l', '-b', d)}"
> >> >> JFFS2_ERASEBLOCK ?= "0x40000"
> >> >> EXTRA_IMAGECMD_jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
> >> >> +EXTRA_IMAGECMD_sum.jffs2 ?= "--pad ${JFFS2_ENDIANNESS} --eraseblock=${JFFS2_ERASEBLOCK} --no-cleanmarkers"
> >> >>
> >> >> # Change these if you want default mkfs behavior (i.e. create minimal inode number)
> >> >> EXTRA_IMAGECMD_ext2 ?= "-i 8192"
> >> >
> >> > This patch is very confused. You say sumtool doesn't take a --pad
> >> > option, yet "EXTRA_IMAGECMD_sum.jffs" which is presumably used with
> >> > sumtool does have the option.
> >> >
> >>
> >> After the commits you've done the creation of sum.jffs2 images is
> >> broken on collie.
> >> The error message is described clearly in the patch:
> >>
> >> sumtool: option '--pad' doesn't allow an argument
> >>
> >> Why that? Because collie needs to customize the padding.
> >> EXTRA_IMAGECMD_jffs2 = "--pad=14680064 -l -e ${JFFS2_ERASEBLOCK}"
> >>
> >> This --pad=14680064 is then passed to IMAGE_CMD_sum.jffs2 and we get
> >> build error.
> >>
> >> That's why we need to redefine both IMAGE_CMD_sum.jffs2 and
> >> EXTRA_IMAGECMD_sum.jffs2 so to pass --pad=XY only to mkfs.jffs2 and
> >> not to the sumtool part of IMAGE_CMD_sum.jffs2.
> >>
> >> ---
> >> By the way this sum.jffs2 imagetype is used only in meta-handheld afaik.
> >> It would be also ok if you'd remove IMAGE_CMD_sum.jffs2 so we can
> >> append the whole stuff in a customized EXTRA_IMAGECMD_jffs2.
> >
> > I understand your problem however:
> >
> > a) Copy and pasting the entire IMAGE_CMD_jffs2 command into the second
> > one is rather ugly.
> Yes, but using the IMAGE_CMD_jffs2 we incorporate the
> EXTRA_IMAGECMD_jffs2 = "--pad=xyz.."
>
> > b) You still end up passing "--pad ${JFFS2_ENDIANNESS} --eraseblock=
> > ${JFFS2_ERASEBLOCK} --no-cleanmarkers" as parameters to sumtool,
> > including a --pad option with no argument. Why/how?
>
> Because this padding is limited to the last eraseblock.
> " -p, --pad Pad the OUTPUT with 0xFF to the end of the
> final eraseblock"
>
> >
> > You call:
> >
> > sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}
> >
> > and EXTRA_IMAGECMD is set to EXTRA_IMAGECMD_sum.jffs2 due to the use of
> > overrides in the image generation code.
>
> We cannot reuse IMAGE_CMD_jffs2 because in that case
> EXTRA_IMAGECMD_jffs2 is already evaluated and this contains the
> infamous --pad==xy.
>
> >
> > So whilst collie is broken, I do not think this patch makes readable
> > code and I think we need to find something better.
> >
> > The thing I'm confused on is whether there should be any --pad option to
> > sumtool or not.
> >
>
> Yes, but without <size> argument
>
> http://git.infradead.org/mtd-utils.git/blob/HEAD:/sumtool.c
Ok, we need to make this clearer in the commit message. Looking and
thinking about this further, how about we add some dependency ordering
into this code, something like the patch below. This moves the jffs2
call into its own true namespace. You should then just be able to
explicitly set EXTRA_IMGAGECMD_sum.jffs2 to the value you need (yet
still have EXTRA_IMGAGECMD_jffs2 able to take a different value).
See if you can build something on top of the patch below...
Cheers,
Richard
>From 4ff883824221ce5f1b993239249b1ce1e3ab9b32 Mon Sep 17 00:00:00 2001
From: Richard Purdie <richard.purdie at linuxfoundation.org>
Date: Tue, 29 Oct 2013 15:26:54 +0000
Subject: image_types: Improve dependency handling between types (and use for sum.jffs2)
We're seeing a pattern of one image type needing to depend on another
type. A good example is jffs2 and sum.jffs2. This patch makes sum.jffs2
depend on jffs2 which will then allow a EXTRA_IMGAGECMD to be set for
sum.jffs2 individually without changing the jffs2 command. This allows the
-pad option to be configured differently.
Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
---
diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index b8779e0..5ce1ddb 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -7,14 +7,20 @@ def get_imagecmds(d):
ctypes = d.getVar('COMPRESSIONTYPES', True).split()
cimages = {}
+ # Image type b depends on a having been generated first
+ def addtypedepends(a, b):
+ if a in alltypes:
+ alltypes.remove(a):
+ if b not in alltypes:
+ alltypes.append(b)
+ alltypes.append(a)
+
# The elf image depends on the cpio.gz image already having
# been created, so we add that explicit ordering here.
+ addtypedepends("elf", "cpio.gz")
- if "elf" in alltypes:
- alltypes.remove("elf")
- if "cpio.gz" not in alltypes:
- alltypes.append("cpio.gz")
- alltypes.append("elf")
+ # jffs2 sumtool'd images need jffs2
+ addtypedepends("sum.jffs2", "jffs2")
# Filter out all the compressed images from alltypes
for type in alltypes:
@@ -141,8 +147,7 @@ XZ_INTEGRITY_CHECK ?= "crc32"
XZ_THREADS ?= "-T 0"
IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 ${EXTRA_IMAGECMD}"
-IMAGE_CMD_sum.jffs2 = "${IMAGE_CMD_jffs2} && sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
- -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
+IMAGE_CMD_sum.jffs2 = "sumtool -i ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 -o ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.sum.jffs2 ${EXTRA_IMAGECMD}"
IMAGE_CMD_cramfs = "mkfs.cramfs ${IMAGE_ROOTFS} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.cramfs ${EXTRA_IMAGECMD}"
More information about the Openembedded-core
mailing list