* [master][PATCH 0/5] Allow PR Service and hash equiv together
@ 2020-08-24 23:29 Mark Hatle
2020-08-24 23:29 ` [master][PATCH 1/5] package_tar.bbclass: Sync to the other package_* classes Mark Hatle
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Mark Hatle @ 2020-08-24 23:29 UTC (permalink / raw)
To: openembedded-core
Before PR service didn't work reliably with hash equivalency. Generally
you ended up with results that may not be reproducible, even if you
started with the same PR service database and hash equivalency database.
Overtime, intermediate PR values would be created that would cause thing
to get out of sync in the case of certain rebuilds or other corner cases.
The set refactors the PR service to work along side the new hash equiv
system. It moves the PR and AUTOINC lookup to AFTER the do_package task
is complete. This allows us to use the do_package unihash for lookup.
Additionally this fixed a small issue with the kernel, where the PR value
could get incremented twice. The fix is an artifact of the other changes
that cause us to only run the PR service work once per recipe.
This has been tested with the following workflow, which covers one of
the critical corner cases for me:
configure local.conf with:
BB_HASHSERVE = "auto"
BB_SIGNATURE_HANDLER = "OEEquivHash"
PRSERV_HOST ??= "localhost:0"
INHERIT += "reproducible_build"
INHERIT += "buildhistory"
bitbake glibc linux-yocto
# Modify meta/recipes-core/glibc/glibc_2.32.bb, add a comment
# to the do_patch_append(). This will taint the hash of this
# function.
bitbake glibc linux-yocto
# The system should have detected the output was the same, and
# no proceed past do_package in glibc. The kernel should not
# have built at all.
# Store/mv the tmp and sstate-cache from that build elsewhere
# repeat the run
bitbake glibc linux-yocto
# Compare the results of tmp/deploy/<package>/* between last
# and current run.
#
# The contents should be the same (filenames specifically).
#
# Also the kernel should be r0.0, not r0.1.
Note: if the hash equivalency database or PR server database (located
in the cache directory) is removed, the values may not be the same
as the previous run.
Additionally while testing the various package_*.bbclass files, it
was noted that package_tar.bbclass was not working the same way as
the others. This was correct as a standalone patch.
Mark Hatle (5):
package_tar.bbclass: Sync to the other package_* classes
hash equivalency and pr service
package.bbclass: Move package_get_auto_pr to packagedata.bbclass
package / packagedata bbclass: Change the way PRAUTO and AUTOINC are
handled
buildhistory.bbclass: Rework to use read_subpackage_metadata
meta/classes/buildhistory.bbclass | 49 ++++++------
meta/classes/kernel.bbclass | 4 +-
meta/classes/nopackages.bbclass | 1 +
meta/classes/package.bbclass | 120 +++++++++++++++---------------
meta/classes/package_tar.bbclass | 11 +--
meta/classes/packagedata.bbclass | 69 +++++++++++++++++
meta/conf/bitbake.conf | 1 +
meta/lib/oe/packagedata.py | 8 +-
meta/lib/oe/sstatesig.py | 2 +
9 files changed, 169 insertions(+), 96 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [master][PATCH 1/5] package_tar.bbclass: Sync to the other package_* classes
2020-08-24 23:29 [master][PATCH 0/5] Allow PR Service and hash equiv together Mark Hatle
@ 2020-08-24 23:29 ` Mark Hatle
2020-08-25 10:49 ` [OE-core] " Richard Purdie
2020-08-24 23:29 ` [master][PATCH 2/5] hash equivalency and pr service Mark Hatle
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Mark Hatle @ 2020-08-24 23:29 UTC (permalink / raw)
To: openembedded-core
Sync up the task definitions with the other package classes. This may not
have been strictly necessary but will make overall maintenance easier as
the various package classes are now in sync.
Additional, there was a missing deltask in the nopackages.bbclass related
to the package_tar which has been corrected. This could cause problems on
native recipes when package_tar was enabled.
Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
---
meta/classes/nopackages.bbclass | 1 +
meta/classes/package_tar.bbclass | 11 ++++++-----
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/meta/classes/nopackages.bbclass b/meta/classes/nopackages.bbclass
index 559f5078bd..7a4f632d71 100644
--- a/meta/classes/nopackages.bbclass
+++ b/meta/classes/nopackages.bbclass
@@ -2,6 +2,7 @@ deltask do_package
deltask do_package_write_rpm
deltask do_package_write_ipk
deltask do_package_write_deb
+deltask do_package_write_tar
deltask do_package_qa
deltask do_packagedata
deltask do_package_setscene
diff --git a/meta/classes/package_tar.bbclass b/meta/classes/package_tar.bbclass
index ce3ab4c8e2..8946bc212a 100644
--- a/meta/classes/package_tar.bbclass
+++ b/meta/classes/package_tar.bbclass
@@ -57,10 +57,8 @@ python do_package_tar () {
python () {
if d.getVar('PACKAGES') != '':
- deps = (d.getVarFlag('do_package_write_tar', 'depends') or "").split()
- deps.append('tar-native:do_populate_sysroot')
- deps.append('virtual/fakeroot-native:do_populate_sysroot')
- d.setVarFlag('do_package_write_tar', 'depends', " ".join(deps))
+ deps = ' tar-native:do_populate_sysroot virtual/fakeroot-native:do_populate_sysroot'
+ d.appendVarFlag('do_package_write_tar', 'depends', deps)
d.setVarFlag('do_package_write_tar', 'fakeroot', "1")
}
@@ -70,4 +68,7 @@ python do_package_write_tar () {
bb.build.exec_func("do_package_tar", d)
}
do_package_write_tar[dirs] = "${D}"
-addtask package_write_tar before do_build after do_packagedata do_package
+do_package_write_tar[depends] += "${@oe.utils.build_depends_string(d.getVar('PACKAGE_WRITE_DEPS'), 'do_populate_sysroot')}"
+addtask package_write_tar after do_packagedata do_package
+
+do_build[recrdeptask] += "do_package_write_tar"
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [master][PATCH 2/5] hash equivalency and pr service
2020-08-24 23:29 [master][PATCH 0/5] Allow PR Service and hash equiv together Mark Hatle
2020-08-24 23:29 ` [master][PATCH 1/5] package_tar.bbclass: Sync to the other package_* classes Mark Hatle
@ 2020-08-24 23:29 ` Mark Hatle
2020-08-25 12:50 ` [OE-core] " Joshua Watt
2020-08-24 23:29 ` [master][PATCH 3/5] package.bbclass: Move package_get_auto_pr to packagedata.bbclass Mark Hatle
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Mark Hatle @ 2020-08-24 23:29 UTC (permalink / raw)
To: openembedded-core
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"
+
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'):
+ continue
if f == 'fixmepath':
continue
process(os.path.join(root, f))
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [master][PATCH 3/5] package.bbclass: Move package_get_auto_pr to packagedata.bbclass
2020-08-24 23:29 [master][PATCH 0/5] Allow PR Service and hash equiv together Mark Hatle
2020-08-24 23:29 ` [master][PATCH 1/5] package_tar.bbclass: Sync to the other package_* classes Mark Hatle
2020-08-24 23:29 ` [master][PATCH 2/5] hash equivalency and pr service Mark Hatle
@ 2020-08-24 23:29 ` Mark Hatle
2020-08-24 23:29 ` [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled Mark Hatle
` (2 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Mark Hatle @ 2020-08-24 23:29 UTC (permalink / raw)
To: openembedded-core
Running package_get_auto_pr during do_package is 'too early' when we need
to evaluate the do_package output to get it's unihash to look up the
right value in the PR database.
Move this function packagedata.bbclass (no changes yet), as subsequent
patches will refactor this function and the order in which it's called.
Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
---
meta/classes/package.bbclass | 50 ---------------------------
meta/classes/packagedata.bbclass | 59 ++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 50 deletions(-)
diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 5fa96eb331..003eb71a6e 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -672,56 +672,6 @@ def runtime_mapping_rename (varname, pkg, d):
# Package functions suitable for inclusion in PACKAGEFUNCS
#
-python package_get_auto_pr() {
- import oe.prservice
- import re
-
- # Support per recipe PRSERV_HOST
- pn = d.getVar('PN')
- host = d.getVar("PRSERV_HOST_" + pn)
- if not (host is None):
- d.setVar("PRSERV_HOST", host)
-
- pkgv = d.getVar("PKGV")
-
- # PR Server not active, handle AUTOINC
- if not d.getVar('PRSERV_HOST'):
- if 'AUTOINC' in pkgv:
- d.setVar("PKGV", pkgv.replace("AUTOINC", "0"))
- return
-
- auto_pr = None
- pv = d.getVar("PV")
- version = d.getVar("PRAUTOINX")
- pkgarch = d.getVar("PACKAGE_ARCH")
- checksum = d.getVar("BB_TASKHASH")
-
- if d.getVar('PRSERV_LOCKDOWN'):
- auto_pr = d.getVar('PRAUTO_' + version + '_' + pkgarch) or d.getVar('PRAUTO_' + version) or None
- if auto_pr is None:
- bb.fatal("Can NOT get PRAUTO from lockdown exported file")
- d.setVar('PRAUTO',str(auto_pr))
- return
-
- try:
- conn = d.getVar("__PRSERV_CONN")
- if conn is None:
- conn = oe.prservice.prserv_make_conn(d)
- if conn is not None:
- if "AUTOINC" in pkgv:
- srcpv = bb.fetch2.get_srcrev(d)
- base_ver = "AUTOINC-%s" % version[:version.find(srcpv)]
- value = conn.getPR(base_ver, pkgarch, srcpv)
- d.setVar("PKGV", pkgv.replace("AUTOINC", str(value)))
-
- auto_pr = conn.getPR(version, pkgarch, checksum)
- except Exception as e:
- bb.fatal("Can NOT get PRAUTO, exception %s" % str(e))
- if auto_pr is None:
- bb.fatal("Can NOT get PRAUTO from remote PR service")
- d.setVar('PRAUTO',str(auto_pr))
-}
-
LOCALEBASEPN ??= "${PN}"
python package_do_split_locales() {
diff --git a/meta/classes/packagedata.bbclass b/meta/classes/packagedata.bbclass
index a903e5cfd2..981c324909 100644
--- a/meta/classes/packagedata.bbclass
+++ b/meta/classes/packagedata.bbclass
@@ -32,3 +32,62 @@ python read_subpackage_metadata () {
else:
d.setVar(key, sdata[key], parsing=True)
}
+
+package_get_auto_pr[vardepsexclude] = "BB_TASKDEPDATA"
+python package_get_auto_pr() {
+ import oe.prservice
+ import re
+
+ def get_do_package_hash(pn):
+ if d.getVar("BB_RUNTASK") != "do_package":
+ taskdepdata = d.getVar("BB_TASKDEPDATA", False)
+ for dep in taskdepdata:
+ if taskdepdata[dep][1] == "do_package" and taskdepdata[dep][0] == pn:
+ return taskdepdata[dep][6]
+ return d.getVar("BB_UNIHASH")
+
+ # Support per recipe PRSERV_HOST
+ pn = d.getVar('PN')
+ host = d.getVar("PRSERV_HOST_" + pn)
+ if not (host is None):
+ d.setVar("PRSERV_HOST", host)
+
+ pkgv = d.getVar("PKGV")
+
+ # PR Server not active, handle AUTOINC
+ if not d.getVar('PRSERV_HOST'):
+ if 'AUTOINC' in pkgv:
+ d.setVar("PKGV", pkgv.replace("AUTOINC", "0"))
+ return
+
+ auto_pr = None
+ pv = d.getVar("PV")
+ version = d.getVar("PRAUTOINX")
+ pkgarch = d.getVar("PACKAGE_ARCH")
+ checksum = get_do_package_hash(pn)
+
+ if d.getVar('PRSERV_LOCKDOWN'):
+ auto_pr = d.getVar('PRAUTO_' + version + '_' + pkgarch) or d.getVar('PRAUTO_' + version) or None
+ if auto_pr is None:
+ bb.fatal("Can NOT get PRAUTO from lockdown exported file")
+ d.setVar('PRAUTO',str(auto_pr))
+ return
+
+ try:
+ conn = d.getVar("__PRSERV_CONN")
+ if conn is None:
+ conn = oe.prservice.prserv_make_conn(d)
+ if conn is not None:
+ if "AUTOINC" in pkgv:
+ srcpv = bb.fetch2.get_srcrev(d)
+ base_ver = "AUTOINC-%s" % version[:version.find(srcpv)]
+ value = conn.getPR(base_ver, pkgarch, srcpv)
+ d.setVar("PKGV", pkgv.replace("AUTOINC", str(value)))
+
+ auto_pr = conn.getPR(version, pkgarch, checksum)
+ except Exception as e:
+ bb.fatal("Can NOT get PRAUTO, exception %s" % str(e))
+ if auto_pr is None:
+ bb.fatal("Can NOT get PRAUTO from remote PR service")
+ d.setVar('PRAUTO',str(auto_pr))
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled
2020-08-24 23:29 [master][PATCH 0/5] Allow PR Service and hash equiv together Mark Hatle
` (2 preceding siblings ...)
2020-08-24 23:29 ` [master][PATCH 3/5] package.bbclass: Move package_get_auto_pr to packagedata.bbclass Mark Hatle
@ 2020-08-24 23:29 ` Mark Hatle
2020-08-25 10:56 ` [OE-core] " Richard Purdie
2020-08-25 11:00 ` Richard Purdie
2020-08-24 23:29 ` [master][PATCH 5/5] buildhistory.bbclass: Rework to use read_subpackage_metadata Mark Hatle
2020-08-24 23:32 ` ✗ patchtest: failure for Allow PR Service and hash equiv together Patchwork
5 siblings, 2 replies; 18+ messages in thread
From: Mark Hatle @ 2020-08-24 23:29 UTC (permalink / raw)
To: openembedded-core
During the do_package operation, we need to convert AUTOINC to some other
value, as PKGV is set and stored to the packagedata at this time. We
create a new variable called "PRSERV_PV_AUTOINC" to handle this case,
with a default value of AUTOINC.
The value of PRSERV_PV_AUTOINC is added to the PKGDATA_VARS_NOHASH so it
does NOT get written as part of the PKGV to the packagedata files, but is
preserved as a bitbake variable for later processing.
As part of this work, we deterined we should not always write out
PKGDATA_VARS_NOHASH, as it may contain incomplete information. For instance
EXTENDPRAUTO at do_package time is "", while later becomes a ".0" or
similar number when the PR Service is enabled.
do_packagedata was modified to explicitly run package_get_auto_pr, and then
preserve the PRAUTO and PRSERV_PV_AUTOINC values. We store these in a new
file called ${PN}_prservice.oe_nohash. Doing it this way will ensure that
subsequent calls to "read_subpackage_metadata", and similar function will
have correct results for the recipes PRAUTO and PRSERV_PV_AUTOINC.
read_subpackage_metadata was modified to read this file as well.
package_get_auto_pr was refactored to add processing to set the
correct PRSERV_PV_AUTOINC values, instead of modifying PKGV directly.
A related change was also needed for the kernel.bbclass. The kernel
needs to get the AUTOINC values used by the various PV/PKGV, but with
this new system we don't want to call auto_pr each time -- instead
call the read_subpackage_metadata to ensure a consistent result.
This also fixes an issue in the old version where the kernel sometimes
caused the PR to be incremented twice, as the auto_pr was called
multiple times from different tasks.
Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
---
meta/classes/kernel.bbclass | 4 ++--
meta/classes/package.bbclass | 27 +++++++++++++++++++--------
meta/classes/packagedata.bbclass | 30 ++++++++++++++++++++----------
meta/conf/bitbake.conf | 1 +
4 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index e2ceb6a333..4449452065 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -416,7 +416,7 @@ kernel_do_install() {
install -d ${D}${sysconfdir}/modules-load.d
install -d ${D}${sysconfdir}/modprobe.d
}
-do_install[prefuncs] += "package_get_auto_pr"
+do_install[prefuncs] += "read_subpackage_metadata"
# Must be ran no earlier than after do_kernel_checkout or else Makefile won't be in ${S}/Makefile
do_kernel_version_sanity_check() {
@@ -749,7 +749,7 @@ kernel_do_deploy() {
done
fi
}
-do_deploy[prefuncs] += "package_get_auto_pr"
+do_deploy[prefuncs] += "read_subpackage_metadata"
addtask deploy after do_populate_sysroot do_packagedata
diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 003eb71a6e..e822174b8a 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -7,7 +7,7 @@
#
# There are the following default steps but PACKAGEFUNCS can be extended:
#
-# a) package_get_auto_pr - get PRAUTO from remote PR service
+# a) package_convert_autoinc - convert autoinc in PKGV to ${PRSERV_PV_AUTOINC}
#
# b) perform_packagecopy - Copy D into PKGD
#
@@ -672,6 +672,13 @@ def runtime_mapping_rename (varname, pkg, d):
# Package functions suitable for inclusion in PACKAGEFUNCS
#
+python package_convert_autoinc() {
+ pkgv = d.getVar("PKGV")
+
+ if 'AUTOINC' in pkgv:
+ d.setVar("PKGV", pkgv.replace("AUTOINC", "${PRSERV_PV_AUTOINC}"))
+}
+
LOCALEBASEPN ??= "${PN}"
python package_do_split_locales() {
@@ -1470,7 +1477,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"
+# Really we only care about PRAUTO, but EXTENDPRAUTO evaluates this
+PKGDATA_VARS_NOHASH = "EXTENDPRAUTO PRSERV_PV_AUTOINC"
python emit_pkgdata() {
from glob import glob
@@ -1584,11 +1592,6 @@ fi
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()
@@ -2322,7 +2325,7 @@ python do_package () {
package_qa_handle_error("var-undefined", msg, d)
return
- bb.build.exec_func("package_get_auto_pr", d)
+ bb.build.exec_func("package_convert_autoinc", d)
###########################################################################
# Optimisations
@@ -2396,6 +2399,14 @@ addtask do_package_setscene
python do_packagedata () {
src = d.expand("${PKGDESTWORK}")
dest = d.expand("${WORKDIR}/pkgdata-pdata-input")
+
+ bb.build.exec_func("package_get_auto_pr", d)
+ # Store this for later retrieval
+ data_file = src + d.expand("/${PN}_prservice.oe_nohash")
+ with open(data_file, 'w') as fd:
+ fd.write('PRAUTO: %s\n' % d.getVar('PRAUTO'))
+ fd.write('PRSERV_PV_AUTOINC: %s\n' % d.getVar("PRSERV_PV_AUTOINC"))
+
oe.path.copyhardlinktree(src, dest)
}
diff --git a/meta/classes/packagedata.bbclass b/meta/classes/packagedata.bbclass
index 981c324909..176bfe6948 100644
--- a/meta/classes/packagedata.bbclass
+++ b/meta/classes/packagedata.bbclass
@@ -9,7 +9,10 @@ python read_subpackage_metadata () {
}
data = oe.packagedata.read_pkgdata(vars["PN"], d)
+ for key in data.keys():
+ d.setVar(key, data[key])
+ data = oe.packagedata.read_pkgdata("%s_prservice" % d.getVar('PN'), d)
for key in data.keys():
d.setVar(key, data[key])
@@ -36,15 +39,15 @@ python read_subpackage_metadata () {
package_get_auto_pr[vardepsexclude] = "BB_TASKDEPDATA"
python package_get_auto_pr() {
import oe.prservice
- import re
def get_do_package_hash(pn):
- if d.getVar("BB_RUNTASK") != "do_package":
- taskdepdata = d.getVar("BB_TASKDEPDATA", False)
- for dep in taskdepdata:
- if taskdepdata[dep][1] == "do_package" and taskdepdata[dep][0] == pn:
- return taskdepdata[dep][6]
- return d.getVar("BB_UNIHASH")
+ if d.getVar("BB_RUNTASK") == "do_package":
+ return d.getVar('BB_UNIHASH')
+ taskdepdata = d.getVar("BB_TASKDEPDATA", False)
+ for dep in taskdepdata:
+ if taskdepdata[dep][1] == "do_package" and taskdepdata[dep][0] == pn:
+ return taskdepdata[dep][6]
+ return None
# Support per recipe PRSERV_HOST
pn = d.getVar('PN')
@@ -56,8 +59,7 @@ python package_get_auto_pr() {
# PR Server not active, handle AUTOINC
if not d.getVar('PRSERV_HOST'):
- if 'AUTOINC' in pkgv:
- d.setVar("PKGV", pkgv.replace("AUTOINC", "0"))
+ d.setVar("PRSERV_PV_AUTOINC", "0")
return
auto_pr = None
@@ -66,6 +68,14 @@ python package_get_auto_pr() {
pkgarch = d.getVar("PACKAGE_ARCH")
checksum = get_do_package_hash(pn)
+ # If do_package isn't in the dependencies, we can't get the checksum...
+ if not checksum:
+ bb.warn('Task %s requested do_package unihash, but it was not available.' % d.getVar('BB_RUNTASK'))
+ #taskdepdata = d.getVar("BB_TASKDEPDATA", False)
+ #for dep in taskdepdata:
+ # bb.warn('%s:%s = %s' % (taskdepdata[dep][0], taskdepdata[dep][1], taskdepdata[dep][6]))
+ return
+
if d.getVar('PRSERV_LOCKDOWN'):
auto_pr = d.getVar('PRAUTO_' + version + '_' + pkgarch) or d.getVar('PRAUTO_' + version) or None
if auto_pr is None:
@@ -82,7 +92,7 @@ python package_get_auto_pr() {
srcpv = bb.fetch2.get_srcrev(d)
base_ver = "AUTOINC-%s" % version[:version.find(srcpv)]
value = conn.getPR(base_ver, pkgarch, srcpv)
- d.setVar("PKGV", pkgv.replace("AUTOINC", str(value)))
+ d.setVar("PRSERV_PV_AUTOINC", str(value))
auto_pr = conn.getPR(version, pkgarch, checksum)
except Exception as e:
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 353caacef9..65b4432c63 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -208,6 +208,7 @@ PF = "${PN}-${EXTENDPE}${PV}-${PR}"
EXTENDPE = "${@['','${PE}_'][int(d.getVar('PE') or 0) > 0]}"
P = "${PN}-${PV}"
+PRSERV_PV_AUTOINC = "AUTOINC"
PRAUTO = ""
EXTENDPRAUTO = "${@['.${PRAUTO}', ''][not d.getVar('PRAUTO')]}"
PRAUTOINX = "${PF}"
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [master][PATCH 5/5] buildhistory.bbclass: Rework to use read_subpackage_metadata
2020-08-24 23:29 [master][PATCH 0/5] Allow PR Service and hash equiv together Mark Hatle
` (3 preceding siblings ...)
2020-08-24 23:29 ` [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled Mark Hatle
@ 2020-08-24 23:29 ` Mark Hatle
2020-08-24 23:32 ` ✗ patchtest: failure for Allow PR Service and hash equiv together Patchwork
5 siblings, 0 replies; 18+ messages in thread
From: Mark Hatle @ 2020-08-24 23:29 UTC (permalink / raw)
To: openembedded-core
Using this mechanism ensures that we have a single point to implement
the loading of the package and subpackage meta data. This also then
allows the buildhistory class to use the regular datastore vs it's
own custom arrays for processing history items.
Without this refactoring PRSERV_PV_AUTOINC and sometimes PRAUTO do not
get expanded properly within the buildhistory which can leave to
error messages about package versions/releases going backwards.
Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
---
meta/classes/buildhistory.bbclass | 60 +++++++++++--------------------
1 file changed, 21 insertions(+), 39 deletions(-)
diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index 2bccdfc953..5d478f38b5 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -99,7 +99,6 @@ 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')
@@ -128,7 +127,6 @@ python buildhistory_emit_pkghistory() {
self.pkge = ""
self.pkgv = ""
self.pkgr = ""
- self.extendprauto = ""
self.size = 0
self.depends = ""
self.rprovides = ""
@@ -165,8 +163,6 @@ 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":
@@ -262,26 +258,15 @@ python buildhistory_emit_pkghistory() {
rcpinfo.config = sortlist(oe.utils.squashspaces(d.getVar('PACKAGECONFIG') or ""))
write_recipehistory(rcpinfo, d)
- pkgdest = d.getVar('PKGDEST')
- for pkg in packagelist:
- 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]
+ bb.build.exec_func("read_subpackage_metadata", d)
- for key in npkgdata.keys():
- pkgdata[key] = npkgdata[key]
+ for pkg in packagelist:
+ localdata = d.createCopy()
+ localdata.setVar('OVERRIDES', d.getVar("OVERRIDES", False) + ":" + pkg)
- pkge = pkgdata.get('PKGE', '0')
- pkgv = pkgdata['PKGV']
- pkgr = pkgdata['PKGR']
- try:
- extendprauto = pkgdata['EXTENDPRAUTO']
- except IndexError:
- extendprauto = d.getVar('EXTENDPRAUTO')
+ pkge = localdata.getVar("PKGE") or '0'
+ pkgv = localdata.getVar("PKGV")
+ pkgr = localdata.getVar("PKGR")
#
# Find out what the last version was
# Make sure the version did not decrease
@@ -298,32 +283,31 @@ python buildhistory_emit_pkghistory() {
pkginfo = PackageInfo(pkg)
# Apparently the version can be different on a per-package basis (see Python)
- pkginfo.pe = pkgdata.get('PE', '0')
- pkginfo.pv = pkgdata['PV']
- pkginfo.pr = pkgdata['PR']
- pkginfo.pkg = pkgdata['PKG']
+ pkginfo.pe = localdata.getVar("PE") or '0'
+ pkginfo.pv = localdata.getVar("PV")
+ pkginfo.pr = localdata.getVar("PR")
+ pkginfo.pkg = localdata.getVar("PKG")
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', "")))
- pkginfo.rsuggests = sortpkglist(oe.utils.squashspaces(pkgdata.get('RSUGGESTS', "")))
- pkginfo.rreplaces = sortpkglist(oe.utils.squashspaces(pkgdata.get('RREPLACES', "")))
- pkginfo.rconflicts = sortpkglist(oe.utils.squashspaces(pkgdata.get('RCONFLICTS', "")))
- pkginfo.files = oe.utils.squashspaces(pkgdata.get('FILES', ""))
+ pkginfo.rprovides = sortpkglist(oe.utils.squashspaces(localdata.getVar("RPROVIDES") or ""))
+ pkginfo.rdepends = sortpkglist(oe.utils.squashspaces(localdata.getVar("RDEPENDS") or ""))
+ pkginfo.rrecommends = sortpkglist(oe.utils.squashspaces(localdata.getVar("RRECOMMENDS") or ""))
+ pkginfo.rsuggests = sortpkglist(oe.utils.squashspaces(localdata.getVar("RSUGGESTS") or ""))
+ pkginfo.replaces = sortpkglist(oe.utils.squashspaces(localdata.getVar("RREPLACES") or ""))
+ pkginfo.rconflicts = sortpkglist(oe.utils.squashspaces(localdata.getVar("RCONFLICTS") or ""))
+ pkginfo.files = sortpkglist(oe.utils.squashspaces(localdata.getVar("FILES") or ""))
for filevar in pkginfo.filevars:
- pkginfo.filevars[filevar] = pkgdata.get(filevar, "")
+ pkginfo.filevars[filevar] = localdata.getVar(filevar) or ""
# Gather information about packaged files
- val = pkgdata.get('FILES_INFO', '')
+ val = localdata.getVar('FILES_INFO') or ''
dictval = json.loads(val)
filelist = list(dictval.keys())
filelist.sort()
pkginfo.filelist = " ".join([shlex.quote(x) for x in filelist])
- pkginfo.size = int(pkgdata['PKGSIZE'])
+ pkginfo.size = int(localdata.getVar('PKGSIZE') or '0')
write_pkghistory(pkginfo, d)
@@ -409,8 +393,6 @@ 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)
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✗ patchtest: failure for Allow PR Service and hash equiv together
2020-08-24 23:29 [master][PATCH 0/5] Allow PR Service and hash equiv together Mark Hatle
` (4 preceding siblings ...)
2020-08-24 23:29 ` [master][PATCH 5/5] buildhistory.bbclass: Rework to use read_subpackage_metadata Mark Hatle
@ 2020-08-24 23:32 ` Patchwork
2020-08-25 1:17 ` Mark Hatle
5 siblings, 1 reply; 18+ messages in thread
From: Patchwork @ 2020-08-24 23:32 UTC (permalink / raw)
To: Mark Hatle; +Cc: openembedded-core
== Series Details ==
Series: Allow PR Service and hash equiv together
Revision: 1
URL : https://patchwork.openembedded.org/series/25758/
State : failure
== Summary ==
Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:
* Patch [master,2/5] hash equivalency and pr service
Issue Shortlog does not follow expected format [test_shortlog_format]
Suggested fix Commit shortlog (first line of commit message) should follow the format "<target>: <summary>"
* Issue Errors in your Python code were encountered [test_pylint]
Suggested fix Correct the lines introduced by your patch
Output Please, fix the listed issues:
meta/lib/oe/packagedata.py does not exist
If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).
---
Guidelines: https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: ✗ patchtest: failure for Allow PR Service and hash equiv together
2020-08-24 23:32 ` ✗ patchtest: failure for Allow PR Service and hash equiv together Patchwork
@ 2020-08-25 1:17 ` Mark Hatle
0 siblings, 0 replies; 18+ messages in thread
From: Mark Hatle @ 2020-08-25 1:17 UTC (permalink / raw)
To: openembedded-core
On 8/24/20 6:32 PM, Patchwork wrote:
> == Series Details ==
>
> Series: Allow PR Service and hash equiv together
> Revision: 1
> URL : https://patchwork.openembedded.org/series/25758/
> State : failure
>
> == Summary ==
>
>
> Thank you for submitting this patch series to OpenEmbedded Core. This is
> an automated response. Several tests have been executed on the proposed
> series by patchtest resulting in the following failures:
>
>
>
> * Patch [master,2/5] hash equivalency and pr service
> Issue Shortlog does not follow expected format [test_shortlog_format]
> Suggested fix Commit shortlog (first line of commit message) should follow the format "<target>: <summary>"
In this case, I think what I did was acceptable. If reviewers disagree I'll
change the format.
> * Issue Errors in your Python code were encountered [test_pylint]
> Suggested fix Correct the lines introduced by your patch
> Output Please, fix the listed issues:
> meta/lib/oe/packagedata.py does not exist
Now this one is _confusing_, it absolutely exists:
https://git.openembedded.org/openembedded-core/tree/meta/lib/oe/packagedata.py
Is the link engine broken?
> If you believe any of these test results are incorrect, please reply to the
> mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
DONE!
--Mark
> Otherwise we would appreciate you correcting the issues and submitting a new
> version of the patchset if applicable. Please ensure you add/increment the
> version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
> [PATCH v3] -> ...).
>
> ---
> Guidelines: https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
> Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
> Test suite: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 1/5] package_tar.bbclass: Sync to the other package_* classes
2020-08-24 23:29 ` [master][PATCH 1/5] package_tar.bbclass: Sync to the other package_* classes Mark Hatle
@ 2020-08-25 10:49 ` Richard Purdie
0 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2020-08-25 10:49 UTC (permalink / raw)
To: Mark Hatle, openembedded-core
On Mon, 2020-08-24 at 18:29 -0500, Mark Hatle wrote:
> Sync up the task definitions with the other package classes. This may not
> have been strictly necessary but will make overall maintenance easier as
> the various package classes are now in sync.
>
> Additional, there was a missing deltask in the nopackages.bbclass related
> to the package_tar which has been corrected. This could cause problems on
> native recipes when package_tar was enabled.
>
> Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
> ---
> meta/classes/nopackages.bbclass | 1 +
> meta/classes/package_tar.bbclass | 11 ++++++-----
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/meta/classes/nopackages.bbclass b/meta/classes/nopackages.bbclass
> index 559f5078bd..7a4f632d71 100644
> --- a/meta/classes/nopackages.bbclass
> +++ b/meta/classes/nopackages.bbclass
> @@ -2,6 +2,7 @@ deltask do_package
> deltask do_package_write_rpm
> deltask do_package_write_ipk
> deltask do_package_write_deb
> +deltask do_package_write_tar
> deltask do_package_qa
> deltask do_packagedata
> deltask do_package_setscene
This part makes sense.
> diff --git a/meta/classes/package_tar.bbclass b/meta/classes/package_tar.bbclass
> index ce3ab4c8e2..8946bc212a 100644
> --- a/meta/classes/package_tar.bbclass
> +++ b/meta/classes/package_tar.bbclass
> @@ -57,10 +57,8 @@ python do_package_tar () {
>
> python () {
> if d.getVar('PACKAGES') != '':
> - deps = (d.getVarFlag('do_package_write_tar', 'depends') or "").split()
> - deps.append('tar-native:do_populate_sysroot')
> - deps.append('virtual/fakeroot-native:do_populate_sysroot')
> - d.setVarFlag('do_package_write_tar', 'depends', " ".join(deps))
> + deps = ' tar-native:do_populate_sysroot virtual/fakeroot-native:do_populate_sysroot'
> + d.appendVarFlag('do_package_write_tar', 'depends', deps)
This is effectively no change but standardises so ok...
> d.setVarFlag('do_package_write_tar', 'fakeroot', "1")
> }
>
> @@ -70,4 +68,7 @@ python do_package_write_tar () {
> bb.build.exec_func("do_package_tar", d)
> }
> do_package_write_tar[dirs] = "${D}"
> -addtask package_write_tar before do_build after do_packagedata do_package
> +do_package_write_tar[depends] += "${@oe.utils.build_depends_string(d.getVar('PACKAGE_WRITE_DEPS'), 'do_populate_sysroot')}"
Does package_tar need those dependencies? it doesn't handle postinsts
so it basically doesn't.
> +addtask package_write_tar after do_packagedata do_package
> +
> +do_build[recrdeptask] += "do_package_write_tar"
This is also incorrect since tar's have no dependencies on other things
and I suspect this difference is deliberate.
Cheers,
Richard
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled
2020-08-24 23:29 ` [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled Mark Hatle
@ 2020-08-25 10:56 ` Richard Purdie
2020-08-25 14:14 ` Mark Hatle
[not found] ` <162E885FFA00831B.12036@lists.openembedded.org>
2020-08-25 11:00 ` Richard Purdie
1 sibling, 2 replies; 18+ messages in thread
From: Richard Purdie @ 2020-08-25 10:56 UTC (permalink / raw)
To: Mark Hatle, openembedded-core
On Mon, 2020-08-24 at 18:29 -0500, Mark Hatle wrote:
> A related change was also needed for the kernel.bbclass. The kernel
>
> needs to get the AUTOINC values used by the various PV/PKGV, but with
> this new system we don't want to call auto_pr each time -- instead
> call the read_subpackage_metadata to ensure a consistent result.
>
> This also fixes an issue in the old version where the kernel
> sometimes
> caused the PR to be incremented twice, as the auto_pr was called
> multiple times from different tasks.
[...]
>
> diff --git a/meta/classes/kernel.bbclass
> b/meta/classes/kernel.bbclass
> index e2ceb6a333..4449452065 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -416,7 +416,7 @@ kernel_do_install() {
> install -d ${D}${sysconfdir}/modules-load.d
> install -d ${D}${sysconfdir}/modprobe.d
> }
> -do_install[prefuncs] += "package_get_auto_pr"
> +do_install[prefuncs] += "read_subpackage_metadata"
>
> # Must be ran no earlier than after do_kernel_checkout or else
> Makefile won't be in ${S}/Makefile
> do_kernel_version_sanity_check() {
> @@ -749,7 +749,7 @@ kernel_do_deploy() {
> done
> fi
> }
> -do_deploy[prefuncs] += "package_get_auto_pr"
> +do_deploy[prefuncs] += "read_subpackage_metadata"
>
> addtask deploy after do_populate_sysroot do_packagedata
I don't think this can be right. I understand why the kernel was
expanding these early but at this point do_package hasn't happened so
reading the subpackage metadata is a null op at best and potentially
changes a lot more in the data store.
I suspect this is not doing the right thing and will be something that
comes back to bite us so we need something more careful here and I'd
like to better understand exactly what change to the variables
do_install and do_deploy need.
Cheers,
Richard
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled
2020-08-24 23:29 ` [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled Mark Hatle
2020-08-25 10:56 ` [OE-core] " Richard Purdie
@ 2020-08-25 11:00 ` Richard Purdie
1 sibling, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2020-08-25 11:00 UTC (permalink / raw)
To: Mark Hatle, openembedded-core
On Mon, 2020-08-24 at 18:29 -0500, Mark Hatle wrote:
@@ -2396,6 +2399,14 @@ addtask do_package_setscene
> python do_packagedata () {
> src = d.expand("${PKGDESTWORK}")
> dest = d.expand("${WORKDIR}/pkgdata-pdata-input")
> +
> + bb.build.exec_func("package_get_auto_pr", d)
> + # Store this for later retrieval
> + data_file = src + d.expand("/${PN}_prservice.oe_nohash")
> + with open(data_file, 'w') as fd:
> + fd.write('PRAUTO: %s\n' % d.getVar('PRAUTO'))
> + fd.write('PRSERV_PV_AUTOINC: %s\n' % d.getVar("PRSERV_PV_AUTOINC"))
> +
> oe.path.copyhardlinktree(src, dest)
> }
My instinct here is to drop this oe_nohash file and instead, run a
sed -i -e "s@${PRSERV_PV_AUTOINC}@val@g" -e "s@${PRAUTO}@val2@g"
over the files in dest to expand out the values.
You can then drop much of the other changes trying to expand these
things out. We don't know what out of tree things are also using the
pkgdata files and I'm not sure we should force everything through the
API either.
Cheers,
Richard
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 2/5] hash equivalency and pr service
2020-08-24 23:29 ` [master][PATCH 2/5] hash equivalency and pr service Mark Hatle
@ 2020-08-25 12:50 ` Joshua Watt
2020-08-25 14:16 ` Mark Hatle
0 siblings, 1 reply; 18+ messages in thread
From: Joshua Watt @ 2020-08-25 12:50 UTC (permalink / raw)
To: Mark Hatle; +Cc: OE-core
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" ?
> +
> 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
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled
2020-08-25 10:56 ` [OE-core] " Richard Purdie
@ 2020-08-25 14:14 ` Mark Hatle
[not found] ` <162E885FFA00831B.12036@lists.openembedded.org>
1 sibling, 0 replies; 18+ messages in thread
From: Mark Hatle @ 2020-08-25 14:14 UTC (permalink / raw)
To: Richard Purdie, openembedded-core
On 8/25/20 5:56 AM, Richard Purdie wrote:
> On Mon, 2020-08-24 at 18:29 -0500, Mark Hatle wrote:
>> A related change was also needed for the kernel.bbclass. The kernel
>>
>> needs to get the AUTOINC values used by the various PV/PKGV, but with
>> this new system we don't want to call auto_pr each time -- instead
>> call the read_subpackage_metadata to ensure a consistent result.
>>
>> This also fixes an issue in the old version where the kernel
>> sometimes
>> caused the PR to be incremented twice, as the auto_pr was called
>> multiple times from different tasks.
>
> [...]
>
>>
>> diff --git a/meta/classes/kernel.bbclass
>> b/meta/classes/kernel.bbclass
>> index e2ceb6a333..4449452065 100644
>> --- a/meta/classes/kernel.bbclass
>> +++ b/meta/classes/kernel.bbclass
>> @@ -416,7 +416,7 @@ kernel_do_install() {
>> install -d ${D}${sysconfdir}/modules-load.d
>> install -d ${D}${sysconfdir}/modprobe.d
>> }
>> -do_install[prefuncs] += "package_get_auto_pr"
>> +do_install[prefuncs] += "read_subpackage_metadata"
>>
>> # Must be ran no earlier than after do_kernel_checkout or else
>> Makefile won't be in ${S}/Makefile
>> do_kernel_version_sanity_check() {
>> @@ -749,7 +749,7 @@ kernel_do_deploy() {
>> done
>> fi
>> }
>> -do_deploy[prefuncs] += "package_get_auto_pr"
>> +do_deploy[prefuncs] += "read_subpackage_metadata"
>>
>> addtask deploy after do_populate_sysroot do_packagedata
>
> I don't think this can be right. I understand why the kernel was
> expanding these early but at this point do_package hasn't happened so
> reading the subpackage metadata is a null op at best and potentially
> changes a lot more in the data store.
>
> I suspect this is not doing the right thing and will be something that
> comes back to bite us so we need something more careful here and I'd
> like to better understand exactly what change to the variables
> do_install and do_deploy need.
I don't know exactly why it's doing this. But I believe it is so that the
PV/PKGV is expanded (AUTOINC translated). I don't see any other reason for it
to be expanded in either do_install or do_deploy.
Best I can find is:
kernel-artifact-names.bbclass:KERNEL_ARTIFACT_NAME ?=
"${PKGE}-${PKGV}-${PKGR}-${MACHINE}${IMAGE_VERSION_SUFFIX}"
(That is then used in other places.)
So the PKGV replacement is desired to be that early.
I'll look into it further.
--Mark
> Cheers,
>
> Richard
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 2/5] hash equivalency and pr service
2020-08-25 12:50 ` [OE-core] " Joshua Watt
@ 2020-08-25 14:16 ` Mark Hatle
2020-08-25 18:36 ` Richard Purdie
0 siblings, 1 reply; 18+ messages in thread
From: Mark Hatle @ 2020-08-25 14:16 UTC (permalink / raw)
To: Joshua Watt; +Cc: OE-core
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
>>
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled
[not found] ` <162E885FFA00831B.12036@lists.openembedded.org>
@ 2020-08-25 16:52 ` Mark Hatle
2020-08-25 17:04 ` Richard Purdie
0 siblings, 1 reply; 18+ messages in thread
From: Mark Hatle @ 2020-08-25 16:52 UTC (permalink / raw)
To: Richard Purdie, openembedded-core, Bruce Ashfield
On 8/25/20 9:14 AM, Mark Hatle wrote:
>
>
> On 8/25/20 5:56 AM, Richard Purdie wrote:
>> On Mon, 2020-08-24 at 18:29 -0500, Mark Hatle wrote:
>>> A related change was also needed for the kernel.bbclass. The kernel
>>>
>>> needs to get the AUTOINC values used by the various PV/PKGV, but with
>>> this new system we don't want to call auto_pr each time -- instead
>>> call the read_subpackage_metadata to ensure a consistent result.
>>>
>>> This also fixes an issue in the old version where the kernel
>>> sometimes
>>> caused the PR to be incremented twice, as the auto_pr was called
>>> multiple times from different tasks.
>>
>> [...]
>>
>>>
>>> diff --git a/meta/classes/kernel.bbclass
>>> b/meta/classes/kernel.bbclass
>>> index e2ceb6a333..4449452065 100644
>>> --- a/meta/classes/kernel.bbclass
>>> +++ b/meta/classes/kernel.bbclass
>>> @@ -416,7 +416,7 @@ kernel_do_install() {
>>> install -d ${D}${sysconfdir}/modules-load.d
>>> install -d ${D}${sysconfdir}/modprobe.d
>>> }
>>> -do_install[prefuncs] += "package_get_auto_pr"
>>> +do_install[prefuncs] += "read_subpackage_metadata"
I talked with Bruce on this. We see no reason do_install[prefuncs] is required
any longer.
It was when that line was written, but when the kernel was refactored to permit
multiple kernel builds and such the need for this went away.
>>> # Must be ran no earlier than after do_kernel_checkout or else
>>> Makefile won't be in ${S}/Makefile
>>> do_kernel_version_sanity_check() {
>>> @@ -749,7 +749,7 @@ kernel_do_deploy() {
>>> done
>>> fi
>>> }
>>> -do_deploy[prefuncs] += "package_get_auto_pr"
>>> +do_deploy[prefuncs] += "read_subpackage_metadata"
>>>
>>> addtask deploy after do_populate_sysroot do_packagedata
>>
>> I don't think this can be right. I understand why the kernel was
>> expanding these early but at this point do_package hasn't happened so
>> reading the subpackage metadata is a null op at best and potentially
>> changes a lot more in the data store.
>>
>> I suspect this is not doing the right thing and will be something that
>> comes back to bite us so we need something more careful here and I'd
>> like to better understand exactly what change to the variables
>> do_install and do_deploy need.
>
> I don't know exactly why it's doing this. But I believe it is so that the
> PV/PKGV is expanded (AUTOINC translated). I don't see any other reason for it
> to be expanded in either do_install or do_deploy.
>
> Best I can find is:
>
> kernel-artifact-names.bbclass:KERNEL_ARTIFACT_NAME ?=
> "${PKGE}-${PKGV}-${PKGR}-${MACHINE}${IMAGE_VERSION_SUFFIX}"
>
> (That is then used in other places.)
>
> So the PKGV replacement is desired to be that early.
>
> I'll look into it further.
In do_deploy, I see multiple usages of the KERNEL_ARTIFACT_NAME (sometimes with
alternative variables.. but in the end both PKGV and PKGR are used here.)
do_deploy is after do_package/do_packagedata (where the values are assigned and
stored). So it should be safe for us to use the read_subpackage_metadata (or
something new if desired) to read back in the stored PKGV (AUTOINC) and PKGR
(PRAUTO) values.
--Mark
> --Mark
>
>> Cheers,
>>
>> Richard
>>
>>
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled
2020-08-25 16:52 ` Mark Hatle
@ 2020-08-25 17:04 ` Richard Purdie
2020-08-25 22:10 ` Mark Hatle
0 siblings, 1 reply; 18+ messages in thread
From: Richard Purdie @ 2020-08-25 17:04 UTC (permalink / raw)
To: Mark Hatle, openembedded-core, Bruce Ashfield
On Tue, 2020-08-25 at 11:52 -0500, Mark Hatle wrote:
>
> On 8/25/20 9:14 AM, Mark Hatle wrote:
> >
> > On 8/25/20 5:56 AM, Richard Purdie wrote:
> > > On Mon, 2020-08-24 at 18:29 -0500, Mark Hatle wrote:
> > > > A related change was also needed for the kernel.bbclass. The
> > > > kernel
> > > >
> > > > needs to get the AUTOINC values used by the various PV/PKGV,
> > > > but with
> > > > this new system we don't want to call auto_pr each time --
> > > > instead
> > > > call the read_subpackage_metadata to ensure a consistent
> > > > result.
> > > >
> > > > This also fixes an issue in the old version where the kernel
> > > > sometimes
> > > > caused the PR to be incremented twice, as the auto_pr was
> > > > called
> > > > multiple times from different tasks.
> > >
> > > [...]
> > >
> > > > diff --git a/meta/classes/kernel.bbclass
> > > > b/meta/classes/kernel.bbclass
> > > > index e2ceb6a333..4449452065 100644
> > > > --- a/meta/classes/kernel.bbclass
> > > > +++ b/meta/classes/kernel.bbclass
> > > > @@ -416,7 +416,7 @@ kernel_do_install() {
> > > > install -d ${D}${sysconfdir}/modules-load.d
> > > > install -d ${D}${sysconfdir}/modprobe.d
> > > > }
> > > > -do_install[prefuncs] += "package_get_auto_pr"
> > > > +do_install[prefuncs] += "read_subpackage_metadata"
>
> I talked with Bruce on this. We see no reason do_install[prefuncs]
> is required
> any longer.
>
> It was when that line was written, but when the kernel was refactored
> to permit multiple kernel builds and such the need for this went
> away.
Ok, lets start with a patch to remove that call.
> > > > # Must be ran no earlier than after do_kernel_checkout or else
> > > > Makefile won't be in ${S}/Makefile
> > > > do_kernel_version_sanity_check() {
> > > > @@ -749,7 +749,7 @@ kernel_do_deploy() {
> > > > done
> > > > fi
> > > > }
> > > > -do_deploy[prefuncs] += "package_get_auto_pr"
> > > > +do_deploy[prefuncs] += "read_subpackage_metadata"
> > > >
> > > > addtask deploy after do_populate_sysroot do_packagedata
> > >
> > > I don't think this can be right. I understand why the kernel was
> > > expanding these early but at this point do_package hasn't
> > > happened so
> > > reading the subpackage metadata is a null op at best and
> > > potentially
> > > changes a lot more in the data store.
> > >
> > > I suspect this is not doing the right thing and will be something
> > > that
> > > comes back to bite us so we need something more careful here and
> > > I'd
> > > like to better understand exactly what change to the variables
> > > do_install and do_deploy need.
> >
> > I don't know exactly why it's doing this. But I believe it is so
> > that the
> > PV/PKGV is expanded (AUTOINC translated). I don't see any other
> > reason for it
> > to be expanded in either do_install or do_deploy.
> >
> > Best I can find is:
> >
> > kernel-artifact-names.bbclass:KERNEL_ARTIFACT_NAME ?=
> > "${PKGE}-${PKGV}-${PKGR}-${MACHINE}${IMAGE_VERSION_SUFFIX}"
> >
> > (That is then used in other places.)
> >
> > So the PKGV replacement is desired to be that early.
> >
> > I'll look into it further.
>
> In do_deploy, I see multiple usages of the KERNEL_ARTIFACT_NAME
> (sometimes with
> alternative variables.. but in the end both PKGV and PKGR are used
> here.)
>
> do_deploy is after do_package/do_packagedata (where the values are
> assigned and
> stored). So it should be safe for us to use the
> read_subpackage_metadata (or
> something new if desired) to read back in the stored PKGV (AUTOINC)
> and PKGR
> (PRAUTO) values.
That sounds reasonable. FWIW do_deploy doesn't have to be after
do_package but we can arrange that in the kernel case.
I'd prefer to have a more granular function to call here than
read_subpackage_metadata, just to make it clear what is happening and
why.
Cheers,
Richard
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 2/5] hash equivalency and pr service
2020-08-25 14:16 ` Mark Hatle
@ 2020-08-25 18:36 ` Richard Purdie
0 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2020-08-25 18:36 UTC (permalink / raw)
To: Mark Hatle, Joshua Watt; +Cc: OE-core
On Tue, 2020-08-25 at 09:16 -0500, Mark Hatle wrote:
> 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:
> > > +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.
Except we do call this "outhash" in various places...
Cheers,
Richard
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [OE-core] [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled
2020-08-25 17:04 ` Richard Purdie
@ 2020-08-25 22:10 ` Mark Hatle
0 siblings, 0 replies; 18+ messages in thread
From: Mark Hatle @ 2020-08-25 22:10 UTC (permalink / raw)
To: Richard Purdie, openembedded-core, Bruce Ashfield
On 8/25/20 12:04 PM, Richard Purdie wrote:
> On Tue, 2020-08-25 at 11:52 -0500, Mark Hatle wrote:
>>
>> On 8/25/20 9:14 AM, Mark Hatle wrote:
>>>
>>> On 8/25/20 5:56 AM, Richard Purdie wrote:
>>>> On Mon, 2020-08-24 at 18:29 -0500, Mark Hatle wrote:
>>>>> A related change was also needed for the kernel.bbclass. The
>>>>> kernel
>>>>>
>>>>> needs to get the AUTOINC values used by the various PV/PKGV,
>>>>> but with
>>>>> this new system we don't want to call auto_pr each time --
>>>>> instead
>>>>> call the read_subpackage_metadata to ensure a consistent
>>>>> result.
>>>>>
>>>>> This also fixes an issue in the old version where the kernel
>>>>> sometimes
>>>>> caused the PR to be incremented twice, as the auto_pr was
>>>>> called
>>>>> multiple times from different tasks.
>>>>
>>>> [...]
>>>>
>>>>> diff --git a/meta/classes/kernel.bbclass
>>>>> b/meta/classes/kernel.bbclass
>>>>> index e2ceb6a333..4449452065 100644
>>>>> --- a/meta/classes/kernel.bbclass
>>>>> +++ b/meta/classes/kernel.bbclass
>>>>> @@ -416,7 +416,7 @@ kernel_do_install() {
>>>>> install -d ${D}${sysconfdir}/modules-load.d
>>>>> install -d ${D}${sysconfdir}/modprobe.d
>>>>> }
>>>>> -do_install[prefuncs] += "package_get_auto_pr"
>>>>> +do_install[prefuncs] += "read_subpackage_metadata"
>>
>> I talked with Bruce on this. We see no reason do_install[prefuncs]
>> is required
>> any longer.
>>
>> It was when that line was written, but when the kernel was refactored
>> to permit multiple kernel builds and such the need for this went
>> away.
>
> Ok, lets start with a patch to remove that call.
>
>>>>> # Must be ran no earlier than after do_kernel_checkout or else
>>>>> Makefile won't be in ${S}/Makefile
>>>>> do_kernel_version_sanity_check() {
>>>>> @@ -749,7 +749,7 @@ kernel_do_deploy() {
>>>>> done
>>>>> fi
>>>>> }
>>>>> -do_deploy[prefuncs] += "package_get_auto_pr"
>>>>> +do_deploy[prefuncs] += "read_subpackage_metadata"
>>>>>
>>>>> addtask deploy after do_populate_sysroot do_packagedata
>>>>
>>>> I don't think this can be right. I understand why the kernel was
>>>> expanding these early but at this point do_package hasn't
>>>> happened so
>>>> reading the subpackage metadata is a null op at best and
>>>> potentially
>>>> changes a lot more in the data store.
>>>>
>>>> I suspect this is not doing the right thing and will be something
>>>> that
>>>> comes back to bite us so we need something more careful here and
>>>> I'd
>>>> like to better understand exactly what change to the variables
>>>> do_install and do_deploy need.
>>>
>>> I don't know exactly why it's doing this. But I believe it is so
>>> that the
>>> PV/PKGV is expanded (AUTOINC translated). I don't see any other
>>> reason for it
>>> to be expanded in either do_install or do_deploy.
>>>
>>> Best I can find is:
>>>
>>> kernel-artifact-names.bbclass:KERNEL_ARTIFACT_NAME ?=
>>> "${PKGE}-${PKGV}-${PKGR}-${MACHINE}${IMAGE_VERSION_SUFFIX}"
>>>
>>> (That is then used in other places.)
>>>
>>> So the PKGV replacement is desired to be that early.
>>>
>>> I'll look into it further.
>>
>> In do_deploy, I see multiple usages of the KERNEL_ARTIFACT_NAME
>> (sometimes with
>> alternative variables.. but in the end both PKGV and PKGR are used
>> here.)
>>
>> do_deploy is after do_package/do_packagedata (where the values are
>> assigned and
>> stored). So it should be safe for us to use the
>> read_subpackage_metadata (or
>> something new if desired) to read back in the stored PKGV (AUTOINC)
>> and PKGR
>> (PRAUTO) values.
>
> That sounds reasonable. FWIW do_deploy doesn't have to be after
> do_package but we can arrange that in the kernel case.
Still working on the refactor, but the kernel.bbclass "do_deploy" runs after
do_packagedata, and do_packagedata is when the PKGV/PKGR dynamic items are
written. So it's safe to do it here.
All of the other users I found were after do_package (and didn't care about PKGV
and PKGR) or were after do_packagedata.
> I'd prefer to have a more granular function to call here than
> read_subpackage_metadata, just to make it clear what is happening and
> why.
My concern is that if we don't load all of the subpackage metadata, that the
PKGV and PKGR won't be loaded -exactly- they were they were defined in
(combination of) do_package and do_packagedata.
We can limit the load to just what was written by do_packagedata, but I don't
believe this is significant less efficient or will cause issues... and doing it
this way ensures we have a single routine for future modifications.
--Mark
> Cheers,
>
> Richard
>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-08-25 22:10 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-24 23:29 [master][PATCH 0/5] Allow PR Service and hash equiv together Mark Hatle
2020-08-24 23:29 ` [master][PATCH 1/5] package_tar.bbclass: Sync to the other package_* classes Mark Hatle
2020-08-25 10:49 ` [OE-core] " Richard Purdie
2020-08-24 23:29 ` [master][PATCH 2/5] hash equivalency and pr service Mark Hatle
2020-08-25 12:50 ` [OE-core] " Joshua Watt
2020-08-25 14:16 ` Mark Hatle
2020-08-25 18:36 ` Richard Purdie
2020-08-24 23:29 ` [master][PATCH 3/5] package.bbclass: Move package_get_auto_pr to packagedata.bbclass Mark Hatle
2020-08-24 23:29 ` [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled Mark Hatle
2020-08-25 10:56 ` [OE-core] " Richard Purdie
2020-08-25 14:14 ` Mark Hatle
[not found] ` <162E885FFA00831B.12036@lists.openembedded.org>
2020-08-25 16:52 ` Mark Hatle
2020-08-25 17:04 ` Richard Purdie
2020-08-25 22:10 ` Mark Hatle
2020-08-25 11:00 ` Richard Purdie
2020-08-24 23:29 ` [master][PATCH 5/5] buildhistory.bbclass: Rework to use read_subpackage_metadata Mark Hatle
2020-08-24 23:32 ` ✗ patchtest: failure for Allow PR Service and hash equiv together Patchwork
2020-08-25 1:17 ` Mark Hatle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox