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

Mark Hatle
2020-08-25T14:16:14Z

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
>>
>>