Rev 5729: (jameinel) Fix tar exporters to always write to binary streams. (John A in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Tue Mar 22 09:57:14 UTC 2011


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5729 [merge]
revision-id: pqm at pqm.ubuntu.com-20110322095711-9bggm9tnxnw9frow
parent: pqm at pqm.ubuntu.com-20110317175943-3p5vbo51qv78vj1w
parent: john at arbash-meinel.com-20110321090834-mki07rp0fpli7f8o
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2011-03-22 09:57:11 +0000
message:
  (jameinel) Fix tar exporters to always write to binary streams. (John A
   Meinel)
modified:
  bzrlib/export/tar_exporter.py  tar_exporter.py-20051114235828-1f6349a2f090a5d0
  bzrlib/tests/blackbox/test_export.py test_export.py-20051229024010-e6c26658e460fb1c
=== modified file 'bzrlib/export/tar_exporter.py'
--- a/bzrlib/export/tar_exporter.py	2011-03-14 14:11:06 +0000
+++ b/bzrlib/export/tar_exporter.py	2011-03-21 09:08:34 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2006, 2008, 2009, 2010 Canonical Ltd
+# Copyright (C) 2005, 2006, 2008-2011 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -108,7 +108,7 @@
         basename = None
         stream = sys.stdout
     else:
-        stream = open(dest.encode(osutils._fs_enc), 'w')
+        stream = open(dest, 'wb')
         # gzip file is used with an explicit fileobj so that
         # the basename can be stored in the gzip file rather than
         # dest. (bug 102234)
@@ -154,7 +154,7 @@
     if dest == '-':
         stream = sys.stdout
     else:
-        stream = open(dest.encode(osutils._fs_enc), 'w')
+        stream = open(dest, 'wb')
     ball = tarfile.open(None, 'w|', stream)
     export_tarball(tree, ball, root, subdir, filtered=filtered,
                    force_mtime=force_mtime)

=== modified file 'bzrlib/tests/blackbox/test_export.py'
--- a/bzrlib/tests/blackbox/test_export.py	2011-03-14 23:18:48 +0000
+++ b/bzrlib/tests/blackbox/test_export.py	2011-03-18 15:35:43 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005-2010 Canonical Ltd
+# Copyright (C) 2005-2011 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -29,6 +29,7 @@
 
 from bzrlib import (
     export,
+    osutils,
     tests,
     )
 from bzrlib.tests import TestCaseWithTransport
@@ -36,37 +37,67 @@
 
 class TestExport(TestCaseWithTransport):
 
-    def test_tar_export(self):
-        tree = self.make_branch_and_tree('tar')
-        self.build_tree(['tar/a'])
+    # On Windows, if we fail to set the binary bit, and a '\r' or '\n'
+    # ends up in the data stream, we will get corruption. Add a fair amount
+    # of random data, to help ensure there is at least one.
+    _file_content = ('!\r\n\t\n \r'
+        + 'r29trp9i1r8k0e24c2o7mcx2isrlaw7toh1hv2mtst3o1udkl36v9xn2z8kt\n'
+          'tvjn7e3i9cj1qs1rw9gcye9w72cbdueiufw8nky7bs08lwggir59d62knecp\n'
+          '7s0537r8sg3e8hdnidji49rswo47c3j8190nh8emef2b6j1mf5kdq45nt3f5\n'
+          '1sz9u7fuzrm4w8bebre7p62sh5os2nkj2iiyuk9n0w0pjpdulu9k2aajejah\n'
+          'ini90ny40qzs12ajuy0ua6l178n93lvy2atqngnntsmtlmqx7yhp0q9a1xr4\n'
+          '1n69kgbo6qu9osjpqq83446r00jijtcstzybfqwm1lnt9spnri2j07bt7bbh\n'
+          'rf3ejatdxta83te2s0pt9rc4hidgy3d2pc53p4wscdt2b1dfxdj9utf5m17f\n'
+          'f03oofcau950o090vyx6m72vfkywo7gp3ajzi6uk02dwqwtumq4r44xx6ho7\n'
+          'nhynborjdjep5j53f9548msb7gd3x9a1xveb4s8zfo6cbdw2kdngcrbakwu8\n'
+          'ql5a8l94gplkwr7oypw5nt1gj5i3xwadyjfr3lb61tfkz31ba7uda9knb294\n'
+          '1azhfta0q3ry9x36lxyanvhp0g5z0t5a0i4wnoc8p4htexi915y1cnw4nznn\n'
+          'aj70dvp88ifiblv2bsp98hz570teinj8g472ddxni9ydmazfzwtznbf3hrg6\n'
+          '84gigirjt6n2yagf70036m8d73cz0jpcighpjtxsmbgzbxx7nb4ewq6jbgnc\n'
+          'hux1b0qtsdi0zfhj6g1otf5jcldmtdvuon8y1ttszkqw3ograwi25yl921hy\n'
+          'izgscmfha9xdhxxabs07b40secpw22ah9iwpbmsns6qz0yr6fswto3ft2ez5\n'
+          'ngn48pdfxj1pw246drmj1y2ll5af5w7cz849rapzd9ih7qvalw358co0yzrs\n'
+          'xan9291d1ivjku4o5gjrsnmllrqwxwy86pcivinbmlnzasa9v3o22lgv4uyd\n'
+          'q8kw77bge3hr5rr5kzwjxk223bkmo3z9oju0954undsz8axr3kb3730otrcr\n'
+          '9cwhu37htnizdwxmpoc5qmobycfm7ubbykfumv6zgkl6b8zlslwl7a8b81vz\n'
+          '3weqkvv5csfza9xvwypr6lo0t03fwp0ihmci3m1muh0lf2u30ze0hjag691j\n'
+          '27fjtd3e3zbiin5n2hq21iuo09ukbs73r5rt7vaw6axvoilvdciir9ugjh2c\n'
+          'na2b8dr0ptftoyhyxv1iwg661y338e28fhz4xxwgv3hnoe98ydfa1oou45vj\n'
+          'ln74oac2keqt0agbylrqhfscin7ireae2bql7z2le823ksy47ud57z8ctomp\n'
+          '31s1vwbczdjwqp0o2jc7mkrurvzg8mj2zwcn2iily4gcl4sy4fsh4rignlyz\n')
+
+    def make_basic_tree(self):
+        tree = self.make_branch_and_tree('tree')
+        self.build_tree_contents([('tree/a', self._file_content)])
         tree.add('a')
-        self.build_tree_contents([('tar/.bzrrules', '')])
-        tree.add('.bzrrules')
-        self.build_tree(['tar/.bzr-adir/', 'tar/.bzr-adir/afile'])
-        tree.add(['.bzr-adir/', '.bzr-adir/afile'])
-
-        os.chdir('tar')
-        self.run_bzr('ignore something')
         tree.commit('1')
+        return tree
+
+    def make_tree_with_extra_bzr_files(self):
+        tree = self.make_basic_tree()
+        self.build_tree_contents([('tree/.bzrrules', '')])
+        self.build_tree(['tree/.bzr-adir/', 'tree/.bzr-adir/afile'])
+        tree.add(['.bzrrules', '.bzr-adir/', '.bzr-adir/afile'])
+
+        self.run_bzr('ignore something -d tree')
+        tree.commit('2')
+        return tree
+
+    def test_tar_export_ignores_bzr(self):
+        tree = self.make_tree_with_extra_bzr_files()
 
         self.failUnless(tree.has_filename('.bzrignore'))
         self.failUnless(tree.has_filename('.bzrrules'))
         self.failUnless(tree.has_filename('.bzr-adir'))
         self.failUnless(tree.has_filename('.bzr-adir/afile'))
-        self.run_bzr('export test.tar.gz')
+        self.run_bzr('export test.tar.gz -d tree')
         ball = tarfile.open('test.tar.gz')
         # Make sure the tarball contains 'a', but does not contain
         # '.bzrignore'.
-        self.assertEqual(['test/a'], sorted(ball.getnames()))
-
-        if sys.version_info < (2, 5, 2) and sys.platform == 'darwin':
-            raise tests.KnownFailure('python %r has a tar related bug, upgrade'
-                                     % (sys.version_info,))
-        out, err = self.run_bzr('export --format=tgz --root=test -')
-        ball = tarfile.open('', fileobj=StringIO(out))
-        self.assertEqual(['test/a'], sorted(ball.getnames()))
-
-    def test_tar_export_unicode(self):
+        self.assertEqual(['test/a'],
+                         sorted(ball.getnames()))
+
+    def test_tar_export_unicode_filename(self):
         self.requireFeature(tests.UnicodeFilenameFeature)
         tree = self.make_branch_and_tree('tar')
         # FIXME: using fname = u'\xe5.txt' below triggers a bug revealed since
@@ -77,8 +108,7 @@
         tree.add([fname])
         tree.commit('first')
 
-        os.chdir('tar')
-        self.run_bzr('export test.tar')
+        self.run_bzr('export test.tar -d tar')
         ball = tarfile.open('test.tar')
         # all paths are prefixed with the base name of the tarball
         self.assertEqual(['test/' + fname.encode('utf8')],
@@ -89,64 +119,79 @@
         self.requireFeature(tests.UnicodeFilenameFeature)
         basedir = u'\N{euro sign}'
         os.mkdir(basedir)
-        os.chdir(basedir)
-        self.run_bzr(['init', 'branch'])
-        os.chdir('branch')
-        self.run_bzr(['export', '--format', 'tgz', u'test.tar.gz'])
-
-    def test_zip_export(self):
-        tree = self.make_branch_and_tree('zip')
-        self.build_tree(['zip/a'])
-        tree.add('a')
-        self.build_tree_contents([('zip/.bzrrules', '')])
-        tree.add('.bzrrules')
-        self.build_tree(['zip/.bzr-adir/', 'zip/.bzr-adir/afile'])
-        tree.add(['.bzr-adir/', '.bzr-adir/afile'])
-
-        os.chdir('zip')
-        self.run_bzr('ignore something')
-        tree.commit('1')
+        self.run_bzr(['init', basedir])
+        self.run_bzr(['export', '--format', 'tgz', u'test.tar.gz',
+                      '-d', basedir])
+
+    def test_zip_export_ignores_bzr(self):
+        tree = self.make_tree_with_extra_bzr_files()
 
         self.failUnless(tree.has_filename('.bzrignore'))
         self.failUnless(tree.has_filename('.bzrrules'))
         self.failUnless(tree.has_filename('.bzr-adir'))
         self.failUnless(tree.has_filename('.bzr-adir/afile'))
-        self.run_bzr('export test.zip')
+        self.run_bzr('export test.zip -d tree')
 
         zfile = zipfile.ZipFile('test.zip')
         # Make sure the zipfile contains 'a', but does not contain
         # '.bzrignore'.
         self.assertEqual(['test/a'], sorted(zfile.namelist()))
 
+    # TODO: This really looks like something that should be using permutation
+    #       testing. Though the actual setup and teardown functions are pretty
+    #       different for each
+    def assertZipANameAndContent(self, zfile, root=''):
+        """The file should only contain name 'a' and _file_content"""
+        fname = root + 'a'
+        self.assertEqual([fname], sorted(zfile.namelist()))
+        zfile.testzip()
+        self.assertEqualDiff(self._file_content, zfile.read(fname))
+
     def test_zip_export_stdout(self):
-        tree = self.make_branch_and_tree('zip')
-        self.build_tree(['zip/a'])
-        tree.add('a')
-        tree.commit('1')
-        os.chdir('zip')
-        contents = self.run_bzr('export --format=zip -')[0]
-        zfile = zipfile.ZipFile(StringIO(contents))
-        self.assertEqual(['a'], sorted(zfile.namelist()))
-
-    def test_tgz_export_stdout(self):
-        tree = self.make_branch_and_tree('z')
-        self.build_tree(['z/a'])
-        tree.add('a')
-        tree.commit('1')
-        os.chdir('z')
-        contents = self.run_bzr('export --format=tgz -')[0]
-        ball = tarfile.open(mode='r|gz', fileobj=StringIO(contents))
-        self.assertEqual(['a'], ball.getnames())
-
-    def test_tbz2_export_stdout(self):
-        tree = self.make_branch_and_tree('z')
-        self.build_tree(['z/a'])
-        tree.add('a')
-        tree.commit('1')
-        os.chdir('z')
-        contents = self.run_bzr('export --format=tbz2 -')[0]
-        ball = tarfile.open(mode='r|bz2', fileobj=StringIO(contents))
-        self.assertEqual(['a'], ball.getnames())
+        tree = self.make_basic_tree()
+        contents = self.run_bzr('export -d tree --format=zip -')[0]
+        self.assertZipANameAndContent(zipfile.ZipFile(StringIO(contents)))
+
+    def test_zip_export_file(self):
+        tree = self.make_basic_tree()
+        self.run_bzr('export -d tree test.zip')
+        self.assertZipANameAndContent(zipfile.ZipFile('test.zip'),
+                                      root='test/')
+
+    def assertTarANameAndContent(self, ball, root=''):
+        fname = root + 'a'
+        tar_info = ball.next()
+        self.assertEqual(fname, tar_info.name)
+        self.assertEqual(tarfile.REGTYPE, tar_info.type)
+        self.assertEqual(len(self._file_content), tar_info.size)
+        f = ball.extractfile(tar_info)
+        if self._file_content != f.read():
+            self.fail('File content has been corrupted.'
+                      ' Check that all streams are handled in binary mode.')
+        # There should be no other files in the tarball
+        self.assertIs(None, ball.next())
+
+    def run_tar_export_disk_and_stdout(self, extension, tarfile_flags):
+        tree = self.make_basic_tree()
+        fname = 'test.%s' % (extension,)
+        mode = 'r|%s' % (tarfile_flags,)
+        self.run_bzr('export -d tree %s' % (fname,))
+        ball = tarfile.open(fname, mode=mode)
+        self.assertTarANameAndContent(ball, root='test/')
+        content = self.run_bzr('export -d tree --format=%s -' % (extension,))[0]
+        ball = tarfile.open(mode=mode, fileobj=StringIO(content))
+        self.assertTarANameAndContent(ball, root='')
+
+    def test_tar_export(self):
+        self.run_tar_export_disk_and_stdout('tar', '')
+
+    def test_tgz_export(self):
+        self.run_tar_export_disk_and_stdout('tgz', 'gz')
+
+    def test_tbz2_export(self):
+        self.run_tar_export_disk_and_stdout('tbz2', 'bz2')
+
+    # TODO: test_xz_export, I don't have pylzma working here to test it.
 
     def test_zip_export_unicode(self):
         self.requireFeature(tests.UnicodeFilenameFeature)




More information about the bazaar-commits mailing list