Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD

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

Am Wed, 24 Nov 2021 11:15:04 +0100
schrieb "Moessbauer, Felix (T RDA IOT SES-DE)"
<felix.moessbauer@siemens.com>:

> > -----Original Message-----
> > From: Schmidt, Adriaan (T RDA IOT SES-DE)
> > <adriaan.schmidt@siemens.com> Sent: Wednesday, November 24, 2021
> > 10:31 AM To: Anton Mikanovich <amikan@ilbers.de>; Moessbauer, Felix
> > (T RDA IOT SES-DE) <felix.moessbauer@siemens.com>
> > Cc: isar-users@googlegroups.com; Schild, Henning (T RDA IOT SES-DE)
> > <henning.schild@siemens.com>; Baurzhan Ismagulov <ibr@ilbers.de>
> > Subject: RE: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
> >
> > Anton Mikanovich, 22. November 2021 16:28:  
> > > To: Moessbauer, Felix (T RDA IOT SES-DE)
> > > <felix.moessbauer@siemens.com>
> > > Cc: isar-users@googlegroups.com; Schild, Henning (T RDA IOT
> > > SES-DE) <henning.schild@siemens.com>; Schmidt, Adriaan (T RDA IOT
> > > SES-DE) <adriaan.schmidt@siemens.com>; Baurzhan Ismagulov
> > > <ibr@ilbers.de> Subject: Re: [PATCH v4 0/2] Improve handling of
> > > ISAR_RELEASE_CMD
> > >
> > > 17.11.2021 18:57, Moessbauer, Felix wrote:  
> > > >> -----Original Message-----
> > > >> From: Anton Mikanovich <amikan@ilbers.de>
> > > >> Sent: Wednesday, November 17, 2021 2:06 PM
> > > >> To: Moessbauer, Felix (T RDA IOT SES-DE)
> > > >> <felix.moessbauer@siemens.com>
> > > >> Cc: isar-users@googlegroups.com
> > > >> Subject: Re: [PATCH v4 0/2] Improve handling of
> > > >> ISAR_RELEASE_CMD
> > > >>
> > > >> 17.11.2021 13:45, Moessbauer, Felix wrote:  
> > > >>> Hi Anton,
> > > >>>
> > > >>> Unfortunately I cannot reproduce this, but this is very likely
> > > >>> related to  
> > > a not  
> > > >> idempotent ISAR_RELEASE_CMD.  
> > > >>> As stated in the API changelog, the ISAR_RELEASE_CMD shall be
> > > >>> idempotent  
> > > >> (and technically must be for MC targets).  
> > > >>> By that, no things like timestamps must be included.
> > > >>>
> > > >>> If you point me to the location where the ISAR_RELEASE_CMD is
> > > >>> set for CI  
> > > >> builds, I can have a look.  
> > > >>> Another issue could be that changes to the git happen during
> > > >>> build  
> > (e.g.  
> > > adding  
> > > >> a tag, making the repo dirty, etc...).  
> > > >>> In that case (starting with a clean build, ending up with a
> > > >>> dirty one),  
> > > the CI  
> > > >> generated files have to be added to the .gitignore.  
> > > >>> In Yocto there is a lengthy discussion about the idempotence
> > > >>> sanity check  
> > > [1]  
> > > >> and why recipes have to be written in this way.  
> > > >>> Best regards,
> > > >>> Felix
> > > >>>
> > > >>> [1]
> > > >>>  
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F  
> > > >>> patc
> > > >>>  
> > > >>  
> > hwork.openembedded.org%2Fpatch%2F133517%2F&amp;data=04%7C01%7
> > Cfe  
> > > >> lix.mo
> > > >>  
> > essbauer%40siemens.com%7C8e3a031610c74e6dd76c08d9a9caf3f5%7C38ae
> > 3  
> > > >> bcd95
> > > >>  
> > 794fd4addab42e1495d55a%7C1%7C0%7C637727511405820881%7CUnknown
> > %  
> > > >> 7CTWFpbG
> > > >>  
> > Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> > M  
> > > >> n
> > > >> 0%
> > > >>  
> > 3D%7C3000&amp;sdata=od2HCs8VUwZqOzifBdItzcjb5a7j85g33J44%2BiMMjC
> > M  
> > > >> %3D&a  
> > > >>> mp;reserved=0
> > > >>>  
> > > >> Default ISAR_RELEASE_CMD can be found in  
> > meta/classes/image.bbclass as:  
> > > >> ISAR_RELEASE_CMD_DEFAULT = "git -C ${LAYERDIR_core} describe
> > > >> --  
> > tags  
> > > >> -- dirty --match 'v[0-9].[0-9]*'"
> > > >> which results in something like `v0.7-534-g6752a45` This issue
> > > >> is  
> > > reproduced  
> > > >> inside Jenkins only, but not locally or in gitlab/kas.  
> > > > Then it's likely that Jenkins modifies files in the source tree.
> > > > One thing you could try is to explicitly make the git "dirty",
> > > > or simply  
> > > try a static ISAR_RELEASE_CMD.  
> > > >
> > > > Anyways, how should we proceed here?
> > > >
> > > > Felix  
> > > >> --
> > > >> Anton Mikanovich
> > > >> Promwad Ltd.
> > > >> External service provider of ilbers GmbH Maria-Merian-Str. 8
> > > >> 85521 Ottobrunn, Germany
> > > >> +49 (89) 122 67 24-0
> > > >> Commercial register Munich, HRB 214197 General Manager:
> > > >> Baurzhan Ismagulov  
> > >
> > > We've reproduced the issue even locally without CI by manually
> > > setting username, clone and build:
> > >
> > > $ sudo chroot --userspec=<username> <chroot_path> /bin/bash -c "cd
> > > /tmp && git clone -b <branch>
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > >  
> > ub.com%2Filbers%2Fisar%2F&amp;data=04%7C01%7Cfelix.moessbauer%40s
> > iemens.com%7C764b1f78193440841ad508d9af2d1949%7C38ae3bcd95794fd4
> > addab42e1495d55a%7C1%7C0%7C637733430489004007%7CUnknown%7CTW
> > FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> > VCI6Mn0%3D%7C3000&amp;sdata=Z52wROzfuVc2F49yGYmmut7JmJ2I46N%
> > 2Bd911OolR%2Fi4%3D&amp;reserved=0 isar-repo && cd isar-repo &&
> > source isar-init-build-env && bitbake
> > mc:qemuamd64-bullseye:isar-image-base"  
> > >
> > > So the issue is related to chroot build but not only Jenkins.  
> >
> > I was able to reproduce, and I'm seeing that sometimes `git
> > describe` returns:
> >
> > warning: unable to access '/root/.config/git/attributes':
> > Permission denied v2.0-1-gf7f18a4-dirty (with the extra warning
> > line in its stdout, which then becomes part of IMAGE_BUILD_ID)
> >
> > So sometimes git wants to access config in $HOME, which is `/root`
> > when you run `sudo chroot`, and thus not readable to the user set
> > via --userspec. It works if you `export
> > HOME=/somewhere/readable/by/user` after entering the chroot.  
> 
> Thanks for debugging this.
> IMO this is neither an ISAR issue in general, nor related to the
> patches. The patches just make the misconfiguration visible.

Agreed.

> We could get around that by telling git to not consider any user or
> system config files (this requires a very recent git 2.32): export
> GIT_CONFIG_SYSTEM=/dev/null export GIT_CONFIG_GLOBAL=/dev/null

Not agreed. If at all the variables should become part of the CMD.
Because they would be workarounds to cater for that command ... if it
is even git based. Setting them globally could have very fun
side-effects on things like SRC_URI=git The real problem is that $HOME
is wrong.

I would change the default cmd to set the two env variables to /dev/null
in the patch that felix proposed.
If it was in the env that sudo chroot will need a "-E". Variables we
have in our bitbake shells tend to get lost with sudo. And while we
have some we take good care of ... git specific ones probably do not
count.

An alternative could be "/bin/su -" instead of userspec. (but userspec
is a pretty widely used pattern) 

Henning

> This could be done in the isar-init-build-env.
> However, I am not sure if we really want to do so.
> The ISAR release CMD is provided by the user and by that, also the
> user should be responsible for a correctly setup environment.
> 
> Opinions?
> 
> Felix
> >
> > I don't know why that access to $HOME/.config only happens on some
> > of the calls.
> >
> > Adriaan  
>