Bug 911375 - Part 1: Add support for optional existing files; r=glandium
authorGregory Szorc <gps@mozilla.com>
Tue, 03 Sep 2013 22:16:47 -0700
changeset 158322 5298dd852af0556dee14053f50616532e398c0c8
parent 158321 49c8cb0df3378b6301db79b632ca145429148ec6
child 158323 6baf14826ea3a157ff2c8e6b5ffd0c16ae55d27f
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs911375
milestone26.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 911375 - Part 1: Add support for optional existing files; r=glandium
python/mozbuild/mozpack/files.py
python/mozbuild/mozpack/manifests.py
python/mozbuild/mozpack/test/test_files.py
python/mozbuild/mozpack/test/test_manifests.py
--- a/python/mozbuild/mozpack/files.py
+++ b/python/mozbuild/mozpack/files.py
@@ -254,33 +254,44 @@ class AbsoluteSymlinkFile(File):
         except EnvironmentError:
             os.remove(temp_dest)
             raise
 
         os.rename(temp_dest, dest)
         return True
 
 
-class RequiredExistingFile(BaseFile):
+class ExistingFile(BaseFile):
     '''
-    File class that represents a file that must exist in the destination.
+    File class that represents a file that may exist but whose content comes
+    from elsewhere.
 
-    The purpose of this class is to account for files that are installed
-    via external means.
+    This purpose of this class is to account for files that are installed via
+    external means. It is typically only used in manifests or in registries to
+    account for files.
 
     When asked to copy, this class does nothing because nothing is known about
-    the source file/data. However, since this file is required, we do validate
-    that the destination path exists.
+    the source file/data.
+
+    Instances of this class come in two flavors: required and optional. If an
+    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):
             dest = Dest(dest)
         else:
             assert isinstance(dest, Dest)
 
+        if not self.required:
+            return
+
         if not dest.exists():
             errors.fatal("Required existing file doesn't exist: %s" %
                 dest.path)
 
 
 class GeneratedFile(BaseFile):
     '''
     File class for content with no previous existence on the filesystem.
--- a/python/mozbuild/mozpack/manifests.py
+++ b/python/mozbuild/mozpack/manifests.py
@@ -4,18 +4,18 @@
 
 from __future__ import unicode_literals
 
 from contextlib import contextmanager
 
 from .copier import FilePurger
 from .files import (
     AbsoluteSymlinkFile,
+    ExistingFile,
     File,
-    RequiredExistingFile,
 )
 import mozpack.path as mozpath
 
 
 # This probably belongs in a more generic module. Where?
 @contextmanager
 def _auto_fileobj(path, fileobj, mode='r'):
     if path and fileobj:
@@ -125,23 +125,32 @@ class InstallManifest(object):
 
       copy -- The file specified as the source path will be copied to the
           destination path.
 
       symlink -- The destination path will be a symlink to the source path.
           If symlinks are not supported, a copy will be performed.
 
       exists -- The destination path is accounted for and won't be deleted by
-          the FileCopier.
+          the FileCopier. If the destination path doesn't exist, an error is
+          raised.
+
+      optional -- The destination path is accounted for and won't be deleted by
+          the FileCopier. No error is raised if the destination path does not
+          exist.
+
+    Versions 1 and 2 of the manifest format are similar. Version 2 added
+    optional path support.
     """
     FIELD_SEPARATOR = '\x1f'
 
     SYMLINK = 1
     COPY = 2
     REQUIRED_EXISTS = 3
+    OPTIONAL_EXISTS = 4
 
     def __init__(self, path=None, fileobj=None):
         """Create a new InstallManifest entry.
 
         If path is defined, the manifest will be populated with data from the
         file path.
 
         If fh is defined, the manifest will be populated with data read
@@ -154,17 +163,17 @@ class InstallManifest(object):
         if not path and not fileobj:
             return
 
         with _auto_fileobj(path, fileobj, 'rb') as fh:
             self._load_from_fileobj(fh)
 
     def _load_from_fileobj(self, fileobj):
         version = fileobj.readline().rstrip()
-        if version != '1':
+        if version not in ('1', '2'):
             raise UnreadableInstallManifest('Unknown manifest version: ' %
                 version)
 
         for line in fileobj:
             line = line.rstrip()
 
             fields = line.split(self.FIELD_SEPARATOR)
 
@@ -180,16 +189,21 @@ class InstallManifest(object):
                 self.add_copy(source, dest)
                 continue
 
             if record_type == self.REQUIRED_EXISTS:
                 _, path = fields
                 self.add_required_exists(path)
                 continue
 
+            if record_type == self.OPTIONAL_EXISTS:
+                _, path = fields
+                self.add_optional_exists(path)
+                continue
+
             raise UnreadableInstallManifest('Unknown record type: %d' %
                 record_type)
 
     def __len__(self):
         return len(self._dests)
 
     def __contains__(self, item):
         return item in self._dests
@@ -213,17 +227,17 @@ class InstallManifest(object):
         """Serialize this manifest to a file or file object.
 
         If path is specified, that file will be written to. If fileobj is specified,
         the serialized content will be written to that file object.
 
         It is an error if both are specified.
         """
         with _auto_fileobj(path, fileobj, 'wb') as fh:
-            fh.write('1\n')
+            fh.write('2\n')
 
             for dest in sorted(self._dests):
                 entry = self._dests[dest]
 
                 parts = ['%d' % entry[0], dest]
                 parts.extend(entry[1:])
                 fh.write('%s\n' % self.FIELD_SEPARATOR.join(
                     p.encode('utf-8') for p in parts))
@@ -238,22 +252,31 @@ class InstallManifest(object):
     def add_copy(self, source, dest):
         """Add a copy to this manifest.
 
         source will be copied to dest.
         """
         self._add_entry(dest, (self.COPY, source))
 
     def add_required_exists(self, dest):
-        """Record that a destination file may exist.
+        """Record that a destination file must exist.
 
         This effectively prevents the listed file from being deleted.
         """
         self._add_entry(dest, (self.REQUIRED_EXISTS,))
 
+    def add_optional_exists(self, dest):
+        """Record that a destination file may exist.
+
+        This effectively prevents the listed file from being deleted. Unlike a
+        "required exists" file, files of this type do not raise errors if the
+        destination file does not exist.
+        """
+        self._add_entry(dest, (self.OPTIONAL_EXISTS,))
+
     def _add_entry(self, dest, entry):
         if dest in self._dests:
             raise ValueError('Item already in manifest: %s' % dest)
 
         self._dests[dest] = entry
 
     def populate_registry(self, registry):
         """Populate a mozpack.copier.FileRegistry instance with data from us.
@@ -270,13 +293,17 @@ class InstallManifest(object):
                 registry.add(dest, AbsoluteSymlinkFile(entry[1]))
                 continue
 
             if install_type == self.COPY:
                 registry.add(dest, File(entry[1]))
                 continue
 
             if install_type == self.REQUIRED_EXISTS:
-                registry.add(dest, RequiredExistingFile())
+                registry.add(dest, ExistingFile(required=True))
+                continue
+
+            if install_type == self.OPTIONAL_EXISTS:
+                registry.add(dest, ExistingFile(required=False))
                 continue
 
             raise Exception('Unknown install type defined in manifest: %d' %
                 install_type)
--- a/python/mozbuild/mozpack/test/test_files.py
+++ b/python/mozbuild/mozpack/test/test_files.py
@@ -1,25 +1,25 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from mozpack.errors import ErrorMessage
 from mozpack.files import (
     AbsoluteSymlinkFile,
+    DeflatedFile,
     Dest,
+    ExistingFile,
+    FileFinder,
     File,
     GeneratedFile,
-    DeflatedFile,
+    JarFinder,
     ManifestFile,
-    XPTFile,
     MinifiedProperties,
-    FileFinder,
-    JarFinder,
-    RequiredExistingFile,
+    XPTFile,
 )
 from mozpack.mozjar import (
     JarReader,
     JarWriter,
 )
 from mozpack.chrome.manifest import (
     ManifestContent,
     ManifestResource,
@@ -320,28 +320,40 @@ class TestAbsoluteSymlinkFile(TestWithTm
 
         s = AbsoluteSymlinkFile(source)
         self.assertFalse(s.copy(dest))
 
         link = os.readlink(dest)
         self.assertEqual(link, source)
 
 
-class TestRequiredExistingFile(TestWithTmpDir):
-    def test_missing_dest(self):
+class TestExistingFile(TestWithTmpDir):
+    def test_required_missing_dest(self):
         with self.assertRaisesRegexp(ErrorMessage, 'Required existing file'):
-            f = RequiredExistingFile()
+            f = ExistingFile(required=True)
             f.copy(self.tmppath('dest'))
 
-    def test_existing_dest(self):
+    def test_required_existing_dest(self):
         p = self.tmppath('dest')
         with open(p, 'a'):
             pass
 
-        f = RequiredExistingFile()
+        f = ExistingFile(required=True)
+        f.copy(p)
+
+    def test_optional_missing_dest(self):
+        f = ExistingFile(required=False)
+        f.copy(self.tmppath('dest'))
+
+    def test_optional_existing_dest(self):
+        p = self.tmppath('dest')
+        with open(p, 'a'):
+            pass
+
+        f = ExistingFile(required=False)
         f.copy(p)
 
 
 class TestGeneratedFile(TestWithTmpDir):
     def test_generated_file(self):
         '''
         Check that GeneratedFile.copy yields the proper content in the
         destination file in all situations that trigger different code paths
--- a/python/mozbuild/mozpack/test/test_manifests.py
+++ b/python/mozbuild/mozpack/test/test_manifests.py
@@ -56,59 +56,66 @@ class TestInstallManifest(TestWithTmpDir
         m = InstallManifest()
         self.assertEqual(len(m), 0)
 
     def test_adds(self):
         m = InstallManifest()
         m.add_symlink('s_source', 's_dest')
         m.add_copy('c_source', 'c_dest')
         m.add_required_exists('e_dest')
+        m.add_optional_exists('o_dest')
 
-        self.assertEqual(len(m), 3)
+        self.assertEqual(len(m), 4)
         self.assertIn('s_dest', m)
         self.assertIn('c_dest', m)
         self.assertIn('e_dest', m)
+        self.assertIn('o_dest', m)
 
         with self.assertRaises(ValueError):
             m.add_symlink('s_other', 's_dest')
 
         with self.assertRaises(ValueError):
             m.add_copy('c_other', 'c_dest')
 
         with self.assertRaises(ValueError):
             m.add_required_exists('e_dest')
 
+        with self.assertRaises(ValueError):
+            m.add_optional_exists('o_dest')
+
     def _get_test_manifest(self):
         m = InstallManifest()
         m.add_symlink(self.tmppath('s_source'), 's_dest')
         m.add_copy(self.tmppath('c_source'), 'c_dest')
         m.add_required_exists('e_dest')
+        m.add_optional_exists('o_dest')
 
         return m
 
     def test_serialization(self):
         m = self._get_test_manifest()
 
         p = self.tmppath('m')
         m.write(path=p)
         self.assertTrue(os.path.isfile(p))
 
         with open(p, 'rb') as fh:
             c = fh.read()
 
-        self.assertEqual(c.count('\n'), 4)
+        self.assertEqual(c.count('\n'), 5)
 
         lines = c.splitlines()
-        self.assertEqual(len(lines), 4)
+        self.assertEqual(len(lines), 5)
 
-        self.assertEqual(lines[0], '1')
+        self.assertEqual(lines[0], '2')
         self.assertEqual(lines[1], '2\x1fc_dest\x1f%s' %
             self.tmppath('c_source'))
         self.assertEqual(lines[2], '3\x1fe_dest')
-        self.assertEqual(lines[3], '1\x1fs_dest\x1f%s' %
+        self.assertEqual(lines[3], '4\x1fo_dest')
+        self.assertEqual(lines[4], '1\x1fs_dest\x1f%s' %
             self.tmppath('s_source'))
 
         m2 = InstallManifest(path=p)
         self.assertEqual(m, m2)
         p2 = self.tmppath('m2')
         m2.write(path=p2)
 
         with open(p2, 'rb') as fh:
@@ -116,29 +123,29 @@ class TestInstallManifest(TestWithTmpDir
 
         self.assertEqual(c, c2)
 
     def test_populate_registry(self):
         m = self._get_test_manifest()
         r = FileRegistry()
         m.populate_registry(r)
 
-        self.assertEqual(len(r), 3)
-        self.assertEqual(r.paths(), ['c_dest', 'e_dest', 's_dest'])
+        self.assertEqual(len(r), 4)
+        self.assertEqual(r.paths(), ['c_dest', 'e_dest', 'o_dest', 's_dest'])
 
     def test_or(self):
         m1 = self._get_test_manifest()
         m2 = InstallManifest()
         m2.add_symlink('s_source2', 's_dest2')
         m2.add_copy('c_source2', 'c_dest2')
 
         m1 |= m2
 
         self.assertEqual(len(m2), 2)
-        self.assertEqual(len(m1), 5)
+        self.assertEqual(len(m1), 6)
 
         self.assertIn('s_dest2', m1)
         self.assertIn('c_dest2', m1)
 
     def test_copier_application(self):
         dest = self.tmppath('dest')
         os.mkdir(dest)
 
@@ -150,36 +157,40 @@ class TestInstallManifest(TestWithTmpDir
             fh.write('symlink!')
 
         with open(self.tmppath('c_source'), 'wt') as fh:
             fh.write('copy!')
 
         with open(self.tmppath('dest/e_dest'), 'a'):
             pass
 
+        with open(self.tmppath('dest/o_dest'), 'a'):
+            pass
+
         m = self._get_test_manifest()
         c = FileCopier()
         m.populate_registry(c)
         result = c.copy(dest)
 
         self.assertTrue(os.path.exists(self.tmppath('dest/s_dest')))
         self.assertTrue(os.path.exists(self.tmppath('dest/c_dest')))
         self.assertTrue(os.path.exists(self.tmppath('dest/e_dest')))
+        self.assertTrue(os.path.exists(self.tmppath('dest/o_dest')))
         self.assertFalse(os.path.exists(to_delete))
 
         with open(self.tmppath('dest/s_dest'), 'rt') as fh:
             self.assertEqual(fh.read(), 'symlink!')
 
         with open(self.tmppath('dest/c_dest'), 'rt') as fh:
             self.assertEqual(fh.read(), 'copy!')
 
         self.assertEqual(result.updated_files, set(self.tmppath(p) for p in (
             'dest/s_dest', 'dest/c_dest')))
         self.assertEqual(result.existing_files,
-            set([self.tmppath('dest/e_dest')]))
+            set([self.tmppath('dest/e_dest'), self.tmppath('dest/o_dest')]))
         self.assertEqual(result.removed_files, {to_delete})
         self.assertEqual(result.removed_directories, set())
 
 
 
 
 if __name__ == '__main__':
     mozunit.main()