Bug 1547730 - Finish up some final py3-izing in getting mozjar.py to work in py3 r=#build
☠☠ backed out by ee4b88439111 ☠ ☠
authorJustin Wood <Callek@gmail.com>
Thu, 18 Apr 2019 15:21:31 -0400
changeset 536088 3df19a96f8ac74b2c4100d40dc91b18efcb59749
parent 536087 27ff9602d80a13414d81422eaa02d01d60475a4e
child 536089 d282ddabcb3d214f603704468f8e172d09bc7c2d
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1547730
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1547730 - Finish up some final py3-izing in getting mozjar.py to work in py3 r=#build Differential Revision: https://phabricator.services.mozilla.com/D28120
python/mozbuild/mozpack/copier.py
python/mozbuild/mozpack/files.py
python/mozbuild/mozpack/mozjar.py
python/mozbuild/mozpack/test/test_mozjar.py
--- a/python/mozbuild/mozpack/copier.py
+++ b/python/mozbuild/mozpack/copier.py
@@ -291,17 +291,17 @@ class FileCopier(FileRegistry):
         destination directory (at least those that are not required to
         be regular directories): pass
         remove_all_directory_symlinks=False. Exercise caution with
         this flag: you almost certainly do not want to preserve
         directory symlinks.
 
         Returns a FileCopyResult that details what changed.
         '''
-        assert isinstance(destination, basestring)
+        assert isinstance(destination, six.string_types)
         assert not os.path.exists(destination) or os.path.isdir(destination)
 
         result = FileCopyResult()
         have_symlinks = hasattr(os, 'symlink')
         destination = os.path.normpath(destination)
 
         # We create the destination directory specially. We can't do this as
         # part of the loop doing mkdir() below because that loop munges
@@ -560,17 +560,17 @@ class Jarrer(FileRegistry, BaseFile):
                     from mozpack.mozjar import Deflater
                     self.deflater = Deflater(self.compress)
                     self.mode = 'w'
                 self.deflater.write(data)
 
             def exists(self):
                 return self.deflater is not None
 
-        if isinstance(dest, basestring):
+        if isinstance(dest, six.string_types):
             dest = Dest(dest)
         assert isinstance(dest, Dest)
 
         from mozpack.mozjar import JarWriter, JarReader, JAR_BROTLI
         try:
             old_jar = JarReader(fileobj=dest)
         except Exception:
             old_jar = []
--- a/python/mozbuild/mozpack/files.py
+++ b/python/mozbuild/mozpack/files.py
@@ -172,17 +172,17 @@ class BaseFile(object):
         '''
         Copy the BaseFile content to the destination given as a string or a
         Dest instance. Avoids replacing existing files if the BaseFile content
         matches that of the destination, or in case of plain files, if the
         destination is newer than the original file. This latter behaviour is
         disabled when skip_if_older is False.
         Returns whether a copy was actually performed (True) or not (False).
         '''
-        if isinstance(dest, basestring):
+        if isinstance(dest, six.string_types):
             dest = Dest(dest)
         else:
             assert isinstance(dest, Dest)
 
         can_skip_content_check = False
         if not dest.exists():
             can_skip_content_check = True
         elif getattr(self, 'path', None) and getattr(dest, 'path', None):
@@ -292,21 +292,21 @@ class ExecutableFile(File):
     '''
 
     def __init__(self, path, xz_compress=False):
         File.__init__(self, path)
         self.xz_compress = xz_compress
 
     def copy(self, dest, skip_if_older=True):
         real_dest = dest
-        if not isinstance(dest, basestring):
+        if not isinstance(dest, six.string_types):
             fd, dest = mkstemp()
             os.close(fd)
             os.remove(dest)
-        assert isinstance(dest, basestring)
+        assert isinstance(dest, six.string_types)
         # If File.copy didn't actually copy because dest is newer, check the
         # file sizes. If dest is smaller, it means it is already stripped and
         # elfhacked and xz_compressed, so we can skip.
         if not File.copy(self, dest, skip_if_older) and \
                 os.path.getsize(self.path) > os.path.getsize(dest):
             return False
         try:
             if may_strip(dest):
@@ -335,17 +335,17 @@ class AbsoluteSymlinkFile(File):
 
     def __init__(self, path):
         if not os.path.isabs(path):
             raise ValueError('Symlink target not absolute: %s' % path)
 
         File.__init__(self, path)
 
     def copy(self, dest, skip_if_older=True):
-        assert isinstance(dest, basestring)
+        assert isinstance(dest, six.string_types)
 
         # The logic in this function is complicated by the fact that symlinks
         # aren't universally supported. So, where symlinks aren't supported, we
         # fall back to file copying. Keep in mind that symlink support is
         # per-filesystem, not per-OS.
 
         # Handle the simple case where symlinks are definitely not supported by
         # falling back to file copy.
@@ -426,17 +426,17 @@ class HardlinkFile(File):
 
     This is similar to the AbsoluteSymlinkFile, but with hard links. The symlink
     implementation requires paths to be absolute, because they are resolved at
     read time, which makes relative paths messy. Hard links resolve paths at
     link-creation time, so relative paths are fine.
     '''
 
     def copy(self, dest, skip_if_older=True):
-        assert isinstance(dest, basestring)
+        assert isinstance(dest, six.string_types)
 
         if not hasattr(os, 'link'):
             return super(HardlinkFile, self).copy(
                 dest, skip_if_older=skip_if_older
             )
 
         try:
             path_st = os.stat(self.path)
@@ -488,17 +488,17 @@ class ExistingFile(BaseFile):
     existing file is required, it must exist during copy() or an error is
     raised.
     '''
 
     def __init__(self, required):
         self.required = required
 
     def copy(self, dest, skip_if_older=True):
-        if isinstance(dest, basestring):
+        if isinstance(dest, six.string_types):
             dest = Dest(dest)
         else:
             assert isinstance(dest, Dest)
 
         if not self.required:
             return
 
         if not dest.exists():
@@ -535,17 +535,17 @@ class PreprocessedFile(BaseFile):
 
         # This always yields at least self.path.
         return pp.includes
 
     def copy(self, dest, skip_if_older=True):
         '''
         Invokes the preprocessor to create the destination file.
         '''
-        if isinstance(dest, basestring):
+        if isinstance(dest, six.string_types):
             dest = Dest(dest)
         else:
             assert isinstance(dest, Dest)
 
         # We have to account for the case where the destination exists and is a
         # symlink to something. Since we know the preprocessor is certainly not
         # going to create a symlink, we can just remove the existing one. If the
         # destination is not a symlink, we leave it alone, since we're going to
--- a/python/mozbuild/mozpack/mozjar.py
+++ b/python/mozbuild/mozpack/mozjar.py
@@ -62,17 +62,17 @@ class JarStruct(object):
         obj['version']
         obj['filename']
     filename_size would be obtained with len(obj['filename']).
 
     JarStruct subclasses instances can be either initialized from existing data
     (deserialized), or with empty fields.
     '''
 
-    TYPE_MAPPING = {'uint32': ('I', 4), 'uint16': ('H', 2)}
+    TYPE_MAPPING = {'uint32': (b'I', 4), 'uint16': (b'H', 2)}
 
     def __init__(self, data=None):
         '''
         Create an instance from the given data. Data may be omitted to create
         an instance with empty fields.
         '''
         assert self.MAGIC and isinstance(self.STRUCT, OrderedDict)
         self.size_fields = set(t for t in six.itervalues(self.STRUCT)
@@ -622,17 +622,17 @@ class JarWriter(object):
             compress = JAR_DEFLATED
         if compress is False:
             compress = JAR_STORED
         if (isinstance(data, (JarFileReader, Deflater)) and
                 data.compress == compress):
             deflater = data
         else:
             deflater = Deflater(compress, compress_level=self._compress_level)
-            if isinstance(data, basestring):
+            if isinstance(data, (six.binary_type, six.string_types)):
                 deflater.write(data)
             elif hasattr(data, 'read'):
                 if hasattr(data, 'seek'):
                     data.seek(0)
                 deflater.write(data.read())
             else:
                 raise JarWriterError("Don't know how to handle %s" %
                                      type(data))
--- a/python/mozbuild/mozpack/test/test_mozjar.py
+++ b/python/mozbuild/mozpack/test/test_mozjar.py
@@ -70,18 +70,18 @@ class TestJarStruct(unittest.TestCase):
         self.assertRaises(JarReaderError, TestJarStruct.Foo, data[2:])
 
         foo = TestJarStruct.Foo(data[1:])
         self.assertEqual(foo['foo'], 0x45444342)
         self.assertEqual(foo['bar'], 0xcdab)
         self.assertEqual(foo['qux'], 0x01ef)
         self.assertFalse('length' in foo)
         self.assertFalse('length2' in foo)
-        self.assertEqual(foo['string'], '012345')
-        self.assertEqual(foo['string2'], '67')
+        self.assertEqual(foo['string'], b'012345')
+        self.assertEqual(foo['string2'], b'67')
 
     def test_read_jar_struct(self):
         data = b'\x00\x04\x03\x02\x01\x42\x43\x44\x45\xab\xcd\xef' + \
                b'\x01\x06\x00\x02\x0001234567890'
         self.do_test_read_jar_struct(data)
 
     def test_read_jar_struct_memoryview(self):
         data = b'\x00\x04\x03\x02\x01\x42\x43\x44\x45\xab\xcd\xef' + \
@@ -125,17 +125,17 @@ class TestDeflater(unittest.TestCase):
         deflater.write(self.wrap(b'aaaaaaaaaaaaanopqrstuvwxyz'))
         self.assertEqual(deflater.crc32, 0xd46b97ed)
 
     def test_deflater_empty(self):
         deflater = Deflater(False)
         self.assertFalse(deflater.compressed)
         self.assertEqual(deflater.uncompressed_size, 0)
         self.assertEqual(deflater.compressed_size, deflater.uncompressed_size)
-        self.assertEqual(deflater.compressed_data, '')
+        self.assertEqual(deflater.compressed_data, b'')
         self.assertEqual(deflater.crc32, 0)
 
 
 class TestDeflaterMemoryView(TestDeflater):
     def wrap(self, data):
         return memoryview(data)
 
 
@@ -146,124 +146,124 @@ class TestJar(unittest.TestCase):
             jar.add('foo', b'foo')
             self.assertRaises(JarWriterError, jar.add, 'foo', b'bar')
             jar.add('bar', b'aaaaaaaaaaaaanopqrstuvwxyz')
             jar.add('baz/qux', b'aaaaaaaaaaaaanopqrstuvwxyz', False)
             jar.add('baz\\backslash', b'aaaaaaaaaaaaaaa')
 
         files = [j for j in JarReader(fileobj=s)]
 
-        self.assertEqual(files[0].filename, 'foo')
+        self.assertEqual(files[0].filename, b'foo')
         self.assertFalse(files[0].compressed)
         self.assertEqual(files[0].read(), b'foo')
 
-        self.assertEqual(files[1].filename, 'bar')
+        self.assertEqual(files[1].filename, b'bar')
         self.assertTrue(files[1].compressed)
         self.assertEqual(files[1].read(), b'aaaaaaaaaaaaanopqrstuvwxyz')
 
-        self.assertEqual(files[2].filename, 'baz/qux')
+        self.assertEqual(files[2].filename, b'baz/qux')
         self.assertFalse(files[2].compressed)
         self.assertEqual(files[2].read(), b'aaaaaaaaaaaaanopqrstuvwxyz')
 
         if os.sep == '\\':
-            self.assertEqual(files[3].filename, 'baz/backslash',
+            self.assertEqual(files[3].filename, b'baz/backslash',
                              'backslashes in filenames on Windows should get normalized')
         else:
-            self.assertEqual(files[3].filename, 'baz\\backslash',
+            self.assertEqual(files[3].filename, b'baz\\backslash',
                              'backslashes in filenames on POSIX platform are untouched')
 
         s = MockDest()
         with JarWriter(fileobj=s, compress=False) as jar:
             jar.add('bar', b'aaaaaaaaaaaaanopqrstuvwxyz')
             jar.add('foo', b'foo')
             jar.add('baz/qux', b'aaaaaaaaaaaaanopqrstuvwxyz', True)
 
         jar = JarReader(fileobj=s)
         files = [j for j in jar]
 
-        self.assertEqual(files[0].filename, 'bar')
+        self.assertEqual(files[0].filename, b'bar')
         self.assertFalse(files[0].compressed)
         self.assertEqual(files[0].read(), b'aaaaaaaaaaaaanopqrstuvwxyz')
 
-        self.assertEqual(files[1].filename, 'foo')
+        self.assertEqual(files[1].filename, b'foo')
         self.assertFalse(files[1].compressed)
         self.assertEqual(files[1].read(), b'foo')
 
-        self.assertEqual(files[2].filename, 'baz/qux')
+        self.assertEqual(files[2].filename, b'baz/qux')
         self.assertTrue(files[2].compressed)
         self.assertEqual(files[2].read(), b'aaaaaaaaaaaaanopqrstuvwxyz')
 
-        self.assertTrue('bar' in jar)
-        self.assertTrue('foo' in jar)
-        self.assertFalse('baz' in jar)
-        self.assertTrue('baz/qux' in jar)
-        self.assertTrue(jar['bar'], files[1])
-        self.assertTrue(jar['foo'], files[0])
-        self.assertTrue(jar['baz/qux'], files[2])
+        self.assertTrue(b'bar' in jar)
+        self.assertTrue(b'foo' in jar)
+        self.assertFalse(b'baz' in jar)
+        self.assertTrue(b'baz/qux' in jar)
+        self.assertTrue(jar[b'bar'], files[1])
+        self.assertTrue(jar[b'foo'], files[0])
+        self.assertTrue(jar[b'baz/qux'], files[2])
 
         s.seek(0)
         jar = JarReader(fileobj=s)
-        self.assertTrue('bar' in jar)
-        self.assertTrue('foo' in jar)
-        self.assertFalse('baz' in jar)
-        self.assertTrue('baz/qux' in jar)
+        self.assertTrue(b'bar' in jar)
+        self.assertTrue(b'foo' in jar)
+        self.assertFalse(b'baz' in jar)
+        self.assertTrue(b'baz/qux' in jar)
 
         files[0].seek(0)
-        self.assertEqual(jar['bar'].filename, files[0].filename)
-        self.assertEqual(jar['bar'].compressed, files[0].compressed)
-        self.assertEqual(jar['bar'].read(), files[0].read())
+        self.assertEqual(jar[b'bar'].filename, files[0].filename)
+        self.assertEqual(jar[b'bar'].compressed, files[0].compressed)
+        self.assertEqual(jar[b'bar'].read(), files[0].read())
 
         files[1].seek(0)
-        self.assertEqual(jar['foo'].filename, files[1].filename)
-        self.assertEqual(jar['foo'].compressed, files[1].compressed)
-        self.assertEqual(jar['foo'].read(), files[1].read())
+        self.assertEqual(jar[b'foo'].filename, files[1].filename)
+        self.assertEqual(jar[b'foo'].compressed, files[1].compressed)
+        self.assertEqual(jar[b'foo'].read(), files[1].read())
 
         files[2].seek(0)
-        self.assertEqual(jar['baz/qux'].filename, files[2].filename)
-        self.assertEqual(jar['baz/qux'].compressed, files[2].compressed)
-        self.assertEqual(jar['baz/qux'].read(), files[2].read())
+        self.assertEqual(jar[b'baz/qux'].filename, files[2].filename)
+        self.assertEqual(jar[b'baz/qux'].compressed, files[2].compressed)
+        self.assertEqual(jar[b'baz/qux'].read(), files[2].read())
 
     def test_rejar(self):
         s = MockDest()
         with JarWriter(fileobj=s) as jar:
             jar.add('foo', b'foo')
             jar.add('bar', b'aaaaaaaaaaaaanopqrstuvwxyz')
             jar.add('baz/qux', b'aaaaaaaaaaaaanopqrstuvwxyz', False)
 
         new = MockDest()
         with JarWriter(fileobj=new) as jar:
             for j in JarReader(fileobj=s):
                 jar.add(j.filename, j)
 
         jar = JarReader(fileobj=new)
         files = [j for j in jar]
 
-        self.assertEqual(files[0].filename, 'foo')
+        self.assertEqual(files[0].filename, b'foo')
         self.assertFalse(files[0].compressed)
         self.assertEqual(files[0].read(), b'foo')
 
-        self.assertEqual(files[1].filename, 'bar')
+        self.assertEqual(files[1].filename, b'bar')
         self.assertTrue(files[1].compressed)
         self.assertEqual(files[1].read(), b'aaaaaaaaaaaaanopqrstuvwxyz')
 
-        self.assertEqual(files[2].filename, 'baz/qux')
+        self.assertEqual(files[2].filename, b'baz/qux')
         self.assertTrue(files[2].compressed)
         self.assertEqual(files[2].read(), b'aaaaaaaaaaaaanopqrstuvwxyz')
 
     def test_add_from_finder(self):
         s = MockDest()
         with JarWriter(fileobj=s) as jar:
             finder = FileFinder(test_data_path)
             for p, f in finder.find('test_data'):
                 jar.add('test_data', f)
 
         jar = JarReader(fileobj=s)
         files = [j for j in jar]
 
-        self.assertEqual(files[0].filename, 'test_data')
+        self.assertEqual(files[0].filename, b'test_data')
         self.assertFalse(files[0].compressed)
         self.assertEqual(files[0].read(), b'test_data')
 
 
 class TestPreload(unittest.TestCase):
     def test_preload(self):
         s = MockDest()
         with JarWriter(fileobj=s) as jar:
@@ -273,25 +273,25 @@ class TestPreload(unittest.TestCase):
 
         jar = JarReader(fileobj=s)
         self.assertEqual(jar.last_preloaded, None)
 
         with JarWriter(fileobj=s) as jar:
             jar.add('foo', b'foo')
             jar.add('bar', b'abcdefghijklmnopqrstuvwxyz')
             jar.add('baz/qux', b'aaaaaaaaaaaaanopqrstuvwxyz')
-            jar.preload(['baz/qux', b'bar'])
+            jar.preload(['baz/qux', 'bar'])
 
         jar = JarReader(fileobj=s)
         self.assertEqual(jar.last_preloaded, b'bar')
         files = [j for j in jar]
 
-        self.assertEqual(files[0].filename, 'baz/qux')
-        self.assertEqual(files[1].filename, 'bar')
-        self.assertEqual(files[2].filename, 'foo')
+        self.assertEqual(files[0].filename, b'baz/qux')
+        self.assertEqual(files[1].filename, b'bar')
+        self.assertEqual(files[2].filename, b'foo')
 
 
 class TestJarLog(unittest.TestCase):
     def test_jarlog(self):
         s = six.moves.cStringIO('\n'.join([
             'bar/baz.jar first',
             'bar/baz.jar second',
             'bar/baz.jar third',