Re: [RFC PATCH 2/2] recipes-core: Add recipe for base-files

["vijaikumar....@gmail.com" <vijaikumar.kanagarajan@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 13553 bytes --]



On Thursday, February 25, 2021 at 9:27:51 AM UTC+5:30 
vijaikumar....@gmail.com wrote:

> On Wed, Feb 24, 2021 at 5:43 PM Henning Schild 
> <henning...@siemens.com> wrote: 
> > 
> > Am Tue, 23 Feb 2021 14:08:29 +0530 
> > schrieb vijai kumar <vijaikumar....@gmail.com>: 
> > 
> > > On Sat, Feb 20, 2021 at 1:59 PM Henning Schild 
> > > <henning...@siemens.com> wrote: 
> > > > 
> > > > Am Sat, 20 Feb 2021 01:27:19 +0530 
> > > > schrieb Vijai Kumar K <Vijaikumar_...@mentor.com>: 
> > > > 
> > > > > /etc/os-release is a symlink to /usr/lib/os-release and belongs to 
> > > > > the base-files package. 
> > > > > 
> > > > > ISAR has been modifying the /etc/os-release during postprocessing 
> > > > > to inject custom data onto it. 
> > > > > 
> > > > > Since this file belongs to base-files, an update/reinstall of the 
> > > > > package would overwrite the file with the one provided by 
> > > > > base-files. 
> > > > 
> > > > To some degree such an update would be right to do so. The BUILD_ID 
> > > > will for sure be invalidated, other custom fields might still be 
> > > > valid though. 
> > > > 
> > > > > Instead of modifying the contents of /etc/os-release during 
> > > > > post-processing, provide a modified base-files recipe in ISAR 
> > > > > which provides the similar changes in os-release. 
> > > > 
> > > > I am beginning to wonder if we have to write certain bits to other 
> > > > files. Bits that need to go into /etc/os-release could be appended 
> > > > with a hook 
> > > > See isar-disable-apt-cache, or we use the divert that Silvano 
> > > > proposed. 
> > > 
> > > I have not used hooks before, but looks like it might help in our 
> > > case. 
> > > 
> > > > 
> > > > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com> 
> > > > > --- 
> > > > > .../recipes-core/images/isar-image-base.bb | 2 + 
> > > > > meta/classes/image-postproc-extension.bbclass | 40 -------------- 
> > > > > meta/classes/image.bbclass | 20 ------- 
> > > > > meta/recipes-core/base-files/base-files.bb | 6 ++ 
> > > > > meta/recipes-core/base-files/base-files.inc | 55 
> > > > > +++++++++++++++++++ 5 files changed, 63 insertions(+), 60 
> > > > > deletions(-) create mode 100644 
> > > > > meta/recipes-core/base-files/base-files.bb create mode 100644 
> > > > > meta/recipes-core/base-files/base-files.inc 
> > > > > 
> > > > > diff --git a/meta-isar/recipes-core/images/isar-image-base.bb 
> > > > > b/meta-isar/recipes-core/images/isar-image-base.bb index 
> > > > > b381d85..4aa7e66 100644 --- 
> > > > > a/meta-isar/recipes-core/images/isar-image-base.bb +++ 
> > > > > b/meta-isar/recipes-core/images/isar-image-base.bb @@ -11,3 +11,5 
> > > > > @@ LIC_FILES_CHKSUM = 
> > > > > "file://${LAYERDIR_core}/licenses/COPYING.GPLv2;md5=751419260 PV = 
> > > > > "1.0" inherit image 
> > > > > + 
> > > > > +IMAGE_INSTALL += "base-files" 
> > > > > diff --git a/meta/classes/image-postproc-extension.bbclass 
> > > > > b/meta/classes/image-postproc-extension.bbclass index 
> > > > > 3f00c21..22c6a95 100644 --- 
> > > > > a/meta/classes/image-postproc-extension.bbclass +++ 
> > > > > b/meta/classes/image-postproc-extension.bbclass @@ -1,38 +1,6 @@ 
> > > > > # This software is a part of ISAR. 
> > > > > # Copyright (C) Siemens AG, 2019 
> > > > > 
> > > > > -update_etc_os_release() { 
> > > > > - OS_RELEASE_BUILD_ID="" 
> > > > > - OS_RELEASE_VARIANT="" 
> > > > > - OS_RELEASE_VARIANT_VERSION="" 
> > > > > - while true; do 
> > > > > - case "$1" in 
> > > > > - --build-id) OS_RELEASE_BUILD_ID=$2; shift ;; 
> > > > > - --variant) OS_RELEASE_VARIANT=$2; shift ;; 
> > > > > - --version) OS_RELEASE_VARIANT_VERSION=$2; shift ;; 
> > > > > - -*) bbfatal "$0: invalid option specified: $1" ;; 
> > > > > - *) break ;; 
> > > > > - esac 
> > > > > - shift 
> > > > > - done 
> > > > > - 
> > > > > - if [ -n "${OS_RELEASE_BUILD_ID}" ]; then 
> > > > > - sudo sed -i '/^BUILD_ID=.*/d' 
> > > > > '${IMAGE_ROOTFS}/etc/os-release' 
> > > > > - echo "BUILD_ID=\"${OS_RELEASE_BUILD_ID}\"" | \ 
> > > > > - sudo tee -a '${IMAGE_ROOTFS}/etc/os-release' 
> > > > > - fi 
> > > > > - if [ -n "${OS_RELEASE_VARIANT}" ]; then 
> > > > > - sudo sed -i '/^VARIANT=.*/d' 
> > > > > '${IMAGE_ROOTFS}/etc/os-release' 
> > > > > - echo "VARIANT=\"${OS_RELEASE_VARIANT}\"" | \ 
> > > > > - sudo tee -a '${IMAGE_ROOTFS}/etc/os-release' 
> > > > > - fi 
> > > > > - if [ -n "${OS_RELEASE_VARIANT_VERSION}" ]; then 
> > > > > - sudo sed -i '/^ISAR_IMAGE_VERSION=.*/d' 
> > > > > '${IMAGE_ROOTFS}/etc/os-release' 
> > > > > - echo "VARIANT_VERSION=\"${PV}\"" | \ 
> > > > > - sudo tee -a '${IMAGE_ROOTFS}/etc/os-release' 
> > > > > - fi 
> > > > > -} 
> > > > 
> > > > This is in the image for a good reason, it can not be part of a 
> > > > package, otherwise it would already be and we might not have that 
> > > > evil postprocess feature. 
> > > > 
> > > > Try a run, append a package to IMAGE_PREINSTALL, build again 
> > > > 
> > > > I kind of bet that BUILD_ID will be wrong and not show your new 
> > > > commit or "dirty" 
> > > 
> > > Yes, It is wrong. 
> > > 
> > > Since we definitely know that we are bringing in a variable 
> > > that changes everytime when there is a change to the source code, we 
> > > could very well do do_prepare_build[nostamp]="1". 
> > > 
> > > That with some changes should solve this problem. 
> > 
> > It will mean that every change will trigger a full reinstall of the 
> > image, many changes do that, but it sounds bad. 
> > 
> > And the false-sharing between images and multiconfigs still remains. 
> > 
> > Try two images in one layer with differing DESCRIPTIONS and see what 
> > happens. That would be the simple case without even thinking mc 
>
> So basically os-release varies based on the image built. With a single 
> package 
> like this, it is no longer possible. At anypoint we could have only one 
> version 
> of the package. 
>

Basically we just considered each image to be a "derivative" . Which I 
don't suppose
is the case, at least with upstream ISAR.
 

>
> Then, again, it makes me wonder if os-release is an apt place for such 
> information. 
> We lose updatability with the other approach. We definitely leave one 
> file (/etc/os-release or /usr/lib/os-release) 
> outdated on a dist upgrade. 


> Thanks, 
> Vijai Kumar K 
>
> > 
> > Henning 
> > 
> > > Thanks, 
> > > Vijai Kumar K 
> > > 
> > > > 
> > > > > ROOTFS_POSTPROCESS_COMMAND =+ "image_postprocess_configure" 
> > > > > image_postprocess_configure() { 
> > > > > # Configure root filesystem 
> > > > > @@ -45,14 +13,6 @@ image_postprocess_configure() { 
> > > > > fi 
> > > > > } 
> > > > > 
> > > > > -ROOTFS_POSTPROCESS_COMMAND =+ "image_postprocess_mark" 
> > > > > - 
> > > > > -image_postprocess_mark() { 
> > > > > - BUILD_ID=$(get_build_id) 
> > > > > - update_etc_os_release \ 
> > > > > - --build-id "${BUILD_ID}" --variant "${DESCRIPTION}" 
> > > > > --version "${PV}" -} 
> > > > > - 
> > > > > ROOTFS_POSTPROCESS_COMMAND =+ "image_postprocess_machine_id" 
> > > > > image_postprocess_machine_id() { 
> > > > > # systemd(1) takes care of recreating the machine-id on first 
> > > > > boot diff --git a/meta/classes/image.bbclass 
> > > > > b/meta/classes/image.bbclass index eddc444..d849175 100644 
> > > > > --- a/meta/classes/image.bbclass 
> > > > > +++ b/meta/classes/image.bbclass 
> > > > > @@ -49,9 +49,6 @@ SRC_URI += "${@ cfg_script(d) }" 
> > > > > 
> > > > > DEPENDS += "${IMAGE_INSTALL}" 
> > > > > 
> > > > > -ISAR_RELEASE_CMD_DEFAULT = "git -C ${LAYERDIR_core} describe 
> > > > > --tags --dirty --match 'v[0-9].[0-9]*'" -ISAR_RELEASE_CMD ?= 
> > > > > "${ISAR_RELEASE_CMD_DEFAULT}" - 
> > > > > image_do_mounts() { 
> > > > > sudo flock ${MOUNT_LOCKFILE} -c ' \ 
> > > > > mkdir -p "${BUILDROOT_DEPLOY}" "${BUILDROOT_ROOTFS}" 
> > > > > "${BUILDROOT_WORK}" @@ -91,23 +88,6 @@ def get_rootfs_size(d): 
> > > > > 
> > > > > return base_size + rootfs_extra * 1024 
> > > > > 
> > > > > -# here we call a command that should describe your whole build 
> > > > > system, -# this could be "git describe" or something similar. 
> > > > > -# set ISAR_RELEASE_CMD to customize, or override do_mark_rootfs 
> > > > > to do something -# completely different 
> > > > > -get_build_id() { 
> > > > > - if [ $(echo ${BBLAYERS} | wc -w) -ne 2 ] && 
> > > > > - [ "${ISAR_RELEASE_CMD}" = "${ISAR_RELEASE_CMD_DEFAULT}" 
> > > > > ]; then 
> > > > > - bbwarn "You are using external layers that will not 
> > > > > be" \ 
> > > > > - "considered in the build_id. Consider 
> > > > > changing" \ 
> > > > > - "ISAR_RELEASE_CMD." 
> > > > > - fi 
> > > > > - if ! ( ${ISAR_RELEASE_CMD} ) 2>/dev/null; then 
> > > > > - bbwarn "\"${ISAR_RELEASE_CMD}\" failed, returning 
> > > > > empty build_id." 
> > > > > - echo "" 
> > > > > - fi 
> > > > > -} 
> > > > > - 
> > > > > python set_image_size () { 
> > > > > rootfs_size = get_rootfs_size(d) 
> > > > > d.setVar('ROOTFS_SIZE', str(rootfs_size)) 
> > > > > diff --git a/meta/recipes-core/base-files/base-files.bb 
> > > > > b/meta/recipes-core/base-files/base-files.bb new file mode 100644 
> > > > > index 0000000..d250fc5 
> > > > > --- /dev/null 
> > > > > +++ b/meta/recipes-core/base-files/base-files.bb 
> > > > > @@ -0,0 +1,6 @@ 
> > > > > +# This software is a part of ISAR. 
> > > > > +# Copyright (c) Mentor, A Siemens Business, 2021 
> > > > > +# 
> > > > > +# SPDX-License-Identifier: MIT 
> > > > > + 
> > > > > +require base-files.inc 
> > > > > diff --git a/meta/recipes-core/base-files/base-files.inc 
> > > > > b/meta/recipes-core/base-files/base-files.inc new file mode 100644 
> > > > > index 0000000..68d0e3a 
> > > > > --- /dev/null 
> > > > > +++ b/meta/recipes-core/base-files/base-files.inc 
> > > > > @@ -0,0 +1,55 @@ 
> > > > > +# This software is a part of ISAR. 
> > > > > +# Copyright (c) Mentor, A Siemens Business, 2021 
> > > > > +# 
> > > > > +# SPDX-License-Identifier: MIT 
> > > > > + 
> > > > > +inherit dpkg 
> > > > > + 
> > > > > +SRC_URI = "apt://${PN}" 
> > > > > + 
> > > > > +S="${WORKDIR}/${PN}" 
> > > > > + 
> > > > > +MAINTAINER = "isar-users <isar-...@googlegroups.com>" 
> > > > > +CHANGELOG_V = "<orig-version>+isar" 
> > > > 
> > > > Just one such package. We will need for for every single image we 
> > > > build in the tree. There can be multiple, especially when 
> > > > multiconfig comes into play, but even without. 
> > > > 
> > > > Think of the internal layer you just helped with for a release, it 
> > > > builds the product and the debug variant, which differ in their 
> > > > DESCRIPTION and must not share that package. 
> > > > 
> > > > I am afraid a package will not work. We would need a package per 
> > > > image. And that package would need to rebuild every time the 
> > > > BUILD_ID changes. 
> > > > 
> > > > But now you still have the problem that someone could install a more 
> > > > recent "base-files" ... Might as well use preferences to pin it 
> > > > forever, instead of forking it and giving it a number that will 
> > > > hopefully never be smaller than the one from debian 
> > > > 
> > > > Henning 
> > > > 
> > > > > +ISAR_RELEASE_CMD_DEFAULT = "git -C ${LAYERDIR_core} describe 
> > > > > --tags --dirty --match 'v[0-9].[0-9]*'" +ISAR_RELEASE_CMD ?= 
> > > > > "${ISAR_RELEASE_CMD_DEFAULT}" +OS_RELEASE_VARIANT = "Isar target 
> > > > > filesystem" +OS_RELEASE_VARIANT_VERSION = "1.0" 
> > > > > + 
> > > > > +# here we call a command that should describe your whole build 
> > > > > system, +# this could be "git describe" or something similar. 
> > > > > +# set ISAR_RELEASE_CMD to customize, or override do_mark_rootfs 
> > > > > to do something +# completely different 
> > > > > +get_build_id() { 
> > > > > + if [ $(echo ${BBLAYERS} | wc -w) -ne 2 ] && 
> > > > > + [ "${ISAR_RELEASE_CMD}" = "${ISAR_RELEASE_CMD_DEFAULT}" ]; 
> > > > > then 
> > > > > + bbwarn "You are using external layers that will not 
> > > > > be" \ 
> > > > > + "considered in the build_id. Consider 
> > > > > changing" \ 
> > > > > + "ISAR_RELEASE_CMD." 
> > > > > + fi 
> > > > > + if ! ( ${ISAR_RELEASE_CMD} ) 2>/dev/null; then 
> > > > > + bbwarn "\"${ISAR_RELEASE_CMD}\" failed, returning empty 
> > > > > build_id." 
> > > > > + echo "" 
> > > > > + fi 
> > > > > +} 
> > > > > + 
> > > > > +do_prepare_build() { 
> > > > > + deb_add_changelog 
> > > > > + OS_RELEASE_BUILD_ID=$(get_build_id) 
> > > > > + if [ -n "${OS_RELEASE_BUILD_ID}" ]; then 
> > > > > + sed -i '/^BUILD_ID=.*/d' '${S}/etc/os-release' 
> > > > > + echo "BUILD_ID=\"${OS_RELEASE_BUILD_ID}\"" | \ 
> > > > > + tee -a '${S}/etc/os-release' 
> > > > > + fi 
> > > > > + if [ -n "${OS_RELEASE_VARIANT}" ]; then 
> > > > > + sed -i '/^VARIANT=.*/d' '${S}/etc/os-release' 
> > > > > + echo "VARIANT=\"${OS_RELEASE_VARIANT}\"" | \ 
> > > > > + tee -a '${S}/etc/os-release' 
> > > > > + fi 
> > > > > + if [ -n "${OS_RELEASE_VARIANT_VERSION}" ]; then 
> > > > > + sed -i '/^VARIANT_VERSION=.*/d' '${S}/etc/os-release' 
> > > > > + echo "VARIANT_VERSION=\"${OS_RELEASE_VARIANT_VERSION}\"" 
> > > > > | \ 
> > > > > + tee -a '${S}/etc/os-release' 
> > > > > + fi 
> > > > > +} 
> > > > 
> > > > -- 
> > > > You received this message because you are subscribed to the Google 
> > > > Groups "isar-users" group. To unsubscribe from this group and stop 
> > > > receiving emails from it, send an email to 
> > > > isar-users+...@googlegroups.com. To view this discussion on 
> > > > the web visit 
> > > > 
> https://groups.google.com/d/msgid/isar-users/20210220092859.382eeb3a%40md1za8fc.ad001.siemens.net. 
>
> > > > 
> > 
>

[-- Attachment #1.2: Type: text/html, Size: 21340 bytes --]