Bug 1382697 - Use a single InstallManifest for entire jar.mn file; r?glandium draft
authorGregory Szorc <gps@mozilla.com>
Fri, 21 Jul 2017 18:00:50 -0700
changeset 613574 5fe8a81e5c6fa7596e09276249cc7ce069b2214b
parent 613573 039a886d8347b7918037b7e192329c37296661ea
child 613575 63f7354cd30d9989bb0cdc949d00f30e144902e7
push id69818
push userbmo:gps@mozilla.com
push dateSat, 22 Jul 2017 02:09:10 +0000
reviewersglandium
bugs1382697
milestone56.0a1
Bug 1382697 - Use a single InstallManifest for entire jar.mn file; r?glandium The change to introduce InstallManifest did the simplest thing and preserved the existing file processing model. A downside was we were potentially processing multiple InstallManifest per jar.mn file. FileCopier is more efficient the more files you throw at it. So, this commit consolidates down to a single InstallManifest/FileCopier when processing jar.mn files. MozReview-Commit-ID: Jl2zkprWxtJ
python/mozbuild/mozbuild/jar.py
--- a/python/mozbuild/mozbuild/jar.py
+++ b/python/mozbuild/mozbuild/jar.py
@@ -174,17 +174,16 @@ class JarMaker(object):
         self.pp = Preprocessor()
         self.topsourcedir = None
         self.sourcedirs = []
         self.localedirs = None
         self.l10nbase = None
         self.l10nmerge = None
         self.relativesrcdir = None
         self.rootManifestAppId = None
-        self._seen_output = set()
 
     def getCommandLineParser(self):
         '''Get a optparse.OptionParser for jarmaker.
 
         This OptionParser has the options for jarmaker as well as
         the options for the inner PreProcessor.
         '''
 
@@ -293,18 +292,24 @@ class JarMaker(object):
                 self.generateLocaleDirs(self.relativesrcdir)
         if isinstance(infile, basestring):
             logging.info('processing ' + infile)
             self.sourcedirs.append(_normpath(os.path.dirname(infile)))
         pp = self.pp.clone()
         pp.out = JarManifestParser()
         pp.do_include(infile)
 
+        manifest = InstallManifest()
+
         for info in pp.out:
-            self.processJarSection(info, jardir)
+            self.processJarSection(manifest, info, jardir)
+
+        copier = FileCopier()
+        manifest.populate_registry(copier)
+        copier.copy(jardir, remove_unaccounted=False)
 
     def generateLocaleDirs(self, relativesrcdir):
         if os.path.basename(relativesrcdir) == 'locales':
             # strip locales
             l10nrelsrcdir = os.path.dirname(relativesrcdir)
         else:
             l10nrelsrcdir = relativesrcdir
         locdirs = []
@@ -315,48 +320,48 @@ class JarMaker(object):
         if self.l10nbase:
             locdirs.append(os.path.join(self.l10nbase, l10nrelsrcdir))
         if self.l10nmerge or not self.l10nbase:
             # add en-US if we merge, or if it's not l10n
             locdirs.append(os.path.join(self.topsourcedir,
                            relativesrcdir, 'en-US'))
         return locdirs
 
-    def processJarSection(self, jarinfo, jardir):
+    def processJarSection(self, manifest, jarinfo, jardir):
         '''Internal method called by makeJar to actually process a section
         of a jar.mn file.
         '''
 
         # chromebasepath is used for chrome registration manifests
         # {0} is getting replaced with chrome/ for chrome.manifest, and with
         # an empty string for jarfile.manifest
 
         chromebasepath = '{0}%s/' % os.path.basename(jarinfo.name)
 
-        dest_path = os.path.join(jardir, jarinfo.base, jarinfo.name)
+        rel_path = mozpath.join(jarinfo.base, jarinfo.name)
 
         if jarinfo.relativesrcdir:
             self.localedirs = self.generateLocaleDirs(jarinfo.relativesrcdir)
 
         manifest = InstallManifest()
 
         for e in jarinfo.entries:
-            self._processEntryLine(manifest, e)
-
-        copier = FileCopier()
-        manifest.populate_registry(copier)
-        copier.copy(dest_path, remove_unaccounted=False)
+            self._processEntryLine(manifest, rel_path, e)
 
         self.finalizeJar(jardir, jarinfo.base, jarinfo.name, chromebasepath,
                          jarinfo.chrome_manifests)
 
-    def _processEntryLine(self, manifest, e):
+        return manifest
+
+    def _processEntryLine(self, manifest, rel_path, e):
         out = e.output
         src = e.source
 
+        out = mozpath.join(rel_path, out)
+
         # pick the right sourcedir -- l10n, topsrc or src
 
         if e.is_locale:
             src_base = self.localedirs
         elif src.startswith('/'):
             # path/in/jar/file_name.xul     (/path/in/sourcetree/file_name.xul)
             # refers to a path relative to topsourcedir, use that as base
             # and strip the leading '/'
@@ -384,33 +389,29 @@ class JarMaker(object):
                         continue
                     emitted.add(reduced_path)
                     e = JarManifestEntry(
                         mozpath.join(out, reduced_path),
                         path,
                         is_locale=e.is_locale,
                         preprocess=e.preprocess,
                     )
-                    self._processEntryLine(manifest, e)
+                    self._processEntryLine(manifest, rel_path, e)
             return
 
         # check if the source file exists
         realsrc = None
         for _srcdir in src_base:
             if os.path.isfile(os.path.join(_srcdir, src)):
                 realsrc = os.path.join(_srcdir, src)
                 break
         if realsrc is None:
             raise RuntimeError('File "{0}" not found in {1}'.format(src,
                                ', '.join(src_base)))
 
-        if out in self._seen_output:
-            raise RuntimeError('%s already added' % out)
-        self._seen_output.add(out)
-
         if e.preprocess:
             content = io.BytesIO()
 
             with open(realsrc) as fh:
                 pp = self.pp.clone()
                 if src[-4:] == '.css':
                     pp.setMarker('%')
                 pp.out = content