Issue 6291 - add ManifoldJS packaging for Edge

packagerEdge.py

Index: packagerEdge.py
diff --git a/packagerEdge.py b/packagerEdge.py
index a86ffc45d44c532a24319eb809530a4f956f644d..706467507f3914d41571b2af65fabbc71b2c3c60 100644
--- a/packagerEdge.py
+++ b/packagerEdge.py
@@ -2,25 +2,19 @@
 # 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 base64
-import hashlib
 import json
-import mimetypes
 import os
-import zipfile
+import shutil
+from StringIO import StringIO
+import subprocess
+import tempfile
+from xml.etree import ElementTree
+from zipfile import ZipFile
 
 import packager
 import packagerChrome
 
-# Files and directories expected inside of the .APPX archive.
-MANIFEST = 'AppxManifest.xml'
-CONTENT_TYPES = '[Content_Types].xml'
-BLOCKMAP = 'AppxBlockMap.xml'
-EXTENSION_DIR = 'Extension'
-ASSETS_DIR = 'Assets'
-
-# Size of uncompressed block in the APPX block map.
-BLOCKSIZE = 64 * 1024
+MANIFEST = 'appxmanifest.xml'
 
 defaultLocale = packagerChrome.defaultLocale
 
@@ -29,122 +23,75 @@ def _get_template_for(filename):
     return packager.getTemplate('edge/{}.tmpl'.format(filename))
 
 
-def _lfh_size(filename):
-    """Compute the size of zip local file header for `filename`."""
-    try:
-        filename = filename.encode('utf-8')
-    except UnicodeDecodeError:
-        pass  # filename is already a byte string.
-    return zipfile.sizeFileHeader + len(filename)
-
-
-def _make_blockmap_entry(filename, data):
-    blocks = [data[i:i + BLOCKSIZE] for i in range(0, len(data), BLOCKSIZE)]
-    return {
-        'name': filename.replace('/', '\\'),
-        'size': len(data),
-        'lfh_size': _lfh_size(filename),
-        'blocks': [
-            {'hash': base64.b64encode(hashlib.sha256(block).digest())}
-            for block in blocks
-        ],
-    }
-
-
-def create_appx_blockmap(files):
-    """Create APPX blockmap for the list of files."""
-    # We don't support AppxBlockmap.xml generation for compressed zip files at
-    # the moment. The only way to reliably calculate the compressed size of
-    # each 64k chunk in the zip file is to override the relevant parts of
-    # `zipfile` library. We have chosen to not do it so we produce an
-    # uncompressed zip file that is later repackaged by Windows Store with
-    # compression.
-    template = _get_template_for(BLOCKMAP)
-    files = [_make_blockmap_entry(n, d) for n, d in files.items()]
-    return template.render(files=files).encode('utf-8')
-
-
 def load_translation(files, locale):
     """Load translation strings for locale from files."""
-    path = '{}/_locales/{}/messages.json'.format(EXTENSION_DIR, locale)
+    path = '_locales/{}/messages.json'.format(locale)
     return json.loads(files[path])
 
 
-def get_appx_version(metadata, build_num):
-    """Get the version number for usage in AppxManifest.xml.
-
-    As required by the Windows Store, the returned version string has four
-    components, where the 3rd component is replaced with the build number
-    if available, and the 4th component is always zero (e.g. 1.2.1000.0).
-    """
-    components = metadata.get('general', 'version').split('.')[:3]
-    components.extend(['0'] * (4 - len(components)))
-    if build_num:
-        components[2] = build_num
-    return '.'.join(components)
-
-
-def create_appx_manifest(params, files, build_num, release_build):
-    """Create AppxManifest.xml."""
-    params = dict(params)
-    metadata = params['metadata']
-    w = params['windows_version'] = {}
-    w['min'], w['max'] = metadata.get('compat', 'windows').split('/')
-    params['version'] = get_appx_version(metadata, build_num)
-
-    metadata_suffix = 'release' if release_build else 'devbuild'
-    app_extension_id = 'extension_id_' + metadata_suffix
-    if metadata.has_option('general', app_extension_id):
-        params['app_extension_id'] = metadata.get('general', app_extension_id)
-    else:
-        params['app_extension_id'] = 'EdgeExtension'
+def register_xml_namespaces(manifest_path):
+    """Register namespaces of the given file, in order to preserve defaults."""
+    with open(manifest_path, 'r') as fp:
+        ns = dict([node for _, node in ElementTree.iterparse(
+            fp, events=['start-ns'])])
+    for prefix, uri in ns.items():
+        ElementTree.register_namespace(prefix, uri)
+
+    return ns
+
 
-    params['app_id'] = packager.get_app_id(release_build, metadata)
+def update_appx_manifest(manifest_path, files, metadata, release_build):
+    namespaces = register_xml_namespaces(manifest_path)

[Sebastian Noack, 2018/07/12 14:43:10]

I wonder if this is necessary? What happens if you don't register the namespace?
Also does this cause the schema to be downloaded? (It would be great to avoid
that.)

[tlucas, 2018/07/19 12:30:27]

This is necessary so that xml.etree.ElementTree.write() preserves the file's
actual namespace configuration (otherwise it would rewrite the configured
namespaces to have auto-generated prefixed, e.g. ns0, ns1 and so forth)

To my knowing (and from what i observed), nothing is downloaded.

+
+    def traverse(current_elem, overwrite):
+        if isinstance(overwrite, dict):
+            for key, value in overwrite.items():
+                if isinstance(key, tuple):
+                    prefix, element = key
+                    next_elem = current_elem.find(
+                        '{{{}}}{}'.format(namespaces[prefix], element))
+                    traverse(next_elem, value)
+                else:
+                    current_elem.attrib.update(overwrite)
+        else:
+            current_elem.text = overwrite
 
     translation = load_translation(files, defaultLocale)
     name_key = 'name' if release_build else 'name_devbuild'
-    params['display_name'] = translation[name_key]['message']
-    params['description'] = translation['description']['message']
-
-    for size in ['44', '50', '150']:
-        path = '{}/logo_{}.png'.format(ASSETS_DIR, size)
-        if path not in files:
-            raise KeyError(path + ' is not found in files')
-        params['logo_' + size] = path.replace('/', '\\')
-
-    template = _get_template_for(MANIFEST)
-    return template.render(params).encode('utf-8')
-
-
-def move_files_to_extension(files):
-    """Move all files into `Extension` folder for APPX packaging."""
-    # We sort the files to ensure that 'Extension/xyz' is moved before 'xyz'.
-    # If 'xyz' is moved first, it would overwrite 'Extension/xyz' and its
-    # original content would be lost.
-    names = sorted(files.keys(), key=len, reverse=True)
-    for filename in names:
-        files['{}/{}'.format(EXTENSION_DIR, filename)] = files.pop(filename)
-
-
-def create_content_types_map(filenames):
-    """Create [Content_Types].xml -- a mime type map."""
-    params = {'defaults': {}, 'overrides': {}}
-    overrides = {
-        BLOCKMAP: 'application/vnd.ms-appx.blockmap+xml',
-        MANIFEST: 'application/vnd.ms-appx.manifest+xml',
+    v_min, v_max = metadata.get('compat', 'windows').split('/')
+
+    overwrite = {

[Sebastian Noack, 2018/07/12 14:43:10]

Instead of a dictionaries you might want to use lists:

1. Since you don't do lookups (but merely iterate over the items) anyway.
2. If new nodes are added, they will be in predictable order.

[tlucas, 2018/07/19 12:30:28]

Since order doesn't matter in the resulting XML (or anywhere in between), i
don't see a real benefit from switching to lists - could you please clarify?

[Sebastian Noack, 2018/07/25 19:18:41]

Originally I thought that nodes may be created for the overrides here, but in
fact those nodes and attributes already exist in the output of manifoldJS and
you merely override them without adding new nodes/attributes.

Though, in theory, it could happen in the future that we'll have attributes
listed here that don't exist in the template, and then the order of these
attributes in the resulting XML will be arbitrary like the order of dict keys.

Regardless of this (theoretical) issue, I don't see a point in using a
dictionary, if you never do a single lookup, but always turn it back into a list
in order to iterate over it. Why not using a list in the first place?

Anyway, I am wondering whether we cannot use a fully-qualified XPath expression.
In fact the find() method, you already use in traverse() is expecting an XPath
expression (but you currently merely call it with an element name). Furthermore,
you can pass in the namespaces as second argument to find() so that you don't
have to explicitly look up the namespace prefix, like that:

  overrides = [
      ("ns:name/ns:name", [
          ("attributeName", "attributeValue"),
          ...
      ]),
      ...
  ]

  ...

  for xpath, attributes in overrides:
      for element in doc.findall(xpath, namespaces):
          for attr, value in attributes:
              element.set(attr, value)

The only problem appears to be that the default namespace doesn't seem to get
resolved, so that instead of "Identity" you'd have to write
"{http://schemas.microsoft.com/appx/manifest/foundation/windows10}Identity", or
give a non-empty prefix to the default namespace so that you can write
"x:Identity" for example which isn't great either, but perhaps I overlooked
something, or there might be a workaround.

[tlucas, 2018/08/08 09:35:54]

It looks like adding the default namespaces with an arbitrary identifier is
common practice, (see e.g.
https://stackoverflow.com/questions/34009992/python-elementtree-default-names...
)


Additionally, i decided to use "find" instead of "findall" so that we will
notice in case some of our defined attributes or elements are not supported
anymore (the loop you suggested would just gracefully ignore missing elements) 

+        ('', 'Identity'): {
+            'Name': packager.get_app_id(release_build, metadata),
+            'Publisher': metadata.get('general', 'publisher_id'),
+        },
+        ('', 'Properties'): {
+            ('', 'DisplayName'): translation[name_key]['message'],

[Sebastian Noack, 2018/07/12 14:43:10]

It seems we can just leave the DisplayName alone. ManifoldJS will set it to
ms-resource:DisplayName, and creates the respective resource files. While we
don't need to bother about translating the product name, if we get it for free,
there is no reason to explicitly bypass that mechanism.

[tlucas, 2018/07/19 12:30:27]

True indeed, removed this.

+            ('', 'PublisherDisplayName'): metadata.get('general', 'author'),
+            ('', 'Logo'): 'Assets/logo_50.png',
+        },
+        ('', 'Dependencies'): {
+            ('', 'TargetDeviceFamily'): {
+                'MaxVersionTested': v_max,
+                'MinVersion': v_min,
+            },
+        },
+        ('', 'Applications'): {
+            ('', 'Application'): {
+                ('uap', 'VisualElements'): {

[Sebastian Noack, 2018/07/12 14:43:10]

Perhaps this structure is more readable if keys are always strings (with "{uap}"
prefix if necessary). That way the logic in traverse() can also be simplified.

[tlucas, 2018/07/19 12:30:27]

You may be right about it being (only imho) a bit more readable, but i
personally wouldn't want to trade explicitly being able to distinguish between
elements/children and attributes for this.

[Sebastian Noack, 2018/07/25 19:18:41]

What I meant is using "uap:VisualElements" as key (not
"{http://schemas.microsoft.com/appx/manifest/uap/windows10}VisualElements"),
which would be more readable, IMO.

[tlucas, 2018/08/08 09:35:54]

Done.

+                    'Square150x150Logo': 'Assets/logo_150.png',
+                    'Square44x44Logo': 'Assets/logo_44.png',

[Sebastian Noack, 2018/07/12 14:43:10]

Instead of hard-coding the assets, we could identify them based on the
[appx_assets] section in metadata.edge, reusing the makeIcons() function from
packagerChrome:


  assets = makeIcons(files, filenames)
  overwrite = {
      ...
      ('', 'Properties'): {
          ...
          ('', 'Logo'): assets[50],
      },
      ...
      ('', 'Applications'): {
          ('', 'Application'): {
              ('uap', 'VisualElements'): {
                  'Square150x150Logo': assets[150],
                  'Square44x44Logo': assets[44],
              },
          },
      },
  }

[tlucas, 2018/07/19 12:30:27]

Done.

+                },
+            },
+        },
     }
-    types = mimetypes.MimeTypes()
-    types.add_type('application/octet-stream', '.otf')
-    for filename in filenames:
-        ext = os.path.splitext(filename)[1]
-        if ext:
-            content_type = types.guess_type(filename, strict=False)[0]
-            if content_type is not None:
-                params['defaults'][ext[1:]] = content_type
-        if filename in overrides:
-            params['overrides']['/' + filename] = overrides[filename]
-    content_types_template = _get_template_for(CONTENT_TYPES)
-    return content_types_template.render(params).encode('utf-8')
+
+    tree = ElementTree.parse(manifest_path)
+    root = tree.getroot()
+
+    traverse(root, overwrite)
+
+    tree.write(manifest_path, encoding='utf-8', xml_declaration=True)
 
 
 def createBuild(baseDir, type='edge', outFile=None,  # noqa: preserve API.
@@ -191,16 +138,57 @@ def createBuild(baseDir, type='edge', outFile=None,  # noqa: preserve API.
     if devenv:
         packagerChrome.add_devenv_requirements(files, metadata, params)
 
-    move_files_to_extension(files)
+    zipped = StringIO()
+    files.zip(zipped)
+
+    zipped.seek(0)
+
+    if devenv:
+        shutil.copyfileobj(zipped, outfile)
+        return
+
+    tmp_dir = tempfile.mkdtemp('adblockplus_package')
+    try:
+        src_dir = os.path.join(tmp_dir, 'src')
+        ext_dir = os.path.join(tmp_dir, 'ext')
+
+        with ZipFile(zipped, 'r') as zip_file:
+            zip_file.extractall(src_dir)
+
+        cmd_env = os.environ.copy()
+        cmd_env['SRC_FOLDER'] = src_dir
+        cmd_env['EXT_FOLDER'] = ext_dir
+
+        manifold_folder = os.path.join(ext_dir, 'MSGname', 'edgeextension')
+        manifest_folder = os.path.join(manifold_folder, 'manifest')
+        asset_folder = os.path.join(manifest_folder, 'Assets')
+
+        # prepare the extension with manifoldjs
+        cmd = ['npm', 'run', '--silent', 'build-edge']
+        subprocess.check_call(cmd, env=cmd_env, cwd=os.path.dirname(__file__))
+
+        # update incomplete appxmanifest
+        intermediate_manifest = os.path.join(manifest_folder, MANIFEST)
+        update_appx_manifest(intermediate_manifest, files, metadata,
+                             releaseBuild)
+
+        # cleanup placeholders, copy actual images
+        shutil.rmtree(asset_folder)
+        os.mkdir(asset_folder)
+        if metadata.has_section('appx_assets'):
+            for name, path in metadata.items('appx_assets'):
+                path = os.path.join(baseDir, path)
+                target = os.path.join(asset_folder, name)
+                shutil.copyfile(path, target)
+
+        # package app with manifoldjs
+        cmd = ['npm', 'run', '--silent', 'package-edge']
 
-    if metadata.has_section('appx_assets'):
-        for name, path in metadata.items('appx_assets'):
-            path = os.path.join(baseDir, path)
-            files.read(path, '{}/{}'.format(ASSETS_DIR, name))
+        subprocess.check_call(cmd, env=cmd_env, cwd=os.path.dirname(__file__))
 
-    files[MANIFEST] = create_appx_manifest(params, files,
-                                           buildNum, releaseBuild)
-    files[BLOCKMAP] = create_appx_blockmap(files)
-    files[CONTENT_TYPES] = create_content_types_map(files.keys() + [BLOCKMAP])
+        package = os.path.join(manifold_folder, 'package',
+                               'edgeExtension.appx')
 
-    files.zip(outfile, compression=zipfile.ZIP_STORED)
+        shutil.copyfile(package, outfile)
+    finally:
+        shutil.rmtree(tmp_dir, ignore_errors=True)