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

sitescripts/extensions/bin/createNightlies.py

Index: sitescripts/extensions/bin/createNightlies.py
===================================================================
--- a/sitescripts/extensions/bin/createNightlies.py
+++ b/sitescripts/extensions/bin/createNightlies.py
@@ -59,43 +59,42 @@ class NightlyBuild(object):
   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)
@@ -257,30 +256,30 @@ class NightlyBuild(object):
     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):
@@ -346,28 +345,22 @@ class NightlyBuild(object):
     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: