[PATCH 00/10] #10619: wic refactoring (done)

[PATCH 00/10] #10619: wic refactoring (done)

[Ed Bartosh]

Hi,

This patchset contains refactoring of wic plugin system. It hopefully
makes the code more understandable and easier to read and maintain.
It reduced plugin system API to only one call: get_plugins, speed up
the system by using lazy evaluation of plugin info, shortened the code
and moved it to one module.

This is a last patchset of wic refactoring series. Although there are still
some places in wic code that I'd like to optimize, I decided to stop here.
The code looks more maintainable for me now and even though I want to make it
much better I have other tasks to do and important bugs to fix.

Just in case anybody is interested in continuing to refactor wic code I can point
out to the areas that still require refactoring from my point of view:
- make use of ArgumentParser: bug #9636
- refactor engine.py and help.py
- replace plugin code that configures bootloaders with using configuration
  files (--configfile .wks option)
- refactor source plugins
- get rid of custom runner module in favor of using subprocess
- move the rest of code from utils/ subdirectory to lib/wic/utils.py

The following changes since commit e4e061bbdadce3f4fe420768bb132f6aac5fca56:

  wic: move WicError to lib/wic/__init__.py (2017-02-16 13:24:03 +0200)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib ed/wic/wip
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=ed/wic/wip

Ed Bartosh (10):
  wic: use PluginMgr directly
  wic: remove PluginMgr.get_source_plugins
  wic: reimplement PluginMgr.get_plugin_methods
  wic: reimplement PluginMgr.get_plugins
  wic: pluginbase: use python 3 metaclass syntax
  wic: throw exception if required API is not implemented
  wic: remove PluginMgr.get_plugin_methods
  wic: plugin: cache results in get_plugins
  wic: move PluginMgr class to pluginbase
  wic: pluginbase: use global dictionary

 scripts/lib/wic/engine.py                |   6 +-
 scripts/lib/wic/help.py                  |  11 +--
 scripts/lib/wic/partition.py             |  27 ++----
 scripts/lib/wic/plugin.py                | 151 -------------------------------
 scripts/lib/wic/pluginbase.py            |  59 ++++++++++--
 scripts/lib/wic/plugins/imager/direct.py |  13 +--
 6 files changed, 73 insertions(+), 194 deletions(-)
 delete mode 100644 scripts/lib/wic/plugin.py

-- 
Regards,
Ed

[PATCH 01/10] wic: use PluginMgr directly

[Ed Bartosh]

Instead of making a singleton object of PluginMgr class
it's simpler to use PluginMgr class directly as any class
is a singleton.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/engine.py                |  6 +--
 scripts/lib/wic/help.py                  |  4 +-
 scripts/lib/wic/partition.py             |  6 +--
 scripts/lib/wic/plugin.py                | 83 ++++++++++++++------------------
 scripts/lib/wic/plugins/imager/direct.py |  4 +-
 5 files changed, 47 insertions(+), 56 deletions(-)

diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py
index c29924b..98e7b95 100644
--- a/scripts/lib/wic/engine.py
+++ b/scripts/lib/wic/engine.py
@@ -32,7 +32,7 @@ import logging
 import os
 
 from wic import WicError
-from wic.plugin import pluginmgr
+from wic.plugin import PluginMgr
 from wic.utils.misc import get_bitbake_var
 
 logger = logging.getLogger('wic')
@@ -139,7 +139,7 @@ def list_source_plugins():
     """
     List the available source plugins i.e. plugins available for --source.
     """
-    plugins = pluginmgr.get_source_plugins()
+    plugins = PluginMgr.get_source_plugins()
 
     for plugin in plugins:
         print("  %s" % plugin)
@@ -185,7 +185,7 @@ def wic_create(wks_file, rootfs_dir, bootimg_dir, kernel_dir,
         os.makedirs(options.outdir)
 
     pname = 'direct'
-    plugin_class = pluginmgr.get_plugins('imager').get(pname)
+    plugin_class = PluginMgr.get_plugins('imager').get(pname)
     if not plugin_class:
         raise WicError('Unknown plugin: %s' % pname)
 
diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index 47e3d16..4aba12d 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -28,7 +28,7 @@
 import subprocess
 import logging
 
-from wic.plugin import pluginmgr, PLUGIN_TYPES
+from wic.plugin import PluginMgr, PLUGIN_TYPES
 
 logger = logging.getLogger('wic')
 
@@ -68,7 +68,7 @@ def get_wic_plugins_help():
     result = wic_plugins_help
     for plugin_type in PLUGIN_TYPES:
         result += '\n\n%s PLUGINS\n\n' % plugin_type.upper()
-        for name, plugin in pluginmgr.get_plugins(plugin_type).items():
+        for name, plugin in PluginMgr.get_plugins(plugin_type).items():
             result += "\n %s plugin:\n" % name
             if plugin.__doc__:
                 result += plugin.__doc__
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 8e531e4..f8d4274 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -30,7 +30,7 @@ import tempfile
 
 from wic import WicError
 from wic.utils.misc import exec_cmd, exec_native_cmd, get_bitbake_var
-from wic.plugin import pluginmgr
+from wic.plugin import PluginMgr
 
 logger = logging.getLogger('wic')
 
@@ -154,7 +154,7 @@ class Partition():
                         break
             return
 
-        plugins = pluginmgr.get_source_plugins()
+        plugins = PluginMgr.get_source_plugins()
 
         if self.source not in plugins:
             raise WicError("The '%s' --source specified for %s doesn't exist.\n\t"
@@ -176,7 +176,7 @@ class Partition():
             "do_configure_partition": None
         }
 
-        methods = pluginmgr.get_source_plugin_methods(self.source,
+        methods = PluginMgr.get_source_plugin_methods(self.source,
                                                       partition_methods)
         methods["do_configure_partition"](self, srcparams_dict, creator,
                                           cr_workdir, oe_builddir, bootimg_dir,
diff --git a/scripts/lib/wic/plugin.py b/scripts/lib/wic/plugin.py
index ac55ecf..7632c3a 100644
--- a/scripts/lib/wic/plugin.py
+++ b/scripts/lib/wic/plugin.py
@@ -22,8 +22,6 @@ import logging
 from wic import pluginbase, WicError
 from wic.utils.misc import get_bitbake_var
 
-__ALL__ = ['PluginMgr', 'pluginmgr']
-
 PLUGIN_TYPES = ["imager", "source"]
 
 PLUGIN_DIR = "/lib/wic/plugins" # relative to scripts
@@ -31,32 +29,22 @@ SCRIPTS_PLUGIN_DIR = "scripts" + PLUGIN_DIR
 
 logger = logging.getLogger('wic')
 
-class PluginMgr():
+class PluginMgr:
     plugin_dirs = {}
-
-    # make the manager class as singleton
-    _instance = None
-    def __new__(cls, *args, **kwargs):
-        if not cls._instance:
-            cls._instance = super(PluginMgr, cls).__new__(cls, *args, **kwargs)
-
-        return cls._instance
-
-    def __init__(self):
-        wic_path = os.path.dirname(__file__)
-        eos = wic_path.rfind('scripts') + len('scripts')
-        scripts_path = wic_path[:eos]
-        self.scripts_path = scripts_path
-        self.plugin_dir = scripts_path + PLUGIN_DIR
-        self.layers_path = None
-
-    def _build_plugin_dir_list(self, plugin_dir, ptype):
-        if self.layers_path is None:
-            self.layers_path = get_bitbake_var("BBLAYERS")
+    wic_path = os.path.dirname(__file__)
+    eos = wic_path.rfind('scripts') + len('scripts')
+    scripts_path = wic_path[:eos]
+    plugin_dir = scripts_path + PLUGIN_DIR
+    layers_path = None
+
+    @classmethod
+    def _build_plugin_dir_list(cls, plugin_dir, ptype):
+        if cls.layers_path is None:
+            cls.layers_path = get_bitbake_var("BBLAYERS")
         layer_dirs = []
 
-        if self.layers_path is not None:
-            for layer_path in self.layers_path.split():
+        if cls.layers_path is not None:
+            for layer_path in cls.layers_path.split():
                 path = os.path.join(layer_path, SCRIPTS_PLUGIN_DIR, ptype)
                 layer_dirs.append(path)
 
@@ -65,25 +53,28 @@ class PluginMgr():
 
         return layer_dirs
 
-    def append_dirs(self, dirs):
+    @classmethod
+    def append_dirs(cls, dirs):
         for path in dirs:
-            self._add_plugindir(path)
+            cls._add_plugindir(path)
 
         # load all the plugins AGAIN
-        self._load_all()
+        cls._load_all()
 
-    def _add_plugindir(self, path):
+    @classmethod
+    def _add_plugindir(cls, path):
         path = os.path.abspath(os.path.expanduser(path))
 
         if not os.path.isdir(path):
             return
 
-        if path not in self.plugin_dirs:
-            self.plugin_dirs[path] = False
+        if path not in cls.plugin_dirs:
+            cls.plugin_dirs[path] = False
             # the value True/False means "loaded"
 
-    def _load_all(self):
-        for (pdir, loaded) in self.plugin_dirs.items():
+    @classmethod
+    def _load_all(cls):
+        for (pdir, loaded) in cls.plugin_dirs.items():
             if loaded:
                 continue
 
@@ -91,12 +82,11 @@ class PluginMgr():
             for mod in [x[:-3] for x in os.listdir(pdir) if x.endswith(".py")]:
                 if mod and mod != '__init__':
                     if mod in sys.modules:
-                        #self.plugin_dirs[pdir] = True
                         logger.warning("Module %s already exists, skip", mod)
                     else:
                         try:
                             pymod = __import__(mod)
-                            self.plugin_dirs[pdir] = True
+                            cls.plugin_dirs[pdir] = True
                             logger.debug("Plugin module %s:%s imported",
                                          mod, pymod.__file__)
                         except ImportError as err:
@@ -105,30 +95,33 @@ class PluginMgr():
 
             del sys.path[0]
 
-    def get_plugins(self, ptype):
+    @classmethod
+    def get_plugins(cls, ptype):
         """ the return value is dict of name:class pairs """
 
         if ptype not in PLUGIN_TYPES:
             raise WicError('%s is not valid plugin type' % ptype)
 
-        plugins_dir = self._build_plugin_dir_list(self.plugin_dir, ptype)
+        plugins_dir = cls._build_plugin_dir_list(cls.plugin_dir, ptype)
 
-        self.append_dirs(plugins_dir)
+        cls.append_dirs(plugins_dir)
 
         return pluginbase.get_plugins(ptype)
 
-    def get_source_plugins(self):
+    @classmethod
+    def get_source_plugins(cls):
         """
         Return list of available source plugins.
         """
-        plugins_dir = self._build_plugin_dir_list(self.plugin_dir, 'source')
+        plugins_dir = cls._build_plugin_dir_list(cls.plugin_dir, 'source')
 
-        self.append_dirs(plugins_dir)
+        cls.append_dirs(plugins_dir)
 
-        return self.get_plugins('source')
+        return cls.get_plugins('source')
 
 
-    def get_source_plugin_methods(self, source_name, methods):
+    @classmethod
+    def get_source_plugin_methods(cls, source_name, methods):
         """
         The methods param is a dict with the method names to find.  On
         return, the dict values will be filled in with pointers to the
@@ -136,7 +129,7 @@ class PluginMgr():
         None is returned.
         """
         return_methods = None
-        for _source_name, klass in self.get_plugins('source').items():
+        for _source_name, klass in cls.get_plugins('source').items():
             if _source_name == source_name:
                 for _method_name in methods:
                     if not hasattr(klass, _method_name):
@@ -147,5 +140,3 @@ class PluginMgr():
                     methods[_method_name] = func
                     return_methods = methods
         return return_methods
-
-pluginmgr = PluginMgr()
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index 6d1ed8c..b93273e 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -35,7 +35,7 @@ from time import strftime
 from wic import WicError
 from wic.filemap import sparse_copy
 from wic.ksparser import KickStart, KickStartError
-from wic.plugin import pluginmgr
+from wic.plugin import PluginMgr
 from wic.pluginbase import ImagerPlugin
 from wic.utils.misc import get_bitbake_var, exec_cmd, exec_native_cmd
 
@@ -198,7 +198,7 @@ class DirectPlugin(ImagerPlugin):
         disk_name = self.parts[0].disk
         if source_plugin:
             name = "do_install_disk"
-            methods = pluginmgr.get_source_plugin_methods(source_plugin,
+            methods = PluginMgr.get_source_plugin_methods(source_plugin,
                                                           {name: None})
             methods["do_install_disk"](self._image, disk_name, self, self.workdir,
                                        self.oe_builddir, self.bootimg_dir,
-- 
2.1.4

[PATCH 02/10] wic: remove PluginMgr.get_source_plugins

[Ed Bartosh]

Used get_plugins('source') instead of get_source_plugins to
unify and simplify API to get plugins.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/engine.py    |  2 +-
 scripts/lib/wic/partition.py |  2 +-
 scripts/lib/wic/plugin.py    | 12 ------------
 3 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py
index 98e7b95..4d0901d 100644
--- a/scripts/lib/wic/engine.py
+++ b/scripts/lib/wic/engine.py
@@ -139,7 +139,7 @@ def list_source_plugins():
     """
     List the available source plugins i.e. plugins available for --source.
     """
-    plugins = PluginMgr.get_source_plugins()
+    plugins = PluginMgr.get_plugins('source')
 
     for plugin in plugins:
         print("  %s" % plugin)
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index f8d4274..d3cd593 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -154,7 +154,7 @@ class Partition():
                         break
             return
 
-        plugins = PluginMgr.get_source_plugins()
+        plugins = PluginMgr.get_plugins('source')
 
         if self.source not in plugins:
             raise WicError("The '%s' --source specified for %s doesn't exist.\n\t"
diff --git a/scripts/lib/wic/plugin.py b/scripts/lib/wic/plugin.py
index 7632c3a..064243d 100644
--- a/scripts/lib/wic/plugin.py
+++ b/scripts/lib/wic/plugin.py
@@ -109,18 +109,6 @@ class PluginMgr:
         return pluginbase.get_plugins(ptype)
 
     @classmethod
-    def get_source_plugins(cls):
-        """
-        Return list of available source plugins.
-        """
-        plugins_dir = cls._build_plugin_dir_list(cls.plugin_dir, 'source')
-
-        cls.append_dirs(plugins_dir)
-
-        return cls.get_plugins('source')
-
-
-    @classmethod
     def get_source_plugin_methods(cls, source_name, methods):
         """
         The methods param is a dict with the method names to find.  On
-- 
2.1.4

[PATCH 03/10] wic: reimplement PluginMgr.get_plugin_methods

[Ed Bartosh]

Simplified the implementation of get_plugin_methods:
- get rid of looping over the dicrtionary, used access by key instead
- get rid of filling a dictionary that passed as a parameter

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/help.py                  |  2 +-
 scripts/lib/wic/partition.py             | 13 +++++--------
 scripts/lib/wic/plugin.py                | 22 +++++++++-------------
 scripts/lib/wic/plugins/imager/direct.py | 10 +++++-----
 4 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index 4aba12d..196896c 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -374,7 +374,7 @@ DESCRIPTION
     This scheme is extensible - adding more hooks is a simple matter
     of adding more plugin methods to SourcePlugin and derived classes.
     The code that then needs to call the plugin methods uses
-    plugin.get_source_plugin_methods() to find the method(s) needed by
+    plugin.get_plugin_methods() to find the method(s) needed by
     the call; this is done by filling up a dict with keys containing
     the method names of interest - on success, these will be filled in
     with the actual methods. Please see the implementation for
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index d3cd593..1f384be 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -170,14 +170,11 @@ class Partition():
             splitted = self.sourceparams.split(',')
             srcparams_dict = dict(par.split('=') for par in splitted if par)
 
-        partition_methods = {
-            "do_stage_partition": None,
-            "do_prepare_partition": None,
-            "do_configure_partition": None
-        }
-
-        methods = PluginMgr.get_source_plugin_methods(self.source,
-                                                      partition_methods)
+        partition_methods = ["do_configure_partition", "do_stage_partition",
+                             "do_prepare_partition"]
+
+        methods = PluginMgr.get_plugin_methods('source', self.source,
+                                               partition_methods)
         methods["do_configure_partition"](self, srcparams_dict, creator,
                                           cr_workdir, oe_builddir, bootimg_dir,
                                           kernel_dir, native_sysroot)
diff --git a/scripts/lib/wic/plugin.py b/scripts/lib/wic/plugin.py
index 064243d..c200822 100644
--- a/scripts/lib/wic/plugin.py
+++ b/scripts/lib/wic/plugin.py
@@ -109,22 +109,18 @@ class PluginMgr:
         return pluginbase.get_plugins(ptype)
 
     @classmethod
-    def get_source_plugin_methods(cls, source_name, methods):
+    def get_plugin_methods(cls, ptype, pname, methods):
         """
         The methods param is a dict with the method names to find.  On
         return, the dict values will be filled in with pointers to the
         corresponding methods.  If one or more methods are not found,
         None is returned.
         """
-        return_methods = None
-        for _source_name, klass in cls.get_plugins('source').items():
-            if _source_name == source_name:
-                for _method_name in methods:
-                    if not hasattr(klass, _method_name):
-                        logger.warning("Unimplemented %s source interface for: %s",
-                                       _method_name, _source_name)
-                        return None
-                    func = getattr(klass, _method_name)
-                    methods[_method_name] = func
-                    return_methods = methods
-        return return_methods
+        result = {}
+        plugin = cls.get_plugins(ptype).get(pname)
+        for method in methods:
+            if not hasattr(plugin, method):
+                raise WicError("Unimplemented %s plugin interface for: %s" %
+                               (method, pname))
+            result[method] = getattr(plugin, method)
+        return result
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index b93273e..4ab1955 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -198,11 +198,11 @@ class DirectPlugin(ImagerPlugin):
         disk_name = self.parts[0].disk
         if source_plugin:
             name = "do_install_disk"
-            methods = PluginMgr.get_source_plugin_methods(source_plugin,
-                                                          {name: None})
-            methods["do_install_disk"](self._image, disk_name, self, self.workdir,
-                                       self.oe_builddir, self.bootimg_dir,
-                                       self.kernel_dir, self.native_sysroot)
+            method = PluginMgr.get_plugin_methods('source', source_plugin,
+                                                   [name])[name]
+            method(self._image, disk_name, self, self.workdir,
+                   self.oe_builddir, self.bootimg_dir,
+                   self.kernel_dir, self.native_sysroot)
 
         full_path = self._image.path
         # Generate .bmap
-- 
2.1.4

[PATCH 04/10] wic: reimplement PluginMgr.get_plugins

[Ed Bartosh]

Removed all private methods and complicated logic.
Put all code to get plugins into get_plugins method.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/plugin.py | 99 +++++++++++++----------------------------------
 1 file changed, 27 insertions(+), 72 deletions(-)

diff --git a/scripts/lib/wic/plugin.py b/scripts/lib/wic/plugin.py
index c200822..b45478c 100644
--- a/scripts/lib/wic/plugin.py
+++ b/scripts/lib/wic/plugin.py
@@ -16,95 +16,50 @@
 # Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 
 import os
-import sys
 import logging
 
+from importlib.machinery import SourceFileLoader
+
 from wic import pluginbase, WicError
 from wic.utils.misc import get_bitbake_var
 
 PLUGIN_TYPES = ["imager", "source"]
 
-PLUGIN_DIR = "/lib/wic/plugins" # relative to scripts
-SCRIPTS_PLUGIN_DIR = "scripts" + PLUGIN_DIR
+SCRIPTS_PLUGIN_DIR = "scripts/lib/wic/plugins"
 
 logger = logging.getLogger('wic')
 
 class PluginMgr:
-    plugin_dirs = {}
-    wic_path = os.path.dirname(__file__)
-    eos = wic_path.rfind('scripts') + len('scripts')
-    scripts_path = wic_path[:eos]
-    plugin_dir = scripts_path + PLUGIN_DIR
-    layers_path = None
-
-    @classmethod
-    def _build_plugin_dir_list(cls, plugin_dir, ptype):
-        if cls.layers_path is None:
-            cls.layers_path = get_bitbake_var("BBLAYERS")
-        layer_dirs = []
-
-        if cls.layers_path is not None:
-            for layer_path in cls.layers_path.split():
-                path = os.path.join(layer_path, SCRIPTS_PLUGIN_DIR, ptype)
-                layer_dirs.append(path)
-
-        path = os.path.join(plugin_dir, ptype)
-        layer_dirs.append(path)
-
-        return layer_dirs
-
-    @classmethod
-    def append_dirs(cls, dirs):
-        for path in dirs:
-            cls._add_plugindir(path)
-
-        # load all the plugins AGAIN
-        cls._load_all()
-
-    @classmethod
-    def _add_plugindir(cls, path):
-        path = os.path.abspath(os.path.expanduser(path))
-
-        if not os.path.isdir(path):
-            return
-
-        if path not in cls.plugin_dirs:
-            cls.plugin_dirs[path] = False
-            # the value True/False means "loaded"
-
-    @classmethod
-    def _load_all(cls):
-        for (pdir, loaded) in cls.plugin_dirs.items():
-            if loaded:
-                continue
-
-            sys.path.insert(0, pdir)
-            for mod in [x[:-3] for x in os.listdir(pdir) if x.endswith(".py")]:
-                if mod and mod != '__init__':
-                    if mod in sys.modules:
-                        logger.warning("Module %s already exists, skip", mod)
-                    else:
-                        try:
-                            pymod = __import__(mod)
-                            cls.plugin_dirs[pdir] = True
-                            logger.debug("Plugin module %s:%s imported",
-                                         mod, pymod.__file__)
-                        except ImportError as err:
-                            logger.warning('Failed to load plugin %s/%s: %s',
-                                           os.path.basename(pdir), mod, err)
-
-            del sys.path[0]
+    _plugin_dirs = []
+    _loaded = []
 
     @classmethod
     def get_plugins(cls, ptype):
-        """ the return value is dict of name:class pairs """
-
+        """Get dictionary of <plugin_name>:<class> pairs."""
         if ptype not in PLUGIN_TYPES:
             raise WicError('%s is not valid plugin type' % ptype)
 
-        plugins_dir = cls._build_plugin_dir_list(cls.plugin_dir, ptype)
-
-        cls.append_dirs(plugins_dir)
+        # collect plugin directories
+        if not cls._plugin_dirs:
+            cls._plugin_dirs = [os.path.join(os.path.dirname(__file__), 'plugins')]
+            layers = get_bitbake_var("BBLAYERS") or ''
+            for layer_path in layers.split():
+                path = os.path.join(layer_path, SCRIPTS_PLUGIN_DIR)
+                path = os.path.abspath(os.path.expanduser(path))
+                if path not in cls._plugin_dirs and os.path.isdir(path):
+                    cls._plugin_dirs.insert(0, path)
+
+        # load plugins
+        for pdir in cls._plugin_dirs:
+            ppath = os.path.join(pdir, ptype)
+            if ppath not in cls._loaded:
+                if os.path.isdir(ppath):
+                    for fname in os.listdir(ppath):
+                        if fname.endswith('.py'):
+                            mname = fname[:-3]
+                            mpath = os.path.join(ppath, fname)
+                            SourceFileLoader(mname, mpath).load_module()
+                cls._loaded.append(ppath)
 
         return pluginbase.get_plugins(ptype)
 
-- 
2.1.4

[PATCH 05/10] wic: pluginbase: use python 3 metaclass syntax

[Ed Bartosh]

Used more readable syntax to specify metaclass for
the base plugin classes.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/pluginbase.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/wic/pluginbase.py b/scripts/lib/wic/pluginbase.py
index aea9c02..07ccf62 100644
--- a/scripts/lib/wic/pluginbase.py
+++ b/scripts/lib/wic/pluginbase.py
@@ -32,10 +32,10 @@ class PluginMeta(type):
 
         return class_type
 
-class ImagerPlugin(PluginMeta("Plugin", (), {})):
+class ImagerPlugin(metaclass=PluginMeta):
     wic_plugin_type = "imager"
 
-class SourcePlugin(PluginMeta("Plugin", (), {})):
+class SourcePlugin(metaclass=PluginMeta):
     wic_plugin_type = "source"
     """
     The methods that can be implemented by --source plugins.
-- 
2.1.4

[PATCH 06/10] wic: throw exception if required API is not implemented

[Ed Bartosh]

Throw WicError if do_create method of imager plugin is
not implemented.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/pluginbase.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/lib/wic/pluginbase.py b/scripts/lib/wic/pluginbase.py
index 07ccf62..743170a 100644
--- a/scripts/lib/wic/pluginbase.py
+++ b/scripts/lib/wic/pluginbase.py
@@ -21,6 +21,8 @@ import logging
 
 from collections import defaultdict
 
+from wic import WicError
+
 logger = logging.getLogger('wic')
 
 class PluginMeta(type):
@@ -35,6 +37,10 @@ class PluginMeta(type):
 class ImagerPlugin(metaclass=PluginMeta):
     wic_plugin_type = "imager"
 
+    def do_create(self):
+        raise WicError("Method %s.do_create is not implemented" %
+                       self.__class__.__name__)
+
 class SourcePlugin(metaclass=PluginMeta):
     wic_plugin_type = "source"
     """
-- 
2.1.4

[PATCH 07/10] wic: remove PluginMgr.get_plugin_methods

[Ed Bartosh]

Call methods directly instead of getting them with
get_plugin_methods and then calling.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/help.py                  |  7 +------
 scripts/lib/wic/partition.py             | 20 ++++++++------------
 scripts/lib/wic/plugin.py                | 17 -----------------
 scripts/lib/wic/plugins/imager/direct.py | 10 ++++------
 4 files changed, 13 insertions(+), 41 deletions(-)

diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index 196896c..c08ad34 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -373,12 +373,7 @@ DESCRIPTION
 
     This scheme is extensible - adding more hooks is a simple matter
     of adding more plugin methods to SourcePlugin and derived classes.
-    The code that then needs to call the plugin methods uses
-    plugin.get_plugin_methods() to find the method(s) needed by
-    the call; this is done by filling up a dict with keys containing
-    the method names of interest - on success, these will be filled in
-    with the actual methods. Please see the implementation for
-    examples and details.
+    Please see the implementation for details.
 """
 
 wic_overview_help = """
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 1f384be..adf44b7 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -170,20 +170,16 @@ class Partition():
             splitted = self.sourceparams.split(',')
             srcparams_dict = dict(par.split('=') for par in splitted if par)
 
-        partition_methods = ["do_configure_partition", "do_stage_partition",
-                             "do_prepare_partition"]
-
-        methods = PluginMgr.get_plugin_methods('source', self.source,
-                                               partition_methods)
-        methods["do_configure_partition"](self, srcparams_dict, creator,
-                                          cr_workdir, oe_builddir, bootimg_dir,
-                                          kernel_dir, native_sysroot)
-        methods["do_stage_partition"](self, srcparams_dict, creator,
+        plugin = PluginMgr.get_plugins('source')[self.source]
+        plugin.do_configure_partition(self, srcparams_dict, creator,
                                       cr_workdir, oe_builddir, bootimg_dir,
                                       kernel_dir, native_sysroot)
-        methods["do_prepare_partition"](self, srcparams_dict, creator,
-                                        cr_workdir, oe_builddir, bootimg_dir,
-                                        kernel_dir, rootfs_dir, native_sysroot)
+        plugin.do_stage_partition(self, srcparams_dict, creator,
+                                  cr_workdir, oe_builddir, bootimg_dir,
+                                  kernel_dir, native_sysroot)
+        plugin.do_prepare_partition(self, srcparams_dict, creator,
+                                    cr_workdir, oe_builddir, bootimg_dir,
+                                    kernel_dir, rootfs_dir, native_sysroot)
 
         # further processing required Partition.size to be an integer, make
         # sure that it is one
diff --git a/scripts/lib/wic/plugin.py b/scripts/lib/wic/plugin.py
index b45478c..36a120b 100644
--- a/scripts/lib/wic/plugin.py
+++ b/scripts/lib/wic/plugin.py
@@ -62,20 +62,3 @@ class PluginMgr:
                 cls._loaded.append(ppath)
 
         return pluginbase.get_plugins(ptype)
-
-    @classmethod
-    def get_plugin_methods(cls, ptype, pname, methods):
-        """
-        The methods param is a dict with the method names to find.  On
-        return, the dict values will be filled in with pointers to the
-        corresponding methods.  If one or more methods are not found,
-        None is returned.
-        """
-        result = {}
-        plugin = cls.get_plugins(ptype).get(pname)
-        for method in methods:
-            if not hasattr(plugin, method):
-                raise WicError("Unimplemented %s plugin interface for: %s" %
-                               (method, pname))
-            result[method] = getattr(plugin, method)
-        return result
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index 4ab1955..7221648 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -197,12 +197,10 @@ class DirectPlugin(ImagerPlugin):
         source_plugin = self.ks.bootloader.source
         disk_name = self.parts[0].disk
         if source_plugin:
-            name = "do_install_disk"
-            method = PluginMgr.get_plugin_methods('source', source_plugin,
-                                                   [name])[name]
-            method(self._image, disk_name, self, self.workdir,
-                   self.oe_builddir, self.bootimg_dir,
-                   self.kernel_dir, self.native_sysroot)
+            plugin = PluginMgr.get_plugins('source')[source_plugin]
+            plugin.do_install_disk(self._image, disk_name, self, self.workdir,
+                                   self.oe_builddir, self.bootimg_dir,
+                                   self.kernel_dir, self.native_sysroot)
 
         full_path = self._image.path
         # Generate .bmap
-- 
2.1.4

[PATCH 08/10] wic: plugin: cache results in get_plugins

[Ed Bartosh]

Store results of PluginMgr.get_plugins to avoid
loading plugins more than once.

This should speed up finding plugins.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/plugin.py | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/scripts/lib/wic/plugin.py b/scripts/lib/wic/plugin.py
index 36a120b..094a878 100644
--- a/scripts/lib/wic/plugin.py
+++ b/scripts/lib/wic/plugin.py
@@ -31,7 +31,7 @@ logger = logging.getLogger('wic')
 
 class PluginMgr:
     _plugin_dirs = []
-    _loaded = []
+    _plugins = {}
 
     @classmethod
     def get_plugins(cls, ptype):
@@ -39,6 +39,9 @@ class PluginMgr:
         if ptype not in PLUGIN_TYPES:
             raise WicError('%s is not valid plugin type' % ptype)
 
+        if ptype in cls._plugins:
+            return cls._plugins[ptype]
+
         # collect plugin directories
         if not cls._plugin_dirs:
             cls._plugin_dirs = [os.path.join(os.path.dirname(__file__), 'plugins')]
@@ -52,13 +55,12 @@ class PluginMgr:
         # load plugins
         for pdir in cls._plugin_dirs:
             ppath = os.path.join(pdir, ptype)
-            if ppath not in cls._loaded:
-                if os.path.isdir(ppath):
-                    for fname in os.listdir(ppath):
-                        if fname.endswith('.py'):
-                            mname = fname[:-3]
-                            mpath = os.path.join(ppath, fname)
-                            SourceFileLoader(mname, mpath).load_module()
-                cls._loaded.append(ppath)
+            if os.path.isdir(ppath):
+                for fname in os.listdir(ppath):
+                    if fname.endswith('.py'):
+                        mname = fname[:-3]
+                        mpath = os.path.join(ppath, fname)
+                        SourceFileLoader(mname, mpath).load_module()
 
-        return pluginbase.get_plugins(ptype)
+        cls._plugins[ptype] = pluginbase.get_plugins(ptype)
+        return cls._plugins[ptype]
-- 
2.1.4

[PATCH 09/10] wic: move PluginMgr class to pluginbase

[Ed Bartosh]

As PluginMgr class contains only one method it's
better to move it to pluginbase to have all plugin
related APIs in one module.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/engine.py                |  2 +-
 scripts/lib/wic/help.py                  |  2 +-
 scripts/lib/wic/partition.py             |  2 +-
 scripts/lib/wic/plugin.py                | 66 --------------------------------
 scripts/lib/wic/pluginbase.py            | 47 +++++++++++++++++++++--
 scripts/lib/wic/plugins/imager/direct.py |  3 +-
 6 files changed, 48 insertions(+), 74 deletions(-)
 delete mode 100644 scripts/lib/wic/plugin.py

diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py
index 4d0901d..f59821f 100644
--- a/scripts/lib/wic/engine.py
+++ b/scripts/lib/wic/engine.py
@@ -32,7 +32,7 @@ import logging
 import os
 
 from wic import WicError
-from wic.plugin import PluginMgr
+from wic.pluginbase import PluginMgr
 from wic.utils.misc import get_bitbake_var
 
 logger = logging.getLogger('wic')
diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index c08ad34..148da89 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -28,7 +28,7 @@
 import subprocess
 import logging
 
-from wic.plugin import PluginMgr, PLUGIN_TYPES
+from wic.pluginbase import PluginMgr, PLUGIN_TYPES
 
 logger = logging.getLogger('wic')
 
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index adf44b7..6ef8d7f 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -30,7 +30,7 @@ import tempfile
 
 from wic import WicError
 from wic.utils.misc import exec_cmd, exec_native_cmd, get_bitbake_var
-from wic.plugin import PluginMgr
+from wic.pluginbase import PluginMgr
 
 logger = logging.getLogger('wic')
 
diff --git a/scripts/lib/wic/plugin.py b/scripts/lib/wic/plugin.py
deleted file mode 100644
index 094a878..0000000
--- a/scripts/lib/wic/plugin.py
+++ /dev/null
@@ -1,66 +0,0 @@
-#!/usr/bin/env python -tt
-#
-# Copyright (c) 2011 Intel, Inc.
-#
-# This program is free software; you can redistribute it and/or modify it
-# under the terms of the GNU General Public License as published by the Free
-# Software Foundation; version 2 of the License
-#
-# This program is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
-# or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
-# for more details.
-#
-# You should have received a copy of the GNU General Public License along
-# with this program; if not, write to the Free Software Foundation, Inc., 59
-# Temple Place - Suite 330, Boston, MA 02111-1307, USA.
-
-import os
-import logging
-
-from importlib.machinery import SourceFileLoader
-
-from wic import pluginbase, WicError
-from wic.utils.misc import get_bitbake_var
-
-PLUGIN_TYPES = ["imager", "source"]
-
-SCRIPTS_PLUGIN_DIR = "scripts/lib/wic/plugins"
-
-logger = logging.getLogger('wic')
-
-class PluginMgr:
-    _plugin_dirs = []
-    _plugins = {}
-
-    @classmethod
-    def get_plugins(cls, ptype):
-        """Get dictionary of <plugin_name>:<class> pairs."""
-        if ptype not in PLUGIN_TYPES:
-            raise WicError('%s is not valid plugin type' % ptype)
-
-        if ptype in cls._plugins:
-            return cls._plugins[ptype]
-
-        # collect plugin directories
-        if not cls._plugin_dirs:
-            cls._plugin_dirs = [os.path.join(os.path.dirname(__file__), 'plugins')]
-            layers = get_bitbake_var("BBLAYERS") or ''
-            for layer_path in layers.split():
-                path = os.path.join(layer_path, SCRIPTS_PLUGIN_DIR)
-                path = os.path.abspath(os.path.expanduser(path))
-                if path not in cls._plugin_dirs and os.path.isdir(path):
-                    cls._plugin_dirs.insert(0, path)
-
-        # load plugins
-        for pdir in cls._plugin_dirs:
-            ppath = os.path.join(pdir, ptype)
-            if os.path.isdir(ppath):
-                for fname in os.listdir(ppath):
-                    if fname.endswith('.py'):
-                        mname = fname[:-3]
-                        mpath = os.path.join(ppath, fname)
-                        SourceFileLoader(mname, mpath).load_module()
-
-        cls._plugins[ptype] = pluginbase.get_plugins(ptype)
-        return cls._plugins[ptype]
diff --git a/scripts/lib/wic/pluginbase.py b/scripts/lib/wic/pluginbase.py
index 743170a..93f0b66 100644
--- a/scripts/lib/wic/pluginbase.py
+++ b/scripts/lib/wic/pluginbase.py
@@ -15,16 +15,59 @@
 # with this program; if not, write to the Free Software Foundation, Inc., 59
 # Temple Place - Suite 330, Boston, MA 02111-1307, USA.
 
-__all__ = ['ImagerPlugin', 'SourcePlugin', 'get_plugins']
+__all__ = ['ImagerPlugin', 'SourcePlugin']
 
+import os
 import logging
 
 from collections import defaultdict
+from importlib.machinery import SourceFileLoader
 
 from wic import WicError
+from wic.utils.misc import get_bitbake_var
+
+PLUGIN_TYPES = ["imager", "source"]
+
+SCRIPTS_PLUGIN_DIR = "scripts/lib/wic/plugins"
 
 logger = logging.getLogger('wic')
 
+class PluginMgr:
+    _plugin_dirs = []
+    _plugins = {}
+
+    @classmethod
+    def get_plugins(cls, ptype):
+        """Get dictionary of <plugin_name>:<class> pairs."""
+        if ptype not in PLUGIN_TYPES:
+            raise WicError('%s is not valid plugin type' % ptype)
+
+        if ptype in cls._plugins:
+            return cls._plugins[ptype]
+
+        # collect plugin directories
+        if not cls._plugin_dirs:
+            cls._plugin_dirs = [os.path.join(os.path.dirname(__file__), 'plugins')]
+            layers = get_bitbake_var("BBLAYERS") or ''
+            for layer_path in layers.split():
+                path = os.path.join(layer_path, SCRIPTS_PLUGIN_DIR)
+                path = os.path.abspath(os.path.expanduser(path))
+                if path not in cls._plugin_dirs and os.path.isdir(path):
+                    cls._plugin_dirs.insert(0, path)
+
+        # load plugins
+        for pdir in cls._plugin_dirs:
+            ppath = os.path.join(pdir, ptype)
+            if os.path.isdir(ppath):
+                for fname in os.listdir(ppath):
+                    if fname.endswith('.py'):
+                        mname = fname[:-3]
+                        mpath = os.path.join(ppath, fname)
+                        SourceFileLoader(mname, mpath).load_module()
+
+        cls._plugins[ptype] = PluginMeta.plugins.get(ptype)
+        return cls._plugins[ptype]
+
 class PluginMeta(type):
     plugins = defaultdict(dict)
     def __new__(cls, name, bases, attrs):
@@ -97,5 +140,3 @@ class SourcePlugin(metaclass=PluginMeta):
         """
         logger.debug("SourcePlugin: do_prepare_partition: part: %s", part)
 
-def get_plugins(typen):
-    return PluginMeta.plugins.get(typen)
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index 7221648..b7e324a 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -35,8 +35,7 @@ from time import strftime
 from wic import WicError
 from wic.filemap import sparse_copy
 from wic.ksparser import KickStart, KickStartError
-from wic.plugin import PluginMgr
-from wic.pluginbase import ImagerPlugin
+from wic.pluginbase import PluginMgr, ImagerPlugin
 from wic.utils.misc import get_bitbake_var, exec_cmd, exec_native_cmd
 
 logger = logging.getLogger('wic')
-- 
2.1.4

[PATCH 10/10] wic: pluginbase: use global dictionary

[Ed Bartosh]

Made PluginMeta to populate global PLUGINS dictionary that
is accessed by PluginMgr. This should make the code more
understandable as PluginMgr don't need to get data directly
from PlugnMeta attribute.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/pluginbase.py | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/scripts/lib/wic/pluginbase.py b/scripts/lib/wic/pluginbase.py
index 93f0b66..fb3d179 100644
--- a/scripts/lib/wic/pluginbase.py
+++ b/scripts/lib/wic/pluginbase.py
@@ -32,9 +32,10 @@ SCRIPTS_PLUGIN_DIR = "scripts/lib/wic/plugins"
 
 logger = logging.getLogger('wic')
 
+PLUGINS = defaultdict(dict)
+
 class PluginMgr:
     _plugin_dirs = []
-    _plugins = {}
 
     @classmethod
     def get_plugins(cls, ptype):
@@ -42,9 +43,6 @@ class PluginMgr:
         if ptype not in PLUGIN_TYPES:
             raise WicError('%s is not valid plugin type' % ptype)
 
-        if ptype in cls._plugins:
-            return cls._plugins[ptype]
-
         # collect plugin directories
         if not cls._plugin_dirs:
             cls._plugin_dirs = [os.path.join(os.path.dirname(__file__), 'plugins')]
@@ -55,25 +53,25 @@ class PluginMgr:
                 if path not in cls._plugin_dirs and os.path.isdir(path):
                     cls._plugin_dirs.insert(0, path)
 
-        # load plugins
-        for pdir in cls._plugin_dirs:
-            ppath = os.path.join(pdir, ptype)
-            if os.path.isdir(ppath):
-                for fname in os.listdir(ppath):
-                    if fname.endswith('.py'):
-                        mname = fname[:-3]
-                        mpath = os.path.join(ppath, fname)
-                        SourceFileLoader(mname, mpath).load_module()
+        if ptype not in PLUGINS:
+            # load all ptype plugins
+            for pdir in cls._plugin_dirs:
+                ppath = os.path.join(pdir, ptype)
+                if os.path.isdir(ppath):
+                    for fname in os.listdir(ppath):
+                        if fname.endswith('.py'):
+                            mname = fname[:-3]
+                            mpath = os.path.join(ppath, fname)
+                            logger.debug("loading plugin module %s", mpath)
+                            SourceFileLoader(mname, mpath).load_module()
 
-        cls._plugins[ptype] = PluginMeta.plugins.get(ptype)
-        return cls._plugins[ptype]
+        return PLUGINS.get(ptype)
 
 class PluginMeta(type):
-    plugins = defaultdict(dict)
     def __new__(cls, name, bases, attrs):
         class_type = type.__new__(cls, name, bases, attrs)
         if 'name' in attrs:
-            cls.plugins[class_type.wic_plugin_type][attrs['name']] = class_type
+            PLUGINS[class_type.wic_plugin_type][attrs['name']] = class_type
 
         return class_type
 
-- 
2.1.4