Merge lp:~mvo/deb-thumbnailer/mvo into lp:deb-thumbnailer

[Michael Vogt]

ok

Hi,

I like this project a lot, the idea is just great. While looking at the code I noticed
that there are some bits and piece as could be cleaned up. Like the use of os.system()
can be dangerous because its directly feed into a shell. The subprocess.call() is a
better option. If you are ok with my changes I will look a bit more and see what else
will benefit from cleanup so that we can tweak it so that its ready for inclusion
into the archive.

Thanks,
 Michael

[Alex Eftimie (alexeftimie)]

Looks fine to me.

review: Approve

[lp:~mvo/deb-thumbnailer/mvo]

64. By Michael Vogt on 2010-12-06

add simple test/ dir

65. By Michael Vogt on 2010-12-06

more tests, seperate out get_icons_from_deb()

[George Dumitrescu (geod)]

ok

review: Approve

[Michael Vogt (mvo)]

Thanks a lot for the merge! I'm a bit short on time currently unfortunately, but I plan to add more test :)

Cheers,
 Michael

=== modified file 'Makefile'
--- Makefile	2010-07-01 06:04:24 +0000
+++ Makefile	2010-12-06 14:29:01 +0000
@@ -3,6 +3,9 @@
 clean:
 	rm -fr build
 
+test:
+	(set -e; cd tests; for f in *.py; do python $$f; done)
+
 pack:
 	tar czvf deb-thumbnailer.tgz deb-thumbnailer install README INSTALL COPYING AUTHORS share debian make-deb Makefile
 
 
=== modified file 'deb-thumbnailer'
--- deb-thumbnailer	2010-08-30 18:48:25 +0000
+++ deb-thumbnailer	2010-12-06 14:29:01 +0000
@@ -5,55 +5,66 @@
 __date__ ="$Jun 28, 2010 05:00:40 PM$"
 
 from PIL import Image
-import tempfile
+
 import os
 import glob
 import shutil
+import string
+import subprocess
 import sys
-import subprocess
+
+import tempfile
 import urllib
+
 import gtk
 import gconf
 import hashlib
 import logging
+import xdg.BaseDirectory
 
-GENERIC_APP=''
-GENERIC_LIB=''
 VERSION='0.8.2'
 CONFIG_DIR = '/apps/deb-thumbnailer'
 ICON_THEME = gtk.icon_theme_get_default()
-HOME_DIR = os.getenv("HOME")
-DT_DIR = HOME_DIR + '/.deb-thumbnailer'
-LOG_FILENAME = '/tmp/deb-thumbnailer.log'
 HASH_NAME = ''
 DEB_FILE = ''
 MD = hashlib.md5()
-logging.basicConfig(filename=LOG_FILENAME,level=logging.DEBUG)
-
-def get_config():
+
+# special names to watch for 
+SPECIAL_ICON_NAMES = [ "opera-browser", 
+                       "apps/opera.png",
+                       "product_logo_128.png",
+                       "AdobeAIR.png",
+                       "AdobeReader[0-9]*\.png"
+                     ]
+
+
+#LOG_FILENAME = '/tmp/deb-thumbnailer.log'
+#logging.basicConfig(filename=LOG_FILENAME,level=logging.DEBUG)
+
+def pkgname_from_deb(debfile):
+    """ return the binary package name of the given deb """
+    pkgname = subprocess.Popen(["dpkg-deb","-f", debfile, "Package"],
+                               stdout=subprocess.PIPE).communicate()[0].strip()
+    return pkgname
+
+def get_config_icon_default():
     """ get gconf configuration """
     gclient = gconf.client_get_default()
-    config_value = gclient.get(CONFIG_DIR+'/icon_default')
-    try:
-        return config_value.get_bool()
-    except:
-        return False
+    return gclient.get_bool(CONFIG_DIR+'/icon_default')
     
-def set_config(setit):
+def set_config_icon_default(new_value):
     """ set gconf configuration """
     gclient = gconf.client_get_default()
-    gvalue = gconf.Value(gconf.VALUE_BOOL)
-    gvalue.set_bool(setit)
-    gclient.set(CONFIG_DIR+'/icon_default', gvalue)
+    gclient.set_bool(CONFIG_DIR+'/icon_default', new_value)
 
 def pkg_overlay(background, source, destination, percent=0.5, size=48):
     """ Merges background with source scalled by percent, aligned bottom-right 
     Saves the result into destination file
     """
-    if not get_config():
+    if not get_config_icon_default():
         background, source = source, background
     bg = Image.open(background).convert('RGBA')
-    if not get_config():
+    if not get_config_icon_default():
         bg = bg.resize((size, size), Image.ANTIALIAS)
 
     if source != '':
@@ -66,29 +77,63 @@
         
     bg.save(destination, 'PNG')
 
+def get_filelist_from_deb(debfile):
+    """ return all files inside a deb as a string list """
+    # a alternative is apt_inst.DebFile()
+    p1 = subprocess.Popen(["dpkg", "--fsys-tarfile", debfile],
+                          stdout=subprocess.PIPE)
+    p2 = subprocess.Popen(["tar", "tf","-"], 
+                          stdin=p1.stdout,
+                          stdout=subprocess.PIPE)
+    return map(string.strip, p2.stdout.readlines())
+
+def extract_file_from_deb(debfile, filename, targetdir="."):
+    """ extract the given filename into targetdir """
+    # a alternative is apt_inst.DebFile() (again)
+    p1 = subprocess.Popen(["dpkg", "--fsys-tarfile", debfile],
+                          stdout=subprocess.PIPE)
+    p2 = subprocess.Popen(["tar", "xf","-", filename],
+                          stdin=p1.stdout,
+                          stdout=subprocess.PIPE,
+                          cwd=targetdir)
+    return p2.wait()
+
+def get_icons_from_deb(debfile, tmpdir="."):
+    logging.debug("get_icons_from_deb() %s in %s" % (debfile, tmpdir))
+    icons = []
+    debfile = os.path.abspath(debfile)
+    if not os.path.exists(debfile):
+        raise IOError("file '%s' not found" % debfile)
+    pname = pkgname_from_deb(debfile)
+    for line in get_filelist_from_deb(debfile):
+        # FIXME: also test for SPECIAL_ICON_NAMES
+        if (line.startswith("./usr/share/icons") or 
+            line.startswith("./usr/share/pixmaps")):
+            icons.append(line)
+    icons.sort()
+    logging.debug("found icons: '%s'" % icons)
+    res = extract_file_from_deb(debfile, icons[-1], tmpdir)
+    return icons
+
 def process_deb(debfile, size=48):
+    """ Unpacks a .deb file and returns the icon path """
     global HASH_NAME
     global DEB_FILE
-    """ Unpacks a .deb file and returns the icon path """
+    
     curdir = os.getcwd()
     tmpdir = tempfile.mkdtemp('-deb-thumbnailer')
     icons = []
     svg = False
     DEB_FILE = debfile
     
-    # Get package name safely
-    try:
-        pname = os.path.split(debfile)[1]
-        debfileName = pname
-        pname = pname.split('_')[0]
-    except:
-        logging.debug("Failed to obtain %s filename" % debfile)
-        pname = debfile
-        debfileName = pname
+    # Get package name safely (read from debian/control)
+    debfileName = os.path.basename(debfile)
+    pname = pkgname_from_deb(debfile)
         
+    # hash it
     MD.update(debfileName)
     HASH_NAME = MD.hexdigest()
-    if not get_config():
+    if not get_config_icon_default():
         HASH_NAME = HASH_NAME + '1'
     # asking system for app icon (faster)
     if pname == 'deb-thumbnailer':
@@ -103,25 +148,20 @@
     if not len(icons) == 0:
         iconFile = os.path.split(icons[1])[1]
         iconExt = os.path.splitext(iconFile)[1]
-        os.system("cp %s %s" % (icons[1], tmpdir + '/' + iconFile))
+        shutil.copy(icons[1], os.path.join(tmpdir, iconFile))
         icons[1] = iconFile
         if iconExt == '.svg':
             svg = True
     
     # searching in package
     if len(icons) == 0:
-        os.chdir(tmpdir)
-        if pname == "opera-browser":
-            icons = subprocess.Popen('ar -x "%s"; tar -tf data.tar.* | egrep "\.png$|\.svg$|\.xpm$" | egrep "(opera\-browser)|apps/opera.png$"' % debfile, shell=True, stdout=subprocess.PIPE).stdout.read().split("\n")
-        else:
-            icons = subprocess.Popen('ar -x "%s"; tar -tf data.tar.* | egrep "\.png$|\.svg$|\.xpm$" | egrep "(usr/share/icons|usr/share/pixmaps|product_logo_128\.png|[0-9]/AdobeReader[0-9]\.png|[0-9]/AdobeAIR.png)"' % debfile, shell=True, stdout=subprocess.PIPE).stdout.read().split("\n")
-        icons.sort()
-        os.system('tar -xf data.tar.* "%s"' % icons[-1])
+        icons = get_icons_from_deb(debfile, tmpdir)
         # Search for SVG icons
         iconFile = os.path.split(icons[-1].replace("./", ""))[1]
         iconExt = os.path.splitext(iconFile)[1]
         if iconExt == ".svg":
             svg = True
+
     # Go back 
     os.chdir(curdir)
     if len(icons) > 1:
@@ -134,7 +174,7 @@
                     shutil.copyfile(tmpdir + '/' + icon.replace("./", ""), tmpfile)
         # convert SVG
         if svg == True:
-            os.system("rsvg-convert %s -o %s.png" % (tmpfile, tmpfile))
+            subprocess.call(["rsvg-convert", tmpfile, "-o", "%s.png" %  tmpfile])
             if os.path.exists(tmpfile):
                 os.unlink(tmpfile)
             tmpfile = '%s.png' % tmpfile
@@ -152,10 +192,11 @@
     pkgIconExt = os.path.splitext(pkgIcon)[1]
     # TODO: ask theme for png directly | INFO: not all themes provide png
     if pkgIconExt == '.svg':
-        if os.path.exists('/tmp/x-deb-%d.png' % size):
-            os.unlink('/tmp/x-deb-%d.png' % size)
-        os.system("rsvg-convert %s -w %d -h %d -o /tmp/x-deb-%d.png" % (pkgIcon, size, size, size))
-        return "/tmp/x-deb-%d.png" % size
+        target = tempfile.NamedTemporaryFile(prefix="x-deb-%d" % size)
+        if os.path.exists(target):
+            os.unlink(target)
+        subprocess.call(["rsvg-convert", pkgIcon, "-w", size, "-h", size, "-o", target])
+        return target
     else:
         return pkgIcon
 
@@ -164,12 +205,12 @@
     switch = subprocess.Popen('zenity --list --radiolist --column="" --column="type" TRUE "Package icon in background (default)" FALSE "Application icon in background" --title="Icon type"', shell=True, stdout=subprocess.PIPE).stdout.read().split('\n')[0]
     config_change = False
     if switch == "Package icon in background (default)":
-        if get_config() != True:
-            set_config(True)
+        if get_config_icon_default() != True:
+            set_config_icon_default(True)
             config_change = True
     elif switch == "Application icon in background":
-        if get_config() != False:
-            set_config(False)
+        if get_config_icon_default() != False:
+            set_config_icon_default(False)
             config_change = True
     if config_change:
         print "Refreshing thumbnails cache ..."
@@ -182,7 +223,7 @@
         reset_icons()
         thumbdir = HOME_DIR + '/.thumbnails'
         if os.path.isdir(thumbdir):
-            os.system("rm -rf %s" % thumbdir)
+            shutil.rmtree(thumbdir)
 
 def reset_icons():
     """ reseting icons to .deb only in home folder """
@@ -192,13 +233,14 @@
         if os.path.exists(debs[i]):
             info = subprocess.Popen("gvfs-info -a metadata::custom-icon \"%s\"" % debs[i], shell=True, stdout=subprocess.PIPE).stdout.read().split("\n")[1]
             if info != '':
-                os.system('gvfs-set-attribute "%s" -t unset metadata::custom-icon' % debs[i])
+                subprocess.call(["gvfs-set-attribute", debs[i], "-t", "unset", "metadata::custom-icon"])
 
 if __name__ == "__main__":
-    global DEB_NAME
     proceed = False
+    DT_DIR = os.path.join(xdg.BaseDirectory.xdg_cache_home, "deb-thumbnailer")
     if not os.path.exists(DT_DIR):
         os.makedirs(DT_DIR)
+    # ???
     if len(sys.argv) == 4:
         try:
             size = int(sys.argv[3])
@@ -219,7 +261,8 @@
         shutil.copy(output, DT_DIR + '/' + HASH_NAME + '.png')
         dt_path = DT_DIR + '/' + HASH_NAME + '.png'
         if os.path.exists(dt_path):
-            os.system("gvfs-set-attribute \"%s\" metadata::custom-icon file://%s" % (DEB_FILE, dt_path))
+            subprocess.call(["gvfs-set-attribute", DEB_FILE, "metadata::custom-icon", "file://%s" % dt_path])
+    # options?
     elif len(sys.argv) == 2:
         if sys.argv[1] == '--switch':
             icon_switch()
@@ -240,7 +283,7 @@
                         proceed = True
             else:
                 logging.debug("File %s is not an .deb file" % os.path.split(input)[1])
-                os.system('zenity --error --title="Deb-Thumbnailer: Error" --text="File \'%s\' is not an .deb file"' % os.path.split(input)[1])
+                subprocess.call(['zenity', '--error', '--title=Deb-Thumbnailer: Error', '--text=File \'%s\' is not an .deb file' % os.path.split(input)[1]])
     
     if proceed:
         if not os.path.exists(input):
 
=== added symlink 'deb_thumbnailer.py'
=== target is u'deb-thumbnailer'
=== added directory 'tests'
=== added file 'tests/test_thumbnailer.py'
--- tests/test_thumbnailer.py	1970-01-01 00:00:00 +0000
+++ tests/test_thumbnailer.py	2010-12-06 14:29:01 +0000
@@ -0,0 +1,52 @@
+#!/usr/bin/python
+
+
+import sys
+sys.path.insert(0,"../")
+
+import glob
+import os
+import hashlib
+import subprocess
+import unittest
+import urllib
+
+from deb_thumbnailer import (process_deb, get_icons_from_deb, pkgname_from_deb,
+                             get_filelist_from_deb)
+
+class TestDebThumbnailer(unittest.TestCase):
+    """ tests the xapian database """
+
+    def setUp(self):
+        gcalctool_url = "http://archive.ubuntu.com/ubuntu/pool/main/g/gcalctool/gcalctool_5.30.0.is.5.28.2-0ubuntu2_i386.deb"
+        self.gcalctool = os.path.basename(gcalctool_url)
+        if not os.path.exists(self.gcalctool):
+            (filename, res) = urllib.urlretrieve(
+                gcalctool_url, filename=self.gcalctool)
+            m = hashlib.md5()
+            m.update(open(self.gcalctool).read())
+            self.assertEqual(m.hexdigest(), "5914fe02d32f9153eb2994dd6c80c8f6")
+
+    def test_process_deb(self):
+        # this url is valid until 2015
+        icon = process_deb(self.gcalctool)
+        # ensure we get a icon
+        self.assertTrue(os.path.exists(icon))
+        self.assertTrue(os.path.getsize(icon) > 0)
+
+    def test_get_icons_from_deb(self):
+        import tempfile
+        tmpdir = tempfile.mkdtemp()
+        get_icons_from_deb(self.gcalctool, tmpdir)
+
+    def test_pkgname_from_deb(self):
+        self.assertEqual(pkgname_from_deb(self.gcalctool), "gcalctool")
+
+    def test_filelist_from_deb(self):
+        flist = get_filelist_from_deb(self.gcalctool)
+        self.assertTrue("./usr/share/omf/gcalctool/gcalctool-bg.omf" in flist)
+
+if __name__ == "__main__":
+    import logging
+    logging.basicConfig(level=logging.DEBUG)
+    unittest.main()