Re: [PATCH v2 1/3] isar-bootstrap|configscript: implement one shot config option

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

Hi Henning,

On 2018-05-23 16:06, Henning Schild wrote:
> Am Wed, 23 May 2018 15:12:46 +0200
> schrieb "[ext] claudius.heine.ext@siemens.com"
> <claudius.heine.ext@siemens.com>:
> 
>> From: Claudius Heine <ch@denx.de>
>>
>> Some configuration tasks on the image require certain other packages,
>> but those packages might not be required on the final image.
>>
>> One way to solve this is use a special package that is installed to
>> the image and then removed again. When installing it makes certain
>> that all required packages are installed to the image as well. Then
>> the image can be configured in the postinst hook. When this package
>> is removed and all unused dependencies are removed as well, no not
>> required packages used by this package should remain on the image.
>>
>> Signed-off-by: Claudius Heine <ch@denx.de>
>> ---
>>   .../images/files/debian-configscript.sh       |  1 -
>>   .../images/files/raspbian-configscript.sh     |  1 -
>>   meta/classes/isar-bootstrap-helper.bbclass    | 23
>> +++++++++++++++---- 3 files changed, 19 insertions(+), 6 deletions(-)
>>
>> diff --git
>> a/meta-isar/recipes-core/images/files/debian-configscript.sh
>> b/meta-isar/recipes-core/images/files/debian-configscript.sh index
>> 7ca0562..9b3ff30 100755 ---
>> a/meta-isar/recipes-core/images/files/debian-configscript.sh +++
>> b/meta-isar/recipes-core/images/files/debian-configscript.sh @@ -40,4
>> +40,3 @@ fi # Purge unused locale and installed packages' .deb files
>>   localepurge
>> -apt-get clean
>> diff --git
>> a/meta-isar/recipes-core/images/files/raspbian-configscript.sh
>> b/meta-isar/recipes-core/images/files/raspbian-configscript.sh index
>> b240de9..448dea5 100644 ---
>> a/meta-isar/recipes-core/images/files/raspbian-configscript.sh +++
>> b/meta-isar/recipes-core/images/files/raspbian-configscript.sh @@
>> -40,4 +40,3 @@ kernel=$KERNEL_IMAGE EOF
>>   localepurge
>> -apt-get clean
>> diff --git a/meta/classes/isar-bootstrap-helper.bbclass
>> b/meta/classes/isar-bootstrap-helper.bbclass index 4284d02..76e20f6
>> 100644 --- a/meta/classes/isar-bootstrap-helper.bbclass
>> +++ b/meta/classes/isar-bootstrap-helper.bbclass
>> @@ -5,15 +5,17 @@
>>   #
>>   # SPDX-License-Identifier: MIT
>>   
>> +IMAGE_CFG_PACKAGE ??= ""
>> +
>>   setup_root_file_system() {
>>       ROOTFSDIR="$1"
>> -    CLEANHOSTLEAK="$2"
>> +    CLEAN="$2"
>>       shift
>>       shift
>>       PACKAGES="$@"
>>       APT_ARGS="install --yes --allow-unauthenticated \
>>                 -o Debug::pkgProblemResolver=yes"
> 
> I guess if you take the "install" out you can reuse that a few more
> times.

Where? I only see purge and autoremove that doesn't use it already. 
Replaceing '-y' with '${APT_ARGS}' there does seem like an overkill, 
does it not? The other options in it don't do much for purge and 
autoremove AFAIK.

If you think of this as a general improvement to have the command 
outside of the 'APT_ARGS' variable for readability reasons, then I 
agree, but that would be a different patch.

> 
>> -    CLEANHOSTLEAK_FILES="${ROOTFSDIR}/etc/hostname
>> ${ROOTFSDIR}/etc/resolv.conf"
>> +    CLEAN_FILES="${ROOTFSDIR}/etc/hostname
>> ${ROOTFSDIR}/etc/resolv.conf"
>>       sudo cp -Trpfx \
>>           "${DEPLOY_DIR_IMAGE}/isar-bootstrap-${DISTRO}-${DISTRO_ARCH}/"
>> \ @@ -41,8 +43,21 @@ setup_root_file_system() {
>>           -o Dir::Etc::sourceparts="-" \
>>           -o APT::Get::List-Cleanup="0"
>>       sudo -E chroot "$ROOTFSDIR" \
>> -        /usr/bin/apt-get ${APT_ARGS} --download-only $PACKAGES
>> -    [ "clean" = ${CLEANHOSTLEAK} ] && sudo rm -f
>> ${CLEANHOSTLEAK_FILES}
>> +        /usr/bin/apt-get ${APT_ARGS} --download-only $PACKAGES \
>> +            ${IMAGE_CFG_PACKAGE}
>> +    [ "clean" = ${CLEAN} ] && sudo rm -f ${CLEAN_FILES}
>>       sudo -E chroot "$ROOTFSDIR" \
>>           /usr/bin/apt-get ${APT_ARGS} $PACKAGES
>> +    if [ -n "${IMAGE_CFG_PACKAGE}" ]; then
>> +        sudo -E chroot "$ROOTFSDIR" \
>> +            /usr/bin/apt-get ${APT_ARGS} ${IMAGE_CFG_PACKAGE}
> 
> Say we have multiple packages here, are we sure apt-get will get the
> order right?
> Would it make sense to use a loop to deal with potential lists and
> their order?
> 
> Henning
> 
>> +        sudo -E chroot "$ROOTFSDIR" \
>> +            /usr/bin/apt-get purge -y ${IMAGE_CFG_PACKAGE}
>> +    fi
>> +    if [ "clean" = ${CLEAN} ]; then
>> +        sudo -E chroot "$ROOTFSDIR" \
>> +            /usr/bin/apt-get autoremove --purge -y
>> +        sudo -E chroot "$ROOTFSDIR" \
>> +            /usr/bin/apt-get clean
>> +    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