From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6815578895823142912 X-Received: by 2002:aca:aa8c:: with SMTP id t134mr4542468oie.103.1587497309068; Tue, 21 Apr 2020 12:28:29 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 2002:a9d:69d7:: with SMTP id v23ls2287616oto.0.gmail; Tue, 21 Apr 2020 12:28:28 -0700 (PDT) X-Google-Smtp-Source: APiQypKv8R/3tFUXxIMln4aUMSnPrYVO9hE9eGF8gSD9B0vRiKQKfxtR6r8TTwbUUk7p8WuyIG9SYt34nw== X-Received: by 2002:a05:6830:1e7c:: with SMTP id m28mr15505942otr.151.1587497308531; Tue, 21 Apr 2020 12:28:28 -0700 (PDT) X-Google-Thread-Subscription: Yes X-Google-Web-Client: true Date: Tue, 21 Apr 2020 12:28:28 -0700 (PDT) From: cedric_hombourger@mentor.com To: isar-users Message-Id: In-Reply-To: <20200421204939.0ddaa020@md1za8fc.ad001.siemens.net> References: <20200421150134.30325-1-Quirin.Gylstorff@siemens.com> <20200421204939.0ddaa020@md1za8fc.ad001.siemens.net> Subject: Re: [PATCH v2] classes/image-postproc: Add image version MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_1330_660268889.1587497308381" X-Google-Token: ENyS_fQF5qU6yI4jL640 X-Google-IP: 139.181.48.2 X-TUID: nsAgWDQMiDfR ------=_Part_1330_660268889.1587497308381 Content-Type: multipart/alternative; boundary="----=_Part_1331_16197371.1587497308382" ------=_Part_1331_16197371.1587497308382 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On Tuesday, April 21, 2020 at 8:49:43 PM UTC+2, Henning Schild wrote: > > On Tue, 21 Apr 2020 17:01:34 +0200 > "[ext] Q. Gylstorff" > wrote: > > > From: Quirin Gylstorff > > > > > Add the image version as additional identifier to /etc/os-release. > > This allows in a update scenario an easier identification of the > > the currently used image. > > > > Signed-off-by: Quirin Gylstorff > > > --- > > meta/classes/image-postproc-extension.bbclass | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/meta/classes/image-postproc-extension.bbclass > > b/meta/classes/image-postproc-extension.bbclass index > > 7280202..1091fa4 100644 --- > > a/meta/classes/image-postproc-extension.bbclass +++ > > b/meta/classes/image-postproc-extension.bbclass @@ -4,10 +4,12 @@ > > update_etc_os_release() { > > OS_RELEASE_BUILD_ID="" > > OS_RELEASE_VARIANT="" > > + OS_RELEASE_VARIANT_VERSION="" > > while true; do > > case "$1" in > > --build-id) OS_RELEASE_BUILD_ID=$2; shift ;; > > --variant) OS_RELEASE_VARIANT=$2; shift ;; > > + --version) OS_RELEASE_VARIANT_VERSION=$2; shift ;; > > -*) bbfatal "$0: invalid option specified: $1" ;; > > *) break ;; > > esac > > @@ -24,6 +26,11 @@ update_etc_os_release() { > > echo "VARIANT=\"${OS_RELEASE_VARIANT}\"" | \ > > sudo tee -a '${IMAGE_ROOTFS}/etc/os-release' > > fi > > + if [ -n "${OS_RELEASE_VARIANT_VERSION}" ]; then > > + sudo sed -i '/^ISAR_IMAGE_VERSION=.*/d' > > '${IMAGE_ROOTFS}/etc/os-release' > > + echo "VARIANT_VERSION=\"${PV}\"" | \ > > + sudo tee -a '${IMAGE_ROOTFS}/etc/os-release' > > Looking at > https://www.freedesktop.org/software/systemd/man/os-release.html > > VARIANT_VERSION does not show up in the list of > "The following OS identifications parameters may be set using > os-release" > > So i would conclude we may not use it. I would suggest either finding a > variable that we may use and debian does not use yet. > for everyone's benefits, Henning and I were discussing this offline. from my perspective, the freedesktop folks aren't that rigid. 2 quotes from them: (1) The file is extensible? Awesome! I want a new field XYZ= in it! Sure, it's extensible, and we are happy if distributions extend it. Please prefix your keys with your distribution's name however. (2) If you are working on a small/embedded distribution, or a legacy-free distribution we encourage you to adopt only this file and not establish any other per-distro release file. I am guilty of adding custom fields in the Debian-based distro we are producing over here. I had missed the recommendation to prefix new keys with the name of the distro. That's a good thing to do for sure. With that said, I am not saying that Isar should or should not add custom entries there. Just wanted to say that it is not forbidden Ref: http://0pointer.de/blog/projects/os-release (link found from: http://0pointer.de/blog/projects/os-release) Cedric > Or the layer with the requirement looks at BUILD_ID and sets a custom > ISAR_RELEASE_CMD to inject their clue. > Here is an example where people decided to include the date of a build > > ISAR_RELEASE_CMD = "echo $(git -C ${LAYERDIR_isar-XXX} describe --long > --dirty --always) $(date --utc --rfc-3339=seconds)" > > Henning > > > + fi > > } > > > > ROOTFS_POSTPROCESS_COMMAND =+ "image_postprocess_configure" > > @@ -43,7 +50,7 @@ ROOTFS_POSTPROCESS_COMMAND =+ > > "image_postprocess_mark" image_postprocess_mark() { > > BUILD_ID=$(get_build_id) > > update_etc_os_release \ > > - --build-id "${BUILD_ID}" --variant "${DESCRIPTION}" > > + --build-id "${BUILD_ID}" --variant "${DESCRIPTION}" > > --version "${PV}" } > > > > ROOTFS_POSTPROCESS_COMMAND =+ "image_postprocess_machine_id" > > ------=_Part_1331_16197371.1587497308382 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable


On Tuesday, April 21, 2020 at 8:49:43 PM UTC+2, He= nning Schild wrote:
On Tue, 21 = Apr 2020 17:01:34 +0200
"[ext] Q. Gylstorff" <Quirin....@siemens.com> wrote:

> From: Quirin Gylstorff <quirin....@siemens.com>
>=20
> Add the image version as additional identifier to /etc/os-release.
> This allows in a update scenario an easier identification of the
> the currently used image.
>=20
> Signed-off-by: Quirin Gylstorff <quirin....@siemens.com>
> ---
> =C2=A0meta/classes/image-postproc-extension.bbclass | 9 +++++= +++-
> =C2=A01 file changed, 8 insertions(+), 1 deletion(-)
>=20
> diff --git a/meta/classes/image-postproc-extension.bbclass
> b/meta/classes/image-postproc-extension.bbclass index
> 7280202..1091fa4 100644 ---
> a/meta/classes/image-postproc-extension.bbclass +++
> b/meta/classes/image-postproc-extension.bbclass @@ -4,10 +4,1= 2 @@
> =C2=A0update_etc_os_release() {
> =C2=A0 =C2=A0 =C2=A0OS_RELEASE_BUILD_ID=3D""
> =C2=A0 =C2=A0 =C2=A0OS_RELEASE_VARIANT=3D""
> + =C2=A0 =C2=A0OS_RELEASE_VARIANT_VERSION=3D""
> =C2=A0 =C2=A0 =C2=A0while true; do
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case "$1" in
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0--build-id) OS_RELEASE_BUILD_ID= =3D$2; shift ;;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0--variant) OS_RELEASE_VARIANT=3D= $2; shift ;;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0--version) OS_RELEASE_VARIANT_VERSION= =3D$2; shift ;;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0-*) bbfatal "$0: invalid op= tion specified: $1" ;;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*) break ;;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0esac
> @@ -24,6 +26,11 @@ update_etc_os_release() {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "VARIANT=3D\"${OS= _RELEASE_VARIANT}\"" | \
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sudo tee -a '$= {IMAGE_ROOTFS}/etc/os-release'
> =C2=A0 =C2=A0 =C2=A0fi
> + =C2=A0 =C2=A0if [ -n "${OS_RELEASE_VARIANT_VERSION}&qu= ot; ]; then
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0sudo sed -i '/^ISAR_IMAGE_VERSION= =3D.*/d'
> '${IMAGE_ROOTFS}/etc/os-release'
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "VARIANT_VERSION=3D\"$= {PV}\"" | \
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sudo tee -a '${IMAG= E_ROOTFS}/etc/os-release'

Looking at
https://www.freedesktop.or= g/software/systemd/man/os-release.html

VARIANT_VERSION does not show up in the list of=20
"The following OS identifications parameters may be set using
os-release"

So i would conclude we may not use it. I would suggest either finding a
variable that we may use and debian does not use yet.

for everyone's benefits, Henning a= nd I were discussing this offline. from my perspective, the freedesktop fol= ks aren't that rigid. 2 quotes from them:

(1) = The file is extensible? Awesome! I want a new field XYZ=3D in it! Sure, it's extensible, and we are happy if distributions extend it. Please pr= efix your keys with your distribution's name however.

(2) If you are working on a small/embedded distribution, or a legacy-fre= e distribution we encourage you to adopt only this file and not establish any other per-distro release file.

I am guilty of addi= ng custom fields in the Debian-based distro we are producing over here. I h= ad missed the recommendation to prefix new keys with the name of the distro= . That's a good thing to do for sure.

With tha= t said, I am not saying that Isar should or should not add custom entries t= here. Just wanted to say that it is not forbidden
Ref: http://0po= inter.de/blog/projects/os-release (link found from: http://0pointer.de/blog= /projects/os-release)

Cedric


Or the layer with the requirement looks at BUILD_ID and sets a custom
ISAR_RELEASE_CMD to inject their clue.
Here is an example where people decided to include the date of a build

ISAR_RELEASE_CMD =3D "echo $(git -C ${LAYERDIR_isar-XXX} describe = --long
--dirty --always) $(date --utc --rfc-3339=3Dseconds)"

Henning

> + =C2=A0 =C2=A0fi
> =C2=A0}
> =C2=A0
> =C2=A0ROOTFS_POSTPROCESS_COMMAND =3D+ "image_postprocess_conf= igure"
> @@ -43,7 +50,7 @@ ROOTFS_POSTPROCESS_COMMAND =3D+
> "image_postprocess_mark" image_postprocess_mark() {
> =C2=A0 =C2=A0 =C2=A0BUILD_ID=3D$(get_build_id)
> =C2=A0 =C2=A0 =C2=A0update_etc_os_release \
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0--build-id "${BUILD_ID}" --= variant "${DESCRIPTION}"
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0--build-id "${BUILD_ID}" --= variant "${DESCRIPTION}"
> --version "${PV}" }
> =C2=A0
> =C2=A0ROOTFS_POSTPROCESS_COMMAND =3D+ "image_postprocess_mach= ine_id"

------=_Part_1331_16197371.1587497308382-- ------=_Part_1330_660268889.1587497308381--