Bug 903149 - Part 3: Support for minifying packaged JavaScript; r=glandium
authorGregory Szorc <gps@mozilla.com>
Wed, 11 Sep 2013 19:54:19 -0700
changeset 176747 796ec394eef2
parent 176746 92f909129ecb
child 176748 84861ae010f9
push id41836
push usermshal@mozilla.com
push date2014-04-02 19:12 +0000
treeherdermozilla-inbound@84861ae010f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs903149
milestone31.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 903149 - Part 3: Support for minifying packaged JavaScript; r=glandium
build/mach_bootstrap.py
python/mozbuild/mozpack/files.py
python/mozbuild/mozpack/test/support/minify_js_verify.py
python/mozbuild/mozpack/test/test_files.py
toolkit/mozapps/installer/js-compare-ast.js
toolkit/mozapps/installer/packager.mk
toolkit/mozapps/installer/packager.py
--- a/build/mach_bootstrap.py
+++ b/build/mach_bootstrap.py
@@ -26,16 +26,17 @@ want to export this environment variable
 # TODO Bug 794506 Integrate with the in-tree virtualenv configuration.
 SEARCH_PATHS = [
     'python/mach',
     'python/mozboot',
     'python/mozbuild',
     'python/mozversioncontrol',
     'python/blessings',
     'python/configobj',
+    'python/jsmin',
     'python/psutil',
     'python/which',
     'build/pymake',
     'config',
     'dom/bindings',
     'dom/bindings/parser',
     'other-licenses/ply',
     'xpcom/idl-parser',
--- a/python/mozbuild/mozpack/files.py
+++ b/python/mozbuild/mozpack/files.py
@@ -1,18 +1,18 @@
 # 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/.
 
 import errno
 import os
 import platform
-import re
 import shutil
 import stat
+import subprocess
 import uuid
 import mozbuild.makeutil as makeutil
 from mozbuild.preprocessor import Preprocessor
 from mozbuild.util import FileAvoidWrite
 from mozpack.executables import (
     is_executable,
     may_strip,
     strip,
@@ -23,17 +23,21 @@ from mozpack.chrome.manifest import Mani
 from io import BytesIO
 from mozpack.errors import (
     ErrorMessage,
     errors,
 )
 from mozpack.mozjar import JarReader
 import mozpack.path
 from collections import OrderedDict
-from tempfile import mkstemp
+from jsmin import JavascriptMinify
+from tempfile import (
+    mkstemp,
+    NamedTemporaryFile,
+)
 
 
 class Dest(object):
     '''
     Helper interface for BaseFile.copy. The interface works as follows:
     - read() and write() can be used to sequentially read/write from the
       underlying file.
     - a call to read() after a write() will re-open the underlying file and
@@ -589,25 +593,86 @@ class MinifiedProperties(BaseFile):
         '''
         Return a file-like object allowing to read() the minified content of
         the properties file.
         '''
         return BytesIO(''.join(l for l in self._file.open().readlines()
                                if not l.startswith('#')))
 
 
+class MinifiedJavaScript(BaseFile):
+    '''
+    File class for minifying JavaScript files.
+    '''
+    def __init__(self, file, verify_command=None):
+        assert isinstance(file, BaseFile)
+        self._file = file
+        self._verify_command = verify_command
+
+    def open(self):
+        output = BytesIO()
+        minify = JavascriptMinify(self._file.open(), output)
+        minify.minify()
+        output.seek(0)
+
+        if not self._verify_command:
+            return output
+
+        input_source = self._file.open().read()
+        output_source = output.getvalue()
+
+        with NamedTemporaryFile() as fh1, NamedTemporaryFile() as fh2:
+            fh1.write(input_source)
+            fh2.write(output_source)
+            fh1.flush()
+            fh2.flush()
+
+            try:
+                args = list(self._verify_command)
+                args.extend([fh1.name, fh2.name])
+                subprocess.check_output(args, stderr=subprocess.STDOUT)
+            except subprocess.CalledProcessError as e:
+                errors.warn('JS minification verification failed for %s:' %
+                    (getattr(self._file, 'path', '<unknown>')))
+                # Prefix each line with "Warning:" so mozharness doesn't
+                # think these error messages are real errors.
+                for line in e.output.splitlines():
+                    errors.warn(line)
+
+                return self._file.open()
+
+        return output
+
+
 class BaseFinder(object):
-    def __init__(self, base, minify=False):
+    def __init__(self, base, minify=False, minify_js=False,
+        minify_js_verify_command=None):
         '''
-        Initializes the instance with a reference base directory. The
-        optional minify argument specifies whether file types supporting
-        minification (currently only "*.properties") should be minified.
+        Initializes the instance with a reference base directory.
+
+        The optional minify argument specifies whether minification of code
+        should occur. minify_js is an additional option to control minification
+        of JavaScript. It requires minify to be True.
+
+        minify_js_verify_command can be used to optionally verify the results
+        of JavaScript minification. If defined, it is expected to be an iterable
+        that will constitute the first arguments to a called process which will
+        receive the filenames of the original and minified JavaScript files.
+        The invoked process can then verify the results. If minification is
+        rejected, the process exits with a non-0 exit code and the original
+        JavaScript source is used. An example value for this argument is
+        ('/path/to/js', '/path/to/verify/script.js').
         '''
+        if minify_js and not minify:
+            raise ValueError('minify_js requires minify.')
+
         self.base = base
         self._minify = minify
+        self._minify_js = minify_js
+        self._minify_js_verify_command = minify_js_verify_command
 
     def find(self, pattern):
         '''
         Yield path, BaseFile_instance pairs for all files under the base
         directory and its subdirectories that match the given pattern. See the
         mozpack.path.match documentation for a description of the handled
         patterns.
         '''
@@ -639,21 +704,26 @@ class BaseFinder(object):
         return any(self.find(pattern))
 
     def _minify_file(self, path, file):
         '''
         Return an appropriate MinifiedSomething wrapper for the given BaseFile
         instance (file), according to the file type (determined by the given
         path), if the FileFinder was created with minification enabled.
         Otherwise, just return the given BaseFile instance.
-        Currently, only "*.properties" files are handled.
         '''
-        if self._minify and not isinstance(file, ExecutableFile):
-            if path.endswith('.properties'):
-                return MinifiedProperties(file)
+        if not self._minify or isinstance(file, ExecutableFile):
+            return file
+
+        if path.endswith('.properties'):
+            return MinifiedProperties(file)
+
+        if self._minify_js and path.endswith(('.js', '.jsm')):
+            return MinifiedJavaScript(file, self._minify_js_verify_command)
+
         return file
 
 
 class FileFinder(BaseFinder):
     '''
     Helper to get appropriate BaseFile instances from the file system.
     '''
     def __init__(self, base, find_executables=True, ignore=(), **kargs):
new file mode 100644
--- /dev/null
+++ b/python/mozbuild/mozpack/test/support/minify_js_verify.py
@@ -0,0 +1,11 @@
+# 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/.
+
+import sys
+
+
+if len(sys.argv) != 4:
+    raise Exception('Usage: minify_js_verify <exitcode> <orig> <minified>')
+
+sys.exit(int(sys.argv[1]))
--- a/python/mozbuild/mozpack/test/test_files.py
+++ b/python/mozbuild/mozpack/test/test_files.py
@@ -10,16 +10,17 @@ from mozpack.files import (
     DeflatedFile,
     Dest,
     ExistingFile,
     FileFinder,
     File,
     GeneratedFile,
     JarFinder,
     ManifestFile,
+    MinifiedJavaScript,
     MinifiedProperties,
     PreprocessedFile,
     XPTFile,
 )
 from mozpack.mozjar import (
     JarReader,
     JarWriter,
 )
@@ -30,16 +31,17 @@ from mozpack.chrome.manifest import (
     ManifestOverride,
 )
 import unittest
 import mozfile
 import mozunit
 import os
 import random
 import string
+import sys
 import mozpack.path
 from tempfile import mkdtemp
 from io import BytesIO
 from xpt import Typelib
 
 
 class TestWithTmpDir(unittest.TestCase):
     def setUp(self):
@@ -748,16 +750,59 @@ class TestMinifiedProperties(TestWithTmp
                          ['foo = bar\n', '\n'])
         open(self.tmppath('prop'), 'wb').write('\n'.join(propLines))
         MinifiedProperties(File(self.tmppath('prop'))) \
             .copy(self.tmppath('prop2'))
         self.assertEqual(open(self.tmppath('prop2')).readlines(),
                          ['foo = bar\n', '\n'])
 
 
+class TestMinifiedJavaScript(TestWithTmpDir):
+    orig_lines = [
+        '// Comment line',
+        'let foo = "bar";',
+        'var bar = true;',
+        '',
+        '// Another comment',
+    ]
+
+    def test_minified_javascript(self):
+        orig_f = GeneratedFile('\n'.join(self.orig_lines))
+        min_f = MinifiedJavaScript(orig_f)
+
+        mini_lines = min_f.open().readlines()
+        self.assertTrue(mini_lines)
+        self.assertTrue(len(mini_lines) < len(self.orig_lines))
+
+    def _verify_command(self, code):
+        our_dir = os.path.abspath(os.path.dirname(__file__))
+        return [
+            sys.executable,
+            os.path.join(our_dir, 'support', 'minify_js_verify.py'),
+            code,
+        ]
+
+    def test_minified_verify_success(self):
+        orig_f = GeneratedFile('\n'.join(self.orig_lines))
+        min_f = MinifiedJavaScript(orig_f,
+            verify_command=self._verify_command('0'))
+
+        mini_lines = min_f.open().readlines()
+        self.assertTrue(mini_lines)
+        self.assertTrue(len(mini_lines) < len(self.orig_lines))
+
+    def test_minified_verify_failure(self):
+        orig_f = GeneratedFile('\n'.join(self.orig_lines))
+        min_f = MinifiedJavaScript(orig_f,
+            verify_command=self._verify_command('1'))
+
+        mini_lines = min_f.open().readlines()
+        self.assertEqual(mini_lines, orig_f.open().readlines())
+
+
 class MatchTestTemplate(object):
     def prepare_match_test(self, with_dotfiles=False):
         self.add('bar')
         self.add('foo/bar')
         self.add('foo/baz')
         self.add('foo/qux/1')
         self.add('foo/qux/bar')
         self.add('foo/qux/2/test')
new file mode 100644
--- /dev/null
+++ b/toolkit/mozapps/installer/js-compare-ast.js
@@ -0,0 +1,28 @@
+/* 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/. */
+
+/**
+ * This script compares the AST of two JavaScript files passed as arguments.
+ * The script exits with a 0 status code if both files parse properly and the
+ * ASTs of both files are identical modulo location differences. The script
+ * exits with status code 1 if any of these conditions don't hold.
+ *
+ * This script is used as part of packaging to verify minified JavaScript files
+ * are identical to their original files.
+ */
+
+"use strict";
+
+function ast(filename) {
+  return JSON.stringify(Reflect.parse(snarf(filename), {loc: 0}));
+}
+
+if (scriptArgs.length !== 2) {
+  throw "usage: js js-compare-ast.js FILE1.js FILE2.js";
+}
+
+let ast0 = ast(scriptArgs[0]);
+let ast1 = ast(scriptArgs[1]);
+
+quit(ast0 == ast1 ? 0 : 1);
--- a/toolkit/mozapps/installer/packager.mk
+++ b/toolkit/mozapps/installer/packager.mk
@@ -700,27 +700,40 @@ MOZ_PACKAGER_FORMAT = $(error MOZ_PACKAG
 endif
 
 ifneq (android,$(MOZ_WIDGET_TOOLKIT))
 OPTIMIZEJARS = 1
 endif
 
 export NO_PKG_FILES USE_ELF_HACK ELF_HACK_FLAGS
 
+# A js binary is needed to perform verification of JavaScript minification.
+# We can only use the built binary when not cross-compiling. Environments
+# (such as release automation) can provide their own js binary to enable
+# verification when cross-compiling.
+ifndef JS_BINARY
+ifndef CROSS_COMPILE
+JS_BINARY = $(wildcard $(DIST)/bin/js)
+endif
+endif
+
 # Override the value of OMNIJAR_NAME from config.status with the value
 # set earlier in this file.
 
 stage-package: $(MOZ_PKG_MANIFEST)
 	@rm -rf $(DIST)/$(PKG_PATH)$(PKG_BASENAME).tar $(DIST)/$(PKG_PATH)$(PKG_BASENAME).dmg $@ $(EXCLUDE_LIST)
 	OMNIJAR_NAME=$(OMNIJAR_NAME) \
 	$(PYTHON) $(MOZILLA_DIR)/toolkit/mozapps/installer/packager.py $(DEFINES) \
 		--format $(MOZ_PACKAGER_FORMAT) \
 		$(addprefix --removals ,$(MOZ_PKG_REMOVALS)) \
 		$(if $(filter-out 0,$(MOZ_PKG_FATAL_WARNINGS)),,--ignore-errors) \
 		$(if $(MOZ_PACKAGER_MINIFY),--minify) \
+		$(if $(MOZ_PACKAGER_MINIFY_JS),--minify-js \
+		  $(addprefix --js-binary ,$(JS_BINARY)) \
+		) \
 		$(if $(JARLOG_DIR),$(addprefix --jarlog ,$(wildcard $(JARLOG_FILE_AB_CD)))) \
 		$(if $(OPTIMIZEJARS),--optimizejars) \
 		$(addprefix --unify ,$(UNIFY_DIST)) \
 		$(MOZ_PKG_MANIFEST) $(DIST) $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)$(if $(MOZ_PKG_MANIFEST),,$(_BINPATH)) \
 		$(if $(filter omni,$(MOZ_PACKAGER_FORMAT)),$(if $(NON_OMNIJAR_FILES),--non-resource $(NON_OMNIJAR_FILES)))
 	$(PYTHON) $(MOZILLA_DIR)/toolkit/mozapps/installer/find-dupes.py $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR)
 ifndef LIBXUL_SDK
 ifdef MOZ_PACKAGE_JSSHELL
--- a/toolkit/mozapps/installer/packager.py
+++ b/toolkit/mozapps/installer/packager.py
@@ -243,16 +243,22 @@ def main():
                         help='Choose the chrome format for packaging ' +
                         '(omni, jar or flat ; default: %(default)s)')
     parser.add_argument('--removals', default=None,
                         help='removed-files source file')
     parser.add_argument('--ignore-errors', action='store_true', default=False,
                         help='Transform errors into warnings.')
     parser.add_argument('--minify', action='store_true', default=False,
                         help='Make some files more compact while packaging')
+    parser.add_argument('--minify-js', action='store_true',
+                        help='Minify JavaScript files while packaging.')
+    parser.add_argument('--js-binary',
+                        help='Path to js binary. This is used to verify '
+                        'minified JavaScript. If this is not defined, '
+                        'minification verification will not be performed.')
     parser.add_argument('--jarlog', default='', help='File containing jar ' +
                         'access logs')
     parser.add_argument('--optimizejars', action='store_true', default=False,
                         help='Enable jar optimizations')
     parser.add_argument('--unify', default='',
                         help='Base directory of another build to unify with')
     parser.add_argument('manifest', default=None, nargs='?',
                         help='Manifest file name')
@@ -306,22 +312,32 @@ def main():
         args.source, args.unify = sorted([args.source, args.unify],
                                          key=is_native, reverse=True)
         if is_native(args.source):
             launcher.tooldir = args.source
     elif not buildconfig.substs['CROSS_COMPILE']:
         launcher.tooldir = buildconfig.substs['LIBXUL_DIST']
 
     with errors.accumulate():
+        finder_args = dict(
+            minify=args.minify,
+            minify_js=args.minify_js,
+        )
+        if args.js_binary:
+            finder_args['minify_js_verify_command'] = [
+                args.js_binary,
+                os.path.join(os.path.abspath(os.path.dirname(__file__)),
+                    'js-compare-ast.js')
+            ]
         if args.unify:
             finder = UnifiedBuildFinder(FileFinder(args.source),
                                         FileFinder(args.unify),
-                                        minify=args.minify)
+                                        **finder_args)
         else:
-            finder = FileFinder(args.source, minify=args.minify)
+            finder = FileFinder(args.source, **finder_args)
         if 'NO_PKG_FILES' in os.environ:
             sinkformatter = NoPkgFilesRemover(formatter,
                                               args.manifest is not None)
         else:
             sinkformatter = formatter
         sink = SimpleManifestSink(finder, sinkformatter)
         if args.manifest:
             preprocess_manifest(sink, args.manifest, defines)