[auh] upgradehelper: Replace do_checkpkg usage with direct tinfoil calls

Submitted by Richard Purdie on Dec. 14, 2018, 4:07 p.m. | Patch ID: 157135

Details

Message ID 20181214160747.4732-1-richard.purdie@linuxfoundation.org
State New
Headers show

Commit Message

Richard Purdie Dec. 14, 2018, 4:07 p.m.
The code in distrodata.bbclass related to the do_checkpkg task is rather
dated, has holes in it (ignoring some recipes) and has horrible locking
and csv related issues.

We should use modern APIs such as tinfoil to make the calls we need directly
against bitbake, cutting out the middleman and clarifing the code.

This change imports the bits of distrodata.bbclass this code needs in their
current form. Its likely it can be further improved from here but this is a
good start and appears to function as before, with slightly wider recipe
coverage as some things skipped by distrodata are not skipped here (images,
pieces of gcc, nativesdk only recipes).

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 upgradehelper.py | 148 ++++++++++++++++++++++++++---------------------
 1 file changed, 82 insertions(+), 66 deletions(-)

Patch hide | download patch | download mbox

diff --git a/upgradehelper.py b/upgradehelper.py
index 8fb27d3..7ad4917 100755
--- a/upgradehelper.py
+++ b/upgradehelper.py
@@ -59,6 +59,25 @@  from statistics import Statistics
 from steps import upgrade_steps
 from testimage import TestImage
 
+if not os.getenv('BUILDDIR', False):
+    E(" You must source oe-init-build-env before running this script!\n")
+    E(" It is recommended to create a fresh build directory with it:\n")
+    E(" $ . oe-init-build-env build-auh\n")
+    exit(1)
+
+import shutil
+# Use the location of devtool to find scriptpath and hence bb/oe libs
+scripts_path = os.path.abspath(os.path.dirname(shutil.which("devtool")))
+sys.path = sys.path + [scripts_path + '/lib']
+import scriptpath
+scriptpath.add_bitbake_lib_path()
+scriptpath.add_oe_lib_path()
+
+import bb.tinfoil
+import oe.recipeutils
+from bb.utils import vercmp_string
+
+
 help_text = """Usage examples:
 * To upgrade xmodmap recipe to the latest available version:
     $ upgrade-helper.py xmodmap
@@ -599,59 +618,6 @@  class UniverseUpdater(Updater):
             I(" Removing tmp directory ...")
             shutil.rmtree(self.base_env['TMPDIR'])
 
-    def _check_upstream_versions(self):
-        I(" Fetching upstream version(s) ...")
-
-        if self.recipes:
-            recipe = " ".join(self.recipes)
-        else:
-            recipe = 'universe'
-
-        try:
-            self.bb.checkpkg(recipe)
-        except Error as e:
-            for line in e.stdout.split('\n'):
-                if line.find("ERROR: Task do_checkpkg does not exist") != -1:
-                    C(" \"distrodata.bbclass\" not inherited. Consider adding "
-                      "the following to your local.conf:\n\n"
-                      "INHERIT =+ \"distrodata\"\n")
-                    exit(1)
-
-    def _parse_checkpkg_file(self, file_path):
-        import csv
-
-        pkgs_list = []
-
-        with open(file_path, "r") as f:
-            reader = csv.reader(f, delimiter='\t')
-            for row in reader:
-                if reader.line_num == 1: # skip header line
-                    continue
-
-                pn = row[0]
-                cur_ver = row[1]
-                if self.args.to_version:
-                    next_ver = self.args.to_version
-                else:
-                    next_ver = row[2]
-                status = row[11]
-                revision = row[12]
-                maintainer = row[14]
-                no_upgrade_reason = row[15]
-
-                if status == 'UPDATE' and not no_upgrade_reason:
-                    pkgs_list.append((pn, cur_ver, next_ver, maintainer, revision))
-                else:
-                    if no_upgrade_reason:
-                        I(" Skip package %s (status = %s, current version = %s," \
-                            " next version = %s, no upgrade reason = %s)" %
-                            (pn, status, cur_ver, next_ver, no_upgrade_reason))
-                    else:
-                        I(" Skip package %s (status = %s, current version = %s," \
-                            " next version = %s)" %
-                            (pn, status, cur_ver, next_ver))
-        return pkgs_list
-
     # checks if maintainer is in whitelist and that the recipe itself is not
     # blacklisted: python, gcc, etc. Also, check the history if the recipe
     # hasn't already been tried
@@ -688,16 +654,71 @@  class UniverseUpdater(Updater):
         return True
 
     def _get_packages_to_upgrade(self, packages=None):
-        self._check_upstream_versions()
-        last_checkpkg_file = os.path.realpath(self.base_env['TMPDIR'] + "/log/checkpkg.csv")
 
         pkgs_list = []
-        for pkg in self._parse_checkpkg_file(last_checkpkg_file):
-            # Always do the upgrade if recipes are specified
-            if self.recipes and pkg[0] in self.recipes:
-                pkgs_list.append(pkg)
-            elif self._pkg_upgradable(pkg[0], pkg[2], pkg[3]):
-                pkgs_list.append(pkg)
+        with bb.tinfoil.Tinfoil() as tinfoil:
+            tinfoil.prepare(config_only=False)
+            recipes = self.recipes
+            if not recipes:
+                recipes = tinfoil.all_recipe_files(variants=False)
+
+            for fn in recipes:
+                try:
+                    if fn.startswith("/"):
+                        data = tinfoil.parse_recipe_file(fn)
+                    else:
+                        data = tinfoil.parse_recipe(fn)
+                except bb.providers.NoProvider:
+                    I(" No provider for %s" % fn)
+                    continue
+
+                unreliable = data.getVar('UPSTREAM_CHECK_UNRELIABLE')
+                if unreliable == "1":
+                    I(" Skip package %s as upstream check unreliable" % pn)
+                    continue
+
+                uv = oe.recipeutils.get_recipe_upstream_version(data)
+
+                pn = data.getVar('PN')
+                cur_ver = uv['current_version']
+
+
+                upstream_version_unknown = data.getVar('UPSTREAM_VERSION_UNKNOWN')
+                if not uv['version']:
+                    status = "UNKNOWN" if upstream_version_unknown else "UNKNOWN_BROKEN"
+                else:
+                    cmp = vercmp_string(uv['current_version'], uv['version'])
+                    if cmp == -1:
+                        status = "UPDATE" if not upstream_version_unknown else "KNOWN_BROKEN"
+                    elif cmp == 0:
+                        status = "MATCH" if not upstream_version_unknown else "KNOWN_BROKEN"
+                    else:
+                        status = "UNKNOWN" if upstream_version_unknown else "UNKNOWN_BROKEN"
+
+                if self.args.to_version:
+                    next_ver = self.args.to_version
+                else:
+                    next_ver = uv['version'] if uv['version'] else "N/A"
+                revision = uv['revision'] if uv['revision'] else "N/A"
+                maintainer = data.getVar('RECIPE_MAINTAINER')
+                no_upgrade_reason = data.getVar('RECIPE_NO_UPDATE_REASON')
+
+                if status == 'UPDATE' and not no_upgrade_reason:
+                    # Always do the upgrade if recipes are specified
+                    if self.recipes and pn in self.recipes:
+                        pkgs_list.append((pn, cur_ver, next_ver, maintainer, revision))
+
+                    elif self._pkg_upgradable(pn, next_ver, maintainer):
+                        pkgs_list.append((pn, cur_ver, next_ver, maintainer, revision))
+                else:
+                    if no_upgrade_reason:
+                        I(" Skip package %s (status = %s, current version = %s," \
+                            " next version = %s, no upgrade reason = %s)" %
+                            (pn, status, cur_ver, next_ver, no_upgrade_reason))
+                    else:
+                        I(" Skip package %s (status = %s, current version = %s," \
+                            " next version = %s)" %
+                            (pn, status, cur_ver, next_ver))
 
         return pkgs_list
 
@@ -716,11 +737,6 @@  if __name__ == "__main__":
     global settings
     global maintainer_override
 
-    if not os.getenv('BUILDDIR', False):
-        E(" You must source oe-init-build-env before running this script!\n")
-        E(" It is recommended to create a fresh build directory with it:\n")
-        E(" $ . oe-init-build-env build-auh\n")
-        exit(1)
 
     devnull = open(os.devnull, 'wb')
     if subprocess.call(["git", "config", "user.name"], stdout=devnull,stderr=devnull) or \

Comments

Aníbal Limón Dec. 14, 2018, 4:53 p.m.
LGTM,

Anibal

On Fri, Dec 14, 2018 at 10:08 AM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> The code in distrodata.bbclass related to the do_checkpkg task is rather
> dated, has holes in it (ignoring some recipes) and has horrible locking
> and csv related issues.
>
> We should use modern APIs such as tinfoil to make the calls we need
> directly
> against bitbake, cutting out the middleman and clarifing the code.
>
> This change imports the bits of distrodata.bbclass this code needs in their
> current form. Its likely it can be further improved from here but this is a
> good start and appears to function as before, with slightly wider recipe
> coverage as some things skipped by distrodata are not skipped here (images,
> pieces of gcc, nativesdk only recipes).
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  upgradehelper.py | 148 ++++++++++++++++++++++++++---------------------
>  1 file changed, 82 insertions(+), 66 deletions(-)
>
> diff --git a/upgradehelper.py b/upgradehelper.py
> index 8fb27d3..7ad4917 100755
> --- a/upgradehelper.py
> +++ b/upgradehelper.py
> @@ -59,6 +59,25 @@ from statistics import Statistics
>  from steps import upgrade_steps
>  from testimage import TestImage
>
> +if not os.getenv('BUILDDIR', False):
> +    E(" You must source oe-init-build-env before running this script!\n")
> +    E(" It is recommended to create a fresh build directory with it:\n")
> +    E(" $ . oe-init-build-env build-auh\n")
> +    exit(1)
> +
> +import shutil
> +# Use the location of devtool to find scriptpath and hence bb/oe libs
> +scripts_path = os.path.abspath(os.path.dirname(shutil.which("devtool")))
> +sys.path = sys.path + [scripts_path + '/lib']
> +import scriptpath
> +scriptpath.add_bitbake_lib_path()
> +scriptpath.add_oe_lib_path()
> +
> +import bb.tinfoil
> +import oe.recipeutils
> +from bb.utils import vercmp_string
> +
> +
>  help_text = """Usage examples:
>  * To upgrade xmodmap recipe to the latest available version:
>      $ upgrade-helper.py xmodmap
> @@ -599,59 +618,6 @@ class UniverseUpdater(Updater):
>              I(" Removing tmp directory ...")
>              shutil.rmtree(self.base_env['TMPDIR'])
>
> -    def _check_upstream_versions(self):
> -        I(" Fetching upstream version(s) ...")
> -
> -        if self.recipes:
> -            recipe = " ".join(self.recipes)
> -        else:
> -            recipe = 'universe'
> -
> -        try:
> -            self.bb.checkpkg(recipe)
> -        except Error as e:
> -            for line in e.stdout.split('\n'):
> -                if line.find("ERROR: Task do_checkpkg does not exist") !=
> -1:
> -                    C(" \"distrodata.bbclass\" not inherited. Consider
> adding "
> -                      "the following to your local.conf:\n\n"
> -                      "INHERIT =+ \"distrodata\"\n")
> -                    exit(1)
> -
> -    def _parse_checkpkg_file(self, file_path):
> -        import csv
> -
> -        pkgs_list = []
> -
> -        with open(file_path, "r") as f:
> -            reader = csv.reader(f, delimiter='\t')
> -            for row in reader:
> -                if reader.line_num == 1: # skip header line
> -                    continue
> -
> -                pn = row[0]
> -                cur_ver = row[1]
> -                if self.args.to_version:
> -                    next_ver = self.args.to_version
> -                else:
> -                    next_ver = row[2]
> -                status = row[11]
> -                revision = row[12]
> -                maintainer = row[14]
> -                no_upgrade_reason = row[15]
> -
> -                if status == 'UPDATE' and not no_upgrade_reason:
> -                    pkgs_list.append((pn, cur_ver, next_ver, maintainer,
> revision))
> -                else:
> -                    if no_upgrade_reason:
> -                        I(" Skip package %s (status = %s, current version
> = %s," \
> -                            " next version = %s, no upgrade reason = %s)"
> %
> -                            (pn, status, cur_ver, next_ver,
> no_upgrade_reason))
> -                    else:
> -                        I(" Skip package %s (status = %s, current version
> = %s," \
> -                            " next version = %s)" %
> -                            (pn, status, cur_ver, next_ver))
> -        return pkgs_list
> -
>      # checks if maintainer is in whitelist and that the recipe itself is
> not
>      # blacklisted: python, gcc, etc. Also, check the history if the recipe
>      # hasn't already been tried
> @@ -688,16 +654,71 @@ class UniverseUpdater(Updater):
>          return True
>
>      def _get_packages_to_upgrade(self, packages=None):
> -        self._check_upstream_versions()
> -        last_checkpkg_file = os.path.realpath(self.base_env['TMPDIR'] +
> "/log/checkpkg.csv")
>
>          pkgs_list = []
> -        for pkg in self._parse_checkpkg_file(last_checkpkg_file):
> -            # Always do the upgrade if recipes are specified
> -            if self.recipes and pkg[0] in self.recipes:
> -                pkgs_list.append(pkg)
> -            elif self._pkg_upgradable(pkg[0], pkg[2], pkg[3]):
> -                pkgs_list.append(pkg)
> +        with bb.tinfoil.Tinfoil() as tinfoil:
> +            tinfoil.prepare(config_only=False)
> +            recipes = self.recipes
> +            if not recipes:
> +                recipes = tinfoil.all_recipe_files(variants=False)
> +
> +            for fn in recipes:
> +                try:
> +                    if fn.startswith("/"):
> +                        data = tinfoil.parse_recipe_file(fn)
> +                    else:
> +                        data = tinfoil.parse_recipe(fn)
> +                except bb.providers.NoProvider:
> +                    I(" No provider for %s" % fn)
> +                    continue
> +
> +                unreliable = data.getVar('UPSTREAM_CHECK_UNRELIABLE')
> +                if unreliable == "1":
> +                    I(" Skip package %s as upstream check unreliable" %
> pn)
> +                    continue
> +
> +                uv = oe.recipeutils.get_recipe_upstream_version(data)
> +
> +                pn = data.getVar('PN')
> +                cur_ver = uv['current_version']
> +
> +
> +                upstream_version_unknown =
> data.getVar('UPSTREAM_VERSION_UNKNOWN')
> +                if not uv['version']:
> +                    status = "UNKNOWN" if upstream_version_unknown else
> "UNKNOWN_BROKEN"
> +                else:
> +                    cmp = vercmp_string(uv['current_version'],
> uv['version'])
> +                    if cmp == -1:
> +                        status = "UPDATE" if not upstream_version_unknown
> else "KNOWN_BROKEN"
> +                    elif cmp == 0:
> +                        status = "MATCH" if not upstream_version_unknown
> else "KNOWN_BROKEN"
> +                    else:
> +                        status = "UNKNOWN" if upstream_version_unknown
> else "UNKNOWN_BROKEN"
> +
> +                if self.args.to_version:
> +                    next_ver = self.args.to_version
> +                else:
> +                    next_ver = uv['version'] if uv['version'] else "N/A"
> +                revision = uv['revision'] if uv['revision'] else "N/A"
> +                maintainer = data.getVar('RECIPE_MAINTAINER')
> +                no_upgrade_reason = data.getVar('RECIPE_NO_UPDATE_REASON')
> +
> +                if status == 'UPDATE' and not no_upgrade_reason:
> +                    # Always do the upgrade if recipes are specified
> +                    if self.recipes and pn in self.recipes:
> +                        pkgs_list.append((pn, cur_ver, next_ver,
> maintainer, revision))
> +
> +                    elif self._pkg_upgradable(pn, next_ver, maintainer):
> +                        pkgs_list.append((pn, cur_ver, next_ver,
> maintainer, revision))
> +                else:
> +                    if no_upgrade_reason:
> +                        I(" Skip package %s (status = %s, current version
> = %s," \
> +                            " next version = %s, no upgrade reason = %s)"
> %
> +                            (pn, status, cur_ver, next_ver,
> no_upgrade_reason))
> +                    else:
> +                        I(" Skip package %s (status = %s, current version
> = %s," \
> +                            " next version = %s)" %
> +                            (pn, status, cur_ver, next_ver))
>
>          return pkgs_list
>
> @@ -716,11 +737,6 @@ if __name__ == "__main__":
>      global settings
>      global maintainer_override
>
> -    if not os.getenv('BUILDDIR', False):
> -        E(" You must source oe-init-build-env before running this
> script!\n")
> -        E(" It is recommended to create a fresh build directory with
> it:\n")
> -        E(" $ . oe-init-build-env build-auh\n")
> -        exit(1)
>
>      devnull = open(os.devnull, 'wb')
>      if subprocess.call(["git", "config", "user.name"],
> stdout=devnull,stderr=devnull) or \
> --
> 2.17.1
>
> --
> _______________________________________________
> yocto mailing list
> yocto@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/yocto
>
Alexander Kanavin Dec. 14, 2018, 4:56 p.m.
On Fri, 14 Dec 2018 at 17:07, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> +        with bb.tinfoil.Tinfoil() as tinfoil:
> +            tinfoil.prepare(config_only=False)
> +            recipes = self.recipes
> +            if not recipes:
> +                recipes = tinfoil.all_recipe_files(variants=False)
> +
> +            for fn in recipes:
> +                try:
> +                    if fn.startswith("/"):
> +                        data = tinfoil.parse_recipe_file(fn)
> +                    else:
> +                        data = tinfoil.parse_recipe(fn)
> +                except bb.providers.NoProvider:
> +                    I(" No provider for %s" % fn)
> +                    continue
> +
> +                unreliable = data.getVar('UPSTREAM_CHECK_UNRELIABLE')
> +                if unreliable == "1":
> +                    I(" Skip package %s as upstream check unreliable" % pn)
> +                    continue
> +
> +                uv = oe.recipeutils.get_recipe_upstream_version(data)
> +
> +                pn = data.getVar('PN')
> +                cur_ver = uv['current_version']
> +
> +
> +                upstream_version_unknown = data.getVar('UPSTREAM_VERSION_UNKNOWN')
> +                if not uv['version']:
> +                    status = "UNKNOWN" if upstream_version_unknown else "UNKNOWN_BROKEN"
> +                else:
> +                    cmp = vercmp_string(uv['current_version'], uv['version'])
> +                    if cmp == -1:
> +                        status = "UPDATE" if not upstream_version_unknown else "KNOWN_BROKEN"
> +                    elif cmp == 0:
> +                        status = "MATCH" if not upstream_version_unknown else "KNOWN_BROKEN"
> +                    else:
> +                        status = "UNKNOWN" if upstream_version_unknown else "UNKNOWN_BROKEN"
> +

This part is generally useful, and should be placed somewhere in
lib/oe/recipeutils.py perhaps? I can imagine having a script (AUH
itself perhaps) that can use the data to print statistics or detect
broken upstream checks. Agree about issues with do_checkpkg  and
general awfulness of distrodata class :)

Alex