[poky,master,PATCHv2] buildhistory.bbclass: Enable exporting more recipe and package data

Message ID 20220209092947.25677-1-sanakazisk19@gmail.com
State New
Headers show
Series [poky,master,PATCHv2] buildhistory.bbclass: Enable exporting more recipe and package data | expand

Commit Message

Sana Kazi Feb. 9, 2022, 9:29 a.m. UTC
Used BUILDHISTORY_EXPORT_RECIPE_VARIABLES and
BUILDHISTORY_EXPORT_PACKAGE_VARIABLES to export recipe and package
data to the latest file of buildhistory and sorted it alphabetically.

This makes extending data in buildhistory git tree simple and avoids
patches to it for users who care about things like SRC_URI and like
to track it in buildhistory git tree.

Now we can add additional information as per our requirement to the
buildhistory like LICENSE, SRC_URI AND MAINTAINER to the buildhistory
by appending them in a recipe or distro specific conf file as follows:

BUILDHISTORY_EXPORT_RECIPE_VARIABLES += "MAINTAINER"
BUILDHISTORY_EXPORT_PACKAGE_VARIABLES += "MAINTAINER"

Signed-off-by: Sana Kazi <Sana.Kazi@kpit.com>
Signed-off-by: Sana Kazi <sanakazisk19@gmail.com>
---
 meta/classes/buildhistory.bbclass | 111 ++++++++++++++++++++----------
 1 file changed, 73 insertions(+), 38 deletions(-)

Comments

Richard Purdie Feb. 9, 2022, 12:23 p.m. UTC | #1
On Wed, 2022-02-09 at 14:59 +0530, sana kazi wrote:
> Used BUILDHISTORY_EXPORT_RECIPE_VARIABLES and
> BUILDHISTORY_EXPORT_PACKAGE_VARIABLES to export recipe and package
> data to the latest file of buildhistory and sorted it alphabetically.
> 
> This makes extending data in buildhistory git tree simple and avoids
> patches to it for users who care about things like SRC_URI and like
> to track it in buildhistory git tree.
> 
> Now we can add additional information as per our requirement to the
> buildhistory like LICENSE, SRC_URI AND MAINTAINER to the buildhistory
> by appending them in a recipe or distro specific conf file as follows:
> 
> BUILDHISTORY_EXPORT_RECIPE_VARIABLES += "MAINTAINER"
> BUILDHISTORY_EXPORT_PACKAGE_VARIABLES += "MAINTAINER"
> 
> Signed-off-by: Sana Kazi <Sana.Kazi@kpit.com>
> Signed-off-by: Sana Kazi <sanakazisk19@gmail.com>
> ---
>  meta/classes/buildhistory.bbclass | 111 ++++++++++++++++++++----------
>  1 file changed, 73 insertions(+), 38 deletions(-)
> 
> diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
> index daa96f3b63..377b325518 100644
> --- a/meta/classes/buildhistory.bbclass
> +++ b/meta/classes/buildhistory.bbclass
> @@ -69,6 +69,11 @@ BUILDHISTORY_PRESERVE = "latest latest_srcrev sysroot"
>  PATCH_GIT_USER_EMAIL ?= "buildhistory@oe"
>  PATCH_GIT_USER_NAME ?= "OpenEmbedded"
>  
> +# Set BUILDHISTORY_EXPORT_RECIPE_VARIABLES and BUILDHISTORY_EXPORT_PACKAGE_VARIABLES
> +# to export recipe and package data to the latest file of buildhistory
> +BUILDHISTORY_EXPORT_RECIPE_VARIABLES ?= "PR PV PE LAYER DEPENDS PACKAGES SRC_URI LICENSE CONFIG"
> +BUILDHISTORY_EXPORT_PACKAGE_VARIABLES ?= "PE PV PR PKG PKGE PKGV PKGR RPROVIDES RDEPENDS RRECOMMENDS RSUGGESTS RREPLACES RCONFLICTS PKGSIZE FILES FILELIST"
> +
>  #
>  # Write out the contents of the sysroot
>  #
> @@ -264,12 +269,11 @@ python buildhistory_emit_pkghistory() {
>      rcpinfo.pe = pe
>      rcpinfo.pv = pv
>      rcpinfo.pr = pr
> -    rcpinfo.depends = sortlist(oe.utils.squashspaces(d.getVar('DEPENDS') or ""))
>      rcpinfo.packages = packages
>      rcpinfo.layer = layer
> -    rcpinfo.license = license
>      rcpinfo.config = sortlist(oe.utils.squashspaces(d.getVar('PACKAGECONFIG') or ""))
> -    rcpinfo.src_uri = oe.utils.squashspaces(d.getVar('SRC_URI') or "")
> +    export_recipe_variables = d.getVar('BUILDHISTORY_EXPORT_RECIPE_VARIABLES') or ''
> +    rcpinfo.export_recipe_variables = export_recipe_variables
>      write_recipehistory(rcpinfo, d)
>  
>      bb.build.exec_func("read_subpackage_metadata", d)
> @@ -323,6 +327,9 @@ python buildhistory_emit_pkghistory() {
>  
>          pkginfo.size = int(localdata.getVar('PKGSIZE') or '0')
>  
> +        export_package_variables = d.getVar('BUILDHISTORY_EXPORT_PACKAGE_VARIABLES') or ''
> +        pkginfo.export_package_variables = export_package_variables
> +
>          write_pkghistory(pkginfo, d)
>  
>      oe.qa.exit_if_errors(d)
> @@ -370,17 +377,22 @@ def write_recipehistory(rcpinfo, d):
>      pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
>  
>      infofile = os.path.join(pkghistdir, "latest")
> +    export_recipe_variables = set(rcpinfo.export_recipe_variables.split())
> +    ret = []
>      with open(infofile, "w") as f:
> -        if rcpinfo.pe != "0":
> -            f.write(u"PE = %s\n" %  rcpinfo.pe)
> -        f.write(u"PV = %s\n" %  rcpinfo.pv)
> -        f.write(u"PR = %s\n" %  rcpinfo.pr)
> -        f.write(u"DEPENDS = %s\n" %  rcpinfo.depends)
> -        f.write(u"PACKAGES = %s\n" %  rcpinfo.packages)
> -        f.write(u"LAYER = %s\n" %  rcpinfo.layer)
> -        f.write(u"LICENSE = %s\n" %  rcpinfo.license)
> -        f.write(u"CONFIG = %s\n" %  rcpinfo.config)
> -        f.write(u"SRC_URI = %s\n" %  rcpinfo.src_uri)
> +        for var in export_recipe_variables:
> +            if var == "PE":
> +                if rcpinfo.pe != "0":
> +                    ret.append("%s = %s" % (var, rcpinfo.pe))
> +            elif var == "LAYER":
> +                ret.append("%s = %s" % (var, rcpinfo.layer))
> +            elif var == "CONFIG":
> +                ret.append("%s = %s" % (var, rcpinfo.config))
> +            else:
> +                ret.append("%s = %s" % (var," ".join((str(d.getVar(var)).split()))))
> +        ret.sort()
> +        for element in ret:
> +            f.write(element + "\n")
>  
>      write_latest_srcrev(d, pkghistdir)
>  
> @@ -394,32 +406,55 @@ def write_pkghistory(pkginfo, d):
>          bb.utils.mkdirhier(pkgpath)
>  
>      infofile = os.path.join(pkgpath, "latest")
> +    export_package_variables = set(pkginfo.export_package_variables.split())
> +    ret = []
>      with open(infofile, "w") as f:
> -        if pkginfo.pe != "0":
> -            f.write(u"PE = %s\n" %  pkginfo.pe)
> -        f.write(u"PV = %s\n" %  pkginfo.pv)
> -        f.write(u"PR = %s\n" %  pkginfo.pr)
> -
> -        if pkginfo.pkg != pkginfo.name:
> -            f.write(u"PKG = %s\n" % pkginfo.pkg)
> -        if pkginfo.pkge != pkginfo.pe:
> -            f.write(u"PKGE = %s\n" % pkginfo.pkge)
> -        if pkginfo.pkgv != pkginfo.pv:
> -            f.write(u"PKGV = %s\n" % pkginfo.pkgv)
> -        if pkginfo.pkgr != pkginfo.pr:
> -            f.write(u"PKGR = %s\n" % pkginfo.pkgr)
> -        f.write(u"RPROVIDES = %s\n" %  pkginfo.rprovides)
> -        f.write(u"RDEPENDS = %s\n" %  pkginfo.rdepends)
> -        f.write(u"RRECOMMENDS = %s\n" %  pkginfo.rrecommends)
> -        if pkginfo.rsuggests:
> -            f.write(u"RSUGGESTS = %s\n" %  pkginfo.rsuggests)
> -        if pkginfo.rreplaces:
> -            f.write(u"RREPLACES = %s\n" %  pkginfo.rreplaces)
> -        if pkginfo.rconflicts:
> -            f.write(u"RCONFLICTS = %s\n" %  pkginfo.rconflicts)
> -        f.write(u"PKGSIZE = %d\n" %  pkginfo.size)
> -        f.write(u"FILES = %s\n" %  pkginfo.files)
> -        f.write(u"FILELIST = %s\n" %  pkginfo.filelist)
> +        for var in export_package_variables:
> +            if var == "PE":
> +                if pkginfo.pe != "0":
> +                    ret.append("%s = %s" % (var, pkginfo.pe))
> +            elif var == "PV":
> +                ret.append("%s = %s" % (var, pkginfo.pv))
> +            elif var == "PR":
> +                ret.append("%s = %s" % (var, pkginfo.pr))
> +            elif var == "RPROVIDES":
> +                ret.append("%s = %s" % (var, pkginfo.rprovides))
> +            elif var == "RDEPENDS":
> +                ret.append("%s = %s" % (var, pkginfo.rdepends))
> +            elif var == "RRECOMMENDS":
> +                ret.append("%s = %s" % (var, pkginfo.rrecommends))
> +            elif var == "PKGSIZE":
> +                ret.append("%s = %s" % (var, pkginfo.size))
> +            elif var == "FILES":
> +                ret.append("%s = %s" % (var, pkginfo.files))
> +            elif var == "FILELIST":
> +                ret.append("%s = %s" % (var, pkginfo.filelist))
> +            elif var == "RSUGGESTS":
> +                if pkginfo.rsuggests:
> +                    ret.append(u"RSUGGESTS = %s" %  pkginfo.rsuggests)
> +            elif var == "RREPLACES":
> +                if pkginfo.rreplaces:
> +                    ret.append(u"RREPLACES = %s" %  pkginfo.rreplaces)
> +            elif var == "RCONFLICTS":
> +                if pkginfo.rconflicts:
> +                    ret.append(u"RCONFLICTS = %s" %  pkginfo.rconflicts)
> +            elif var == "PKG":
> +                if pkginfo.pkg != pkginfo.name:
> +                    ret.append(u"PKG = %s" % pkginfo.pkg)
> +            elif var == "PKGE":
> +                if pkginfo.pkge != pkginfo.pe :
> +                    ret.append(u"PKGE = %s" % pkginfo.pkge)
> +            elif var == "PKGV":
> +                if pkginfo.pkgv != pkginfo.pv:
> +                    ret.append(u"PKGV = %s" % pkginfo.pkgv)
> +            elif var == "PKGR":
> +                if pkginfo.pkgr != pkginfo.pr:
> +                    ret.append(u"PKGR = %s" % pkginfo.pkgr)
> +            else:
> +                ret.append("%s = %s" % (var, d.getVar(var)))
> +        ret.sort()
> +        for element in ret:
> +            f.write(element + "\n")


People have requested changes like this before and I rejected it as I'm worried
that allowing people to customise this code will just fork the project into many
different directions. 

The elif block above illustrates part of my concern since it still isn't scaling
well and just potentially makes every buildhistory output difference (even with
different ordering). There are reasons the data is taken from pkginfo rather
than getVar too and I think this could introduce subtle bugs in the future.

Which variables are you actually interested in when changing this?

Cheers,

Richard
Sana Kazi Feb. 9, 2022, 1:03 p.m. UTC | #2
Hi Richard,

I need all the variables but am also interested in additional information
like CVE_PRODUCT or MAINTAINER. Prepared these changes so that any project
specific additional information like that can be added by appending them in
a recipe or distro specific conf.

Regards,
Sana Kazi

On Wed, 9 Feb 2022 at 17:53, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Wed, 2022-02-09 at 14:59 +0530, sana kazi wrote:
> > Used BUILDHISTORY_EXPORT_RECIPE_VARIABLES and
> > BUILDHISTORY_EXPORT_PACKAGE_VARIABLES to export recipe and package
> > data to the latest file of buildhistory and sorted it alphabetically.
> >
> > This makes extending data in buildhistory git tree simple and avoids
> > patches to it for users who care about things like SRC_URI and like
> > to track it in buildhistory git tree.
> >
> > Now we can add additional information as per our requirement to the
> > buildhistory like LICENSE, SRC_URI AND MAINTAINER to the buildhistory
> > by appending them in a recipe or distro specific conf file as follows:
> >
> > BUILDHISTORY_EXPORT_RECIPE_VARIABLES += "MAINTAINER"
> > BUILDHISTORY_EXPORT_PACKAGE_VARIABLES += "MAINTAINER"
> >
> > Signed-off-by: Sana Kazi <Sana.Kazi@kpit.com>
> > Signed-off-by: Sana Kazi <sanakazisk19@gmail.com>
> > ---
> >  meta/classes/buildhistory.bbclass | 111 ++++++++++++++++++++----------
> >  1 file changed, 73 insertions(+), 38 deletions(-)
> >
> > diff --git a/meta/classes/buildhistory.bbclass
> b/meta/classes/buildhistory.bbclass
> > index daa96f3b63..377b325518 100644
> > --- a/meta/classes/buildhistory.bbclass
> > +++ b/meta/classes/buildhistory.bbclass
> > @@ -69,6 +69,11 @@ BUILDHISTORY_PRESERVE = "latest latest_srcrev sysroot"
> >  PATCH_GIT_USER_EMAIL ?= "buildhistory@oe"
> >  PATCH_GIT_USER_NAME ?= "OpenEmbedded"
> >
> > +# Set BUILDHISTORY_EXPORT_RECIPE_VARIABLES and
> BUILDHISTORY_EXPORT_PACKAGE_VARIABLES
> > +# to export recipe and package data to the latest file of buildhistory
> > +BUILDHISTORY_EXPORT_RECIPE_VARIABLES ?= "PR PV PE LAYER DEPENDS
> PACKAGES SRC_URI LICENSE CONFIG"
> > +BUILDHISTORY_EXPORT_PACKAGE_VARIABLES ?= "PE PV PR PKG PKGE PKGV PKGR
> RPROVIDES RDEPENDS RRECOMMENDS RSUGGESTS RREPLACES RCONFLICTS PKGSIZE FILES
> FILELIST"
> > +
> >  #
> >  # Write out the contents of the sysroot
> >  #
> > @@ -264,12 +269,11 @@ python buildhistory_emit_pkghistory() {
> >      rcpinfo.pe = pe
> >      rcpinfo.pv = pv
> >      rcpinfo.pr = pr
> > -    rcpinfo.depends =
> sortlist(oe.utils.squashspaces(d.getVar('DEPENDS') or ""))
> >      rcpinfo.packages = packages
> >      rcpinfo.layer = layer
> > -    rcpinfo.license = license
> >      rcpinfo.config =
> sortlist(oe.utils.squashspaces(d.getVar('PACKAGECONFIG') or ""))
> > -    rcpinfo.src_uri = oe.utils.squashspaces(d.getVar('SRC_URI') or "")
> > +    export_recipe_variables =
> d.getVar('BUILDHISTORY_EXPORT_RECIPE_VARIABLES') or ''
> > +    rcpinfo.export_recipe_variables = export_recipe_variables
> >      write_recipehistory(rcpinfo, d)
> >
> >      bb.build.exec_func("read_subpackage_metadata", d)
> > @@ -323,6 +327,9 @@ python buildhistory_emit_pkghistory() {
> >
> >          pkginfo.size = int(localdata.getVar('PKGSIZE') or '0')
> >
> > +        export_package_variables =
> d.getVar('BUILDHISTORY_EXPORT_PACKAGE_VARIABLES') or ''
> > +        pkginfo.export_package_variables = export_package_variables
> > +
> >          write_pkghistory(pkginfo, d)
> >
> >      oe.qa.exit_if_errors(d)
> > @@ -370,17 +377,22 @@ def write_recipehistory(rcpinfo, d):
> >      pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
> >
> >      infofile = os.path.join(pkghistdir, "latest")
> > +    export_recipe_variables =
> set(rcpinfo.export_recipe_variables.split())
> > +    ret = []
> >      with open(infofile, "w") as f:
> > -        if rcpinfo.pe != "0":
> > -            f.write(u"PE = %s\n" %  rcpinfo.pe)
> > -        f.write(u"PV = %s\n" %  rcpinfo.pv)
> > -        f.write(u"PR = %s\n" %  rcpinfo.pr)
> > -        f.write(u"DEPENDS = %s\n" %  rcpinfo.depends)
> > -        f.write(u"PACKAGES = %s\n" %  rcpinfo.packages)
> > -        f.write(u"LAYER = %s\n" %  rcpinfo.layer)
> > -        f.write(u"LICENSE = %s\n" %  rcpinfo.license)
> > -        f.write(u"CONFIG = %s\n" %  rcpinfo.config)
> > -        f.write(u"SRC_URI = %s\n" %  rcpinfo.src_uri)
> > +        for var in export_recipe_variables:
> > +            if var == "PE":
> > +                if rcpinfo.pe != "0":
> > +                    ret.append("%s = %s" % (var, rcpinfo.pe))
> > +            elif var == "LAYER":
> > +                ret.append("%s = %s" % (var, rcpinfo.layer))
> > +            elif var == "CONFIG":
> > +                ret.append("%s = %s" % (var, rcpinfo.config))
> > +            else:
> > +                ret.append("%s = %s" % (var,"
> ".join((str(d.getVar(var)).split()))))
> > +        ret.sort()
> > +        for element in ret:
> > +            f.write(element + "\n")
> >
> >      write_latest_srcrev(d, pkghistdir)
> >
> > @@ -394,32 +406,55 @@ def write_pkghistory(pkginfo, d):
> >          bb.utils.mkdirhier(pkgpath)
> >
> >      infofile = os.path.join(pkgpath, "latest")
> > +    export_package_variables =
> set(pkginfo.export_package_variables.split())
> > +    ret = []
> >      with open(infofile, "w") as f:
> > -        if pkginfo.pe != "0":
> > -            f.write(u"PE = %s\n" %  pkginfo.pe)
> > -        f.write(u"PV = %s\n" %  pkginfo.pv)
> > -        f.write(u"PR = %s\n" %  pkginfo.pr)
> > -
> > -        if pkginfo.pkg != pkginfo.name:
> > -            f.write(u"PKG = %s\n" % pkginfo.pkg)
> > -        if pkginfo.pkge != pkginfo.pe:
> > -            f.write(u"PKGE = %s\n" % pkginfo.pkge)
> > -        if pkginfo.pkgv != pkginfo.pv:
> > -            f.write(u"PKGV = %s\n" % pkginfo.pkgv)
> > -        if pkginfo.pkgr != pkginfo.pr:
> > -            f.write(u"PKGR = %s\n" % pkginfo.pkgr)
> > -        f.write(u"RPROVIDES = %s\n" %  pkginfo.rprovides)
> > -        f.write(u"RDEPENDS = %s\n" %  pkginfo.rdepends)
> > -        f.write(u"RRECOMMENDS = %s\n" %  pkginfo.rrecommends)
> > -        if pkginfo.rsuggests:
> > -            f.write(u"RSUGGESTS = %s\n" %  pkginfo.rsuggests)
> > -        if pkginfo.rreplaces:
> > -            f.write(u"RREPLACES = %s\n" %  pkginfo.rreplaces)
> > -        if pkginfo.rconflicts:
> > -            f.write(u"RCONFLICTS = %s\n" %  pkginfo.rconflicts)
> > -        f.write(u"PKGSIZE = %d\n" %  pkginfo.size)
> > -        f.write(u"FILES = %s\n" %  pkginfo.files)
> > -        f.write(u"FILELIST = %s\n" %  pkginfo.filelist)
> > +        for var in export_package_variables:
> > +            if var == "PE":
> > +                if pkginfo.pe != "0":
> > +                    ret.append("%s = %s" % (var, pkginfo.pe))
> > +            elif var == "PV":
> > +                ret.append("%s = %s" % (var, pkginfo.pv))
> > +            elif var == "PR":
> > +                ret.append("%s = %s" % (var, pkginfo.pr))
> > +            elif var == "RPROVIDES":
> > +                ret.append("%s = %s" % (var, pkginfo.rprovides))
> > +            elif var == "RDEPENDS":
> > +                ret.append("%s = %s" % (var, pkginfo.rdepends))
> > +            elif var == "RRECOMMENDS":
> > +                ret.append("%s = %s" % (var, pkginfo.rrecommends))
> > +            elif var == "PKGSIZE":
> > +                ret.append("%s = %s" % (var, pkginfo.size))
> > +            elif var == "FILES":
> > +                ret.append("%s = %s" % (var, pkginfo.files))
> > +            elif var == "FILELIST":
> > +                ret.append("%s = %s" % (var, pkginfo.filelist))
> > +            elif var == "RSUGGESTS":
> > +                if pkginfo.rsuggests:
> > +                    ret.append(u"RSUGGESTS = %s" %  pkginfo.rsuggests)
> > +            elif var == "RREPLACES":
> > +                if pkginfo.rreplaces:
> > +                    ret.append(u"RREPLACES = %s" %  pkginfo.rreplaces)
> > +            elif var == "RCONFLICTS":
> > +                if pkginfo.rconflicts:
> > +                    ret.append(u"RCONFLICTS = %s" %  pkginfo.rconflicts)
> > +            elif var == "PKG":
> > +                if pkginfo.pkg != pkginfo.name:
> > +                    ret.append(u"PKG = %s" % pkginfo.pkg)
> > +            elif var == "PKGE":
> > +                if pkginfo.pkge != pkginfo.pe :
> > +                    ret.append(u"PKGE = %s" % pkginfo.pkge)
> > +            elif var == "PKGV":
> > +                if pkginfo.pkgv != pkginfo.pv:
> > +                    ret.append(u"PKGV = %s" % pkginfo.pkgv)
> > +            elif var == "PKGR":
> > +                if pkginfo.pkgr != pkginfo.pr:
> > +                    ret.append(u"PKGR = %s" % pkginfo.pkgr)
> > +            else:
> > +                ret.append("%s = %s" % (var, d.getVar(var)))
> > +        ret.sort()
> > +        for element in ret:
> > +            f.write(element + "\n")
>
>
> People have requested changes like this before and I rejected it as I'm
> worried
> that allowing people to customise this code will just fork the project
> into many
> different directions.
>
> The elif block above illustrates part of my concern since it still isn't
> scaling
> well and just potentially makes every buildhistory output difference (even
> with
> different ordering). There are reasons the data is taken from pkginfo
> rather
> than getVar too and I think this could introduce subtle bugs in the future.
>
> Which variables are you actually interested in when changing this?
>
> Cheers,
>
> Richard
>
>
>
>
>
>
>
>
Mikko Rapeli Feb. 9, 2022, 1:27 p.m. UTC | #3
Hi,

On Wed, Feb 09, 2022 at 12:23:39PM +0000, Richard Purdie wrote:
> People have requested changes like this before and I rejected it as I'm worried
> that allowing people to customise this code will just fork the project into many
> different directions. 

It's the other way round. There are a lot of needs to extract metadata from
build system into something where reports can be generated. I'd prefer this to
be buildhistory as it also tracks progress across releases and versions and custom
builds if the setup has been done correctly. If this change isn't going to be accepted,
projects need to continue with their custom patches on top of current buildhistory
bbclass in poky.

> The elif block above illustrates part of my concern since it still isn't scaling
> well and just potentially makes every buildhistory output difference (even with
> different ordering). There are reasons the data is taken from pkginfo rather
> than getVar too and I think this could introduce subtle bugs in the future.

The elif block is just for backwards compatibility. Changing the order of fields would
be annoying in buildhistory. The new ones will be in alphabetical order so I don't
think this is so bad. It is possibly to completely rewrite and simplify buildhistory
but this would be annoying for users who actually track the changes.

> Which variables are you actually interested in when changing this?

LICENSE, CVE_PRODUCT, SRC_URI, MAINTAINER and some project specific variables too.
It is very useful to have these tracked in buildhistory git repo where differences
between releases can easily be seen.

Cheers,

-Mikko
Richard Purdie Feb. 9, 2022, 1:40 p.m. UTC | #4
On Wed, 2022-02-09 at 13:27 +0000, Mikko.Rapeli@bmw.de wrote:
> Hi,
> 
> On Wed, Feb 09, 2022 at 12:23:39PM +0000, Richard Purdie wrote:
> > People have requested changes like this before and I rejected it as I'm worried
> > that allowing people to customise this code will just fork the project into many
> > different directions. 
> 
> It's the other way round. There are a lot of needs to extract metadata from
> build system into something where reports can be generated.

I don't doubt that however buildhistory was written for a specific purpose and
if we start adding the ability to customise it heavily we lose the ability for
comparisions to be made, or sstate reuse and so on.

I'm partly channelling the original author's views on this since they had some
very specific thoughts on this change. I do sometimes wonder if I should
continue doing that though :/.

>  I'd prefer this to be buildhistory as it also tracks progress across releases and 
> versions and custom builds if the setup has been done correctly. If this change isn't 
> going to be accepted, projects need to continue with their custom patches on top of 
> current buildhistory bbclass in poky.
> 
> > The elif block above illustrates part of my concern since it still isn't scaling
> > well and just potentially makes every buildhistory output difference (even with
> > different ordering). There are reasons the data is taken from pkginfo rather
> > than getVar too and I think this could introduce subtle bugs in the future.
> 
> The elif block is just for backwards compatibility. Changing the order of fields would
> be annoying in buildhistory. The new ones will be in alphabetical order so I don't
> think this is so bad. It is possibly to completely rewrite and simplify buildhistory
> but this would be annoying for users who actually track the changes.
>
> > Which variables are you actually interested in when changing this?
> 
> LICENSE, CVE_PRODUCT, SRC_URI, MAINTAINER and some project specific variables too.
> It is very useful to have these tracked in buildhistory git repo where differences
> between releases can easily be seen.

The trouble is this turns it into a "metadata difference" tool rather than a
build output difference tool which is different to the design. It also makes it
much harder to write the buildhistory analysis tool since all of a sudden we're
dealing with any variable rather than package specific ones and we can no longer
assume any particular output would be present (and it is no longer just based
upon packagedata).

I guess we can see what others think, but I am concerned about letting people
remove existing output from it too.

Cheers,

Richard
Mikko Rapeli Feb. 9, 2022, 2:27 p.m. UTC | #5
On Wed, Feb 09, 2022 at 01:40:22PM +0000, Richard Purdie wrote:
> On Wed, 2022-02-09 at 13:27 +0000, Mikko.Rapeli@bmw.de wrote:
> > Hi,
> > 
> > On Wed, Feb 09, 2022 at 12:23:39PM +0000, Richard Purdie wrote:
> > > People have requested changes like this before and I rejected it as I'm worried
> > > that allowing people to customise this code will just fork the project into many
> > > different directions. 
> > 
> > It's the other way round. There are a lot of needs to extract metadata from
> > build system into something where reports can be generated.
> 
> I don't doubt that however buildhistory was written for a specific purpose and
> if we start adding the ability to customise it heavily we lose the ability for
> comparisions to be made, or sstate reuse and so on.
> 
> I'm partly channelling the original author's views on this since they had some
> very specific thoughts on this change. I do sometimes wonder if I should
> continue doing that though :/.

Then how should yocto users export CVE_NAME, LICENSE, PN, PV, SRC_URI etc from
the build system to generate SW bill of materials (BOM) for their product
and track progress?

Yes, SPDX can be the other answer but I don't find that human readable or working
out of the box atm.

> >  I'd prefer this to be buildhistory as it also tracks progress across releases and 
> > versions and custom builds if the setup has been done correctly. If this change isn't 
> > going to be accepted, projects need to continue with their custom patches on top of 
> > current buildhistory bbclass in poky.
> > 
> > > The elif block above illustrates part of my concern since it still isn't scaling
> > > well and just potentially makes every buildhistory output difference (even with
> > > different ordering). There are reasons the data is taken from pkginfo rather
> > > than getVar too and I think this could introduce subtle bugs in the future.
> > 
> > The elif block is just for backwards compatibility. Changing the order of fields would
> > be annoying in buildhistory. The new ones will be in alphabetical order so I don't
> > think this is so bad. It is possibly to completely rewrite and simplify buildhistory
> > but this would be annoying for users who actually track the changes.
> >
> > > Which variables are you actually interested in when changing this?
> > 
> > LICENSE, CVE_PRODUCT, SRC_URI, MAINTAINER and some project specific variables too.
> > It is very useful to have these tracked in buildhistory git repo where differences
> > between releases can easily be seen.
> 
> The trouble is this turns it into a "metadata difference" tool rather than a
> build output difference tool which is different to the design. It also makes it
> much harder to write the buildhistory analysis tool since all of a sudden we're
> dealing with any variable rather than package specific ones and we can no longer
> assume any particular output would be present (and it is no longer just based
> upon packagedata).

What tool or feature from yocto should we use then?

> I guess we can see what others think, but I am concerned about letting people
> remove existing output from it too.

Well buildhistory can have defaults. It can be extended. I don't see your reasons
for limiting the output to a few variables which you care about.

I frequently debug issues by looking at buildhistory data about images, binary
packages and recipes. It is a very useful tool.

-Mikko
Richard Purdie Feb. 9, 2022, 3:34 p.m. UTC | #6
On Wed, 2022-02-09 at 14:27 +0000, Mikko.Rapeli@bmw.de wrote:
> On Wed, Feb 09, 2022 at 01:40:22PM +0000, Richard Purdie wrote:
> > On Wed, 2022-02-09 at 13:27 +0000, Mikko.Rapeli@bmw.de wrote:
> > > Hi,
> > > 
> > > On Wed, Feb 09, 2022 at 12:23:39PM +0000, Richard Purdie wrote:
> > > > People have requested changes like this before and I rejected it as I'm worried
> > > > that allowing people to customise this code will just fork the project into many
> > > > different directions. 
> > > 
> > > It's the other way round. There are a lot of needs to extract metadata from
> > > build system into something where reports can be generated.
> > 
> > I don't doubt that however buildhistory was written for a specific purpose and
> > if we start adding the ability to customise it heavily we lose the ability for
> > comparisions to be made, or sstate reuse and so on.
> > 
> > I'm partly channelling the original author's views on this since they had some
> > very specific thoughts on this change. I do sometimes wonder if I should
> > continue doing that though :/.
> 
> Then how should yocto users export CVE_NAME, LICENSE, PN, PV, SRC_URI etc from
> the build system to generate SW bill of materials (BOM) for their product
> and track progress?
> 
> Yes, SPDX can be the other answer but I don't find that human readable or working
> out of the box atm.

buildhistory was not intended for SBOM generation, that is what create-spdx is
being developed for. They have two quite different intentions and trying to turn
one into the other is why I have concerns about this patch.

For example, of we did go this way, next, we may need to either write a
converter of buildhistory to SPDX format, or change buildhistory to use SPDX
format so that it has a standard SBOM output form. This is not the direction we
want/need to go.

Cheers,

Richard
Paul Eggleton Feb. 11, 2022, 3:49 a.m. UTC | #7
On Thursday, 10 February 2022 04:34:16 NZDT Richard Purdie wrote:
> On Wed, 2022-02-09 at 14:27 +0000, Mikko.Rapeli@bmw.de wrote:
> > On Wed, Feb 09, 2022 at 01:40:22PM +0000, Richard Purdie wrote:
> > > On Wed, 2022-02-09 at 13:27 +0000, Mikko.Rapeli@bmw.de wrote:
> > > > Hi,
> > > > 
> > > > On Wed, Feb 09, 2022 at 12:23:39PM +0000, Richard Purdie wrote:
> > > > > People have requested changes like this before and I rejected it as
> > > > > I'm worried that allowing people to customise this code will just
> > > > > fork the project into many different directions.
> > > > 
> > > > It's the other way round. There are a lot of needs to extract metadata
> > > > from
> > > > build system into something where reports can be generated.
> > > 
> > > I don't doubt that however buildhistory was written for a specific
> > > purpose and if we start adding the ability to customise it heavily we
> > > lose the ability for comparisions to be made, or sstate reuse and so
> > > on.
> > > 
> > > I'm partly channelling the original author's views on this since they
> > > had some very specific thoughts on this change. I do sometimes wonder
> > > if I should continue doing that though :/.
> > 
> > Then how should yocto users export CVE_NAME, LICENSE, PN, PV, SRC_URI etc
> > from the build system to generate SW bill of materials (BOM) for their
> > product and track progress?
> > 
> > Yes, SPDX can be the other answer but I don't find that human readable or
> > working out of the box atm.
> 
> buildhistory was not intended for SBOM generation, that is what create-spdx
> is being developed for. They have two quite different intentions and trying
> to turn one into the other is why I have concerns about this patch.
> 
> For example, of we did go this way, next, we may need to either write a
> converter of buildhistory to SPDX format, or change buildhistory to use SPDX
> format so that it has a standard SBOM output form. This is not the
> direction we want/need to go.

FWIW I'll just chime in here as the original author[1] and say I agree with 
Richard. If folks are needing an alternative SBOM generation mechanism to 
SPDX, or have other use cases for extracting build information, then I'd 
prefer we take a step back and design such a thing properly. The original idea 
was (1) to expose changes in the output that might not readily apparent 
otherwise, and (2) expose selected "input" values that would (or could) 
materially influence the output, so you hopefully have a ready explanation for 
why an image / package increased in size or dependencies got added etc. 

I will accept that over time buildhistory has added things here and there that 
further the idea of using it for SBOM, folks no doubt have been using these 
for such purposes, and I'm not 100% opposed to making small additions where 
they make sense. However, even with this proposed extension, buildhistory is 
still not capable of recording sufficient information that (I believe) is 
expected in a modern SBOM - for example, the list of patches applied / CVEs 
resolved as a result - and I don't think it would be wise to extend it to do 
so. We certainly don't want to give folks the idea that buildhistory is good 
enough to resolve their SBOM requirements when we haven't designed it to do 
so, particularly as for many folks there are legal and regulatory concerns 
involved.

Cheers
Paul

[1] based upon the earlier testlab and packagehistory classes that were 
written by others
Mikko Rapeli Feb. 11, 2022, 8 a.m. UTC | #8
Hi,

On Fri, Feb 11, 2022 at 04:49:21PM +1300, Paul Eggleton wrote:
> FWIW I'll just chime in here as the original author[1] and say I agree with 
> Richard. If folks are needing an alternative SBOM generation mechanism to 
> SPDX, or have other use cases for extracting build information, then I'd 
> prefer we take a step back and design such a thing properly. The original idea 
> was (1) to expose changes in the output that might not readily apparent 
> otherwise, and (2) expose selected "input" values that would (or could) 
> materially influence the output, so you hopefully have a ready explanation for 
> why an image / package increased in size or dependencies got added etc. 

First of all, thanks for buildhistory, Paul!

> I will accept that over time buildhistory has added things here and there that 
> further the idea of using it for SBOM, folks no doubt have been using these 
> for such purposes, and I'm not 100% opposed to making small additions where 
> they make sense. However, even with this proposed extension, buildhistory is 
> still not capable of recording sufficient information that (I believe) is 
> expected in a modern SBOM - for example, the list of patches applied / CVEs 
> resolved as a result - and I don't think it would be wise to extend it to do 
> so. We certainly don't want to give folks the idea that buildhistory is good 
> enough to resolve their SBOM requirements when we haven't designed it to do 
> so, particularly as for many folks there are legal and regulatory concerns 
> involved.

Maybe I'm "abusing" buildhistory then. The way it enables to have a git repo
view with full history of releases on metadata available in SW builds is
really invaluable. We seem to be abusing it to export various bitbake recipe
variables from product configuration so that we can later on do some QA checks
on them and record the details in nice git history view.

There could be other ways to implement the same use cases. We could have real
QA checks in the bitbake builds which make sure certain recipe variables
are correctly filled. We could export them into image manifest files or even SDPX
to record the details per release. Then we could manually diff the files between
releases to check for delta. Or a custom data format, maybe in json.

Yocto CVE checker uses a custom text format, a bit similar to buildhistory as output.
There is the json patch from Microsoft in review.

Currently I don't see SPDX output replacing buildhistory and CVE checker output completely.

Then there is the current discussion about ABI dumps and how to store and export them
and buildhistory was at least initially mentioned there.

Gah, now I thought about saving SPDX, CVE checker and ABI dump output in buildhistory too.
Would actually be nice. Sorry, yet another use case where easy access build metadata
to compare over time and releases would be really handy. This never ends.
You've created a monster :)

-Mikko
Jacob Kroon Feb. 11, 2022, 8:04 a.m. UTC | #9
On 2/11/22 09:00, Mikko Rapeli wrote:
> Hi,
> 
> On Fri, Feb 11, 2022 at 04:49:21PM +1300, Paul Eggleton wrote:
>> FWIW I'll just chime in here as the original author[1] and say I agree with 
>> Richard. If folks are needing an alternative SBOM generation mechanism to 
>> SPDX, or have other use cases for extracting build information, then I'd 
>> prefer we take a step back and design such a thing properly. The original idea 
>> was (1) to expose changes in the output that might not readily apparent 
>> otherwise, and (2) expose selected "input" values that would (or could) 
>> materially influence the output, so you hopefully have a ready explanation for 
>> why an image / package increased in size or dependencies got added etc. 
> 
> First of all, thanks for buildhistory, Paul!
> 
>> I will accept that over time buildhistory has added things here and there that 
>> further the idea of using it for SBOM, folks no doubt have been using these 
>> for such purposes, and I'm not 100% opposed to making small additions where 
>> they make sense. However, even with this proposed extension, buildhistory is 
>> still not capable of recording sufficient information that (I believe) is 
>> expected in a modern SBOM - for example, the list of patches applied / CVEs 
>> resolved as a result - and I don't think it would be wise to extend it to do 
>> so. We certainly don't want to give folks the idea that buildhistory is good 
>> enough to resolve their SBOM requirements when we haven't designed it to do 
>> so, particularly as for many folks there are legal and regulatory concerns 
>> involved.
> 
> Maybe I'm "abusing" buildhistory then. The way it enables to have a git repo
> view with full history of releases on metadata available in SW builds is
> really invaluable. We seem to be abusing it to export various bitbake recipe
> variables from product configuration so that we can later on do some QA checks
> on them and record the details in nice git history view.
> 
> There could be other ways to implement the same use cases. We could have real
> QA checks in the bitbake builds which make sure certain recipe variables
> are correctly filled. We could export them into image manifest files or even SDPX
> to record the details per release. Then we could manually diff the files between
> releases to check for delta. Or a custom data format, maybe in json.
> 
> Yocto CVE checker uses a custom text format, a bit similar to buildhistory as output.
> There is the json patch from Microsoft in review.
> 
> Currently I don't see SPDX output replacing buildhistory and CVE checker output completely.
> 
> Then there is the current discussion about ABI dumps and how to store and export them
> and buildhistory was at least initially mentioned there.
> 
> Gah, now I thought about saving SPDX, CVE checker and ABI dump output in buildhistory too.
> Would actually be nice. Sorry, yet another use case where easy access build metadata
> to compare over time and releases would be really handy. This never ends.
> You've created a monster :)
> 

I want to add file checksums to that wishlist.

Jacob

Patch

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index daa96f3b63..377b325518 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -69,6 +69,11 @@  BUILDHISTORY_PRESERVE = "latest latest_srcrev sysroot"
 PATCH_GIT_USER_EMAIL ?= "buildhistory@oe"
 PATCH_GIT_USER_NAME ?= "OpenEmbedded"
 
+# Set BUILDHISTORY_EXPORT_RECIPE_VARIABLES and BUILDHISTORY_EXPORT_PACKAGE_VARIABLES
+# to export recipe and package data to the latest file of buildhistory
+BUILDHISTORY_EXPORT_RECIPE_VARIABLES ?= "PR PV PE LAYER DEPENDS PACKAGES SRC_URI LICENSE CONFIG"
+BUILDHISTORY_EXPORT_PACKAGE_VARIABLES ?= "PE PV PR PKG PKGE PKGV PKGR RPROVIDES RDEPENDS RRECOMMENDS RSUGGESTS RREPLACES RCONFLICTS PKGSIZE FILES FILELIST"
+
 #
 # Write out the contents of the sysroot
 #
@@ -264,12 +269,11 @@  python buildhistory_emit_pkghistory() {
     rcpinfo.pe = pe
     rcpinfo.pv = pv
     rcpinfo.pr = pr
-    rcpinfo.depends = sortlist(oe.utils.squashspaces(d.getVar('DEPENDS') or ""))
     rcpinfo.packages = packages
     rcpinfo.layer = layer
-    rcpinfo.license = license
     rcpinfo.config = sortlist(oe.utils.squashspaces(d.getVar('PACKAGECONFIG') or ""))
-    rcpinfo.src_uri = oe.utils.squashspaces(d.getVar('SRC_URI') or "")
+    export_recipe_variables = d.getVar('BUILDHISTORY_EXPORT_RECIPE_VARIABLES') or ''
+    rcpinfo.export_recipe_variables = export_recipe_variables
     write_recipehistory(rcpinfo, d)
 
     bb.build.exec_func("read_subpackage_metadata", d)
@@ -323,6 +327,9 @@  python buildhistory_emit_pkghistory() {
 
         pkginfo.size = int(localdata.getVar('PKGSIZE') or '0')
 
+        export_package_variables = d.getVar('BUILDHISTORY_EXPORT_PACKAGE_VARIABLES') or ''
+        pkginfo.export_package_variables = export_package_variables
+
         write_pkghistory(pkginfo, d)
 
     oe.qa.exit_if_errors(d)
@@ -370,17 +377,22 @@  def write_recipehistory(rcpinfo, d):
     pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
 
     infofile = os.path.join(pkghistdir, "latest")
+    export_recipe_variables = set(rcpinfo.export_recipe_variables.split())
+    ret = []
     with open(infofile, "w") as f:
-        if rcpinfo.pe != "0":
-            f.write(u"PE = %s\n" %  rcpinfo.pe)
-        f.write(u"PV = %s\n" %  rcpinfo.pv)
-        f.write(u"PR = %s\n" %  rcpinfo.pr)
-        f.write(u"DEPENDS = %s\n" %  rcpinfo.depends)
-        f.write(u"PACKAGES = %s\n" %  rcpinfo.packages)
-        f.write(u"LAYER = %s\n" %  rcpinfo.layer)
-        f.write(u"LICENSE = %s\n" %  rcpinfo.license)
-        f.write(u"CONFIG = %s\n" %  rcpinfo.config)
-        f.write(u"SRC_URI = %s\n" %  rcpinfo.src_uri)
+        for var in export_recipe_variables:
+            if var == "PE":
+                if rcpinfo.pe != "0":
+                    ret.append("%s = %s" % (var, rcpinfo.pe))
+            elif var == "LAYER":
+                ret.append("%s = %s" % (var, rcpinfo.layer))
+            elif var == "CONFIG":
+                ret.append("%s = %s" % (var, rcpinfo.config))
+            else:
+                ret.append("%s = %s" % (var," ".join((str(d.getVar(var)).split()))))
+        ret.sort()
+        for element in ret:
+            f.write(element + "\n")
 
     write_latest_srcrev(d, pkghistdir)
 
@@ -394,32 +406,55 @@  def write_pkghistory(pkginfo, d):
         bb.utils.mkdirhier(pkgpath)
 
     infofile = os.path.join(pkgpath, "latest")
+    export_package_variables = set(pkginfo.export_package_variables.split())
+    ret = []
     with open(infofile, "w") as f:
-        if pkginfo.pe != "0":
-            f.write(u"PE = %s\n" %  pkginfo.pe)
-        f.write(u"PV = %s\n" %  pkginfo.pv)
-        f.write(u"PR = %s\n" %  pkginfo.pr)
-
-        if pkginfo.pkg != pkginfo.name:
-            f.write(u"PKG = %s\n" % pkginfo.pkg)
-        if pkginfo.pkge != pkginfo.pe:
-            f.write(u"PKGE = %s\n" % pkginfo.pkge)
-        if pkginfo.pkgv != pkginfo.pv:
-            f.write(u"PKGV = %s\n" % pkginfo.pkgv)
-        if pkginfo.pkgr != pkginfo.pr:
-            f.write(u"PKGR = %s\n" % pkginfo.pkgr)
-        f.write(u"RPROVIDES = %s\n" %  pkginfo.rprovides)
-        f.write(u"RDEPENDS = %s\n" %  pkginfo.rdepends)
-        f.write(u"RRECOMMENDS = %s\n" %  pkginfo.rrecommends)
-        if pkginfo.rsuggests:
-            f.write(u"RSUGGESTS = %s\n" %  pkginfo.rsuggests)
-        if pkginfo.rreplaces:
-            f.write(u"RREPLACES = %s\n" %  pkginfo.rreplaces)
-        if pkginfo.rconflicts:
-            f.write(u"RCONFLICTS = %s\n" %  pkginfo.rconflicts)
-        f.write(u"PKGSIZE = %d\n" %  pkginfo.size)
-        f.write(u"FILES = %s\n" %  pkginfo.files)
-        f.write(u"FILELIST = %s\n" %  pkginfo.filelist)
+        for var in export_package_variables:
+            if var == "PE":
+                if pkginfo.pe != "0":
+                    ret.append("%s = %s" % (var, pkginfo.pe))
+            elif var == "PV":
+                ret.append("%s = %s" % (var, pkginfo.pv))
+            elif var == "PR":
+                ret.append("%s = %s" % (var, pkginfo.pr))
+            elif var == "RPROVIDES":
+                ret.append("%s = %s" % (var, pkginfo.rprovides))
+            elif var == "RDEPENDS":
+                ret.append("%s = %s" % (var, pkginfo.rdepends))
+            elif var == "RRECOMMENDS":
+                ret.append("%s = %s" % (var, pkginfo.rrecommends))
+            elif var == "PKGSIZE":
+                ret.append("%s = %s" % (var, pkginfo.size))
+            elif var == "FILES":
+                ret.append("%s = %s" % (var, pkginfo.files))
+            elif var == "FILELIST":
+                ret.append("%s = %s" % (var, pkginfo.filelist))
+            elif var == "RSUGGESTS":
+                if pkginfo.rsuggests:
+                    ret.append(u"RSUGGESTS = %s" %  pkginfo.rsuggests)
+            elif var == "RREPLACES":
+                if pkginfo.rreplaces:
+                    ret.append(u"RREPLACES = %s" %  pkginfo.rreplaces)
+            elif var == "RCONFLICTS":
+                if pkginfo.rconflicts:
+                    ret.append(u"RCONFLICTS = %s" %  pkginfo.rconflicts)
+            elif var == "PKG":
+                if pkginfo.pkg != pkginfo.name:
+                    ret.append(u"PKG = %s" % pkginfo.pkg)
+            elif var == "PKGE":
+                if pkginfo.pkge != pkginfo.pe :
+                    ret.append(u"PKGE = %s" % pkginfo.pkge)
+            elif var == "PKGV":
+                if pkginfo.pkgv != pkginfo.pv:
+                    ret.append(u"PKGV = %s" % pkginfo.pkgv)
+            elif var == "PKGR":
+                if pkginfo.pkgr != pkginfo.pr:
+                    ret.append(u"PKGR = %s" % pkginfo.pkgr)
+            else:
+                ret.append("%s = %s" % (var, d.getVar(var)))
+        ret.sort()
+        for element in ret:
+            f.write(element + "\n")
 
     for filevar in pkginfo.filevars:
         filevarpath = os.path.join(pkgpath, "latest.%s" % filevar)