Issue 2956 - Ensure the correct version of jsshell is present

utils.py

 # coding: utf-8
 
 # This Source Code is subject to the terms of the Mozilla Public License
 # version 2.0 (the "License"). You can obtain a copy of the License at
 # http://mozilla.org/MPL/2.0/.
 
+from contextlib import closing
 import os
 import platform
 from StringIO import StringIO
-import shutil
-import subprocess
 import sys
 import urllib
 import zipfile
 
+JSSHELL_DIR = "mozilla-esr31"
+JSSHELL_URL = ("https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly"
+               "/2015/02/2015-02-25-00-22-19-%s/jsshell-%%s.zip" % JSSHELL_DIR)
+
+JSSHELL_SUPPORTED_PLATFORMS = {
+  "win32": "win32",
+  "linux2": {
+    "i686": "linux-i686",
+    "x86_64": "linux-x86_64"
+  },
+  "darwin": "mac"
+}
+
 def ensureJSShell():
   baseDir = os.path.dirname(__file__)
-  shell_dir = os.path.join(baseDir, 'mozilla')
+  shell_dir = os.path.join(baseDir, JSSHELL_DIR)
 
   if not os.path.exists(shell_dir):
     os.makedirs(shell_dir)
   if sys.platform == 'win32':
     path = os.path.join(shell_dir, 'js.exe')
   else:
     path = os.path.join(shell_dir, 'js')
 
   if os.path.exists(path):
-    # Return the path if jsshell is present and its version is correct
-    try:
-      version = [s for s in subprocess.check_output([path, "--help"]).split("\n")

[Sebastian Noack, 2015/11/18 19:53:37]

This can be simplified:

  m = re.search(r"^Version:.*(\d+)", subprocess.check_output([path, "--help"]),
re.MULTILINE)
  if m and m.group(1) == "31":
    ..

Also consider moving the version as well as the URL into a global at the top of
the file to make it easier to find the stuff that need to be changed when
updating.

[kzar, 2015/11/19 10:56:41]

Done.

-                 if s.startswith("Version")][0].split()[1]
-    except (IndexError, subprocess.CalledProcessError):
-      pass
-    else:
-      if version == "JavaScript-C31.5.0esrpre":

[sergei, 2015/11/18 15:30:06]

Nit: I don't know the convention for buildtools but I would read this version
string from some text file. So, the updater of jsshell need to edit this file
instead of python script. Seems also better for the history log.

[sergei, 2015/11/18 15:30:06]

Nit: In addition it seems better to show some warning explaining what happened.
Otherwise, I'm afraid, the updater will have to spend some time investigating
who has reverted just updated version?...

I would not like to overcomplicate it, however I would like to mention the
following dirty solution. The updater should append the line with a new version.
So
if version list is not empty:
  if version is not in version list:
    # instead of downgrading show the warning
    warning("Don't forget to update jsshell.version")
  else if version is not last entry in version list:
    trace("Updating jsshell")
    rmtree...

[kzar, 2015/11/18 15:40:49]

Well I would have separated the version string from the code, but as the
downloaded version is also determined by a hard-coded string in this script I
didn't see the point.

As for showing a warning instead of upgrading jsshell, I disagree. I think that
the behaviour I implemented is more consistent with how it currently works. (Ie
the correct version of jsshell is grabbed magically in the background.)

[sergei, 2015/11/18 15:50:18]

I have missed it, in this case it's totally fine for me and, I guess, we can
omit the downgrading warning.
I meant the warning about downgrading, upgrading can be quite.

-        return path
-    # Otherwise remove the old / faulty version, we'll re-download
-    shutil.rmtree(shell_dir)
-    os.makedirs(shell_dir)
+    return path
 
-  supported_platforms = {
-    'win32': 'win32',
-    'linux2': {
-      'i686': 'linux-i686',
-      'x86_64': 'linux-x86_64'
-    },
-    'darwin': 'mac',
-  }
   try:
-    build = supported_platforms[sys.platform]
+    build = JSSHELL_SUPPORTED_PLATFORMS[sys.platform]
     if isinstance(build, dict):
       build = build[platform.machine()]
   except KeyError:
     raise Exception('Platform %s (%s) not supported by JS shell' % (
       sys.platform, platform.machine()
     ))
 
-  download_url = 'https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/02/2015-02-25-00-22-19-mozilla-esr31/jsshell-%s.zip' % build
-  data = StringIO(urllib.urlopen(download_url).read())
-  zip = zipfile.ZipFile(data)
-  zip.extractall(shell_dir)
-  zip.close()
+  with closing(urllib.urlopen(JSSHELL_URL % build)) as response, \
+       zipfile.ZipFile(StringIO(response.read())) as zip:
+    zip.extractall(shell_dir)
 
   if not os.path.exists(path):
     raise Exception('Downloaded package didn\'t contain JS shell executable')
 
   try:
     os.chmod(path, 0700)
   except:
     pass
 
   return path