Code review comment for lp:~dooferlad/linaro-image-tools/copy-files-new-syntax

On 11 September 2012 10:37, Paul Sokolovsky <email address hidden> wrote:
01234567890123456789012345678901234567890123456789012345678901234567890123456789
> Just for the record, I don't think such extreme refactoring was required,
> and it being started, is not complete - instead of moving entire file
> processing to be package based and happen on the level of
> linaro-media-create, this still leaves bootloader.file, bootloader.spl_file
> as it was, just flips copy_files handling - instead of being done consistent
> with how bootloader.file/bootloader.spl_file works, it is now package based,
> but that means we have file-extraction code spread around both
> linaro-hwpack-create and linaro-media-create.
>
> But for the purpose of having it done, it's ok.

I agree that we could move all file extraction to the hardware pack
install stage, and it is something I considered. I think it is worth
doing as part of a separate update because it would change existing
functionality (well, change it more than has already been done as part
of supporting multiple boards and bootloaders). I don't know if anyone
is replacing bootloader binaries in a hardware pack after they have
created it with linaro-hwpack-create. If they are, we need to discuss
changes to the hardware pack format with them.

I didn't make the decision to start refactoring lightly. I only did
when it became clear that I was going to have to write some rather
ugly code to support the new copy_files format as well as
re-implementing storing files that exist in a package - it seemed that
just using packages would be a lot cleaner!

I expect that the reason for U-Boot and the second stage bootloader
binaries being in a separate folder in the hardware pack it is
historical - I could not see any mention of the reasons in the code!
It adds unnecessary complication to the hardware pack create and
install stages and now that we support multiple bootloaders and boards
in a single hardware pack, we have already missed some bugs around it,
which you fixed (thanks!)

Thanks for the other comments. I will address them now and get an
updated branch ready.

--
James Tunnicliffe