image_types.bbclass: what's with "elf" and "cpio.gz"?

image_types.bbclass: what's with "elf" and "cpio.gz"?

[Robert P. J. Day]

  this snippet from image_types.bbclass:

    if "elf" in alltypes:
        alltypes.remove("elf")
        if "cpio.gz" not in alltypes:
                alltypes.append("cpio.gz")
        alltypes.append("elf")

    # Filter out all the compressed images from types
    for type in alltypes:
    ... snip ...

i was going to trivially fix that comment (it's now "alltypes", not
"types"), but i was distracted by the code above that -- what the heck
is going on there?  what's the rationale for removing elf, then adding
cpio.gz and elf back on at the end?  if someone can clarify that, i
can add that as more comment, as well as a few other tweaks in that
file for more documentation for anyone reading TFS.  thanks.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

Re: image_types.bbclass: what's with "elf" and "cpio.gz"?

[Richard Purdie]

On Sat, 2013-01-12 at 06:22 -0500, Robert P. J. Day wrote:
> this snippet from image_types.bbclass:
> 
>     if "elf" in alltypes:
>         alltypes.remove("elf")
>         if "cpio.gz" not in alltypes:
>                 alltypes.append("cpio.gz")
>         alltypes.append("elf")
> 
>     # Filter out all the compressed images from types
>     for type in alltypes:
>     ... snip ...
> 
> i was going to trivially fix that comment (it's now "alltypes", not
> "types"), but i was distracted by the code above that -- what the heck
> is going on there?  what's the rationale for removing elf, then adding
> cpio.gz and elf back on at the end?  if someone can clarify that, i
> can add that as more comment, as well as a few other tweaks in that
> file for more documentation for anyone reading TFS.  thanks.

Order is important. The elf image depends on the cpio.gz image already
having been created so a) it must be created and b) it must happen
before elf.

Cheers,

Richard

Re: image_types.bbclass: what's with "elf" and "cpio.gz"?

[Chris Larson]

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

On Sat, Jan 12, 2013 at 6:05 AM, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> > i was going to trivially fix that comment (it's now "alltypes", not
> > "types"), but i was distracted by the code above that -- what the heck
> > is going on there?  what's the rationale for removing elf, then adding
> > cpio.gz and elf back on at the end?  if someone can clarify that, i
> > can add that as more comment, as well as a few other tweaks in that
> > file for more documentation for anyone reading TFS.  thanks.
>
> Order is important. The elf image depends on the cpio.gz image already
> having been created so a) it must be created and b) it must happen
> before elf.


I'd think we could implement a data-driven method for this, rather than
hardcoding this knowledge. Either declare the deps via a flag, or have one
directly call the other.
-- 
Christopher Larson

[-- Attachment #2: Type: text/html, Size: 1301 bytes --]

Re: image_types.bbclass: what's with "elf" and "cpio.gz"?

[Robert P. J. Day]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2641 bytes --]

On Sat, 12 Jan 2013, Chris Larson wrote:

>
> On Sat, Jan 12, 2013 at 6:05 AM, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
>       > i was going to trivially fix that comment (it's now "alltypes", not
>       > "types"), but i was distracted by the code above that -- what the heck
>       > is going on there?  what's the rationale for removing elf, then adding
>       > cpio.gz and elf back on at the end?  if someone can clarify that, i
>       > can add that as more comment, as well as a few other tweaks in that
>       > file for more documentation for anyone reading TFS.  thanks.
>
> Order is important.

  we'll come back to this shortly.

> The elf image depends on the cpio.gz image already having been
> created so a) it must be created and b) it must happen before elf.

  i'd immediately assumed that's what was going on -- what confused me
was that i was unaware that ordering was important in simply
*tracking* the collection of types one wanted to create.  (it isn't,
of course, but the code led me to believe that.)

> I'd think we could implement a data-driven method for this, rather
> than hardcoding this knowledge. Either declare the deps via a flag,
> or have one directly call the other.

  yes, any of the above, since in many places, the code seems
unnecessarily confusing by implementing with (ordered) lists what
should be supported by sets.  for example, as in this case, the
collection of image types should be a set to not have to mess with
additional code checking for prior membership, uniqueness, etc.

  and yet, in files like image_types.bbclass, you read unnecessarily
complicated code like:

        if "ext3" not in types:
            types.append("ext3")
        if "ext3" not in alltypes:
            alltypes.append("ext3")

simply because "types" is a list and not a set.  things like that
should be handled by sets, and any order-dependent processing should
be left until the very end and hidden from normal view in whatever
specific routine *needs* to care about order.  as you suggest,
data-driven, deps via flags, whatever.

  anyway, just my $0.02 at way too early on a sunday morning.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

Re: image_types.bbclass: what's with "elf" and "cpio.gz"?

[Richard Purdie]

On Sat, 2013-01-12 at 12:15 -0700, Chris Larson wrote:

>         Order is important. The elf image depends on the cpio.gz image
>         already
>         having been created so a) it must be created and b) it must
>         happen
>         before elf.
> 
> I'd think we could implement a data-driven method for this, rather
> than hardcoding this knowledge. Either declare the deps via a flag, or
> have one directly call the other.

Calling one from the other would cause issues like knowing when the
image type had already run (or cause it to run multiple times) but the
deps idea in flags would be better I agree. Its just the small matter of
implementing it and the complexities that entails.

With pseudo now able to safely store state, there is probably no excuse
not to split the rootfs task into multiple pieces and have the image
creation parts as separate tasks. This would mean we can use the normal
dependency logic and not have to reinvent the wheel. It would also allow
parallelism too, all with existing code. 

The more I think about this, the more I like the idea...

Cheers,

Richard