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

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

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

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