launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24999
[Merge] ~cjwatson/launchpad:tarfile-helpers-bytesio into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:tarfile-helpers-bytesio into launchpad:master.
Commit message:
Port lp.services.tarfile_helpers to BytesIO
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/387211
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:tarfile-helpers-bytesio into launchpad:master.
diff --git a/lib/lp/archivepublisher/tests/test_debian_installer.py b/lib/lp/archivepublisher/tests/test_debian_installer.py
index 8ea4184..0b106e1 100644
--- a/lib/lp/archivepublisher/tests/test_debian_installer.py
+++ b/lib/lp/archivepublisher/tests/test_debian_installer.py
@@ -177,8 +177,8 @@ class TestDebianInstaller(RunPartsMixin, TestCaseWithFactory):
"""))
os.fchmod(f.fileno(), 0o755)
self.openArchive()
- self.addFile("images/list", "a list")
- self.addFile("images/SHA256SUMS", "a checksum")
+ self.addFile("images/list", b"a list")
+ self.addFile("images/SHA256SUMS", b"a checksum")
self.process()
self.assertThat(
self.getInstallerPath("images"),
diff --git a/lib/lp/archivepublisher/tests/test_dist_upgrader.py b/lib/lp/archivepublisher/tests/test_dist_upgrader.py
index c7621ab..1da88f9 100644
--- a/lib/lp/archivepublisher/tests/test_dist_upgrader.py
+++ b/lib/lp/archivepublisher/tests/test_dist_upgrader.py
@@ -125,8 +125,8 @@ class TestDistUpgrader(RunPartsMixin, TestCaseWithFactory):
"""))
os.fchmod(f.fileno(), 0o755)
self.openArchive("20060302.0120")
- self.tarfile.add_file("20060302.0120/list", "a list")
- self.tarfile.add_file("20060302.0120/foo.tar.gz", "a tarball")
+ self.tarfile.add_file("20060302.0120/list", b"a list")
+ self.tarfile.add_file("20060302.0120/foo.tar.gz", b"a tarball")
self.process()
self.assertThat(
os.path.join(self.getUpgraderPath(), "20060302.0120"),
diff --git a/lib/lp/archivepublisher/tests/test_signing.py b/lib/lp/archivepublisher/tests/test_signing.py
index e51bb25..6c70073 100644
--- a/lib/lp/archivepublisher/tests/test_signing.py
+++ b/lib/lp/archivepublisher/tests/test_signing.py
@@ -1379,10 +1379,10 @@ class TestLocalSigningUpload(RunPartsMixin, TestSigningHelpers):
self.setUpKmodKeys()
self.setUpOpalKeys()
self.openArchive("test", "1.0", "amd64")
- self.tarfile.add_file("1.0/empty.efi", "")
- self.tarfile.add_file("1.0/empty.ko", "")
- self.tarfile.add_file("1.0/empty.opal", "")
- self.tarfile.add_file("1.0/empty.sipl", "")
+ self.tarfile.add_file("1.0/empty.efi", b"")
+ self.tarfile.add_file("1.0/empty.ko", b"")
+ self.tarfile.add_file("1.0/empty.opal", b"")
+ self.tarfile.add_file("1.0/empty.sipl", b"")
self.process_emulate()
sha256file = os.path.join(self.getSignedPath("test", "amd64"),
"1.0", "SHA256SUMS")
@@ -1811,7 +1811,8 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
# Write data on the archive
self.openArchive("test", "1.0", "amd64")
for filename in filenames:
- self.tarfile.add_file(filename, b"somedata for %s" % filename)
+ self.tarfile.add_file(
+ filename, ("somedata for %s" % filename).encode("UTF-8"))
self.assertRaises(IOError, self.process_emulate)
@@ -1827,7 +1828,8 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
self.openArchive("test", "1.0", "amd64")
for filename in filenames:
- self.tarfile.add_file(filename, b"some data for %s" % filename)
+ self.tarfile.add_file(
+ filename, ("some data for %s" % filename).encode("UTF-8"))
self.process_emulate()
@@ -1860,7 +1862,7 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
self.openArchive("test", "1.0", "amd64")
for filename in filenames:
self.tarfile.add_file(
- filename, b"data - %s" % filename.encode("UTF-8"))
+ filename, ("data - %s" % filename).encode("UTF-8"))
self.tarfile.close()
self.buffer.close()
@@ -1975,7 +1977,7 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
self.openArchive("test", "1.0", "amd64")
for filename in filenames:
self.tarfile.add_file(
- filename, b"data - %s" % filename.encode("UTF-8"))
+ filename, ("data - %s" % filename).encode("UTF-8"))
self.tarfile.close()
self.buffer.close()
@@ -2041,7 +2043,7 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
self.openArchive("test", "1.0", "amd64")
for filename in filenames:
self.tarfile.add_file(
- filename, b"data - %s" % filename.encode("UTF-8"))
+ filename, ("data - %s" % filename).encode("UTF-8"))
self.tarfile.close()
self.buffer.close()
@@ -2098,7 +2100,7 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
self.openArchive("test", "1.0", "amd64")
for filename in filenames:
self.tarfile.add_file(
- filename, b"data - %s" % filename.encode("UTF-8"))
+ filename, ("data - %s" % filename).encode("UTF-8"))
self.tarfile.close()
self.buffer.close()
diff --git a/lib/lp/services/doc/tarfile_helpers.txt b/lib/lp/services/doc/tarfile_helpers.txt
index f90effe..769dec7 100644
--- a/lib/lp/services/doc/tarfile_helpers.txt
+++ b/lib/lp/services/doc/tarfile_helpers.txt
@@ -40,12 +40,12 @@ output we will get.
We can add files individually. First argument is the file name, second
argument is the file content.
- >>> archive.add_file('foo', '1')
+ >>> archive.add_file('foo', b'1')
Or add many files simultaneously using a dictionary that use the key as
the file name and the value the file content.
- >>> archive.add_files({'bar': '2', 'baz': '3'})
+ >>> archive.add_files({'bar': b'2', 'baz': b'3'})
We can add symbolic links.
@@ -77,8 +77,8 @@ file content as the value, to a stream...
If we have several files to import...
>>> files = {
- ... 'eins': 'zwei',
- ... 'drei': 'vier'
+ ... 'eins': b'zwei',
+ ... 'drei': b'vier',
... }
...then we can easily turn it into a tarfile file object with
@@ -109,7 +109,7 @@ If a filename contains slashes, containing directories are automatically
created.
>>> archive = LaunchpadWriteTarFile.files_to_tarfile({
- ... 'uno/dos/tres/cuatro': 'blah'
+ ... 'uno/dos/tres/cuatro': b'blah',
... })
>>> examine_tarfile(archive)
uno | <directory>
@@ -120,8 +120,8 @@ created.
Also, if there is a duplicated file, last one is the one that remains there.
>>> archive = LaunchpadWriteTarFile.files_to_tarfile({
- ... 'foo.po': 'blah',
- ... 'foo.po': 'duplicated file'
+ ... 'foo.po': b'blah',
+ ... 'foo.po': b'duplicated file',
... })
>>> examine_tarfile(archive)
foo.po | duplicated file
diff --git a/lib/lp/services/tarfile_helpers.py b/lib/lp/services/tarfile_helpers.py
index 3efec10..abecc10 100644
--- a/lib/lp/services/tarfile_helpers.py
+++ b/lib/lp/services/tarfile_helpers.py
@@ -9,16 +9,16 @@ __all__ = [
'LaunchpadWriteTarFile',
]
+import io
import os
-from StringIO import StringIO
import tarfile
import tempfile
import time
-# A note about tarballs, StringIO and unicode. SQLObject returns unicode
+# A note about tarballs, BytesIO and unicode. SQLObject returns unicode
# values for columns which are declared as StringCol. We have to be careful
# not to pass unicode instances to the tarfile module, because when the
-# tarfile's filehandle is a StringIO object, the StringIO object gets upset
+# tarfile's filehandle is a BytesIO object, the BytesIO object gets upset
# later when we ask it for its value and it tries to join together its
# buffers. This is why the tarball code is sprinkled with ".encode('ascii')".
# If we get separate StringCol and UnicodeCol column types, we won't need this
@@ -107,7 +107,7 @@ class LaunchpadWriteTarFile:
tarinfo = self._make_skeleton_tarinfo(path, now)
tarinfo.mode = 0o644
tarinfo.size = len(contents)
- self.tarfile.addfile(tarinfo, StringIO(contents))
+ self.tarfile.addfile(tarinfo, io.BytesIO(contents))
def add_files(self, files):
"""Add a number of files to the archive.
diff --git a/lib/lp/services/tests/test_helpers.py b/lib/lp/services/tests/test_helpers.py
index 3a1bc62..714e764 100644
--- a/lib/lp/services/tests/test_helpers.py
+++ b/lib/lp/services/tests/test_helpers.py
@@ -26,18 +26,18 @@ def make_test_tarball_1():
return LaunchpadWriteTarFile.files_to_tarfile({
'uberfrob-0.1/README':
- 'Uberfrob is an advanced frobnicator.',
+ b'Uberfrob is an advanced frobnicator.',
'uberfrob-0.1/po/cy.po':
- '# Blah.',
+ b'# Blah.',
'uberfrob-0.1/po/es.po':
- '# Blah blah.',
+ b'# Blah blah.',
'uberfrob-0.1/po/uberfrob.pot':
- '# Yowza!',
+ b'# Yowza!',
'uberfrob-0.1/blah/po/la':
- 'la la',
+ b'la la',
'uberfrob-0.1/uberfrob.py':
- 'import sys\n'
- 'print "Frob!"\n',
+ b'import sys\n'
+ b'print "Frob!"\n',
})
@@ -64,13 +64,13 @@ def make_test_tarball_2():
# Test POT file.
msgid "foo"
msgstr ""
- """).strip()
+ """).strip().encode('UTF-8')
po = dedent("""
# Test PO file.
msgid "foo"
msgstr "bar"
- """).strip()
+ """).strip().encode('UTF-8')
return LaunchpadWriteTarFile.files_to_tarfile({
'test/test.pot': pot,
diff --git a/lib/lp/soyuz/scripts/tests/test_gina.py b/lib/lp/soyuz/scripts/tests/test_gina.py
index bf5f2b8..e36bc6a 100644
--- a/lib/lp/soyuz/scripts/tests/test_gina.py
+++ b/lib/lp/soyuz/scripts/tests/test_gina.py
@@ -248,10 +248,10 @@ class TestSourcePackageData(TestCaseWithFactory):
with open(os.path.join(
pool_dir, "foo_1.0-1.debian.tar.gz"), "wb+") as buffer:
debian_tar = LaunchpadWriteTarFile(buffer)
- debian_tar.add_file("debian/source/format", "3.0 (quilt)\n")
+ debian_tar.add_file("debian/source/format", b"3.0 (quilt)\n")
debian_tar.add_file(
- "debian/patches/ubuntu.series", "--- corrupt patch\n")
- debian_tar.add_file("debian/rules", "")
+ "debian/patches/ubuntu.series", b"--- corrupt patch\n")
+ debian_tar.add_file("debian/rules", b"")
debian_tar.close()
buffer.seek(0)
debian_tar_contents = buffer.read()
@@ -303,9 +303,10 @@ class TestSourcePackageData(TestCaseWithFactory):
with open(os.path.join(
pool_dir, "foo_1.0-1.debian.tar.gz"), "wb+") as buffer:
debian_tar = LaunchpadWriteTarFile(buffer)
- debian_tar.add_file("debian/source/format", "3.0 (quilt)\n")
- debian_tar.add_file("debian/patches/series", "--- corrupt patch\n")
- debian_tar.add_file("debian/rules", "")
+ debian_tar.add_file("debian/source/format", b"3.0 (quilt)\n")
+ debian_tar.add_file(
+ "debian/patches/series", b"--- corrupt patch\n")
+ debian_tar.add_file("debian/rules", b"")
debian_tar.close()
buffer.seek(0)
debian_tar_contents = buffer.read()
diff --git a/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py b/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py
index b593471..cef16f6 100644
--- a/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py
+++ b/lib/lp/soyuz/tests/test_packagetranslationsuploadjob.py
@@ -69,8 +69,8 @@ class LocalTestHelper(TestCaseWithFactory):
"""Create an LibraryFileAlias containing dummy translation data."""
if tar_content is None:
tar_content = {
- 'source/po/foo.pot': 'Foo template',
- 'source/po/eo.po': 'Foo translation',
+ 'source/po/foo.pot': b'Foo template',
+ 'source/po/eo.po': b'Foo translation',
}
tarfile_content = LaunchpadWriteTarFile.files_to_string(
tar_content)
@@ -128,7 +128,7 @@ class TestPackageTranslationsUploadJob(LocalTestHelper):
def test_smoke(self):
tar_content = {
- 'source/po/foobar.pot': 'FooBar template',
+ 'source/po/foobar.pot': b'FooBar template',
}
spr, sp, job = self.makeJob(tar_content=tar_content)
transaction.commit()
diff --git a/lib/lp/translations/doc/translationimportqueue.txt b/lib/lp/translations/doc/translationimportqueue.txt
index 1c950a6..89431b9 100644
--- a/lib/lp/translations/doc/translationimportqueue.txt
+++ b/lib/lp/translations/doc/translationimportqueue.txt
@@ -969,9 +969,9 @@ later with poimport script.
We get a sample tarball to be uploaded into the system.
>>> test_tar_content = {
- ... 'foo.pot': 'Foo template',
- ... 'es.po': 'Spanish translation',
- ... 'fr.po': 'French translation',
+ ... 'foo.pot': b'Foo template',
+ ... 'es.po': b'Spanish translation',
+ ... 'fr.po': b'French translation',
... }
>>> tarfile_content = LaunchpadWriteTarFile.files_to_string(
... test_tar_content)
diff --git a/lib/lp/translations/scripts/language_pack.py b/lib/lp/translations/scripts/language_pack.py
index d0bef16..b71be13 100644
--- a/lib/lp/translations/scripts/language_pack.py
+++ b/lib/lp/translations/scripts/language_pack.py
@@ -154,7 +154,8 @@ def export(distroseries, component, update, force_utf8, logger):
# started, not when it finished because that notes how old is the
# information the export contains.
archive.add_file(
- 'rosetta-%s/timestamp.txt' % distroseries.name, '%s\n' % start_date)
+ 'rosetta-%s/timestamp.txt' % distroseries.name,
+ ('%s\n' % start_date).encode('UTF-8'))
logger.info("Adding mapping file")
mapping_text = ''
@@ -162,7 +163,8 @@ def export(distroseries, component, update, force_utf8, logger):
for sourcepackagename, translationdomain in mapping:
mapping_text += "%s %s\n" % (sourcepackagename, translationdomain)
archive.add_file(
- 'rosetta-%s/mapping.txt' % distroseries.name, mapping_text)
+ 'rosetta-%s/mapping.txt' % distroseries.name,
+ mapping_text.encode('UTF-8'))
logger.info("Done.")
diff --git a/lib/lp/translations/utilities/tests/test_export_file_storage.py b/lib/lp/translations/utilities/tests/test_export_file_storage.py
index 82aef6f..182bfbe 100644
--- a/lib/lp/translations/utilities/tests/test_export_file_storage.py
+++ b/lib/lp/translations/utilities/tests/test_export_file_storage.py
@@ -30,7 +30,7 @@ class ExportFileStorageTestCase(unittest.TestCase):
"""Behaviour of isFull."""
mime = 'application/x-po'
storage = ExportFileStorage()
- storage.addFile('/tmp/a/test/file.po', 'po', 'test file', mime)
+ storage.addFile('/tmp/a/test/file.po', 'po', b'test file', mime)
# The storage object starts out with a SingleFileStorageStrategy, so
# it's full now that we've added one file.
self.assertTrue(storage._store.isFull())
@@ -38,19 +38,19 @@ class ExportFileStorageTestCase(unittest.TestCase):
# switches to a TarballFileStorageStrategy. That type of storage
# object is never full.
storage.addFile(
- '/tmp/another/test/file.po', 'po', 'test file two', mime)
+ '/tmp/another/test/file.po', 'po', b'test file two', mime)
self.assertFalse(storage._store.isFull())
# We can now add any number of files without filling the storage
# object.
storage.addFile(
- '/tmp/yet/another/test/file.po', 'po', 'test file 3', mime)
+ '/tmp/yet/another/test/file.po', 'po', b'test file 3', mime)
self.assertFalse(storage._store.isFull())
def testSingle(self):
"""Test export of single file."""
mime = 'application/x-po'
storage = ExportFileStorage()
- storage.addFile('/tmp/a/test/file.po', 'po', 'test file', mime)
+ storage.addFile('/tmp/a/test/file.po', 'po', b'test file', mime)
outfile = storage.export()
self.assertEqual(outfile.path, '/tmp/a/test/file.po')
self.assertEqual(outfile.file_extension, 'po')
@@ -60,9 +60,9 @@ class ExportFileStorageTestCase(unittest.TestCase):
"""Test export of tarball."""
mime = 'application/x-po'
storage = ExportFileStorage()
- storage.addFile('/tmp/a/test/file.po', 'po', 'test file', mime)
+ storage.addFile('/tmp/a/test/file.po', 'po', b'test file', mime)
storage.addFile(
- '/tmp/another/test.po', 'po', 'another test file', mime)
+ '/tmp/another/test.po', 'po', b'another test file', mime)
outfile = storage.export()
tarball = TarFile.open(mode='r|gz', fileobj=StringIO(outfile.read()))
elements = set(tarball.getnames())
