Re: [PATCH v2 2/3] Revert "image.bbclass: make KERNEL_IMAGE & INITRD_IMAGE variable fixed"

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

Hi Maxim,

On 13/05/2019 11.01, Maxim Yu. Osipov wrote:
> On 5/13/19 10:11 AM, Claudius Heine wrote:
>> Hi Maxim,
>>
>> On 13/05/2019 09.39, Maxim Yu. Osipov wrote:
>>> On 5/13/19 9:31 AM, Claudius Heine wrote:
>>>> Hi Maxim,
>>>>
>>>> On 12/05/2019 21.30, Maxim Yu. Osipov wrote:
>>>>> This reverts commit b92589c574596ac98f97cf14369cbf62872e6481.
>>>>>
>>>>> vmlinuz and initrd are getting overwritten for the same machine
>>>>> by other distros under deploy/images/${MACHINE}
>>>>
>>>> NACK. I don't see a patch that addresses the issue described in the 
>>>> commit message of that reverted patch in this patchset in a 
>>>> different way, so this is a regression.
>>>
>>>
>>> Do you confirm that issue #53 appeared after applying your series 
>>> into the 'next'?
>>>
>>> just look in
>>> http://isar-build.org:8080/job/isar_next/ws/build/tmp/deploy/images/qemuarm/ 
>>>
>>>
>>> vmlinuz/initrd for stretch was overwritten by subsequent build for 
>>> buster.
>>
>> So would not the fix for that be to add the DISTRO variable to the 
>> image deploy path or the the vmlinuz/initrd file name?
> 
> I agree with the intention of your patch b92589c5 "image.bbclass: make 
> KERNEL_IMAGE & INITRD_IMAGE variable fixed"  - I also had headaches with 
> variables KERNEL_IMAGE/INITRD_IMAGE when trying to use them. 
> Unfortunately, this patch leads to the mentioned problem with 
> overwriting vmlinuz/initrd images for the same MACHINE. >
> In my opinion the MACHINE deploy images w/o the patch use good naming 
> scheme (see below).
> 
> 1) The version of kernel is worth to see,

I disagree, since the version of the kernel/initrd makes the file names 
in the deploy directory unpredictable, which is difficult to write 
documentation and scripts for.

Have you tried something like this?

   KERNEL_IMAGE ?= "${PF}.vmlinuz"
   INITRD_IMAGE ?= "${PF}.initrd.img"

regards,
Claudius

> not sure that one should add 
> another directory DISTRO and distro name is included into vmlinuz/initrd.
> 
> 
> http://isar-build.org:8080/job/isar_master/ws/build/tmp/deploy/images/qemuarm/ 
> 
> 
> isar-image-base-debian-buster-qemuarm.ext4.img
> isar-image-base-debian-buster-qemuarm.initrd.img-4.19.0-4-armmp
> isar-image-base-debian-buster-qemuarm.vmlinuz-4.19.0-4-armmp
> isar-image-base-debian-stretch-qemuarm.ext4.img
> isar-image-base-debian-stretch-qemuarm.initrd.img-4.9.0-9-armmp
> isar-image-base-debian-stretch-qemuarm.vmlinuz-4.9.0-9-armmp
> 
> 
> 2) Another point is the image name itself - just compare isar-image-base 
> and isar-image-debug - it's also worth to add image name into 
> vmlinuz/initrd, f.e.:
> 
> isar/build/tmp/deploy/images/qemuamd64:
> isar-image-base-debian-stretch-qemuamd64.initrd.img-4.9.0-9-amd64
> isar-image-base-debian-stretch-qemuamd64.wic.img 
> isar-image-base-debian-stretch-qemuamd64.vmlinuz-4.9.0-9-amd64
> isar-image-debug-debian-stretch-qemuamd64.vmlinuz-4.9.0-9-amd64
> isar-image-debug-debian-stretch-qemuamd64.initrd.img-4.9.0-9-amd64 
> isar-image-debug-debian-stretch-qemuamd64.wic.img

Your concerns are valid. We need to figure out a good system for the 
file names and paths.

> 
> Reagrds,
> Maxim.
> 
> 
> 
>> regards,
>> Claudius
>>
>>>
>>> Regards,
>>> Maxim.
>>>
>>>> regards,
>>>> Claudius
>>>>
>>>>>
>>>>> Closes: #56
>>>>> Signed-off-by: Maxim Yu. Osipov <mosipov@ilbers.de>
>>>>> ---
>>>>>   meta/classes/image.bbclass | 6 ++++--
>>>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>>>>> index fb35a4c..8e6f2c9 100644
>>>>> --- a/meta/classes/image.bbclass
>>>>> +++ b/meta/classes/image.bbclass
>>>>> @@ -13,8 +13,8 @@ IMAGE_INSTALL += "${@ ("linux-image-" + 
>>>>> d.getVar("KERNEL_NAME", True)) if d.getV
>>>>>   IMAGE_FULLNAME = "${PF}"
>>>>>   # These variables are used by wic and start_vm
>>>>> -KERNEL_IMAGE ?= "vmlinuz"
>>>>> -INITRD_IMAGE ?= "initrd.img"
>>>>> +KERNEL_IMAGE ?= "${@get_image_name(d, 'vmlinuz')[1]}"
>>>>> +INITRD_IMAGE ?= "${@get_image_name(d, 'initrd.img')[1]}"
>>>>>   # Useful variables for imager implementations:
>>>>>   PP = "/home/builder/${PN}"
>>>>> @@ -149,10 +149,12 @@ addtask rootfs_install before do_build after 
>>>>> do_unpack
>>>>>   do_copy_boot_files[dirs] = "${DEPLOY_DIR_IMAGE}"
>>>>>   do_copy_boot_files() {
>>>>> +    KERNEL_IMAGE=${@get_image_name(d, 'vmlinuz')[1]}
>>>>>       if [ -n "${KERNEL_IMAGE}" ]; then
>>>>>           cp -f ${IMAGE_ROOTFS}/boot/${@get_image_name(d, 
>>>>> 'vmlinuz')[0]} ${DEPLOY_DIR_IMAGE}/${KERNEL_IMAGE}
>>>>>       fi
>>>>> +    INITRD_IMAGE=${@get_image_name(d, 'initrd.img')[1]}
>>>>>       if [ -n "${INITRD_IMAGE}" ]; then
>>>>>           sudo cp -f ${IMAGE_ROOTFS}/boot/${@get_image_name(d, 
>>>>> 'initrd.img')[0]} ${DEPLOY_DIR_IMAGE}/${INITRD_IMAGE}
>>>>>       fi
>>>>>
>>>>
>>>
>>>
>>
> 
> 

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