Noissue - Use sets in build.py were appropriate

build.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 os
 import sys
 import re
 import subprocess
 import shutil
 from getopt import getopt, GetoptError
@@ skipping 316 matching lines @@
 
     # JSDoc struggles wih huge objects: https://github.com/jsdoc3/jsdoc/issues/976
     if type == 'chrome':
         sources = [os.path.join(source_dir, filename) for filename in os.listdir(source_dir) if filename != 'publicSuffixList.js']
 
     buildtools_path = os.path.dirname(__file__)
     config = os.path.join(buildtools_path, 'jsdoc.conf')
 
     command = ['npm', 'run-script', 'jsdoc', '--', '--destination', targetDir,
                '--configure', config] + sources
-    if any(opt in ('-q', '--quiet') for opt, _ in opts):
+    if any(opt in {'-q', '--quiet'} for opt, _ in opts):
         process = subprocess.Popen(command, stdout=subprocess.PIPE,
                                    stderr=subprocess.PIPE, cwd=buildtools_path)
         stderr = process.communicate()[1]
         retcode = process.poll()
         if retcode:
             sys.stderr.write(stderr)
             raise subprocess.CalledProcessError(command, retcode)
     else:
         subprocess.check_call(command, cwd=buildtools_path)
 
 
 def runReleaseAutomation(baseDir, scriptName, opts, args, type):
     keyFile = None
     downloadsRepo = os.path.join(baseDir, '..', 'downloads')
     for option, value in opts:
-        if option in ('-k', '--key'):
+        if option in {'-k', '--key'}:
             keyFile = value
-        elif option in ('-d', '--downloads'):
+        elif option in {'-d', '--downloads'}:
             downloadsRepo = value
 
     if len(args) == 0:
         print 'No version number specified for the release'
         usage(scriptName, type, 'release')
         return
     version = args[0]
     if re.search(r'[^\d\.]', version):
         print 'Wrong version number format'
         usage(scriptName, type, 'release')
@@ skipping 14 matching lines @@
 
 
 with addCommand(lambda baseDir, scriptName, opts, args, type: usage(scriptName, type), ('help', '-h', '--help')) as command:
     command.shortDescription = 'Show this message'
 
 with addCommand(runBuild, 'build') as command:
     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('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',))
+    command.addOption('File containing private key and certificates required to sign the package', short='k', long='key', value='file', types={'chrome'})
     command.addOption('Create a release build', short='r', long='release')
-    command.supportedTypes = ('gecko-webext', 'chrome', 'edge')

[Sebastian Noack, 2017/10/10 18:34:28]

Can we please turn these into sets as well?

[tlucas, 2017/10/11 13:21:15]

Done.

+    command.supportedTypes = {'gecko-webext', 'chrome', 'edge'}
 
 with addCommand(createDevEnv, 'devenv') as command:
     command.shortDescription = 'Set up a development environment'
     command.description = 'Will set up or update the devenv folder as an unpacked extension folder for development.'
-    command.supportedTypes = ('gecko-webext', 'chrome')
+    command.supportedTypes = {'gecko-webext', 'chrome'}
 
 with addCommand(setupTranslations, 'setuptrans') as command:
     command.shortDescription = 'Sets up translation languages'
     command.description = 'Sets up translation languages for the project on crowdin.net.'
     command.params = '[options] project-key'
 
 with addCommand(updateTranslationMaster, 'translate') as command:
     command.shortDescription = 'Updates translation master files'
     command.description = 'Updates the translation master files in the project on crowdin.net.'
     command.params = '[options] project-key'
 
 with addCommand(uploadTranslations, 'uploadtrans') as command:
     command.shortDescription = 'Uploads existing translations'
     command.description = 'Uploads already existing translations to the project on crowdin.net.'
     command.params = '[options] project-key'
 
 with addCommand(getTranslations, 'gettranslations') as command:
     command.shortDescription = 'Downloads translation updates'
     command.description = 'Downloads updated translations from crowdin.net.'
     command.params = '[options] project-key'
 
 with addCommand(generateDocs, 'docs') as command:
     command.shortDescription = 'Generate documentation (requires node.js)'
     command.description = ('Generate documentation files and write them into '
                            'the specified directory.')
     command.addOption('Suppress JsDoc output', short='q', long='quiet')
     command.params = '[options] <directory>'
-    command.supportedTypes = ('chrome',)
+    command.supportedTypes = {'chrome'}
 
 with addCommand(runReleaseAutomation, 'release') as command:
     command.shortDescription = 'Run release automation'
     command.description = 'Note: If you are not the project owner then you '        "probably don't want to run this!\n\n"        'Runs release automation: creates downloads for the new version, tags '        'source code repository as well as downloads and buildtools repository.'
-    command.addOption('File containing private key and certificates required to sign the release.', short='k', long='key', value='file', types=('chrome', 'edge'))
+    command.addOption('File containing private key and certificates required to sign the release.', short='k', long='key', value='file', types={'chrome', 'edge'})

[tlucas, 2017/10/11 13:21:15]

These are handled in the same manner as supportedTypes for commands, i figured
changing them as well would be logical.

     command.addOption('Directory containing downloads repository (if omitted ../downloads is assumed)', short='d', long='downloads', value='dir')
     command.params = '[options] <version>'
-    command.supportedTypes = ('chrome', 'edge')
+    command.supportedTypes = {'chrome', 'edge'}
 
 with addCommand(updatePSL, 'updatepsl') as command:
     command.shortDescription = 'Updates Public Suffix List'
     command.description = 'Downloads Public Suffix List (see http://publicsuffix.org/) and generates lib/publicSuffixList.js from it.'
-    command.supportedTypes = ('chrome',)
+    command.supportedTypes = {'chrome'}
 
 
 def getType(baseDir, scriptName, args):
     # Look for an explicit type parameter (has to be the first parameter)
     if len(args) >= 2 and args[0] == '-t':
         type = args[1]
         del args[1]
         del args[0]
         if type not in knownTypes:
             print '''
@@ skipping 12 matching lines @@
         return types[0]
     elif len(types) > 1:
         print '''
 Ambiguous repository type, please specify -t parameter explicitly, e.g.
 %s -t %s build
 ''' % (scriptName, types[0])
         return None
     else:
         print '''
 No metadata file found in this repository, a metadata file like
-metadata.%s is required.
-''' % knownTypes[0]

[kzar, 2017/10/10 13:45:59]

What about this, won't this fail?

[Sebastian Noack, 2017/10/10 18:34:28]

Well spotted, it would fail. I suggest to just change the messsage like this:

  No metadata file found in this repository, a metadata.* is required.

[tlucas, 2017/10/11 13:21:15]

Done.

+metadata.* is required.
+'''
         return None
 
 
 def processArgs(baseDir, args):
     global commands
 
     scriptName = os.path.basename(args[0])
     args = args[1:]
     type = getType(baseDir, scriptName, args)
     if type == None:
@@ skipping 10 matching lines @@
     command = args[0]
     if command in commands:
         if commands[command].isSupported(type):
             try:
                 opts, args = commands[command].parseArgs(type, args[1:])
             except GetoptError as e:
                 print str(e)
                 usage(scriptName, type, command)
                 sys.exit(2)
             for option, value in opts:
-                if option in ('-h', '--help'):
+                if option in {'-h', '--help'}:
                     usage(scriptName, type, command)
                     sys.exit()
             commands[command](baseDir, scriptName, opts, args, type)
         else:
             print 'Command %s is not supported for this application type' % command
             usage(scriptName, type)
     else:
         print 'Command %s is unrecognized' % command
         usage(scriptName, type)