Bug 890097 - Part 1: Use more Pythonic API for PurgeManifest; r=glandium

python/mozbuild/mozbuild/action/purge_manifests.py

@@ -14,11 +14,13 @@ import threading
 
 from mozpack.manifests import PurgeManifest
 
+
 def do_purge(purger, dest, state):
     state['result'] = purger.purge(dest)
 
+
 def process_manifest(topdir, manifest_path):
-    manifest = PurgeManifest.from_path(manifest_path)
+    manifest = PurgeManifest(path=manifest_path)
     purger = manifest.get_purger()
     full = os.path.join(topdir, manifest.relpath)
 

python/mozbuild/mozbuild/backend/recursivemake.py

@@ -386,10 +386,9 @@ class RecursiveMakeBackend(BuildBackend):
 
         for k, manifest in self._purge_manifests.items():
             purger.add(k)
-            full = os.path.join(man_dir, k)
 
             fh = FileAvoidWrite(os.path.join(man_dir, k))
-            manifest.write_fileobj(fh)
+            manifest.write(fileobj=fh)
             self._update_from_avoid_write(fh.close())
 
         purger.purge(man_dir)

python/mozbuild/mozbuild/test/backend/test_recursivemake.py

@@ -248,7 +248,7 @@ class TestRecursiveMakeBackend(BackendTester):
         ])
 
         # EXPORTS files should appear in the dist_include purge manifest.
-        m = PurgeManifest.from_path(os.path.join(env.topobjdir,
+        m = PurgeManifest(path=os.path.join(env.topobjdir,
             '_build_manifests', 'purge', 'dist_include'))
         self.assertIn('foo.h', m.entries)
         self.assertIn('mozilla/mozilla1.h', m.entries)
@@ -300,7 +300,7 @@ class TestRecursiveMakeBackend(BackendTester):
             full = os.path.join(purge_dir, e)
             self.assertTrue(os.path.exists(full))
 
-        m = PurgeManifest.from_path(os.path.join(purge_dir, 'dist_bin'))
+        m = PurgeManifest(path=os.path.join(purge_dir, 'dist_bin'))
         self.assertEqual(m.relpath, 'dist/bin')
 
     def test_old_purge_manifest_deleted(self):
@@ -311,7 +311,7 @@ class TestRecursiveMakeBackend(BackendTester):
         manifest_path = os.path.join(purge_dir, 'old_manifest')
         os.makedirs(purge_dir)
         m = PurgeManifest()
-        m.write_file(manifest_path)
+        m.write(path=manifest_path)
 
         self.assertTrue(os.path.exists(manifest_path))
         self._consume('stub0', RecursiveMakeBackend, env)

python/mozbuild/mozpack/manifests.py

@@ -4,10 +4,31 @@
 
 from __future__ import unicode_literals
 
+from contextlib import contextmanager
+
 from .copier import FilePurger
 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:
+        raise AssertionError('Only 1 of path or fileobj may be defined.')
+
+    if not path and not fileobj:
+        raise AssertionError('Must specified 1 of path or fileobj.')
+
+    if path:
+        fileobj = open(path, mode)
+
+    try:
+        yield fileobj
+    finally:
+        if path:
+            fileobj.close()
+
+
 class UnreadablePurgeManifest(Exception):
     """Error for failure when reading content of a serialized PurgeManifest."""
 
@@ -26,51 +47,44 @@ class PurgeManifest(object):
     Don't be confused by the name of this class: entries are files that are
     *not* purged.
     """
-    def __init__(self, relpath=''):
+    def __init__(self, relpath='', path=None, fileobj=None):
         self.relpath = relpath
         self.entries = set()
 
+        if not path and not fileobj:
+            return
+
+        with _auto_fileobj(path, fileobj, mode='rt') as fh:
+            self._read_from_fh(fh)
+
     def __eq__(self, other):
         if not isinstance(other, PurgeManifest):
             return False
 
         return other.relpath == self.relpath and other.entries == self.entries
 
-    @staticmethod
-    def from_path(path):
-        with open(path, 'rt') as fh:
-            return PurgeManifest.from_fileobj(fh)
-
-    @staticmethod
-    def from_fileobj(fh):
-        m = PurgeManifest()
-
-        version = fh.readline().rstrip()
+    def _read_from_fh(self, fileobj):
+        version = fileobj.readline().rstrip()
         if version != '1':
             raise UnreadablePurgeManifest('Unknown manifest version: %s' %
                 version)
 
-        m.relpath = fh.readline().rstrip()
+        self.relpath = fileobj.readline().rstrip()
 
-        for entry in fh:
-            m.entries.add(entry.rstrip())
-
-        return m
+        for entry in fileobj:
+            self.entries.add(entry.rstrip())
 
     def add(self, path):
         return self.entries.add(path)
 
-    def write_file(self, path):
-        with open(path, 'wt') as fh:
-            return self.write_fileobj(fh)
+    def write(self, path=None, fileobj=None):
+        with _auto_fileobj(path, fileobj, 'wt') as fh:
+            fh.write('1\n')
+            fh.write('%s\n' % self.relpath)
 
-    def write_fileobj(self, fh):
-        fh.write('1\n')
-        fh.write('%s\n' % self.relpath)
-
-        # We write sorted so written output is consistent.
-        for entry in sorted(self.entries):
-            fh.write('%s\n' % entry)
+            # We write sorted so written output is consistent.
+            for entry in sorted(self.entries):
+                fh.write('%s\n' % entry)
 
     def get_purger(self, prepend_relpath=False):
         """Obtain a FilePurger instance from this manifest.

python/mozbuild/mozpack/test/test_manifests.py

@@ -27,11 +27,11 @@ class TestPurgeManifest(TestWithTmpDir):
         m.add('foo')
         m.add('bar')
         p = self.tmppath('m')
-        m.write_file(p)
+        m.write(path=p)
 
         self.assertTrue(os.path.exists(p))
 
-        m2 = PurgeManifest.from_path(p)
+        m2 = PurgeManifest(path=p)
         self.assertEqual(m.relpath, m2.relpath)
         self.assertEqual(m.entries, m2.entries)
         self.assertEqual(m, m2)
@@ -44,7 +44,7 @@ class TestPurgeManifest(TestWithTmpDir):
             fh.write('not relevant')
 
         with self.assertRaises(UnreadablePurgeManifest):
-            PurgeManifest.from_path(p)
+            PurgeManifest(path=p)
 
 
 if __name__ == '__main__':