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

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

Am Thu, 25 Feb 2021 09:27:38 +0530
schrieb vijai kumar <vijaikumar.kanagarajan@gmail.com>:

> On Wed, Feb 24, 2021 at 5:43 PM Henning Schild
> <henning.schild@siemens.com> wrote:
> >
> > Am Tue, 23 Feb 2021 14:08:29 +0530
> > schrieb vijai kumar <vijaikumar.kanagarajan@gmail.com>:
> >  
> > > On Sat, Feb 20, 2021 at 1:59 PM Henning Schild
> > > <henning.schild@siemens.com> wrote:  
> > > >
> > > > Am Sat, 20 Feb 2021 01:27:19 +0530
> > > > schrieb Vijai Kumar K <Vijaikumar_Kanagarajan@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_Kanagarajan@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.
> 
> 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.

Exactly what i said earlier. We should use another file. And in case we
want to keep some information really in /etc/os-release we need to
write a hook to merge our bits in every single time apt owns it again.

Henning

> 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-users@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+unsubscribe@googlegroups.com. To view this
> > > > discussion on the web visit
> > > > https://groups.google.com/d/msgid/isar-users/20210220092859.382eeb3a%40md1za8fc.ad001.siemens.net.
> > > >  
> >