Issue 4865 - Make sure Identity.Name attribute is different for devbuilds 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 base64
 import hashlib
 import json
 import mimetypes
 import os
 import zipfile
@@ skipping 67 matching lines @@
 
 def create_appx_manifest(params, files, release_build=False):
     """Create AppxManifest.xml."""
     params = dict(params)
     metadata = params['metadata']
     w = params['windows_version'] = {}
     w['min'], w['max'] = metadata.get('compat', 'windows').split('/')
     params.update(metadata.items('general'))
     params['version'] = pad_version(params['version'])
 
-    metadata_id_suffix = 'release' if release_build else 'devbuild'
-    metadata_id = 'extension_id_' + metadata_id_suffix
-    if metadata.has_option('general', metadata_id):
-        params['app_extension_id'] = metadata.get('general', metadata_id)
+    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'
 
-    if not release_build:
-        params['app_id'] += 'developmentbuild'

[Sebastian Noack, 2017/02/07 14:44:38]

During the review of the initial implementation we discussed whether we should
automatically generate the app_id, as it is essentially '{}.{}'.format(author,
name).replace(' ', ''), and we already have the "author" and "name" configured
separately. In the end Vasily convinced me that configuring the app_id in the
metadata file is preferable over code that derives the app_id automatically.

Later, when it turned out that we need a different app_extension_id (not to be
confused with app_id) for the Adblock Plus release and development builds,
hard-coding that logic in the packager like this patch does, got rejected in
favor of having separate options in the metadata file for the release and
development builds.

Both decisions, and the resulting implementation, is inconsistent with the
change suggested here. Instead, we should either properly derive the app_id from
the extensions's author and name, or replace app_id with app_id_release and
app_id_devbuild (as we did for app_extension_id_*).

Vasily, Winsley; since you worked on this before, what do you think?

[Vasily Kuznetsov, 2017/02/07 14:57:41]

I prefer the second option. Constructing ids from other variables seems to give
a wrong idea of what it is. Since app_id links the package to its entry in the
Windows store (unless I'm misremembering the context) I think it's more logical
to set it manually. That was also the reason why I preferred that approach when
this code was initially written.

[Oleksandr, 2017/02/08 06:56:55]

Done.

+    app_id = 'app_id_' + metadata_suffix
+    params['app_id'] = metadata.get('general', app_id)
 
     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')
@@ skipping 84 matching lines @@
     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))
 
     files[MANIFEST] = create_appx_manifest(params, files, releaseBuild)
     files[BLOCKMAP] = create_appx_blockmap(files)
     files[CONTENT_TYPES] = create_content_types_map(files.keys() + [BLOCKMAP])
 
     files.zip(outfile, compression=zipfile.ZIP_STORED)