Issue 6291 - add ManifoldJS packaging for Edge

packagerEdge.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 json
 import os
 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'
-EXTENSION_DIR = 'Extension'
 ASSETS_DIR = 'Assets'
-
-# Size of uncompressed block in the APPX block map.
-BLOCKSIZE = 64 * 1024
 
 defaultLocale = packagerChrome.defaultLocale
 
 
 def _get_template_for(filename):
     return packager.getTemplate('edge/{}.tmpl'.format(filename))
 
 
-def load_translation(files, locale):
-    """Load translation strings for locale from files."""
-    path = '{}/_locales/{}/messages.json'.format(EXTENSION_DIR, locale)
-    return json.loads(files[path])
+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)
+
+    # Make the default namespace available in an xpath expression
+    ns['_d'] = ns['']
+
+    return ns
 
 
-def get_appx_version(metadata, build_num):
-    """Get the version number for usage in AppxManifest.xml.
+def update_appx_manifest(manifest_path, base_dir, files, metadata,
+                         release_build):
+    namespaces = register_xml_namespaces(manifest_path)
 
-    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)
+    v_min, v_max = metadata.get('compat', 'windows').split('/')
 
+    filenames = []
 
-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)
+    for name, path in metadata.items('appx_assets'):
+        path = os.path.join(base_dir, path)
+        icon_path = '{}/{}'.format(ASSETS_DIR, name)
 
-    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'
+        files.read(path, icon_path)
+        filenames.append(icon_path)
 
-    params['app_id'] = packager.get_app_id(release_build, metadata)
+    assets = packagerChrome.makeIcons(files, filenames)
 
-    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']
+    author = metadata.get('general', 'author')
 
-    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('/', '\\')
+    overrides = [
+        ('_d:Identity', None, [
+            ('Name', packager.get_app_id(release_build, metadata)),
+            ('Publisher', metadata.get('general', 'publisher_id')),
+        ]),
+        ('_d:Properties/_d:PublisherDisplayName', author, []),
+        ('_d:Properties/_d:Logo', assets[50], []),
+        ('_d:Dependencies/_d:TargetDeviceFamily', None, [
+            ('MaxVersionTested', v_max),
+            ('MinVersion', v_min),
+        ]),
+        ('_d:Applications/_d:Application/uap:VisualElements', None, [
+            ('Square150x150Logo', assets[150]),
+            ('Square44x44Logo', assets[44]),
+        ]),
+    ]
 
-    template = _get_template_for(MANIFEST)
-    return template.render(params).encode('utf-8')
+    tree = ElementTree.parse(manifest_path)
+    root = tree.getroot()
 
+    for xpath, text, attributes in overrides:
+        element = root.find(xpath, namespaces)
+        if text:
+            element.text = text
+        for attr, value in attributes:
+                element.set(attr, value)
 
-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)
+    tree.write(manifest_path, encoding='utf-8', xml_declaration=True)
 
 
 def createBuild(baseDir, type='edge', outFile=None,  # noqa: preserve API.
                 buildNum=None, releaseBuild=False, keyFile=None,
                 devenv=False):
 
     metadata = packager.readMetadata(baseDir, type)
     version = packager.getBuildVersion(baseDir, metadata, releaseBuild,
                                        buildNum)
 
@@ skipping 26 matching lines @@
         files.preprocess(metadata.options('preprocess'), {'needsExt': True})
 
     if metadata.has_section('import_locales'):
         packagerChrome.import_locales(params, files)
 
     files['manifest.json'] = packagerChrome.createManifest(params, files)
 
     if devenv:
         packagerChrome.add_devenv_requirements(files, metadata, params)
 
-    move_files_to_extension(files)

[Sebastian Noack, 2018/07/11 16:26:47]

Since we skip the packaging for devenv now, there is no reason to move files to
the "Extension" sub-folder anymore in devenv mode. Neither do we have to create
the "Assets" folder (below) for devenv.

[tlucas, 2018/07/12 09:31:54]

Turns out this is not necessary anymore at all - removed this alltogether.

-
-    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))
-
-    if not devenv:

[tlucas, 2018/07/11 12:18:03]

It turned out that devenvs for Edge don't need the AppxManifest.xml file to be
present, same applies to AppxBlockMap.xml and [Content_Types].xml (see changes
in test_packagerWebExt.py) @Ollie, i verified that on my own Windows-PC, i'd
appreciate another verification though.

[Oleksandr, 2018/07/11 14:23:46]

I can confirm that for devenv only the `Extension` part is used. All the other
stuff is redundant.

-        files[MANIFEST] = create_appx_manifest(params, files,

[Sebastian Noack, 2018/07/11 16:26:47]

As discussed before, I would not override the APPX manifest, but rather parse
the one generated by ManifoldJS, and merely add the fields it cannot generate
automatically. This seems to be the workflow suggested in the documentation, and
will be more robust against future changes by Microsoft.

[tlucas, 2018/07/12 09:31:55]

Done (new function update_appx_manifest)

-                                               buildNum, releaseBuild)
-
     zipped = StringIO()
     files.zip(zipped)
 
     zipped.seek(0)
 
     if devenv:
         shutil.copyfileobj(zipped, outfile)

[Sebastian Noack, 2018/07/11 16:26:46]

Don't we have to extract the contents for the devenv?

[tlucas, 2018/07/12 09:31:54]

Yes - but that's done in build.py, with all 3 extensions packed before.

         return
 
     tmp_dir = tempfile.mkdtemp('adblockplus_package')
-    src_dir = os.path.join(tmp_dir, 'src')
-    ext_dir = os.path.join(tmp_dir, 'ext')
+    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)
+        with ZipFile(zipped, 'r') as zip_file:
+            zip_file.extractall(src_dir)
 
-    try:

[Sebastian Noack, 2018/07/11 16:26:46]

This try-block should probably start right after the call to mkdtemp().

[tlucas, 2018/07/12 09:31:54]

Done.

         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')
+        asset_folder = os.path.join(manifest_folder, ASSETS_DIR)
 
         # prepare the extension with manifoldjs
         cmd = ['npm', 'run', '--silent', 'build-edge']
         subprocess.check_call(cmd, env=cmd_env, cwd=os.path.dirname(__file__))
 
-        # overwrite incomplete appxmanifest
+        # update incomplete appxmanifest
         intermediate_manifest = os.path.join(manifest_folder, MANIFEST)
-        shutil.copyfile(os.path.join(src_dir, MANIFEST), intermediate_manifest)
+        update_appx_manifest(intermediate_manifest, baseDir, 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']
 
         subprocess.check_call(cmd, env=cmd_env, cwd=os.path.dirname(__file__))
 
         package = os.path.join(manifold_folder, 'package',
                                'edgeExtension.appx')
 
-        if isinstance(outfile, basestring):
-            shutil.copyfile(package, outfile)
-        else:
-            with open(package, 'rb') as fp:

[Sebastian Noack, 2018/07/11 16:26:46]

This is essentially dead code now, since only in case of devenv, a file-like
object is passed in. So I wonder whether we should just assert that assumption,
rather than having dead/untested code in here?

  assert isinstance(outfile, basestring)
  shutil.copyfile(package, outfile)

[tlucas, 2018/07/12 09:31:55]

That's not an assumption, at this point 'outfile' will definitely be a string.

Removed dead code accordingly.

-                shutil.copyfileobj(fp, outfile)
+        shutil.copyfile(package, outfile)
     finally:
         shutil.rmtree(tmp_dir, ignore_errors=True)