Merge lp:~james-w/linaro-image-tools/tarfile-improvements into lp:linaro-image-tools/11.11

Proposed by James Westby
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: 68
Merged at revision: 54
Proposed branch: lp:~james-w/linaro-image-tools/tarfile-improvements
Merge into: lp:linaro-image-tools/11.11
Prerequisite: lp:~james-w/linaro-image-tools/config-docs
Diff against target: 294 lines (+274/-0)
3 files modified
hwpack/better_tarfile.py (+97/-0)
hwpack/tests/__init__.py (+1/-0)
hwpack/tests/test_better_tarfile.py (+176/-0)
To merge this branch: bzr merge lp:~james-w/linaro-image-tools/tarfile-improvements
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle (community) Approve
Review via email: mp+34144@code.launchpad.net

Description of the change

Hi,

This adds a subclass of the stdlib tarfile.TarFile that aids
in creating tarfiles without touching the filesystem.

I want this for two reasons, the first being for testing my next
branch, which adds some matchers for tarfile contents. I want to
test those without hitting the filesystem.

The second reason is that if possible I want to create the hwpack
tarball without putting all the files on the filesystem first.

Thanks,

James

To post a comment you must log in.
66. By James Westby

Merged config-docs into tarfile-improvements.

67. By James Westby

Factor out some common code in the tests.

68. By James Westby

Use dict.pop instead of the custom implementation. Thanks Michael.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

hwpack/better_tarfile.py could use a module docstring.

It's not clear to me why standard_tarfile or better_tarfile have default arguments: they're both only ever called with the non-defaulted arguments supplied (relatedly, I wonder what the combination of mode and fileobj to TarFile.__init__ means.....)

The standard_tarfile context manager is only used in tests, so maybe it should be defined in the test module.

I don't like the way that the tests "know" that create_simple_tarball creates a file called foo with content bar. I think it would be better to use a pattern like bzrlib's build_tree where you say create_simple_tarball([('foo', 'bar')]) or something.

All else looks ok.

review: Approve
Revision history for this message
James Westby (james-w) wrote :

On Tue, 31 Aug 2010 01:13:40 -0000, Michael Hudson <email address hidden> wrote:
> Review: Approve
> hwpack/better_tarfile.py could use a module docstring.

Done, thanks.

> It's not clear to me why standard_tarfile or better_tarfile have
> default arguments: they're both only ever called with the
> non-defaulted arguments supplied (relatedly, I wonder what the
> combination of mode and fileobj to TarFile.__init__ means.....)

Later branches supply different mode arguments, so I probably should
have made that change there.

mode can also be used to specify the compression that will be used, so
while a mode of "w" on a read-only file object makes no sense, it is
needed to have both.

> The standard_tarfile context manager is only used in tests, so maybe
> it should be defined in the test module.

It's used in two test modules later, which is why I moved it here, but
I'll move it to a testing module (if it stays given your comments on the
next proposal.)

> I don't like the way that the tests "know" that create_simple_tarball
> creates a file called foo with content bar. I think it would be
> better to use a pattern like bzrlib's build_tree where you say
> create_simple_tarball([('foo', 'bar')]) or something.

I think that's a good idea. I had them all explicitly creating the
paths, but given that they all created identical paths I just factored
it out.

The other option would be fixture, where the path would be an attribute,
and so wouldn't be hardcoded like that.

> All else looks ok.

Thanks, tweaks are on their way.

James

69. By James Westby

Add a module docstring to better_tarfile. Thanks Michael.

70. By James Westby

Less magic in the tests. Thanks Michael.

Revision history for this message
James Westby (james-w) wrote :

On Tue, 31 Aug 2010 10:48:06 -0400, James Westby <email address hidden> wrote:
> On Tue, 31 Aug 2010 01:13:40 -0000, Michael Hudson <email address hidden> wrote:
> > The standard_tarfile context manager is only used in tests, so maybe
> > it should be defined in the test module.
>
> It's used in two test modules later, which is why I moved it here, but
> I'll move it to a testing module (if it stays given your comments on the
> next proposal.)

Moved.

> > I don't like the way that the tests "know" that create_simple_tarball
> > creates a file called foo with content bar. I think it would be
> > better to use a pattern like bzrlib's build_tree where you say
> > create_simple_tarball([('foo', 'bar')]) or something.
>
> I think that's a good idea. I had them all explicitly creating the
> paths, but given that they all created identical paths I just factored
> it out.

Made the change as suggested. I don't like the duplication with the
helper I add in the next branch, but I'm not sure I like indirecting
through a helper in order to test the functionality.

Thanks,

James

71. By James Westby

Merged config-docs into tarfile-improvements.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'hwpack/better_tarfile.py'
2--- hwpack/better_tarfile.py 1970-01-01 00:00:00 +0000
3+++ hwpack/better_tarfile.py 2010-09-01 20:48:41 +0000
4@@ -0,0 +1,97 @@
5+from contextlib import contextmanager
6+from StringIO import StringIO
7+from tarfile import DIRTYPE, TarFile as StandardTarFile, TarInfo
8+
9+"""Improvements to the standard library's tarfile module.
10+
11+In particular this module provides a tarfile.TarFile subclass that aids
12+in adding paths to the tarfile that aren't present on the filesystem,
13+with the ability to specify file content as strings, and provide
14+default values for the mtime, uid, etc. of the created paths.
15+"""
16+
17+@contextmanager
18+def writeable_tarfile(backing_file, mode="w", **kwargs):
19+ """A context manager to get a writeable better tarfile.
20+
21+ :param backing_file: a file object to write the tarfile contents
22+ to.
23+ :param mode: the mode to open the tarfile with. Default is
24+ "w".
25+ :param kwargs: other keyword arguments to pass to the TarFile
26+ constructor.
27+ """
28+ tf = TarFile.open(mode=mode, fileobj=backing_file, **kwargs)
29+ try:
30+ yield tf
31+ finally:
32+ tf.close()
33+
34+
35+class TarFile(StandardTarFile):
36+ """An improvement to tarfile that can add paths not on the filesystem.
37+
38+ With the standard tarfile implementation adding paths that are not
39+ present on the filesystem is convoluted. This subclass adds methods
40+ to create paths in the tarfile that are not present on the filesystem.
41+
42+ In addition, it can take constructor parameters to set the defaults
43+ of various attributes of the paths that it adds.
44+ """
45+
46+ def __init__(self, *args, **kwargs):
47+ """Create a TarFile.
48+
49+ :param default_mtime: the default mtime to create paths with,
50+ an int or None to use the stdlib default.
51+ :param default_uid: the default user id to set as the owner of
52+ created paths, an int or None to use the stdlib default.
53+ :param default_gid: the default group id to set as the owner of
54+ created paths, an int or None to use the stdlib default.
55+ :param default_uname: the default user name to set as the owner
56+ of created paths, a string, or None to use the stdlib default.
57+ :param default_gname: the default group name ot set as the owner
58+ of created paths, a string, or None to use the stdlib default.
59+ """
60+ self.default_mtime = kwargs.pop("default_mtime", None)
61+ self.default_uid = kwargs.pop("default_uid", None)
62+ self.default_gid = kwargs.pop("default_gid", None)
63+ self.default_uname = kwargs.pop("default_uname", None)
64+ self.default_gname = kwargs.pop("default_gname", None)
65+ super(TarFile, self).__init__(*args, **kwargs)
66+
67+ def _set_defaults(self, tarinfo):
68+ if self.default_mtime is not None:
69+ tarinfo.mtime = self.default_mtime
70+ if self.default_uid is not None:
71+ tarinfo.uid = self.default_uid
72+ if self.default_gid is not None:
73+ tarinfo.gid = self.default_gid
74+ if self.default_uname is not None:
75+ tarinfo.uname = self.default_uname
76+ if self.default_gname is not None:
77+ tarinfo.gname = self.default_gname
78+
79+ def create_file_from_string(self, filename, content):
80+ """Create a file with the contents passed as a string.
81+
82+ :param filename: the path to put the file at inside the
83+ tarfile.
84+ :param content: the content to put in the created file.
85+ """
86+ tarinfo = TarInfo(name=filename)
87+ tarinfo.size = len(content)
88+ self._set_defaults(tarinfo)
89+ fileobj = StringIO(content)
90+ self.addfile(tarinfo, fileobj=fileobj)
91+
92+ def create_dir(self, path):
93+ """Create a directory within the tarfile.
94+
95+ :param path: the path to put the directory at.
96+ """
97+ tarinfo = TarInfo(name=path)
98+ tarinfo.type = DIRTYPE
99+ tarinfo.mode = 0755
100+ self._set_defaults(tarinfo)
101+ self.addfile(tarinfo)
102
103=== modified file 'hwpack/tests/__init__.py'
104--- hwpack/tests/__init__.py 2010-08-27 19:35:40 +0000
105+++ hwpack/tests/__init__.py 2010-09-01 20:48:41 +0000
106@@ -2,6 +2,7 @@
107
108 def test_suite():
109 module_names = ['hwpack.tests.test_config',
110+ 'hwpack.tests.test_better_tarfile',
111 ]
112 loader = unittest.TestLoader()
113 suite = loader.loadTestsFromNames(module_names)
114
115=== added file 'hwpack/tests/test_better_tarfile.py'
116--- hwpack/tests/test_better_tarfile.py 1970-01-01 00:00:00 +0000
117+++ hwpack/tests/test_better_tarfile.py 2010-09-01 20:48:41 +0000
118@@ -0,0 +1,176 @@
119+from contextlib import contextmanager
120+from StringIO import StringIO
121+import tarfile
122+
123+from testtools import TestCase
124+
125+from hwpack.better_tarfile import writeable_tarfile
126+
127+
128+@contextmanager
129+def standard_tarfile(backing_file, mode="r", seek=True):
130+ """A context manager to open a stdlib tarfile.
131+
132+ :param backing_file: the file object to take the tarfile
133+ contents from.
134+ :param mode: the mode to open the tarfile with.
135+ :param seek: whether to seek the backing file to 0 before
136+ opening.
137+ """
138+ if seek:
139+ backing_file.seek(0)
140+ tf = tarfile.TarFile.open(mode=mode, fileobj=backing_file)
141+ try:
142+ yield tf
143+ finally:
144+ tf.close()
145+
146+
147+class TarFileTests(TestCase):
148+
149+ def test_creates_empty_tarfile(self):
150+ backing_file = StringIO()
151+ with writeable_tarfile(backing_file):
152+ pass
153+ with standard_tarfile(backing_file, seek=False) as tf:
154+ self.assertEqual([], tf.getnames())
155+
156+ def create_simple_tarball(self, contents, **kwargs):
157+ backing_file = StringIO()
158+ with writeable_tarfile(backing_file, **kwargs) as tf:
159+ for path, content in contents:
160+ if path[-1] == '/':
161+ tf.create_dir(path)
162+ else:
163+ tf.create_file_from_string(path, content)
164+ return backing_file
165+
166+ def test_create_file_from_string_adds_path(self):
167+ backing_file = self.create_simple_tarball([("foo", "bar")])
168+ with standard_tarfile(backing_file) as tf:
169+ self.assertEqual(["foo"], tf.getnames())
170+
171+ def test_create_file_from_string_uses_content(self):
172+ backing_file = self.create_simple_tarball([("foo", "bar")])
173+ with standard_tarfile(backing_file) as tf:
174+ self.assertEqual("bar", tf.extractfile("foo").read())
175+
176+ def test_create_file_from_string_sets_size(self):
177+ backing_file = self.create_simple_tarball([("foo", "bar")])
178+ with standard_tarfile(backing_file) as tf:
179+ self.assertEqual(3, tf.getmember("foo").size)
180+
181+ def test_create_file_from_string_sets_mode(self):
182+ backing_file = self.create_simple_tarball([("foo", "bar")])
183+ with standard_tarfile(backing_file) as tf:
184+ self.assertEqual(0644, tf.getmember("foo").mode)
185+
186+ def test_create_file_from_string_sets_type(self):
187+ backing_file = self.create_simple_tarball([("foo", "bar")])
188+ with standard_tarfile(backing_file) as tf:
189+ self.assertEqual(tarfile.REGTYPE, tf.getmember("foo").type)
190+
191+ def test_create_file_from_string_sets_linkname(self):
192+ backing_file = self.create_simple_tarball([("foo", "bar")])
193+ with standard_tarfile(backing_file) as tf:
194+ self.assertEqual('', tf.getmember("foo").linkname)
195+
196+ def test_create_file_uses_default_mtime(self):
197+ now = 126793
198+ backing_file = self.create_simple_tarball(
199+ [("foo", "bar")], default_mtime=now)
200+ with standard_tarfile(backing_file) as tf:
201+ self.assertEqual(now, tf.getmember("foo").mtime)
202+
203+ def test_create_file_uses_default_uid(self):
204+ uid = 1259
205+ backing_file = self.create_simple_tarball(
206+ [("foo", "bar")], default_uid=uid)
207+ with standard_tarfile(backing_file) as tf:
208+ self.assertEqual(uid, tf.getmember("foo").uid)
209+
210+ def test_create_file_uses_default_gid(self):
211+ gid = 2259
212+ backing_file = self.create_simple_tarball(
213+ [("foo", "bar")], default_gid=gid)
214+ with standard_tarfile(backing_file) as tf:
215+ self.assertEqual(gid, tf.getmember("foo").gid)
216+
217+ def test_create_file_uses_default_uname(self):
218+ uname = "someperson"
219+ backing_file = self.create_simple_tarball(
220+ [("foo", "bar")], default_uname=uname)
221+ with standard_tarfile(backing_file) as tf:
222+ self.assertEqual(uname, tf.getmember("foo").uname)
223+
224+ def test_create_file_uses_default_gname(self):
225+ gname = "somegroup"
226+ backing_file = self.create_simple_tarball(
227+ [("foo", "bar")], default_gname=gname)
228+ with standard_tarfile(backing_file) as tf:
229+ self.assertEqual(gname, tf.getmember("foo").gname)
230+
231+ def test_create_dir_adds_path(self):
232+ backing_file = self.create_simple_tarball([("foo/", "")])
233+ with standard_tarfile(backing_file) as tf:
234+ self.assertEqual(["foo"], tf.getnames())
235+
236+ def test_create_dir_sets_name(self):
237+ backing_file = self.create_simple_tarball([("foo/", "")])
238+ with standard_tarfile(backing_file) as tf:
239+ self.assertEqual("foo", tf.getmember("foo").name)
240+
241+ def test_create_dir_sets_type(self):
242+ backing_file = self.create_simple_tarball([("foo/", "")])
243+ with standard_tarfile(backing_file) as tf:
244+ self.assertEqual(tarfile.DIRTYPE, tf.getmember("foo").type)
245+
246+ def test_create_dir_sets_size(self):
247+ backing_file = self.create_simple_tarball([("foo/", "")])
248+ with standard_tarfile(backing_file) as tf:
249+ self.assertEqual(0, tf.getmember("foo").size)
250+
251+ def test_create_dir_sets_mode(self):
252+ backing_file = self.create_simple_tarball([("foo/", "")])
253+ with standard_tarfile(backing_file) as tf:
254+ self.assertEqual(0755, tf.getmember("foo").mode)
255+
256+ def test_create_dir_sets_linkname(self):
257+ backing_file = self.create_simple_tarball([("foo/", "")])
258+ with standard_tarfile(backing_file) as tf:
259+ self.assertEqual('', tf.getmember("foo").linkname)
260+
261+ def test_create_dir_uses_default_mtime(self):
262+ now = 126793
263+ backing_file = self.create_simple_tarball(
264+ [("foo/", "")], default_mtime=now)
265+ with standard_tarfile(backing_file) as tf:
266+ self.assertEqual(now, tf.getmember("foo").mtime)
267+
268+ def test_create_dir_uses_default_uid(self):
269+ uid = 1259
270+ backing_file = self.create_simple_tarball(
271+ [("foo/", "")], default_uid=uid)
272+ with standard_tarfile(backing_file) as tf:
273+ self.assertEqual(uid, tf.getmember("foo").uid)
274+
275+ def test_create_dir_uses_default_gid(self):
276+ gid = 2259
277+ backing_file = self.create_simple_tarball(
278+ [("foo/", "")], default_gid=gid)
279+ with standard_tarfile(backing_file) as tf:
280+ self.assertEqual(gid, tf.getmember("foo").gid)
281+
282+ def test_create_dir_uses_default_uname(self):
283+ uname = "someperson"
284+ backing_file = self.create_simple_tarball(
285+ [("foo/", "")], default_uname=uname)
286+ with standard_tarfile(backing_file) as tf:
287+ self.assertEqual(uname, tf.getmember("foo").uname)
288+
289+ def test_create_dir_uses_default_gname(self):
290+ gname = "somegroup"
291+ backing_file = self.create_simple_tarball(
292+ [("foo/", "")], default_gname=gname)
293+ with standard_tarfile(backing_file) as tf:
294+ self.assertEqual(gname, tf.getmember("foo").gname)

Subscribers

People subscribed via source and target branches