Re: [PATCH] meta/image: leave BUILD_ID and VARIANT in /etc/os-release

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

Am Fri, 29 Jun 2018 12:04:25 +0200
schrieb "Maxim Yu. Osipov" <mosipov@ilbers.de>:

> Hi Henning,
> 
> On 06/28/2018 05:09 PM, Henning Schild wrote:
> > Am Thu, 28 Jun 2018 17:05:21 +0200
> > schrieb Henning Schild <henning.schild@siemens.com>:
> >   
> >> This commit adds support for leaving a mark in the resulting image.
> >> That should allow you to tell what code you used to create it. By
> >> default "git describe" will be used on Isar itself to come up with
> >> a BUILD_ID. And the DESCRIPTION of the image will become the
> >> VARIANT.
> >>
> >> see
> >> https://www.freedesktop.org/software/systemd/man/os-release.html
> >>
> >> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> >> ---
> >>   meta-isar/recipes-core/images/isar-image-base.bb |  2 ++
> >>   meta/classes/image.bbclass                       | 27
> >> ++++++++++++++++++++++++
> >> meta/classes/isar-bootstrap-helper.bbclass       | 26
> >> +++++++++++++++++++++++ 3 files changed, 55 insertions(+)
> >>
> >> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb
> >> b/meta-isar/recipes-core/images/isar-image-base.bb index
> >> 6cdc6b2..c72bfdb 100644 ---
> >> a/meta-isar/recipes-core/images/isar-image-base.bb +++
> >> b/meta-isar/recipes-core/images/isar-image-base.bb @@ -24,6 +24,8
> >> @@ IMAGE_TRANSIENT_PACKAGES += "isar-cfg-localepurge"
> >>   WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
> >>   
> >> +ISAR_GIT_RELEASE_PATH ?= "${LAYERDIR_isar}"
> >> +
> >>   do_rootfs[root_cleandirs] = "${IMAGE_ROOTFS} \
> >>                                ${IMAGE_ROOTFS}/isar-apt"
> >>   
> >> diff --git a/meta/classes/image.bbclass
> >> b/meta/classes/image.bbclass index 3bdcb2f..00040e5 100644
> >> --- a/meta/classes/image.bbclass
> >> +++ b/meta/classes/image.bbclass
> >> @@ -34,6 +34,25 @@ def get_rootfs_size(d):
> >>   
> >>       return base_size + rootfs_extra * 1024
> >>   
> >> +# we assume that one git commit can describe the whole image, so
> >> you should be +# using submodules, kas, or something like that
> >> +# set ISAR_GIT_RELEASE_PATH to that one "most significant" layer
> >> +# when not using git, override do_mark_rootfs
> >> +def get_build_id(d):
> >> +    import subprocess
> >> +    if (len(d.getVar("BBLAYERS", True).strip().split(' ')) != 2
> >> and
> >> +        (d.getVar("ISAR_GIT_RELEASE_PATH", True) ==
> >> +         d.getVar("LAYERDIR_isar", True))):
> >> +        bb.warn('You are using external layers that will not be
> >> considered' +
> >> +                ' in the build_id. Considder setting
> >> ISAR_GIT_RELEASE_PATH.')
> >> +    base = ["git", "-C", d.getVar("ISAR_GIT_RELEASE_PATH", True)]
> >> +    if (0 == subprocess.call(base + ["rev-parse"])):
> >> +        v = subprocess.check_output(base +
> >> +                                    ["describe", "--long",
> >> "--dirty",
> >> +                                     "--always"],  
> > 
> > Here i am not sure what arguments to use. That also depends on the
> > tagging of the project. We might want to make this configurable,
> > since the "most significant" layer might use different tagging.
> > Opinions?  
> 
> In my opinion 'git describe --dirty --match "v[0-9].[0-9]*" would be
> enough.
> 
> It would be nice to add document describing versioning 
> docs/VERSIONING.md. Something like that:
> 
>  >>>  
> ISAR Versioning
> 
> ISAR uses for release versioning unsigned, annotated tag
> in format "v[0-9].[0-9]*".
> 
> The following procedure has to be followed for release versioning:
> 
> 1) Update CHANGELOG.md in ISAR root directory.
> 
> 2) Before creation of final release, prepare release candidate(s)
> with the help of command 'git tag -a <release_candidate_tagname>',
> where <release_candidate_tagname> has the format
> "v[0-9].[0-9]-rc[0-9]"
> 
> 3) When final release is ready create annotated tag
> with the help of command 'git tag -a <release_tagname>', where
> <release_tagname> has the format "v[0-9].[0-9]"
> 
> BUILD_ID field in /etc/os-release keeps ISAR repository version
> derived with the help of command 'git describe --dirty --match 
> "v[0-9].[0-9]*'.
> 
> This command finds the most recent annotated tag that is reachable
> from a commit.
> If the tag points to the commit, then only the tag is shown.
> Otherwise, it suffixes the tag name with the number of additional
> commits on top of the tagged object and the abbreviated object name
> of the most recent commit. If the working tree is modified '-dirty'
> will be appended to the end of the version string.
> 
> For example, BUILD_ID may have the values like
> - "v3.0-rc2" for 3.0 release candidate 2
> - "v3.0" for released 3.0 version
> - "v3.0-112-g118cd4b" for developer's version,
> meaning that 112 commits exist since recent release and the most
> recent commit has abbreviated name 118cd4b.
> - "v3.0-112-g118cd4b-dirty" for developer's version,
> meaning that 112 commits exist since recent release, the most
> recent commit has abbreviated name 118cd4b and the working tree
> is modified.
>  >>>  
> 
> Is it OK?

Thanks for citing half of the manpage ;). I think you can do whatever
you want. The point is that get_build_id should be generic enough to
also work for repos calling their tags completely differently, or not
using tags at all.
Think of a private layer-repo that happens to use Isar but has another
versioning policy. The BUILD_ID in the image should be the one of that
layer, set by ISAR_GIT_RELEASE_PATH.
People that do not use git at all override do_mark_rootfs, but for the
ones using git it would be nice if they would not have to.

Henning

> 
> > 
> > In that regard i realized that the tags on Isar are not consistent.
> > v0.3 seems to be the last annotated one.  
> 
> Thank you for pointing this out - after applying your patch we will 
> start to follow well defined versioning/release policy.
> 
> Kind regards,
> Maxim.
> 
> > 
> > Henning
> >   
> >> universal_newlines=True)
> >> +        return v.rstrip()
> >> +    return ""
> >> +
> >>   python set_image_size () {
> >>       rootfs_size = get_rootfs_size(d)
> >>       d.setVar('ROOTFS_SIZE', str(rootfs_size))
> >> @@ -56,6 +75,14 @@ do_rootfs() {
> >>   addtask rootfs before do_build after do_unpack
> >>   do_rootfs[deptask] = "do_deploy_deb"
> >>   
> >> +do_mark_rootfs() {
> >> +    update_etc_os_release \
> >> +        --build-id "${@get_build_id(d)}" --variant
> >> "${DESCRIPTION}" \
> >> +        "${IMAGE_ROOTFS}"
> >> +}
> >> +
> >> +addtask mark_rootfs before do_copy_boot_files after do_rootfs
> >> +
> >>   do_copy_boot_files() {
> >>       KERNEL_IMAGE=${@get_image_name(d, 'vmlinuz')[1]}
> >>       if [ -n "${KERNEL_IMAGE}" ]; then
> >> diff --git a/meta/classes/isar-bootstrap-helper.bbclass
> >> b/meta/classes/isar-bootstrap-helper.bbclass index 3c10fc7..f375853
> >> 100644 --- a/meta/classes/isar-bootstrap-helper.bbclass
> >> +++ b/meta/classes/isar-bootstrap-helper.bbclass
> >> @@ -14,6 +14,32 @@ def reverse_bb_array(d, varname):
> >>       array = reversed(array.split())
> >>       return " ".join(i for i in array)
> >>   
> >> +update_etc_os_release() {
> >> +    OS_RELEASE_BUILD_ID=""
> >> +    OS_RELEASE_VARIANT=""
> >> +    while true; do
> >> +        case "$1" in
> >> +	--build-id) OS_RELEASE_BUILD_ID=$2; shift ;;
> >> +	--variant) OS_RELEASE_VARIANT=$2; shift ;;
> >> +        -*) bbfatal "$0: invalid option specified: $1" ;;
> >> +        *) break ;;
> >> +        esac
> >> +        shift
> >> +    done
> >> +    ROOTFSDIR="$1"
> >> +
> >> +    if [ -n "${OS_RELEASE_BUILD_ID}" ]; then
> >> +        sudo sed -i '/^BUILD_ID=.*/d' ${ROOTFSDIR}/etc/os-release
> >> +        echo "BUILD_ID=\"${OS_RELEASE_BUILD_ID}\"" | \
> >> +            sudo tee -a ${ROOTFSDIR}/etc/os-release
> >> +    fi
> >> +    if [ -n "${OS_RELEASE_VARIANT}" ]; then
> >> +        sudo sed -i '/^VARIANT=.*/d' ${ROOTFSDIR}/etc/os-release
> >> +        echo "VARIANT=\"${OS_RELEASE_VARIANT}\"" | \
> >> +            sudo tee -a ${ROOTFSDIR}/etc/os-release
> >> +    fi
> >> +}
> >> +
> >>   setup_root_file_system() {
> >>       CLEAN=""
> >>       FSTAB=""  
> >   
> 
>