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

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

[claudius.heine.ext]

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')"
+    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:
-- 
2.20.1

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

[Henning Schild]

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:

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

[Claudius Heine]

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

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

[Maxim Yu. Osipov]

On 5/21/19 4:28 PM, claudius.heine.ext@siemens.com wrote:
> From: Claudius Heine <ch@denx.de>
> 
> This function is only used in 'do_copy_boot_files' where a simple shell
> function fits better.

Applied to the 'next'.

Maxim.

> 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')"
> +    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:
> 


-- 
Maxim Osipov
ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn
Germany
+49 (151) 6517 6917
mosipov@ilbers.de
http://ilbers.de/
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov