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

Proposed by Michael Vogt on 2010-12-03
Status: Merged
Approved by: Alex Eftimie on 2010-12-03
Approved revision: 63
Merged at revision: 61
Proposed branch: lp:~mvo/deb-thumbnailer/mvo
Merge into: lp:deb-thumbnailer
Diff against target: 356 lines (+152/-54)
3 files modified
Makefile (+3/-0)
deb-thumbnailer (+97/-54)
tests/test_thumbnailer.py (+52/-0)
To merge this branch: bzr merge lp:~mvo/deb-thumbnailer/mvo
Reviewer Review Type Date Requested Status
George Dumitrescu Approve on 2010-12-07
Alex Eftimie 2010-12-03 Approve on 2010-12-03
Review via email: mp+42658@code.launchpad.net

Commit message

ok

Description of the change

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

To post a comment you must log in.
Alex Eftimie (alexeftimie) wrote :

Looks fine to me.

review: Approve
lp:~mvo/deb-thumbnailer/mvo updated on 2010-12-06
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) wrote :

ok

review: Approve
Michael Vogt (mvo) wrote :

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

Cheers,
 Michael

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2010-07-01 06:04:24 +0000
3+++ Makefile 2010-12-06 14:29:01 +0000
4@@ -3,6 +3,9 @@
5 clean:
6 rm -fr build
7
8+test:
9+ (set -e; cd tests; for f in *.py; do python $$f; done)
10+
11 pack:
12 tar czvf deb-thumbnailer.tgz deb-thumbnailer install README INSTALL COPYING AUTHORS share debian make-deb Makefile
13
14
15=== modified file 'deb-thumbnailer'
16--- deb-thumbnailer 2010-08-30 18:48:25 +0000
17+++ deb-thumbnailer 2010-12-06 14:29:01 +0000
18@@ -5,55 +5,66 @@
19 __date__ ="$Jun 28, 2010 05:00:40 PM$"
20
21 from PIL import Image
22-import tempfile
23+
24 import os
25 import glob
26 import shutil
27+import string
28+import subprocess
29 import sys
30-import subprocess
31+
32+import tempfile
33 import urllib
34+
35 import gtk
36 import gconf
37 import hashlib
38 import logging
39+import xdg.BaseDirectory
40
41-GENERIC_APP=''
42-GENERIC_LIB=''
43 VERSION='0.8.2'
44 CONFIG_DIR = '/apps/deb-thumbnailer'
45 ICON_THEME = gtk.icon_theme_get_default()
46-HOME_DIR = os.getenv("HOME")
47-DT_DIR = HOME_DIR + '/.deb-thumbnailer'
48-LOG_FILENAME = '/tmp/deb-thumbnailer.log'
49 HASH_NAME = ''
50 DEB_FILE = ''
51 MD = hashlib.md5()
52-logging.basicConfig(filename=LOG_FILENAME,level=logging.DEBUG)
53-
54-def get_config():
55+
56+# special names to watch for
57+SPECIAL_ICON_NAMES = [ "opera-browser",
58+ "apps/opera.png",
59+ "product_logo_128.png",
60+ "AdobeAIR.png",
61+ "AdobeReader[0-9]*\.png"
62+ ]
63+
64+
65+#LOG_FILENAME = '/tmp/deb-thumbnailer.log'
66+#logging.basicConfig(filename=LOG_FILENAME,level=logging.DEBUG)
67+
68+def pkgname_from_deb(debfile):
69+ """ return the binary package name of the given deb """
70+ pkgname = subprocess.Popen(["dpkg-deb","-f", debfile, "Package"],
71+ stdout=subprocess.PIPE).communicate()[0].strip()
72+ return pkgname
73+
74+def get_config_icon_default():
75 """ get gconf configuration """
76 gclient = gconf.client_get_default()
77- config_value = gclient.get(CONFIG_DIR+'/icon_default')
78- try:
79- return config_value.get_bool()
80- except:
81- return False
82+ return gclient.get_bool(CONFIG_DIR+'/icon_default')
83
84-def set_config(setit):
85+def set_config_icon_default(new_value):
86 """ set gconf configuration """
87 gclient = gconf.client_get_default()
88- gvalue = gconf.Value(gconf.VALUE_BOOL)
89- gvalue.set_bool(setit)
90- gclient.set(CONFIG_DIR+'/icon_default', gvalue)
91+ gclient.set_bool(CONFIG_DIR+'/icon_default', new_value)
92
93 def pkg_overlay(background, source, destination, percent=0.5, size=48):
94 """ Merges background with source scalled by percent, aligned bottom-right
95 Saves the result into destination file
96 """
97- if not get_config():
98+ if not get_config_icon_default():
99 background, source = source, background
100 bg = Image.open(background).convert('RGBA')
101- if not get_config():
102+ if not get_config_icon_default():
103 bg = bg.resize((size, size), Image.ANTIALIAS)
104
105 if source != '':
106@@ -66,29 +77,63 @@
107
108 bg.save(destination, 'PNG')
109
110+def get_filelist_from_deb(debfile):
111+ """ return all files inside a deb as a string list """
112+ # a alternative is apt_inst.DebFile()
113+ p1 = subprocess.Popen(["dpkg", "--fsys-tarfile", debfile],
114+ stdout=subprocess.PIPE)
115+ p2 = subprocess.Popen(["tar", "tf","-"],
116+ stdin=p1.stdout,
117+ stdout=subprocess.PIPE)
118+ return map(string.strip, p2.stdout.readlines())
119+
120+def extract_file_from_deb(debfile, filename, targetdir="."):
121+ """ extract the given filename into targetdir """
122+ # a alternative is apt_inst.DebFile() (again)
123+ p1 = subprocess.Popen(["dpkg", "--fsys-tarfile", debfile],
124+ stdout=subprocess.PIPE)
125+ p2 = subprocess.Popen(["tar", "xf","-", filename],
126+ stdin=p1.stdout,
127+ stdout=subprocess.PIPE,
128+ cwd=targetdir)
129+ return p2.wait()
130+
131+def get_icons_from_deb(debfile, tmpdir="."):
132+ logging.debug("get_icons_from_deb() %s in %s" % (debfile, tmpdir))
133+ icons = []
134+ debfile = os.path.abspath(debfile)
135+ if not os.path.exists(debfile):
136+ raise IOError("file '%s' not found" % debfile)
137+ pname = pkgname_from_deb(debfile)
138+ for line in get_filelist_from_deb(debfile):
139+ # FIXME: also test for SPECIAL_ICON_NAMES
140+ if (line.startswith("./usr/share/icons") or
141+ line.startswith("./usr/share/pixmaps")):
142+ icons.append(line)
143+ icons.sort()
144+ logging.debug("found icons: '%s'" % icons)
145+ res = extract_file_from_deb(debfile, icons[-1], tmpdir)
146+ return icons
147+
148 def process_deb(debfile, size=48):
149+ """ Unpacks a .deb file and returns the icon path """
150 global HASH_NAME
151 global DEB_FILE
152- """ Unpacks a .deb file and returns the icon path """
153+
154 curdir = os.getcwd()
155 tmpdir = tempfile.mkdtemp('-deb-thumbnailer')
156 icons = []
157 svg = False
158 DEB_FILE = debfile
159
160- # Get package name safely
161- try:
162- pname = os.path.split(debfile)[1]
163- debfileName = pname
164- pname = pname.split('_')[0]
165- except:
166- logging.debug("Failed to obtain %s filename" % debfile)
167- pname = debfile
168- debfileName = pname
169+ # Get package name safely (read from debian/control)
170+ debfileName = os.path.basename(debfile)
171+ pname = pkgname_from_deb(debfile)
172
173+ # hash it
174 MD.update(debfileName)
175 HASH_NAME = MD.hexdigest()
176- if not get_config():
177+ if not get_config_icon_default():
178 HASH_NAME = HASH_NAME + '1'
179 # asking system for app icon (faster)
180 if pname == 'deb-thumbnailer':
181@@ -103,25 +148,20 @@
182 if not len(icons) == 0:
183 iconFile = os.path.split(icons[1])[1]
184 iconExt = os.path.splitext(iconFile)[1]
185- os.system("cp %s %s" % (icons[1], tmpdir + '/' + iconFile))
186+ shutil.copy(icons[1], os.path.join(tmpdir, iconFile))
187 icons[1] = iconFile
188 if iconExt == '.svg':
189 svg = True
190
191 # searching in package
192 if len(icons) == 0:
193- os.chdir(tmpdir)
194- if pname == "opera-browser":
195- 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")
196- else:
197- 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")
198- icons.sort()
199- os.system('tar -xf data.tar.* "%s"' % icons[-1])
200+ icons = get_icons_from_deb(debfile, tmpdir)
201 # Search for SVG icons
202 iconFile = os.path.split(icons[-1].replace("./", ""))[1]
203 iconExt = os.path.splitext(iconFile)[1]
204 if iconExt == ".svg":
205 svg = True
206+
207 # Go back
208 os.chdir(curdir)
209 if len(icons) > 1:
210@@ -134,7 +174,7 @@
211 shutil.copyfile(tmpdir + '/' + icon.replace("./", ""), tmpfile)
212 # convert SVG
213 if svg == True:
214- os.system("rsvg-convert %s -o %s.png" % (tmpfile, tmpfile))
215+ subprocess.call(["rsvg-convert", tmpfile, "-o", "%s.png" % tmpfile])
216 if os.path.exists(tmpfile):
217 os.unlink(tmpfile)
218 tmpfile = '%s.png' % tmpfile
219@@ -152,10 +192,11 @@
220 pkgIconExt = os.path.splitext(pkgIcon)[1]
221 # TODO: ask theme for png directly | INFO: not all themes provide png
222 if pkgIconExt == '.svg':
223- if os.path.exists('/tmp/x-deb-%d.png' % size):
224- os.unlink('/tmp/x-deb-%d.png' % size)
225- os.system("rsvg-convert %s -w %d -h %d -o /tmp/x-deb-%d.png" % (pkgIcon, size, size, size))
226- return "/tmp/x-deb-%d.png" % size
227+ target = tempfile.NamedTemporaryFile(prefix="x-deb-%d" % size)
228+ if os.path.exists(target):
229+ os.unlink(target)
230+ subprocess.call(["rsvg-convert", pkgIcon, "-w", size, "-h", size, "-o", target])
231+ return target
232 else:
233 return pkgIcon
234
235@@ -164,12 +205,12 @@
236 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]
237 config_change = False
238 if switch == "Package icon in background (default)":
239- if get_config() != True:
240- set_config(True)
241+ if get_config_icon_default() != True:
242+ set_config_icon_default(True)
243 config_change = True
244 elif switch == "Application icon in background":
245- if get_config() != False:
246- set_config(False)
247+ if get_config_icon_default() != False:
248+ set_config_icon_default(False)
249 config_change = True
250 if config_change:
251 print "Refreshing thumbnails cache ..."
252@@ -182,7 +223,7 @@
253 reset_icons()
254 thumbdir = HOME_DIR + '/.thumbnails'
255 if os.path.isdir(thumbdir):
256- os.system("rm -rf %s" % thumbdir)
257+ shutil.rmtree(thumbdir)
258
259 def reset_icons():
260 """ reseting icons to .deb only in home folder """
261@@ -192,13 +233,14 @@
262 if os.path.exists(debs[i]):
263 info = subprocess.Popen("gvfs-info -a metadata::custom-icon \"%s\"" % debs[i], shell=True, stdout=subprocess.PIPE).stdout.read().split("\n")[1]
264 if info != '':
265- os.system('gvfs-set-attribute "%s" -t unset metadata::custom-icon' % debs[i])
266+ subprocess.call(["gvfs-set-attribute", debs[i], "-t", "unset", "metadata::custom-icon"])
267
268 if __name__ == "__main__":
269- global DEB_NAME
270 proceed = False
271+ DT_DIR = os.path.join(xdg.BaseDirectory.xdg_cache_home, "deb-thumbnailer")
272 if not os.path.exists(DT_DIR):
273 os.makedirs(DT_DIR)
274+ # ???
275 if len(sys.argv) == 4:
276 try:
277 size = int(sys.argv[3])
278@@ -219,7 +261,8 @@
279 shutil.copy(output, DT_DIR + '/' + HASH_NAME + '.png')
280 dt_path = DT_DIR + '/' + HASH_NAME + '.png'
281 if os.path.exists(dt_path):
282- os.system("gvfs-set-attribute \"%s\" metadata::custom-icon file://%s" % (DEB_FILE, dt_path))
283+ subprocess.call(["gvfs-set-attribute", DEB_FILE, "metadata::custom-icon", "file://%s" % dt_path])
284+ # options?
285 elif len(sys.argv) == 2:
286 if sys.argv[1] == '--switch':
287 icon_switch()
288@@ -240,7 +283,7 @@
289 proceed = True
290 else:
291 logging.debug("File %s is not an .deb file" % os.path.split(input)[1])
292- os.system('zenity --error --title="Deb-Thumbnailer: Error" --text="File \'%s\' is not an .deb file"' % os.path.split(input)[1])
293+ subprocess.call(['zenity', '--error', '--title=Deb-Thumbnailer: Error', '--text=File \'%s\' is not an .deb file' % os.path.split(input)[1]])
294
295 if proceed:
296 if not os.path.exists(input):
297
298=== added symlink 'deb_thumbnailer.py'
299=== target is u'deb-thumbnailer'
300=== added directory 'tests'
301=== added file 'tests/test_thumbnailer.py'
302--- tests/test_thumbnailer.py 1970-01-01 00:00:00 +0000
303+++ tests/test_thumbnailer.py 2010-12-06 14:29:01 +0000
304@@ -0,0 +1,52 @@
305+#!/usr/bin/python
306+
307+
308+import sys
309+sys.path.insert(0,"../")
310+
311+import glob
312+import os
313+import hashlib
314+import subprocess
315+import unittest
316+import urllib
317+
318+from deb_thumbnailer import (process_deb, get_icons_from_deb, pkgname_from_deb,
319+ get_filelist_from_deb)
320+
321+class TestDebThumbnailer(unittest.TestCase):
322+ """ tests the xapian database """
323+
324+ def setUp(self):
325+ gcalctool_url = "http://archive.ubuntu.com/ubuntu/pool/main/g/gcalctool/gcalctool_5.30.0.is.5.28.2-0ubuntu2_i386.deb"
326+ self.gcalctool = os.path.basename(gcalctool_url)
327+ if not os.path.exists(self.gcalctool):
328+ (filename, res) = urllib.urlretrieve(
329+ gcalctool_url, filename=self.gcalctool)
330+ m = hashlib.md5()
331+ m.update(open(self.gcalctool).read())
332+ self.assertEqual(m.hexdigest(), "5914fe02d32f9153eb2994dd6c80c8f6")
333+
334+ def test_process_deb(self):
335+ # this url is valid until 2015
336+ icon = process_deb(self.gcalctool)
337+ # ensure we get a icon
338+ self.assertTrue(os.path.exists(icon))
339+ self.assertTrue(os.path.getsize(icon) > 0)
340+
341+ def test_get_icons_from_deb(self):
342+ import tempfile
343+ tmpdir = tempfile.mkdtemp()
344+ get_icons_from_deb(self.gcalctool, tmpdir)
345+
346+ def test_pkgname_from_deb(self):
347+ self.assertEqual(pkgname_from_deb(self.gcalctool), "gcalctool")
348+
349+ def test_filelist_from_deb(self):
350+ flist = get_filelist_from_deb(self.gcalctool)
351+ self.assertTrue("./usr/share/omf/gcalctool/gcalctool-bg.omf" in flist)
352+
353+if __name__ == "__main__":
354+ import logging
355+ logging.basicConfig(level=logging.DEBUG)
356+ unittest.main()

Subscribers

People subscribed via source and target branches