Skip to contentSkip to conversation

recipeutils and distrodata fixes

PATCH In review Aníbal Limón 2 participants 9 messages Jun 4, 2015 – Jun 16, 2015

lore
Share & tools
lore
#0 lore raw

v2 cover recipeutils and distrodata fixes

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(-)
Signature · 1 line
1.9.1
#1 lore raw

PATCH v2 1/4 recipeutils: Improve get_recipe_pv_without_srcpv function

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

---
 meta/lib/oe/recipeutils.py | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)
meta/lib/oe/recipeutils.py +2 -10
index 37efefb..26bbf3e 100644
@@ -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])
 
Signature · 1 line
1.9.1
#2 lore raw

PATCH v2 2/4 distrodata: Remove unnecessary include of package_regex.inc

In reply to 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"
---
 meta/classes/distrodata.bbclass | 1 -
 1 file changed, 1 deletion(-)
meta/classes/distrodata.bbclass +0 -1
index 83aa381..e1fc6dd 100644
@@ -1,4 +1,3 @@
-include conf/distro/include/package_regex.inc
 addhandler distro_eventhandler
 distro_eventhandler[eventmask] = "bb.event.BuildStarted"
 python distro_eventhandler() {
Signature · 1 line
1.9.1
#4 lore raw

PATCH v2 3/4 distrodata: checkpkg make usage of oe.recipeutils.get_recipe_upstream_version

In reply to Aníbal Limón

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

---
 meta/classes/distrodata.bbclass | 84 ++++++++++++++++++-----------------------
 1 file changed, 37 insertions(+), 47 deletions(-)
meta/classes/distrodata.bbclass +37 -47
index e1fc6dd..092c372 100644
@@ -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"))
Signature · 1 line
1.9.1
#5 lore raw

PATCH v2 4/4 distrodata: Use Python CSV instead of did by hand

In reply to 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]

---
 meta/classes/distrodata.bbclass | 125 ++++++++++++++++++++--------------------
 1 file changed, 64 insertions(+), 61 deletions(-)
meta/classes/distrodata.bbclass +64 -61
index 092c372..0cefa7a 100644
@@ -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
 }
Signature · 1 line
1.9.1
#6 lore raw

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

Quoted 4 lines · 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.

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

#7 lore raw

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

Quoted 4 lines · Ross

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

#8 lore raw

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

Hi Ross,

Comments below,

alimon

Quoted 5 lines · Ross
Quoted 3 lines · Ross

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...

Quoted 2 lines

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.

Quoted 3 lines

Ross

#3 lore raw

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

In reply to Aníbal Limón
Quoted 10 lines · 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"

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