Re: [PATCH] image.bbclass: remove 'get_image_name' function

[Claudius Heine <claudius.heine.ext@siemens.com>]

Hi Henning,

On 23/05/2019 14.46, Henning Schild wrote:
> Am Tue, 21 May 2019 16:28:18 +0200
> schrieb "[ext] claudius.heine.ext@siemens.com"
> <claudius.heine.ext@siemens.com>:
> 
>> From: Claudius Heine <ch@denx.de>
>>
>> This function is only used in 'do_copy_boot_files' where a simple
>> shell function fits better.
>>
>> Signed-off-by: Claudius Heine <ch@denx.de>
>> ---
>>   meta/classes/image.bbclass | 33 ++++++---------------------------
>>   1 file changed, 6 insertions(+), 27 deletions(-)
>>
>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>> index 3eb261f..45ed8ba 100644
>> --- a/meta/classes/image.bbclass
>> +++ b/meta/classes/image.bbclass
>> @@ -64,29 +64,6 @@ inherit image-postproc-extension
>>   # Extra space for rootfs in MB
>>   ROOTFS_EXTRA ?= "64"
>>   
>> -def get_image_name(d, name_link):
>> -    S = d.getVar("IMAGE_ROOTFS", True)
>> -    path_link = os.path.join(S, name_link)
>> -
>> -    # If path_link does not exist, it might be a symlink
>> -    # in the target rootfs.  This block attempts to resolve
>> -    # it relative to the rootfs location.
>> -    if not os.path.exists(path_link):
>> -        path_link = os.path.join(
>> -            S,
>> -            os.path.relpath(
>> -                os.path.realpath(path_link),
>> -                "/",
>> -            ),
>> -        )
>> -
>> -    if os.path.exists(path_link):
>> -        base = os.path.basename(os.path.realpath(path_link))
>> -        full = d.getVar("IMAGE_FULLNAME", True) + "." + base
>> -        return [base, full]
>> -
>> -    return ["", ""]
>> -
>>   def get_rootfs_size(d):
>>       import subprocess
>>       rootfs_extra = int(d.getVar("ROOTFS_EXTRA", True))
>> @@ -149,12 +126,14 @@ addtask rootfs_install before do_build after
>> do_unpack
>>   do_copy_boot_files[dirs] = "${DEPLOY_DIR_IMAGE}"
>>   do_copy_boot_files() {
>> -    if [ -n "${KERNEL_IMAGE}" ]; then
>> -        cp -f ${IMAGE_ROOTFS}/boot/${@get_image_name(d,
>> 'vmlinuz')[0]} ${DEPLOY_DIR_IMAGE}/${KERNEL_IMAGE}
>> +    kernel="$(realpath -q '${IMAGE_ROOTFS}/vmlinuz')"
> 
> Will this work if ${IMAGE_ROOTFS}/vmlinuz is a symlink to
> "/boot/vmlinuz-foo-bar"?

Do you have a different solution in mind?

Those links are created by linux-update-symlinks from the linux-base 
package and I think that using absolute paths there will probably cause 
many different issues, so I don't think they did that accidentally.

I tried around for a while to cover all different cases, but I stopped 
because don't think that makes sense here until we find a case where 
those links are absolute.

> The funny thing with the kernel and the initrd is that we should
> probably stop deploying them anyways. As far as i know there are
> probably 2 users of them. The first is the qemu test script, which
> should be fixed to use IMAGE_RFS/vmlinuz and deal with possible
> symlinks.
> The second one is the legacy bios wic. This one is also just broken
> because it creates a boot partition with copies of kernel and initrd,
> that debian does not even know about. So you will also never be able to
> update the kernel with apt ... If any other imager needs the deployed
> KERNEL and INITRD ... that other imager probably has the same issue.

If a post-install step removes the kernel+initrd from the /boot, then 
the fitimage class still needs to find the kernel+dtb and possible the 
initrd. This would be the case if you have a project that should be 
deployed on a small nand. Updating via apt is not important there.

> So in the long run we should just drop that and fix those imagers to
> turn the real /boot into the boot partition and fix up /etc/fstab.

You don't have a boot partition on some devices.

regards,
Claudius

> 
> Henning
> 
>> +    if [ -n "$kernel" ]; then
>> +        cp -f "$kernel" '${DEPLOY_DIR_IMAGE}/${KERNEL_IMAGE}'
>>       fi
>>   
>> -    if [ -n "${INITRD_IMAGE}" ]; then
>> -        sudo cp -f ${IMAGE_ROOTFS}/boot/${@get_image_name(d,
>> 'initrd.img')[0]} ${DEPLOY_DIR_IMAGE}/${INITRD_IMAGE}
>> +    initrd="$(realpath -q '${IMAGE_ROOTFS}/initrd.img')"
>> +    if [ -n "$initrd" ]; then
>> +        cp -f "$initrd" '${DEPLOY_DIR_IMAGE}/${INITRD_IMAGE}'
>>       fi
>>   
>>       # Check DTB_FILE via inline python to handle unset case:
> 

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de