Issue 4028 - Add support for Edge extensions to buildtools

build.py

Index: build.py
===================================================================
--- a/build.py
+++ b/build.py
@@ -7,17 +7,17 @@
 import re
 import subprocess
 import shutil
 import buildtools
 from getopt import getopt, GetoptError
 from StringIO import StringIO
 from zipfile import ZipFile
 
-knownTypes = ('gecko', 'chrome', 'safari', 'generic')
+knownTypes = ('gecko', 'chrome', 'safari', 'generic', 'edge')
 
 
 class Command(object):
     name = property(lambda self: self._name)
     shortDescription = property(
         lambda self: self._shortDescription,
         lambda self, value: self.__dict__.update({'_shortDescription': value})
     )
@@ -171,50 +171,43 @@
             'name': command.name,
             'params': command.params,
             'description': description,
             'options': '\n'.join(options)
         }
 
 
 def runBuild(baseDir, scriptName, opts, args, type):
-    locales = None
-    buildNum = None
-    multicompartment = False
-    releaseBuild = False
-    keyFile = None
+    kwargs = {}
     for option, value in opts:
-        if option in ('-l', '--locales'):
-            locales = value.split(',')
+        if option in ('-l', '--locales') and type == 'gecko':

[Sebastian Noack, 2016/10/04 19:58:04]

While refactoring this code anyway, this should be a set rather than a tuple,
same below.

[Vasily Kuznetsov, 2016/10/11 15:58:27]

Done.

+            kwargs['locales'] = value.split(',')
         elif option in ('-b', '--build'):
-            buildNum = value
-            if type != 'gecko' and not re.search(r'^\d+$', buildNum):
+            kwargs['buildNum'] = value
+            if type != 'gecko' and not kwargs['buildNum'].isdigit():
                 raise TypeError('Build number must be numerical')
         elif option in ('-k', '--key'):
-            keyFile = value
-        elif option in ('-m', '--multi-compartment'):
-            multicompartment = True
+            kwargs['keyFile'] = value
+        elif option in ('-m', '--multi-compartment') and type == 'gecko':
+            kwargs['multicompartment'] = True
         elif option in ('-r', '--release'):
-            releaseBuild = True
-    outFile = args[0] if len(args) > 0 else None
+            kwargs['releaseBuild'] = True
+    if len(args) > 0:
+        kwargs['outFile'] = args[0]
 
     if type == 'gecko':
         import buildtools.packagerGecko as packager
-        packager.createBuild(baseDir, type=type, outFile=outFile,
-                             locales=locales, buildNum=buildNum,
-                             releaseBuild=releaseBuild,
-                             multicompartment=multicompartment)
     elif type == 'chrome':
         import buildtools.packagerChrome as packager
-        packager.createBuild(baseDir, type=type, outFile=outFile, buildNum=buildNum,
-                             releaseBuild=releaseBuild, keyFile=keyFile)
     elif type == 'safari':
         import buildtools.packagerSafari as packager
-        packager.createBuild(baseDir, type=type, outFile=outFile, buildNum=buildNum,
-                             releaseBuild=releaseBuild, keyFile=keyFile)
+    elif type == 'edge':
+        import buildtools.packagerEdge as packager
+
+    packager.createBuild(baseDir, type=type, **kwargs)

[Sebastian Noack, 2016/10/04 19:58:05]

Since you are apparently changing the calling code anyway, how about renaming
createBuild() to create_build() in existing modules, rather than adding #noqa in
the new code?

[Vasily Kuznetsov, 2016/10/11 15:58:27]

That's a lot of unrelated changes to pull in. Also `releaseAutomation.py` calls
`createBuild` and god knows what else outside of buildtools. I would prefer to
make a separate commit for these kinds of renames.

[Sebastian Noack, 2016/10/12 11:07:48]

I was only talking about packagerEdge.create_build() which isn't used outside of
buildtools yet, since it doesn't exist before these changes. Sure, this would be
inconsistent (for now) but might be better than introducing new names using
camalcase in new code.

[Vasily Kuznetsov, 2016/10/12 14:13:42]

I tried doing it, but unfortunately we still can't remove the #noqa. There are
two other naming issues in the definition of `packagerEdge.createBuild`: one
argument is called `type` (that redefines a builtin) and all other arguments are
in camelcase. The first issue is easy to address by removing `type` from the
signature and adding a `**kw` (ugly but works) but the second one would require
either putting just `*args, **kw` for arguments and then reconstructing them
manually or calling `packagerEdge.create_build` in a different way from all
other `createBuild` functions. I don't like either of these options so this
leaves us with the choice between renaming `packagerEdge.createBuild` to
`create_build` and still having the #noqa or just leaving things as they are. It
seems like renaming is not really worth it. What do you think?

[Sebastian Noack, 2016/10/12 14:43:26]

I guess, let's leave it as is for now. We still should probably change the
calling convention for all packagers in one go with a different issue.

[Vasily Kuznetsov, 2016/10/13 11:56:35]

Fully agree.

 
 
 def runAutoInstall(baseDir, scriptName, opts, args, type):
     if len(args) == 0:
         print 'Port of the Extension Auto-Installer needs to be specified'
         usage(scriptName, type, 'autoinstall')
         return
 
@@ -469,17 +462,17 @@
     command.shortDescription = 'Create a build'
     command.description = 'Creates an extension build with given file name. If output_file is missing a default name will be chosen.'
     command.params = '[options] [output_file]'
     command.addOption('Only include the given locales (if omitted: all locales not marked as incomplete)', short='l', long='locales', value='l1,l2,l3', types=('gecko'))
     command.addOption('Use given build number (if omitted the build number will be retrieved from Mercurial)', short='b', long='build', value='num')
     command.addOption('File containing private key and certificates required to sign the package', short='k', long='key', value='file', types=('chrome', 'safari'))
     command.addOption('Create a build for leak testing', short='m', long='multi-compartment', types=('gecko'))
     command.addOption('Create a release build', short='r', long='release')
-    command.supportedTypes = ('gecko', 'chrome', 'safari')
+    command.supportedTypes = ('gecko', 'chrome', 'safari', 'edge')
 
 with addCommand(runAutoInstall, 'autoinstall') as command:
     command.shortDescription = 'Install extension automatically'
     command.description = 'Will automatically install the extension in a browser running Extension Auto-Installer. If host parameter is omitted assumes that the browser runs on localhost.'
     command.params = '[<host>:]<port>'
     command.addOption('Create a build for leak testing', short='m', long='multi-compartment')
     command.supportedTypes = ('gecko')