[PATCH 0/3] Cleanups for buildstats.bbclass

[PATCH 0/3] Cleanups for buildstats.bbclass

[Paul Eggleton]

The following changes since commit 4563788a21fb1715920cd5056b43d85c69563c32:

  connman: upgrade to 1.29 (2015-05-05 22:14:10 +0100)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib paule/buildstats
  http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=paule/buildstats

Paul Eggleton (3):
  classes/buildstats: whitespace cleanup
  classes/buildstats: don't catch all errors from mkdirhier
  classes/buildstats: misc cleanups

 meta/classes/buildstats.bbclass | 215 ++++++++++++++++++----------------------
 1 file changed, 98 insertions(+), 117 deletions(-)

-- 
2.1.0

[PATCH 1/3] classes/buildstats: whitespace cleanup

[Paul Eggleton]

* Drop trailing whitespace
* Use spaces around equals in assignments
* Replace an errant tab with spaces

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/classes/buildstats.bbclass | 50 ++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/meta/classes/buildstats.bbclass b/meta/classes/buildstats.bbclass
index 4844264..f15c45d 100644
--- a/meta/classes/buildstats.bbclass
+++ b/meta/classes/buildstats.bbclass
@@ -3,12 +3,12 @@ BNFILE = "${BUILDSTATS_BASE}/.buildname"
 DEVFILE = "${BUILDSTATS_BASE}/.device"
 
 ################################################################################
-# Build statistics gathering. 
+# Build statistics gathering.
 #
 # The CPU and Time gathering/tracking functions and bbevent inspiration
-# were written by Christopher Larson and can be seen here: 
+# were written by Christopher Larson and can be seen here:
 # http://kergoth.pastey.net/142813
-# 
+#
 ################################################################################
 
 def get_process_cputime(pid):
@@ -43,36 +43,36 @@ def set_device(e):
     except:
         pass
     ############################################################################
-    # We look for the volume TMPDIR lives on. To do all disks would make little 
+    # We look for the volume TMPDIR lives on. To do all disks would make little
     # sense and not give us any particularly useful data. In theory we could do
     # something like stick DL_DIR on a different partition and this would
     # throw stats gathering off. The same goes with SSTATE_DIR. However, let's
-    # get the basics in here and work on the cornercases later. 
+    # get the basics in here and work on the cornercases later.
     # A note. /proc/diskstats does not contain info on encryptfs, tmpfs, etc.
     # If we end up hitting one of these fs, we'll just skip diskstats collection.
     ############################################################################
-    device=os.stat(tmpdir)
-    majordev=os.major(long(device.st_dev))
-    minordev=os.minor(long(device.st_dev))
+    device = os.stat(tmpdir)
+    majordev = os.major(long(device.st_dev))
+    minordev = os.minor(long(device.st_dev))
     ############################################################################
-    # Bug 1700: 
+    # Bug 1700:
     # Because tmpfs/encryptfs/ramfs etc inserts no entry in /proc/diskstats
     # we set rdev to NoLogicalDevice and search for it later. If we find NLD
     # we do not collect diskstats as the method to collect meaningful statistics
-    # for these fs types requires a bit more research. 
+    # for these fs types requires a bit more research.
     ############################################################################
-    rdev="NoLogicalDevice"
+    rdev = "NoLogicalDevice"
     try:
         with open("/proc/diskstats", "r") as f:
             for line in f:
                 if majordev == int(line.split()[0]) and minordev == int(line.split()[1]):
-                    rdev=line.split()[2]
+                    rdev = line.split()[2]
     except:
         pass
     file = open(e.data.getVar('DEVFILE', True), "w")
     file.write(rdev)
     file.close()
-    
+
 def get_device(e):
     file = open(e.data.getVar('DEVFILE', True))
     device = file.readline()
@@ -84,7 +84,7 @@ def get_diskstats(dev):
     ############################################################################
     # For info on what these are, see kernel doc file iostats.txt
     ############################################################################
-    DSTAT_KEYS = ['ReadsComp', 'ReadsMerged', 'SectRead', 'TimeReads', 'WritesComp', 'SectWrite', 'TimeWrite', 'IOinProgress', 'TimeIO', 'WTimeIO']  
+    DSTAT_KEYS = ['ReadsComp', 'ReadsMerged', 'SectRead', 'TimeReads', 'WritesComp', 'SectWrite', 'TimeWrite', 'IOinProgress', 'TimeIO', 'WTimeIO']
     try:
         with open("/proc/diskstats", "r") as f:
             for x in f:
@@ -106,9 +106,9 @@ def get_diskdata(var, dev, data):
     newdiskdata = get_diskstats(dev)
     for key in olddiskdata.iterkeys():
         diskdata["Start"+key] = str(int(olddiskdata[key]))
-        diskdata["End"+key] = str(int(newdiskdata[key]))    
+        diskdata["End"+key] = str(int(newdiskdata[key]))
     return diskdata
-    
+
 def set_timedata(var, data, server_time=None):
     import time
     if server_time:
@@ -137,7 +137,7 @@ def get_timedata(var, data, server_time=None):
     else:
         cpuperc = None
     return timediff, cpuperc
-    
+
 def write_task_data(status, logfile, dev, e):
     bn = get_bn(e)
     bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
@@ -164,7 +164,7 @@ def write_task_data(status, logfile, dev, e):
             for key in sorted(diskdata.iterkeys()):
                 file.write(key + ": " + diskdata[key] + "\n")
     if status is "passed":
-	    file.write("Status: PASSED \n")
+        file.write("Status: PASSED \n")
     else:
         file.write("Status: FAILED \n")
     file.write("Ended: %0.2f \n" % e.time)
@@ -189,7 +189,7 @@ python run_buildstats () {
         bn = get_bn(e)
         set_device(e)
         device = get_device(e)
-        
+
         bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
         try:
             bb.utils.mkdirhier(bsdir)
@@ -209,7 +209,7 @@ python run_buildstats () {
         file.write("\n")
         file.write("Build Started: %0.2f \n" % time.time())
         file.close()
-                
+
     elif isinstance(e, bb.event.BuildCompleted):
         bn = get_bn(e)
         device = get_device(e)
@@ -260,10 +260,10 @@ python run_buildstats () {
         write_task_data("passed", os.path.join(taskdir, e.task), device, e)
         if e.task == "do_rootfs":
             bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
-            bs=os.path.join(bsdir, "build_stats")
-            file = open(bs,"a") 
+            bs = os.path.join(bsdir, "build_stats")
+            file = open(bs,"a")
             rootfs = e.data.getVar('IMAGE_ROOTFS', True)
-            rootfs_size = subprocess.Popen(["du", "-sh", rootfs], stdout=subprocess.PIPE).stdout.read() 
+            rootfs_size = subprocess.Popen(["du", "-sh", rootfs], stdout=subprocess.PIPE).stdout.read()
             file.write("Uncompressed Rootfs size: %s" % rootfs_size)
             file.close()
 
@@ -274,8 +274,8 @@ python run_buildstats () {
         taskdir = os.path.join(bsdir, e.data.expand("${PF}"))
         write_task_data("failed", os.path.join(taskdir, e.task), device, e)
         ########################################################################
-        # Lets make things easier and tell people where the build failed in 
-        # build_status. We do this here because BuildCompleted triggers no 
+        # Lets make things easier and tell people where the build failed in
+        # build_status. We do this here because BuildCompleted triggers no
         # matter what the status of the build actually is
         ########################################################################
         build_status = os.path.join(bsdir, "build_stats")
-- 
2.1.0

[PATCH 2/3] classes/buildstats: don't catch all errors from mkdirhier

[Paul Eggleton]

mkdirhier() doesn't raise an exception if the directory exists, so if it
does raise one we should just be failing, otherwise we're just going
to hit errors later anyway.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/classes/buildstats.bbclass | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/meta/classes/buildstats.bbclass b/meta/classes/buildstats.bbclass
index f15c45d..54d9f72 100644
--- a/meta/classes/buildstats.bbclass
+++ b/meta/classes/buildstats.bbclass
@@ -181,20 +181,14 @@ python run_buildstats () {
         # at first pass make the buildstats heriarchy and then
         # set the buildname
         ########################################################################
-        try:
-            bb.utils.mkdirhier(e.data.getVar('BUILDSTATS_BASE', True))
-        except:
-            pass
+        bb.utils.mkdirhier(e.data.getVar('BUILDSTATS_BASE', True))
         set_bn(e)
         bn = get_bn(e)
         set_device(e)
         device = get_device(e)
 
         bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
-        try:
-            bb.utils.mkdirhier(bsdir)
-        except:
-            pass
+        bb.utils.mkdirhier(bsdir)
         if device != "NoLogicalDevice":
             set_diskdata("__diskdata_build", device, e.data)
         set_timedata("__timedata_build", e.data)
@@ -242,10 +236,7 @@ python run_buildstats () {
         if device != "NoLogicalDevice":
             set_diskdata("__diskdata_task", device, e.data)
         set_timedata("__timedata_task", e.data, e.time)
-        try:
-            bb.utils.mkdirhier(taskdir)
-        except:
-            pass
+        bb.utils.mkdirhier(taskdir)
         # write into the task event file the name and start time
         file = open(os.path.join(taskdir, e.task), "a")
         file.write("Event: %s \n" % bb.event.getName(e))
-- 
2.1.0

[PATCH 3/3] classes/buildstats: misc cleanups

[Paul Eggleton]

* Rename BNFILE and DEVFILE so that they are clearly specific to this
  class, since they end up in the global scope
* Use "with open" when opening files consistently
* Use getVar('PF', True) instead of expand('${PF}')
* Drop some unnecessary assignments

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/classes/buildstats.bbclass | 156 +++++++++++++++++++---------------------
 1 file changed, 73 insertions(+), 83 deletions(-)

diff --git a/meta/classes/buildstats.bbclass b/meta/classes/buildstats.bbclass
index 54d9f72..22ec571 100644
--- a/meta/classes/buildstats.bbclass
+++ b/meta/classes/buildstats.bbclass
@@ -1,6 +1,6 @@
 BUILDSTATS_BASE = "${TMPDIR}/buildstats/"
-BNFILE = "${BUILDSTATS_BASE}/.buildname"
-DEVFILE = "${BUILDSTATS_BASE}/.device"
+BUILDSTATS_BNFILE = "${BUILDSTATS_BASE}/.buildname"
+BUILDSTATS_DEVFILE = "${BUILDSTATS_BASE}/.device"
 
 ################################################################################
 # Build statistics gathering.
@@ -25,21 +25,22 @@ def get_cputime():
 def set_bn(e):
     bn = e.getPkgs()[0] + "-" + e.data.getVar('MACHINE', True)
     try:
-        os.remove(e.data.getVar('BNFILE', True))
+        os.remove(e.data.getVar('BUILDSTATS_BNFILE', True))
     except:
         pass
-    with open(e.data.getVar('BNFILE', True), "w") as f:
+    with open(e.data.getVar('BUILDSTATS_BNFILE', True), "w") as f:
         f.write(os.path.join(bn, e.data.getVar('BUILDNAME', True)))
 
 def get_bn(e):
-    with open(e.data.getVar('BNFILE', True)) as f:
+    with open(e.data.getVar('BUILDSTATS_BNFILE', True)) as f:
         bn = f.readline()
     return bn
 
 def set_device(e):
     tmpdir = e.data.getVar('TMPDIR', True)
+    devfile = e.data.getVar('BUILDSTATS_DEVFILE', True)
     try:
-        os.remove(e.data.getVar('DEVFILE', True))
+        os.remove(devfile)
     except:
         pass
     ############################################################################
@@ -69,14 +70,12 @@ def set_device(e):
                     rdev = line.split()[2]
     except:
         pass
-    file = open(e.data.getVar('DEVFILE', True), "w")
-    file.write(rdev)
-    file.close()
+    with open(devfile, "w") as f:
+        f.write(rdev)
 
 def get_device(e):
-    file = open(e.data.getVar('DEVFILE', True))
-    device = file.readline()
-    file.close()
+    with open(e.data.getVar('BUILDSTATS_DEVFILE', True)) as f:
+        device = f.readline()
     return device
 
 def get_diskstats(dev):
@@ -141,34 +140,32 @@ def get_timedata(var, data, server_time=None):
 def write_task_data(status, logfile, dev, e):
     bn = get_bn(e)
     bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
-    taskdir = os.path.join(bsdir, e.data.expand("${PF}"))
-    file = open(os.path.join(logfile), "a")
-    timedata = get_timedata("__timedata_task", e.data, e.time)
-    if timedata:
-        elapsedtime, cpu = timedata
-        file.write(bb.data.expand("${PF}: %s: Elapsed time: %0.2f seconds \n" %
-                                 (e.task, elapsedtime), e.data))
-        if cpu:
-            file.write("CPU usage: %0.1f%% \n" % cpu)
-    ############################################################################
-    # Here we gather up disk data. In an effort to avoid lying with stats
-    # I do a bare minimum of analysis of collected data.
-    # The simple fact is, doing disk io collection on a per process basis
-    # without effecting build time would be difficult.
-    # For the best information, running things with BB_TOTAL_THREADS = "1"
-    # would return accurate per task results.
-    ############################################################################
-    if dev != "NoLogicalDevice":
-        diskdata = get_diskdata("__diskdata_task", dev, e.data)
-        if diskdata:
-            for key in sorted(diskdata.iterkeys()):
-                file.write(key + ": " + diskdata[key] + "\n")
-    if status is "passed":
-        file.write("Status: PASSED \n")
-    else:
-        file.write("Status: FAILED \n")
-    file.write("Ended: %0.2f \n" % e.time)
-    file.close()
+    with open(os.path.join(logfile), "a") as f:
+        timedata = get_timedata("__timedata_task", e.data, e.time)
+        if timedata:
+            elapsedtime, cpu = timedata
+            f.write(bb.data.expand("${PF}: %s: Elapsed time: %0.2f seconds \n" %
+                                    (e.task, elapsedtime), e.data))
+            if cpu:
+                f.write("CPU usage: %0.1f%% \n" % cpu)
+        ############################################################################
+        # Here we gather up disk data. In an effort to avoid lying with stats
+        # I do a bare minimum of analysis of collected data.
+        # The simple fact is, doing disk io collection on a per process basis
+        # without effecting build time would be difficult.
+        # For the best information, running things with BB_TOTAL_THREADS = "1"
+        # would return accurate per task results.
+        ############################################################################
+        if dev != "NoLogicalDevice":
+            diskdata = get_diskdata("__diskdata_task", dev, e.data)
+            if diskdata:
+                for key in sorted(diskdata.iterkeys()):
+                    f.write(key + ": " + diskdata[key] + "\n")
+        if status is "passed":
+            f.write("Status: PASSED \n")
+        else:
+            f.write("Status: FAILED \n")
+        f.write("Ended: %0.2f \n" % e.time)
 
 python run_buildstats () {
     import bb.build
@@ -194,75 +191,69 @@ python run_buildstats () {
         set_timedata("__timedata_build", e.data)
         build_time = os.path.join(bsdir, "build_stats")
         # write start of build into build_time
-        file = open(build_time,"a")
-        host_info = platform.uname()
-        file.write("Host Info: ")
-        for x in host_info:
-            if x:
-                file.write(x + " ")
-        file.write("\n")
-        file.write("Build Started: %0.2f \n" % time.time())
-        file.close()
+        with open(build_time, "a") as f:
+            host_info = platform.uname()
+            f.write("Host Info: ")
+            for x in host_info:
+                if x:
+                    f.write(x + " ")
+            f.write("\n")
+            f.write("Build Started: %0.2f \n" % time.time())
 
     elif isinstance(e, bb.event.BuildCompleted):
         bn = get_bn(e)
         device = get_device(e)
         bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
-        taskdir = os.path.join(bsdir, e.data.expand("${PF}"))
         build_time = os.path.join(bsdir, "build_stats")
-        file = open(build_time, "a")
-        ########################################################################
-        # Write build statistics for the build
-        ########################################################################
-        timedata = get_timedata("__timedata_build", e.data)
-        if timedata:
-            time, cpu = timedata
-            # write end of build and cpu used into build_time
-            file.write("Elapsed time: %0.2f seconds \n" % (time))
-            if cpu:
-                file.write("CPU usage: %0.1f%% \n" % cpu)
-        if device != "NoLogicalDevice":
-            diskio = get_diskdata("__diskdata_build", device, e.data)
-            if diskio:
-                for key in sorted(diskio.iterkeys()):
-                    file.write(key + ": " + diskio[key] + "\n")
-        file.close()
+        with open(build_time, "a") as f:
+            ########################################################################
+            # Write build statistics for the build
+            ########################################################################
+            timedata = get_timedata("__timedata_build", e.data)
+            if timedata:
+                time, cpu = timedata
+                # write end of build and cpu used into build_time
+                f.write("Elapsed time: %0.2f seconds \n" % (time))
+                if cpu:
+                    f.write("CPU usage: %0.1f%% \n" % cpu)
+            if device != "NoLogicalDevice":
+                diskio = get_diskdata("__diskdata_build", device, e.data)
+                if diskio:
+                    for key in sorted(diskio.iterkeys()):
+                        f.write(key + ": " + diskio[key] + "\n")
 
     if isinstance(e, bb.build.TaskStarted):
         bn = get_bn(e)
         device = get_device(e)
         bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
-        taskdir = os.path.join(bsdir, e.data.expand("${PF}"))
+        taskdir = os.path.join(bsdir, e.data.getVar('PF', True))
         if device != "NoLogicalDevice":
             set_diskdata("__diskdata_task", device, e.data)
         set_timedata("__timedata_task", e.data, e.time)
         bb.utils.mkdirhier(taskdir)
         # write into the task event file the name and start time
-        file = open(os.path.join(taskdir, e.task), "a")
-        file.write("Event: %s \n" % bb.event.getName(e))
-        file.write("Started: %0.2f \n" % e.time)
-        file.close()
+        with open(os.path.join(taskdir, e.task), "a") as f:
+            f.write("Event: %s \n" % bb.event.getName(e))
+            f.write("Started: %0.2f \n" % e.time)
 
     elif isinstance(e, bb.build.TaskSucceeded):
         bn = get_bn(e)
         device = get_device(e)
         bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
-        taskdir = os.path.join(bsdir, e.data.expand("${PF}"))
+        taskdir = os.path.join(bsdir, e.data.getVar('PF', True))
         write_task_data("passed", os.path.join(taskdir, e.task), device, e)
         if e.task == "do_rootfs":
-            bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
             bs = os.path.join(bsdir, "build_stats")
-            file = open(bs,"a")
-            rootfs = e.data.getVar('IMAGE_ROOTFS', True)
-            rootfs_size = subprocess.Popen(["du", "-sh", rootfs], stdout=subprocess.PIPE).stdout.read()
-            file.write("Uncompressed Rootfs size: %s" % rootfs_size)
-            file.close()
+            with open(bs, "a") as f:
+                rootfs = e.data.getVar('IMAGE_ROOTFS', True)
+                rootfs_size = subprocess.Popen(["du", "-sh", rootfs], stdout=subprocess.PIPE).stdout.read()
+                f.write("Uncompressed Rootfs size: %s" % rootfs_size)
 
     elif isinstance(e, bb.build.TaskFailed):
         bn = get_bn(e)
         device = get_device(e)
         bsdir = os.path.join(e.data.getVar('BUILDSTATS_BASE', True), bn)
-        taskdir = os.path.join(bsdir, e.data.expand("${PF}"))
+        taskdir = os.path.join(bsdir, e.data.getVar('PF', True))
         write_task_data("failed", os.path.join(taskdir, e.task), device, e)
         ########################################################################
         # Lets make things easier and tell people where the build failed in
@@ -270,9 +261,8 @@ python run_buildstats () {
         # matter what the status of the build actually is
         ########################################################################
         build_status = os.path.join(bsdir, "build_stats")
-        file = open(build_status,"a")
-        file.write(e.data.expand("Failed at: ${PF} at task: %s \n" % e.task))
-        file.close()
+        with open(build_status, "a") as f:
+            f.write(e.data.expand("Failed at: ${PF} at task: %s \n" % e.task))
 }
 
 addhandler run_buildstats
-- 
2.1.0