[PATCH 00/10][AUH] Improvements and small fixes.

[PATCH 00/10][AUH] Improvements and small fixes.

[Aníbal Limón]

Aníbal Limón (10):
  upgradehelper.py: Add step for build gcc-runtime at init.
  upgradehelper.py: Fix bug when buildhistory isn't enabled
  recipe.py: Improvements and refactor of fetch method.
  buildhistory.py: Don't modify BB_ENV_EXTRAWHITE before init.
  upgradehelper.py: Add sanity test for ensure that git is configured.
  buildhistory.py: Add missing warning import.     git.py: Fix
    last_commit method.
  upgradehelper.py: Improve work directory structure
  upgradehelper.py: Add support for do recipe upgrades based on builddep
  upgradehelper.py: Add support for preserve statistics into work    
    directory.
  requirements.txt: Add file for now only with GitPython

 buildhistory.py  |   8 +--
 git.py           |   2 +-
 recipe.py        |  58 +++++++++++--------
 requirements.txt |   1 +
 upgradehelper.py | 170 +++++++++++++++++++++++++++++++++++++++++--------------
 5 files changed, 168 insertions(+), 71 deletions(-)
 create mode 100644 requirements.txt

-- 
2.1.4

[PATCH 01/10][AUH] upgradehelper.py: Add step for build gcc-runtime at init.

[Aníbal Limón]

When try to upgrade a recipe the first recipe in the list
takes too much time because gcc-runtime build is needed.

So add previous step to build gcc-runtime for every machine.

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 upgradehelper.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/upgradehelper.py b/upgradehelper.py
index b5a8abd..bb19b65 100755
--- a/upgradehelper.py
+++ b/upgradehelper.py
@@ -467,6 +467,11 @@ class Updater(object):
             W("No recipes attempted, not sending status mail!")
 
     def run(self, package_list=None):
+        I(" Building gcc runtimes ...")
+        for machine in self.machines:
+            I("  building gcc runtime for %s" % machine)
+            self.bb.complete("gcc-runtime", machine)
+
         pkgs_to_upgrade = self._get_packages_to_upgrade(package_list)
 
         total_pkgs = len(pkgs_to_upgrade)
-- 
2.1.4

[PATCH 02/10][AUH] upgradehelper.py: Fix bug when buildhistory isn't enabled

[Aníbal Limón]

Don't use buildhistory object if buildhistory_enabled is False
because cause undefined variable error.

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 upgradehelper.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/upgradehelper.py b/upgradehelper.py
index bb19b65..9b321f6 100755
--- a/upgradehelper.py
+++ b/upgradehelper.py
@@ -310,7 +310,7 @@ class Updater(object):
         for machine in self.machines:
             I(" %s: compiling for %s ..." % (self.pn, machine))
             self.recipe.compile(machine)
-            if self.buildhistory is not None:
+            if self.buildhistory_enabled == True:
                 self.buildhistory.add()
 
     def _buildhistory_diff(self):
-- 
2.1.4

[PATCH 03/10][AUH] recipe.py: Improvements and refactor of fetch method.

[Aníbal Limón]

Make more clear fetch method changing recursive manner to
loop and split logic into try fetch and suffix change.

Now when fails only retry changing recipe suffix when recipe
isn't git based because does not make sense suffixes in SCM's.

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 recipe.py | 58 +++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/recipe.py b/recipe.py
index 6fd8f24..0e8799f 100644
--- a/recipe.py
+++ b/recipe.py
@@ -55,7 +55,6 @@ class Recipe(object):
             "tar.gz", "tgz", "zip", "tar.bz2", "tar.xz", "tar.lz4", "bz2",
             "lz4", "orig.tar.gz", "src.tar.gz", "src.rpm", "src.tgz",
             "svnr\d+.tar.bz2", "stable.tar.gz", "src.rpm"]
-        self.suffix_index = 0
         self.old_env = None
 
         self.commit_msg = self.env['PN'] + ": upgrade to " + self.new_ver + "\n\n"
@@ -540,33 +539,42 @@ class Recipe(object):
         self.bb.unpack(self.env['PN'])
 
     def fetch(self):
-        try:
-            self.bb.fetch(self.env['PN'])
-        except Error as e:
-            machine, failed_recipes = self._get_failed_recipes(e.stdout)
-            if not self.env['PN'] in failed_recipes:
-                raise Error("unknown error occured during fetch")
-
-            fetch_log = failed_recipes[self.env['PN']][1]
-            if self.suffix_index < len(self.suffixes) and self._is_uri_failure(fetch_log):
-                I(" Trying new SRC_URI suffix: %s ..." % self.suffixes[self.suffix_index])
-                self._change_source_suffix(self.suffixes[self.suffix_index])
-                self.suffix_index += 1
-                self.fetch()
-            
-            if not self._is_uri_failure(fetch_log):
-                if not self.checksums_changed:
+        from gitrecipe import GitRecipe
+
+        def _try_fetch():
+            try:
+                self.bb.fetch(self.env['PN'])
+                return
+            except Error as e:
+                machine, failed_recipes = self._get_failed_recipes(e.stdout)
+                if not self.env['PN'] in failed_recipes:
+                    raise Error("Unknown error occured during fetch",
+                            stdout = e.stdout, stderr = e.stderr)
+
+                fetch_log = failed_recipes[self.env['PN']][1]
+
+                if not self._is_uri_failure(fetch_log) and not \
+                        self.checksums_changed:
                     self._change_recipe_checksums(fetch_log)
-                    return
-                else:
-                    raise FetchError()
+                    self.checksums_changed = True
+                    return True
 
-                if self.recipes_renamed and not self.checksums_changed:
-                    raise Error("fetch succeeded without changing checksums")
+                return False
+
+        succeed = _try_fetch()
+        if succeed is False and not isinstance(self, GitRecipe):
+            for sfx in self.suffixes:
+                I(" Trying new SRC_URI suffix: %s ..." % sfx)
+                self._change_source_suffix(sfx)
+
+                succeed = _try_fetch()
+                if succeed is True:
+                    break
 
-            elif self.suffix_index == len(self.suffixes):
-                # Every suffix tried without success
-                raise FetchError()                
+        if succeed is False:
+            raise Error("Can't built a valid SRC_URI")
+        elif self.recipes_renamed and not self.checksums_changed:
+            raise Error("Fetch succeeded without changing checksums")
 
     def cleanall(self):
         self.bb.cleanall(self.env['PN'])
-- 
2.1.4

[PATCH 04/10][AUH] buildhistory.py: Don't modify BB_ENV_EXTRAWHITE before init.

[Aníbal Limón]

Move code for add BUILDHISTORY_DIR to __init__ method because
it fails when import in early step.

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 buildhistory.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/buildhistory.py b/buildhistory.py
index 1732f23..6eca6e1 100644
--- a/buildhistory.py
+++ b/buildhistory.py
@@ -30,11 +30,10 @@ from errors import *
 from bitbake import *
 from git import Git
 
-os.environ['BB_ENV_EXTRAWHITE'] = os.environ['BB_ENV_EXTRAWHITE'] + \
-                                    " BUILDHISTORY_DIR"
-
 class BuildHistory(object):
     def __init__(self, bb, pn, workdir):
+        os.environ['BB_ENV_EXTRAWHITE'] = os.environ['BB_ENV_EXTRAWHITE'] + \
+                                    " BUILDHISTORY_DIR"
         self.bb = bb
         self.pn = pn
         self.workdir = workdir
-- 
2.1.4

[PATCH 05/10][AUH] upgradehelper.py: Add sanity test for ensure that git is configured.

[Aníbal Limón]

[YOCTO #8390]

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 upgradehelper.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/upgradehelper.py b/upgradehelper.py
index 9b321f6..ae40fb1 100755
--- a/upgradehelper.py
+++ b/upgradehelper.py
@@ -28,6 +28,7 @@
 
 import argparse
 import os
+from subprocess import call
 import logging as log
 from logging import debug as D
 from logging import info as I
@@ -734,17 +735,22 @@ 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")
+        exit(1)
+
+    devnull = open(os.devnull, 'wb');
+    if call(["git", "config", "user.name"], stdout=devnull, stderr=devnull) or \
+        call(["git", "config", "user.email"], stdout=devnull, stderr=devnull):
+        E(" Git isn't configure please configure user name and email\n")
+        exit(1)
+
     signal.signal(signal.SIGINT, close_child_processes)
 
     debug_levels = [log.CRITICAL, log.ERROR, log.WARNING, log.INFO, log.DEBUG]
     args = parse_cmdline()
     log.basicConfig(format='%(levelname)s:%(message)s',
                     level=debug_levels[args.debug_level - 1])
-
-    if not os.getenv('BUILDDIR', False):
-        E(" You must source oe-init-build-env before running this script!\n")
-        exit(1)
-
     settings, maintainer_override = parse_config_file(args.config_file)
 
     if args.recipe == "all":
-- 
2.1.4

[PATCH 06/10][AUH] buildhistory.py: Add missing warning import. git.py: Fix last_commit method.

[Aníbal Limón]

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 buildhistory.py | 3 ++-
 git.py          | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/buildhistory.py b/buildhistory.py
index 6eca6e1..2e442d2 100644
--- a/buildhistory.py
+++ b/buildhistory.py
@@ -20,8 +20,9 @@
 
 import os
 import logging as log
-from logging import info as I
 from logging import debug as D
+from logging import info as I
+from logging import warning as W
 from logging import error as E
 from logging import critical as C
 import sys
diff --git a/git.py b/git.py
index 3eebac3..9661364 100644
--- a/git.py
+++ b/git.py
@@ -88,7 +88,7 @@ class Git(object):
         return self._cmd("clean -fd")
 
     def last_commit(self, branch_name):
-        return self._cmd("log --pretty=format:\"%H\" -1" + branch_name)
+        return self._cmd("log --pretty=format:\"%H\" -1 " + branch_name)
 
     def ls_remote(self, repo_url=None, options=None, refs=None):
         cmd = "ls-remote"
-- 
2.1.4

[PATCH 07/10][AUH] upgradehelper.py: Improve work directory structure

[Aníbal Limón]

Add work directory with datetime suffix to preserve recipe upgrades.

Create tree folders all, succeed and failed,
	all	- Contains all the recipes work
	succeed	- Contains a symlinks to the recipes that successful
		  upgrade
	failed	- Contains a symlinks to the recipes that fail
		  upgrade

This helps to navigate over patches that have a successful upgrade.

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 upgradehelper.py | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/upgradehelper.py b/upgradehelper.py
index ae40fb1..aeb9cf1 100755
--- a/upgradehelper.py
+++ b/upgradehelper.py
@@ -132,9 +132,16 @@ class Updater(object):
         if not os.path.exists(self.uh_dir):
             os.mkdir(self.uh_dir)
 
-        self.uh_work_dir = os.path.join(self.uh_dir, "work")
-        if not os.path.exists(self.uh_work_dir):
-            os.mkdir(self.uh_work_dir)
+        self.uh_work_dir = os.path.join(self.uh_dir, "work-%s" % \
+                datetime.now().strftime("%Y%m%d%H%M"))
+        os.mkdir(self.uh_work_dir)
+
+        self.uh_recipes_all_dir = os.path.join(self.uh_work_dir, "all")
+        os.mkdir(self.uh_recipes_all_dir)
+        self.uh_recipes_succeed_dir = os.path.join(self.uh_work_dir, "succeed")
+        os.mkdir(self.uh_recipes_succeed_dir)
+        self.uh_recipes_failed_dir = os.path.join(self.uh_work_dir, "failed")
+        os.mkdir(self.uh_recipes_failed_dir)
 
         self.bb = Bitbake(get_build_dir())
         self.git = None
@@ -223,10 +230,7 @@ class Updater(object):
         self.env = self._get_env(self.pn)
 
     def _create_workdir(self):
-        self.workdir = os.path.join(self.uh_work_dir, self.pn)
-
-        if os.path.exists(self.workdir):
-            shutil.rmtree(self.workdir)
+        self.workdir = os.path.join(self.uh_recipes_all_dir, self.pn)
         os.mkdir(self.workdir)
 
     def _detect_repo(self):
@@ -490,6 +494,9 @@ class Updater(object):
                         I(" %s: %s" % (self.pn, msg))
                     step()
 
+                os.symlink(self.workdir, os.path.join( \
+                    self.uh_recipes_succeed_dir, self.pn))
+
                 I(" %s: Upgrade SUCCESSFUL! Please test!" % self.pn)
             except Exception as e:
                 if isinstance(e, UpgradeNotNeededError):
@@ -511,6 +518,9 @@ class Updater(object):
 
                 error = e
 
+                os.symlink(self.workdir, os.path.join( \
+                    self.uh_recipes_failed_dir, self.pn))
+
             self._commit_changes()
 
             self.pkg_upgrade_handler(error)
-- 
2.1.4

[PATCH 08/10][AUH] upgradehelper.py: Add support for do recipe upgrades based on builddep

[Aníbal Limón]

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 upgradehelper.py | 114 +++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 86 insertions(+), 28 deletions(-)

diff --git a/upgradehelper.py b/upgradehelper.py
index aeb9cf1..a08833e 100755
--- a/upgradehelper.py
+++ b/upgradehelper.py
@@ -91,7 +91,6 @@ def parse_cmdline():
 def get_build_dir():
     return os.getenv('BUILDDIR')
 
-
 def parse_config_file(config_file):
     settings = dict()
     maintainer_override = dict()
@@ -435,26 +434,6 @@ class Updater(object):
             I(" %s: %s" % (self.pn, e.stdout))
             raise e
 
-    def _order_list(self, package_list):
-        try:
-            self.bb.dependency_graph(' '.join(p[0] for p in package_list))
-        except Error as e:
-            multiple_providers = False
-            for l in e.stdout.split('\n'):
-                if l.find("ERROR: Multiple .bb files are due to be built which each provide") == 0:
-                    multiple_providers = True
-
-            if not multiple_providers:
-                raise e
-
-        dep_file = os.path.join(get_build_dir(), "pn-buildlist")
-        ordered_list = []
-        with open(dep_file) as deps:
-            for d in deps:
-                ordered_list.extend(p for p in package_list if p[0] == d.strip())
-
-        return ordered_list
-
     def send_status_mail(self):
         if "status_recipients" not in settings:
             E("Could not send status email, no recipients set!")
@@ -471,16 +450,100 @@ class Updater(object):
         else:
             W("No recipes attempted, not sending status mail!")
 
+
+    def _order_pkgs_to_upgrade(self, pkgs_to_upgrade):
+        def _get_pn_dep_dic(pn_list, dependency_file): 
+            import re
+
+            pn_dep_dic = {}
+
+            with open(dependency_file) as dep:
+                data = dep.read()
+                dep.close()
+
+                for line in data.split('\n'):
+                    m = re.search('^"(.*)" -> "(.*)"$', line)
+                    if not m:
+                        continue
+
+                    pn = m.group(1)
+                    pn_dep = m.group(2)
+                    if pn == pn_dep:
+                        continue
+
+                    if pn in pn_list:
+                        if pn_dep in pn_list:
+                            if pn in pn_dep_dic.keys():
+                                pn_dep_dic[pn].append(pn_dep)
+                            else:
+                                pn_dep_dic[pn] = [pn_dep]
+                        elif not pn in pn_dep_dic.keys():
+                            pn_dep_dic[pn] = []
+
+            return pn_dep_dic
+
+        def _dep_resolve(graph, node, resolved, seen):
+            seen.append(node)
+
+            for edge in graph[node]:
+                if edge not in resolved:
+                    if edge in seen:
+                        raise RuntimeError("Packages %s and %s have " \
+                                "a circular dependency." \
+                                % (node, edge))
+                    _dep_resolve(graph, edge, resolved, seen)
+
+            resolved.append(node)
+
+
+        pn_list = []
+        for pn, new_ver, maintainer in pkgs_to_upgrade:
+            pn_list.append(pn)
+
+        try:
+           self.bb.dependency_graph(' '.join(pn_list))
+        except Error as e:
+            multiple_providers = False
+            for l in e.stdout.split('\n'):
+                if l.find("ERROR: Multiple .bb files are due to be built which each provide") == 0:
+                    multiple_providers = True
+            if not multiple_providers:
+                raise e
+
+        dependency_file = os.path.join(get_build_dir(), "pn-depends.dot")
+
+        pkgs_to_upgrade_ordered = []
+        pn_list_ordered = []
+
+        pn_dep_dic = _get_pn_dep_dic(pn_list, dependency_file)
+        if pn_dep_dic:
+            root = "__root_node__"
+            pn_dep_dic[root] = pn_dep_dic.keys()
+            _dep_resolve(pn_dep_dic, root, pn_list_ordered, [])
+            pn_list_ordered.remove(root)
+
+        for pn_ordered in pn_list_ordered:
+            for pn, new_ver, maintainer in pkgs_to_upgrade:
+                if pn == pn_ordered: 
+                    pkgs_to_upgrade_ordered.append([pn, new_ver, maintainer])
+
+        return pkgs_to_upgrade_ordered
+
     def run(self, package_list=None):
         I(" Building gcc runtimes ...")
         for machine in self.machines:
             I("  building gcc runtime for %s" % machine)
             self.bb.complete("gcc-runtime", machine)
 
-        pkgs_to_upgrade = self._get_packages_to_upgrade(package_list)
-
+        pkgs_to_upgrade = self._order_pkgs_to_upgrade(
+                self._get_packages_to_upgrade(package_list))
         total_pkgs = len(pkgs_to_upgrade)
 
+        I(" ########### The list of recipes to be upgraded #############")
+        for p, v, m in pkgs_to_upgrade:
+            I(" %s, %s, %s" % (p, v, m))
+        I(" ############################################################")
+
         attempted_pkgs = 0
         for self.pn, self.new_ver, self.maintainer in pkgs_to_upgrade:
             error = None
@@ -708,11 +771,6 @@ class UniverseUpdater(Updater):
             last_check.write(current_date + "," + cur_master_commit + "," +
                              last_checkpkg_file)
 
-        I(" ########### The list of recipes to be upgraded ############")
-        for p, v, m in pkgs_list:
-            I(" %s, %s, %s" % (p, v, m))
-        I(" ############################################################")
-
         return pkgs_list
 
     def _update_history(self, pn, new_ver, maintainer, upgrade_status):
-- 
2.1.4

[PATCH 09/10][AUH] upgradehelper.py: Add support for preserve statistics into work directory.

[Aníbal Limón]

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 upgradehelper.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/upgradehelper.py b/upgradehelper.py
index a08833e..4f33d2f 100755
--- a/upgradehelper.py
+++ b/upgradehelper.py
@@ -591,7 +591,16 @@ class Updater(object):
             self.statistics.update(self.pn, self.new_ver, self.maintainer, error)
 
         if (attempted_pkgs > 1):
-            I("%s" % self.statistics.pkg_stats())
+            statistics_summary = self.statistics.pkg_stats() + \
+                    self.statistics.maintainer_stats()
+
+            statistics_file = os.path.join(self.uh_work_dir,
+                    "statistics_summary")
+            with open(statistics_file, "w+") as f:
+                f.write(statistics_summary)
+
+            I("%s" % statistics_summary)
+
             if self.send_email:
                 self.send_status_mail()
 
-- 
2.1.4

[PATCH 10/10][AUH] requirements.txt: Add file for now only with GitPython

[Aníbal Limón]

Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
---
 requirements.txt | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 requirements.txt

diff --git a/requirements.txt b/requirements.txt
new file mode 100644
index 0000000..64b1ada
--- /dev/null
+++ b/requirements.txt
@@ -0,0 +1 @@
+GitPython
-- 
2.1.4

Re: [PATCH 03/10][AUH] recipe.py: Improvements and refactor of fetch method.

[Paul Eggleton]

Hi Aníbal,

On Monday 09 November 2015 16:01:34 Aníbal Limón wrote:
> Make more clear fetch method changing recursive manner to
> loop and split logic into try fetch and suffix change.
> 
> Now when fails only retry changing recipe suffix when recipe
> isn't git based because does not make sense suffixes in SCM's.
> 
> Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
> ---
>  recipe.py | 58 +++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 33 insertions(+), 25 deletions(-)
> 
> diff --git a/recipe.py b/recipe.py
> index 6fd8f24..0e8799f 100644
> --- a/recipe.py
> +++ b/recipe.py
> @@ -55,7 +55,6 @@ class Recipe(object):
>              "tar.gz", "tgz", "zip", "tar.bz2", "tar.xz", "tar.lz4", "bz2",
>              "lz4", "orig.tar.gz", "src.tar.gz", "src.rpm", "src.tgz",
>              "svnr\d+.tar.bz2", "stable.tar.gz", "src.rpm"]
> -        self.suffix_index = 0
>          self.old_env = None
> 
>          self.commit_msg = self.env['PN'] + ": upgrade to " + self.new_ver +
> "\n\n" @@ -540,33 +539,42 @@ class Recipe(object):
>          self.bb.unpack(self.env['PN'])
> 
>      def fetch(self):
> -        try:
> -            self.bb.fetch(self.env['PN'])
> -        except Error as e:
> -            machine, failed_recipes = self._get_failed_recipes(e.stdout)
> -            if not self.env['PN'] in failed_recipes:
> -                raise Error("unknown error occured during fetch")
> -
> -            fetch_log = failed_recipes[self.env['PN']][1]
> -            if self.suffix_index < len(self.suffixes) and
> self._is_uri_failure(fetch_log): -                I(" Trying new SRC_URI
> suffix: %s ..." % self.suffixes[self.suffix_index]) -               
> self._change_source_suffix(self.suffixes[self.suffix_index]) -             
>   self.suffix_index += 1
> -                self.fetch()
> -
> -            if not self._is_uri_failure(fetch_log):
> -                if not self.checksums_changed:
> +        from gitrecipe import GitRecipe
> +
> +        def _try_fetch():
> +            try:
> +                self.bb.fetch(self.env['PN'])
> +                return
> +            except Error as e:
> +                machine, failed_recipes =
> self._get_failed_recipes(e.stdout) +                if not self.env['PN']
> in failed_recipes:
> +                    raise Error("Unknown error occured during fetch",
> +                            stdout = e.stdout, stderr = e.stderr)
> +
> +                fetch_log = failed_recipes[self.env['PN']][1]
> +
> +                if not self._is_uri_failure(fetch_log) and not \
> +                        self.checksums_changed:
>                      self._change_recipe_checksums(fetch_log)
> -                    return
> -                else:
> -                    raise FetchError()
> +                    self.checksums_changed = True
> +                    return True
> 
> -                if self.recipes_renamed and not self.checksums_changed:
> -                    raise Error("fetch succeeded without changing
> checksums") +                return False
> +
> +        succeed = _try_fetch()
> +        if succeed is False and not isinstance(self, GitRecipe):
> +            for sfx in self.suffixes:
> +                I(" Trying new SRC_URI suffix: %s ..." % sfx)
> +                self._change_source_suffix(sfx)
> +
> +                succeed = _try_fetch()
> +                if succeed is True:
> +                    break
> 
> -            elif self.suffix_index == len(self.suffixes):
> -                # Every suffix tried without success
> -                raise FetchError()
> +        if succeed is False:

"x is True" or "x is False" is very odd syntax - the "is" operator is for when 
you want to compare the instance of two things, and whilst that may work that 
isn't what you want here. Simply "x" or "not x" would be the more standard way 
to do this.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 04/10][AUH] buildhistory.py: Don't modify BB_ENV_EXTRAWHITE before init.

[Paul Eggleton]

On Monday 09 November 2015 16:01:35 Aníbal Limón wrote:
> Move code for add BUILDHISTORY_DIR to __init__ method because
> it fails when import in early step.
> 
> Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
> ---
>  buildhistory.py | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/buildhistory.py b/buildhistory.py
> index 1732f23..6eca6e1 100644
> --- a/buildhistory.py
> +++ b/buildhistory.py
> @@ -30,11 +30,10 @@ from errors import *
>  from bitbake import *
>  from git import Git
> 
> -os.environ['BB_ENV_EXTRAWHITE'] = os.environ['BB_ENV_EXTRAWHITE'] + \
> -                                    " BUILDHISTORY_DIR"
> -
>  class BuildHistory(object):
>      def __init__(self, bb, pn, workdir):
> +        os.environ['BB_ENV_EXTRAWHITE'] = os.environ['BB_ENV_EXTRAWHITE'] +
> \ +                                    " BUILDHISTORY_DIR"
>          self.bb = bb
>          self.pn = pn
>          self.workdir = workdir

I'm not particularly happy with setting this through the environment. Leaving 
that aside though, rather than putting it at the top of the constructor can 
you please put this just before the line where we actually set 
BUILDHISTORY_DIR in the environment?

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 05/10][AUH] upgradehelper.py: Add sanity test for ensure that git is configured.

[Paul Eggleton]

On Monday 09 November 2015 16:01:36 Aníbal Limón wrote:
> [YOCTO #8390]
> 
> Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
> ---
>  upgradehelper.py | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/upgradehelper.py b/upgradehelper.py
> index 9b321f6..ae40fb1 100755
> --- a/upgradehelper.py
> +++ b/upgradehelper.py
> @@ -28,6 +28,7 @@
> 
>  import argparse
>  import os
> +from subprocess import call
>  import logging as log
>  from logging import debug as D
>  from logging import info as I
> @@ -734,17 +735,22 @@ 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") 
> +        exit(1)
> +
> +    devnull = open(os.devnull, 'wb');
> +    if call(["git", "config", "user.name"], stdout=devnull, stderr=devnull)
> or \ 
> +        call(["git", "config", "user.email"], stdout=devnull,
> stderr=devnull): 
> +        E(" Git isn't configure please configure user name and email\n") 
> +        exit(1)

"isn't configure" -> "isn't configured"

Also, we're doing an awful lot of "from xxxx import yyyy" in this code which 
for the most part seems like poor style. For this addition can you simply 
import subprocess and then use subprocess.call() ?

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 08/10][AUH] upgradehelper.py: Add support for do recipe upgrades based on builddep

[Paul Eggleton]

On Monday 09 November 2015 16:01:39 Aníbal Limón wrote:
> Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
> ---

Please provide a proper commit message for this one.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 03/10][AUH] recipe.py: Improvements and refactor of fetch method.

[Aníbal Limón]

Hi Paul,


On 11/10/2015 02:46 AM, Paul Eggleton wrote:
> Hi Aníbal,
> 
> On Monday 09 November 2015 16:01:34 Aníbal Limón wrote:
>> Make more clear fetch method changing recursive manner to
>> loop and split logic into try fetch and suffix change.
>>
>> Now when fails only retry changing recipe suffix when recipe
>> isn't git based because does not make sense suffixes in SCM's.
>>
>> Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
>> ---
>>  recipe.py | 58 +++++++++++++++++++++++++++++++++-------------------------
>>  1 file changed, 33 insertions(+), 25 deletions(-)
>>
>> diff --git a/recipe.py b/recipe.py
>> index 6fd8f24..0e8799f 100644
>> --- a/recipe.py
>> +++ b/recipe.py
>> @@ -55,7 +55,6 @@ class Recipe(object):
>>              "tar.gz", "tgz", "zip", "tar.bz2", "tar.xz", "tar.lz4", "bz2",
>>              "lz4", "orig.tar.gz", "src.tar.gz", "src.rpm", "src.tgz",
>>              "svnr\d+.tar.bz2", "stable.tar.gz", "src.rpm"]
>> -        self.suffix_index = 0
>>          self.old_env = None
>>
>>          self.commit_msg = self.env['PN'] + ": upgrade to " + self.new_ver +
>> "\n\n" @@ -540,33 +539,42 @@ class Recipe(object):
>>          self.bb.unpack(self.env['PN'])
>>
>>      def fetch(self):
>> -        try:
>> -            self.bb.fetch(self.env['PN'])
>> -        except Error as e:
>> -            machine, failed_recipes = self._get_failed_recipes(e.stdout)
>> -            if not self.env['PN'] in failed_recipes:
>> -                raise Error("unknown error occured during fetch")
>> -
>> -            fetch_log = failed_recipes[self.env['PN']][1]
>> -            if self.suffix_index < len(self.suffixes) and
>> self._is_uri_failure(fetch_log): -                I(" Trying new SRC_URI
>> suffix: %s ..." % self.suffixes[self.suffix_index]) -               
>> self._change_source_suffix(self.suffixes[self.suffix_index]) -             
>>   self.suffix_index += 1
>> -                self.fetch()
>> -
>> -            if not self._is_uri_failure(fetch_log):
>> -                if not self.checksums_changed:
>> +        from gitrecipe import GitRecipe
>> +
>> +        def _try_fetch():
>> +            try:
>> +                self.bb.fetch(self.env['PN'])
>> +                return
>> +            except Error as e:
>> +                machine, failed_recipes =
>> self._get_failed_recipes(e.stdout) +                if not self.env['PN']
>> in failed_recipes:
>> +                    raise Error("Unknown error occured during fetch",
>> +                            stdout = e.stdout, stderr = e.stderr)
>> +
>> +                fetch_log = failed_recipes[self.env['PN']][1]
>> +
>> +                if not self._is_uri_failure(fetch_log) and not \
>> +                        self.checksums_changed:
>>                      self._change_recipe_checksums(fetch_log)
>> -                    return
>> -                else:
>> -                    raise FetchError()
>> +                    self.checksums_changed = True
>> +                    return True
>>
>> -                if self.recipes_renamed and not self.checksums_changed:
>> -                    raise Error("fetch succeeded without changing
>> checksums") +                return False
>> +
>> +        succeed = _try_fetch()
>> +        if succeed is False and not isinstance(self, GitRecipe):
>> +            for sfx in self.suffixes:
>> +                I(" Trying new SRC_URI suffix: %s ..." % sfx)
>> +                self._change_source_suffix(sfx)
>> +
>> +                succeed = _try_fetch()
>> +                if succeed is True:
>> +                    break
>>
>> -            elif self.suffix_index == len(self.suffixes):
>> -                # Every suffix tried without success
>> -                raise FetchError()
>> +        if succeed is False:
> 
> "x is True" or "x is False" is very odd syntax - the "is" operator is for when 
> you want to compare the instance of two things, and whilst that may work that 
> isn't what you want here. Simply "x" or "not x" would be the more standard way 
> to do this.

I didn't know about that convention, but if you said it i'll change the
code to be Pythonic.


> 
> Cheers,
> Paul
> 

Re: [PATCH 04/10][AUH] buildhistory.py: Don't modify BB_ENV_EXTRAWHITE before init.

[Aníbal Limón]

On 11/10/2015 02:54 AM, Paul Eggleton wrote:
> On Monday 09 November 2015 16:01:35 Aníbal Limón wrote:
>> Move code for add BUILDHISTORY_DIR to __init__ method because
>> it fails when import in early step.
>>
>> Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
>> ---
>>  buildhistory.py | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/buildhistory.py b/buildhistory.py
>> index 1732f23..6eca6e1 100644
>> --- a/buildhistory.py
>> +++ b/buildhistory.py
>> @@ -30,11 +30,10 @@ from errors import *
>>  from bitbake import *
>>  from git import Git
>>
>> -os.environ['BB_ENV_EXTRAWHITE'] = os.environ['BB_ENV_EXTRAWHITE'] + \
>> -                                    " BUILDHISTORY_DIR"
>> -
>>  class BuildHistory(object):
>>      def __init__(self, bb, pn, workdir):
>> +        os.environ['BB_ENV_EXTRAWHITE'] = os.environ['BB_ENV_EXTRAWHITE'] +
>> \ +                                    " BUILDHISTORY_DIR"
>>          self.bb = bb
>>          self.pn = pn
>>          self.workdir = workdir
> 
> I'm not particularly happy with setting this through the environment. Leaving 
> that aside though, rather than putting it at the top of the constructor can 
> you please put this just before the line where we actually set 
> BUILDHISTORY_DIR in the environment?

I know that you aren't happy with this setting in the environment but
now is the only way to have a organized output (per recipe) of AUH as
you can see in the Status email i sent.

I'll change the code to be together this set of BUILDHISTORY_DIR.


> 
> Cheers,
> Paul
> 

Re: [PATCH 05/10][AUH] upgradehelper.py: Add sanity test for ensure that git is configured.

[Aníbal Limón]

On 11/10/2015 02:59 AM, Paul Eggleton wrote:
> On Monday 09 November 2015 16:01:36 Aníbal Limón wrote:
>> [YOCTO #8390]
>> 
>> Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com> --- 
>> upgradehelper.py | 16 +++++++++++----- 1 file changed, 11
>> insertions(+), 5 deletions(-)
>> 
>> diff --git a/upgradehelper.py b/upgradehelper.py index
>> 9b321f6..ae40fb1 100755 --- a/upgradehelper.py +++
>> b/upgradehelper.py @@ -28,6 +28,7 @@
>> 
>> import argparse import os +from subprocess import call import
>> logging as log from logging import debug as D from logging import
>> info as I @@ -734,17 +735,22 @@ 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") +
>> exit(1) + +    devnull = open(os.devnull, 'wb'); +    if
>> call(["git", "config", "user.name"], stdout=devnull,
>> stderr=devnull) or \ +        call(["git", "config",
>> "user.email"], stdout=devnull, stderr=devnull): +        E(" Git
>> isn't configure please configure user name and email\n") +
>> exit(1)
> 
> "isn't configure" -> "isn't configured"
> 
> Also, we're doing an awful lot of "from xxxx import yyyy" in this
> code which for the most part seems like poor style. For this
> addition can you simply import subprocess and then use
> subprocess.call() ?
> 

Ok.

> Cheers, Paul
> 

Re: [PATCH 08/10][AUH] upgradehelper.py: Add support for do recipe upgrades based on builddep

[Aníbal Limón]

Sure. I forget it :).

On 11/10/2015 03:09 AM, Paul Eggleton wrote:
> On Monday 09 November 2015 16:01:39 Aníbal Limón wrote:
>> Signed-off-by: Aníbal Limón <anibal.limon@linux.intel.com>
>> ---
> 
> Please provide a proper commit message for this one.
> 
> Cheers,
> Paul
>