[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Providing (armhf) u-boot images together with d-i images?



On Sat, 2014-12-27 at 00:00 +0100, Karsten Merker wrote:
> attached is V2 of the patchset. Changes since V1:

I should start by saying that I'm not personally particularly interested
in this functionality, but I don't want to be a blocker for people who
are so since I've been asked I'll give my 2pence...

I've been wondering if this image generation should be in debian
installer or if it should be a separate project (similar to how
debian-cd is separate). The advantage of doing it separately would be
that the images can be rev'd or expanded (e.g. to add a new platform)
independently from the installer releases, plus we don't need to worry
so much about exploding the size of the main installer releases as much.
Just an idle thought really.

Please could you post the diff in the lists of files generated by the
build, including the sizes of the new stuff.

FWIW review would be a lot easier if the patches were patchbombed to the
list in the normal git send-email way rather than as multiple
attachments on a single mail (people can kill-thread if they aren't
interested), but here we go:

0001-Add-boot-arm-u-boot-image-config.patch

        Seems fine.

0002-Provide-u-boot-binaries-for-armhf-systems-without-u-.patch

        There would be a lot less shell-in-make quoting faff if there
        was a helper under util/ with the bulk of the code in.

        I think this isn't actually publishing the u-boot binaries, but
        rather sd-card images with the u-boot inlined, is that right? Is
        there any use in shipping the raw binaries too?

0003-Add-utils-gen-hd-image.patch

        I didn't review in detail, but perhaps the functionality of
        patch 0002 could be folded in? TBH I'm not sure what the
        distinction is between what 0002 does and what this script would
        produce (except I can see the script clearly has more
        functionality)

0004-Add-SD-card-image-build-support-for-hd-media-builds-.patch

        I don't think we need all of (.gz|.bz2|.xz) (like the README
        suggests, although it's not clear these are all actually
        present). Just one would do IMHO.

        It seems to be creating a copy of dtbs again, please lets try
        and keep it to one set of these files in the top level
        component.

        The README should describe how to actually do the concatenation.

0005-Add-SD-card-image-and-tftpboot-tarball-build-support.patch

        What is this providing? Is it a mini.iso like netboot sd image?
        I'm not sure how this is different from what the previous
        patches introduced.
        
        For the tftpboot part, u-boot supports standard pxelinux.cfg
        syntax configuration files -- would it be better to just
        generate a suitable one of those?

0006-Add-additional-dependencies-needed-for-building-boot.patch

        Seems ok. But by having this last at least half the previous
        patches won't work in isolation. Better IMHO to add each
        dependency as the dependency is introduced.

I suppose my main overall concern is that this seems to be providing the
same thing 3 or 4 times and I'm not sure what the difference between
each of those things is (or perhaps as likely I've misunderstood what is
going on somewhere along the line). I was already concerned about the
proliferation of images which taking this approach was implying in
general and adding a multiplier to that just makes me more
uncomfortable.

Perhaps a summary of the new toplevel output directories (i.e. things
added alongside netboot, hd-media, network-console etc) describing what
each one is would help?

Ian.


Reply to: