public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
@ 2021-11-04 11:05 Felix Moessbauer
  2021-11-04 11:05 ` [PATCH v4 1/2] always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on change Felix Moessbauer
                   ` (3 more replies)
  0 siblings, 4 replies; 30+ messages in thread
From: Felix Moessbauer @ 2021-11-04 11:05 UTC (permalink / raw)
  To: isar-users; +Cc: henning.schild, adriaan.schmidt, Felix Moessbauer

Changes since v3:

- add API changelog entry

Changes since v2:

- fix bug in update_etc_os_release
- Strip newlines from ISAR_RELEASE_CMD

Changes since v1:

- remove obsolete get_build_id function

Felix Moessbauer (2):
  always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on
    change
  Ensure generation of /etc/os-release is idempotent

 RECIPE-API-CHANGELOG.md                       | 13 ++++++++
 meta/classes/image-postproc-extension.bbclass |  6 ++--
 meta/classes/image.bbclass                    | 32 ++++++++++---------
 meta/classes/rootfs.bbclass                   |  2 +-
 4 files changed, 34 insertions(+), 19 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v4 1/2] always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on change
  2021-11-04 11:05 [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD Felix Moessbauer
@ 2021-11-04 11:05 ` Felix Moessbauer
  2021-11-04 11:05 ` [PATCH v4 2/2] Ensure generation of /etc/os-release is idempotent Felix Moessbauer
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 30+ messages in thread
From: Felix Moessbauer @ 2021-11-04 11:05 UTC (permalink / raw)
  To: isar-users; +Cc: henning.schild, adriaan.schmidt, Felix Moessbauer

The command in ISAR_RELEASE_CMD is used to add version information
of the image under /etc/os-release. As we cannot predict the
output of the command, we cannot cache it.
Prior to this patch, this situation was just accepted resulting
in misleading build-ids on incremental builds.

When caching artifacts across builds (e.g. using sstate),
this also affects fresh builds.

The patch ensures that ISAR_RELEASE_CMD is always invoked
and the output is stored in the variable IMAGE_BUILD_ID.
This variable is added as a vardep in the rootfs_postprocess
task, so changes invalidate all downstream tasks.

By that, we ensure that images always contain correct release
information.

Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 RECIPE-API-CHANGELOG.md                       | 13 ++++++++
 meta/classes/image-postproc-extension.bbclass |  4 +--
 meta/classes/image.bbclass                    | 32 ++++++++++---------
 meta/classes/rootfs.bbclass                   |  2 +-
 4 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/RECIPE-API-CHANGELOG.md b/RECIPE-API-CHANGELOG.md
index 7312d4d..48458a5 100644
--- a/RECIPE-API-CHANGELOG.md
+++ b/RECIPE-API-CHANGELOG.md
@@ -301,3 +301,16 @@ Kernel update with "apt-get" will not work since bootloader configuration will
 not be updated. It used to "kind of work" for grub and efi, that hack is gone.
 
 When using the plugins it is advised to name the partition "/boot" and to exclude boot from the follwing rootfs to not waste space.
+
+### ISAR_RELEASE_CMD is executed on every build
+
+The `ISAR_RELEASE_CMD` used to be only executed when changes to the target rootfs where made.
+This lead to outdated data in `/etc/os-release` on incremental builds.
+
+The command is now executed on every build.
+Downstream tasks are invalidated in case the output of the command changes.
+
+When using a custom command it is advised to stick to the following rules:
+
+- no visible side effects (the command is executed multiple times during a build)
+- idempotence (always emit the same output for a single build, i.e. no timestamp)
diff --git a/meta/classes/image-postproc-extension.bbclass b/meta/classes/image-postproc-extension.bbclass
index 3f00c21..6e6b257 100644
--- a/meta/classes/image-postproc-extension.bbclass
+++ b/meta/classes/image-postproc-extension.bbclass
@@ -46,11 +46,11 @@ image_postprocess_configure() {
 }
 
 ROOTFS_POSTPROCESS_COMMAND =+ "image_postprocess_mark"
+ROOTFS_POSTPROCESS_VARDEPS =+ "IMAGE_BUILD_ID"
 
 image_postprocess_mark() {
-    BUILD_ID=$(get_build_id)
     update_etc_os_release \
-        --build-id "${BUILD_ID}" --variant "${DESCRIPTION}" --version "${PV}"
+        --build-id "${IMAGE_BUILD_ID}" --variant "${DESCRIPTION}" --version "${PV}"
 }
 
 ROOTFS_POSTPROCESS_COMMAND =+ "image_postprocess_machine_id"
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 5a0f32e..36883a8 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -92,21 +92,23 @@ 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
+# the IMAGE_BUILD_ID depends on external conditions injected via
+# ISAR_RELEASE_CMD. By that, we have to compute the value
+# on each invocation
+python() {
+    bblayers = d.getVar('BBLAYERS', True)
+    release_cmd = d.getVar('ISAR_RELEASE_CMD', True)
+    if len(bblayers.split()) != 2:
+        if release_cmd == d.getVar('ISAR_RELEASE_CMD_DEFAULT', True):
+            bb.warn('You are using external layers that will not be '
+                    'considered in the build_id. Consider changing '
+                    'ISAR_RELEASE_CMD.')
+    try:
+        out,err = bb.process.run(release_cmd)
+        d.setVar('IMAGE_BUILD_ID', out.replace('\n', ''))
+    except(bb.process.ExecutionError):
+        bb.warn(f'"{release_cmd}" failed, returning empty build_id.')
+        d.setVar('IMAGE_BUILD_ID', '')
 }
 
 python set_image_size () {
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index 20ccb00..e8c7fa0 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -239,7 +239,7 @@ rootfs_export_dpkg_status() {
        '${ROOTFS_DPKGSTATUS_DEPLOY_DIR}'/'${PF}'.dpkg_status
 }
 
-do_rootfs_postprocess[vardeps] = "${ROOTFS_POSTPROCESS_COMMAND}"
+do_rootfs_postprocess[vardeps] = "${ROOTFS_POSTPROCESS_COMMAND} ${ROOTFS_POSTPROCESS_VARDEPS}"
 python do_rootfs_postprocess() {
     # Take care that its correctly mounted:
     bb.build.exec_func('rootfs_do_mounts', d)
-- 
2.30.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v4 2/2] Ensure generation of /etc/os-release is idempotent
  2021-11-04 11:05 [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD Felix Moessbauer
  2021-11-04 11:05 ` [PATCH v4 1/2] always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on change Felix Moessbauer
@ 2021-11-04 11:05 ` Felix Moessbauer
  2021-11-04 13:12 ` [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD Henning Schild
  2021-11-16 18:09 ` Anton Mikanovich
  3 siblings, 0 replies; 30+ messages in thread
From: Felix Moessbauer @ 2021-11-04 11:05 UTC (permalink / raw)
  To: isar-users; +Cc: henning.schild, adriaan.schmidt, Felix Moessbauer

Prior to this patch, the generation of the os-release file
was not idempotent as there was a name-glitch in VARIAN_VERSION.
By that, the script did not detect an existing VARIANT_VERSION
and add one additional one per invocation.

Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 meta/classes/image-postproc-extension.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/image-postproc-extension.bbclass b/meta/classes/image-postproc-extension.bbclass
index 6e6b257..87749c8 100644
--- a/meta/classes/image-postproc-extension.bbclass
+++ b/meta/classes/image-postproc-extension.bbclass
@@ -27,7 +27,7 @@ update_etc_os_release() {
             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'
+        sudo sed -i '/^VARIANT_VERSION=.*/d' '${IMAGE_ROOTFS}/etc/os-release'
         echo "VARIANT_VERSION=\"${PV}\"" | \
             sudo tee -a '${IMAGE_ROOTFS}/etc/os-release'
     fi
-- 
2.30.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-04 11:05 [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD Felix Moessbauer
  2021-11-04 11:05 ` [PATCH v4 1/2] always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on change Felix Moessbauer
  2021-11-04 11:05 ` [PATCH v4 2/2] Ensure generation of /etc/os-release is idempotent Felix Moessbauer
@ 2021-11-04 13:12 ` Henning Schild
  2021-11-16 18:09 ` Anton Mikanovich
  3 siblings, 0 replies; 30+ messages in thread
From: Henning Schild @ 2021-11-04 13:12 UTC (permalink / raw)
  To: Felix Moessbauer; +Cc: isar-users, adriaan.schmidt

ACK, and yes that should be done before sstate

Henning

Am Thu, 4 Nov 2021 12:05:05 +0100
schrieb Felix Moessbauer <felix.moessbauer@siemens.com>:

> Changes since v3:
> 
> - add API changelog entry
> 
> Changes since v2:
> 
> - fix bug in update_etc_os_release
> - Strip newlines from ISAR_RELEASE_CMD
> 
> Changes since v1:
> 
> - remove obsolete get_build_id function
> 
> Felix Moessbauer (2):
>   always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on
>     change
>   Ensure generation of /etc/os-release is idempotent
> 
>  RECIPE-API-CHANGELOG.md                       | 13 ++++++++
>  meta/classes/image-postproc-extension.bbclass |  6 ++--
>  meta/classes/image.bbclass                    | 32
> ++++++++++--------- meta/classes/rootfs.bbclass                   |
> 2 +- 4 files changed, 34 insertions(+), 19 deletions(-)
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-04 11:05 [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD Felix Moessbauer
                   ` (2 preceding siblings ...)
  2021-11-04 13:12 ` [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD Henning Schild
@ 2021-11-16 18:09 ` Anton Mikanovich
  2021-11-17 10:45   ` Moessbauer, Felix
  3 siblings, 1 reply; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-16 18:09 UTC (permalink / raw)
  To: Felix Moessbauer, isar-users; +Cc: henning.schild, adriaan.schmidt

04.11.2021 14:05, Felix Moessbauer wrote:
> Changes since v3:
>
> - add API changelog entry
>
> Changes since v2:
>
> - fix bug in update_etc_os_release
> - Strip newlines from ISAR_RELEASE_CMD
>
> Changes since v1:
>
> - remove obsolete get_build_id function
>
> Felix Moessbauer (2):
>    always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on
>      change
>    Ensure generation of /etc/os-release is idempotent
>
>   RECIPE-API-CHANGELOG.md                       | 13 ++++++++
>   meta/classes/image-postproc-extension.bbclass |  6 ++--
>   meta/classes/image.bbclass                    | 32 ++++++++++---------
>   meta/classes/rootfs.bbclass                   |  2 +-
>   4 files changed, 34 insertions(+), 19 deletions(-)
>
This patchset fails on CI with the following error:

NOTE: Running task 171 of 173 
(mc:qemuamd64-bullseye:/meta-isar/recipes-core/images/isar-image-base.bb:do_image)
ERROR: When reparsing 
mc:qemuamd64-bullseye:/meta-isar/recipes-core/images/isar-image-base.bb:do_rootfs_postprocess,
the basehash value changed from 
16a8399888ee56cde01e3fd03937f3847e0f945712e4183858f58527bebc58a7 to
b01a488185cef694cbc7e29bf07d0cf9547355f6f00c79f664491eb058b91632. The 
metadata is not deterministic and this needs to be fixed.

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-16 18:09 ` Anton Mikanovich
@ 2021-11-17 10:45   ` Moessbauer, Felix
  2021-11-17 13:05     ` Anton Mikanovich
  0 siblings, 1 reply; 30+ messages in thread
From: Moessbauer, Felix @ 2021-11-17 10:45 UTC (permalink / raw)
  To: Anton Mikanovich, isar-users; +Cc: henning.schild, Schmidt, Adriaan

Hi Anton,

> -----Original Message-----
> From: Anton Mikanovich <amikan@ilbers.de>
> Sent: Tuesday, November 16, 2021 7:10 PM
> To: Moessbauer, Felix (T RDA IOT SES-DE) <felix.moessbauer@siemens.com>;
> isar-users@googlegroups.com
> Cc: Schild, Henning (T RDA IOT SES-DE) <henning.schild@siemens.com>;
> Schmidt, Adriaan (T RDA IOT SES-DE) <adriaan.schmidt@siemens.com>
> Subject: Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
> 
> 04.11.2021 14:05, Felix Moessbauer wrote:
> > Changes since v3:
> >
> > - add API changelog entry
> >
> > Changes since v2:
> >
> > - fix bug in update_etc_os_release
> > - Strip newlines from ISAR_RELEASE_CMD
> >
> > Changes since v1:
> >
> > - remove obsolete get_build_id function
> >
> > Felix Moessbauer (2):
> >    always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on
> >      change
> >    Ensure generation of /etc/os-release is idempotent
> >
> >   RECIPE-API-CHANGELOG.md                       | 13 ++++++++
> >   meta/classes/image-postproc-extension.bbclass |  6 ++--
> >   meta/classes/image.bbclass                    | 32 ++++++++++---------
> >   meta/classes/rootfs.bbclass                   |  2 +-
> >   4 files changed, 34 insertions(+), 19 deletions(-)
> >
> This patchset fails on CI with the following error:
> 
> NOTE: Running task 171 of 173
> (mc:qemuamd64-bullseye:/meta-isar/recipes-core/images/isar-image-
> base.bb:do_image)
> ERROR: When reparsing
> mc:qemuamd64-bullseye:/meta-isar/recipes-core/images/isar-image-
> base.bb:do_rootfs_postprocess,
> the basehash value changed from
> 16a8399888ee56cde01e3fd03937f3847e0f945712e4183858f58527bebc58a7 to
> b01a488185cef694cbc7e29bf07d0cf9547355f6f00c79f664491eb058b91632.
> The metadata is not deterministic and this needs to be fixed.

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://patchwork.openembedded.org/patch/133517/
> 
> --
> 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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-17 10:45   ` Moessbauer, Felix
@ 2021-11-17 13:05     ` Anton Mikanovich
  2021-11-17 15:57       ` Moessbauer, Felix
  2021-11-17 16:54       ` Henning Schild
  0 siblings, 2 replies; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-17 13:05 UTC (permalink / raw)
  To: Moessbauer, Felix; +Cc: isar-users

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://patchwork.openembedded.org/patch/133517/
>
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.

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-17 13:05     ` Anton Mikanovich
@ 2021-11-17 15:57       ` Moessbauer, Felix
  2021-11-17 16:39         ` Baurzhan Ismagulov
  2021-11-22 15:28         ` Anton Mikanovich
  2021-11-17 16:54       ` Henning Schild
  1 sibling, 2 replies; 30+ messages in thread
From: Moessbauer, Felix @ 2021-11-17 15:57 UTC (permalink / raw)
  To: Anton Mikanovich; +Cc: isar-users, henning.schild, Schmidt, Adriaan

> -----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%2Fpatc
> >
> hwork.openembedded.org%2Fpatch%2F133517%2F&amp;data=04%7C01%7Cfe
> lix.mo
> >
> essbauer%40siemens.com%7C8e3a031610c74e6dd76c08d9a9caf3f5%7C38ae3
> bcd95
> >
> 794fd4addab42e1495d55a%7C1%7C0%7C637727511405820881%7CUnknown%
> 7CTWFpbG
> >
> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%
> >
> 3D%7C3000&amp;sdata=od2HCs8VUwZqOzifBdItzcjb5a7j85g33J44%2BiMMjCM
> %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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-17 15:57       ` Moessbauer, Felix
@ 2021-11-17 16:39         ` Baurzhan Ismagulov
  2021-11-22 15:28         ` Anton Mikanovich
  1 sibling, 0 replies; 30+ messages in thread
From: Baurzhan Ismagulov @ 2021-11-17 16:39 UTC (permalink / raw)
  To: isar-users
  Cc: Moessbauer, Felix, Anton Mikanovich, henning.schild, Schmidt, Adriaan

On Wed, Nov 17, 2021 at 03:57:53PM +0000, Moessbauer, Felix wrote:
> > 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?

This does point to some environment issue around our CI. That said, as yet I
doubt it's specifically Jenkins. I also think that in any case, we should
understand what is going on. As the issue is only reproducible on our side, I
suggest that we look at it tomorrow and let you know.

With kind regards,
Baurzhan.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-17 13:05     ` Anton Mikanovich
  2021-11-17 15:57       ` Moessbauer, Felix
@ 2021-11-17 16:54       ` Henning Schild
  2021-11-18  8:36         ` Moessbauer, Felix
  1 sibling, 1 reply; 30+ messages in thread
From: Henning Schild @ 2021-11-17 16:54 UTC (permalink / raw)
  To: Anton Mikanovich; +Cc: Moessbauer, Felix, isar-users

Am Wed, 17 Nov 2021 16:05:31 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 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://patchwork.openembedded.org/patch/133517/
> >  
> 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.

Well once you sed in there you might also get a "dirty" somewhere. The
reason the output might differ in gitlab is simply the clone depth
which defaults to something like 50. If your last tag is 534 commits
away you will get only g<sha> because there are no tags in your not
super-deep clone.

Workarounds are to tag more often or to clone deeper. I am not sure git
describe can be told how many commits to consider in the past. So its
output might simply differ depending on how deep you cloned.

Henning

^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-17 16:54       ` Henning Schild
@ 2021-11-18  8:36         ` Moessbauer, Felix
  0 siblings, 0 replies; 30+ messages in thread
From: Moessbauer, Felix @ 2021-11-18  8:36 UTC (permalink / raw)
  To: henning.schild; +Cc: isar-users, Baurzhan Ismagulov, Anton Mikanovich

> -----Original Message-----
> From: Henning Schild <henning.schild@siemens.com>
> Sent: Wednesday, November 17, 2021 5:55 PM
> To: Anton Mikanovich <amikan@ilbers.de>
> Cc: Moessbauer, Felix (T RDA IOT SES-DE) <felix.moessbauer@siemens.com>;
> isar-users@googlegroups.com
> Subject: Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
> 
> Am Wed, 17 Nov 2021 16:05:31 +0300
> schrieb Anton Mikanovich <amikan@ilbers.de>:
> 
> > 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%2Fpa
> > >
> tchwork.openembedded.org%2Fpatch%2F133517%2F&amp;data=04%7C01%7C
> feli
> > >
> x.moessbauer%40siemens.com%7C41165dba8b90412ea40208d9a9eaf011%7C3
> 8ae
> > >
> 3bcd95794fd4addab42e1495d55a%7C1%7C0%7C637727648792760989%7CUnk
> nown%
> > >
> 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> C
> > >
> JXVCI6Mn0%3D%7C3000&amp;sdata=tiU23Wb9SkVh01PyueQHb9i7K4p3YuhmN
> k4Lj3
> > > Bvg8U%3D&amp;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.
> 
> Well once you sed in there you might also get a "dirty" somewhere. The reason
> the output might differ in gitlab is simply the clone depth which defaults to
> something like 50. If your last tag is 534 commits away you will get only g<sha>
> because there are no tags in your not super-deep clone.

I don't think that is the case here. As long as the ISAR_RELEASE_CMD reports the exactly same value for a single bitbake invocation, we are fine.
The value itself doesn't matter and can also change across invocations of bitbake.
That's exactly the point of the patch to correctly detect changes to the environment and reflect them in /etc/os-release.

Just to make sure we are all on the same page:
With this patch, ISAR_RELEASE_CMD is executed in a anonymous function of the image recipe.
By that it runs at bitbake parsing time and sets the variable IMAGE_BUILD_ID to the output of the command.
As this variable is part of the metadata, it must not change during a single bitbake invocation.
Otherwise bitbake reports "metadata is non-deterministic".

However, it is perfectly fine to change the value across builds (e.g. incremental ones and even without a single modification to a recipe).
That will become relevant once SState is enabled:
In case the rootfs is cached, the ISAR_RELEASE_CMD was not executed again, leading to invalid information in /etc/os-release (e.g. by missing a tag).
This is fixed by the patch.

> 
> Workarounds are to tag more often or to clone deeper. I am not sure git
> describe can be told how many commits to consider in the past. So its output
> might simply differ depending on how deep you cloned.

What you describe here is just about unexpected results in /etc/os-release, because the expected git tag is not in the history.
That's a different story and cannot be addressed by ISAR, but only by the user.

Felix

> 
> Henning

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-17 15:57       ` Moessbauer, Felix
  2021-11-17 16:39         ` Baurzhan Ismagulov
@ 2021-11-22 15:28         ` Anton Mikanovich
  2021-11-24  9:30           ` Schmidt, Adriaan
  1 sibling, 1 reply; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-22 15:28 UTC (permalink / raw)
  To: Moessbauer, Felix
  Cc: isar-users, henning.schild, Schmidt, Adriaan, Baurzhan Ismagulov

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%2Fpatc
>>>
>> hwork.openembedded.org%2Fpatch%2F133517%2F&amp;data=04%7C01%7Cfe
>> lix.mo
>> essbauer%40siemens.com%7C8e3a031610c74e6dd76c08d9a9caf3f5%7C38ae3
>> bcd95
>> 794fd4addab42e1495d55a%7C1%7C0%7C637727511405820881%7CUnknown%
>> 7CTWFpbG
>> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
>> 0%
>> 3D%7C3000&amp;sdata=od2HCs8VUwZqOzifBdItzcjb5a7j85g33J44%2BiMMjCM
>> %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://github.com/ilbers/isar/ 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.

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-22 15:28         ` Anton Mikanovich
@ 2021-11-24  9:30           ` Schmidt, Adriaan
  2021-11-24 10:15             ` Moessbauer, Felix
                               ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Schmidt, Adriaan @ 2021-11-24  9:30 UTC (permalink / raw)
  To: Anton Mikanovich, Moessbauer, Felix
  Cc: isar-users, henning.schild, Baurzhan Ismagulov

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%2Fpatc
> >>>
> >> hwork.openembedded.org%2Fpatch%2F133517%2F&amp;data=04%7C01%7Cfe
> >> lix.mo
> >> essbauer%40siemens.com%7C8e3a031610c74e6dd76c08d9a9caf3f5%7C38ae3
> >> bcd95
> >> 794fd4addab42e1495d55a%7C1%7C0%7C637727511405820881%7CUnknown%
> >> 7CTWFpbG
> >> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> >> 0%
> >> 3D%7C3000&amp;sdata=od2HCs8VUwZqOzifBdItzcjb5a7j85g33J44%2BiMMjCM
> >> %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://github.com/ilbers/isar/ 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.

I don't know why that access to $HOME/.config only happens on some of the calls.

Adriaan


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-24  9:30           ` Schmidt, Adriaan
@ 2021-11-24 10:15             ` Moessbauer, Felix
  2021-11-24 11:25               ` Jan Kiszka
  2021-11-24 12:11               ` Henning Schild
  2021-11-24 11:53             ` Anton Mikanovich
  2021-11-24 12:35             ` Henning Schild
  2 siblings, 2 replies; 30+ messages in thread
From: Moessbauer, Felix @ 2021-11-24 10:15 UTC (permalink / raw)
  To: Schmidt, Adriaan, Anton Mikanovich
  Cc: isar-users, henning.schild, Baurzhan Ismagulov

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

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

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-24 10:15             ` Moessbauer, Felix
@ 2021-11-24 11:25               ` Jan Kiszka
  2021-11-24 11:54                 ` Anton Mikanovich
  2021-11-24 12:11               ` Henning Schild
  1 sibling, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2021-11-24 11:25 UTC (permalink / raw)
  To: Moessbauer, Felix, Schmidt, Adriaan, Anton Mikanovich
  Cc: isar-users, henning.schild, Baurzhan Ismagulov

On 24.11.21 11:15, Moessbauer, Felix wrote:
>> -----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://
>>>>>> 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://gith
>>>
>> 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.
> 
> 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
> 
> 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?

I think it would make sense to silence git error/warning messages in the
default ISAR_RELEASE_CMD definition. But anything else would go beyond
reasonable, specifically due to unpredictable side effects.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-24  9:30           ` Schmidt, Adriaan
  2021-11-24 10:15             ` Moessbauer, Felix
@ 2021-11-24 11:53             ` Anton Mikanovich
  2021-11-24 12:35             ` Henning Schild
  2 siblings, 0 replies; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-24 11:53 UTC (permalink / raw)
  To: Schmidt, Adriaan, Moessbauer, Felix
  Cc: isar-users, henning.schild, Baurzhan Ismagulov

Thanks for that digging, I also got the same results: the command was 
executed with the same euid, cwd and HOME values, but tried to access 
~/.config/git/attributes only once for some reason.

24.11.2021 12:30, Schmidt, Adriaan wrote:
> 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%2Fpatc
>>>>>
>>>> hwork.openembedded.org%2Fpatch%2F133517%2F&amp;data=04%7C01%7Cfe
>>>> lix.mo
>>>> essbauer%40siemens.com%7C8e3a031610c74e6dd76c08d9a9caf3f5%7C38ae3
>>>> bcd95
>>>> 794fd4addab42e1495d55a%7C1%7C0%7C637727511405820881%7CUnknown%
>>>> 7CTWFpbG
>>>> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
>>>> 0%
>>>> 3D%7C3000&amp;sdata=od2HCs8VUwZqOzifBdItzcjb5a7j85g33J44%2BiMMjCM
>>>> %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://github.com/ilbers/isar/ 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.
>
> I don't know why that access to $HOME/.config only happens on some of the calls.
>
> Adriaan
>
-- 
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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-24 11:25               ` Jan Kiszka
@ 2021-11-24 11:54                 ` Anton Mikanovich
  0 siblings, 0 replies; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-24 11:54 UTC (permalink / raw)
  To: Jan Kiszka, Moessbauer, Felix, Schmidt, Adriaan
  Cc: isar-users, henning.schild, Baurzhan Ismagulov

To silence that warning we need to change the upstream (git), because 
this warning was printed to stdout, not stderr.
And of course we can't filter it later, because we can't predict what 
user can set as ISAR_RELEASE_CMD.
But maybe we can just use some other default command.

24.11.2021 14:25, Jan Kiszka wrote:
> On 24.11.21 11:15, Moessbauer, Felix wrote:
>>> -----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://
>>>>>>> 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://gith
>>>>
>>> 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.
>>
>> 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
>>
>> 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?
> I think it would make sense to silence git error/warning messages in the
> default ISAR_RELEASE_CMD definition. But anything else would go beyond
> reasonable, specifically due to unpredictable side effects.
>
> Jan
>
-- 
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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-24 10:15             ` Moessbauer, Felix
  2021-11-24 11:25               ` Jan Kiszka
@ 2021-11-24 12:11               ` Henning Schild
  2021-11-29  9:09                 ` Anton Mikanovich
  1 sibling, 1 reply; 30+ messages in thread
From: Henning Schild @ 2021-11-24 12:11 UTC (permalink / raw)
  To: Moessbauer, Felix (T RDA IOT SES-DE)
  Cc: Schmidt, Adriaan (T RDA IOT SES-DE),
	Anton Mikanovich, isar-users, Baurzhan Ismagulov

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  
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-24  9:30           ` Schmidt, Adriaan
  2021-11-24 10:15             ` Moessbauer, Felix
  2021-11-24 11:53             ` Anton Mikanovich
@ 2021-11-24 12:35             ` Henning Schild
  2021-11-24 12:52               ` Anton Mikanovich
  2 siblings, 1 reply; 30+ messages in thread
From: Henning Schild @ 2021-11-24 12:35 UTC (permalink / raw)
  To: Schmidt, Adriaan (T RDA IOT SES-DE)
  Cc: Anton Mikanovich, Moessbauer, Felix (T RDA IOT SES-DE),
	isar-users, Baurzhan Ismagulov

Am Wed, 24 Nov 2021 10:30:48 +0100
schrieb "Schmidt, Adriaan (T RDA IOT SES-DE)"
<adriaan.schmidt@siemens.com>:

> 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%2Fpatc
> > >>>  
> > >> hwork.openembedded.org%2Fpatch%2F133517%2F&amp;data=04%7C01%7Cfe
> > >> lix.mo
> > >> essbauer%40siemens.com%7C8e3a031610c74e6dd76c08d9a9caf3f5%7C38ae3
> > >> bcd95
> > >> 794fd4addab42e1495d55a%7C1%7C0%7C637727511405820881%7CUnknown%
> > >> 7CTWFpbG
> > >> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> > >> 0%
> > >> 3D%7C3000&amp;sdata=od2HCs8VUwZqOzifBdItzcjb5a7j85g33J44%2BiMMjCM
> > >> %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://github.com/ilbers/isar/
> > 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 wondering where the sudo chroot was put around the RELEASE_CMD
and could not find that. Because that place is just wrong if it does
not also set HOME or use sudo -E or use bash -l. If one wants a proper
shell inside a chroot one needs to "log in" and not inherit env ...
especially not with sudo (which messes with env)

But it seems that no such code is part of isar. But maybe part of the
ilbers ci?

Henning

> 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.
> 
> I don't know why that access to $HOME/.config only happens on some of
> the calls.
> 
> Adriaan
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-24 12:35             ` Henning Schild
@ 2021-11-24 12:52               ` Anton Mikanovich
  0 siblings, 0 replies; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-24 12:52 UTC (permalink / raw)
  To: Henning Schild, Schmidt, Adriaan (T RDA IOT SES-DE)
  Cc: Moessbauer, Felix (T RDA IOT SES-DE), isar-users, Baurzhan Ismagulov

24.11.2021 15:35, Henning Schild wrote:
> I was wondering where the sudo chroot was put around the RELEASE_CMD
> and could not find that. Because that place is just wrong if it does
> not also set HOME or use sudo -E or use bash -l. If one wants a proper
> shell inside a chroot one needs to "log in" and not inherit env ...
> especially not with sudo (which messes with env)
>
> But it seems that no such code is part of isar. But maybe part of the
> ilbers ci?
>
> Henning

Yes, this is a part of Jenkins CI.

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-24 12:11               ` Henning Schild
@ 2021-11-29  9:09                 ` Anton Mikanovich
  2021-11-29  9:50                   ` Henning Schild
  2021-11-29 13:04                   ` Anton Mikanovich
  0 siblings, 2 replies; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-29  9:09 UTC (permalink / raw)
  To: Henning Schild, Moessbauer, Felix (T RDA IOT SES-DE)
  Cc: Schmidt, Adriaan (T RDA IOT SES-DE), isar-users, Baurzhan Ismagulov

24.11.2021 15:11, Henning Schild wrote:
>
>>> An alternative could be "/bin/su -" instead of userspec. (but userspec
>>> is a pretty widely used pattern)
>>>
>>> Henning
>>>
I've tried to use `su -`: Jenkins got the correct env and at least 
started building, but now fails on RebuildTest with hash diff issue.
This test case just changes dpkg-base.bbclass and rebuilds one target.

Can't reproduce the issue locally (out of CI) for now.
Will try to obtain the real hashes diff from CI.

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-29  9:09                 ` Anton Mikanovich
@ 2021-11-29  9:50                   ` Henning Schild
  2021-11-29  9:55                     ` Anton Mikanovich
  2021-11-29 13:04                   ` Anton Mikanovich
  1 sibling, 1 reply; 30+ messages in thread
From: Henning Schild @ 2021-11-29  9:50 UTC (permalink / raw)
  To: Anton Mikanovich
  Cc: Moessbauer, Felix (T RDA IOT SES-DE),
	Schmidt, Adriaan (T RDA IOT SES-DE),
	isar-users, Baurzhan Ismagulov

Am Mon, 29 Nov 2021 12:09:13 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 24.11.2021 15:11, Henning Schild wrote:
> >  
> >>> An alternative could be "/bin/su -" instead of userspec. (but
> >>> userspec is a pretty widely used pattern)
> >>>
> >>> Henning
> >>>  
> I've tried to use `su -`: Jenkins got the correct env and at least 
> started building, but now fails on RebuildTest with hash diff issue.
> This test case just changes dpkg-base.bbclass and rebuilds one target.

This might have to do with avocado running maybe multiple tests in the
same tree in parallel.
Any test that applies patches must revert them and must not run in
parallel when in the same tree.

Just an educated guess based on weaknesses of our use of avocado that
have been pointed out in the past.

Henning

> Can't reproduce the issue locally (out of CI) for now.
> Will try to obtain the real hashes diff from CI.
> 


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-29  9:50                   ` Henning Schild
@ 2021-11-29  9:55                     ` Anton Mikanovich
  0 siblings, 0 replies; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-29  9:55 UTC (permalink / raw)
  To: Henning Schild
  Cc: Moessbauer, Felix (T RDA IOT SES-DE),
	Schmidt, Adriaan (T RDA IOT SES-DE),
	isar-users, Baurzhan Ismagulov

29.11.2021 12:50, Henning Schild wrote:
> Am Mon, 29 Nov 2021 12:09:13 +0300
> schrieb Anton Mikanovich <amikan@ilbers.de>:
>
>> 24.11.2021 15:11, Henning Schild wrote:
>>>   
>>>>> An alternative could be "/bin/su -" instead of userspec. (but
>>>>> userspec is a pretty widely used pattern)
>>>>>
>>>>> Henning
>>>>>   
>> I've tried to use `su -`: Jenkins got the correct env and at least
>> started building, but now fails on RebuildTest with hash diff issue.
>> This test case just changes dpkg-base.bbclass and rebuilds one target.
> This might have to do with avocado running maybe multiple tests in the
> same tree in parallel.
> Any test that applies patches must revert them and must not run in
> parallel when in the same tree.
>
> Just an educated guess based on weaknesses of our use of avocado that
> have been pointed out in the past.
>
> Henning
>
>> Can't reproduce the issue locally (out of CI) for now.
>> Will try to obtain the real hashes diff from CI.
>>
But the parallel build is disabled in our avocado CI scenario.
Everything is working in the same way as shell-based CI did.

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-29  9:09                 ` Anton Mikanovich
  2021-11-29  9:50                   ` Henning Schild
@ 2021-11-29 13:04                   ` Anton Mikanovich
  2021-11-29 13:18                     ` Henning Schild
  1 sibling, 1 reply; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-29 13:04 UTC (permalink / raw)
  To: Moessbauer, Felix (T RDA IOT SES-DE)
  Cc: Henning Schild, Schmidt, Adriaan (T RDA IOT SES-DE),
	isar-users, Baurzhan Ismagulov

29.11.2021 12:09, Anton Mikanovich wrote:
> 24.11.2021 15:11, Henning Schild wrote:
>>
>>>> An alternative could be "/bin/su -" instead of userspec. (but userspec
>>>> is a pretty widely used pattern)
>>>>
>>>> Henning
>>>>
> I've tried to use `su -`: Jenkins got the correct env and at least 
> started building, but now fails on RebuildTest with hash diff issue.
> This test case just changes dpkg-base.bbclass and rebuilds one target.
>
> Can't reproduce the issue locally (out of CI) for now.
> Will try to obtain the real hashes diff from CI.
>
It was caused by the change IMAGE_BUILD_ID from 'v0.7-537-g6085d11' to 
'v0.7-537-g6085d11-dirty' which sounds reasonable.
Was this case (change of source, then rebuild) actually tested?

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-29 13:04                   ` Anton Mikanovich
@ 2021-11-29 13:18                     ` Henning Schild
  2021-11-29 14:20                       ` Anton Mikanovich
  0 siblings, 1 reply; 30+ messages in thread
From: Henning Schild @ 2021-11-29 13:18 UTC (permalink / raw)
  To: Anton Mikanovich
  Cc: Moessbauer, Felix (T RDA IOT SES-DE),
	Schmidt, Adriaan (T RDA IOT SES-DE),
	isar-users, Baurzhan Ismagulov

Am Mon, 29 Nov 2021 16:04:42 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:

> 29.11.2021 12:09, Anton Mikanovich wrote:
> > 24.11.2021 15:11, Henning Schild wrote:  
> >>  
> >>>> An alternative could be "/bin/su -" instead of userspec. (but
> >>>> userspec is a pretty widely used pattern)
> >>>>
> >>>> Henning
> >>>>  
> > I've tried to use `su -`: Jenkins got the correct env and at least 
> > started building, but now fails on RebuildTest with hash diff issue.
> > This test case just changes dpkg-base.bbclass and rebuilds one
> > target.
> >
> > Can't reproduce the issue locally (out of CI) for now.
> > Will try to obtain the real hashes diff from CI.
> >  
> It was caused by the change IMAGE_BUILD_ID from 'v0.7-537-g6085d11'
> to 'v0.7-537-g6085d11-dirty' which sounds reasonable.
> Was this case (change of source, then rebuild) actually tested?
> 

Well that is to expect! I guess that patch did pass our gitlab based
CI, but i do not know.
Please give more details on the failure you see. "hash diff issue" ...
Otherwise we will be in the "works for me, vs have you tried turning it
off and on again" business.


Henning

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-29 13:18                     ` Henning Schild
@ 2021-11-29 14:20                       ` Anton Mikanovich
  2021-11-30 10:03                         ` Moessbauer, Felix
  0 siblings, 1 reply; 30+ messages in thread
From: Anton Mikanovich @ 2021-11-29 14:20 UTC (permalink / raw)
  To: Henning Schild
  Cc: Moessbauer, Felix (T RDA IOT SES-DE),
	Schmidt, Adriaan (T RDA IOT SES-DE),
	isar-users, Baurzhan Ismagulov

29.11.2021 16:18, Henning Schild wrote:
> Well that is to expect! I guess that patch did pass our gitlab based
> CI, but i do not know.
> Please give more details on the failure you see. "hash diff issue" ...
> Otherwise we will be in the "works for me, vs have you tried turning it
> off and on again" business.
>
>
> Henning

As I've already mentioned before I've just run 'fast' CI (ci_build.sh -q 
-f -r) on Jenkins which failed on RebuildTest:

19:27:22  (07/10) 
/build/isar_am_devel_fast/256/testsuite/build_test/build_test.py:RebuildTest.test_rebuild: 
ERROR: When reparsing 
mc:qemuamd64-stretch:/workspace/build/isar_am_devel_fast/256/meta-isar/recipes-core/images/isar-image-base.bb:do_rootfs_postprocess, 
the basehash value changed from 
6553fa78201729c3311387f6074b35e0bd2ec2d7e745044374c4736dab3c4829 to 
562abcf3978252df402e4956d05bbbaed9e962ee379be1f9d85c43696e18f009. The 
metadata is not deterministic and this needs to be fixed.

Previous 6 test cases were passed.
Analysis with bitbake-diffsigs gives the real cause:

basehash changed from 
2d5398895dd3d1f478f58a07e1f411c4f4d374a4d9d2bbae7e3dfc1517f03278 to 
d459621430969975b7eed659471d4833ca17315e117c16ac1cc10e020886007a
Variable IMAGE_BUILD_ID value changed from 'v0.7-537-g6085d11' to 
'v0.7-537-g6085d11-dirty'

Originally the repo was 'clean', then 'dirty' was caused by dummy 
do_fetch_append(){} added by the test.
This test case emulates the scenario with adding some changes into the 
source and rebuilding the image.

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-29 14:20                       ` Anton Mikanovich
@ 2021-11-30 10:03                         ` Moessbauer, Felix
  2021-12-01 10:39                           ` Anton Mikanovich
  2021-12-02 14:55                           ` Anton Mikanovich
  0 siblings, 2 replies; 30+ messages in thread
From: Moessbauer, Felix @ 2021-11-30 10:03 UTC (permalink / raw)
  To: Anton Mikanovich, henning.schild
  Cc: Schmidt, Adriaan, isar-users, Baurzhan Ismagulov

> -----Original Message-----
> From: Anton Mikanovich <amikan@ilbers.de>
> Sent: Monday, November 29, 2021 3:20 PM
> To: Schild, Henning (T RDA IOT SES-DE) <henning.schild@siemens.com>
> Cc: Moessbauer, Felix (T RDA IOT SES-DE)
> <felix.moessbauer@siemens.com>; Schmidt, Adriaan (T RDA IOT SES-DE)
> <adriaan.schmidt@siemens.com>; isar-users@googlegroups.com; Baurzhan
> Ismagulov <ibr@ilbers.de>
> Subject: Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
> 
> 29.11.2021 16:18, Henning Schild wrote:
> > Well that is to expect! I guess that patch did pass our gitlab based
> > CI, but i do not know.
> > Please give more details on the failure you see. "hash diff issue" ...
> > Otherwise we will be in the "works for me, vs have you tried turning
> > it off and on again" business.
> >
> >
> > Henning
> 
> As I've already mentioned before I've just run 'fast' CI (ci_build.sh -q -f -r) on
> Jenkins which failed on RebuildTest:
> 
> 19:27:22  (07/10)
> /build/isar_am_devel_fast/256/testsuite/build_test/build_test.py:RebuildTe
> st.test_rebuild:
> ERROR: When reparsing
> mc:qemuamd64-stretch:/workspace/build/isar_am_devel_fast/256/meta-
> isar/recipes-core/images/isar-image-base.bb:do_rootfs_postprocess,
> the basehash value changed from
> 6553fa78201729c3311387f6074b35e0bd2ec2d7e745044374c4736dab3c4829 to
> 562abcf3978252df402e4956d05bbbaed9e962ee379be1f9d85c43696e18f009.
> The metadata is not deterministic and this needs to be fixed.
> 
> Previous 6 test cases were passed.
> Analysis with bitbake-diffsigs gives the real cause:
> 
> basehash changed from
> 2d5398895dd3d1f478f58a07e1f411c4f4d374a4d9d2bbae7e3dfc1517f03278 to
> d459621430969975b7eed659471d4833ca17315e117c16ac1cc10e020886007a
> Variable IMAGE_BUILD_ID value changed from 'v0.7-537-g6085d11' to 'v0.7-
> 537-g6085d11-dirty'
> 
> Originally the repo was 'clean', then 'dirty' was caused by dummy
> do_fetch_append(){} added by the test.
> This test case emulates the scenario with adding some changes into the
> source and rebuilding the image.

That is what we expect. However something changes the git state during a build.
The state must ONLY be changed between builds (two bitbake invocations).
In case multiple tests are run in parallel in the same build-tree, this will break.
Even if the patches have not been fully tested in the CI, that's what we use in our daily business (it works).

Unfortunately I have a really hard time to run the CI in the Siemens environment, as it often fails due to proxy issues or network timeouts.
Also running just the repro target itself does not work due to other issues.
I tried with: avocado run testsuite/build_test/build_test.py:RebuildTest.test_rebuild

[stdout] E: Unable to find a source package for
[stdlog] 2021-11-30 10:58:04,196 cibuilder        L0100 ERROR| E: Unable to find a source package for
[stdout] + bb_exit_handler
[stdlog] 2021-11-30 10:58:04,196 cibuilder        L0100 ERROR| + bb_exit_handler

Anyways, if there are more issues to be ruled out for this patch series, I actually prefer to merge the SState series first.
Currently, a lot of patches are waiting for SState, as Henning mentioned.

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-30 10:03                         ` Moessbauer, Felix
@ 2021-12-01 10:39                           ` Anton Mikanovich
  2021-12-02 14:55                           ` Anton Mikanovich
  1 sibling, 0 replies; 30+ messages in thread
From: Anton Mikanovich @ 2021-12-01 10:39 UTC (permalink / raw)
  To: Moessbauer, Felix
  Cc: henning.schild, Schmidt, Adriaan, isar-users, Baurzhan Ismagulov

30.11.2021 13:03, Moessbauer, Felix wrote:
> That is what we expect. However something changes the git state during a build.
> The state must ONLY be changed between builds (two bitbake invocations).
> In case multiple tests are run in parallel in the same build-tree, this will break.

Avocado is not using parallel test case execution currently. It can, but 
out CI is not jet ready for it.

> Unfortunately I have a really hard time to run the CI in the Siemens environment, as it often fails due to proxy issues or network timeouts.
> Also running just the repro target itself does not work due to other issues.
> I tried with: avocado run testsuite/build_test/build_test.py:RebuildTest.test_rebuild
>
> [stdout] E: Unable to find a source package for
> [stdlog] 2021-11-30 10:58:04,196 cibuilder        L0100 ERROR| E: Unable to find a source package for
> [stdout] + bb_exit_handler
> [stdlog] 2021-11-30 10:58:04,196 cibuilder        L0100 ERROR| + bb_exit_handler

You can find an example of direct avocado usage in ci_build.sh.
The minimal working cmd for fast testing will look like:

$ avocado run testsuite/build_test/build_test.py -t fast 
--test-runner=runner

Or in case of rebuild test only:

$ avocado run testsuite/build_test/build_test.py -t rebuild 
--test-runner=runner

But the issue was not reproduced if execute rebuild test itself, it 
happens only in case of all fast tests running in a row.

> Anyways, if there are more issues to be ruled out for this patch series, I actually prefer to merge the SState series first.
> Currently, a lot of patches are waiting for SState, as Henning mentioned.
>
> Felix
>
I also prefer sstate merging first. Will do it tomorrow if no objections.

-- 

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-11-30 10:03                         ` Moessbauer, Felix
  2021-12-01 10:39                           ` Anton Mikanovich
@ 2021-12-02 14:55                           ` Anton Mikanovich
  2021-12-03  9:51                             ` Moessbauer, Felix
  1 sibling, 1 reply; 30+ messages in thread
From: Anton Mikanovich @ 2021-12-02 14:55 UTC (permalink / raw)
  To: Moessbauer, Felix, henning.schild
  Cc: Schmidt, Adriaan, isar-users, Baurzhan Ismagulov

30.11.2021 13:03, Moessbauer, Felix wrote:
> That is what we expect. However something changes the git state during a build.
> The state must ONLY be changed between builds (two bitbake invocations).
> In case multiple tests are run in parallel in the same build-tree, this will break.
> Even if the patches have not been fully tested in the CI, that's what we use in our daily business (it works).
>
> Unfortunately I have a really hard time to run the CI in the Siemens environment, as it often fails due to proxy issues or network timeouts.
> Also running just the repro target itself does not work due to other issues.
> I tried with: avocado run testsuite/build_test/build_test.py:RebuildTest.test_rebuild
>
> [stdout] E: Unable to find a source package for
> [stdlog] 2021-11-30 10:58:04,196 cibuilder        L0100 ERROR| E: Unable to find a source package for
> [stdout] + bb_exit_handler
> [stdlog] 2021-11-30 10:58:04,196 cibuilder        L0100 ERROR| + bb_exit_handler
>
> Anyways, if there are more issues to be ruled out for this patch series, I actually prefer to merge the SState series first.
> Currently, a lot of patches are waiting for SState, as Henning mentioned.
>
> Felix

Ok, there are easy steps to reproduce the issue with kas:

$ cat isar.yml
header:
   version: 10

distro: debian-bullseye
machine: qemuamd64
target: mc:qemuamd64-bullseye:isar-image-base
repos:
   isar:
     url: "git@github.com:/ilbers/isar"
     refspec: "4dc41fba8d664fbc00ec56640fec066c9e57b19d"
     layers:
       meta:
       meta-isar:

bblayers_conf_header:
   standard: |
     BBPATH = "${TOPDIR}"
     BBFILES ?= ""

$ time ~/kas/kas-docker --isar build isar.yml
$ echo -e "do_fetch_append() {\n\n}" >> isar/meta/classes/dpkg-base.bbclass
$ time ~/kas/kas-docker --isar build isar.yml

On the first obtain of IMAGE_BUILD_ID it gets correct 'dirty' value, but 
compare it with old hash from the previous build at fails.

Even if the patchset is working for upstream it should not introduce 
such a regression.

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* RE: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
  2021-12-02 14:55                           ` Anton Mikanovich
@ 2021-12-03  9:51                             ` Moessbauer, Felix
  0 siblings, 0 replies; 30+ messages in thread
From: Moessbauer, Felix @ 2021-12-03  9:51 UTC (permalink / raw)
  To: Anton Mikanovich, Schmidt, Adriaan
  Cc: isar-users, Baurzhan Ismagulov, henning.schild

> From: Anton Mikanovich <amikan@ilbers.de>
> Sent: Thursday, December 2, 2021 3:55 PM
> To: Moessbauer, Felix (T RDA IOT SES-DE) <felix.moessbauer@siemens.com>;
> Schild, Henning (T RDA IOT SES-DE) <henning.schild@siemens.com>
> Cc: Schmidt, Adriaan (T RDA IOT SES-DE) <adriaan.schmidt@siemens.com>; isar-
> users@googlegroups.com; Baurzhan Ismagulov <ibr@ilbers.de>
> Subject: Re: [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD
> 
> 30.11.2021 13:03, Moessbauer, Felix wrote:
> > That is what we expect. However something changes the git state during a
> build.
> > The state must ONLY be changed between builds (two bitbake invocations).
> > In case multiple tests are run in parallel in the same build-tree, this will break.
> > Even if the patches have not been fully tested in the CI, that's what we use in
> our daily business (it works).
> >
> > Unfortunately I have a really hard time to run the CI in the Siemens
> environment, as it often fails due to proxy issues or network timeouts.
> > Also running just the repro target itself does not work due to other issues.
> > I tried with: avocado run
> > testsuite/build_test/build_test.py:RebuildTest.test_rebuild
> >
> > [stdout] E: Unable to find a source package for
> > [stdlog] 2021-11-30 10:58:04,196 cibuilder        L0100 ERROR| E: Unable to
> find a source package for
> > [stdout] + bb_exit_handler
> > [stdlog] 2021-11-30 10:58:04,196 cibuilder        L0100 ERROR| +
> bb_exit_handler
> >
> > Anyways, if there are more issues to be ruled out for this patch series, I
> actually prefer to merge the SState series first.
> > Currently, a lot of patches are waiting for SState, as Henning mentioned.
> >
> > Felix
> 
> Ok, there are easy steps to reproduce the issue with kas:
> 
> $ cat isar.yml
> header:
>    version: 10
> 
> distro: debian-bullseye
> machine: qemuamd64
> target: mc:qemuamd64-bullseye:isar-image-base
> repos:
>    isar:
>      url: "git@github.com:/ilbers/isar"
>      refspec: "4dc41fba8d664fbc00ec56640fec066c9e57b19d"
>      layers:
>        meta:
>        meta-isar:
> 
> bblayers_conf_header:
>    standard: |
>      BBPATH = "${TOPDIR}"
>      BBFILES ?= ""
> 
> $ time ~/kas/kas-docker --isar build isar.yml $ echo -e "do_fetch_append()
> {\n\n}" >> isar/meta/classes/dpkg-base.bbclass
> $ time ~/kas/kas-docker --isar build isar.yml
> 
> On the first obtain of IMAGE_BUILD_ID it gets correct 'dirty' value, but compare
> it with old hash from the previous build at fails.

Ok thanks for the reproducer. By that I was able to navigate bitbake into the error.

Unfortunately this problem is not easy to solve, as it is simply not allowed to modify the metadata on the fly.
By that, we cannot use an anonymous python function to taint a task depending on a condition.
If we want up-to-date versioning information, we would have to use a "nostamp" task (but that makes rootfs caching irrelevant)...

A better solution would be to reorder the tasks to make sure that the sstate caches the rootfs without /etc/os-release information.
This at least restores the previous behavior (re-building without change does not re-run ISAR_RELEASE_CMD).
Without these changes, probably a lot of CI generated images that use a persistent sstate will end up with outdated version information.
@Adriaan: What's your opinion?

> 
> Even if the patchset is working for upstream it should not introduce such a
> regression.

Agreed!

Patch 2 of this series should be fine (just a bugfix).

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


^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2021-12-03  9:51 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 11:05 [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD Felix Moessbauer
2021-11-04 11:05 ` [PATCH v4 1/2] always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on change Felix Moessbauer
2021-11-04 11:05 ` [PATCH v4 2/2] Ensure generation of /etc/os-release is idempotent Felix Moessbauer
2021-11-04 13:12 ` [PATCH v4 0/2] Improve handling of ISAR_RELEASE_CMD Henning Schild
2021-11-16 18:09 ` Anton Mikanovich
2021-11-17 10:45   ` Moessbauer, Felix
2021-11-17 13:05     ` Anton Mikanovich
2021-11-17 15:57       ` Moessbauer, Felix
2021-11-17 16:39         ` Baurzhan Ismagulov
2021-11-22 15:28         ` Anton Mikanovich
2021-11-24  9:30           ` Schmidt, Adriaan
2021-11-24 10:15             ` Moessbauer, Felix
2021-11-24 11:25               ` Jan Kiszka
2021-11-24 11:54                 ` Anton Mikanovich
2021-11-24 12:11               ` Henning Schild
2021-11-29  9:09                 ` Anton Mikanovich
2021-11-29  9:50                   ` Henning Schild
2021-11-29  9:55                     ` Anton Mikanovich
2021-11-29 13:04                   ` Anton Mikanovich
2021-11-29 13:18                     ` Henning Schild
2021-11-29 14:20                       ` Anton Mikanovich
2021-11-30 10:03                         ` Moessbauer, Felix
2021-12-01 10:39                           ` Anton Mikanovich
2021-12-02 14:55                           ` Anton Mikanovich
2021-12-03  9:51                             ` Moessbauer, Felix
2021-11-24 11:53             ` Anton Mikanovich
2021-11-24 12:35             ` Henning Schild
2021-11-24 12:52               ` Anton Mikanovich
2021-11-17 16:54       ` Henning Schild
2021-11-18  8:36         ` Moessbauer, Felix

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox