Bug 903149 - Part 3: Support for minifying packaged JavaScript; r=glandium

build/mach_bootstrap.py

@@ -31,6 +31,7 @@ SEARCH_PATHS = [
     'python/mozversioncontrol',
     'python/blessings',
     'python/configobj',
+    'python/jsmin',
     'python/psutil',
     'python/which',
     'build/pymake',

python/mozbuild/mozpack/files.py

@@ -5,9 +5,9 @@
 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
@@ -28,7 +28,11 @@ from mozpack.errors import (
 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):
@@ -594,15 +598,76 @@ class MinifiedProperties(BaseFile):
                                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
+        Initializes the instance with a reference base directory.
-        optional minify argument specifies whether file types supporting
+
-        minification (currently only "*.properties") should be minified.
+        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):
         '''
@@ -644,11 +709,16 @@ class BaseFinder(object):
         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 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
 
 

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]))

python/mozbuild/mozpack/test/test_files.py

@@ -15,6 +15,7 @@ from mozpack.files import (
     GeneratedFile,
     JarFinder,
     ManifestFile,
+    MinifiedJavaScript,
     MinifiedProperties,
     PreprocessedFile,
     XPTFile,
@@ -35,6 +36,7 @@ import mozunit
 import os
 import random
 import string
+import sys
 import mozpack.path
 from tempfile import mkdtemp
 from io import BytesIO
@@ -753,6 +755,49 @@ class TestMinifiedProperties(TestWithTmpDir):
                          ['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')

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);

toolkit/mozapps/installer/packager.mk

@@ -705,6 +705,16 @@ 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.
 
@@ -716,6 +726,9 @@ stage-package: $(MOZ_PKG_MANIFEST)
 		$(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)) \

toolkit/mozapps/installer/packager.py

@@ -248,6 +248,12 @@ def main():
                         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,
@@ -311,12 +317,22 @@ def main():
         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)