From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 7026321669488640000 X-Received: by 2002:aa7:cb41:: with SMTP id w1mr61572110edt.327.1635946943029; Wed, 03 Nov 2021 06:42:23 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 2002:aa7:c944:: with SMTP id h4ls459241edt.1.gmail; Wed, 03 Nov 2021 06:42:22 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyNBswLhtJzjgdzyCzSkoe8oF0GcKx0ytqgJ2CZuir4gFzuHM3/Fx/FzGEJ8MMxC5oH4bKp X-Received: by 2002:a05:6402:402:: with SMTP id q2mr60629507edv.248.1635946942047; Wed, 03 Nov 2021 06:42:22 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1635946942; cv=none; d=google.com; s=arc-20160816; b=xy879vjU/wkTZeN3asrCnVe26XMVEa47YlhoMw0/6DckSRMdk/qZZoTZ8Q40YJ+ljD U6oYvtGyjmZ1j7GFeEj9B7Dke/8iZCs0uamqvYpF1onI9ZLQoclSB7/6l5U2osQC+seR 3x2dj/bQPQKvMnwTPQbGpLr3Fsqe9GkWOMrZMp8rZaxQIibau6UFmZi2KaRaDj5iexlj +7XPI9y0cbsYbrSAgyFym19MuTnLoiwZdOyYntJqZXeMZfM0WcqR0MhaaGhmvkeZG8tL j7gNJgk7Gvx7kp5fIuNjnKqVJACiNANSBNXyFBsDiw5SXVLhwwlxSZgsGLsNhbdCvNTD dt7A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date; bh=vt3GkycWZES0SonnGBTNJsF8L+d5BUfz7MqU8UWg9GM=; b=tE7AvK0QMuc6GFGrtN5RKtKI3ZBvvBWtxKrXeRzTic8uqBhgMzSyYvw6Z7pBFO6mup +OuGZ4g3RjEJDn5oOK1Nq3GtF4Cr7BxLtwc7P/upZlUlPboRmMuwshMDQHPZ83K1MUk4 stdBEtZAolE5fXW4xD2c5Iqlex3/Z85GQQa7rf/z0WcmWFMS9wwv65Z/KROm8ttpX29J +n3DaKHQnRIZH1LrulfX40u734Temm9YGZRYmybEVeUikaYolRS8KvNCz0LiiEaI0laG x/n7SbXhkWgUMn9KUDRQM7sAk9QaG4Nwsevg8frfSnvyHi9AtOeXj6uS18O4AtTOHC3u BhXQ== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=pass (google.com: domain of henning.schild@siemens.com designates 192.35.17.14 as permitted sender) smtp.mailfrom=henning.schild@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Return-Path: Received: from david.siemens.de (david.siemens.de. [192.35.17.14]) by gmr-mx.google.com with ESMTPS id dr8si325155ejc.0.2021.11.03.06.42.21 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Nov 2021 06:42:22 -0700 (PDT) Received-SPF: pass (google.com: domain of henning.schild@siemens.com designates 192.35.17.14 as permitted sender) client-ip=192.35.17.14; Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of henning.schild@siemens.com designates 192.35.17.14 as permitted sender) smtp.mailfrom=henning.schild@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Received: from mail1.sbs.de (mail1.sbs.de [192.129.41.35]) by david.siemens.de (8.15.2/8.15.2) with ESMTPS id 1A3DgLfE003403 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 3 Nov 2021 14:42:21 +0100 Received: from md1za8fc.ad001.siemens.net ([139.25.69.154]) by mail1.sbs.de (8.15.2/8.15.2) with ESMTP id 1A3DgLKa011725; Wed, 3 Nov 2021 14:42:21 +0100 Date: Wed, 3 Nov 2021 14:42:20 +0100 From: Henning Schild To: Felix Moessbauer Cc: , Subject: Re: [PATCH v3 1/2] always invoke ISAR_RELEASE_CMD to invalidate downstream tasks on change Message-ID: <20211103144220.3c8e537e@md1za8fc.ad001.siemens.net> In-Reply-To: <20211103123624.3015125-2-felix.moessbauer@siemens.com> References: <20211103123624.3015125-1-felix.moessbauer@siemens.com> <20211103123624.3015125-2-felix.moessbauer@siemens.com> X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TUID: mOOxlWmxzzGS Am Wed, 3 Nov 2021 13:36:23 +0100 schrieb 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. Yes, in fact i would further accept that instead of rebuilding. > When caching artifacts across builds (e.g. using sstate), > this also affects fresh builds. Does it? It is an image postprocess step, and images are not sstate cached. Unless we do image-in-package tricks for containers and VMs, because packages will be sstate-cached. > 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. It is really hard to say what is correct. If a git change does not trigger the image to be rebuild ... the code change very likely does not do anything anyways. So whichever "git describe" comes out, both are equivavlent and kind of "correct". And you save the second "pointless" build. I have seen layers that "echo $git_describe $date", those would never be "correct" ;). And say you do not commit you will only get "sha-dirty" but never "sha-very-dirty". Not sure this patch is valid. Would it rebuild if i did "touch hello; git add hello; git commit -m hello" ... so not change anything that would affect the image? Or would it even rebuild every single time? Henning > Signed-off-by: Felix Moessbauer > --- > meta/classes/image-postproc-extension.bbclass | 4 +-- > meta/classes/image.bbclass | 32 > ++++++++++--------- meta/classes/rootfs.bbclass | > 2 +- 3 files changed, 20 insertions(+), 18 deletions(-) > > 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)