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

[Henning Schild <henning.schild@siemens.com>]

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"?

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.

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.

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: