[PATCHv2 0/4] recipeutils and distrodata fixes

[PATCHv2 0/4] recipeutils and distrodata fixes

[Aníbal Limón]

  xserver-xorg: Upgrade 1.16.3 -> 1.17.1 (OELAYOUT_ABI change) (2015-06-03 16:38:49 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib alimon/distrodata
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=alimon/distrodata

Aníbal Limón (4):
  recipeutils: Improve get_recipe_pv_without_srcpv function
  distrodata: Remove unnecessary include of package_regex.inc
  distrodata: checkpkg make usage of
    oe.recipeutils.get_recipe_upstream_version
  distrodata: Use Python CSV instead of did by hand

 meta/classes/distrodata.bbclass | 210 +++++++++++++++++++---------------------
 meta/lib/oe/recipeutils.py      |  12 +--
 2 files changed, 103 insertions(+), 119 deletions(-)

-- 
1.9.1

[PATCHv2 1/4] recipeutils: Improve get_recipe_pv_without_srcpv function

[Aníbal Limón]

Use pv instead of rd this make the function more generic and
avoid copy recipe data.

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 meta/lib/oe/recipeutils.py | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/meta/lib/oe/recipeutils.py b/meta/lib/oe/recipeutils.py
index 37efefb..26bbf3e 100644
--- a/meta/lib/oe/recipeutils.py
+++ b/meta/lib/oe/recipeutils.py
@@ -626,23 +626,17 @@ def replace_dir_vars(path, d):
         path = path.replace(dirpath, '${%s}' % dirvars[dirpath])
     return path
 
-def get_recipe_pv_without_srcpv(rd, uri_type):
+def get_recipe_pv_without_srcpv(pv, uri_type):
     """
     Get PV without SRCPV common in SCM's for now only
     support git.
 
     Returns tuple with pv, prefix and suffix.
     """
-    pv = ''
     pfx = ''
     sfx = ''
 
     if uri_type == 'git':
-        rd_tmp = rd.createCopy()
-
-        rd_tmp.setVar('SRCPV', '')
-        pv = rd_tmp.getVar('PV', True)
-
         git_regex = re.compile("(?P<pfx>(v|))(?P<ver>((\d+[\.\-_]*)+))(?P<sfx>(\+|)(git|)(r|)(AUTOINC|)(\+|))(?P<rev>.*)")
         m = git_regex.match(pv)
 
@@ -650,8 +644,6 @@ def get_recipe_pv_without_srcpv(rd, uri_type):
             pv = m.group('ver')
             pfx = m.group('pfx')
             sfx = m.group('sfx')
-    else:
-        pv = rd.getVar('PV', True)
 
     return (pv, pfx, sfx)
 
@@ -704,7 +696,7 @@ def get_recipe_upstream_version(rd):
         pupver = ud.method.latest_versionstring(ud, rd)
 
         if uri_type == 'git':
-            (pv, pfx, sfx) = get_recipe_pv_without_srcpv(rd, uri_type)
+            (pv, pfx, sfx) = get_recipe_pv_without_srcpv(pv, uri_type)
 
             latest_revision = ud.method.latest_revision(ud, rd, ud.names[0])
 
-- 
1.9.1

[PATCHv2 2/4] distrodata: Remove unnecessary include of package_regex.inc

[Aníbal Limón]

This causes a warning when follow documentation to use distrodata
class that points to include,

	include conf/distro/include/distro_alias.inc
	include conf/distro/include/recipe_color.inc
	include conf/distro/include/maintainers.inc
	include conf/distro/include/upstream_tracking.inc
	include conf/distro/include/package_regex.inc
	INHERIT+= "distrodata"

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 meta/classes/distrodata.bbclass | 1 -
 1 file changed, 1 deletion(-)

diff --git a/meta/classes/distrodata.bbclass b/meta/classes/distrodata.bbclass
index 83aa381..e1fc6dd 100644
--- a/meta/classes/distrodata.bbclass
+++ b/meta/classes/distrodata.bbclass
@@ -1,4 +1,3 @@
-include conf/distro/include/package_regex.inc
 addhandler distro_eventhandler
 distro_eventhandler[eventmask] = "bb.event.BuildStarted"
 python distro_eventhandler() {
-- 
1.9.1

[PATCHv2 3/4] distrodata: checkpkg make usage of oe.recipeutils.get_recipe_upstream_version

[Aníbal Limón]

Now get_recipe_upstream_version function exists in oe.recipeutils module
to avoid duplicate code make usage of it.

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 meta/classes/distrodata.bbclass | 84 ++++++++++++++++++-----------------------
 1 file changed, 37 insertions(+), 47 deletions(-)

diff --git a/meta/classes/distrodata.bbclass b/meta/classes/distrodata.bbclass
index e1fc6dd..092c372 100644
--- a/meta/classes/distrodata.bbclass
+++ b/meta/classes/distrodata.bbclass
@@ -266,11 +266,15 @@ python do_checkpkg() {
         import re
         import tempfile
         import subprocess
+        import oe.recipeutils
+        from bb.utils import vercmp_string
+        from bb.fetch2 import FetchError, NoMethodError, decodeurl
 
         """first check whether a uri is provided"""
         src_uri = d.getVar('SRC_URI', True)
         if not src_uri:
                 return
+        uri_type, _, _, _, _, _ = decodeurl(src_uri)
 
         """initialize log files."""
         logpath = d.getVar('LOG_DIR', True)
@@ -310,10 +314,7 @@ python do_checkpkg() {
 
         pdesc = localdata.getVar('DESCRIPTION', True)
         pgrp = localdata.getVar('SECTION', True)
-        if localdata.getVar('PRSPV', True):
-                pversion = localdata.getVar('PRSPV', True)
-        else:
-                pversion = localdata.getVar('PV', True)
+        pversion = localdata.getVar('PV', True)
         plicense = localdata.getVar('LICENSE', True)
         psection = localdata.getVar('SECTION', True)
         phome = localdata.getVar('HOMEPAGE', True)
@@ -325,61 +326,50 @@ python do_checkpkg() {
         maintainer = localdata.getVar('RECIPE_MAINTAINER', True)
 
         """ Get upstream version version """
-        pupver = None
-        pstatus = "ErrUnknown"
-        found = 0
-
-        for uri in src_uri.split():
-            m = re.compile('(?P<type>[^:]*)').match(uri)
-            if not m:
-                raise MalformedUrl(uri)
-            elif m.group('type') in ('http', 'https', 'ftp', 'cvs', 'svn', 'git'):
-                found = 1
-                psrcuri = uri
-                pproto = m.group('type')
-                break
-        if not found:
-                pproto = "file"
-
-        if pproto in ['http', 'https', 'ftp', 'git']:
-            try:
-                ud = bb.fetch2.FetchData(psrcuri, d)
-                pupver = ud.method.latest_versionstring(ud, d)
-                if pproto == 'git':
-                    if pupver == "":
-                        pupver = pversion.rsplit("+")[0]
-                    if re.search(pversion, "gitrAUTOINC"):
-                        pupver += "+gitrAUTOINC+"
-                    else:
-                        pupver += "+gitAUTOINC+"
-                    latest_revision = ud.method.latest_revision(ud, d, ud.names[0])
-                    pupver += latest_revision[:10]
-            except Exception as inst:
-                bb.warn("%s: unexpected error: %s" % (pname, repr(inst)))
+        pupver = ""
+        pstatus = ""
+
+        try:
+            uv = oe.recipeutils.get_recipe_upstream_version(localdata)
+
+            pupver = uv['version']
+        except Exception as e:
+            if e is FetchError:
                 pstatus = "ErrAccess"
-        elif pproto == "file":
-            """Local files are always updated"""
-            pupver = pversion
-        else:
-            pstatus = "ErrUnsupportedProto"
-            bb.note("do_checkpkg, protocol %s isn't implemented" % pproto)
+            elif e is NoMethodError:
+                pstatus = "ErrUnsupportedProto"
+            else:
+                pstatus = "ErrUnknown"
 
+        """Set upstream version status"""
         if not pupver:
             pupver = "N/A"
-        elif pupver == pversion:
-            pstatus = "MATCH"
         else:
-            pstatus = "UPDATE"
+            pv, _, _ = oe.recipeutils.get_recipe_pv_without_srcpv(pversion, uri_type)
+            upv, _, _ = oe.recipeutils.get_recipe_pv_without_srcpv(pupver, uri_type)
+
+            cmp = vercmp_string(pv, upv)
+            if cmp == -1:
+                pstatus = "UPDATE"
+            elif cmp == 0:
+                pstatus = "MATCH"
 
         """Read from manual distro tracking fields as alternative"""
         pmver = d.getVar("RECIPE_UPSTREAM_VERSION", True)
         if not pmver:
             pmver = "N/A"
             pmstatus = "ErrNoRecipeData"
-        elif pmver == pupver:
-            pmstatus = "MATCH"
         else:
-            pmstatus = "UPDATE"
+            mpv, _, _ = oe.recipeutils.get_recipe_pv_without_srcpv(pmver, uri_type)
+            upv, _, _ = oe.recipeutils.get_recipe_pv_without_srcpv(pupver, uri_type)
+
+            cmp = vercmp_string(mpv, upv)
+            if cmp == -1:
+                pmstatus = "UPDATE"
+            elif cmp == 0:
+                pmstatus = "MATCH"
+            else:
+                pmstatus = ""
 
         pdepends = "".join(pdepends.split("\t"))
         pdesc = "".join(pdesc.split("\t"))
-- 
1.9.1

[PATCHv2 4/4] distrodata: Use Python CSV instead of did by hand

[Aníbal Limón]

Fix CSV generation in distrodata class using Python CSV
module before it some errors happen when read due to
incorrect quoting/delimiters.

[YOCTO #7777]

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 meta/classes/distrodata.bbclass | 125 ++++++++++++++++++++--------------------
 1 file changed, 64 insertions(+), 61 deletions(-)

diff --git a/meta/classes/distrodata.bbclass b/meta/classes/distrodata.bbclass
index 092c372..0cefa7a 100644
--- a/meta/classes/distrodata.bbclass
+++ b/meta/classes/distrodata.bbclass
@@ -2,11 +2,16 @@ addhandler distro_eventhandler
 distro_eventhandler[eventmask] = "bb.event.BuildStarted"
 python distro_eventhandler() {
     import oe.distro_check as dc
+    import csv
     logfile = dc.create_log_file(e.data, "distrodata.csv")
+
     lf = bb.utils.lockfile("%s.lock" % logfile)
-    f = open(logfile, "a")
-    f.write("Package,Description,Owner,License,VerMatch,Version,Upsteam,Reason,Recipe Status,Distro 1,Distro 2,Distro 3\n")
-    f.close()
+    with open(logfile, "a") as f:
+        writer = csv.writer(f)
+        writer.writerow(['Package', 'Description', 'Owner', 'License', 
+            'VerMatch', 'Version', 'Upsteam', 'Reason', 'Recipe Status',
+            'Distro 1', 'Distro 2', 'Distro 3'])
+        f.close()
     bb.utils.unlockfile(lf)
 
     return
@@ -98,6 +103,7 @@ python do_distrodata_np() {
 addtask distrodata
 do_distrodata[nostamp] = "1"
 python do_distrodata() {
+        import csv
         logpath = d.getVar('LOG_DIR', True)
         bb.utils.mkdirhier(logpath)
         logfile = os.path.join(logpath, "distrodata.csv")
@@ -176,14 +182,13 @@ python do_distrodata() {
         result = dist_check.compare_in_distro_packages_list(distro_check_dir, localdata)
 
         lf = bb.utils.lockfile("%s.lock" % logfile)
-        f = open(logfile, "a")
-        f.write("%s,%s,%s,%s,%s,%s,%s,%s,%s" % \
-                  (pname, pdesc, maintainer, plicense, vermatch, pcurver, pupver, noupdate_reason, rstatus))
-        line = ""
-        for i in result:
-            line = line + "," + i
-        f.write(line + "\n")
-        f.close()
+        with open(logfile, "a") as f:
+            row = [pname, pdesc, maintainer, plicense, vermatch, pcurver, pupver, noupdate_reason, rstatus]
+            row.extend(result)
+
+            writer = csv.writer(f)
+            writer.writerow(row)
+            f.close()
         bb.utils.unlockfile(lf)
 }
 
@@ -198,45 +203,33 @@ do_distrodataall() {
 addhandler checkpkg_eventhandler
 checkpkg_eventhandler[eventmask] = "bb.event.BuildStarted bb.event.BuildCompleted"
 python checkpkg_eventhandler() {
+    import csv
+
     def parse_csv_file(filename):
         package_dict = {}
-        fd = open(filename, "r")
-        lines = fd.read().rsplit("\n")
-        fd.close()
-
-        first_line = ''
-        index = 0
-        for line in lines:
-            #Skip the first line
-            if index == 0:
-                first_line = line
-                index += 1
-                continue
-            elif line == '':
-                continue
-            index += 1
-            package_name = line.rsplit("\t")[0]
-            if '-native' in package_name or 'nativesdk-' in package_name:
-                original_name = package_name.rsplit('-native')[0]
-                if original_name == '':
-                    original_name = package_name.rsplit('nativesdk-')[0]
-                if original_name in package_dict:
+
+        with open(filename, "r") as f:
+            reader = csv.reader(f, delimiter='\t')
+            for row in reader:
+                pn = row[0]
+
+                if reader.line_num == 1:
+                    header = row
                     continue
-                else:
-                    package_dict[package_name] = line
-            else:
-                new_name = package_name + "-native"
-                if not(new_name in package_dict):
-                    new_name = 'nativesdk-' + package_name
-                if new_name in package_dict:
-                    del package_dict[new_name]
-                package_dict[package_name] = line
-
-        fd = open(filename, "w")
-        fd.write("%s\n"%first_line)
-        for el in package_dict:
-            fd.write(package_dict[el] + "\n")
-        fd.close()
+
+                if '-native' in pn or 'nativesdk-' in pn:
+                    continue
+
+                if not pn in package_dict.keys():
+                    package_dict[pn] = row
+            f.close()
+
+        with open(filename, "w") as f:
+            writer = csv.writer(f, delimiter='\t')
+            writer.writerow(header)
+            for pn in package_dict.keys():
+                writer.writerow(package_dict[pn])
+            f.close()
 
         del package_dict
 
@@ -245,9 +238,13 @@ python checkpkg_eventhandler() {
         logfile = dc.create_log_file(e.data, "checkpkg.csv")
 
         lf = bb.utils.lockfile("%s.lock" % logfile)
-        f = open(logfile, "a")
-        f.write("Package\tVersion\tUpver\tLicense\tSection\tHome\tRelease\tDepends\tBugTracker\tPE\tDescription\tStatus\tTracking\tURI\tMAINTAINER\tNoUpReason\n")
-        f.close()
+        with open(logfile, "a") as f:
+            writer = csv.writer(f, delimiter='\t')
+            headers = ['Package', 'Version', 'Upver', 'License', 'Section',
+                'Home', 'Release', 'Depends', 'BugTracker', 'PE', 'Description',
+                'Status', 'Tracking', 'URI', 'MAINTAINER', 'NoUpReason']
+            writer.writerow(headers)
+            f.close()
         bb.utils.unlockfile(lf)
     elif bb.event.getName(e) == "BuildCompleted":
         import os
@@ -263,6 +260,7 @@ addtask checkpkg
 do_checkpkg[nostamp] = "1"
 python do_checkpkg() {
         localdata = bb.data.createCopy(d)
+        import csv
         import re
         import tempfile
         import subprocess
@@ -375,10 +373,12 @@ python do_checkpkg() {
         pdesc = "".join(pdesc.split("\t"))
         no_upgr_reason = d.getVar('RECIPE_NO_UPDATE_REASON', True)
         lf = bb.utils.lockfile("%s.lock" % logfile)
-        f = open(logfile, "a")
-        f.write("%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n" % \
-                  (pname,pversion,pupver,plicense,psection, phome,prelease, pdepends,pbugtracker,ppe,pdesc,pstatus,pmver,psrcuri,maintainer, no_upgr_reason))
-        f.close()
+        with open(logfile, "a") as f:
+            writer = csv.writer(f, delimiter='\t')
+            writer.writerow([pname, pversion, pupver, plicense, psection, phome, 
+                prelease, pdepends, pbugtracker, ppe, pdesc, pstatus, pmver, 
+                psrcuri, maintainer, no_upgr_reason])
+            f.close()
         bb.utils.unlockfile(lf)
 }
 
@@ -441,12 +441,14 @@ addhandler checklicense_eventhandler
 checklicense_eventhandler[eventmask] = "bb.event.BuildStarted"
 python checklicense_eventhandler() {
     """initialize log files."""
+    import csv
     import oe.distro_check as dc
     logfile = dc.create_log_file(e.data, "missinglicense.csv")
     lf = bb.utils.lockfile("%s.lock" % logfile)
-    f = open(logfile, "a")
-    f.write("Package\tLicense\tMissingLicense\n")
-    f.close()
+    with open(logfile, "a") as f:
+        writer = csv.writer(f, delimiter='\t')
+        writer.writerow(['Package', 'License', 'MissingLicense'])
+        f.close()
     bb.utils.unlockfile(lf)
     return
 }
@@ -454,6 +456,7 @@ python checklicense_eventhandler() {
 addtask checklicense
 do_checklicense[nostamp] = "1"
 python do_checklicense() {
+    import csv
     import shutil
     logpath = d.getVar('LOG_DIR', True)
     bb.utils.mkdirhier(logpath)
@@ -466,10 +469,10 @@ python do_checklicense() {
                           .replace(',', '').replace(" ", "").split("&"))):
         if not os.path.isfile(os.path.join(generic_directory, license_type)):
             lf = bb.utils.lockfile("%s.lock" % logfile)
-            f = open(logfile, "a")
-            f.write("%s\t%s\t%s\n" % \
-                (pn,license_types,license_type))
-            f.close()
+            with open(logfile, "a") as f:
+                writer = csv.writer(f, delimiter='\t')
+                writer.writerow([pn, license_types, license_type])
+                f.close()
             bb.utils.unlockfile(lf)
     return
 }
-- 
1.9.1

Re: [PATCHv2 4/4] distrodata: Use Python CSV instead of did by hand

[Burton, Ross]

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

On 4 June 2015 at 19:42, Aníbal Limón <anibal.limon@linux.intel.com> wrote:

> Fix CSV generation in distrodata class using Python CSV
> module before it some errors happen when read due to
> incorrect quoting/delimiters.
>

So whereas before the SRC_URI field in checkpkg.cvs generally just listed
the first entry (the main source) it now lists the entire of SRC_URI.
Presumably this bug fix is desired behaviour?

Ross

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

Re: [PATCHv2 4/4] distrodata: Use Python CSV instead of did by hand

[Burton, Ross]

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

On 4 June 2015 at 21:06, Burton, Ross <ross.burton@intel.com> wrote:

> So whereas before the SRC_URI field in checkpkg.cvs generally just listed
> the first entry (the main source) it now lists the entire of SRC_URI.
> Presumably this bug fix is desired behaviour?
>

Also fields that used to say "None" are now empty.  Again, this seems like
a fix to me, although it changes behaviour.

Ross

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

Re: [PATCHv2 4/4] distrodata: Use Python CSV instead of did by hand

[Aníbal Limón]

Hi Ross,

Comments below,

     alimon

On 04/06/15 15:08, Burton, Ross wrote:
> On 4 June 2015 at 21:06, Burton, Ross <ross.burton@intel.com> wrote:
>
>> So whereas before the SRC_URI field in checkpkg.cvs generally just listed
>> the first entry (the main source) it now lists the entire of SRC_URI.
>> Presumably this bug fix is desired behaviour?
No i made a mistake here thanks for notice, sending v3...
> Also fields that used to say "None" are now empty.  Again, this seems like
> a fix to me, although it changes behaviour.
Yes it changes the behavior but it is right to me also.
>
> Ross
>

Re: [PATCHv2 2/4] distrodata: Remove unnecessary include of package_regex.inc

[Burton, Ross]

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

On 4 June 2015 at 19:42, Aníbal Limón <anibal.limon@linux.intel.com> wrote:

> This causes a warning when follow documentation to use distrodata
> class that points to include,
>
>         include conf/distro/include/distro_alias.inc
>         include conf/distro/include/recipe_color.inc
>         include conf/distro/include/maintainers.inc
>         include conf/distro/include/upstream_tracking.inc
>         include conf/distro/include/package_regex.inc
>         INHERIT+= "distrodata"
>

Can we revert this? Instead make the class include the configuration files
that it needs, and update the documentation.  As Paul pointed out in a bug
earlier several of these files can be deleted now anyway, and having
distrodata working with just a simple INHERIT is much neater.

(by which I mean: I've reverted this in master-under-test, shout if you
have a good argument against this)

Ross

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