Re: [PATCH] meta/image: allow customization of command we use fo BUILD_ID

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

Am Mon, 23 Jul 2018 08:40:50 +0200
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 2018-07-23 08:38, Jan Kiszka wrote:
> > On 2018-07-03 09:39, Henning Schild wrote:  
> >> One of the previous patches introduced the feature to leave some
> >> build system information in the rootfs. The git command to derive
> >> that information was kept pretty repository agnostic.
> >>
> >> This patch makes the command Isar-specific and allows users to
> >> customize the command to their needs if they use a different
> >> tagging scheme in their repository, or if they do not use git at
> >> all.
> >>
> >> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> >> ---
> >>   meta-isar/recipes-core/images/isar-image-base.bb |  3 ++-
> >>   meta/classes/image.bbclass                       | 27 
> >> ++++++++++++------------
> >>   2 files changed, 16 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb 
> >> b/meta-isar/recipes-core/images/isar-image-base.bb
> >> index c72bfdb..a24aa3e 100644
> >> --- a/meta-isar/recipes-core/images/isar-image-base.bb
> >> +++ b/meta-isar/recipes-core/images/isar-image-base.bb
> >> @@ -24,7 +24,8 @@ IMAGE_TRANSIENT_PACKAGES +=
> >> "isar-cfg-localepurge" WORKDIR =
> >> "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
> >> -ISAR_GIT_RELEASE_PATH ?= "${LAYERDIR_isar}"
> >> +ISAR_RELEASE_CMD_DEFAULT = "git -C ${LAYERDIR_isar} describe
> >> --tags --dirty --match 'v[0-9].[0-9]*'"  
> > 
> > I think we should keep ISAR_GIT_RELEASE_PATH as overridable API 
> > variable. Many project will use the default pattern and just
> > deviate in the layer name.
> >   
> 
> Just to clarify: We should have *both* ISAR_GIT_RELEASE_PATH and 
> ISAR_RELEASE_CMD as API. The former for the common customizations,
> the latter for deviating tag schemes or different version control
> systems.

I think that would introduce too many knobs to turn. The CMD instead of
the default git is not just there to abstract away from git. It also
takes care of its arguments, like matching a tag.
While it is more than likely that people will use git, i am not sure
the args will match for everyone.

If you still want that variable back, i will send an update.

Henning 

> Jan
> 
> >> +ISAR_RELEASE_CMD ?= "${ISAR_RELEASE_CMD_DEFAULT}"
> >>   do_rootfs[root_cleandirs] = "${IMAGE_ROOTFS} \
> >>                                ${IMAGE_ROOTFS}/isar-apt"
> >> diff --git a/meta/classes/image.bbclass
> >> b/meta/classes/image.bbclass index 7935b69..077c550 100644
> >> --- a/meta/classes/image.bbclass
> >> +++ b/meta/classes/image.bbclass
> >> @@ -34,24 +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
> >> +# 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
> >>   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))):
> >> +        (d.getVar("ISAR_RELEASE_CMD", True) ==
> >> +         d.getVar("ISAR_RELEASE_CMD_DEFAULT", 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"], 
> >> universal_newlines=True)
> >> +                ' in the build_id. Consider changing
> >> ISAR_RELEASE_CMD.')
> >> +    cmd = d.getVar("ISAR_RELEASE_CMD", True)
> >> +    try:
> >> +        v = subprocess.check_output(cmd, shell=True, 
> >> universal_newlines=True)
> >>           return v.rstrip()
> >> -    return ""
> >> +    except subprocess.CalledProcessError as e:
> >> +        bb.warn('\"%s\" returned %d, returning empty build_id' %
> >> +                (e.cmd, e.returncode))
> >> +        return ""
> >>   python set_image_size () {
> >>       rootfs_size = get_rootfs_size(d)
> >>  
> > 
> >   
> 
>