Re: [OE-core] [master][PATCH 2/5] hash equivalency and pr service

["Mark Hatle" <mark.hatle@kernel.crashing.org>]

On 8/25/20 7:50 AM, Joshua Watt wrote:
> On Mon, Aug 24, 2020 at 6:29 PM Mark Hatle
> <mark.hatle@kernel.crashing.org> wrote:
>>
>> When the PR service is enabled a number of small changes happen to variables.
>>
>> During the do_package task, EXTENDPRAUTO will get a value placed into it that
>> is intended to be used to extend the PR.  This value will get written to
>> various intermediate files and will result in the computed hash never being
>> equivalent.
>>
>> To resolve this issue, we create a new file with the extension ".oe_nohash".
>> This extension contains various values that need to be passed from task to
>> task, but NOT calculated within the scope of the task hash.  The items
>> placed into the nohash are captured in package.bbclass in PKGDATA_VAR_NOHASH.
>>
>> The PKGDATA_VAR_NOHASH is also used to prevent expansion of the value when
>> users are written to the various pkgdata files.  This expansion is prevented
>> by removeing the variables, resulting in the system printing the literal
>> ${VAR} when used.
>>
>> Additionally package dependencies rely on variables that also eventually
>> include EXTENDPRAUTO.  So instead of resolving the variable at getVar time,
>> we keep them as references by using the same technique as above.
>>
>> Due to this change, buildhistory needs a few minor modifications to
>> know how to deal with EXTENDPRAUTO not always being available for comparison.
>>
>> Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
>> ---
>>  meta/classes/buildhistory.bbclass | 29 ++++++++++----
>>  meta/classes/package.bbclass      | 63 ++++++++++++++++++++++++-------
>>  meta/lib/oe/packagedata.py        |  8 +++-
>>  meta/lib/oe/sstatesig.py          |  2 +
>>  4 files changed, 80 insertions(+), 22 deletions(-)
>>
>> diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
>> index 805e976ac5..2bccdfc953 100644
>> --- a/meta/classes/buildhistory.bbclass
>> +++ b/meta/classes/buildhistory.bbclass
>> @@ -99,6 +99,7 @@ python buildhistory_emit_pkghistory() {
>>      import json
>>      import shlex
>>      import errno
>> +    import oe.packagedata
>>
>>      pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
>>      oldpkghistdir = d.getVar('BUILDHISTORY_OLD_DIR_PACKAGE')
>> @@ -127,6 +128,7 @@ python buildhistory_emit_pkghistory() {
>>              self.pkge = ""
>>              self.pkgv = ""
>>              self.pkgr = ""
>> +            self.extendprauto = ""
>>              self.size = 0
>>              self.depends = ""
>>              self.rprovides = ""
>> @@ -163,6 +165,8 @@ python buildhistory_emit_pkghistory() {
>>                      pkginfo.pkgv = value
>>                  elif name == "PKGR":
>>                      pkginfo.pkgr = value
>> +                elif name == "EXTENDPRAUTO":
>> +                    pkginfo.extendprauto = value
>>                  elif name == "RPROVIDES":
>>                      pkginfo.rprovides = value
>>                  elif name == "RDEPENDS":
>> @@ -260,18 +264,24 @@ python buildhistory_emit_pkghistory() {
>>
>>      pkgdest = d.getVar('PKGDEST')
>>      for pkg in packagelist:
>> -        pkgdata = {}
>> -        with open(os.path.join(pkgdata_dir, 'runtime', pkg)) as f:
>> -            for line in f.readlines():
>> -                item = line.rstrip('\n').split(': ', 1)
>> -                key = item[0]
>> -                if key.endswith('_' + pkg):
>> -                    key = key[:-len(pkg)-1]
>> -                pkgdata[key] = item[1].encode('latin-1').decode('unicode_escape')
>> +        pkgdata = oe.packagedata.read_pkgdatafile(os.path.join(pkgdata_dir, 'runtime', pkg))
>> +
>> +        npkgdata = {}
>> +        for key in pkgdata.keys():
>> +            if key.endswith('_' + pkg):
>> +                nkey = key[:-len(pkg)-1]
>> +                npkgdata[nkey] = pkgdata[key]
>> +
>> +        for key in npkgdata.keys():
>> +          pkgdata[key] = npkgdata[key]
>>
>>          pkge = pkgdata.get('PKGE', '0')
>>          pkgv = pkgdata['PKGV']
>>          pkgr = pkgdata['PKGR']
>> +        try:
>> +            extendprauto = pkgdata['EXTENDPRAUTO']
>> +        except IndexError:
>> +            extendprauto = d.getVar('EXTENDPRAUTO')
>>          #
>>          # Find out what the last version was
>>          # Make sure the version did not decrease
>> @@ -295,6 +305,7 @@ python buildhistory_emit_pkghistory() {
>>          pkginfo.pkge = pkge
>>          pkginfo.pkgv = pkgv
>>          pkginfo.pkgr = pkgr
>> +        pkginfo.extendprauto = extendprauto
>>          pkginfo.rprovides = sortpkglist(oe.utils.squashspaces(pkgdata.get('RPROVIDES', "")))
>>          pkginfo.rdepends = sortpkglist(oe.utils.squashspaces(pkgdata.get('RDEPENDS', "")))
>>          pkginfo.rrecommends = sortpkglist(oe.utils.squashspaces(pkgdata.get('RRECOMMENDS', "")))
>> @@ -398,6 +409,8 @@ def write_pkghistory(pkginfo, d):
>>              f.write(u"PKGV = %s\n" % pkginfo.pkgv)
>>          if pkginfo.pkgr != pkginfo.pr:
>>              f.write(u"PKGR = %s\n" % pkginfo.pkgr)
>> +        if pkginfo.extendprauto:
>> +            f.write(u"EXTENDPRAUTO = %s\n" % pkginfo.extendprauto)
>>          f.write(u"RPROVIDES = %s\n" %  pkginfo.rprovides)
>>          f.write(u"RDEPENDS = %s\n" %  pkginfo.rdepends)
>>          f.write(u"RRECOMMENDS = %s\n" %  pkginfo.rrecommends)
>> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
>> index 7a36262eb6..5fa96eb331 100644
>> --- a/meta/classes/package.bbclass
>> +++ b/meta/classes/package.bbclass
>> @@ -652,7 +652,12 @@ def runtime_mapping_rename (varname, pkg, d):
>>      #bb.note("%s before: %s" % (varname, d.getVar(varname)))
>>
>>      new_depends = {}
>> -    deps = bb.utils.explode_dep_versions2(d.getVar(varname) or "")
>> +    localdata = d.createCopy()
>> +    for exclude_var in (d.getVar('PKGDATA_VARS_NOHASH') or "").split():
>> +        # Remove the variable so it expands fully
>> +        localdata.delVar(exclude_var)
>> +
>> +    deps = bb.utils.explode_dep_versions2(localdata.getVar(varname) or "")
>>      for depend, depversions in deps.items():
>>          new_depend = get_package_mapping(depend, pkg, d, depversions)
>>          if depend != new_depend:
>> @@ -1486,8 +1491,13 @@ python package_fixsymlinks () {
>>              if found == False:
>>                  bb.note("%s contains dangling symlink to %s" % (pkg, l))
>>
>> +    localdata = d.createCopy()
>> +    for exclude_var in (d.getVar('PKGDATA_VARS_NOHASH') or "").split():
>> +        # Remove the variable so it expands fully
>> +        localdata.delVar(exclude_var)
>> +
>>      for pkg in newrdepends:
>> -        rdepends = bb.utils.explode_dep_versions2(d.getVar('RDEPENDS_' + pkg) or "")
>> +        rdepends = bb.utils.explode_dep_versions2(localdata.getVar('RDEPENDS_' + pkg) or "")
>>          for p in newrdepends[pkg]:
>>              if p not in rdepends:
>>                  rdepends[p] = []
>> @@ -1510,6 +1520,8 @@ PKGDESTWORK = "${WORKDIR}/pkgdata"
>>
>>  PKGDATA_VARS = "PN PE PV PR PKGE PKGV PKGR LICENSE DESCRIPTION SUMMARY RDEPENDS RPROVIDES RRECOMMENDS RSUGGESTS RREPLACES RCONFLICTS SECTION PKG ALLOW_EMPTY FILES CONFFILES FILES_INFO PACKAGE_ADD_METADATA pkg_postinst pkg_postrm pkg_preinst pkg_prerm"
>>
>> +PKGDATA_VARS_NOHASH = "EXTENDPRAUTO"
> 
> There are so many hashes to keep track of, can we make it more
> specific than just "HASH"? How about "PKGDATA_VARS_NOOUTHASH" ?

After talking with RP, I'm getting rid of the oe_nohash file completely.  It's
not needed any longer (after the second part of the patch.)

We still need a way to do this clearing of the variable and such -- and I'm not
particular about the name.. but NOTOUTHASH doesn't seem to be any better to me
then NOHASH in this case.

--Mark

>> +
>>  python emit_pkgdata() {
>>      from glob import glob
>>      import json
>> @@ -1548,7 +1560,7 @@ fi
>>                      scriptlet = "set -e\n" + "\n".join(scriptlet_split[0:])
>>              d.setVar('%s_%s' % (scriptlet_name, pkg), scriptlet)
>>
>> -    def write_if_exists(f, pkg, var):
>> +    def write_if_exists(d, f, pkg, var):
>>          def encode(str):
>>              import codecs
>>              c = codecs.getencoder("unicode_escape")
>> @@ -1621,17 +1633,29 @@ fi
>>          add_set_e_to_scriptlets(pkg)
>>
>>          subdata_file = pkgdatadir + "/runtime/%s" % pkg
>> +
>> +        # Write out nohash variables first
>> +        with open("%s.oe_nohash" % subdata_file, 'w') as sf:
>> +            for var in (d.getVar('PKGDATA_VARS_NOHASH') or "").split():
>> +                write_if_exists(d, sf, pkg, var)
>> +
>> +        # Write out regular variables, but sanitize nohash values
>>          with open(subdata_file, 'w') as sf:
>> +            localdata = d.createCopy()
>> +            for exclude_var in (d.getVar('PKGDATA_VARS_NOHASH') or "").split():
>> +                # Remove the variable so it expands fully
>> +                localdata.delVar(exclude_var)
>> +
>>              for var in (d.getVar('PKGDATA_VARS') or "").split():
>> -                val = write_if_exists(sf, pkg, var)
>> +                write_if_exists(localdata, sf, pkg, var)
>>
>> -            write_if_exists(sf, pkg, 'FILERPROVIDESFLIST')
>> -            for dfile in (d.getVar('FILERPROVIDESFLIST_' + pkg) or "").split():
>> -                write_if_exists(sf, pkg, 'FILERPROVIDES_' + dfile)
>> +            write_if_exists(localdata, sf, pkg, 'FILERPROVIDESFLIST')
>> +            for dfile in (localdata.getVar('FILERPROVIDESFLIST_' + pkg) or "").split():
>> +                write_if_exists(localdata, sf, pkg, 'FILERPROVIDES_' + dfile)
>>
>> -            write_if_exists(sf, pkg, 'FILERDEPENDSFLIST')
>> -            for dfile in (d.getVar('FILERDEPENDSFLIST_' + pkg) or "").split():
>> -                write_if_exists(sf, pkg, 'FILERDEPENDS_' + dfile)
>> +            write_if_exists(localdata, sf, pkg, 'FILERDEPENDSFLIST')
>> +            for dfile in (localdata.getVar('FILERDEPENDSFLIST_' + pkg) or "").split():
>> +                write_if_exists(localdata, sf, pkg, 'FILERDEPENDS_' + dfile)
>>
>>              sf.write('%s_%s: %d\n' % ('PKGSIZE', pkg, total_size))
>>
>> @@ -2126,9 +2150,14 @@ def read_libdep_files(d):
>>  python read_shlibdeps () {
>>      pkglibdeps = read_libdep_files(d)
>>
>> +    localdata = d.createCopy()
>> +    for exclude_var in (d.getVar('PKGDATA_VARS_NOHASH') or "").split():
>> +        # Remove the variable so it expands fully
>> +        localdata.delVar(exclude_var)
>> +
>>      packages = d.getVar('PACKAGES').split()
>>      for pkg in packages:
>> -        rdepends = bb.utils.explode_dep_versions2(d.getVar('RDEPENDS_' + pkg) or "")
>> +        rdepends = bb.utils.explode_dep_versions2(localdata.getVar('RDEPENDS_' + pkg) or "")
>>          for dep in sorted(pkglibdeps[pkg]):
>>              # Add the dep if it's not already there, or if no comparison is set
>>              if dep not in rdepends:
>> @@ -2158,9 +2187,13 @@ python package_depchains() {
>>      prefixes  = (d.getVar('DEPCHAIN_PRE') or '').split()
>>
>>      def pkg_adddeprrecs(pkg, base, suffix, getname, depends, d):
>> +        localdata = d.createCopy()
>> +        for exclude_var in (d.getVar('PKGDATA_VARS_NOHASH') or "").split():
>> +            # Remove the variable so it expands fully
>> +            localdata.delVar(exclude_var)
>>
>>          #bb.note('depends for %s is %s' % (base, depends))
>> -        rreclist = bb.utils.explode_dep_versions2(d.getVar('RRECOMMENDS_' + pkg) or "")
>> +        rreclist = bb.utils.explode_dep_versions2(localdata.getVar('RRECOMMENDS_' + pkg) or "")
>>
>>          for depend in sorted(depends):
>>              if depend.find('-native') != -1 or depend.find('-cross') != -1 or depend.startswith('virtual/'):
>> @@ -2179,9 +2212,13 @@ python package_depchains() {
>>          d.setVar('RRECOMMENDS_%s' % pkg, bb.utils.join_deps(rreclist, commasep=False))
>>
>>      def pkg_addrrecs(pkg, base, suffix, getname, rdepends, d):
>> +        localdata = d.createCopy()
>> +        for exclude_var in (d.getVar('PKGDATA_VARS_NOHASH') or "").split():
>> +            # Remove the variable so it expands fully
>> +            localdata.delVar(exclude_var)
>>
>>          #bb.note('rdepends for %s is %s' % (base, rdepends))
>> -        rreclist = bb.utils.explode_dep_versions2(d.getVar('RRECOMMENDS_' + pkg) or "")
>> +        rreclist = bb.utils.explode_dep_versions2(localdata.getVar('RRECOMMENDS_' + pkg) or "")
>>
>>          for depend in sorted(rdepends):
>>              if depend.find('virtual-locale-') != -1:
>> diff --git a/meta/lib/oe/packagedata.py b/meta/lib/oe/packagedata.py
>> index a82085a792..8f82f0fb60 100644
>> --- a/meta/lib/oe/packagedata.py
>> +++ b/meta/lib/oe/packagedata.py
>> @@ -15,7 +15,7 @@ def read_pkgdatafile(fn):
>>          c = codecs.getdecoder("unicode_escape")
>>          return c(str)[0]
>>
>> -    if os.access(fn, os.R_OK):
>> +    def load_file(fn):
>>          import re
>>          with open(fn, 'r') as f:
>>              lines = f.readlines()
>> @@ -25,6 +25,12 @@ def read_pkgdatafile(fn):
>>              if m:
>>                  pkgdata[m.group(1)] = decode(m.group(2))
>>
>> +    if os.access("%s.oe_nohash" % fn, os.R_OK):
>> +        load_file("%s.oe_nohash" % fn)
>> +
>> +    if os.access(fn, os.R_OK):
>> +        load_file(fn)
>> +
>>      return pkgdata
>>
>>  def get_subpkgedata_fn(pkg, d):
>> diff --git a/meta/lib/oe/sstatesig.py b/meta/lib/oe/sstatesig.py
>> index 21ae0a7657..f96b96e261 100644
>> --- a/meta/lib/oe/sstatesig.py
>> +++ b/meta/lib/oe/sstatesig.py
>> @@ -589,6 +589,8 @@ def OEOuthashBasic(path, sigfile, task, d):
>>              # Process this directory and all its child files
>>              process(root)
>>              for f in files:
>> +                if f.endswith('.oe_nohash'):
> 
> Can we use ".oe_noouthash"?
> 
>> +                    continue
>>                  if f == 'fixmepath':
>>                      continue
>>                  process(os.path.join(root, f))
>> --
>> 2.17.1
>>
>>