diff mbox series

toaster: handle changes to "ImagePkgList" event

Message ID 20220318104304.18460-1-david.reyna@windriver.com
State New
Headers show
Series toaster: handle changes to "ImagePkgList" event | expand

Commit Message

Reyna, David March 18, 2022, 10:43 a.m. UTC 
From: David Reyna <David.Reyna@windriver.com>

There are several changes to the "ImagePkgList" content to work around:
1) The key field for the package size ('PKGSIZE') is currently an empty string
2) The list of files changed from 'FILES_INFO' to 'FILES'
3) The 'FILES' is now a dictionary in string form
4) Not all fields are always present for all packages, for example 'LICENSE' and 'SUMMARY'

[YOCTO #14763]

Signed-off-by: David Reyna <David.Reyna@windriver.com>
---
 bitbake/lib/bb/ui/buildinfohelper.py | 71 ++++++++++++++++++++--------
 1 file changed, 52 insertions(+), 19 deletions(-)

Comments

Richard Purdie March 18, 2022, 10:49 a.m. UTC | #1 
On Fri, 2022-03-18 at 03:43 -0700, Reyna, David wrote:
> From: David Reyna <David.Reyna@windriver.com>
> 
> There are several changes to the "ImagePkgList" content to work around:
> 1) The key field for the package size ('PKGSIZE') is currently an empty string
> 2) The list of files changed from 'FILES_INFO' to 'FILES'
> 3) The 'FILES' is now a dictionary in string form
> 4) Not all fields are always present for all packages, for example 'LICENSE' and 'SUMMARY'
> 
> [YOCTO #14763]
> 
> Signed-off-by: David Reyna <David.Reyna@windriver.com>

I'm not sure this looks entirely correct to me. FILES is the pre expanded
version of the files list so it could contain wildcards and FILES_INFO was meant
to be the version where the list of files was expanded out (and may have also
been tweaked by other package processing, e.g. creation of debug files).

I think the fact that some values are "disappearing" is a sign the overrides
change has some impact here which needs to be fixed and this patch is just
partially working around it.

Cheers,

Richard
Reyna, David March 18, 2022, 7:26 p.m. UTC | #2 
> I think the fact that some values are "disappearing" is a sign the overrides
change has some impact here which needs to be fixed and this patch is just
partially working around it.

Yes, this patch is specifically to work around the problems in the event data encountered. Fortunately these were the only events thus affected.

If the original data and formatting does get restored, you will observe that the patch is written to use that original formatting.

After the core problems are fixed, then this patch (or parts) can be dropped.

As a result of these discoveries, I filed Bug 14764 directly against Bitbake since there was obvious missing formatting with 'PKGSIZE'. As for the other problems, I could not tell if they were intentional changes or not.  I will update Bug 14764 so that it is one stop shopping.

David

-----Original Message-----
From: toaster@lists.yoctoproject.org <toaster@lists.yoctoproject.org> On Behalf Of Richard Purdie
Sent: Friday, March 18, 2022 3:50 AM
To: Reyna, David <david.reyna@windriver.com>; toaster@lists.yoctoproject.org
Subject: Re: [Toaster] [PATCH] toaster: handle changes to "ImagePkgList" event

On Fri, 2022-03-18 at 03:43 -0700, Reyna, David wrote:
> From: David Reyna <David.Reyna@windriver.com>
> 
> There are several changes to the "ImagePkgList" content to work around:
> 1) The key field for the package size ('PKGSIZE') is currently an empty string
> 2) The list of files changed from 'FILES_INFO' to 'FILES'
> 3) The 'FILES' is now a dictionary in string form
> 4) Not all fields are always present for all packages, for example 'LICENSE' and 'SUMMARY'
> 
> [YOCTO #14763]
> 
> Signed-off-by: David Reyna <David.Reyna@windriver.com>

I'm not sure this looks entirely correct to me. FILES is the pre expanded
version of the files list so it could contain wildcards and FILES_INFO was meant
to be the version where the list of files was expanded out (and may have also
been tweaked by other package processing, e.g. creation of debug files).

I think the fact that some values are "disappearing" is a sign the overrides
change has some impact here which needs to be fixed and this patch is just
partially working around it.

Cheers,

Richard
Richard Purdie March 19, 2022, 8:02 p.m. UTC | #3 
On Fri, 2022-03-18 at 19:26 +0000, Reyna, David wrote:
> > I think the fact that some values are "disappearing" is a sign the overrides
> change has some impact here which needs to be fixed and this patch is just
> partially working around it.
> 
> Yes, this patch is specifically to work around the problems in the event data
> encountered. Fortunately these were the only events thus affected.
> 
> If the original data and formatting does get restored, you will observe that the
> patch is written to use that original formatting.
> 
> After the core problems are fixed, then this patch (or parts) can be dropped.
> 
> As a result of these discoveries, I filed Bug 14764 directly against Bitbake
> since there was obvious missing formatting with 'PKGSIZE'. As for the other
> problems, I could not tell if they were intentional changes or not.  I will
> update Bug 14764 so that it is one stop shopping.

I think the problem might be fixed with something like:

diff --git a/meta/classes/toaster.bbclass b/meta/classes/toaster.bbclass
index dd5c7f224ba..f365c091420 100644
--- a/meta/classes/toaster.bbclass
+++ b/meta/classes/toaster.bbclass
@@ -101,11 +101,11 @@ def _toaster_load_pkgdatafile(dirpath, filepath):
         for line in fin:
             try:
                 kn, kv = line.strip().split(": ", 1)
-                m = re.match(r"^PKG_([^A-Z:]*)", kn)
+                m = re.match(r"^PKG:([^A-Z:]*)", kn)
                 if m:
                     pkgdata['OPKGN'] = m.group(1)
-                kn = "_".join([x for x in kn.split("_") if x.isupper()])
-                pkgdata[kn] = kv.strip()
+                kn = kn.split(":")[0]
+                pkgdata[kn] = kv
                 if kn.startswith('FILES_INFO'):
                     pkgdata[kn] = json.loads(kv)
 
but I haven't tested that as yet.

Cheers,

Richard
Reyna, David March 20, 2022, 7:14 a.m. UTC | #4 
I will try this out.

David

-----Original Message-----
From: Richard Purdie <richard.purdie@linuxfoundation.org> 
Sent: Saturday, March 19, 2022 1:02 PM
To: Reyna, David <david.reyna@windriver.com>; toaster@lists.yoctoproject.org
Subject: Re: [Toaster] [PATCH] toaster: handle changes to "ImagePkgList" event

On Fri, 2022-03-18 at 19:26 +0000, Reyna, David wrote:
> > I think the fact that some values are "disappearing" is a sign the overrides
> change has some impact here which needs to be fixed and this patch is just
> partially working around it.
> 
> Yes, this patch is specifically to work around the problems in the event data
> encountered. Fortunately these were the only events thus affected.
> 
> If the original data and formatting does get restored, you will observe that the
> patch is written to use that original formatting.
> 
> After the core problems are fixed, then this patch (or parts) can be dropped.
> 
> As a result of these discoveries, I filed Bug 14764 directly against Bitbake
> since there was obvious missing formatting with 'PKGSIZE'. As for the other
> problems, I could not tell if they were intentional changes or not.  I will
> update Bug 14764 so that it is one stop shopping.

I think the problem might be fixed with something like:

diff --git a/meta/classes/toaster.bbclass b/meta/classes/toaster.bbclass
index dd5c7f224ba..f365c091420 100644
--- a/meta/classes/toaster.bbclass
+++ b/meta/classes/toaster.bbclass
@@ -101,11 +101,11 @@ def _toaster_load_pkgdatafile(dirpath, filepath):
         for line in fin:
             try:
                 kn, kv = line.strip().split(": ", 1)
-                m = re.match(r"^PKG_([^A-Z:]*)", kn)
+                m = re.match(r"^PKG:([^A-Z:]*)", kn)
                 if m:
                     pkgdata['OPKGN'] = m.group(1)
-                kn = "_".join([x for x in kn.split("_") if x.isupper()])
-                pkgdata[kn] = kv.strip()
+                kn = kn.split(":")[0]
+                pkgdata[kn] = kv
                 if kn.startswith('FILES_INFO'):
                     pkgdata[kn] = json.loads(kv)
 
but I haven't tested that as yet.

Cheers,

Richard
Reyna, David March 20, 2022, 10:37 p.m. UTC | #5 
Hi Richard,

Your proposed patch works perfectly. All the anomalies disappeared. I will drop my workaround patch and submit your patch instead.

I do have a limitation in that I am only setup to submit patches to bitbake, and not to oe-core. Could someone point me to those instructions, or could you pass that patch to them directly?

Thanks!
David

-----Original Message-----
From: toaster@lists.yoctoproject.org <toaster@lists.yoctoproject.org> On Behalf Of Reyna, David
Sent: Sunday, March 20, 2022 12:15 AM
To: Richard Purdie <richard.purdie@linuxfoundation.org>; toaster@lists.yoctoproject.org
Subject: Re: [Toaster] [PATCH] toaster: handle changes to "ImagePkgList" event

I will try this out.

David

-----Original Message-----
From: Richard Purdie <richard.purdie@linuxfoundation.org> 
Sent: Saturday, March 19, 2022 1:02 PM
To: Reyna, David <david.reyna@windriver.com>; toaster@lists.yoctoproject.org
Subject: Re: [Toaster] [PATCH] toaster: handle changes to "ImagePkgList" event

On Fri, 2022-03-18 at 19:26 +0000, Reyna, David wrote:
> > I think the fact that some values are "disappearing" is a sign the overrides
> change has some impact here which needs to be fixed and this patch is just
> partially working around it.
> 
> Yes, this patch is specifically to work around the problems in the event data
> encountered. Fortunately these were the only events thus affected.
> 
> If the original data and formatting does get restored, you will observe that the
> patch is written to use that original formatting.
> 
> After the core problems are fixed, then this patch (or parts) can be dropped.
> 
> As a result of these discoveries, I filed Bug 14764 directly against Bitbake
> since there was obvious missing formatting with 'PKGSIZE'. As for the other
> problems, I could not tell if they were intentional changes or not.  I will
> update Bug 14764 so that it is one stop shopping.

I think the problem might be fixed with something like:

diff --git a/meta/classes/toaster.bbclass b/meta/classes/toaster.bbclass
index dd5c7f224ba..f365c091420 100644
--- a/meta/classes/toaster.bbclass
+++ b/meta/classes/toaster.bbclass
@@ -101,11 +101,11 @@ def _toaster_load_pkgdatafile(dirpath, filepath):
         for line in fin:
             try:
                 kn, kv = line.strip().split(": ", 1)
-                m = re.match(r"^PKG_([^A-Z:]*)", kn)
+                m = re.match(r"^PKG:([^A-Z:]*)", kn)
                 if m:
                     pkgdata['OPKGN'] = m.group(1)
-                kn = "_".join([x for x in kn.split("_") if x.isupper()])
-                pkgdata[kn] = kv.strip()
+                kn = kn.split(":")[0]
+                pkgdata[kn] = kv
                 if kn.startswith('FILES_INFO'):
                     pkgdata[kn] = json.loads(kv)
 
but I haven't tested that as yet.

Cheers,

Richard
Reyna, David March 21, 2022, 12:18 a.m. UTC | #6 
> I'm seeing builds reach 100% and then just hang in the toaster UI never completing.

See my recent patch for 14765 (attached).

I discovered that new race condition with long builds where the end-of-build event has the build record success outcome properly set, but that commit never makes it in time to the physical database before the build tear-down. It may be related to the upgrade to Django 3 (and if so it is the only oddity with Tim's Django 3 I have encountered).

David

-----Original Message-----
From: toaster@lists.yoctoproject.org <toaster@lists.yoctoproject.org> On Behalf Of Richard Purdie
Sent: Sunday, March 20, 2022 3:44 PM
To: Reyna, David <david.reyna@windriver.com>; toaster@lists.yoctoproject.org
Subject: Re: [Toaster] [PATCH] toaster: handle changes to "ImagePkgList" event

Hi David,

On Sun, 2022-03-20 at 22:37 +0000, Reyna, David wrote:
> Hi Richard,
> 
> Your proposed patch works perfectly. All the anomalies disappeared. I will drop my workaround patch and submit your patch instead.
> 
> I do have a limitation in that I am only setup to submit patches to bitbake, and not to oe-core. Could someone point me to those instructions, or could you pass that patch to them directly?

Thanks for testing, I'm glad it works and sorted the anomalies. I've sent it to
the OE-Core list and queued it for testing:

https://git.yoctoproject.org/poky/commit/?h=master-next&id=c94d49a769244e72df6631fdd4078cd1ad317e04

I did try and do some testing myself however I'm seeing builds reach 100% and
then just hang in the toaster UI never completing. It didn't matter if I ran
them from the UI or the commandline.

Have you seen anything like that before? It could be some issue with my local
setup or corruption from something previous, I'll try and dig a little more
tomorrow.

Cheers,

Richard
From: David Reyna <David.Reyna@windriver.com>

Force a sync point for end-of build event handler force
the build's outcome status commit, to resolve a race
condition with the build completion takedown.

[YOCTO #14765]

Signed-off-by: David Reyna <David.Reyna@windriver.com>
---
 bitbake/lib/bb/ui/buildinfohelper.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/bitbake/lib/bb/ui/buildinfohelper.py b/bitbake/lib/bb/ui/buildinfohelper.py
index 5559387d8e..ec036e3b14 100644
--- a/bitbake/lib/bb/ui/buildinfohelper.py
+++ b/bitbake/lib/bb/ui/buildinfohelper.py
@@ -228,6 +228,12 @@ class ORMWrapper(object):
         build.completed_on = timezone.now()
         build.outcome = outcome
         build.save()
+
+        # We force a sync point here to force the outcome status commit,
+        # which resolves a race condition with the build completion takedown
+        transaction.set_autocommit(True)
+        transaction.set_autocommit(False)
+
         signal_runbuilds()

     def update_target_set_license_manifest(self, target, license_manifest_path):
--
2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#5783): https://lists.yoctoproject.org/g/toaster/message/5783
Mute This Topic: https://lists.yoctoproject.org/mt/89883147/3616810
Group Owner: toaster+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/toaster/unsub [david.reyna@windriver.com]
-=-=-=-=-=-=-=-=-=-=-=-
Richard Purdie March 21, 2022, 3:20 p.m. UTC | #7 
On Mon, 2022-03-21 at 00:18 +0000, Reyna, David wrote:
> > I'm seeing builds reach 100% and then just hang in the toaster UI never
> > completing.
> 
> See my recent patch for 14765 (attached).
> 
> I discovered that new race condition with long builds where the end-of-build
> event has the build record success outcome properly set, but that commit never
> makes it in time to the physical database before the build tear-down. It may
> be related to the upgrade to Django 3 (and if so it is the only oddity with
> Tim's Django 3 I have encountered).

I thought I had that applied but I'd messed up and hadn't, it does work fine
with the patch applied.

I've merged the two bitbake changes and the OE-Core change so toaster is looking
much better on master now ready for kirkstone, thanks.!
Cheers,

Richard
diff mbox series

Patch

diff --git a/bitbake/lib/bb/ui/buildinfohelper.py b/bitbake/lib/bb/ui/buildinfohelper.py
index 8fef663469..5559387d8e 100644
--- a/bitbake/lib/bb/ui/buildinfohelper.py
+++ b/bitbake/lib/bb/ui/buildinfohelper.py
@@ -10,6 +10,7 @@  import sys
 import bb
 import re
 import os
+import json
 
 import django
 from django.utils import timezone
@@ -620,23 +621,39 @@  class ORMWrapper(object):
                     packagedict[p]['object'].version = pkgpnmap[p]['PV']
                     packagedict[p]['object'].installed_name = p
                     packagedict[p]['object'].revision = pkgpnmap[p]['PR']
-                    packagedict[p]['object'].license = pkgpnmap[p]['LICENSE']
-                    packagedict[p]['object'].section = pkgpnmap[p]['SECTION']
-                    packagedict[p]['object'].summary = pkgpnmap[p]['SUMMARY']
-                    packagedict[p]['object'].description = pkgpnmap[p]['DESCRIPTION']
-                    packagedict[p]['object'].size = int(pkgpnmap[p]['PKGSIZE'])
+                    packagedict[p]['object'].license = pkgpnmap[p]['LICENSE'] if 'LICENSE' in pkgpnmap[p] else ''
+                    packagedict[p]['object'].section = pkgpnmap[p]['SECTION'] if 'SECTION' in pkgpnmap[p] else ''
+                    packagedict[p]['object'].summary = pkgpnmap[p]['SUMMARY'] if 'SUMMARY' in pkgpnmap[p] else ''
+                    packagedict[p]['object'].description = pkgpnmap[p]['DESCRIPTION'] if 'DESCRIPTION' in pkgpnmap[p] else ''
+
+                    ### The name key for 'PKGSIZE' is currently a blank string
+                    if not 'PKGSIZE' in pkgpnmap[p]:
+                        if ('' in pkgpnmap[p]) and (pkgpnmap[p][''].isnumeric()):
+                            packagedict[p]['object'].size = int(pkgpnmap[p][''])
+                    else:
+                        packagedict[p]['object'].size = int(pkgpnmap[p]['PKGSIZE'])
 
-                # no files recorded for this package, so save files info
+                    # no files recorded for this package, so save files info
                     packagefile_objects = []
-                    for targetpath in pkgpnmap[p]['FILES_INFO']:
-                        targetfilesize = pkgpnmap[p]['FILES_INFO'][targetpath]
+
+                    ### Event change workaround: name change plus string instead of dict
+                    if 'FILES_INFO' in pkgpnmap[p]:
+                        key = 'FILES_INFO'
+                    else:
+                        key = 'FILES'
+                    pkg_dict = pkgpnmap[p][key]
+                    if isinstance(pkg_dict, str):
+                        pkg_dict = json.loads(pkgpnmap[p][key])
+
+                    for targetpath in pkg_dict:
+                        targetfilesize = pkg_dict[targetpath]
                         packagefile_objects.append(Package_File( package = packagedict[p]['object'],
                             path = targetpath,
                             size = targetfilesize))
                     if packagefile_objects:
                         Package_File.objects.bulk_create(packagefile_objects)
                 except KeyError as e:
-                    errormsg.append("  stpi: Key error, package %s key %s \n" % (p, e))
+                    errormsg.append("  stpi: Key error, package %s key %s map '%s'\n" % (p, e,pkgpnmap[p]))
 
             # save disk installed size
             packagedict[p]['object'].installed_size = packagedict[p]['size']
@@ -726,10 +743,13 @@  class ORMWrapper(object):
             return None
 
         # create and save the object
-        pname = package_info['PKG']
-        built_recipe = recipes[package_info['PN']]
         if 'OPKGN' in package_info.keys():
             pname = package_info['OPKGN']
+        elif 'PKG' in package_info.keys():
+            pname = package_info['PKG']
+        else:
+            pname = package_info['PN']
+        built_recipe = recipes[package_info['PN']]
 
         if built_package:
             bp_object, _ = Package.objects.get_or_create( build = build_obj,
@@ -750,23 +770,36 @@  class ORMWrapper(object):
                              "data package %s" % pname)
                 return
 
-        bp_object.installed_name = package_info['PKG']
+        bp_object.installed_name = pname # package_info['PKG']
         bp_object.recipe = recipe
         bp_object.version = package_info['PKGV']
         bp_object.revision = package_info['PKGR']
-        bp_object.summary = package_info['SUMMARY']
-        bp_object.description = package_info['DESCRIPTION']
-        bp_object.size = int(package_info['PKGSIZE'])
-        bp_object.section = package_info['SECTION']
-        bp_object.license = package_info['LICENSE']
+        bp_object.summary = package_info['SUMMARY'] if 'SUMMARY' in package_info else ''
+        bp_object.description = package_info['DESCRIPTION'] if 'DESCRIPTION' in package_info else ''
+        bp_object.size = int(package_info['PKGSIZE']) if 'PKGSIZE' in package_info else ''
+        bp_object.section = package_info['SECTION'] if 'SECTION' in package_info else ''
+        bp_object.license = package_info['LICENSE'] if 'LICENSE' in package_info else ''
+        # Workround...
+        if (not 'PKGSIZE' in package_info) and ('' in package_info):
+            bp_object.size = int(package_info[''])
         bp_object.save()
 
         # save any attached file information
         packagefile_objects = []
-        for path in package_info['FILES_INFO']:
+
+        ### Event change workaround
+        if 'FILES_INFO' in package_info:
+            key = 'FILES_INFO'
+        else:
+            key = 'FILES'
+        pkg_dict = package_info[key]
+        if isinstance(pkg_dict, str):
+            pkg_dict = json.loads(package_info[key])
+
+        for path in pkg_dict:
             packagefile_objects.append(Package_File( package = bp_object,
                                         path = path,
-                                        size = package_info['FILES_INFO'][path] ))
+                                        size = pkg_dict[path] ))
         if packagefile_objects:
             Package_File.objects.bulk_create(packagefile_objects)