Issue 5535 - Replace our module system with webpack

packagerChrome.py

 # 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 io
 import json
 import os
 import re
-import shutil
 from StringIO import StringIO
 import struct
 import subprocess
 import sys
-import tempfile
 
 from ensure_dependencies import read_deps
 from packager import (readMetadata, getDefaultFileName, getBuildVersion,
                       getTemplate, Files)
 
 defaultLocale = 'en_US'
 
 
 def getIgnoredFiles(params):
     return {'store.description'}
@@ skipping 109 matching lines @@
     # Normalize JSON structure
     licenseComment = re.compile(r'/\*.*?\*/', re.S)
     data = json.loads(re.sub(licenseComment, '', manifest, 1))
     if '_dummy' in data:
         del data['_dummy']
     manifest = json.dumps(data, sort_keys=True, indent=2)
 
     return manifest.encode('utf-8')
 
 
+def toJson(data):
+    return json.dumps(
+        data, ensure_ascii=False, sort_keys=True,
+        indent=2, separators=(',', ': ')
+    ).encode('utf-8') + '\n'
+
+
 def create_bundles(params, files):
     base_extension_path = params['baseDir']
     info_templates = {
         'chrome': 'chromeInfo.js.tmpl',
         'edge': 'edgeInfo.js.tmpl',
         'gecko-webext': 'geckoInfo.js.tmpl'
     }
-    info_module = None
 
     # Historically we didn't use relative paths when requiring modules, so in
     # order for webpack to know where to find them we need to pass in a list of
     # resolve paths. Going forward we should always use relative paths, once we
     # do that consistently this can be removed. See issues 5760, 5761 and 5762.
-    resolve_paths = ' '.join(
-        [os.path.join(base_extension_path, dir, 'lib')
-         for dir in ['', 'adblockpluscore', 'adblockplusui']]
-    )
+    resolve_paths = [os.path.join(base_extension_path, dir, 'lib')
+                     for dir in ['', 'adblockpluscore', 'adblockplusui']]
 
-    temp_dir = tempfile.mkdtemp()
-    try:
-        info_module = os.path.join(temp_dir, 'info.js')
-        template = getTemplate(info_templates[params['type']])
-        with open(info_module, 'w') as info_file:
-            info_file.write(
-                template.render(
-                    basename=params['metadata'].get('general', 'basename'),
-                    version=params['metadata'].get('general', 'version')
-                ).encode('utf-8')
-            )
+    info_template = getTemplate(info_templates[params['type']])
+    info_module = info_template.render(
+        basename=params['metadata'].get('general', 'basename'),
+        version=params['metadata'].get('general', 'version')
+    ).encode('utf-8')
 
-        for item in params['metadata'].items('bundles'):
-            name, value = item
-            base_item_path = os.path.dirname(item.source)
+    boundary = '=============================WEBPACK-BOUNDARY'
+    configuration = {
+        'BOUNDARY': boundary,
+        'BUNDLES': [],
+        'EXTENSION_PATH': base_extension_path,
+        'INFO_MODULE': info_module,
+        'RESOLVE_PATHS': resolve_paths,

[Wladimir Palant, 2017/10/09 10:54:54]

Nit: it's a JSON structure, why are properties all-caps?

[kzar, 2017/10/09 13:52:14]

Done.

+    }
 
-            bundle_file = os.path.relpath(os.path.join(base_item_path, name),
-                                          base_extension_path)
-            entry_files = [os.path.join(base_item_path, module_path)
-                           for module_path in value.split()]
-            subprocess.check_call(
-                ['npm', 'run-script', 'webpack', '--silent'],
-                cwd=os.path.dirname(__file__),
-                env={
-                    'EXTENSION_PATH': base_extension_path,
-                    'ENTRY_POINTS': ' '.join(entry_files),
-                    'OUTPUT_PATH': temp_dir,
-                    'BUNDLE_NAME': bundle_file,
-                    'RESOLVE_PATHS': resolve_paths,
-                    'INFO_PATH': info_module,
-                    'PATH': os.environ['PATH']
-                }
-            )
-            for file_name in [bundle_file, bundle_file + '.map']:
-                with open(os.path.join(temp_dir, file_name), 'r') as f:
-                    files[file_name] = f.read()
-    finally:
-        shutil.rmtree(temp_dir)
+    for item in params['metadata'].items('bundles'):
+        name, value = item
+        base_item_path = os.path.dirname(item.source)
 
+        bundle_file = os.path.relpath(os.path.join(base_item_path, name),
+                                      base_extension_path)
+        entry_files = [os.path.join(base_item_path, module_path)
+                       for module_path in value.split()]
+        configuration['BUNDLES'].append({
+            'BUNDLE_NAME': bundle_file,
+            'ENTRY_POINTS': entry_files,
+        })
 
-def toJson(data):
-    return json.dumps(
-        data, ensure_ascii=False, sort_keys=True,
-        indent=2, separators=(',', ': ')
-    ).encode('utf-8') + '\n'
+    output = subprocess.check_output(
+        ['node',
+         os.path.join(os.path.dirname(__file__), 'webpack_runner.js'),
+         toJson(configuration)]

[Wladimir Palant, 2017/10/09 10:54:55]

Passing the configuration as parameter has an ugly side-effect: if the command
fails, the error output is massive due to quoting the command line parameters.
Also, the size of command line parameters might be limited on some platforms
(Windows?). You could use subprocess.communicate() to pass data to the process
via stdin. Note also that info_loader.js doesn't need to parse input separately
- webpack_runner.js could require("./info_loader") and call some method to set
info module source.

[kzar, 2017/10/09 13:52:15]

Done, you're right the error output looks way nicer now. (We could also catch
the subprocess.CalledProcessError, and re-throw with the configuration removed
from the command I suppose. That would be simpler but seems like kind of a
hack.)

+    ).split(boundary)
+    for i in range(0, len(output) - 1, 2):
+        files[output[i].strip()] = output[i + 1].strip()
 
 
 def import_string_webext(data, key, source):
     """Import a single translation from the source dictionary into data"""
     data[key] = source
 
 
 def import_string_gecko(data, key, value):
     """Import Gecko-style locales into data.
 
@@ skipping 215 matching lines @@
                 params, 'testIndex.html.tmpl', ('general', 'testScripts')
             )
 
     zipdata = files.zipToString()
     signature = None
     pubkey = None
     if keyFile != None:
         signature = signBinary(zipdata, keyFile)
         pubkey = getPublicKey(keyFile)
     writePackage(outFile, pubkey, signature, zipdata)