Re: dogtail-devel PROPOSAL: new options in config module for customized environments.



Alexander Todorov wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hi all,
I'm proposing two new options in the config module and I have already written
some of the code. Both options will help script development (or dogtail
integration) in customized environments.

Hi Alexander,

1) checkForA11y (boolean), default True. If False don't use GConf to check if
a11y is enabled or not.

This looks fine.

2) distroClass (class Distro), default None. If set don't perform the
distribution check and use this class.

I've just committed a patch (attached) to only import the distro module when necessary, i.e. when using i18n.loadTranslationsFromPackageMoFiles() or i18n.getMoFilesForPackage(). If you use those functions, distro will be imported. If you don't, it won't. Will that work?

+
1) -- get rid of gconf modules and related in customized environment. Such an
environment at present is Anaconda (Fedora installer) which needed ~2MB of GConf
libs only because of that. It also needed a customized XML file with the
necessary settings.

2) Pretty much the same as 1). If you know what you are doing then no need to
automatically detect the system. Further more this detection is based on
/etc/$DISTRO-release which is not very robust will fail anytime this file is not
present.

The only time dogtail currently needs to know which distro it's running on is when it needs to locate translation files. So with the patch I've just committed that's the only time it checks. If Anaconda needs to load translation files, we'll have to figure out how to do that.

As for the detection mechanism not being robust, I respectfully disagree. If a distribution is "odd" enough that we don't detect it currently, then we need to add support for it. Any distribution really should have some way for applications identify it, and we can use what it provides.

- -
1) Ugly code. Looks like:

def isA11yEnabled():
    if config.checkForA11y:
        import gconf
        gconfClient = gconf.client_get_default()

        gconfEnabled = gconfClient.get_bool(a11yGConfKey)
        if os.environ.get('GTK_MODULES','').find('gail:atk-bridge') == -1:
            envEnabled = False
        else: envEnabled = True
        return (gconfEnabled or envEnabled)
    else:
        return True

and then pretty much the same for enableA11y()
I can't figure out how to "nicely" import the gconf module if needed.

I think I would rather not even execute those functions if config.checkForA11y is False, but I haven't looked into it yet.

2) The detection code looks now like that:
if (config.distroClass):
    logger.log(message + config.distroClass.__name__ + " (defined in
configuration)")
    distro = config.distroClass()
elif os.environ.get("CERTIFIED_GNOMIE", "no") == "yes":
    logger.log(message + "JHBuild environment")
    distro = JHBuild()
....
That's in distro.py and gets executed every time this module is imported.
As far as I can see only packageDb is used and it's used only in i18n.py and
examples probably.
Can anyone advise how to change this to something more human friendly ?

Let me know what you think of the changes I've just made, and if it will work for Anaconda. If more work needs to be done, we'll figure out the best way to do it.

Zack
Index: dogtail/logging.py
===================================================================
--- dogtail/logging.py	(revision 363)
+++ dogtail/logging.py	(working copy)
@@ -109,9 +109,6 @@
             raise IOError, \
                     "Log path %s does not exist or is not a directory" % logDir
 
-        #self.createFile()
-
-
     def findUniqueName(self):
         # generate a logfile name and check if it already exists
         self.fileName = config.logDir + self.stamper.fileStamp(self.fileName) \
@@ -129,40 +126,32 @@
 
     def createFile(self):
         # Try to create the file and write the header info
-        try:
-            print "Creating logfile at %s ..." % self.fileName
-            self.file = codecs.open(self.fileName, mode = 'wb', encoding = \
-                    'utf-8')
-            self.file.write("##### " + os.path.basename(self.fileName) + '\n')
-            self.file.flush()
-            return True
-        except IOError:
-            print "Could not create and write to " + self.fileName
-            self.file = False
-            return False
+        print "Creating logfile at %s ..." % self.fileName
+        self.file = codecs.open(self.fileName, mode = 'wb', encoding = \
+                'utf-8')
+        self.file.write("##### " + os.path.basename(self.fileName) + '\n')
+        self.file.flush()
 
-    def log(self, message):
+    def log(self, message, newline = True):
         """
         Hook used for logging messages. Might eventually be a virtual
         function, but nice and simple for now.
         """
         message = message.decode('utf-8', 'replace')
 
-        # Also write to standard out.
-        if self.stdOut and config.logDebugToStdOut: print message
-
         # Try to open and write the result to the log file.
-        if isinstance(self.file, bool): 
-            if not config.logDebugToFile: return
-            elif not self.createFile(): return
-        try:
-            #self.file = open(self.fileName, 'a')
-            self.file.write(message + '\n')
+        if isinstance(self.file, bool) and config.logDebugToFile:
+            self.createFile()
+
+        if config.logDebugToFile:
+            if newline: self.file.write(message + '\n')
+            else: self.file.write(message + ' ')
             self.file.flush()
-            #self.file.close()
-        except IOError:
-            print "Could not write to file " + self.fileName
 
+        if self.stdOut and config.logDebugToStdOut: 
+            if newline: print message
+            else: print message,
+
 class ResultsLogger(Logger):
     """
     Writes entries into the Dogtail log
Index: dogtail/distro.py
===================================================================
--- dogtail/distro.py	(revision 382)
+++ dogtail/distro.py	(working copy)
@@ -34,7 +34,8 @@
 
 class PackageDb:
     """
-    Class to abstract the details of whatever software package database is in use (RPM, APT, etc)
+    Class to abstract the details of whatever software package database is in
+    use (RPM, APT, etc)
     """
     def __init__(self):
         self.prefix = '/usr'
@@ -42,20 +43,24 @@
 
     def getVersion(self, packageName):
         """
-        Method to get the version of an installed package as a Version instance (or raise an exception if not found)
+        Method to get the version of an installed package as a Version 
+        instance (or raise an exception if not found)
+        
         Note: does not know about distributions' internal revision numbers.
         """
         raise NotImplementedError
 
     def getFiles(self, packageName):
         """
-        Method to get a list of filenames owned by the package, or raise an exception if not found.
+        Method to get a list of filenames owned by the package, or raise an 
+        exception if not found.
         """
         raise NotImplementedError
 
     def getMoFiles(self, locale = None):
         """
-        Method to get a list of all .mo files on the system, optionally for a specific locale.
+        Method to get a list of all .mo files on the system, optionally for a
+        specific locale.
         """
         moFiles = {}
 
@@ -181,14 +186,17 @@
         PackageDb.__init__(self)
 
     def getVersion(self, packageName):
-        # the portage utilities are almost always going to be in /usr/lib/portage/pym
+        # the portage utilities are almost always going to be in 
+        # /usr/lib/portage/pym
         import sys
         sys.path.append ('/usr/lib/portage/pym')
         import portage
-        # FIXME: this takes the first package returned in the list, in the case that there are
-        # slotted packages, and removes the leading category such as 'sys-apps'
+        # FIXME: this takes the first package returned in the list, in the 
+        # case that there are slotted packages, and removes the leading 
+        # category such as 'sys-apps'
         gentooPackageName = portage.db["/"]["vartree"].dbapi.match(packageName)[0].split('/')[1];
-        # this removes the distribution specific versioning returning only the upstream version
+        # this removes the distribution specific versioning returning only the
+        # upstream version
         upstreamVersion = portage.pkgsplit(gentooPackageName)[1]
         #print "Version of package is: " + upstreamVersion
         return Version.fromString(upstreamVersion);
@@ -209,7 +217,6 @@
 # getVersion not implemented because on Solaris multiple modules are installed
 # in single packages, so it is hard to tell what version number of a specific
 # module.
-#
 class _SolarisPackageDb(PackageDb):
     def __init__(self):
         PackageDb.__init__(self)
@@ -236,101 +243,80 @@
     """
     Class representing a distribution.
 
-    Scripts may want to do arbitrary logic based on whichever distro is in use (e.g. handling differences in names of packages, distribution-specific patches, etc.)
+    Scripts may want to do arbitrary logic based on whichever distro is in use
+    (e.g. handling differences in names of packages, distribution-specific 
+    patches, etc.)
 
-    We can either create methods in the Distro class to handle these, or we can use constructs like isinstance(distro, Ubuntu) to handle this. We can even create hierarchies of distro subclasses to handle this kind of thing (could get messy fast though)
+    We can either create methods in the Distro class to handle these, or we 
+    can use constructs like isinstance(distro, Ubuntu) to handle this. We can
+    even create hierarchies of distro subclasses to handle this kind of thing
+    (could get messy fast though)
     """
 
-class RedHatOrFedora(Distro):
-    """
-    Class representing one of Red Hat Linux, Fedora, Red Hat Enterprise Linux, or one of the rebuild-style derivatives
-    """
+class Fedora(Distro):
     def __init__(self):
         self.packageDb = _RpmPackageDb()
 
+class RHEL(Fedora):
+    pass
+
 class Debian(Distro):
-    """
-    Class representing one of the Debian or Debian-derived distributions
-    """
     def __init__(self):
         self.packageDb = _AptPackageDb()
 
 class Ubuntu(Debian):
-    """
-    Class representing one of the Debian or Debian-derived distributions
-    """
     def __init__(self):
         self.packageDb = _UbuntuAptPackageDb()
 
 class Suse(Distro):
-    """
-    Class representing one of the SuSE or SuSE-derived distributions
-    """
     def __init__(self):
         self.packageDb = _RpmPackageDb()
 
 class Gentoo(Distro):
-    """
-    Class representing one of the Gentoo or Gentoo-derived distributions
-    """
     def __init__(self):
         self.packageDb = _PortagePackageDb()
 
 class Conary(Distro):
-    """
-    Class representing a Conary-based distribution
-    """
     def __init__(self):
         self.packageDb = _ConaryPackageDb()
 
 class Solaris(Distro):
-    """
-    Class representing a Solaris distribution
-    """
     def __init__(self):
         self.packageDb = _SolarisPackageDb()
 
 class JHBuild(Distro):
-    """
-    Class representing a JHBuild environment
-    """
     def __init__(self):
         self.packageDb = JhBuildPackageDb()
 
-message = "Detecting distribution: "
-if os.environ.get("CERTIFIED_GNOMIE", "no") == "yes":
-    logger.log(message + "JHBuild environment")
-    distro = JHBuild()
-elif os.path.exists ("/etc/SuSE-release"):
-    logger.log(message + "SuSE (or derived distribution)")
-    distro = Suse()
-elif os.path.exists ("/etc/fedora-release"):
-    logger.log(message + "Fedora (or derived distribution)")
-    distro = RedHatOrFedora()
-elif os.path.exists ("/etc/redhat-release"):
-    logger.log(message + "Red Hat/Fedora/derived distribution")
-    distro = RedHatOrFedora()
-elif os.path.exists ("/usr/share/doc/ubuntu-minimal"):
-    logger.log(message + "Ubuntu (or derived distribution)")
-    distro = Ubuntu()
-elif os.path.exists ("/etc/debian_version"):
-    logger.log(message + "Debian (or derived distribution)")
-    distro = Debian()
-elif os.path.exists ("/etc/gentoo-release"):
-    logger.log(message + "Gentoo (or derived distribution)")
-    distro = Gentoo()
-elif os.path.exists ("/etc/slackware-version"):
-    logger.log(message + "Slackware")
-    raise DistributionNotSupportedError("Slackware")
-elif os.path.exists ("/var/lib/conarydb/conarydb"):
-    logger.log(message + "Conary-based distribution")
-    distro = Conary()
-elif os.path.exists ("/etc/release") and \
-        re.match (".*Solaris", open ("/etc/release").readline()):
-    logger.log(message + "Solaris distribution")
-    distro = Solaris()
-else:
-    logger.log(message + "Unknown")
-    raise DistributionNotSupportedError("Unknown")
+def detectDistro():
+    logger.log("Detecting distribution:", newline = False)
 
+    if os.environ.get("CERTIFIED_GNOMIE", "no") == "yes":
+        distro = JHBuild()
+    elif os.path.exists("/etc/SuSE-release"):
+        distro = Suse()
+    elif os.path.exists("/etc/fedora-release"):
+        distro = Fedora()
+    elif os.path.exists("/etc/redhat-release"):
+        distro = RHEL()
+    elif os.path.exists("/usr/share/doc/ubuntu-minimal"):
+        distro = Ubuntu()
+    elif os.path.exists("/etc/debian_version"):
+        distro = Debian()
+    elif os.path.exists("/etc/gentoo-release"):
+        distro = Gentoo()
+    elif os.path.exists("/etc/slackware-version"):
+        raise DistributionNotSupportedError("Slackware")
+    elif os.path.exists("/var/lib/conarydb/conarydb"):
+        distro = Conary()
+    elif os.path.exists ("/etc/release") and \
+            re.match(".*Solaris", open ("/etc/release").readline()):
+        distro = Solaris()
+    else:
+        raise DistributionNotSupportedError("Unknown")
+    logger.log(distro.__class__.__name__)
+    return distro
+
+distro = detectDistro()
 packageDb = distro.packageDb
+
Index: dogtail/i18n.py
===================================================================
--- dogtail/i18n.py	(revision 340)
+++ dogtail/i18n.py	(working copy)
@@ -7,7 +7,6 @@
 
 __author__ = """David Malcolm <dmalcolm redhat com>, Zack Cerza <zcerza redhat com>"""
 
-import distro
 import config
 
 import os
@@ -212,6 +211,8 @@
     dependencies. It is possible to restrict the results to those of a certain
     language, for example 'ja'.
     """
+    import distro
+
     result = []
     for filename in distro.packageDb.getFiles(packageName):
         if isMoFile(filename, language):
@@ -255,6 +256,7 @@
     # this special case, aside from the simple fact that there is one,
     # is that it makes automatic translations much slower.
 
+    import distro
     language = os.environ.get('LANGUAGE', os.environ['LANG'])[0:2]
     if isinstance(distro.distro, distro.Ubuntu):
         load('language-pack-gnome-%s' % language, language)
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 384)
+++ ChangeLog	(working copy)
@@ -1,3 +1,17 @@
+2008-02-19  Zack Cerza <zcerza redhat com>
+
+	* dogtail/distro.py: Make distribution detection code a little less
+	ugly. Remove docstrings from the Distro subclasses. Clean up some other 
+	documentation.
+
+	* dogtail/i18n.py: Don't import the distro module until we actually
+	need it.
+
+	* dogtail/logging.py: Add an optional 'newline' arg to Logger.log(),
+	which defaults to True. Passing False will cause the logger to not
+	print or write a newline at the end of the message. Also stop catching
+	some IOErrors, as that made the code awkward for no good reason.
+
 2008-02-06  Zack Cerza <zcerza redhat com>
 
 	* dogtail/dump.py: Properly print Node.actions; ditch "xml" output as


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]