Bug 890097 - Part 1: Use more Pythonic API for PurgeManifest; r=glandium
authorGregory Szorc <gps@mozilla.com>
Tue, 23 Jul 2013 14:35:01 -0700
changeset 152000 0c91f66956b17455de9329ab8309a070bf45ef38
parent 151999 590fc39b6a1ca9e16ddb9dcda54e49b5764fe7e6
child 152001 d27457347dd6249b694432b6698a3f07b31194bb
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs890097
milestone25.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 890097 - Part 1: Use more Pythonic API for PurgeManifest; r=glandium
python/mozbuild/mozbuild/action/purge_manifests.py
python/mozbuild/mozbuild/backend/recursivemake.py
python/mozbuild/mozbuild/test/backend/test_recursivemake.py
python/mozbuild/mozpack/manifests.py
python/mozbuild/mozpack/test/test_manifests.py
--- a/python/mozbuild/mozbuild/action/purge_manifests.py
+++ b/python/mozbuild/mozbuild/action/purge_manifests.py
@@ -9,21 +9,23 @@ from __future__ import print_function, u
 
 import argparse
 import os
 import sys
 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)
 
     state = dict(
         relpath=manifest.relpath,
         result=None,
     )
 
--- a/python/mozbuild/mozbuild/backend/recursivemake.py
+++ b/python/mozbuild/mozbuild/backend/recursivemake.py
@@ -381,15 +381,14 @@ class RecursiveMakeBackend(BuildBackend)
             'purge')
 
         # We have a purger for the manifests themselves to ensure we don't over
         # purge if we delete a purge manifest.
         purger = FilePurger()
 
         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)
--- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
+++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ -243,17 +243,17 @@ class TestRecursiveMakeBackend(BackendTe
             'EXPORTS_mozilla/dom += dom1.h dom2.h',
             'EXPORTS_NAMESPACES += mozilla/gfx',
             'EXPORTS_mozilla/gfx += gfx.h',
             'EXPORTS_NAMESPACES += nspr/private',
             'EXPORTS_nspr/private += pprio.h',
         ])
 
         # 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)
         self.assertIn('mozilla/dom/dom2.h', m.entries)
 
     def test_xpcshell_manifests(self):
         """Ensure XPCSHELL_TESTS_MANIFESTS is written out correctly."""
         env = self._consume('xpcshell_manifests', RecursiveMakeBackend)
@@ -295,28 +295,28 @@ class TestRecursiveMakeBackend(BackendTe
             'dist_sdk',
             'tests',
         ]
 
         for e in expected:
             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):
         # Simulate a purge manifest from a previous backend version. Ensure it
         # is deleted.
         env = self._get_environment('stub0')
         purge_dir = os.path.join(env.topobjdir, '_build_manifests', 'purge')
         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)
         self.assertFalse(os.path.exists(manifest_path))
 
     def test_ipdl_sources(self):
         """Test that IPDL_SOURCES are written to ipdlsrcs.mk correctly."""
         env = self._consume('ipdl_sources', RecursiveMakeBackend)
--- a/python/mozbuild/mozpack/manifests.py
+++ b/python/mozbuild/mozpack/manifests.py
@@ -1,18 +1,39 @@
 # 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 __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."""
 
 
 class PurgeManifest(object):
     """Describes actions to be used with a copier.FilePurger instance.
 
     This class facilitates serialization and deserialization of data used
@@ -21,61 +42,54 @@ class PurgeManifest(object):
     The manifest contains a set of entries (paths that are accounted for and
     shouldn't be purged) and a relative path. The relative path is optional and
     can be used to e.g. have several manifest files in a directory be
     dynamically applied to subdirectories under a common base directory.
 
     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.
 
         If :prepend_relpath is truish, the relative path in the manifest will
         be prepended to paths added to the FilePurger. Otherwise, the raw paths
         will be used.
         """
--- a/python/mozbuild/mozpack/test/test_manifests.py
+++ b/python/mozbuild/mozpack/test/test_manifests.py
@@ -22,30 +22,30 @@ class TestPurgeManifest(TestWithTmpDir):
         self.assertEqual(m.relpath, '')
         self.assertEqual(len(m.entries), 0)
 
     def test_serialization(self):
         m = PurgeManifest(relpath='rel')
         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)
 
     def test_unknown_version(self):
         p = self.tmppath('bad')
 
         with open(p, 'wt') as fh:
             fh.write('2\n')
             fh.write('not relevant')
 
         with self.assertRaises(UnreadablePurgeManifest):
-            PurgeManifest.from_path(p)
+            PurgeManifest(path=p)
 
 
 if __name__ == '__main__':
     mozunit.main()