All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] toaster: Artifact Selection Improvement
@ 2016-07-09  0:52 brian avery
  2016-07-09  0:52 ` [PATCH 1/7] runqueue: improve exception logging brian avery
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: brian avery @ 2016-07-09  0:52 UTC (permalink / raw)
  To: bitbake-devel; +Cc: brian avery

This is part of a patch set that is split over the openembedded-core and
bitbake-devel mailing lists.

This patch removes complexity from toaster.bbclass in order to move those
functions to bitbake/lib/toaster.

The intent of this set is to improve Toaster's ability to locate artifacts such as
kernel images, rootfs, and sdk tar files.

-Brian Avery
an Intel employee

The following changes since commit 308d0a8f1d06b22a9be8e5d55194544a2ee764ef:

  toaster: fix typo in arguments for libtoaster.js function (2016-07-08 16:25:39 -0700)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib poky-contrib/bavery/submit/elliot/toaster/8556-image_fstypes
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=bavery/submit/elliot/toaster/8556-image_fstypes

Ed Bartosh (1):
  runqueue: improve exception logging

Elliot Smith (6):
  toaster: display Target targets in build dashboard
  toaster: do image and artifact scan on BuildCompleted
  toaster: improve image file suffix retrieval
  toaster: attach kernel artifacts to targets
  buildinfohelper: fix retrieval of targets
  toaster: improve scan for SDK artifacts

 lib/bb/runqueue.py                                 |   6 +-
 lib/bb/ui/buildinfohelper.py                       | 370 ++++++++++++++++++---
 lib/bb/ui/toasterui.py                             |  11 +-
 .../migrations/0008_refactor_artifact_models.py    |  39 +++
 lib/toaster/orm/models.py                          | 243 ++++++++++----
 .../toastergui/templates/builddashboard.html       |  84 +++--
 .../toastergui/templatetags/field_values_filter.py |  18 +
 lib/toaster/toastergui/views.py                    |  18 +-
 8 files changed, 645 insertions(+), 144 deletions(-)
 create mode 100644 bitbake/lib/toaster/orm/migrations/0008_refactor_artifact_models.py
 create mode 100644 bitbake/lib/toaster/toastergui/templatetags/field_values_filter.py

--
1.9.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/7] runqueue: improve exception logging
  2016-07-09  0:52 [PATCH 0/7] toaster: Artifact Selection Improvement brian avery
@ 2016-07-09  0:52 ` brian avery
  2016-07-09  4:03   ` Christopher Larson
  2016-07-09  0:52 ` [PATCH 2/7] toaster: display Target targets in build dashboard brian avery
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: brian avery @ 2016-07-09  0:52 UTC (permalink / raw)
  To: bitbake-devel; +Cc: brian avery

From: Ed Bartosh <ed.bartosh@linux.intel.com>

Runqueue errors direct the user to view the "failure below",
but no additional error message is available.

Log the stacktrace so that the user can see what went wrong.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
Signed-off-by: brian avery <brian.avery@intel.com>
---
 lib/bb/runqueue.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/bb/runqueue.py b/lib/bb/runqueue.py
index 57be15a..04f2f4c 100644
--- a/lib/bb/runqueue.py
+++ b/lib/bb/runqueue.py
@@ -1219,8 +1219,10 @@ class RunQueue:
                 pass
             self.state = runQueueComplete
             raise
-        except:
-            logger.error("An uncaught exception occured in runqueue, please see the failure below:")
+        except Exception as err:
+            import traceback
+            logger.error("An uncaught exception occured in runqueue: '%s'" % err)
+            logger.error(traceback.format_exc())
             try:
                 self.teardown_workers()
             except:
--
1.9.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/7] toaster: display Target targets in build dashboard
  2016-07-09  0:52 [PATCH 0/7] toaster: Artifact Selection Improvement brian avery
  2016-07-09  0:52 ` [PATCH 1/7] runqueue: improve exception logging brian avery
@ 2016-07-09  0:52 ` brian avery
  2016-07-09  0:52 ` [PATCH 3/7] toaster: do image and artifact scan on BuildCompleted brian avery
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: brian avery @ 2016-07-09  0:52 UTC (permalink / raw)
  To: bitbake-devel; +Cc: brian avery

From: Elliot Smith <elliot.smith@intel.com>

The build dashboard was showing the targets for the build in the page
heading and title as "Target object".

Add a filter which extracts the "target" from each Target object
as a string so that the heading and title display correctly.

Also sort the image file suffixes alphabetically.

[YOCTO #8556]

Signed-off-by: Elliot Smith <elliot.smith@intel.com>
Signed-off-by: brian avery <brian.avery@intel.com>
---
 lib/toaster/toastergui/templates/builddashboard.html   |  9 +++++----
 .../toastergui/templatetags/field_values_filter.py     | 18 ++++++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 bitbake/lib/toaster/toastergui/templatetags/field_values_filter.py

diff --git a/lib/toaster/toastergui/templates/builddashboard.html b/lib/toaster/toastergui/templates/builddashboard.html
index 04a57ef..dcda2a8 100644
--- a/lib/toaster/toastergui/templates/builddashboard.html
+++ b/lib/toaster/toastergui/templates/builddashboard.html
@@ -1,8 +1,9 @@
 {% extends "basebuildpage.html" %}
 {% load humanize %}
 {% load projecttags %}
+{% load field_values_filter %}

-{% block title %} {{build.target_set.all|dictsort:"target"|join:", "}} {{build.machine}} - {{build.project.name}} - Toaster {% endblock %}
+{% block title %} {{build.get_sorted_target_list|field_values:"target"|join:", "}} {{build.machine}} - {{build.project.name}} - Toaster {% endblock %}
 {% block parentbreadcrumb %}
 {% if build.get_sorted_target_list.count > 0 %}
   {{build.get_sorted_target_list.0.target}}
@@ -15,7 +16,7 @@
 <!-- page title -->
 <div class="col-md-10">
  <div class="page-header build-data">
-     <h1>{{build.target_set.all|dictsort:"target"|join:", "}} {{build.machine}}</h1>
+     <h1>{{build.get_sorted_target_list|field_values:"target"|join:", "}} {{build.machine}}</h1>
  </div>

 <!-- build result bar -->
@@ -113,7 +114,7 @@
             </dt>
             <dd>
                 <ul class="list-unstyled">
-                    {% for i in target.imageFiles %}
+                    {% for i in target.imageFiles|dictsort:"suffix" %}
                         <li>
                             <a href="{% url 'build_artifact' build.pk 'imagefile' i.id %}">
                                 {{i.suffix}}
@@ -261,7 +262,7 @@
         if (location.href.search('#warnings') > -1) {
             $('#warning-info').addClass('in');
         }
-
+
 	//show warnings section when requested from the build outcome
 	$(".show-warnings").click(function() {
 	    $('#warning-info').addClass('in');
diff --git a/lib/toaster/toastergui/templatetags/field_values_filter.py b/lib/toaster/toastergui/templatetags/field_values_filter.py
new file mode 100644
index 0000000..5a73af7
--- /dev/null
+++ b/lib/toaster/toastergui/templatetags/field_values_filter.py
@@ -0,0 +1,18 @@
+from django import template
+
+register = template.Library()
+
+def field_values(iterable, field):
+    """
+    Convert an iterable of models into a list of strings, one for each model,
+    where the string for each model is the value of the field "field".
+    """
+    objects = []
+
+    if field:
+        for item in iterable:
+            objects.append(getattr(item, field))
+
+    return objects
+
+register.filter('field_values', field_values)
\ No newline at end of file
--
1.9.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/7] toaster: do image and artifact scan on BuildCompleted
  2016-07-09  0:52 [PATCH 0/7] toaster: Artifact Selection Improvement brian avery
  2016-07-09  0:52 ` [PATCH 1/7] runqueue: improve exception logging brian avery
  2016-07-09  0:52 ` [PATCH 2/7] toaster: display Target targets in build dashboard brian avery
@ 2016-07-09  0:52 ` brian avery
  2016-07-09  0:52 ` [PATCH 4/7] toaster: improve image file suffix retrieval brian avery
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: brian avery @ 2016-07-09  0:52 UTC (permalink / raw)
  To: bitbake-devel; +Cc: brian avery

From: Elliot Smith <elliot.smith@intel.com>

Move the image and artifact scan code from toaster.bbclass and
consolidate its logic with the existing logic in buildinfohelper.

Remove handler setup for events which used to be fired from
toaster.bbclass but which are now handled directly by buildinfohelper.

[YOCTO #8556]

Signed-off-by: Elliot Smith <elliot.smith@intel.com>
Signed-off-by: brian avery <brian.avery@intel.com>
---
 lib/bb/ui/buildinfohelper.py | 201 ++++++++++++++++++++++++++++++++++++++-----
 lib/bb/ui/toasterui.py       |   6 +-
 2 files changed, 181 insertions(+), 26 deletions(-)

diff --git a/lib/bb/ui/buildinfohelper.py b/lib/bb/ui/buildinfohelper.py
index 447670c..8bdc9cc 100644
--- a/lib/bb/ui/buildinfohelper.py
+++ b/lib/bb/ui/buildinfohelper.py
@@ -678,6 +678,16 @@ class ORMWrapper(object):
                             file_name = file_name,
                             file_size = file_size)

+    def save_artifact_information_no_dedupe(self, build_obj, file_name, file_size):
+        """
+        Save artifact information without checking for duplicate paths;
+        this is used when we are saving data about an artifact which was
+        generated by a previous build but which is also relevant to this build,
+        e.g. a bzImage file.
+        """
+        BuildArtifact.objects.create(build=build_obj, file_name=file_name,
+            file_size=file_size)
+
     def save_artifact_information(self, build_obj, file_name, file_size):
         # we skip the image files from other builds
         if Target_Image_File.objects.filter(file_name = file_name).count() > 0:
@@ -687,7 +697,8 @@ class ORMWrapper(object):
         if BuildArtifact.objects.filter(file_name = file_name).count() > 0:
             return

-        BuildArtifact.objects.create(build = build_obj, file_name = file_name, file_size = file_size)
+        self.save_artifact_information_no_dedupe(self, build_obj, file_name,
+            file_size)

     def create_logmessage(self, log_information):
         assert 'build' in log_information
@@ -1061,17 +1072,6 @@ class BuildInfoHelper(object):

         return self.brbe

-
-    def update_target_image_file(self, event):
-        evdata = BuildInfoHelper._get_data_from_event(event)
-
-        for t in self.internal_state['targets']:
-            if t.is_image == True:
-                output_files = list(evdata.keys())
-                for output in output_files:
-                    if t.target in output and 'rootfs' in output and not output.endswith(".manifest"):
-                        self.orm_wrapper.save_target_image_file_information(t, output, evdata[output])
-
     def update_artifact_image_file(self, event):
         evdata = BuildInfoHelper._get_data_from_event(event)
         for artifact_path in evdata.keys():
@@ -1081,16 +1081,6 @@ class BuildInfoHelper(object):
         if 'build' in self.internal_state:
             self.orm_wrapper.update_build_object(self.internal_state['build'], errors, warnings, taskfailures)

-
-    def store_license_manifest_path(self, event):
-        deploy_dir = BuildInfoHelper._get_data_from_event(event)['deploy_dir']
-        image_name = BuildInfoHelper._get_data_from_event(event)['image_name']
-        path = deploy_dir + "/licenses/" + image_name + "/license.manifest"
-        for target in self.internal_state['targets']:
-            if target.target in image_name:
-                self.orm_wrapper.update_target_set_license_manifest(target, path)
-
-
     def store_started_task(self, event):
         assert isinstance(event, (bb.runqueue.sceneQueueTaskStarted, bb.runqueue.runQueueTaskStarted, bb.runqueue.runQueueTaskSkipped))
         assert 'taskfile' in vars(event)
@@ -1506,6 +1496,173 @@ class BuildInfoHelper(object):

         self.orm_wrapper.create_logmessage(log_information)

+    def _get_files_from_image_license(self, image_license_manifest_path):
+        """
+        Find the FILES line in the image_license.manifest file,
+        which has the basenames of the bzImage and modules files
+        in this format:
+        FILES: bzImage--4.4.11+git0+3a5f494784_53e84104c5-r0-qemux86-20160603165040.bin modules--4.4.11+git0+3a5f494784_53e84104c5-r0-qemux86-20160603165040.tgz
+        """
+        files = []
+        with open(image_license_manifest_path) as image_license:
+            for line in image_license:
+                if line.startswith('FILES'):
+                    files_str = line.split(':')[1].strip()
+                    files_str = re.sub(r' {2,}', ' ', files_str)
+                    files = files_str.split(' ')
+        return files
+
+    def _endswith(self, str_to_test, endings):
+        """
+        Returns True if str ends with one of the strings in the list
+        endings, False otherwise
+        """
+        endswith = False
+        for ending in endings:
+            if str_to_test.endswith(ending):
+                endswith = True
+                break
+        return endswith
+
+    def _get_image_files(self, deploy_dir_image, image_name, image_file_extensions):
+        """
+        Find files in deploy_dir_image whose basename starts with the
+        string image_name and ends with one of the strings in
+        image_file_extensions.
+
+        Returns a list of file dictionaries like
+
+        [
+            {
+                'path': '/path/to/image/file',
+                'size': <file size in bytes>
+            }
+        ]
+        """
+        image_files = []
+
+        for dirpath, _, filenames in os.walk(deploy_dir_image):
+            for filename in filenames:
+                if filename.startswith(image_name) and \
+                self._endswith(filename, image_file_extensions):
+                    image_file_path = os.path.join(dirpath, filename)
+                    image_file_size = os.stat(image_file_path).st_size
+
+                    image_files.append({
+                        'path': image_file_path,
+                        'size': image_file_size
+                    })
+
+        return image_files
+
+    def scan_build_artifacts(self):
+        """
+        Scan for build artifacts in DEPLOY_DIR_IMAGE and associate them
+        with a Target object in self.internal_state['targets'].
+
+        We have two situations to handle:
+
+        1. This is the first time a target + machine has been built, so
+        add files from the DEPLOY_DIR_IMAGE to the target.
+
+        OR
+
+        2. There are no files for the target, so copy them from a
+        previous build with the same target + machine.
+        """
+        deploy_dir_image = \
+            self.server.runCommand(['getVariable', 'DEPLOY_DIR_IMAGE'])[0]
+
+        # if there's no DEPLOY_DIR_IMAGE, there aren't going to be
+        # any build artifacts, so we can return immediately
+        if not deploy_dir_image:
+            return
+
+        buildname = self.server.runCommand(['getVariable', 'BUILDNAME'])[0]
+        machine =  self.server.runCommand(['getVariable', 'MACHINE'])[0]
+        image_name = self.server.runCommand(['getVariable', 'IMAGE_NAME'])[0]
+
+        # location of the image_license.manifest files for this build;
+        # note that this file is only produced if an image is produced
+        license_directory = \
+            self.server.runCommand(['getVariable', 'LICENSE_DIRECTORY'])[0]
+
+        # file name extensions for image files
+        image_file_extensions_unique = {}
+        image_fstypes = self.server.runCommand(
+            ['getVariable', 'IMAGE_FSTYPES'])[0]
+        if image_fstypes != None:
+            image_types_str = image_fstypes.strip()
+            image_file_extensions = re.sub(r' {2,}', ' ', image_types_str)
+            image_file_extensions_unique = set(image_file_extensions.split(' '))
+
+        targets = self.internal_state['targets']
+        image_targets = [target for target in targets if target.is_image]
+        for target in image_targets:
+            # this is set to True if we find at least one file relating to
+            # this target; if this remains False after the scan, we copy the
+            # files from the most-recent Target with the same target + machine
+            # onto this Target instead
+            has_files = False
+
+            # we construct this because by the time we reach
+            # BuildCompleted, this has reset to
+            # 'defaultpkgname-<MACHINE>-<BUILDNAME>';
+            # we need to change it to
+            # <TARGET>-<MACHINE>-<BUILDNAME>
+            real_image_name = re.sub(r'^defaultpkgname', target.target,
+                image_name)
+
+            image_license_manifest_path = os.path.join(
+                license_directory,
+                real_image_name,
+                'image_license.manifest')
+
+            # if image_license.manifest exists, we can read the names of bzImage
+            # and modules files for this build from it, then look for them
+            # in the DEPLOY_DIR_IMAGE; note that this file is only produced
+            # if an image file was produced
+            if os.path.isfile(image_license_manifest_path):
+                has_files = True
+
+                basenames = self._get_files_from_image_license(
+                    image_license_manifest_path)
+
+                for basename in basenames:
+                    artifact_path = os.path.join(deploy_dir_image, basename)
+                    artifact_size = os.stat(artifact_path).st_size
+
+                    self.orm_wrapper.save_artifact_information_no_dedupe(
+                        self.internal_state['build'], artifact_path,
+                        artifact_size)
+
+                # store the license manifest path on the target
+                # (this file is also created any time an image file is created)
+                license_path = os.path.join(license_directory,
+                    real_image_name, 'license.manifest')
+
+                self.orm_wrapper.update_target_set_license_manifest(target,
+                    license_path)
+
+            # scan the directory for image files relating to this build
+            # (via real_image_name); note that we don't have to set
+            # has_files = True, as searching for the license manifest file
+            # will already have set it to true if at least one image file was
+            # produced
+            image_files = self._get_image_files(deploy_dir_image,
+                real_image_name, image_file_extensions_unique)
+
+            for image_file in image_files:
+                self.orm_wrapper.save_target_image_file_information(
+                    target, image_file['path'], image_file['size'])
+
+            if not has_files:
+                # TODO copy artifact and image files from the
+                # most-recently-built Target with the same target + machine
+                # as this Target; also copy the license manifest path,
+                # as that is treated differently
+                pass
+
     def close(self, errorcode):
         if self.brbe is not None:
             self._store_build_done(errorcode)
diff --git a/lib/bb/ui/toasterui.py b/lib/bb/ui/toasterui.py
index 5382935..d8bccdb 100644
--- a/lib/bb/ui/toasterui.py
+++ b/lib/bb/ui/toasterui.py
@@ -363,6 +363,8 @@ def main(server, eventHandler, params):
                     errors += 1
                     errorcode = 1
                     logger.error("Command execution failed: %s", event.error)
+                elif isinstance(event, bb.event.BuildCompleted):
+                    buildinfohelper.scan_build_artifacts()

                 # turn off logging to the current build log
                 _close_build_log(build_log)
@@ -410,12 +412,8 @@ def main(server, eventHandler, params):
                     buildinfohelper.store_target_package_data(event)
                 elif event.type == "MissedSstate":
                     buildinfohelper.store_missed_state_tasks(event)
-                elif event.type == "ImageFileSize":
-                    buildinfohelper.update_target_image_file(event)
                 elif event.type == "ArtifactFileSize":
                     buildinfohelper.update_artifact_image_file(event)
-                elif event.type == "LicenseManifestPath":
-                    buildinfohelper.store_license_manifest_path(event)
                 elif event.type == "SetBRBE":
                     buildinfohelper.brbe = buildinfohelper._get_data_from_event(event)
                 elif event.type == "OSErrorException":
--
1.9.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/7] toaster: improve image file suffix retrieval
  2016-07-09  0:52 [PATCH 0/7] toaster: Artifact Selection Improvement brian avery
                   ` (2 preceding siblings ...)
  2016-07-09  0:52 ` [PATCH 3/7] toaster: do image and artifact scan on BuildCompleted brian avery
@ 2016-07-09  0:52 ` brian avery
  2016-07-09  0:52 ` [PATCH 5/7] toaster: attach kernel artifacts to targets brian avery
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: brian avery @ 2016-07-09  0:52 UTC (permalink / raw)
  To: bitbake-devel; +Cc: brian avery

From: Elliot Smith <elliot.smith@intel.com>

Refactor retrieval of suffix from image file path, making it a
a method on Target_Image_File. This makes it easier to use this
in the build dashboard for individual images, plus reduces the
complexity of the code required to get all of the image file
suffixes for a build.

Signed-off-by: Elliot Smith <elliot.smith@intel.com>
Signed-off-by: brian avery <brian.avery@intel.com>
---
 lib/toaster/orm/models.py | 48 +++++++++++------------------------------------
 1 file changed, 11 insertions(+), 37 deletions(-)

diff --git a/lib/toaster/orm/models.py b/lib/toaster/orm/models.py
index 61737c7..40cdb9e 100644
--- a/lib/toaster/orm/models.py
+++ b/lib/toaster/orm/models.py
@@ -440,51 +440,21 @@ class Build(models.Model):
         """
         Get list of file name extensions for images produced by this build
         """
-        targets = Target.objects.filter(build_id = self.id)
         extensions = []

-        # pattern to match against file path for building extension string
-        pattern = re.compile('\.([^\.]+?)$')
-
+        targets = Target.objects.filter(build_id = self.id)
         for target in targets:
             if (not target.is_image):
                 continue

-            target_image_files = Target_Image_File.objects.filter(target_id = target.id)
+            target_image_files = Target_Image_File.objects.filter(
+                target_id=target.id)

             for target_image_file in target_image_files:
-                file_name = os.path.basename(target_image_file.file_name)
-                suffix = ''
-
-                continue_matching = True
-
-                # incrementally extract the suffix from the file path,
-                # checking it against the list of valid suffixes at each
-                # step; if the path is stripped of all potential suffix
-                # parts without matching a valid suffix, this returns all
-                # characters after the first '.' in the file name
-                while continue_matching:
-                    matches = pattern.search(file_name)
-
-                    if None == matches:
-                        continue_matching = False
-                        suffix = re.sub('^\.', '', suffix)
-                        continue
-                    else:
-                        suffix = matches.group(1) + suffix
-
-                    if suffix in Target_Image_File.SUFFIXES:
-                        continue_matching = False
-                        continue
-                    else:
-                        # reduce the file name and try to find the next
-                        # segment from the path which might be part
-                        # of the suffix
-                        file_name = re.sub('.' + matches.group(1), '', file_name)
-                        suffix = '.' + suffix
-
-                if not suffix in extensions:
-                    extensions.append(suffix)
+                extensions.append(target_image_file.suffix)
+
+        extensions = list(set(extensions))
+        extensions.sort()

         return ', '.join(extensions)

@@ -658,6 +628,10 @@ class Target_Image_File(models.Model):

     @property
     def suffix(self):
+        for suffix in Target_Image_File.SUFFIXES:
+            if self.file_name.endswith(suffix):
+                return suffix
+
         filename, suffix = os.path.splitext(self.file_name)
         suffix = suffix.lstrip('.')
         return suffix
--
1.9.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/7] toaster: attach kernel artifacts to targets
  2016-07-09  0:52 [PATCH 0/7] toaster: Artifact Selection Improvement brian avery
                   ` (3 preceding siblings ...)
  2016-07-09  0:52 ` [PATCH 4/7] toaster: improve image file suffix retrieval brian avery
@ 2016-07-09  0:52 ` brian avery
  2016-07-09  0:52 ` [PATCH 6/7] buildinfohelper: fix retrieval of targets brian avery
  2016-07-09  0:52 ` [PATCH 7/7] toaster: improve scan for SDK artifacts brian avery
  6 siblings, 0 replies; 11+ messages in thread
From: brian avery @ 2016-07-09  0:52 UTC (permalink / raw)
  To: bitbake-devel; +Cc: brian avery

From: Elliot Smith <elliot.smith@intel.com>

The bzImage and modules files were previously attached to a build,
rather than to the target which produced them. This meant it was
not possible to determine which kernel artifact produced by a
build came from which target; which in turn made it difficult to
associate existing kernel artifact with targets when those
targets didn't produce artifacts (e.g. if the same machine + target
combination was built again and didn't produce a bzImage or modules
file because those files already existed).

By associating kernel artifacts with the target (via a new
TargetArtifactFile model), we make it possible to find all
the artifacts for a given machine + target combination. Then, in
cases where a build is completed but its targets don't produce
any artifacts, we can find a previous Target object with the same
machine + target and copy its artifacts to the targets for a
just-completed build.

Note that this doesn't cover SDK artifacts yet, which are still
retrieved in toaster.bbclass and show up as "Other artifacts",
lumped together for the whole build rather than by target.

[YOCTO #8556]

Signed-off-by: Elliot Smith <elliot.smith@intel.com>
Signed-off-by: brian avery <brian.avery@intel.com>
---
 lib/bb/ui/buildinfohelper.py                       |  85 +++++++++-----
 .../orm/migrations/0008_targetartifactfile.py      |  23 ++++
 lib/toaster/orm/models.py                          | 123 ++++++++++++++++++++-
 .../toastergui/templates/builddashboard.html       |  15 ++-
 lib/toaster/toastergui/views.py                    |   7 ++
 5 files changed, 221 insertions(+), 32 deletions(-)
 create mode 100644 bitbake/lib/toaster/orm/migrations/0008_targetartifactfile.py

diff --git a/lib/bb/ui/buildinfohelper.py b/lib/bb/ui/buildinfohelper.py
index 8bdc9cc..a5b2237 100644
--- a/lib/bb/ui/buildinfohelper.py
+++ b/lib/bb/ui/buildinfohelper.py
@@ -37,7 +37,7 @@ os.environ["DJANGO_SETTINGS_MODULE"] =\
 django.setup()

 from orm.models import Build, Task, Recipe, Layer_Version, Layer, Target, LogMessage, HelpText
-from orm.models import Target_Image_File, BuildArtifact
+from orm.models import Target_Image_File, BuildArtifact, TargetArtifactFile
 from orm.models import Variable, VariableHistory
 from orm.models import Package, Package_File, Target_Installed_Package, Target_File
 from orm.models import Task_Dependency, Package_Dependency
@@ -121,6 +121,13 @@ class ORMWrapper(object):

         return vars(self)[dictname][key]

+    def get_similar_target_with_image_files(self, target):
+        """
+        Get a Target object "similar" to target; i.e. with the same target
+        name ('core-image-minimal' etc.) and machine.
+        """
+        return target.get_similar_target_with_image_files()
+
     def _timestamp_to_datetime(self, secs):
         """
         Convert timestamp in seconds to Python datetime
@@ -678,27 +685,32 @@ class ORMWrapper(object):
                             file_name = file_name,
                             file_size = file_size)

-    def save_artifact_information_no_dedupe(self, build_obj, file_name, file_size):
+    def save_target_artifact_file(self, target_obj, file_name, file_size):
         """
-        Save artifact information without checking for duplicate paths;
-        this is used when we are saving data about an artifact which was
-        generated by a previous build but which is also relevant to this build,
-        e.g. a bzImage file.
+        Save artifact file information for a Target target_obj.
+
+        Note that this doesn't include SDK artifacts, only images and
+        related files (e.g. bzImage).
         """
-        BuildArtifact.objects.create(build=build_obj, file_name=file_name,
-            file_size=file_size)
+        TargetArtifactFile.objects.create(target=target_obj,
+            file_name=file_name, file_size=file_size)

     def save_artifact_information(self, build_obj, file_name, file_size):
-        # we skip the image files from other builds
-        if Target_Image_File.objects.filter(file_name = file_name).count() > 0:
-            return
-
+        """
+        TODO this is currently used to save SDK artifacts to the database,
+        but will be replaced in future once SDK artifacts are associated with
+        Target objects (as they eventually should be)
+        """
         # do not update artifacts found in other builds
         if BuildArtifact.objects.filter(file_name = file_name).count() > 0:
             return

-        self.save_artifact_information_no_dedupe(self, build_obj, file_name,
-            file_size)
+        # do not update artifact if already a target artifact file for that path
+        if TargetArtifactFile.objects.filter(file_name = file_name).count() > 0:
+            return
+
+        BuildArtifact.objects.create(build=build_obj, file_name=file_name,
+            file_size=file_size)

     def create_logmessage(self, log_information):
         assert 'build' in log_information
@@ -1496,7 +1508,7 @@ class BuildInfoHelper(object):

         self.orm_wrapper.create_logmessage(log_information)

-    def _get_files_from_image_license(self, image_license_manifest_path):
+    def _get_filenames_from_image_license(self, image_license_manifest_path):
         """
         Find the FILES line in the image_license.manifest file,
         which has the basenames of the bzImage and modules files
@@ -1567,19 +1579,20 @@ class BuildInfoHelper(object):

         OR

-        2. There are no files for the target, so copy them from a
-        previous build with the same target + machine.
+        2. There are no new files for the target (they were already produced by
+        a previous build), so copy them from the most recent previous build with
+        the same target, task and machine.
         """
         deploy_dir_image = \
             self.server.runCommand(['getVariable', 'DEPLOY_DIR_IMAGE'])[0]

         # if there's no DEPLOY_DIR_IMAGE, there aren't going to be
-        # any build artifacts, so we can return immediately
+        # any image artifacts, so we can return immediately
         if not deploy_dir_image:
             return

         buildname = self.server.runCommand(['getVariable', 'BUILDNAME'])[0]
-        machine =  self.server.runCommand(['getVariable', 'MACHINE'])[0]
+        machine = self.server.runCommand(['getVariable', 'MACHINE'])[0]
         image_name = self.server.runCommand(['getVariable', 'IMAGE_NAME'])[0]

         # location of the image_license.manifest files for this build;
@@ -1597,7 +1610,10 @@ class BuildInfoHelper(object):
             image_file_extensions_unique = set(image_file_extensions.split(' '))

         targets = self.internal_state['targets']
+
+        # filter out anything which isn't an image target
         image_targets = [target for target in targets if target.is_image]
+
         for target in image_targets:
             # this is set to True if we find at least one file relating to
             # this target; if this remains False after the scan, we copy the
@@ -1625,16 +1641,17 @@ class BuildInfoHelper(object):
             if os.path.isfile(image_license_manifest_path):
                 has_files = True

-                basenames = self._get_files_from_image_license(
+                basenames = self._get_filenames_from_image_license(
                     image_license_manifest_path)

                 for basename in basenames:
                     artifact_path = os.path.join(deploy_dir_image, basename)
                     artifact_size = os.stat(artifact_path).st_size

-                    self.orm_wrapper.save_artifact_information_no_dedupe(
-                        self.internal_state['build'], artifact_path,
-                        artifact_size)
+                    # note that the artifact will only be saved against this
+                    # build if it hasn't been already
+                    self.orm_wrapper.save_target_artifact_file(target,
+                        artifact_path, artifact_size)

                 # store the license manifest path on the target
                 # (this file is also created any time an image file is created)
@@ -1648,7 +1665,10 @@ class BuildInfoHelper(object):
             # (via real_image_name); note that we don't have to set
             # has_files = True, as searching for the license manifest file
             # will already have set it to true if at least one image file was
-            # produced
+            # produced; note that the real_image_name includes BUILDNAME, which
+            # in turn includes a timestamp; so if no files were produced for
+            # this timestamp (i.e. the build reused existing image files already
+            # in the directory), no files will be recorded against this target
             image_files = self._get_image_files(deploy_dir_image,
                 real_image_name, image_file_extensions_unique)

@@ -1657,11 +1677,18 @@ class BuildInfoHelper(object):
                     target, image_file['path'], image_file['size'])

             if not has_files:
-                # TODO copy artifact and image files from the
-                # most-recently-built Target with the same target + machine
-                # as this Target; also copy the license manifest path,
-                # as that is treated differently
-                pass
+                # copy image files and build artifacts from the
+                # most-recently-built Target with the
+                # same target + machine as this Target; also copy the license
+                # manifest path, as that is not treated as an artifact and needs
+                # to be set separately
+                most_recent = \
+                    self.orm_wrapper.get_similar_target_with_image_files(target)
+
+                if most_recent:
+                    logger.info('image artifacts for target %s cloned from ' \
+                        'target %s' % (target.pk, most_recent.pk))
+                    target.clone_artifacts_from(most_recent)

     def close(self, errorcode):
         if self.brbe is not None:
diff --git a/lib/toaster/orm/migrations/0008_targetartifactfile.py b/lib/toaster/orm/migrations/0008_targetartifactfile.py
new file mode 100644
index 0000000..9f1d9bf
--- /dev/null
+++ b/lib/toaster/orm/migrations/0008_targetartifactfile.py
@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('orm', '0007_auto_20160523_1446'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='TargetArtifactFile',
+            fields=[
+                ('id', models.AutoField(primary_key=True, serialize=False, auto_created=True, verbose_name='ID')),
+                ('file_name', models.FilePathField()),
+                ('file_size', models.IntegerField()),
+                ('target', models.ForeignKey(to='orm.Target')),
+            ],
+        ),
+    ]
diff --git a/lib/toaster/orm/models.py b/lib/toaster/orm/models.py
index 40cdb9e..9edbef3 100644
--- a/lib/toaster/orm/models.py
+++ b/lib/toaster/orm/models.py
@@ -22,7 +22,7 @@
 from __future__ import unicode_literals

 from django.db import models, IntegrityError
-from django.db.models import F, Q, Avg, Max, Sum
+from django.db.models import F, Q, Avg, Max, Sum, Count
 from django.utils import timezone
 from django.utils.encoding import force_bytes

@@ -438,7 +438,9 @@ class Build(models.Model):

     def get_image_file_extensions(self):
         """
-        Get list of file name extensions for images produced by this build
+        Get list of file name extensions for images produced by this build;
+        note that this is the actual list of extensions stored on Target objects
+        for this build, and not the value of IMAGE_FSTYPES.
         """
         extensions = []

@@ -458,6 +460,15 @@ class Build(models.Model):

         return ', '.join(extensions)

+    def get_image_fstypes(self):
+        """
+        Get the IMAGE_FSTYPES variable value for this build as a de-duplicated
+        list of image file suffixes.
+        """
+        image_fstypes = Variable.objects.get(
+            build=self, variable_name='IMAGE_FSTYPES').variable_value
+        return list(set(re.split(r' {1,}', image_fstypes)))
+
     def get_sorted_target_list(self):
         tgts = Target.objects.filter(build_id = self.id).order_by( 'target' );
         return( tgts );
@@ -612,6 +623,114 @@ class Target(models.Model):
     def __unicode__(self):
         return self.target

+    def get_similar_targets(self):
+        """
+        Get targets for the same machine, task and target name
+        (e.g. 'core-image-minimal') from a successful build for this project
+        (but excluding this target).
+
+        Note that we look for targets built by this project because projects
+        can have different configurations from each other, and put their
+        artifacts in different directories.
+        """
+        query = ~Q(pk=self.pk) & \
+            Q(target=self.target) & \
+            Q(build__machine=self.build.machine) & \
+            Q(build__outcome=Build.SUCCEEDED) & \
+            Q(build__project=self.build.project)
+
+        return Target.objects.filter(query)
+
+    def get_similar_target_with_image_files(self):
+        """
+        Get the most recent similar target with Target_Image_Files associated
+        with it, for the purpose of cloning those files onto this target.
+        """
+        similar_target = None
+
+        candidates = self.get_similar_targets()
+        if candidates.count() < 1:
+            return similar_target
+
+        task_subquery = Q(task=self.task)
+
+        # we can look for a 'build' task if this task is a 'populate_sdk_ext'
+        # task, as it will have created images; and vice versa; note that
+        # 'build' targets can have their task set to '';
+        # also note that 'populate_sdk' does not produce image files
+        image_tasks = [
+            '', # aka 'build'
+            'build',
+            'populate_sdk_ext'
+        ]
+        if self.task in image_tasks:
+            task_subquery = Q(task__in=image_tasks)
+
+        query = task_subquery & Q(num_files__gt=0)
+
+        # annotate with the count of files, to exclude any targets which
+        # don't have associated files
+        candidates = candidates.annotate(
+            num_files=Count('target_image_file'))
+
+        candidates = candidates.filter(query)
+
+        if candidates.count() > 0:
+            candidates.order_by('build__completed_on')
+            similar_target = candidates.last()
+
+        return similar_target
+
+    def clone_artifacts_from(self, target):
+        """
+        Make clones of the BuildArtifacts, Target_Image_Files and
+        TargetArtifactFile objects associated with Target target, then
+        associate them with this target.
+
+        Note that for Target_Image_Files, we only want files from the previous
+        build whose suffix matches one of the suffixes defined in this
+        target's build's IMAGE_FSTYPES configuration variable. This prevents the
+        Target_Image_File object for an ext4 image being associated with a
+        target for a project which didn't produce an ext4 image (for example).
+
+        Also sets the license_manifest_path of this target to the same path
+        as that of target being cloned from, as the license manifest path is
+        also a build artifact but is treated differently.
+        """
+
+        image_fstypes = self.build.get_image_fstypes()
+
+        # filter out any image files whose suffixes aren't in the
+        # IMAGE_FSTYPES suffixes variable for this target's build
+        image_files = [target_image_file \
+            for target_image_file in target.target_image_file_set.all() \
+            if target_image_file.suffix in image_fstypes]
+
+        for image_file in image_files:
+            image_file.pk = None
+            image_file.target = self
+            image_file.save()
+
+        artifact_files = target.targetartifactfile_set.all()
+        for artifact_file in artifact_files:
+            artifact_file.pk = None
+            artifact_file.target = self
+            artifact_file.save()
+
+        self.license_manifest_path = target.license_manifest_path
+        self.save()
+
+# an Artifact is anything that results from a target being built, and may
+# be of interest to the user, and is not an image file
+class TargetArtifactFile(models.Model):
+    target = models.ForeignKey(Target)
+    file_name = models.FilePathField()
+    file_size = models.IntegerField()
+
+    @property
+    def basename(self):
+        return os.path.basename(self.file_name)
+
 class Target_Image_File(models.Model):
     # valid suffixes for image files produced by a build
     SUFFIXES = {
diff --git a/lib/toaster/toastergui/templates/builddashboard.html b/lib/toaster/toastergui/templates/builddashboard.html
index dcda2a8..f6d62b9 100644
--- a/lib/toaster/toastergui/templates/builddashboard.html
+++ b/lib/toaster/toastergui/templates/builddashboard.html
@@ -74,7 +74,7 @@
     {% for target in targets %}
         {% if target.target.is_image %}
     <div class="well well-transparent dashboard-section">
-        <h3><a href="{% url 'target' build.pk target.target.pk %}">{{target.target}}</a></h3>
+        <h3><a href="{% url 'target' build.pk target.target.pk %}">{{target.target.target}}</a></h3>
         <dl class="dl-horizontal">
             <dt>Packages included</dt>
             <dd><a href="{% url 'target' build.pk target.target.pk %}">{{target.npkg}}</a></dd>
@@ -124,6 +124,19 @@
                     {% endfor %}
                 </ul>
             </dd>
+            <dt>
+                Kernel artifacts
+            </dt>
+            <dd>
+                <ul class="list-unstyled">
+                    {% for artifact in target.target_artifacts|dictsort:"basename" %}
+                        <li>
+                            <a href="{% url 'build_artifact' build.id 'targetartifactfile' artifact.id %}">{{artifact.basename}}</a>
+                            ({{artifact.file_size|filtered_filesizeformat}})
+                        </li>
+                    {% endfor %}
+                  </ul>
+            </dd>
         </dl>
         {% endif %}
     </div>
diff --git a/lib/toaster/toastergui/views.py b/lib/toaster/toastergui/views.py
index ad85faf..0ec88d9 100755
--- a/lib/toaster/toastergui/views.py
+++ b/lib/toaster/toastergui/views.py
@@ -31,6 +31,7 @@ from django.shortcuts import render, redirect, get_object_or_404
 from orm.models import Build, Target, Task, Layer, Layer_Version, Recipe, LogMessage, Variable
 from orm.models import Task_Dependency, Recipe_Dependency, Package, Package_File, Package_Dependency
 from orm.models import Target_Installed_Package, Target_File, Target_Image_File, BuildArtifact, CustomImagePackage
+from orm.models import TargetArtifactFile
 from orm.models import BitbakeVersion, CustomImageRecipe
 from bldcontrol import bbcontroller
 from django.views.decorators.cache import cache_control
@@ -509,6 +510,8 @@ def builddashboard( request, build_id ):
             targetHasNoImages = True
         elem[ 'imageFiles' ] = imageFiles
         elem[ 'targetHasNoImages' ] = targetHasNoImages
+        elem['target_artifacts'] = t.targetartifactfile_set.all()
+
         targets.append( elem )

     ##
@@ -2294,6 +2297,10 @@ if True:
         elif artifact_type == "buildartifact":
             file_name = BuildArtifact.objects.get(build = build, pk = artifact_id).file_name

+        elif artifact_type == "targetartifactfile":
+            target = TargetArtifactFile.objects.get(pk=artifact_id)
+            file_name = target.file_name
+
         elif artifact_type == "licensemanifest":
             file_name = Target.objects.get(build = build, pk = artifact_id).license_manifest_path

--
1.9.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/7] buildinfohelper: fix retrieval of targets
  2016-07-09  0:52 [PATCH 0/7] toaster: Artifact Selection Improvement brian avery
                   ` (4 preceding siblings ...)
  2016-07-09  0:52 ` [PATCH 5/7] toaster: attach kernel artifacts to targets brian avery
@ 2016-07-09  0:52 ` brian avery
  2016-07-09  0:52 ` [PATCH 7/7] toaster: improve scan for SDK artifacts brian avery
  6 siblings, 0 replies; 11+ messages in thread
From: brian avery @ 2016-07-09  0:52 UTC (permalink / raw)
  To: bitbake-devel; +Cc: brian avery

From: Elliot Smith <elliot.smith@intel.com>

When buildinfohelper records the targets for a build, it looks
up any existing targets for a build and creates them if they
are not present. This is because in the case of Toaster-triggered
builds, the Target objects have already been created (inside
triggerBuild()) and don't need to be recreated; but in the case
of cli builds, the Target objects have to be created by
buildinfohelper.

The issue is that the code for retrieving an existing target for
a build only looks for Targets with a matching target and build,
e.g. Targets for build X with target "core-image-minimal". But it
is perfectly legitimate to call bitbake with a command like
"bitbake core-image-minimal:do_populate_sdk
core-image-minimal:do_populate_sdk_ext". In such a case, the
code which looks for matching targets finds two objects, as it
doesn't filter by task.

Add the task into the filter for the Target so that only one
Target object is be returned. Note that a command
line like "bitbake recipe:task recipe:task" will still cause an
error as bitbake doesn't de-duplicate the command line arguments
and will run the recipe:task combination twice.

Signed-off-by: Elliot Smith <elliot.smith@intel.com>
Signed-off-by: brian avery <brian.avery@intel.com>
---
 lib/bb/ui/buildinfohelper.py | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/bb/ui/buildinfohelper.py b/lib/bb/ui/buildinfohelper.py
index a5b2237..e1b59c3 100644
--- a/lib/bb/ui/buildinfohelper.py
+++ b/lib/bb/ui/buildinfohelper.py
@@ -201,6 +201,10 @@ class ORMWrapper(object):

     @staticmethod
     def get_or_create_targets(target_info):
+        """
+        NB get_or_create() is used here because for Toaster-triggered builds,
+        we already created the targets when the build was triggered.
+        """
         result = []
         for target in target_info['targets']:
             task = ''
@@ -210,13 +214,10 @@ class ORMWrapper(object):
                 task = task[3:]
             if task == 'build':
                 task = ''
-            obj, created = Target.objects.get_or_create(build=target_info['build'],
-                                                        target=target)
-            if created:
-                obj.is_image = False
-                if task:
-                    obj.task = task
-                obj.save()
+
+            obj, _ = Target.objects.get_or_create(build=target_info['build'],
+                                                  target=target,
+                                                  task=task)
             result.append(obj)
         return result

--
1.9.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/7] toaster: improve scan for SDK artifacts
  2016-07-09  0:52 [PATCH 0/7] toaster: Artifact Selection Improvement brian avery
                   ` (5 preceding siblings ...)
  2016-07-09  0:52 ` [PATCH 6/7] buildinfohelper: fix retrieval of targets brian avery
@ 2016-07-09  0:52 ` brian avery
  6 siblings, 0 replies; 11+ messages in thread
From: brian avery @ 2016-07-09  0:52 UTC (permalink / raw)
  To: bitbake-devel; +Cc: brian avery

From: Elliot Smith <elliot.smith@intel.com>

SDK artifacts were previously picked up by toaster.bbclass and
notified to buildinfohelper (via toasterui). The artifacts
were then added to the Build object, so that it wasn't clear
which artifact went with which target; we were also unable
to attach SDK artifacts to a Build if they had already been
attached to a previous build.

Now, toaster.bbclass just notifies the TOOLCHAIN_OUTPUTNAME when
a populate_sdk* target completes. The scan is moved to buildinfohelper,
where we search the SDK deploy directory for files matching
TOOLCHAIN_OUTPUTNAME and attach them to targets (not builds).

If an SDK file is not produced by a target, we now look for a
similar, previously-run target which did produce artifacts.
If there is one, we clone the SDK artifacts from that target
onto the current one.

This all means that we can show SDK artifacts by target, and should
always get artifacts associated with a target, regardless of whether
it really build them.

This requires an additional model, TargetSDKFile, which tracks
the size and path of SDK artifact files with respect to Target
objects.

[YOCTO #8556]

Signed-off-by: Elliot Smith <elliot.smith@intel.com>
Signed-off-by: brian avery <brian.avery@intel.com>
---
 lib/bb/ui/buildinfohelper.py                       | 175 ++++++++++++++++-----
 lib/bb/ui/toasterui.py                             |   7 +-
 ...actfile.py => 0008_refactor_artifact_models.py} |  20 ++-
 lib/toaster/orm/models.py                          | 112 ++++++++-----
 .../toastergui/templates/builddashboard.html       |  64 +++++---
 lib/toaster/toastergui/views.py                    |  19 ++-
 6 files changed, 279 insertions(+), 118 deletions(-)
 rename bitbake/lib/toaster/orm/migrations/{0008_targetartifactfile.py => 0008_refactor_artifact_models.py} (40%)

diff --git a/lib/bb/ui/buildinfohelper.py b/lib/bb/ui/buildinfohelper.py
index e1b59c3..bf032ae 100644
--- a/lib/bb/ui/buildinfohelper.py
+++ b/lib/bb/ui/buildinfohelper.py
@@ -37,7 +37,7 @@ os.environ["DJANGO_SETTINGS_MODULE"] =\
 django.setup()

 from orm.models import Build, Task, Recipe, Layer_Version, Layer, Target, LogMessage, HelpText
-from orm.models import Target_Image_File, BuildArtifact, TargetArtifactFile
+from orm.models import Target_Image_File, TargetKernelFile, TargetSDKFile
 from orm.models import Variable, VariableHistory
 from orm.models import Package, Package_File, Target_Installed_Package, Target_File
 from orm.models import Task_Dependency, Package_Dependency
@@ -128,6 +128,15 @@ class ORMWrapper(object):
         """
         return target.get_similar_target_with_image_files()

+    def get_similar_target_with_sdk_files(self, target):
+        return target.get_similar_target_with_sdk_files()
+
+    def clone_image_artifacts(self, target_from, target_to):
+        target_to.clone_image_artifacts_from(target_from)
+
+    def clone_sdk_artifacts(self, target_from, target_to):
+        target_to.clone_sdk_artifacts_from(target_from)
+
     def _timestamp_to_datetime(self, secs):
         """
         Convert timestamp in seconds to Python datetime
@@ -682,35 +691,22 @@ class ORMWrapper(object):
             logger.warning("buildinfohelper: target_package_info could not identify recipes: \n%s", errormsg)

     def save_target_image_file_information(self, target_obj, file_name, file_size):
-        Target_Image_File.objects.create( target = target_obj,
-                            file_name = file_name,
-                            file_size = file_size)
+        Target_Image_File.objects.create(target=target_obj,
+            file_name=file_name, file_size=file_size)

-    def save_target_artifact_file(self, target_obj, file_name, file_size):
+    def save_target_kernel_file(self, target_obj, file_name, file_size):
         """
-        Save artifact file information for a Target target_obj.
-
-        Note that this doesn't include SDK artifacts, only images and
-        related files (e.g. bzImage).
+        Save kernel file (bzImage, modules*) information for a Target target_obj.
         """
-        TargetArtifactFile.objects.create(target=target_obj,
+        TargetKernelFile.objects.create(target=target_obj,
             file_name=file_name, file_size=file_size)

-    def save_artifact_information(self, build_obj, file_name, file_size):
+    def save_target_sdk_file(self, target_obj, file_name, file_size):
         """
-        TODO this is currently used to save SDK artifacts to the database,
-        but will be replaced in future once SDK artifacts are associated with
-        Target objects (as they eventually should be)
+        Save SDK artifacts to the database, associating them with a
+        Target object.
         """
-        # do not update artifacts found in other builds
-        if BuildArtifact.objects.filter(file_name = file_name).count() > 0:
-            return
-
-        # do not update artifact if already a target artifact file for that path
-        if TargetArtifactFile.objects.filter(file_name = file_name).count() > 0:
-            return
-
-        BuildArtifact.objects.create(build=build_obj, file_name=file_name,
+        TargetSDKFile.objects.create(target=target_obj, file_name=file_name,
             file_size=file_size)

     def create_logmessage(self, log_information):
@@ -1085,11 +1081,6 @@ class BuildInfoHelper(object):

         return self.brbe

-    def update_artifact_image_file(self, event):
-        evdata = BuildInfoHelper._get_data_from_event(event)
-        for artifact_path in evdata.keys():
-            self.orm_wrapper.save_artifact_information(self.internal_state['build'], artifact_path, evdata[artifact_path])
-
     def update_build_information(self, event, errors, warnings, taskfailures):
         if 'build' in self.internal_state:
             self.orm_wrapper.update_build_object(self.internal_state['build'], errors, warnings, taskfailures)
@@ -1568,9 +1559,9 @@ class BuildInfoHelper(object):

         return image_files

-    def scan_build_artifacts(self):
+    def scan_image_artifacts(self):
         """
-        Scan for build artifacts in DEPLOY_DIR_IMAGE and associate them
+        Scan for built image artifacts in DEPLOY_DIR_IMAGE and associate them
         with a Target object in self.internal_state['targets'].

         We have two situations to handle:
@@ -1615,7 +1606,7 @@ class BuildInfoHelper(object):
         # filter out anything which isn't an image target
         image_targets = [target for target in targets if target.is_image]

-        for target in image_targets:
+        for image_target in image_targets:
             # this is set to True if we find at least one file relating to
             # this target; if this remains False after the scan, we copy the
             # files from the most-recent Target with the same target + machine
@@ -1627,7 +1618,7 @@ class BuildInfoHelper(object):
             # 'defaultpkgname-<MACHINE>-<BUILDNAME>';
             # we need to change it to
             # <TARGET>-<MACHINE>-<BUILDNAME>
-            real_image_name = re.sub(r'^defaultpkgname', target.target,
+            real_image_name = re.sub(r'^defaultpkgname', image_target.target,
                 image_name)

             image_license_manifest_path = os.path.join(
@@ -1651,7 +1642,7 @@ class BuildInfoHelper(object):

                     # note that the artifact will only be saved against this
                     # build if it hasn't been already
-                    self.orm_wrapper.save_target_artifact_file(target,
+                    self.orm_wrapper.save_target_kernel_file(image_target,
                         artifact_path, artifact_size)

                 # store the license manifest path on the target
@@ -1659,8 +1650,8 @@ class BuildInfoHelper(object):
                 license_path = os.path.join(license_directory,
                     real_image_name, 'license.manifest')

-                self.orm_wrapper.update_target_set_license_manifest(target,
-                    license_path)
+                self.orm_wrapper.update_target_set_license_manifest(
+                    image_target, license_path)

             # scan the directory for image files relating to this build
             # (via real_image_name); note that we don't have to set
@@ -1675,7 +1666,7 @@ class BuildInfoHelper(object):

             for image_file in image_files:
                 self.orm_wrapper.save_target_image_file_information(
-                    target, image_file['path'], image_file['size'])
+                    image_target, image_file['path'], image_file['size'])

             if not has_files:
                 # copy image files and build artifacts from the
@@ -1683,13 +1674,115 @@ class BuildInfoHelper(object):
                 # same target + machine as this Target; also copy the license
                 # manifest path, as that is not treated as an artifact and needs
                 # to be set separately
-                most_recent = \
-                    self.orm_wrapper.get_similar_target_with_image_files(target)
+                similar_target = \
+                    self.orm_wrapper.get_similar_target_with_image_files(
+                        image_target)

-                if most_recent:
+                if similar_target:
                     logger.info('image artifacts for target %s cloned from ' \
-                        'target %s' % (target.pk, most_recent.pk))
-                    target.clone_artifacts_from(most_recent)
+                        'target %s' % (image_target.pk, similar_target.pk))
+                    self.orm_wrapper.clone_image_artifacts(similar_target,
+                        image_target)
+
+    def _get_sdk_targets(self):
+        """
+        Return targets which could generate SDK artifacts, i.e.
+        "do_populate_sdk" and "do_populate_sdk_ext".
+        """
+        return [target for target in self.internal_state['targets'] \
+            if target.task in ['populate_sdk', 'populate_sdk_ext']]
+
+    def scan_sdk_artifacts(self, event):
+        """
+        Note that we have to intercept an SDKArtifactInfo event from
+        toaster.bbclass (via toasterui) to get hold of the SDK variables we
+        need to be able to scan for files accurately: this is because
+        variables like TOOLCHAIN_OUTPUTNAME have reset to None by the time
+        BuildCompleted is fired by bitbake, so we have to get those values
+        while the build is still in progress.
+
+        For populate_sdk_ext, this runs twice, with two different
+        TOOLCHAIN_OUTPUTNAME settings, each of which will capture some of the
+        files in the SDK output directory.
+        """
+        sdk_vars = BuildInfoHelper._get_data_from_event(event)
+        toolchain_outputname = sdk_vars['TOOLCHAIN_OUTPUTNAME']
+
+        # targets which might have created SDK artifacts
+        sdk_targets = self._get_sdk_targets()
+
+        # location of SDK artifacts
+        tmpdir = self.server.runCommand(['getVariable', 'TMPDIR'])[0]
+        sdk_dir = os.path.join(tmpdir, 'deploy', 'sdk')
+
+        # all files in the SDK directory
+        artifacts = []
+        for dir_path, _, filenames in os.walk(sdk_dir):
+            for filename in filenames:
+                full_path = os.path.join(dir_path, filename)
+                if not os.path.islink(full_path):
+                    artifacts.append(full_path)
+
+        for sdk_target in sdk_targets:
+            # find files in the SDK directory which haven't already been
+            # recorded against a Target and whose basename matches
+            # TOOLCHAIN_OUTPUTNAME
+            for artifact_path in artifacts:
+                basename = os.path.basename(artifact_path)
+
+                toolchain_match = basename.startswith(toolchain_outputname)
+
+                # files which match the name of the target which produced them;
+                # for example,
+                # poky-glibc-x86_64-core-image-sato-i586-toolchain-ext-2.1+snapshot.sh
+                target_match = re.search(sdk_target.target, basename)
+
+                # targets which produce "*-nativesdk-*" files
+                is_ext_sdk_target = sdk_target.task in \
+                    ['do_populate_sdk_ext', 'populate_sdk_ext']
+
+                # SDK files which don't match the target name, i.e.
+                # x86_64-nativesdk-libc.*
+                # poky-glibc-x86_64-buildtools-tarball-i586-buildtools-nativesdk-standalone-2.1+snapshot*
+                is_ext_sdk_file = re.search('-nativesdk-', basename)
+
+                file_from_target = (toolchain_match and target_match) or \
+                    (is_ext_sdk_target and is_ext_sdk_file)
+
+                if file_from_target:
+                    # don't record the file if it's already been added to this
+                    # target
+                    matching_files = TargetSDKFile.objects.filter(
+                        target=sdk_target, file_name=artifact_path)
+
+                    if matching_files.count() == 0:
+                        artifact_size = os.stat(artifact_path).st_size
+
+                        self.orm_wrapper.save_target_sdk_file(
+                            sdk_target, artifact_path, artifact_size)
+
+    def clone_required_sdk_artifacts(self):
+        """
+        If an SDK target doesn't have any SDK artifacts, this means that
+        the postfuncs of populate_sdk or populate_sdk_ext didn't fire, which
+        in turn means that the targets of this build didn't generate any new
+        artifacts.
+
+        In this case, clone SDK artifacts for targets in the current build
+        from existing targets for this build.
+        """
+        sdk_targets = self._get_sdk_targets()
+        for sdk_target in sdk_targets:
+            # only clone for SDK targets which have no TargetSDKFiles yet
+            if sdk_target.targetsdkfile_set.all().count() == 0:
+                similar_target = \
+                    self.orm_wrapper.get_similar_target_with_sdk_files(
+                        sdk_target)
+                if similar_target:
+                    logger.info('SDK artifacts for target %s cloned from ' \
+                        'target %s' % (sdk_target.pk, similar_target.pk))
+                    self.orm_wrapper.clone_sdk_artifacts(similar_target,
+                        sdk_target)

     def close(self, errorcode):
         if self.brbe is not None:
diff --git a/lib/bb/ui/toasterui.py b/lib/bb/ui/toasterui.py
index d8bccdb..f399a7d 100644
--- a/lib/bb/ui/toasterui.py
+++ b/lib/bb/ui/toasterui.py
@@ -364,7 +364,8 @@ def main(server, eventHandler, params):
                     errorcode = 1
                     logger.error("Command execution failed: %s", event.error)
                 elif isinstance(event, bb.event.BuildCompleted):
-                    buildinfohelper.scan_build_artifacts()
+                    buildinfohelper.scan_image_artifacts()
+                    buildinfohelper.clone_required_sdk_artifacts()

                 # turn off logging to the current build log
                 _close_build_log(build_log)
@@ -412,8 +413,8 @@ def main(server, eventHandler, params):
                     buildinfohelper.store_target_package_data(event)
                 elif event.type == "MissedSstate":
                     buildinfohelper.store_missed_state_tasks(event)
-                elif event.type == "ArtifactFileSize":
-                    buildinfohelper.update_artifact_image_file(event)
+                elif event.type == "SDKArtifactInfo":
+                    buildinfohelper.scan_sdk_artifacts(event)
                 elif event.type == "SetBRBE":
                     buildinfohelper.brbe = buildinfohelper._get_data_from_event(event)
                 elif event.type == "OSErrorException":
diff --git a/lib/toaster/orm/migrations/0008_targetartifactfile.py b/lib/toaster/orm/migrations/0008_refactor_artifact_models.py
similarity index 40%
rename from lib/toaster/orm/migrations/0008_targetartifactfile.py
rename to lib/toaster/orm/migrations/0008_refactor_artifact_models.py
index 9f1d9bf..3367582 100644
--- a/lib/toaster/orm/migrations/0008_targetartifactfile.py
+++ b/lib/toaster/orm/migrations/0008_refactor_artifact_models.py
@@ -12,12 +12,28 @@ class Migration(migrations.Migration):

     operations = [
         migrations.CreateModel(
-            name='TargetArtifactFile',
+            name='TargetKernelFile',
             fields=[
-                ('id', models.AutoField(primary_key=True, serialize=False, auto_created=True, verbose_name='ID')),
+                ('id', models.AutoField(auto_created=True, primary_key=True, verbose_name='ID', serialize=False)),
                 ('file_name', models.FilePathField()),
                 ('file_size', models.IntegerField()),
                 ('target', models.ForeignKey(to='orm.Target')),
             ],
         ),
+        migrations.CreateModel(
+            name='TargetSDKFile',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, verbose_name='ID', serialize=False)),
+                ('file_name', models.FilePathField()),
+                ('file_size', models.IntegerField()),
+                ('target', models.ForeignKey(to='orm.Target')),
+            ],
+        ),
+        migrations.RemoveField(
+            model_name='buildartifact',
+            name='build',
+        ),
+        migrations.DeleteModel(
+            name='BuildArtifact',
+        ),
     ]
diff --git a/lib/toaster/orm/models.py b/lib/toaster/orm/models.py
index 9edbef3..61f6a20 100644
--- a/lib/toaster/orm/models.py
+++ b/lib/toaster/orm/models.py
@@ -581,28 +581,6 @@ class Build(models.Model):
     def __str__(self):
         return "%d %s %s" % (self.id, self.project, ",".join([t.target for t in self.target_set.all()]))

-
-# an Artifact is anything that results from a Build, and may be of interest to the user, and is not stored elsewhere
-class BuildArtifact(models.Model):
-    build = models.ForeignKey(Build)
-    file_name = models.FilePathField()
-    file_size = models.IntegerField()
-
-    def get_local_file_name(self):
-        try:
-            deploydir = Variable.objects.get(build = self.build, variable_name="DEPLOY_DIR").variable_value
-            return  self.file_name[len(deploydir)+1:]
-        except:
-            raise
-
-        return self.file_name
-
-    def get_basename(self):
-        return os.path.basename(self.file_name)
-
-    def is_available(self):
-        return self.build.buildrequest.environment.has_artifact(self.file_name)
-
 class ProjectTarget(models.Model):
     project = models.ForeignKey(Project)
     target = models.CharField(max_length=100)
@@ -625,13 +603,19 @@ class Target(models.Model):

     def get_similar_targets(self):
         """
-        Get targets for the same machine, task and target name
+        Get target sfor the same machine, task and target name
         (e.g. 'core-image-minimal') from a successful build for this project
         (but excluding this target).

-        Note that we look for targets built by this project because projects
-        can have different configurations from each other, and put their
-        artifacts in different directories.
+        Note that we only look for targets built by this project because
+        projects can have different configurations from each other, and put
+        their artifacts in different directories.
+
+        The possibility of error when retrieving candidate targets
+        is minimised by the fact that bitbake will rebuild artifacts if MACHINE
+        (or various other variables) change. In this case, there is no need to
+        clone artifacts from another target, as those artifacts will have
+        been re-generated for this target anyway.
         """
         query = ~Q(pk=self.pk) & \
             Q(target=self.target) & \
@@ -649,29 +633,54 @@ class Target(models.Model):
         similar_target = None

         candidates = self.get_similar_targets()
-        if candidates.count() < 1:
+        if candidates.count() == 0:
             return similar_target

         task_subquery = Q(task=self.task)

         # we can look for a 'build' task if this task is a 'populate_sdk_ext'
-        # task, as it will have created images; and vice versa; note that
+        # task, as the latter also creates images; and vice versa; note that
         # 'build' targets can have their task set to '';
         # also note that 'populate_sdk' does not produce image files
         image_tasks = [
             '', # aka 'build'
             'build',
+            'image',
             'populate_sdk_ext'
         ]
         if self.task in image_tasks:
             task_subquery = Q(task__in=image_tasks)

+        # annotate with the count of files, to exclude any targets which
+        # don't have associated files
+        candidates = candidates.annotate(num_files=Count('target_image_file'))
+
         query = task_subquery & Q(num_files__gt=0)

+        candidates = candidates.filter(query)
+
+        if candidates.count() > 0:
+            candidates.order_by('build__completed_on')
+            similar_target = candidates.last()
+
+        return similar_target
+
+    def get_similar_target_with_sdk_files(self):
+        """
+        Get the most recent similar target with TargetSDKFiles associated
+        with it, for the purpose of cloning those files onto this target.
+        """
+        similar_target = None
+
+        candidates = self.get_similar_targets()
+        if candidates.count() == 0:
+            return similar_target
+
         # annotate with the count of files, to exclude any targets which
         # don't have associated files
-        candidates = candidates.annotate(
-            num_files=Count('target_image_file'))
+        candidates = candidates.annotate(num_files=Count('targetsdkfile'))
+
+        query = Q(task=self.task) & Q(num_files__gt=0)

         candidates = candidates.filter(query)

@@ -681,11 +690,10 @@ class Target(models.Model):

         return similar_target

-    def clone_artifacts_from(self, target):
+    def clone_image_artifacts_from(self, target):
         """
-        Make clones of the BuildArtifacts, Target_Image_Files and
-        TargetArtifactFile objects associated with Target target, then
-        associate them with this target.
+        Make clones of the Target_Image_Files and TargetKernelFile objects
+        associated with Target target, then associate them with this target.

         Note that for Target_Image_Files, we only want files from the previous
         build whose suffix matches one of the suffixes defined in this
@@ -711,18 +719,38 @@ class Target(models.Model):
             image_file.target = self
             image_file.save()

-        artifact_files = target.targetartifactfile_set.all()
-        for artifact_file in artifact_files:
-            artifact_file.pk = None
-            artifact_file.target = self
-            artifact_file.save()
+        kernel_files = target.targetkernelfile_set.all()
+        for kernel_file in kernel_files:
+            kernel_file.pk = None
+            kernel_file.target = self
+            kernel_file.save()

         self.license_manifest_path = target.license_manifest_path
         self.save()

-# an Artifact is anything that results from a target being built, and may
-# be of interest to the user, and is not an image file
-class TargetArtifactFile(models.Model):
+    def clone_sdk_artifacts_from(self, target):
+        """
+        Clone TargetSDKFile objects from target and associate them with this
+        target.
+        """
+        sdk_files = target.targetsdkfile_set.all()
+        for sdk_file in sdk_files:
+            sdk_file.pk = None
+            sdk_file.target = self
+            sdk_file.save()
+
+# kernel artifacts for a target: bzImage and modules*
+class TargetKernelFile(models.Model):
+    target = models.ForeignKey(Target)
+    file_name = models.FilePathField()
+    file_size = models.IntegerField()
+
+    @property
+    def basename(self):
+        return os.path.basename(self.file_name)
+
+# SDK artifacts for a target: sh and manifest files
+class TargetSDKFile(models.Model):
     target = models.ForeignKey(Target)
     file_name = models.FilePathField()
     file_size = models.IntegerField()
diff --git a/lib/toaster/toastergui/templates/builddashboard.html b/lib/toaster/toastergui/templates/builddashboard.html
index f6d62b9..e1bde21 100644
--- a/lib/toaster/toastergui/templates/builddashboard.html
+++ b/lib/toaster/toastergui/templates/builddashboard.html
@@ -80,29 +80,30 @@
             <dd><a href="{% url 'target' build.pk target.target.pk %}">{{target.npkg}}</a></dd>
             <dt>Total package size</dt>
             <dd>{{target.pkgsz|filtered_filesizeformat}}</dd>
-                        {% if target.targetHasNoImages %}
-                </dl>
-                <div class="row">
-                  <div class="col-md-7">
-                    <div class="alert alert-info">
-                      <p>
-                      <strong>This build did not create any image files</strong>
-                      </p>
-                      <p>
-                      This is probably because valid image and license manifest
-                      files from a previous build already exist in your
-                      <code>build/tmp/deploy</code>
-                      directory. You can
-                      also <a href="{% url 'target' build.pk target.target.pk %}">view the
-                        license manifest information</a> in Toaster.
-                      </p>
-                    </div>
-                  </div>
+        </dl>
+        {% if target.targetHasNoImages %}
+            <div class="row">
+              <div class="col-md-7">
+                <div class="alert alert-info">
+                  <p>
+                  <strong>This build did not create any image files</strong>
+                  </p>
+                  <p>
+                  This is probably because valid image and license manifest
+                  files from a previous build already exist in your
+                  <code>build/tmp/deploy</code>
+                  directory. You can
+                  also <a href="{% url 'target' build.pk target.target.pk %}">view the
+                    license manifest information</a> in Toaster.
+                  </p>
                 </div>
-        {% else %}
+              </div>
+            </div>
+        {% endif %}
+        {% if not target.targetHasNoImages %}
+          <dl class="dl-horizontal">
             <dt>
                 <span class="glyphicon glyphicon-question-sign get-help" title="The location in disk of the license manifest, a document listing all packages installed in your image and their licenses"></span>
-
                 License manifest
             </dt>
             <dd>
@@ -129,15 +130,32 @@
             </dt>
             <dd>
                 <ul class="list-unstyled">
-                    {% for artifact in target.target_artifacts|dictsort:"basename" %}
+                    {% for artifact in target.target_kernel_artifacts|dictsort:"basename" %}
                         <li>
-                            <a href="{% url 'build_artifact' build.id 'targetartifactfile' artifact.id %}">{{artifact.basename}}</a>
+                            <a href="{% url 'build_artifact' build.id 'targetkernelartifact' artifact.id %}">{{artifact.basename}}</a>
                             ({{artifact.file_size|filtered_filesizeformat}})
                         </li>
                     {% endfor %}
                   </ul>
             </dd>
-        </dl>
+          </dl>
+        {% endif %}
+        {% if target.target_sdk_artifacts_count > 0 %}
+          <dl class="dl-horizontal">
+            <dt>
+              SDK artifacts
+            </dt>
+            <dd>
+              <ul class="list-unstyled">
+                {% for artifact in target.target_sdk_artifacts|dictsort:"basename" %}
+                <li>
+                  <a href="{% url 'build_artifact' build.id 'targetsdkartifact' artifact.id %}">{{artifact.basename}}</a>
+                  ({{artifact.file_size|filtered_filesizeformat}})
+                </li>
+                {% endfor %}
+              </ul>
+            </dd>
+          </dl>
         {% endif %}
     </div>
         {% endif %}
diff --git a/lib/toaster/toastergui/views.py b/lib/toaster/toastergui/views.py
index 0ec88d9..a82a261 100755
--- a/lib/toaster/toastergui/views.py
+++ b/lib/toaster/toastergui/views.py
@@ -30,8 +30,8 @@ from django.db import IntegrityError, Error
 from django.shortcuts import render, redirect, get_object_or_404
 from orm.models import Build, Target, Task, Layer, Layer_Version, Recipe, LogMessage, Variable
 from orm.models import Task_Dependency, Recipe_Dependency, Package, Package_File, Package_Dependency
-from orm.models import Target_Installed_Package, Target_File, Target_Image_File, BuildArtifact, CustomImagePackage
-from orm.models import TargetArtifactFile
+from orm.models import Target_Installed_Package, Target_File, Target_Image_File, CustomImagePackage
+from orm.models import TargetKernelFile, TargetSDKFile
 from orm.models import BitbakeVersion, CustomImageRecipe
 from bldcontrol import bbcontroller
 from django.views.decorators.cache import cache_control
@@ -510,7 +510,11 @@ def builddashboard( request, build_id ):
             targetHasNoImages = True
         elem[ 'imageFiles' ] = imageFiles
         elem[ 'targetHasNoImages' ] = targetHasNoImages
-        elem['target_artifacts'] = t.targetartifactfile_set.all()
+        elem['target_kernel_artifacts'] = t.targetkernelfile_set.all()
+
+        target_sdk_files = t.targetsdkfile_set.all()
+        elem['target_sdk_artifacts_count'] = target_sdk_files.count()
+        elem['target_sdk_artifacts'] = target_sdk_files

         targets.append( elem )

@@ -2294,11 +2298,12 @@ if True:
         elif artifact_type == "imagefile":
             file_name = Target_Image_File.objects.get(target__build = build, pk = artifact_id).file_name

-        elif artifact_type == "buildartifact":
-            file_name = BuildArtifact.objects.get(build = build, pk = artifact_id).file_name
+        elif artifact_type == "targetkernelartifact":
+            target = TargetKernelFile.objects.get(pk=artifact_id)
+            file_name = target.file_name

-        elif artifact_type == "targetartifactfile":
-            target = TargetArtifactFile.objects.get(pk=artifact_id)
+        elif artifact_type == "targetsdkartifact":
+            target = TargetSDKFile.objects.get(pk=artifact_id)
             file_name = target.file_name

         elif artifact_type == "licensemanifest":
--
1.9.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/7] runqueue: improve exception logging
  2016-07-09  0:52 ` [PATCH 1/7] runqueue: improve exception logging brian avery
@ 2016-07-09  4:03   ` Christopher Larson
  2016-07-09 17:10     ` Brian Avery
  0 siblings, 1 reply; 11+ messages in thread
From: Christopher Larson @ 2016-07-09  4:03 UTC (permalink / raw)
  To: brian avery; +Cc: brian avery, bitbake-devel

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]

On Fri, Jul 8, 2016 at 5:52 PM, brian avery <avery.brian@gmail.com> wrote:

> From: Ed Bartosh <ed.bartosh@linux.intel.com>
>
> Runqueue errors direct the user to view the "failure below",
> but no additional error message is available.
>
> Log the stacktrace so that the user can see what went wrong.
>
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> Signed-off-by: brian avery <brian.avery@intel.com>
> ---
>  lib/bb/runqueue.py | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/bb/runqueue.py b/lib/bb/runqueue.py
> index 57be15a..04f2f4c 100644
> --- a/lib/bb/runqueue.py
> +++ b/lib/bb/runqueue.py
> @@ -1219,8 +1219,10 @@ class RunQueue:
>                  pass
>              self.state = runQueueComplete
>              raise
> -        except:
> -            logger.error("An uncaught exception occured in runqueue,
> please see the failure below:")
> +        except Exception as err:
> +            import traceback
> +            logger.error("An uncaught exception occured in runqueue:
> '%s'" % err)
> +            logger.error(traceback.format_exc())
>

Is there a reason we aren't just passing the exception in and letting the
logging format it and pass it through to the UI for formatting? See
logger.exception() and the exc_info= keyword argument for error().
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2391 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/7] runqueue: improve exception logging
  2016-07-09  4:03   ` Christopher Larson
@ 2016-07-09 17:10     ` Brian Avery
  2016-07-09 18:30       ` Christopher Larson
  0 siblings, 1 reply; 11+ messages in thread
From: Brian Avery @ 2016-07-09 17:10 UTC (permalink / raw)
  To: Christopher Larson; +Cc: brian avery, bitbake-devel, Bartosh, Eduard

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]

If you mean something like:

        logger.exception("Ed's message is this err = '%s'"
%(err),exc_info=err)

or

        logger.exception("Ed's message is this err = '%s'"
%(err),exc_info=err,stack_info=True) (if we want to see the exception stack
search)


Then, i'd agree your way is better/clearer.

-b

an intel employee

On Fri, Jul 8, 2016 at 9:03 PM, Christopher Larson <clarson@kergoth.com>
wrote:

>
> On Fri, Jul 8, 2016 at 5:52 PM, brian avery <avery.brian@gmail.com> wrote:
>
>> From: Ed Bartosh <ed.bartosh@linux.intel.com>
>>
>> Runqueue errors direct the user to view the "failure below",
>> but no additional error message is available.
>>
>> Log the stacktrace so that the user can see what went wrong.
>>
>> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
>> Signed-off-by: brian avery <brian.avery@intel.com>
>> ---
>>  lib/bb/runqueue.py | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/bb/runqueue.py b/lib/bb/runqueue.py
>> index 57be15a..04f2f4c 100644
>> --- a/lib/bb/runqueue.py
>> +++ b/lib/bb/runqueue.py
>> @@ -1219,8 +1219,10 @@ class RunQueue:
>>                  pass
>>              self.state = runQueueComplete
>>              raise
>> -        except:
>> -            logger.error("An uncaught exception occured in runqueue,
>> please see the failure below:")
>> +        except Exception as err:
>> +            import traceback
>> +            logger.error("An uncaught exception occured in runqueue:
>> '%s'" % err)
>> +            logger.error(traceback.format_exc())
>>
>
> Is there a reason we aren't just passing the exception in and letting the
> logging format it and pass it through to the UI for formatting? See
> logger.exception() and the exc_info= keyword argument for error().
> --
> Christopher Larson
> clarson at kergoth dot com
> Founder - BitBake, OpenEmbedded, OpenZaurus
> Maintainer - Tslib
> Senior Software Engineer, Mentor Graphics
>

[-- Attachment #2: Type: text/html, Size: 3592 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/7] runqueue: improve exception logging
  2016-07-09 17:10     ` Brian Avery
@ 2016-07-09 18:30       ` Christopher Larson
  0 siblings, 0 replies; 11+ messages in thread
From: Christopher Larson @ 2016-07-09 18:30 UTC (permalink / raw)
  To: Brian Avery; +Cc: brian avery, bitbake-devel, Bartosh, Eduard

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

On Sat, Jul 9, 2016 at 10:10 AM, Brian Avery <avery.brian@gmail.com> wrote:

> If you mean something like:
>
>         logger.exception("Ed's message is this err = '%s'"
> %(err),exc_info=err)
>
> or
>
>         logger.exception("Ed's message is this err = '%s'"
> %(err),exc_info=err,stack_info=True) (if we want to see the exception
> stack search)
>
>
> Then, i'd agree your way is better/clearer.
>
That's the idea, but there are a couple issues with those examples.

exc_info doesn't accept the exception, it takes a boolean, and using
exception() with exc_info is redundant. 'exception(foo)' is just shorthand
for 'error(foo, exc_info=True)', and I think stack_info may also be
unnecessary, as exc_info implies traceback.print_exception(), which prints
exception traceback entries already.

But yes, that's the concept. I think this would be sufficient:

    logger.exception('An uncaught exception occurred in runqueue')

Of course we'd want to generate such an exception to test it.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2355 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-07-09 18:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-09  0:52 [PATCH 0/7] toaster: Artifact Selection Improvement brian avery
2016-07-09  0:52 ` [PATCH 1/7] runqueue: improve exception logging brian avery
2016-07-09  4:03   ` Christopher Larson
2016-07-09 17:10     ` Brian Avery
2016-07-09 18:30       ` Christopher Larson
2016-07-09  0:52 ` [PATCH 2/7] toaster: display Target targets in build dashboard brian avery
2016-07-09  0:52 ` [PATCH 3/7] toaster: do image and artifact scan on BuildCompleted brian avery
2016-07-09  0:52 ` [PATCH 4/7] toaster: improve image file suffix retrieval brian avery
2016-07-09  0:52 ` [PATCH 5/7] toaster: attach kernel artifacts to targets brian avery
2016-07-09  0:52 ` [PATCH 6/7] buildinfohelper: fix retrieval of targets brian avery
2016-07-09  0:52 ` [PATCH 7/7] toaster: improve scan for SDK artifacts brian avery

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.