Issue 4719 - Edge packager does not generate an info module

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 153 matching lines @@
         packagerChrome.convertJS(params, files)
 
     if metadata.has_section('preprocess'):
         files.preprocess(metadata.options('preprocess'), {'needsExt': True})
 
     if metadata.has_section('import_locales'):
         packagerChrome.importGeckoLocales(params, files)
 
     files['manifest.json'] = packagerChrome.createManifest(params, files)
 
-    if (metadata.has_option('general', 'backgroundScripts') and
-        'lib/info.js' in metadata.get('general', 'backgroundScripts').split()

[kzar, 2016/12/13 09:19:22]

IMO this looks kind of ugly, I don't like how the second clause does not end
with the "and". I'll leave it up to Vasily and Sebastian to decide anyway, (FWIW
I checked and Tox does pass.)

[Vasily Kuznetsov, 2016/12/13 18:49:58]

I would have computed the background scripts list beforehand to make the
condition shorter and more readable. Maybe something like this:

if metadata.has_option('general', 'backgroundScripts'):
    bg_scripts = metadata.get('general', 'backgroundScripts').split()
    if 'lib/info.js' in bg_scripts and 'lib/info.js' not in files:
        # yada yada yada

Not a big deal though, LGTM from me anyway.

[Sebastian Noack, 2016/12/15 13:58:06]

I agree with Vasily here. Also, is it even necessery to check for |'lib/info.js'
in files|? On the other hand, the code is consistent with what we have in
packagerChrome.py. So if we change it here we should also change it their for
consistency. No strong opinion from my side.

Also, we probably should adapt the tests to cover this code path and make sure
that the file got added.

[Oleksandr, 2016/12/22 02:53:24]

I think touching packagerChrome.py would require a separate issue. Omitting that
here.
Done.

-            and 'lib/info.js' not in files):
-        files['lib/info.js'] = packagerChrome.createInfoModule(params)
+    if devenv:
+        import buildtools
+        import random
+        files.read(os.path.join(buildtools.__path__[0],
+                   'chromeDevenvPoller__.js'), relpath='devenvPoller__.js')
+        files['devenvVersion__'] = str(random.random())
+
+    if metadata.has_option('general', 'backgroundScripts'):
+        bg_scripts = metadata.get('general', 'backgroundScripts').split()
+        if 'lib/info.js' in bg_scripts and 'lib/info.js' not in files:
+            files['lib/info.js'] = packagerChrome.createInfoModule(params)
 
     move_files_to_extension(files)
 
     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)