[jhbuild/wip/sysdeps-rebase2: 1/5] Move some XML parsing for Package into base class

[Colin Walters <walters src gnome org>]

commit c8023ed315f2273fcbd9a49afd5863974e9c3232
Author: Colin Walters <walters verbum org>
Date:   Wed Jul 13 19:26:57 2011 -0400

    Move some XML parsing for Package into base class
    
    All module types repeatedly parsed the common attributes like 'id',
    and while they shared a function to parse things like dependencies
    and branch, it was still a copied line of code.
    
    Also, all module types had to have two copies of the default values
    of their attributes (like 'autogen.sh' for autogen_sh of AutogenModule)
    due to the way the constructors were invoked.
    
    Clean this up by moving the XML parsing for the common bits into the
    Package class.  Then each module type explicitly only cares about
    the attributes it adds.
    
    The diff speaks for itself; compare e.g. waf.py before and after.
    
    Note - autotools needed a bigger refactoring due to things like the
    $prefix substitutions and changing the default for 'autogen-sh' based
    on whether or not the branch is a tarball.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=654582
    
    https://bugzilla.gnome.org/show_bug.cgi?id=564373

 jhbuild/modtypes/__init__.py   |   11 ++++-
 jhbuild/modtypes/autotools.py  |  102 ++++++++++++++++------------------------
 jhbuild/modtypes/cmake.py      |   25 +++-------
 jhbuild/modtypes/distutils.py  |   22 +++------
 jhbuild/modtypes/linux.py      |   11 ++---
 jhbuild/modtypes/perl.py       |   27 ++++-------
 jhbuild/modtypes/tarball.py    |   17 ++++--
 jhbuild/modtypes/testmodule.py |   19 +++----
 jhbuild/modtypes/waf.py        |   21 +++------
 9 files changed, 106 insertions(+), 149 deletions(-)
---
diff --git a/jhbuild/modtypes/__init__.py b/jhbuild/modtypes/__init__.py
index 0110e65..0e2df03 100644
--- a/jhbuild/modtypes/__init__.py
+++ b/jhbuild/modtypes/__init__.py
@@ -126,8 +126,9 @@ class Package:
     type = 'base'
     PHASE_START = 'start'
     PHASE_DONE  = 'done'
-    def __init__(self, name, dependencies = [], after = [], suggests = []):
+    def __init__(self, name, branch=None, dependencies = [], after = [], suggests = []):
         self.name = name
+        self.branch = branch
         self.dependencies = dependencies
         self.after = after
         self.suggests = suggests
@@ -363,6 +364,14 @@ them into the prefix."""
         """Serialize this module's checkout branch as sxml."""
         return self.branch.to_sxml()
 
+    @classmethod
+    def parse_from_xml(cls, node, config, uri, repositories, default_repo):
+        """Create a new Package instance from a DOM XML node."""
+        name = node.getAttribute('id')
+        instance = cls(name)
+        instance.branch = get_branch(node, repositories, default_repo, config)
+        instance.dependencies, instance.after, instance.suggests = get_dependencies(node)
+        return instance
 
 class DownloadableModule:
     PHASE_CHECKOUT = 'checkout'
diff --git a/jhbuild/modtypes/autotools.py b/jhbuild/modtypes/autotools.py
index 52fa4af..ae05f19 100644
--- a/jhbuild/modtypes/autotools.py
+++ b/jhbuild/modtypes/autotools.py
@@ -26,7 +26,7 @@ import stat
 
 from jhbuild.errors import FatalError, BuildStateError, CommandError
 from jhbuild.modtypes import \
-     Package, DownloadableModule, get_dependencies, get_branch, register_module_type
+     Package, DownloadableModule, register_module_type
 
 __all__ = [ 'AutogenModule' ]
 
@@ -46,17 +46,16 @@ class AutogenModule(Package, DownloadableModule):
     PHASE_DIST           = 'dist'
     PHASE_INSTALL        = 'install'
 
-    def __init__(self, name, branch, autogenargs='', makeargs='',
+    def __init__(self, name,
+                 autogenargs='', makeargs='',
                  makeinstallargs='',
-                 dependencies=[], after=[], suggests=[],
                  supports_non_srcdir_builds=True,
                  skip_autogen=False,
                  autogen_sh='autogen.sh',
                  makefile='Makefile',
                  autogen_template=None,
                  check_target=True):
-        Package.__init__(self, name, dependencies, after, suggests)
-        self.branch = branch
+        Package.__init__(self, name)
         self.autogenargs = autogenargs
         self.makeargs    = makeargs
         self.makeinstallargs = makeinstallargs
@@ -314,78 +313,59 @@ class AutogenModule(Package, DownloadableModule):
 
 
 def parse_autotools(node, config, uri, repositories, default_repo):
-    id = node.getAttribute('id')
-    autogenargs = ''
-    makeargs = ''
-    makeinstallargs = ''
-    supports_non_srcdir_builds = True
-    autogen_sh = None
-    skip_autogen = False
-    check_target = True
-    makefile = 'Makefile'
-    autogen_template = None
+    instance = AutogenModule.parse_from_xml(node, config, uri, repositories, default_repo)
+
+    # Make some substitutions; do special handling of '${prefix}' and '${libdir}'
+    prefix_re = re.compile('(\${prefix})')
+    # I'm not sure the replacement of ${libdir} is necessary for firefox...
+    libdir_re = re.compile('(\${libdir})')
+    libsubdir = '/lib'
+    if config.use_lib64:
+        libsubdir = '/lib64'
+
     if node.hasAttribute('autogenargs'):
         autogenargs = node.getAttribute('autogenargs')
+        autogenargs = prefix_re.sub(config.prefix, autogenargs)
+        autogenargs = libdir_re.sub(config.prefix + libsubdir, autogenargs)        
+        instance.autogenargs = autogenargs
     if node.hasAttribute('makeargs'):
         makeargs = node.getAttribute('makeargs')
+        makeargs = prefix_re.sub(config.prefix, makeargs)
+        makeargs = libdir_re.sub(config.prefix + libsubdir, makeargs)
+        instance.makeargs = makeargs
     if node.hasAttribute('makeinstallargs'):
         makeinstallargs = node.getAttribute('makeinstallargs')
+        makeinstallargs = prefix_re.sub(config.prefix, makeinstallargs)
+        makeinstallargs = libdir_re.sub(config.prefix + libsubdir, makeinstallargs)
+        instance.makeinstallargs = makeinstallargs
+
     if node.hasAttribute('supports-non-srcdir-builds'):
-        supports_non_srcdir_builds = \
-            (node.getAttribute('supports-non-srcdir-builds') != 'no')
+        supports_non_srcdir_builds = (node.getAttribute('supports-non-srcdir-builds') != 'no')
     if node.hasAttribute('skip-autogen'):
         skip_autogen = node.getAttribute('skip-autogen')
         if skip_autogen == 'true':
-            skip_autogen = True
+            instance.skip_autogen = True
         elif skip_autogen == 'never':
-            skip_autogen = 'never'
-        else:
-            skip_autogen = False
+            instance.skip_autogen = 'never'
+
     if node.hasAttribute('check-target'):
-        check_target = (node.getAttribute('check-target') == 'true')
+        instance.check_target = (node.getAttribute('check-target') == 'true')
+
+    from jhbuild.versioncontrol.tarball import TarballBranch
     if node.hasAttribute('autogen-sh'):
         autogen_sh = node.getAttribute('autogen-sh')
+        if autogen_sh is not None:
+            instance.autogen_sh = autogen_sh
+        elif isinstance(instance.branch, TarballBranch):
+            # in tarballs, force autogen-sh to be configure, unless autogen-sh is
+            # already set
+            instance.autogen_sh = 'configure'
+
     if node.hasAttribute('makefile'):
-        makefile = node.getAttribute('makefile')
+        instance.makefile = node.getAttribute('makefile')
     if node.hasAttribute('autogen-template'):
-        autogen_template = node.getAttribute('autogen-template')
-
-    # Make some substitutions; do special handling of '${prefix}' and '${libdir}'
-    p = re.compile('(\${prefix})')
-    autogenargs     = p.sub(config.prefix, autogenargs)
-    makeargs        = p.sub(config.prefix, makeargs)
-    makeinstallargs = p.sub(config.prefix, makeinstallargs)
-    # I'm not sure the replacement of ${libdir} is necessary for firefox...
-    p = re.compile('(\${libdir})')
-    libsubdir = '/lib'
-    if config.use_lib64:
-        libsubdir = '/lib64'
-    autogenargs     = p.sub(config.prefix + libsubdir, autogenargs)
-    makeargs        = p.sub(config.prefix + libsubdir, makeargs)
-    makeinstallargs = p.sub(config.prefix + libsubdir, makeinstallargs)
+        instance.autogen_template = node.getAttribute('autogen-template')
 
-    dependencies, after, suggests = get_dependencies(node)
-    branch = get_branch(node, repositories, default_repo, config)
-
-    from jhbuild.versioncontrol.tarball import TarballBranch
-    if isinstance(branch, TarballBranch):
-        # in tarballs, force autogen-sh to be configure, unless autogen-sh is
-        # already set
-        if autogen_sh is None:
-            autogen_sh = 'configure'
-    elif not autogen_sh:
-        autogen_sh = 'autogen.sh'
-
-    return AutogenModule(id, branch, autogenargs, makeargs,
-                         makeinstallargs=makeinstallargs,
-                         dependencies=dependencies,
-                         after=after,
-                         suggests=suggests,
-                         supports_non_srcdir_builds=supports_non_srcdir_builds,
-                         skip_autogen=skip_autogen,
-                         autogen_sh=autogen_sh,
-                         makefile=makefile,
-                         autogen_template=autogen_template,
-                         check_target=check_target)
+    return instance
 register_module_type('autotools', parse_autotools)
 
diff --git a/jhbuild/modtypes/cmake.py b/jhbuild/modtypes/cmake.py
index b24eb83..6ebf590 100644
--- a/jhbuild/modtypes/cmake.py
+++ b/jhbuild/modtypes/cmake.py
@@ -23,7 +23,7 @@ import os
 
 from jhbuild.errors import BuildStateError
 from jhbuild.modtypes import \
-     Package, DownloadableModule, get_dependencies, get_branch, register_module_type
+     Package, DownloadableModule, register_module_type
 
 __all__ = [ 'CMakeModule' ]
 
@@ -38,10 +38,9 @@ class CMakeModule(Package, DownloadableModule):
     PHASE_DIST = 'dist'
     PHASE_INSTALL = 'install'
 
-    def __init__(self, name, branch, cmakeargs='', makeargs='',
-                 dependencies=[], after=[], suggests=[]):
-        Package.__init__(self, name, dependencies, after, suggests)
-        self.branch = branch
+    def __init__(self, name,
+                 cmakeargs='', makeargs='',):
+        Package.__init__(self, name)
         self.cmakeargs = cmakeargs
         self.makeargs  = makeargs
         self.supports_install_destdir = True
@@ -144,20 +143,12 @@ class CMakeModule(Package, DownloadableModule):
 
 
 def parse_cmake(node, config, uri, repositories, default_repo):
-    id = node.getAttribute('id')
-    cmakeargs = ''
-    makeargs = ''
+    instance = CMakeModule.parse_from_xml(node, config, uri, repositories, default_repo)
     if node.hasAttribute('cmakeargs'):
-        cmakeargs = node.getAttribute('cmakeargs')
+        instance.cmakeargs = node.getAttribute('cmakeargs')
     if node.hasAttribute('makeargs'):
-        makeargs = node.getAttribute('makeargs')
-
-    dependencies, after, suggests = get_dependencies(node)
-    branch = get_branch(node, repositories, default_repo, config)
-
-    return CMakeModule(id, branch, cmakeargs, makeargs,
-                       dependencies = dependencies, after = after,
-                       suggests = suggests)
+        instance.makeargs = node.getAttribute('makeargs')
+    return instance
 
 register_module_type('cmake', parse_cmake)
 
diff --git a/jhbuild/modtypes/distutils.py b/jhbuild/modtypes/distutils.py
index d4d56a1..513aa96 100644
--- a/jhbuild/modtypes/distutils.py
+++ b/jhbuild/modtypes/distutils.py
@@ -23,7 +23,7 @@ import os
 
 from jhbuild.errors import BuildStateError
 from jhbuild.modtypes import \
-     Package, DownloadableModule, get_dependencies, get_branch, register_module_type
+     Package, DownloadableModule, register_module_type
 
 __all__ = [ 'DistutilsModule' ]
 
@@ -37,11 +37,8 @@ class DistutilsModule(Package, DownloadableModule):
     PHASE_BUILD = 'build'
     PHASE_INSTALL = 'install'
 
-    def __init__(self, name, branch,
-                 dependencies = [], after = [], suggests = [],
-                 supports_non_srcdir_builds = True):
-        Package.__init__(self, name, dependencies, after, suggests)
-        self.branch = branch
+    def __init__(self, name, supports_non_srcdir_builds = True):
+        Package.__init__(self, name)
         self.supports_non_srcdir_builds = supports_non_srcdir_builds
         self.supports_install_destdir = True
 
@@ -91,18 +88,13 @@ class DistutilsModule(Package, DownloadableModule):
 
 
 def parse_distutils(node, config, uri, repositories, default_repo):
-    id = node.getAttribute('id')
-    supports_non_srcdir_builds = True
+    instance = DistutilsModule.parse_from_xml(node, config, uri, repositories, default_repo)
 
     if node.hasAttribute('supports-non-srcdir-builds'):
-        supports_non_srcdir_builds = \
+        instance.supports_non_srcdir_builds = \
             (node.getAttribute('supports-non-srcdir-builds') != 'no')
-    dependencies, after, suggests = get_dependencies(node)
-    branch = get_branch(node, repositories, default_repo, config)
 
-    return DistutilsModule(id, branch,
-            dependencies = dependencies, after = after,
-            suggests = suggests,
-            supports_non_srcdir_builds = supports_non_srcdir_builds)
+    return instance
+
 register_module_type('distutils', parse_distutils)
 
diff --git a/jhbuild/modtypes/linux.py b/jhbuild/modtypes/linux.py
index 57c86f2..774f5e3 100644
--- a/jhbuild/modtypes/linux.py
+++ b/jhbuild/modtypes/linux.py
@@ -27,7 +27,7 @@ import errno
 
 from jhbuild.errors import FatalError, BuildStateError
 from jhbuild.modtypes import \
-     Package, get_dependencies, get_branch, register_module_type
+     Package, register_module_type
 
 __all__ = [ 'LinuxModule' ]
 
@@ -62,10 +62,8 @@ class LinuxModule(Package):
     PHASE_HEADERS_INSTALL = 'headers_install'
     PHASE_INSTALL         = 'install'
 
-    def __init__(self, name, branch, kconfigs, makeargs,
-            dependencies, after, suggests):
-        Package.__init__(self, name, dependencies, after, suggests)
-        self.branch = branch
+    def __init__(self, name, kconfigs=None, makeargs=None):
+        Package.__init__(self, name)
         self.kconfigs = kconfigs
         self.makeargs = makeargs
 
@@ -281,7 +279,6 @@ def parse_linux(node, config, uri, repositories, default_repo):
     branch = get_branch(node, repositories, default_repo, config)
     kconfigs = get_kconfigs(node, repositories, default_repo)
 
-    return LinuxModule(id, branch, kconfigs,
-                       makeargs, dependencies, after, suggests)
+    return LinuxModule(id, branch, dependencies, after, suggests, kconfigs, makeargs)
 
 register_module_type('linux', parse_linux)
diff --git a/jhbuild/modtypes/perl.py b/jhbuild/modtypes/perl.py
index f5173b6..9238f04 100644
--- a/jhbuild/modtypes/perl.py
+++ b/jhbuild/modtypes/perl.py
@@ -24,7 +24,7 @@ import re
 
 from jhbuild.errors import BuildStateError
 from jhbuild.modtypes import \
-     Package, DownloadableModule, get_dependencies, get_branch, register_module_type
+     Package, DownloadableModule, register_module_type
 
 __all__ = [ 'PerlModule' ]
 
@@ -38,10 +38,8 @@ class PerlModule(Package, DownloadableModule):
     PHASE_BUILD = 'build'
     PHASE_INSTALL = 'install'
 
-    def __init__(self, name, branch, makeargs='',
-                 dependencies=[], after=[], suggests=[]):
-        Package.__init__(self, name, dependencies, after, suggests)
-        self.branch = branch
+    def __init__(self, name, makeargs=''):
+        Package.__init__(self, name)
         self.makeargs = makeargs
 
     def get_srcdir(self, buildscript):
@@ -81,20 +79,15 @@ class PerlModule(Package, DownloadableModule):
 
 
 def parse_perl(node, config, uri, repositories, default_repo):
-    id = node.getAttribute('id')
-    makeargs = ''
+    instance = PerlModule.parse_from_xml(node, config, uri, repositories, default_repo)
+
+    # Make some substitutions; do special handling of '${prefix}'
+    prefix_re = re.compile('(\${prefix})')
     if node.hasAttribute('makeargs'):
         makeargs = node.getAttribute('makeargs')
+        makeargs = prefix_re.sub(config.prefix, makeargs)
+        instance.makeargs = makeargs
 
-    # Make some substitutions; do special handling of '${prefix}'
-    p = re.compile('(\${prefix})')
-    makeargs = p.sub(config.prefix, makeargs)
-    
-    dependencies, after, suggests = get_dependencies(node)
-    branch = get_branch(node, repositories, default_repo, config)
-
-    return PerlModule(id, branch, makeargs,
-            dependencies=dependencies, after=after,
-            suggests=suggests)
+    return instance
 register_module_type('perl', parse_perl)
 
diff --git a/jhbuild/modtypes/tarball.py b/jhbuild/modtypes/tarball.py
index 70d797a..52b75c7 100644
--- a/jhbuild/modtypes/tarball.py
+++ b/jhbuild/modtypes/tarball.py
@@ -98,11 +98,16 @@ def parse_tarball(node, config, uri, repositories, default_repo):
             source_size, source_hash, None)
     branch.patches = patches
 
-    return AutogenModule(name, branch,
-            autogenargs, makeargs, makeinstallargs,
-            dependencies, after, suggests,
-            supports_non_srcdir_builds = supports_non_srcdir_builds,
-            skip_autogen = False, autogen_sh = 'configure',
-            makefile = makefile)
+    instance = AutogenModule(name,
+                             autogenargs, makeargs, makeinstallargs,
+                             supports_non_srcdir_builds = supports_non_srcdir_builds,
+                             skip_autogen = False, autogen_sh = 'configure',
+                             makefile = makefile)
+    instance.branch = branch
+    instance.dependencies = dependencies
+    instance.after = after
+    instance.suggests = suggests
+
+    return instance
 
 register_module_type('tarball', parse_tarball)
diff --git a/jhbuild/modtypes/testmodule.py b/jhbuild/modtypes/testmodule.py
index cba3528..16d9932 100644
--- a/jhbuild/modtypes/testmodule.py
+++ b/jhbuild/modtypes/testmodule.py
@@ -28,7 +28,7 @@ except ImportError:
 
 from jhbuild.errors import FatalError, CommandError, BuildStateError
 from jhbuild.modtypes import \
-     Package, DownloadableModule, get_dependencies, get_branch, register_module_type
+     Package, DownloadableModule, register_module_type
 from jhbuild.modtypes.autotools import AutogenModule
 
 import xml.dom.minidom
@@ -43,12 +43,9 @@ class TestModule(Package, DownloadableModule):
     PHASE_FORCE_CHECKOUT = DownloadableModule.PHASE_FORCE_CHECKOUT
     PHASE_TEST           = 'test'
     
-    def __init__(self, name, branch, test_type, dependencies=[], after=[], tested_pkgs=[]):
+    def __init__(self, name, test_type=None, tested_pkgs=[]):
         Package.__init__(self, name)
-        self.branch       = branch
         self.test_type    = test_type
-        self.dependencies = dependencies
-        self.after        = after
         self.tested_pkgs  = tested_pkgs
 
         ### modify environ for tests to be working
@@ -336,16 +333,16 @@ def get_tested_packages(node):
     return tested_pkgs
 
 def parse_testmodule(node, config, uri, repositories, default_repo):
-    id = node.getAttribute('id')
+    instance = TestModule.parse_from_xml(node, config, uri, repositories, default_repo)
+
     test_type = node.getAttribute('type')
     if test_type not in __test_types__:
         # FIXME: create an error here
         pass
+    instance.test_type = test_type
 
-    dependencies, after, suggests = get_dependencies(node)
-    branch = get_branch(node, repositories, default_repo, config)
-    tested_pkgs = get_tested_packages(node)
-    return TestModule(id, branch, test_type, dependencies=dependencies,
-            after=after, tested_pkgs=tested_pkgs)
+    instance.tested_pkgs = get_tested_packages(node)
+    
+    return instance
                                    
 register_module_type('testmodule', parse_testmodule)
diff --git a/jhbuild/modtypes/waf.py b/jhbuild/modtypes/waf.py
index e12b16c..c9d5c88 100644
--- a/jhbuild/modtypes/waf.py
+++ b/jhbuild/modtypes/waf.py
@@ -26,7 +26,7 @@ import re
 
 from jhbuild.errors import FatalError, BuildStateError, CommandError
 from jhbuild.modtypes import \
-     Package, DownloadableModule, get_dependencies, get_branch, register_module_type
+     Package, DownloadableModule, register_module_type
 from jhbuild.commands.sanitycheck import inpath
 
 __all__ = [ 'WafModule' ]
@@ -44,10 +44,8 @@ class WafModule(Package, DownloadableModule):
     PHASE_DIST           = 'dist'
     PHASE_INSTALL        = 'install'
 
-    def __init__(self, name, branch, dependencies=[], after=[], suggests=[],
-                 waf_cmd='./waf'):
-        Package.__init__(self, name, dependencies, after, suggests)
-        self.branch = branch
+    def __init__(self, name, waf_cmd='./waf'):
+        Package.__init__(self, name)
         self.waf_cmd = waf_cmd
         self.supports_install_destdir = True
 
@@ -147,16 +145,11 @@ class WafModule(Package, DownloadableModule):
 
 
 def parse_waf(node, config, uri, repositories, default_repo):
-    module_id = node.getAttribute('id')
-    waf_cmd = './waf'
-    if node.hasAttribute('waf-command'):
-        waf_cmd = node.getAttribute('waf-command')
+    instance = WafModule.parse_from_xml(node, config, uri, repositories, default_repo)
 
-    # override revision tag if requested.
-    dependencies, after, suggests = get_dependencies(node)
-    branch = get_branch(node, repositories, default_repo, config)
+    if node.hasAttribute('waf-command'):
+        instance.waf_cmd = node.getAttribute('waf-command')
 
-    return WafModule(module_id, branch, dependencies=dependencies, after=after,
-            suggests=suggests, waf_cmd=waf_cmd)
+    return instance
 
 register_module_type('waf', parse_waf)