Issue 6371 - Update buildtools dep. to c830dfa08e2f, use AMO-signing API

sitescripts/extensions/bin/createNightlies.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-present eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
@@ skipping 330 matching lines @@
             except:
                 # clear broken output if any
                 if os.path.exists(self.path):
                     os.remove(self.path)
                 raise
         else:
             env = os.environ
             spiderMonkeyBinary = self.config.spiderMonkeyBinary
             if spiderMonkeyBinary:
                 env = dict(env, SPIDERMONKEY_BINARY=spiderMonkeyBinary)
+
+            command = [os.path.join(self.tempdir, 'build.py')]
             if self.config.type == 'safari':

[tlucas, 2018/03/07 22:14:19]

Fully adapting to #6021 here (since the 'safari'-target does not yet have it
included)

-                cmd_order = ['-t', self.config.type, 'build']
+                command.extend(['-t', self.config.type, 'build'])
             else:
-                cmd_order = ['build', '-t', self.config.type]
-            command = [os.path.join(self.tempdir, 'build.py')] + cmd_order
-            command += ['-b', self.buildNum]
+                command.extend(['build', '-t', self.config.type])
+            command.extend(['-b', self.buildNum])
 
             if self.config.type not in {'gecko', 'edge'}:
                 command.extend(['-k', self.config.keyFile])
             command.append(self.path)
             subprocess.check_call(command, env=env)
 
         if not os.path.exists(self.path):
             raise Exception("Build failed, output file hasn't been created")
 
         if self.config.type not in self.downloadable_repos:
             linkPath = os.path.join(baseDir,
-                                    '00latest%s' % self.config.packageSuffix)
+                                    '00latest' + self.config.packageSuffix)
             self.symlink_or_copy(self.path, linkPath)
 
     def retireBuilds(self):
         """
           removes outdated builds, returns the sorted version numbers of remaining
           builds
         """
         baseDir = os.path.join(self.config.nightliesDirectory, self.basename)
         versions = []
         prefix = self.basename + '-'
@@ skipping 33 matching lines @@
                 'download': packageFile,
                 'mtime': os.path.getmtime(os.path.join(baseDir, packageFile)),
                 'size': os.path.getsize(os.path.join(baseDir, packageFile))
             }
             if os.path.exists(os.path.join(baseDir, changelogFile)):
                 link['changelog'] = changelogFile
             links.append(link)
         template = get_template(get_config().get('extensions', 'nightlyIndexPage'))
         template.stream({'config': self.config, 'links': links}).dump(outputPath)
 
-    def open_downloads_loackfile(self):

[Sebastian Noack, 2018/03/08 02:39:21]

Shouldn't this be "read" rather than "open"?

[Sebastian Noack, 2018/03/08 02:39:21]

Typo: loackfile => lockfile (or lock_file)

[tlucas, 2018/03/08 09:23:04]

Yes, you're right - Done.

+    def read_downloads_lockfile(self):
         path = get_config().get('extensions', 'downloadLockFile')
-        current = {}

[Sebastian Noack, 2018/03/08 02:39:22]

This can go into the except block.

[tlucas, 2018/03/08 09:23:05]

Done.

[Sebastian Noack, 2018/03/09 00:33:18]

You put it in the try-block, not in the except-block.

[tlucas, 2018/03/09 08:12:57]

Done.

         try:
             with open(path, 'r') as fp:
                 current = json.load(fp)
         except IOError:
             logging.warning('No lockfile found. Creating ' + path)
+            current = {}
 
         return current
 
-    def write_download_lockfile(self, values):
+    def write_downloads_lockfile(self, values):
         path = get_config().get('extensions', 'downloadLockFile')
         with open(path, 'w') as fp:
             json.dump(values, fp)
 
     def add_to_downloads_lockfile(self, platform, values):
-        current = self.open_downloads_loackfile()
+        current = self.read_downloads_lockfile()
 
         current.setdefault(platform, [])
         current[platform].append(values)
 
-        self.write_download_lockfile(current)
+        self.write_downloads_lockfile(current)
 
     def remove_from_downloads_lockfile(self, platform, filter_key,
                                        filter_value):
-        current = self.open_downloads_loackfile()
+        current = self.read_downloads_lockfile()
         try:
             for i, entry in enumerate(current[platform]):
                 if entry[filter_key] == filter_value:
                     del current[platform][i]
                 if len(current[platform]) == 0:
-                    current.pop(platform)

[Sebastian Noack, 2018/03/08 02:39:22]

You can use the del statement. We don't need it's value (returned by pop).

[tlucas, 2018/03/08 09:23:05]

Done.

+                    del current[platform]
         except KeyError:
             pass
-        self.write_download_lockfile(current)
+        self.write_downloads_lockfile(current)
 
     def generate_jwt_request(self, issuer, secret, url, method, data=None,
                              add_headers=[]):
         header = {
             'alg': 'HS256',     # HMAC-SHA256
             'typ': 'JWT',
         }
 
         issued = int(time.time())
         payload = {
@@ skipping 36 matching lines @@
                     'application/x-xpinstall'
                 )
             })
 
         request = self.generate_jwt_request(
             config.get('extensions', 'amo_key'),
             config.get('extensions', 'amo_secret'),
             upload_url,
             'PUT',
             data,
-            (('Content-Type', content_type),),
+            [('Content-Type', content_type)]
         )
 
         try:
             urllib2.urlopen(request).close()
         except urllib2.HTTPError as e:
             try:
                 logging.error(e.read())
             finally:
                 e.close()
             raise
@@ skipping 10 matching lines @@
 
     def download_from_mozilla_addons(self, buildtype, version, app_id):
         config = get_config()
         iss = config.get('extensions', 'amo_key')
         secret = config.get('extensions', 'amo_secret')
 
         url = ('https://addons.mozilla.org/api/v3/addons/{}/'
                'versions/{}/').format(app_id, version)
 
         request = self.generate_jwt_request(iss, secret, url, 'GET')
-

[Sebastian Noack, 2018/03/08 02:39:22]

Nit: This blank line seems redundant.

[tlucas, 2018/03/08 09:23:05]

Done.

         response = json.load(urllib2.urlopen(request))
 
         necessary = ['passed_review', 'reviewed', 'processed', 'valid']
         if all(response[x] for x in necessary):
             download_url = response['files'][0]['download_url']
             checksum = response['files'][0]['hash']
 
-            basename = self.basename
             filename = '{}-{}.xpi'.format(self.basename, version)
             file_path = os.path.join(
-                config.get('extensions', 'nightliesDirectory'), basename)

[Sebastian Noack, 2018/03/08 02:39:22]

Nit: When wrapping right after opening parenthesis, please put closing
parenthesis in the same column where the line with the opening parentheses
start. This is easier to read.

  file_path = os.path.join(
      config.get('extensions', 'nightliesDirectory'), basename
  )

Same below.

[tlucas, 2018/03/08 09:23:04]

Done.

-            file_path = os.path.join(file_path, filename)
+                config.get('extensions', 'nightliesDirectory'),
+                self.basename,
+                filename
+            )
 
             request = self.generate_jwt_request(iss, secret, download_url,
                                                 'GET')
             try:
                 response = urllib2.urlopen(request)
             except urllib2.HTTPError as e:
                 logging.error(e.read())
 
             # Verify the extension's integrity
             file_content = response.read()
-            sha256 = hashlib.sha256()
-            sha256.update(file_content)

[Sebastian Noack, 2018/03/08 02:39:22]

You can pass the data to the sha256 constructor. No need for a separate call to
update().

[tlucas, 2018/03/08 09:23:04]

Done.

+            sha256 = hashlib.sha256(file_content)
             returned_checksum = '{}:{}'.format(sha256.name, sha256.hexdigest())
 
             if returned_checksum != checksum:
                 logging.error('Checksum could not be verified: {} vs {}'
                               ''.format(checksum, returned_checksum))
 
             with open(file_path, 'w') as fp:
                 fp.write(file_content)
 
             self.update_link = os.path.join(
-                config.get('extensions', 'nightliesURL'), self.basename,

[Sebastian Noack, 2018/03/08 02:39:22]

Nit: This is easier to read if you put each argument on a separate line.

[tlucas, 2018/03/08 09:23:04]

Done.

-                filename)
+                config.get('extensions', 'nightliesURL'),
+                self.basename,
+                filename
+            )
 
             self.remove_from_downloads_lockfile(self.config.type,
                                                 'version',
                                                 version)
         elif not response['passed_review'] or not response['valid']:
             # When the review failed for any reason, we want to know about it
             logging.error(json.dumps(response, indent=4))
             self.remove_from_downloads_lockfile(self.config.type,
                                                 'version',
                                                 version)
@@ skipping 284 matching lines @@
         except Exception as ex:
             logging.error('The build for %s failed:', repo)
             logging.exception(ex)
 
     file = open(nightlyConfigFile, 'wb')
     nightlyConfig.write(file)
 
 
 if __name__ == '__main__':
     parser = argparse.ArgumentParser()
-

[Sebastian Noack, 2018/03/08 02:39:21]

Nit: This blank line seems redundant.

[tlucas, 2018/03/08 09:23:04]

Done.

     parser.add_argument('--download', action='store_true', default=False)
     args = parser.parse_args()
     main(args.download)