Make sure subprocess calls don`t ignore result codes indicating errors. Fix JS docs generation whil…

sitescripts/extensions/bin/createNightlies.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2013 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,
@@ skipping 46 matching lines @@
     self.changelogFilename = None
 
   def hasChanges(self):
     return self.revision != self.previousRevision
 
   def getCurrentRevision(self):
     """
       retrieves the current revision number from the repository
     """
     command = ['hg', 'log', '-R', self.config.repository, '-r', 'default', '--template', '{rev}']
-    (result, dummy) = subprocess.Popen(command, stdout=subprocess.PIPE).communicate()
-    return result
+    return subprocess.check_output(command)
 
   def getChanges(self):
     """
       retrieve changes between the current and previous ("first") revision
     """
 
     command = ['hg', 'log', '-R', self.config.repository, '-r', 'tip:0',
       '-b', 'default', '-l', '50',
       '--template', '{date|isodate}\\0{author|person}\\0{rev}\\0{desc}\\0\\0']
-    (result, dummy) = subprocess.Popen(command, stdout=subprocess.PIPE).communicate()
+    result = subprocess.check_output(command)
 
     for change in result.split('\0\0'):
       if change:
         date, author, revision, description = change.split('\0')
         yield {'date': date, 'author': author, 'revision': revision, 'description': description}
 
   def copyRepository(self):
     '''
       Create a repository copy in a temporary directory
     '''
     # We cannot use hg archive here due to
     # http://bz.selenic.com/show_bug.cgi?id=3747, have to clone properly :-(
     self.tempdir = tempfile.mkdtemp(prefix=self.config.repositoryName)
     command = ['hg', 'clone', '-q', self.config.repository, '-u', 'default', self.tempdir]
-    subprocess.Popen(command).communicate()
+    subprocess.check_call(command)
 
   def writeChangelog(self, changes):
     """
       write the changelog file into the cloned repository
     """
     baseDir = os.path.join(self.config.nightliesDirectory, self.basename)
     if not os.path.exists(baseDir):
       os.makedirs(baseDir)
     changelogFile = "%s-%s.changelog.xhtml" % (self.basename, self.version)
     changelogPath = os.path.join(baseDir, changelogFile)
@@ skipping 151 matching lines @@
     """
     baseDir = os.path.join(self.config.nightliesDirectory, self.basename)
     if not os.path.exists(baseDir):
       os.makedirs(baseDir)
     outputFile = "%s-%s%s" % (self.basename, self.version, self.config.packageSuffix)
     outputPath = os.path.join(baseDir, outputFile)
     self.updateURL = urlparse.urljoin(self.config.nightliesURL, self.basename + '/' + outputFile + '?update')
 
     if self.config.type == 'android':
       apkFile = open(outputPath, 'wb')
+
       try:
-        port = get_config().get('extensions', 'androidBuildPort')
-      except ConfigParser.NoOptionError:
-        port = '22'
-      buildCommand = ['ssh', '-p', port, get_config().get('extensions', 'androidBuildHost')]
-      buildCommand += map(pipes.quote, ['/home/android/bin/makedebugbuild.py', '--revision', self.revision, '--version', self.version, '--stdout'])
-      process = subprocess.Popen(buildCommand, stdout=apkFile, stderr=None)
-      status = process.wait()
-      apkFile.close()
-      if status:
+        try:
+          port = get_config().get('extensions', 'androidBuildPort')
+        except ConfigParser.NoOptionError:

[Sebastian Noack, 2013/07/04 13:57:51]

Will still with NoSectionError, when the section 'extensions' doesn't exist at
all.

[Wladimir Palant, 2013/07/05 11:24:33]

Given that androidBuildHost is mandatory in the extensions section (we'll error
out below otherwise) that should be fine. Not that I'm completely happy with how
we deal with missing settings...

+          port = '22'
+        buildCommand = ['ssh', '-p', port, get_config().get('extensions', 'androidBuildHost')]
+        buildCommand += map(pipes.quote, ['/home/android/bin/makedebugbuild.py', '--revision', self.revision, '--version', self.version, '--stdout'])

[Sebastian Noack, 2013/07/04 13:57:51]

It would be better to use, .extend() instead of +=. The former will inject
additional elements to the original list, in place. While the latter (as used
here) will copy the list, which is unnecessary.

Also, the proper and platform-independent way to build that path would be:

os.path.join(os.path.expanduser('~android'), 'bin' 'makedebugbuild.py')

[Wladimir Palant, 2013/07/05 11:24:33]

Good point about list.extend(), changed that.

os.path.expanduser() won't work because that user doesn't exist on the current
machine - we are passing that command to SSH. I'm not really happy with the way
Andrey hardcoded the command here but then again, I guess it does the job.

+        subprocess.check_call(buildCommand, stdout=apkFile, close_fds=True)
+      except:
         # clear broken output if any
-        # exception will be raised later
         if os.path.exists(outputPath):
           os.remove(outputPath)
+        raise
     elif self.config.type == 'chrome' or self.config.type == 'opera':
       import buildtools.packagerChrome as packager
       packager.createBuild(self.tempdir, type=self.config.type, outFile=outputPath, buildNum=self.revision, keyFile=self.config.keyFile, experimentalAPI=self.config.experimental)
     else:
       import buildtools.packagerGecko as packager
       packager.createBuild(self.tempdir, outFile=outputPath, buildNum=self.revision, keyFile=self.config.keyFile)
 
     if not os.path.exists(outputPath):
       raise Exception("Build failed, output file hasn't been created")
 
@@ skipping 55 matching lines @@
       links.append(link)
     template = get_template(get_config().get('extensions', 'nightlyIndexPage'))
     template.stream({'config': self.config, 'links': links}).dump(outputPath)
 
   def updateDocs(self):
     if not self.config.type == 'gecko':
       return
 
     docsdir = tempfile.mkdtemp(prefix='jsdoc')
     command = ['hg', 'archive', '-R', get_config().get('extensions', 'jsdocRepository'), '-r', 'default', docsdir]
-    subprocess.Popen(command).communicate()
+    subprocess.check_call(command)
 
     try:
+      import buildtools.build as build
       outputPath = os.path.join(self.config.docsDirectory, self.basename)
-      command = ['perl', os.path.join(docsdir, 'jsrun.pl'),
-        '-t=' + os.path.join(docsdir, 'templates', 'jsdoc'),
-        '-d=' + outputPath,
-        '-a',
-        '-p',
-        '-x=js',
-        os.path.join(self.tempdir, 'lib')]
-      subprocess.Popen(command, stdout=subprocess.PIPE).communicate()
+      build.generateDocs(self.tempdir, None, [("-t", docsdir), ("-q", "")], [outputPath], self.config.type)
     finally:
       shutil.rmtree(docsdir, ignore_errors=True)
 
   def run(self):
     """
       Run the nightly build process for one extension
     """
     try:
       if self.config.type == 'ie':
         # We cannot build IE builds, simply list the builds already in
@@ skipping 63 matching lines @@
     except Exception, ex:
       print >>sys.stderr, "The build for %s failed:" % repo
       traceback.print_exc()
 
   file = open(nightlyConfigFile, 'wb')
   nightlyConfig.write(file)
 
 
 if __name__ == '__main__':
   main()