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: 13051 bytes --]



On Monday, March 1, 2021 at 1:10:10 PM UTC+5:30 Henning Schild wrote:

> Am Sun, 28 Feb 2021 21:07:05 -0800 (PST) 
> schrieb "vijaikumar....@gmail.com" <vijaikumar....@gmail.com>: 
>
> > On Thursday, February 25, 2021 at 10:07:55 PM UTC+5:30 Henning Schild 
> > wrote: 
> > 
> > > Am Thu, 25 Feb 2021 01:06:16 -0800 (PST) 
> > > schrieb "vijaikumar....@gmail.com" <vijaikumar....@gmail.com>: 
> > > 
> > > > On Thursday, February 25, 2021 at 1:56:02 PM UTC+5:30 Jan Kiszka 
> > > > wrote: 
> > > > 
> > > > > On 25.02.21 06:08, vijaikumar....@gmail.com wrote: 
> > > > > > 
> > > > > > 
> > > > > > 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 
> > > > > > <http://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 
> > > > > > <http://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 
> > > > > > <http://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 
> > > > > > <http://isar-image-base.bb> 
> > > > > > > > > > b/meta-isar/recipes-core/images/isar-image-base.bb 
> > > > > > <http://isar-image-base.bb> index 
> > > > > > > > > > b381d85..4aa7e66 100644 --- 
> > > > > > > > > > a/meta-isar/recipes-core/images/isar-image-base.bb 
> > > > > > <http://isar-image-base.bb> +++ 
> > > > > > > > > > b/meta-isar/recipes-core/images/isar-image-base.bb 
> > > > > > <http://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. 
> > > > > > 
> > > > > 
> > > > > So far, we did not consider os-release in package-based update 
> > > > > scenarios (likely because all our updates were partition based 
> > > > > - which doesn't mean package-based will not come one day as 
> > > > > well). If we include package-based updates, os-release should 
> > > > > be updated by a package as well. 
> > > > > 
> > > > > Our os-release modification (ISAR_RELEASE_CMD) is bound to an 
> > > > > image. An image is not a package, but we could make it depend 
> > > > > on an image-specific package carrying or generating that 
> > > > > os-release data. That package should definitely NOT be 
> > > > > base-files. We need to divert that file and keep it under our 
> > > > > control. Obviously, if the user decides to do a an upgrade of 
> > > > > the Debian version on the device without pulling a new image 
> > > > > version package, things will really divert - but that is no 
> > > > > reasonable use case IMHO. 
> > > > 
> > > > I am not quite sure if I get all that. We are now talking about a 
> > > > new file in a 
> > > > new package that depends on image recipe and have it diverted? So 
> > > > that package would not be in the final image. But incase you need 
> > > > to upgrade you would 
> > > > make sure that the package is pulled by making it depend on a 
> > > > specific base-files 
> > > > package. When base-files upgrades the package would be upgraded 
> > > > to? 
> > > 
> > > My suggestion is to not package that, just like we used to. We just 
> > > put our stuff in a file via the image postinst. We write a package 
> > > to "reserve" that file name against collision, that file will be 
> > > empty in the package, but the hook and its script will live there. 
> > > 
> > > Say we had a package isar-base-files that would contain 
> > > /etc/isar-release and /etc/apt/...hook.conf -> /usr/bin/merger.sh 
> > > 
> > > merger.sh: trigger on every base-files update 
> > > for key in isar-release and key starts with "OS_": 
> > > if key in os-release: 
> > > update 
> > > else 
> > > append 
> > > 
> > > Now we might write DESCRIPTION to /etc/os-release and isar-release 
> > > and BUILD_ID just to os-release ... because the latter is 
> > > invalidated with any sort of live update. 
> > > And yes you can use that to give Debian another name and that will 
> > > survive updates ... because branding is very important! 
> > > 
> > > Henning 
> > > 
> > 
> > 
> > Thanks for the pointer Henning. I will try this implementation out. I 
> > hope no one else is working on this approach. 
>
> I do not think anyone else is currently working on that. And i hope it 
> can work like i described. One thing seems pretty certain ... it is a 
> very bad idea to "brand" debian. Modifications to that file should 
> probably be kept to an absolute minimum. Additional stuff 
> should probably go into additional files. 


> Just to keep that in mind, because the projects you work on seem to 
> have a strong desire on branding. That should be questioned instead of 
> implemented. The initial idea to go for /etc/os-release was limited to 
> two variables and "no updates with apt". 
>

I am not sure why you feel it is a bad idea, there has always been a lot of 
derivatives of Debian. And ISAR as
a tool could provide an example on how to do it. Branding is another 
discussion, and irrespective of that, we
are just trying to find a way to add additional fields to /etc/os-release 
and fix this upgrade issue :) Having custom
base-files was one such approach(had limitations). Since you have an 
entirely new suggestion, I would like to try that out to see
how it becomes a candidate to solve the problem in hand. :)

Thanks,
Vijai Kumar K
 

>
> regards, 
> Henning 
>
> > 
> > > > Thanks, 
> > > > Vijai Kumar K 
> > > > 
> > > > 
> > > > > Jan 
> > > > > 
> > > > > -- 
> > > > > Siemens AG, T RDA IOT 
> > > > > Corporate Competence Center Embedded Linux 
> > > > > 
> > > > 
> > > 
> > > 
> > 
>
>

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